Incorrect ForkId when first fork timestamp matches genesis timestamp #5744
Description
Credit to Antithesis and Nethermind teams for the report...
Problem
For dev/testnets, if you start a network with shanghaiTimestamp as the first fork and the timestamp value matches the genesis block's timestamp, then we incorrectly create an extra ForkId for "genesis at block 0". This errant ForkId feeds into the next one and results in having the incorrect ForkId for cancunTimestamp, therefore we cannot connect to peers.
Besu
fork0 = CRC32(<genesis-hash>)
fork1 = CRC32(<genesis-hash> | uint64(shanghai_timestamp))
fork2 = CRC32(<genesis-hash> | uint64(shanghai_timestamp) | uint64(cancun_timestamp))
Neth
fork`0 = CRC32(<genesis-hash>)
fork`1 = CRC32(<genesis-hash> | uint64(cancun_timestamp))
fork1 != fork`0
fork2 != fork`1
e.g. with genesis.timestamp = 1, shanghaiTimestamp = 1, cancunTimestamp = 2
We get these ForkIds
[ForkId(hash=0x9fe57a35, next=1),
ForkId(hash=0x82cf0773, next=2),
ForkId(hash=0x47cc592b, next=0)]
but it should be
[ForkId(hash=0x9fe57a35, next=2),
ForkId(hash=0x1bc656c9, next=0)]
The Fix
Currently POC'd a fix:
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/forkid/ForkIdManager.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/forkid/ForkIdManager.java
@@ -70,7 +70,7 @@ public class ForkIdManager {
.collect(Collectors.toUnmodifiableList());
this.timestampForks =
timestampForks.stream()
- .filter(fork -> fork > 0L)
+ .filter(fork -> fork > blockchain.getGenesisBlock().getHeader().getTimestamp())
.distinct()
.sorted()
but will need some testing.
A Workaround
The workaround is to make genesis timestamp and shanghaiTimestamp different values. This is the case on the 4844 devnets https://github.com/ethpandaops/4844-testnet/blob/master/network-configs/devnet-7/genesis.json