Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

evm: add error which triggers if code size deposit exceeds the maximum size #2239

Merged
merged 2 commits into from Aug 29, 2022

Conversation

jochem-brouwer
Copy link
Member

Closes #2065

Adds a new error type to EVM, which allows EVM consumers to figure out the difference between (Homestead or later) contract creation goes OOG, or if the deposited code is too large (exceeds maximum code size).

Checked other errors too, but did not find anything "weird" there.

CC @alcuadrado

@jochem-brouwer jochem-brouwer added PR state: needs review type: test all hardforks This special label enables VM state and blockchain tests for all hardforks on the respective PR. package: evm labels Aug 26, 2022
@codecov
Copy link

codecov bot commented Aug 26, 2022

Codecov Report

Merging #2239 (ba549ee) into master (c70a158) will decrease coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

Flag Coverage Δ
block 92.77% <ø> (ø)
blockchain 88.39% <ø> (ø)
client 87.07% <ø> (?)
common 98.09% <ø> (ø)
devp2p 92.25% <ø> (ø)
ethash ?
evm 79.11% <100.00%> (+0.02%) ⬆️
rlp ∅ <ø> (∅)
statemanager 88.16% <ø> (ø)
trie 89.43% <ø> (ø)
tx 97.98% <ø> (ø)
util 92.33% <ø> (ø)
vm 85.27% <ø> (ø)

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

@holgerd77
Copy link
Member

Rebased this via UI.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Yeah, I also stumbled upon this strange combination at some point, nice improvement (also somewhat breaking actually, but so be it, small and local enough to still let in I guess, even from RC to final).

@holgerd77 holgerd77 merged commit de21d49 into master Aug 29, 2022
@holgerd77 holgerd77 deleted the evm-error-msgs branch August 29, 2022 15:11
@alcuadrado
Copy link
Member

Late to the party, but thanks for this @jochem-brouwer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: evm PR state: needs review type: test all hardforks This special label enables VM state and blockchain tests for all hardforks on the respective PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review EVM error handling messages
3 participants