Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 5, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d3fc24
Rework ForkId to get rid of ChainForkId and its conversions
kdeme Nov 4, 2025
1d8738c
Fix ForkId equals by exporting base
kdeme Nov 5, 2025
a3639d2
Update vendor nim-eth
kdeme Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions execution_chain/common/common.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
44 changes: 23 additions & 21 deletions execution_chain/common/hardforks.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
28 changes: 0 additions & 28 deletions execution_chain/networking/chain_forkid.nim
View file
Open in desktop

This file was deleted.

13 changes: 6 additions & 7 deletions execution_chain/networking/eth1_discovery.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import
eth/enode/enode_utils,
./discoveryv5,
./discoveryv4,
./chain_forkid,
./bootnodes

export
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
17 changes: 8 additions & 9 deletions execution_chain/networking/peer_pool.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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]
Expand Down Expand Up @@ -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:
Copy link
Contributor Author

@kdeme kdeme Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason the == here fails with

Expression: x.hash == y.hash
  [1] x.hash: Bytes4
  [2] y.hash: Bytes4

without the added == here:
status-im/nim-eth@21ba892#diff-fdc22bb681374a71a5a754cf46c5ade4b1c54c35c66f163533e86879bab9228aR190-R193

It shouldn't be needed though, and in fact in testing equals win forkid_test.nim, it does work without the added ==.

Copy link
Contributor Author

@kdeme kdeme Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it works without the specific ForkId == added, by adding eth/common/base import in p2p.nim (or exporting base in peer_pool.nim. So I'll go that route.

Depends on order I guess and hitting p2p.nim first,
basically instantiation here https://github.com/status-im/nimbus-eth1/blob/master/execution_chain/networking/p2p.nim#L116 ?

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()

Expand All @@ -199,7 +198,7 @@ proc run(p: PeerPoolRef) {.async: (raises: [CancelledError]).} =
await p.lookupPeers()

if res == p.updateTimer:
p.updateForkID()
p.updateForkId()

#------------------------------------------------------------------------------
# Private functions
Expand Down
4 changes: 2 additions & 2 deletions execution_chain/nimbus_execution_client.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
9 changes: 4 additions & 5 deletions execution_chain/rpc/rpc_utils.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
{.push raises: [].}

import
stew/endians2,
std/[sequtils, algorithm],
./rpc_types,
./params,
Expand Down Expand Up @@ -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)

Expand All @@ -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],
Expand Down
9 changes: 3 additions & 6 deletions execution_chain/sync/wire_protocol/handler.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

import
chronicles, chronos,
stew/endians2,
./types,
./requester,
./broadcast,
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
Loading