From e658ef8ae58f4df8707936efc2a126eb976f944e Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Tue, 28 Apr 2026 16:06:34 +0530 Subject: [PATCH 1/3] pre-flight fcu checks and async fcu call --- .../beacon/api_handler/api_forkchoice.nim | 19 +++++++- execution_chain/core/chain/forked_chain.nim | 45 ++++++++++++++++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/execution_chain/beacon/api_handler/api_forkchoice.nim b/execution_chain/beacon/api_handler/api_forkchoice.nim index 6cd2211683..a3091df923 100644 --- a/execution_chain/beacon/api_handler/api_forkchoice.nim +++ b/execution_chain/beacon/api_handler/api_forkchoice.nim @@ -199,13 +199,30 @@ proc forkchoiceUpdated*(ben: BeaconEngineRef, raise invalidForkChoiceState("safe block not in canonical tree") # similar to headHash, safeBlockHash is saved by FC module - (await chain.queueForkChoice(headHash, finalizedBlockHash, safeBlockHash)).isOkOr: + # Pre-validate that the forkchoice update will succeed in the queue worker. + # We do this synchronously so we can surface a per-call error here; once the + # worker has been kicked off (below), errors can no longer be reported back + # to this fCU response. + chain.preflightForkChoice(headHash, finalizedBlockHash, safeBlockHash).isOkOr: return invalidFCU(error, chain, header) + # Enqueue the actual forkchoice apply. For ack-only fCUs we don't await + # the result: by spec the CL only needs us to confirm validation, and the + # head/finalized application can run after we respond. This avoids the + # response getting blocked behind the shared queue worker (which also + # serializes `validateBlock` and `processUpdateBase`, the latter occasionally + # stalling for hundreds of ms during RocksDB compactions). + let fcuFut = chain.queueForkChoice(headHash, finalizedBlockHash, safeBlockHash) + # If payload generation was requested, create a new block to be potentially # sealed by the beacon client. The payload will be requested later, and we # might replace it arbitrarilly many times in between. if attrsOpt.isSome: + # Payload generation reads `chain.latestHeader`, so we must wait for the + # apply to complete before assembling the block. + (await fcuFut).isOkOr: + return invalidFCU(error, chain, header) + let attrs = attrsOpt.value validateVersion(attrs, com, apiVersion) diff --git a/execution_chain/core/chain/forked_chain.nim b/execution_chain/core/chain/forked_chain.nim index c21ea54646..18a87dece5 100644 --- a/execution_chain/core/chain/forked_chain.nim +++ b/execution_chain/core/chain/forked_chain.nim @@ -743,7 +743,10 @@ proc forkChoice*(c: ForkedChainRef, if safe.isOk: c.fcuSafe.number = safe.number c.fcuSafe.hash = safeHash - ?safe.txFrame.fcuSafe(c.fcuSafe) + # Use `.expect(...)` like the sibling fcuHead/fcuFinalized writes so a + # DB-level failure here is a hard fault rather than a recoverable err - + # the RPC handler may have already acked the fCU before we run. + safe.txFrame.fcuSafe(c.fcuSafe).expect("fcuSafe OK") if headHash == c.latest.hash: if finalizedHash == zeroHash32: @@ -813,6 +816,46 @@ template queueForkChoice*(c: ForkedChainRef, await c.queue.addLast(item) item.responseFut +func preflightForkChoice*(c: ForkedChainRef, + headHash: Hash32, + finalizedHash: Hash32, + safeHash: Hash32 = zeroHash32): + Result[void, string] = + ## Pre-validate a forkchoice update against the current in-memory chain, + ## mirroring the failure paths inside `forkChoice()`. Run this from the RPC + ## handler before enqueuing so we don't ack a request that the queue worker + ## would later reject (since the ack has already been sent by then). + ## + ## Invariant: between this check and the worker actually running our + ## enqueued item, the only mutators of `c.heads`/`hashToBlock` are + ## previously-queued `validateBlock` (adds heads, never removes the + ## validated one) and `updateFinalized` (only prunes branches that don't + ## contain the finalized block we already verified is on head's chain). + ## So a successful preflight remains valid by the time the worker runs. + let head = c.hashToBlock.getOrDefault(headHash) + if head.isNil: + return err("Cannot find head block: " & headHash.short) + + if safeHash != zeroHash32 and c.hashToBlock.getOrDefault(safeHash).isNil: + return err("Cannot find safe block: " & safeHash.short) + + if finalizedHash != zeroHash32: + let fin = c.hashToBlock.getOrDefault(finalizedHash) + if fin.isNil: + return err("Cannot find finalized block: " & finalizedHash.short) + if fin.number > head.number: + return err("Invalid finalizedHash: block is newer than head block") + if c.heads.len > 1: + var found = false + for it in ancestors(head): + if it == fin: + found = true + break + if not found: + return err("Invalid finalizedHash: block not in argument head ancestor lineage") + + ok() + func resolvedFinHash*(c: ForkedChainRef): Hash32 = c.latestFinalized.hash From 7f638fb95c7b76a0caffeaee37a493816845aac5 Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Thu, 30 Apr 2026 16:23:35 +0530 Subject: [PATCH 2/3] add comments --- execution_chain/beacon/api_handler/api_forkchoice.nim | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/execution_chain/beacon/api_handler/api_forkchoice.nim b/execution_chain/beacon/api_handler/api_forkchoice.nim index a3091df923..e37d5e8463 100644 --- a/execution_chain/beacon/api_handler/api_forkchoice.nim +++ b/execution_chain/beacon/api_handler/api_forkchoice.nim @@ -207,11 +207,7 @@ proc forkchoiceUpdated*(ben: BeaconEngineRef, return invalidFCU(error, chain, header) # Enqueue the actual forkchoice apply. For ack-only fCUs we don't await - # the result: by spec the CL only needs us to confirm validation, and the - # head/finalized application can run after we respond. This avoids the - # response getting blocked behind the shared queue worker (which also - # serializes `validateBlock` and `processUpdateBase`, the latter occasionally - # stalling for hundreds of ms during RocksDB compactions). + # the result. This avoids the response getting blocked behind the shared queue worker let fcuFut = chain.queueForkChoice(headHash, finalizedBlockHash, safeBlockHash) # If payload generation was requested, create a new block to be potentially From dd6fd68fcebd11dfa364ab91fc31893dab953399 Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Fri, 1 May 2026 21:00:27 +0530 Subject: [PATCH 3/3] add comments regarding updateBase & updateFinalized --- execution_chain/core/chain/forked_chain.nim | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/execution_chain/core/chain/forked_chain.nim b/execution_chain/core/chain/forked_chain.nim index 58ce80f548..0b5c26660b 100644 --- a/execution_chain/core/chain/forked_chain.nim +++ b/execution_chain/core/chain/forked_chain.nim @@ -222,6 +222,10 @@ func updateHead(c: ForkedChainRef, head: BlockRef) = head.number) func updateFinalized(c: ForkedChainRef, finalized: BlockRef, fcuHead: BlockRef) = + # Invariant: must only be called from inside a queue handler (currently + # `validateBlock` and `forkChoice`). `preflightForkChoice` runs synchronously + # in the RPC handler and relies on this proc — and `updateBase` — never + # mutating `c.heads`/`hashToBlock` outside the single-consumer worker. # Pruning # :: # - B5 - B6 - B7 - B8 @@ -296,6 +300,11 @@ func updateFinalized(c: ForkedChainRef, finalized: BlockRef, fcuHead: BlockRef) c.latest = candidate proc updateBase(c: ForkedChainRef, base: BlockRef): uint = + ## Invariant: must only be called from inside a queue handler (currently + ## `processUpdateBase`, enqueued via `queueUpdateBase`). `preflightForkChoice` + ## runs synchronously in the RPC handler and relies on this proc — and + ## `updateFinalized` — never mutating `c.heads`/`hashToBlock` outside the + ## single-consumer worker. ## ## A1 - A2 - A3 D5 - D6 ## / /