l2geth: use minimal EIP 2929 #1568
Conversation
🦋 Changeset detectedLatest commit: ba65c54 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report
@@ Coverage Diff @@
## experimental #1568 +/- ##
================================================
- Coverage 74.20% 73.62% -0.58%
================================================
Files 67 67
Lines 2210 2207 -3
Branches 321 324 +3
================================================
- Hits 1640 1625 -15
- Misses 570 582 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
l2geth/core/vm/eips.go
Outdated
| // Use minimal eip2929 | ||
| if rcfg.UsingOVM { | ||
| jt[SLOAD].constantGas = params.WarmStorageReadCostEIP2929 | ||
| jt[SLOAD].dynamicGas = gasSLoadEIP2929 |
There was a problem hiding this comment.
From here down it's identical to https://github.com/ethereum/go-ethereum/blob/011fe3eb5e29e246ba14e7c9675c536cf7123305/core/vm/eips.go#L118-L142
Except SELFDESTRUCT: https://github.com/ethereum/go-ethereum/blob/011fe3eb5e29e246ba14e7c9675c536cf7123305/core/vm/eips.go#L144-L147
Why?
There was a problem hiding this comment.
I didn't port the selfdestruct changes because I was thinking that they would be out of scope for this set of changes. We could port them as well if you are opinionated, it would require bringing this function into the codebase and removing the accesslist based logic
https://github.com/ethereum/go-ethereum/blob/a5669ae292274d3b0a223cda07ebe64f943bded4/core/vm/operations_acl.go#L229
There was a problem hiding this comment.
I ended up porting the self destruct change as well
l2geth/core/vm/eips.go
Outdated
| return params.ColdAccountAccessCostEIP2929 - params.WarmStorageReadCostEIP2929, nil | ||
| } | ||
|
|
||
| func makeCallVariantGasCallEIP2929(oldCalculator gasFunc) gasFunc { |
There was a problem hiding this comment.
In Geth, this logic is included in vm/operations_acl.go. Might as well create a file with the same name and move this logic there to simplify the future porting process.
There was a problem hiding this comment.
I've moved functionality to this file
tynes
force-pushed
the
l2geth/minimal-eip2929
branch
2 times, most recently
from
26b205d
to
b4a045b
Compare
Use a minimal version of EIP 2929 for state accessing opcodes. This does not pull in the access list logic but instead charges based on cold access assumptions at all times
tynes
force-pushed
the
l2geth/minimal-eip2929
branch
from
b4a045b
to
ba65c54
Compare
Description
Use a minimal version of EIP 2929 for state accessing
opcodes. This does not pull in the access list logic
but instead charges based on cold access assumptions
at all times