-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix(primitives): remove Hardfork::Eip150, Eip158 #926
Conversation
* removes Hardfork::Eip150 and Hardfork::Eip158, going back to Hardfork::Tangerine. * the From<EthersGenesis> for ChainSpec impl is now only determined by the value of eip_150_fork_block. the value of eip_158_fork_block is ignored.
Codecov Report
@@ Coverage Diff @@
## main #926 +/- ##
==========================================
- Coverage 74.05% 74.00% -0.05%
==========================================
Files 292 292
Lines 31722 31716 -6
==========================================
- Hits 23493 23473 -20
- Misses 8229 8243 +14
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
When was EIP158 included, seems weird to ignore it? If it's part of tangerine can we remove it? Should we call the Config blocks by the hard fork name? Instead of calling it eip150 block? |
EIP158 was in fact succeeded by EIP161 (as @rakita said), and only EIP150 was included in tangerine: However, the flag still exists in geth's This is an inner struct as part of geth's Which means geth's Geth checks for these fields on startup if we want later forks to be activated (such as EIP155 or EIP1559), and will not start unless previous forks are activated. Additionally, this also still refers to EIP150 and EIP158, instead of Tangerine: So we can't remove it from the ethers
I ran into this when trying to configure clique - rather than the chainid being incorrect, EIP-155 was not active.
We can use fork names on reth's end, and in our own configuration formats, if we want to deviate from geth's |
Understood. Appreciate the really detailed answer. |
In essence, the granularity of EIP's activation is different, it shouldn't be a problem as this is old change from 2016 and most ethereum like chains activated it from genesis. Little detail here, EIP-150 is Tangerine but EIP-155 and EIP-161 (Supperseeds Rinkeby activated Tangerine at block 2 and SpuriousDragon at block 3: https://github.com/ethereum/go-ethereum/blob/a35b654f25fdac518d5775b55aa064025040b01a/params/config.go#L184-L187 Goerly has them active from genesis: https://github.com/ethereum/go-ethereum/blob/a35b654f25fdac518d5775b55aa064025040b01a/params/config.go#L228-L231 Either way, calling it EIP is the legacy thing that we don't need to backport from geth. |
This removes
Hardfork::Eip150
andHardfork::Eip158
, going back toHardfork::Tangerine
. TheFrom<EthersGenesis> for ChainSpec
impl is now only determined by the value ofeip_150_fork_block
. The value ofeip_158_fork_block
is ignored.