Fix ForkId compatible bug (#3796)

* Fix ForkId compatible bug

* Cleanup + remove additional test for other PR as it is still faulty

Author: kdeme

execution_chain/common/hardforks.nim

@@ -351,6 +351,7 @@ type
     cache: seq[ForkID]
 
 func newID*(calc: ForkIdCalculator, head, time: uint64): ForkID =
+  # Create a fork ID for a specific block height and timestamp:
   var crc = calc.genesisCRC
   for fork in calc.byBlock:
     if fork <= head:
@@ -370,15 +371,21 @@ func newID*(calc: ForkIdCalculator, head, time: uint64): ForkID =
 
 func compatible*(calc: var ForkIdCalculator, forkId: ForkID): bool =
   if calc.cache.len == 0:
-    calc.cache = newSeqOfCap[ForkID](calc.byBlock.len + calc.byTime.len)
+    calc.cache = newSeqOfCap[ForkID](calc.byBlock.len + calc.byTime.len + 1)
     var crc = calc.genesisCRC
+
+    # Build cache of all valid ForkIds
+    # Each entry is (crc before fork, fork number)
     for fork in calc.byBlock:
-      crc = crc32(crc, fork.toBytesBE)
       calc.cache.add( (crc, fork) )
+      crc = crc32(crc, fork.toBytesBE)
 
     for fork in calc.byTime:
-      crc = crc32(crc, fork.toBytesBE)
       calc.cache.add( (crc, fork) )
+      crc = crc32(crc, fork.toBytesBE)
+
+    # Add last fork ID (after all forks, next=0)
+    calc.cache.add( (crc, 0'u64) )
 
   for id in calc.cache:
     if id == forkId:
@@ -389,6 +396,7 @@ func compatible*(calc: var ForkIdCalculator, forkId: ForkID): bool =
 func initForkIdCalculator*(map: ForkTransitionTable,
                            genesisCRC: uint32,
                            genesisTime: uint64): ForkIdCalculator =
+  # Build ForkIdCalculator from the chain's fork configuration
 
   # Extract the fork rule block number aggregate it
   var forksByBlock: seq[uint64]

execution_chain/networking/eth1_discovery.nim

@@ -104,9 +104,12 @@ func eligibleNode(proto: Eth1Discovery, rec: Record): bool =
     return true
 
   let
-    chainForkIds = try: rlp.decode(bytes, array[1, ChainForkId])
-              except RlpError: return false
-    chainForkId  = chainForkIds[0]
+    ethValue =
+      try:
+        rlp.decode(bytes, array[1, ChainForkId])
+      except RlpError:
+        return false
+    chainForkId  = ethValue[0]
 
   proto.compatibleForkId(chainForkId.to(ForkID))
 

tests/test_forkid.nim

@@ -83,16 +83,19 @@ const
     (number: 123'u64, time: 1762955545'u64, id: (crc: 0x23AA1351'u32, next: 0'u64)),          # Future BPO2 time
   ]
 
-template runTest(network: untyped, name: string) =
+template runForkIdTest(network: untyped, name: string) =
   test name:
     var
       params = networkParams(network)
       com    = CommonRef.new(newCoreDbRef DefaultDbMemory, nil, network, params)
 
-    for i, x in `network IDs`:
-      let id = com.forkId(x.number, x.time)
-      check toHex(id.crc) == toHex(x.id.crc)
-      check id.nextFork == x.id.next
+    for x in `network IDs`:
+      let computedId = com.forkId(x.number, x.time)
+      check toHex(computedId.crc) == toHex(x.id.crc)
+      check computedId.nextFork == x.id.next
+
+      # The computed ID should be compatible with the CommonRef ForkIdCalculator itself
+      check com.compatibleForkId(computedId) == true
 
 func config(shanghai, cancun: uint64): ChainConfig =
   ChainConfig(
@@ -143,8 +146,8 @@ template runGenesisTimeIdTests() =
     check get.nextFork == x.want.next
 
 suite "Fork ID tests":
-  runTest(MainNet, "MainNet")
-  runTest(SepoliaNet, "SepoliaNet")
-  runTest(HoodiNet, "HoodiNet")
+  runForkIdTest(MainNet, "MainNet")
+  runForkIdTest(SepoliaNet, "SepoliaNet")
+  runForkIdTest(HoodiNet, "HoodiNet")
   test "Genesis Time Fork ID":
     runGenesisTimeIdTests()