-
Notifications
You must be signed in to change notification settings - Fork 324
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] implement EIP-3529 #3226
[evm] implement EIP-3529 #3226
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3226 +/- ##
==========================================
- Coverage 74.97% 74.95% -0.02%
==========================================
Files 228 228
Lines 21367 21373 +6
==========================================
+ Hits 16019 16020 +1
- Misses 4488 4493 +5
Partials 860 860
Continue to review full report at Codecov.
|
@@ -395,7 +398,11 @@ func executeInEVM(ctx context.Context, evmParams *Params, stateDB *StateDBAdapte | |||
if stateDB.Error() != nil { | |||
log.L().Debug("statedb error", zap.Error(stateDB.Error())) | |||
} | |||
refund := (evmParams.gas - remainingGas) / 2 | |||
if !london { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is old code (evmParams.gas - remainingGas) / 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
next line
params.RefundQuotient = 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if params.RefundQuotient
is changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check Ethereum implementation: ethereum/go-ethereum#22733
Ethereum needs to keep the change backward-compatible (just like us), so this won't change, otherwise past blocks won't be able to sync?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. Could you add the comment like theirs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -395,7 +398,11 @@ func executeInEVM(ctx context.Context, evmParams *Params, stateDB *StateDBAdapte | |||
if stateDB.Error() != nil { | |||
log.L().Debug("statedb error", zap.Error(stateDB.Error())) | |||
} | |||
refund := (evmParams.gas - remainingGas) / 2 | |||
if !london { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. Could you add the comment like theirs?
@@ -365,8 +366,10 @@ func executeInEVM(ctx context.Context, evmParams *Params, stateDB *StateDBAdapte | |||
var ( | |||
contractRawAddress = action.EmptyAddress | |||
executor = vm.AccountRef(evmParams.txCtx.Origin) | |||
london = evm.ChainConfig().IsLondon(evm.Context.BlockNumber) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLondon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
london
is used in our go-ethereum v0.4.0, would like to keep it the same
Description
here's the Ethereum implementation: ethereum/go-ethereum#22733
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist: