params: core: enable shanghai based on timestamps #25878
Conversation
MariusVanDerWijden
requested review from
holiman,
karalabe,
rjl493456442 and
s1na
as code owners
params/config.go
Outdated
There was a problem hiding this comment.
| func (c *ChainConfig) IsShanghai(time *big.Int) bool { | |
| func (c *ChainConfig) IsShanghai(time uint64) bool { |
It would be nice if either this took a uint64 or if we don't want IsShanghai to take a different parameter type than other isForked methods, maybe a new getter on types.Block and types.Header "TimeBig()" so that each time we check IsShanghai, don't need to jump through a bunch of casting hoops?
There was a problem hiding this comment.
It might be better to have a different type here to detect at compile time if someone is trying to pass in a block number instead of a timestamp. e.g. there are calls to IsShanghai() in the withdrawals PR and EIP-4844 repo that would need to be updated but would not be flagged by the compiler.
karalabe
force-pushed
the
shanghai-by-time
branch
from
2d85996 to
1cd8782
Compare
Starting from [Shanghai](https://github.com/ethereum/execution-specs/blob/master/network-upgrades/mainnet-upgrades/shanghai.md), forks are based on timestamps rather than block heights (see PR #6238). This PR extends [EIP-2124](https://eips.ethereum.org/EIPS/eip-2124) Fork ID to include timestamp-based blocks. See also ethereum/go-ethereum#25878. Co-authored-by: Marius van der Wijden <[email protected]>
|
Forkid filtering is broken in this pr, needs MariusVanDerWijden#42 |
karalabe
force-pushed
the
shanghai-by-time
branch
from
495ffd7 to
a2d3873
Compare
karalabe
force-pushed
the
shanghai-by-time
branch
2 times, most recently
from
1bbb82c to
ca4002e
Compare
karalabe
left a comment
karalabe
left a comment
There was a problem hiding this comment.
SGTM, but I made a ton of changes, so would appreciate a review from someone else now.
MariusVanDerWijden
left a comment
MariusVanDerWijden
left a comment
There was a problem hiding this comment.
LGTM, reviewed @karalabe's latest changes
lightclient
left a comment
lightclient
left a comment
There was a problem hiding this comment.
Took a look at the new changes and tests, generally all seem good.
karalabe
force-pushed
the
shanghai-by-time
branch
from
ca4002e to
3669ca6
Compare
rjl493456442
left a comment
rjl493456442
left a comment
There was a problem hiding this comment.
Mostly looks good to me!
karalabe
force-pushed
the
shanghai-by-time
branch
from
3669ca6 to
754f1b0
Compare
karalabe
force-pushed
the
shanghai-by-time
branch
from
ae1330d to
b56c796
Compare
* geth switched to time-based forking in ethereum/go-ethereum#25878, this changes the name of the `shanghai_block` field to `shanghaiTime` so it is compatible with geth.
* geth switched to time-based forking in ethereum/go-ethereum#25878, this changes the name of the `shanghai_block` field to `shanghaiTime` so it is compatible with geth.
* geth switched to time-based forking in ethereum/go-ethereum#25878, this changes the name of the `shanghai_block` field to `shanghaiTime` so it is compatible with geth.
6 participants
This PR changes Shanghai activation to be based on timestamps instead of block numbers