Rework ForkId to get rid of ChainForkId and its conversions (#3810)

* Rework ForkId to get rid of ChainForkId and its conversions

And be more consistent in the naming + use fields as per EIPs

Author: kdeme

execution_chain/common/common.nim

@@ -350,20 +350,20 @@ func isLondonOrLater*(com: CommonRef, number: BlockNumber): bool =
   # TODO: Fixme, use only London comparator
   com.toHardFork(number.forkDeterminationInfo) >= London
 
-func forkId*(com: CommonRef, head, time: uint64): ForkID {.gcsafe.} =
-  ## Get ForkId for given block number / timestamp (EIP 2364/2124)
-  com.forkIdCalculator.newID(head, time)
+func forkId*(com: CommonRef, head, time: uint64): ForkId {.gcsafe.} =
+  ## Get ForkId for given block number / timestamp (EIP-2124/2364/6122)
+  com.forkIdCalculator.calculateForkId(head, time)
 
-func forkId*(com: CommonRef, forkActivationTime: EthTime): ForkID {.gcsafe.} =
-  ## Get ForkId for given timestamp (EIP 2364/2124)
+func forkId*(com: CommonRef, forkActivationTime: EthTime): ForkId {.gcsafe.} =
+  ## Get ForkId for given timestamp (EIP-2124/2364/6122)
   ## Only works for timestamp based forks
-  com.forkIdCalculator.newID(0'u64, forkActivationTime.uint64)
+  com.forkIdCalculator.calculateForkId(0'u64, forkActivationTime.uint64)
 
-func forkId*(com: CommonRef, head: BlockNumber, time: EthTime): ForkID {.gcsafe.} =
-  ## Get ForkId for given block number / timestamp (EIP-2124 + EIP-6122)
-  com.forkIdCalculator.newID(head, time.uint64)
+func forkId*(com: CommonRef, head: BlockNumber, time: EthTime): ForkId {.gcsafe.} =
+  ## Get ForkId for given block number / timestamp (EIP-2124/2364/6122)
+  com.forkIdCalculator.calculateForkId(head, time.uint64)
 
-func compatibleForkId*(com: CommonRef, forkId: ForkID, blockNumber: BlockNumber, time: EthTime): bool =
+func compatibleForkId*(com: CommonRef, forkId: ForkId, blockNumber: BlockNumber, time: EthTime): bool =
   ## Check if a fork ID is compatible at a specific head position
   com.forkIdCalculator.compatible(forkId, blockNumber, time.uint64)
 

execution_chain/common/hardforks.nim

@@ -10,13 +10,15 @@
 {.push raises: [].}
 
 import
-  std/[strutils],
-  eth/common/[headers],
+  std/strutils,
+  eth/common/[base, headers],
   stew/endians2,
   json_serialization,
   ../utils/utils,
   ./evmforks
 
+export base
+
 type
   HardFork* = enum
     Frontier
@@ -348,40 +350,40 @@ type
     byBlock: seq[uint64]
     byTime: seq[uint64]
     genesisCRC: uint32
-    forkHistory: seq[ForkID]
+    forkHistory: seq[ForkId]
 
-func newID*(calc: ForkIdCalculator, head, time: uint64): ForkID =
+func calculateForkId*(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:
       # Fork already passed, checksum the previous crc and the fork number
       crc = crc32(crc, fork.toBytesBE)
       continue
-    return (crc, fork)
+    return ForkId(hash: crc.to(Bytes4), next: fork)
 
   for fork in calc.byTime:
     if fork <= time:
       # Fork already passed, checksum the previous crc and fork timestamp
       crc = crc32(crc, fork.toBytesBE)
       continue
-    return (crc, fork)
+    return ForkId(hash: crc.to(Bytes4), next: fork)
 
-  (crc, 0'u64)
+  ForkId(hash: crc.to(Bytes4), next: 0'u64)
 
-func compatible*(calc: ForkIdCalculator, forkId: ForkID, number: uint64, time: uint64): bool =
+func compatible*(calc: ForkIdCalculator, forkId: ForkId, number: uint64, time: uint64): bool =
   ## Check forkId compatibility at a specific head position according to EIP-2124 / EIP-6122.
   ## The number and time represent the local chain state at which compatibility needs to be checked
   ## In regular use this should be the current header block number and timestamp, but for testing
   ## arbitrary points in history can be used.
 
   # Calculate our current local fork ID at the given head position (= block and/or time)
-  let localId = calc.newID(number, time)
+  let localId = calc.calculateForkId(number, time)
 
   # Calculate position of local fork ID in history
   var localForkPos = -1
   for i, historicalId in calc.forkHistory:
-    if localId.crc == historicalId.crc:
+    if localId.hash == historicalId.hash:
       localForkPos = i
       break
 
@@ -393,9 +395,9 @@ func compatible*(calc: ForkIdCalculator, forkId: ForkID, number: uint64, time: u
       number
 
   # 1: local and remote FORK_HASH matches
-  if forkId.crc == localId.crc:
-    if forkId.nextFork != 0: # announced fork
-      if head >= forkId.nextFork:
+  if forkId.hash == localId.hash:
+    if forkId.next != 0: # announced fork
+      if head >= forkId.next:
         # 1a - next fork already passed locally
         return false
       else:
@@ -408,19 +410,19 @@ func compatible*(calc: ForkIdCalculator, forkId: ForkID, number: uint64, time: u
 
   # 2 + 3: FORK_HASH is subset or superset of local past forks
   for i, historicalId in calc.forkHistory:
-    if forkId.crc == historicalId.crc:
+    if forkId.hash == historicalId.hash:
       # Need to know if remote (=forkId) is ahead or behind localId
       if i > localForkPos:
         # 3: Remote is at a future fork we also know about
         # (local is syncing)
         return true
         # TODO: Should we still check its next fork?
-      if forkId.nextFork == historicalId.nextFork:
-        # 2: Remote is at a past fork we also know about and its nextFork matches the one for that past fork
+      if forkId.next == historicalId.next:
+        # 2: Remote is at a past fork we also know about and its next fork matches the one for that past fork
         # (remote is syncing)
         return true
       else:
-        # 4: Remote is at a past fork we also know about but its nextFork doesn't match
+        # 4: Remote is at a past fork we also know about but its next fork doesn't match
         return false
 
   # 4: incompatible
@@ -474,20 +476,20 @@ func init*(
     forksByTime.delete(0)
 
   # Calculate the fork ids for the full fork history
-  var forkHistory = newSeqOfCap[ForkID](forksByBlock.len + forksByTime.len + 1)
+  var forkHistory = newSeqOfCap[ForkId](forksByBlock.len + forksByTime.len + 1)
   var crc = genesisCRC
 
   # Each entry is (crc before fork, fork number)
   for fork in forksByBlock:
-    forkHistory.add((crc, fork))
+    forkHistory.add(ForkId(hash: crc.to(Bytes4), next: fork))
     crc = crc32(crc, fork.toBytesBE)
 
   for fork in forksByTime:
-    forkHistory.add((crc, fork))
+    forkHistory.add(ForkId(hash: crc.to(Bytes4), next: fork))
     crc = crc32(crc, fork.toBytesBE)
 
   # Add last fork ID (after all forks, next=0)
-  forkHistory.add((crc, 0'u64))
+  forkHistory.add(ForkId(hash: crc.to(Bytes4), next: 0'u64))
 
   ForkIdCalculator(
     genesisCRC: genesisCRC,

execution_chain/networking/chain_forkid.nim

@@ -19,7 +19,6 @@ import
   eth/enode/enode_utils,
   ./discoveryv5,
   ./discoveryv4,
-  ./chain_forkid,
   ./bootnodes
 
 export
@@ -40,7 +39,7 @@ type
   AddressV4 = discoveryv4.Address
   AddressV5 = discoveryv5.Address
 
-  CompatibleForkIdProc* = proc(id: ForkID): bool {.noSideEffect, raises: [].}
+  CompatibleForkIdProc* = proc(id: ForkId): bool {.noSideEffect, raises: [].}
 
   Eth1Discovery* = ref object
     discv4: DiscV4
@@ -106,12 +105,12 @@ func eligibleNode(proto: Eth1Discovery, rec: Record): bool =
   let
     ethValue =
       try:
-        rlp.decode(bytes, array[1, ChainForkId])
+        rlp.decode(bytes, array[1, ForkId])
       except RlpError:
         return false
-    chainForkId  = ethValue[0]
+    forkId  = ethValue[0]
 
-  proto.compatibleForkId(chainForkId.to(ForkID))
+  proto.compatibleForkId(forkId)
 
 #------------------------------------------------------------------------------
 # Public functions
@@ -225,11 +224,11 @@ proc getRandomBootnode*(proto: Eth1Discovery): Opt[NodeV4] =
           return Opt.none(NodeV4)
       return Opt.some(newNode(enode))
 
-func updateForkID*(proto: Eth1Discovery, forkId: ForkID) =
+func updateForkId*(proto: Eth1Discovery, forkId: ForkId) =
   # https://github.com/ethereum/devp2p/blob/bc76b9809a30e6dc5c8dcda996273f0f9bcf7108/enr-entries/eth.md
   if proto.discv5.isNil.not:
     let
-      list = [forkId.to(ChainForkId)]
+      list = [forkId]
       bytes = rlp.encode(list)
       kv = ("eth", bytes)
     proto.discv5.updateRecord([kv]).isOkOr:

execution_chain/networking/eth1_discovery.nim

@@ -15,12 +15,11 @@
 import
   std/[os, tables, times, sets],
   chronos, chronicles,
+  eth/common/base,
   ./p2p_metrics,
   ./[eth1_discovery, p2p_peers]
 
-from eth/common/base import ForkID
-
-export sets, tables, CompatibleForkIdProc
+export sets, tables, CompatibleForkIdProc, base
 
 logScope:
   topics = "p2p peer_pool"
@@ -32,7 +31,7 @@ type
 
   WorkerFuture = Future[void].Raising([CancelledError])
 
-  ForkIdProc* = proc(): ForkID {.noSideEffect, raises: [].}
+  ForkIdProc* = proc(): ForkId {.noSideEffect, raises: [].}
 
   # Usually Network generic param is instantiated with EthereumNode
   PeerPoolRef*[Network] = ref object
@@ -44,7 +43,7 @@ type
     discovery: Eth1Discovery
     workers: seq[WorkerFuture]
     forkId: ForkIdProc
-    lastForkId: ForkID
+    lastForkId: ForkId
     connectTimer: Future[void].Raising([CancelledError])
     updateTimer: Future[void].Raising([CancelledError])
     connectingNodes*: HashSet[Node]
@@ -162,22 +161,22 @@ proc lookupPeers(p: PeerPoolRef) {.async: (raises: [CancelledError]).} =
     # to be later processed by connection worker
     await p.discovery.lookupRandomNode(p.connQueue)
 
-func updateForkID(p: PeerPoolRef) =
+func updateForkId(p: PeerPoolRef) =
   if p.forkId.isNil:
     return
 
   let forkId = p.forkId()
   if p.lastForkId == forkId:
     return
 
-  p.discovery.updateForkID(forkId)
+  p.discovery.updateForkId(forkId)
   p.lastForkId = forkId
 
 proc run(p: PeerPoolRef) {.async: (raises: [CancelledError]).} =
   trace "Running PeerPool..."
 
   # initial cycle
-  p.updateForkID()
+  p.updateForkId()
   await p.discovery.start()
   await p.lookupPeers()
 
@@ -199,7 +198,7 @@ proc run(p: PeerPoolRef) {.async: (raises: [CancelledError]).} =
       await p.lookupPeers()
 
     if res == p.updateTimer:
-      p.updateForkID()
+      p.updateForkId()
 
 #------------------------------------------------------------------------------
 # Private functions

execution_chain/networking/peer_pool.nim

@@ -104,11 +104,11 @@ proc setupP2P(nimbus: NimbusNode, config: ExecutionClientConf, com: CommonRef) =
     bootstrapNodes = config.getBootstrapNodes()
     fc = nimbus.fc
 
-  func forkIdProc(): ForkID =
+  func forkIdProc(): ForkId =
     let header = fc.latestHeader()
     com.forkId(header.number, header.timestamp)
 
-  func compatibleForkIdProc(id: ForkID): bool =
+  func compatibleForkIdProc(id: ForkId): bool =
     let header = fc.latestHeader()
     com.compatibleForkId(id, header.number, header.timestamp)
 

execution_chain/nimbus_execution_client.nim

@@ -10,7 +10,6 @@
 {.push raises: [].}
 
 import
-  stew/endians2,
   std/[sequtils, algorithm],
   ./rpc_types,
   ./params,
@@ -341,15 +340,15 @@ proc populateConfigObject*(com: CommonRef, fork: HardFork): ConfigObject =
 
   configObject.activationTime = Number com.activationTime(fork).get(EthTime(0))
   configObject.chainId = com.chainId
-  configObject.forkId = FixedBytes[4] com.forkId(
+  configObject.forkId = com.forkId(
     com.activationTime(fork).get(EthTime(0))
-  ).crc.toBytesBE
+  ).hash
   configObject.blobSchedule.max = Number com.maxBlobsPerBlock(fork)
   configObject.blobSchedule.target = Number com.targetBlobsPerBlock(fork)
   configObject.blobSchedule.baseFeeUpdateFraction = Number com.baseFeeUpdateFraction(fork)
 
   # Precompiles
-  let 
+  let
     evmFork = ToEVMFork[fork]
     lastPrecompile = getMaxPrecompile(evmFork)
 
@@ -369,7 +368,7 @@ proc populateConfigObject*(com: CommonRef, fork: HardFork): ConfigObject =
 
   return configObject
 
-proc getEthConfigObject*(com: CommonRef, 
+proc getEthConfigObject*(com: CommonRef,
                          chain: ForkedChainRef,
                          fork: HardFork,
                          nextFork: Opt[HardFork],

execution_chain/rpc/rpc_utils.nim

@@ -12,7 +12,6 @@
 
 import
   chronicles, chronos,
-  stew/endians2,
   ./types,
   ./requester,
   ./broadcast,
@@ -64,10 +63,8 @@ proc getStatus68*(ctx: EthWireRef): Eth68State =
     totalDifficulty: txFrame.headTotalDifficulty,
     genesisHash: com.genesisHash,
     bestBlockHash: ctx.chain.latestHash,
-    forkId: ChainForkId(
-      forkHash: forkId.crc.toBytesBE,
-      forkNext: forkId.nextFork
-  ))
+    forkId: forkId,
+  )
 
 proc getStatus69*(ctx: EthWireRef): Eth69State =
   let
@@ -77,7 +74,7 @@ proc getStatus69*(ctx: EthWireRef): Eth69State =
 
   Eth69State(
     genesisHash: com.genesisHash,
-    forkId: forkId.to(ChainForkId),
+    forkId: forkId,
     earliest: 0,
     latest: bestBlock.number,
     latestHash: ctx.chain.latestHash,