fix(primitives): remove Hardfork::Eip150, Eip158

[Rjected]

This 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-commenter]

Codecov Report

Merging #926 (0205149) into main (82b10fa) will decrease coverage by 0.05%.
The diff coverage is 29.41%.

@@            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     

Flag	Coverage Δ
unit-tests	74.00% <29.41%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
crates/primitives/src/hardfork.rs	36.95% <0.00%> (+0.78%)	⬆️
crates/primitives/src/chain_spec.rs	73.79% <80.00%> (-0.02%)	⬇️
crates/executor/src/config.rs	98.59% <100.00%> (-0.02%)	⬇️
crates/transaction-pool/src/test_utils/mock.rs	53.11% <0.00%> (-6.60%)	⬇️
crates/transaction-pool/src/pool/txpool.rs	58.87% <0.00%> (-0.50%)	⬇️
crates/net/network/src/manager.rs	49.79% <0.00%> (-0.21%)	⬇️
crates/net/network/src/peers/manager.rs	83.42% <0.00%> (+0.18%)	⬆️
crates/net/network/src/session/active.rs	84.65% <0.00%> (+0.36%)	⬆️
crates/stages/src/stages/bodies.rs	93.47% <0.00%> (+0.38%)	⬆️
crates/net/eth-wire/src/types/version.rs	80.00% <0.00%> (+2.22%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[gakonst]

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?

[Rjected]

When was EIP158 included, seems weird to ignore it? If it's part of tangerine can we remove it?

EIP158 was in fact succeeded by EIP161 (as @rakita said), and only EIP150 was included in tangerine:
ethereum/EIPs#158 (comment)

However, the flag still exists in geth's ChainConfig:
https://github.com/ethereum/go-ethereum/blob/a35b654f25fdac518d5775b55aa064025040b01a/params/config.go#L68-L69

This is an inner struct as part of geth's Genesis struct:
https://github.com/ethereum/go-ethereum/blob/a35b654f25fdac518d5775b55aa064025040b01a/core/genesis.go#L50

Which means geth's genesis.json format (which we use to initialize ethers_core::utils::Geth with custom settings like clique configs) does not support any tangerine_block field, and instead still uses eip_150_block and eip_158_block.

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:
https://github.com/ethereum/go-ethereum/blob/a35b654f25fdac518d5775b55aa064025040b01a/params/config.go#L627-L677

So we can't remove it from the ethers ChainConfig format. From the above geth code, checkConfigForkOrder first checks for eip150Block, then eip155Block, then eip158Block. If we removed eip_150_block from the ethers ChainConfig struct, we would not be able to activate EIP-150, and would not be able to activate EIP-155 or any future forks. For example, we would get invalid sender errors when producing transactions for clique blocks if:

• EIP-155 is not active AND
• We do not explicitly modify the signature after signing, to get rid of the chainid from the v before sending.

I ran into this when trying to configure clique - rather than the chainid being incorrect, EIP-155 was not active.

Should we call the Config blocks by the hard fork name? Instead of calling it eip150 block?

We can use fork names on reth's end, and in our own configuration formats, if we want to deviate from geth's genesis.json format. But when we are working with custom geth-compatible genesis configs, we definitely can't, unless the field is already a fork name - eip_150_block, eip_155_block, and eip_158_block are outliers in that respect.

[gakonst]

Understood. Appreciate the really detailed answer.

[rakita]

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 EIP-158) is SpuriousDragon, those are different hardforks. https://github.com/ethereum/execution-specs

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.