Skip to content

Conversation

@jochem-brouwer
Copy link
Member

In our EIP 3860 implementation, we have not added this rule:

"Furthermore, we introduce a charge of 2 gas for every 32-byte chunk of initcode to represent the cost of jumpdest-analysis."

This /does/ happen at CREATEs at the tx-level, but it does not happen if we have a CREATE or a CREATE2

This also passes the current ethereum/tests tests for EIP 3860.

To check:

git checkout update-3860
npm run clean && npm i
cd packages/ethereum-tests
gh pr checkout 990
cd ../vm
npm run test:blockchain -- --fork=London+3860
npm run test:state -- --fork=London+3860

All pass.

Note: problem found by @pk910 in block 6601 on the Shandong testnet.

@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Merging #2397 (e217a83) into master (5776107) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 91.37% <ø> (ø)
blockchain 90.21% <ø> (ø)
client 87.39% <ø> (ø)
common 98.13% <ø> (ø)
devp2p 91.57% <ø> (-0.06%) ⬇️
ethash ∅ <ø> (∅)
evm 79.72% <100.00%> (+0.22%) ⬆️
rlp ∅ <ø> (∅)
statemanager 89.46% <ø> (ø)
trie 90.36% <ø> (ø)
tx 97.80% <ø> (ø)
util 83.89% <ø> (ø)
vm 85.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@acolytec3 acolytec3 merged commit cd551ec into master Nov 3, 2022
@acolytec3 acolytec3 deleted the update-3860 branch November 3, 2022 16:41
@gitpoap-bot
Copy link

gitpoap-bot bot commented Nov 3, 2022

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2022 EthereumJS Contributor:

GitPOAP: 2022 EthereumJS Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

)
st.end()
})
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice test setup! 👍

jochem-brouwer added a commit that referenced this pull request Nov 4, 2022
* evm: fix 3860 implementation + tests

* Adapt original EIP-3860 tests from vm

* add test for Create2

* Add test for CREATE

Co-authored-by: acolytec3 <[email protected]>
@jochem-brouwer jochem-brouwer mentioned this pull request Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants