Fix precompile address checks & value bearing witness cost charging #412
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… value-bearing rule Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
jsign
commented
Apr 8, 2024
Comment on lines
+450
to
+451
| if transfersValue { | ||
| gas, overflow = math.SafeAdd(gas, evm.Accesses.TouchAndChargeValueTransfer(contract.Address().Bytes()[:], address.Bytes()[:])) |
There was a problem hiding this comment.
Charging this was missing?
| @@ -73,8 +78,10 @@ func makeCallVariantGasEIP4762(oldCalculator gasFunc) gasFunc { | |||
| if err != nil { | |||
| return 0, err | |||
| } | |||
| wgas := evm.Accesses.TouchCodeSize(contract.Address().Bytes(), false) | |||
| wgas += evm.Accesses.TouchCodeHash(contract.Address().Bytes(), false) | |||
There was a problem hiding this comment.
Remove this wrong CodeHash touching.
| if _, isPrecompile := evm.precompile(contract.Address()); isPrecompile { | ||
| return gas, nil | ||
| } | ||
| wgas := evm.Accesses.TouchAndChargeMessageCall(contract.Address().Bytes()) |
There was a problem hiding this comment.
Touch version and codesize.
Comment on lines
+101
to
+103
| if _, isPrecompile := evm.precompile(beneficiaryAddr); isPrecompile { | ||
| return 0, nil | ||
| } |
gballet
approved these changes
Apr 9, 2024
Comment on lines
+450
to
+451
| if transfersValue { | ||
| gas, overflow = math.SafeAdd(gas, evm.Accesses.TouchAndChargeValueTransfer(contract.Address().Bytes()[:], address.Bytes()[:])) |
gballet
pushed a commit
that referenced
this pull request
Apr 10, 2024
… value-bearing rule (#412) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
gballet
added a commit
that referenced
this pull request
Apr 15, 2024
* simplified gas accounting layer * integrate some review feedback * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * more suggestions from code review * don't charge creation gas + charge code chunks in create * A couple more fixes * make linter happy * fix create init gas consumption issue * fix: in gas funcs, use tx witness instead of global witness * fix linter issue * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix: EXTCODECOPY gas consumption * fix warm gas costs * fix the order gas is charged in during contract creation epilogue * fix selfdestruct * fix #365 in eip rewrite (#407) * fix: OOG type in code creation OOG (#408) * core/vm: charge BLOCKHASH witness cost (#409) * core/vm: charge BLOCKHASH witness cost Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove gas optimization for now Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove redundant logic for contract creation (#413) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix precompile address check for charging witness costs & fix missing value-bearing rule (#412) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * core/vm: fix wrong check (#416) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * charge for account creation if selfdestruct creates a new account (#417) * add key comparison test (#418) * core/vm: charge contract init before execution logic (#419) * core/vm: charge contract init before execution logic Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix CREATE2 as well --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> * quell linter --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com>
gballet
added a commit
that referenced
this pull request
May 7, 2024
* simplified gas accounting layer * integrate some review feedback * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * more suggestions from code review * don't charge creation gas + charge code chunks in create * A couple more fixes * make linter happy * fix create init gas consumption issue * fix: in gas funcs, use tx witness instead of global witness * fix linter issue * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix: EXTCODECOPY gas consumption * fix warm gas costs * fix the order gas is charged in during contract creation epilogue * fix selfdestruct * fix #365 in eip rewrite (#407) * fix: OOG type in code creation OOG (#408) * core/vm: charge BLOCKHASH witness cost (#409) * core/vm: charge BLOCKHASH witness cost Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove gas optimization for now Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove redundant logic for contract creation (#413) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix precompile address check for charging witness costs & fix missing value-bearing rule (#412) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * core/vm: fix wrong check (#416) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * charge for account creation if selfdestruct creates a new account (#417) * add key comparison test (#418) * core/vm: charge contract init before execution logic (#419) * core/vm: charge contract init before execution logic Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix CREATE2 as well --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> * quell linter --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com>
gballet
added a commit
that referenced
this pull request
May 8, 2024
* simplified gas accounting layer * integrate some review feedback * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * more suggestions from code review * don't charge creation gas + charge code chunks in create * A couple more fixes * make linter happy * fix create init gas consumption issue * fix: in gas funcs, use tx witness instead of global witness * fix linter issue * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix: EXTCODECOPY gas consumption * fix warm gas costs * fix the order gas is charged in during contract creation epilogue * fix selfdestruct * fix #365 in eip rewrite (#407) * fix: OOG type in code creation OOG (#408) * core/vm: charge BLOCKHASH witness cost (#409) * core/vm: charge BLOCKHASH witness cost Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove gas optimization for now Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove redundant logic for contract creation (#413) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix precompile address check for charging witness costs & fix missing value-bearing rule (#412) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * core/vm: fix wrong check (#416) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * charge for account creation if selfdestruct creates a new account (#417) * add key comparison test (#418) * core/vm: charge contract init before execution logic (#419) * core/vm: charge contract init before execution logic Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix CREATE2 as well --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> * quell linter --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com>
gballet
added a commit
that referenced
this pull request
May 8, 2024
* simplified gas accounting layer * integrate some review feedback * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * more suggestions from code review * don't charge creation gas + charge code chunks in create * A couple more fixes * make linter happy * fix create init gas consumption issue * fix: in gas funcs, use tx witness instead of global witness * fix linter issue * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix: EXTCODECOPY gas consumption * fix warm gas costs * fix the order gas is charged in during contract creation epilogue * fix selfdestruct * fix #365 in eip rewrite (#407) * fix: OOG type in code creation OOG (#408) * core/vm: charge BLOCKHASH witness cost (#409) * core/vm: charge BLOCKHASH witness cost Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove gas optimization for now Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove redundant logic for contract creation (#413) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix precompile address check for charging witness costs & fix missing value-bearing rule (#412) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * core/vm: fix wrong check (#416) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * charge for account creation if selfdestruct creates a new account (#417) * add key comparison test (#418) * core/vm: charge contract init before execution logic (#419) * core/vm: charge contract init before execution logic Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix CREATE2 as well --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> * quell linter --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com>
gballet
added a commit
that referenced
this pull request
May 8, 2024
* simplified gas accounting layer * integrate some review feedback * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * more suggestions from code review * don't charge creation gas + charge code chunks in create * A couple more fixes * make linter happy * fix create init gas consumption issue * fix: in gas funcs, use tx witness instead of global witness * fix linter issue * Apply suggestions from code review Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix: EXTCODECOPY gas consumption * fix warm gas costs * fix the order gas is charged in during contract creation epilogue * fix selfdestruct * fix #365 in eip rewrite (#407) * fix: OOG type in code creation OOG (#408) * core/vm: charge BLOCKHASH witness cost (#409) * core/vm: charge BLOCKHASH witness cost Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove gas optimization for now Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * remove redundant logic for contract creation (#413) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix precompile address check for charging witness costs & fix missing value-bearing rule (#412) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * core/vm: fix wrong check (#416) Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * charge for account creation if selfdestruct creates a new account (#417) * add key comparison test (#418) * core/vm: charge contract init before execution logic (#419) * core/vm: charge contract init before execution logic Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> * fix CREATE2 as well --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> * quell linter --------- Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com> Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR: