From a11df91fee172274ddc1c5cf2aed8e35d86e8476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:10:04 -0300 Subject: [PATCH] fix(beacon): cache the anchor's execution hash so its child imports optimistically is_optimistic_candidate_block reads the spec's is_execution_block(parent) off the execution-hash cache, which was filled only when a block was imported. The checkpoint anchor is never imported, so its first child looked like the child of a pre-merge block and, with the EL still syncing, waited out the 128-slot horizon: a checkpoint-synced follower sat at its anchor for up to ~25 min (observed on Hoodi, ~11 min, failing a multisync head-stall check). forkchoiceUpdated also sent a zero finalized and safe hash for the anchor. The spec reads is_execution_block from the block body, and its checkpoint-sync section lets the anchor be assumed valid, so get_forkchoice_store now caches the anchor's payload hash (the hash only, no verdict). A restart emptied the cache the same way, so resume rebuilds it for the unfinalized window from the stored blocks. One helper on SignedBeaconBlock decides which blocks have a hash to cache, shared by import, the anchor and resume. --- .../src/beacon/fork_choice.rs | 103 ++++++++++++++++-- .../common/types/src/beacon/containers/mod.rs | 29 +++++ crates/storage/src/store.rs | 79 +++++++++++++- docs/beacon_engine.md | 37 +++---- 4 files changed, 218 insertions(+), 30 deletions(-) diff --git a/crates/blockchain/state_transition/src/beacon/fork_choice.rs b/crates/blockchain/state_transition/src/beacon/fork_choice.rs index 8ce25d476..b1c9e9b3d 100644 --- a/crates/blockchain/state_transition/src/beacon/fork_choice.rs +++ b/crates/blockchain/state_transition/src/beacon/fork_choice.rs @@ -904,8 +904,10 @@ pub fn mark_validated(store: &mut Store, root: Root) { /// chain has had time to justify around any poison. /// /// Reads `is_execution_block(parent)` off the cached execution hash rather than -/// decoding the parent block: the cache is populated at import for exactly the -/// blocks that have one, so its absence is the answer. +/// decoding the parent block: the cache is populated for exactly the blocks +/// that have one (at import, for the checkpoint anchor in +/// [`get_forkchoice_store`], and on resume for the stored unfinalized window), +/// so its absence is the answer. /// /// `safe_slots` is a parameter rather than the constant read directly, because /// the specification requires the value to be operator-configurable. @@ -1120,9 +1122,19 @@ pub fn get_forkchoice_store( // this file's only outright whole-`BeaconState` clone. `checkpoint_state` // now derives that second copy on demand instead of caching it, so the // anchor is written once. + let anchor_el_hash = anchor_block.execution_block_hash_if_execution_block(); store .insert_signed_block(anchor_root, anchor_block) .expect("insert"); + // Only the hash, not a verdict: the specification's `is_execution_block` + // reads the block body, and the anchor is the one block that never goes + // through `on_block`, which is what fills this cache. Without it the first + // child's parent would look pre-merge, so `is_optimistic_candidate_block` + // would hold it until it is `safe_slots` old, and `forkchoiceUpdated`'s + // finalized and safe hashes would be zero for the anchor. + if let Some(hash) = anchor_el_hash { + store.insert_beacon_el_block_hash(anchor_root, anchor_slot, hash); + } store .insert_state(anchor_root, anchor_state) .expect("insert"); @@ -4725,7 +4737,7 @@ pub fn on_block( // Add new block to the store, and the new state for this block to the // store. - let signed_block_el_hash = signed_block.execution_block_hash(); + let signed_block_el_hash = signed_block.execution_block_hash_if_execution_block(); store .insert_signed_block(block_root, signed_block) .expect("insert"); @@ -4757,11 +4769,8 @@ pub fn on_block( // zero (see `stf::bellatrix::default_execution_payload`), and caching that // would make its children look like descendants of a merge block and skip // the age horizon that exists precisely to guard the merge transition. - // Testing the block hash alone is enough: it is a keccak digest in a real - // payload and zero in the empty one. - if let Some(el_block_hash) = signed_block_el_hash - && !el_block_hash.is_zero() - { + // The rule lives in `SignedBeaconBlock::execution_block_hash_if_execution_block`. + if let Some(el_block_hash) = signed_block_el_hash { store.insert_beacon_el_block_hash(block_root, block_slot, el_block_hash); } match payload_validity { @@ -6794,6 +6803,82 @@ mod tests { )); } + /// An exact fulu anchor pair whose payload carries `el_hash`, built as + /// `get_forkchoice_store_seeds_a_gloas_anchor_and_leaves_an_earlier_one_alone` + /// builds its gloas one. + fn fulu_anchor_pair(el_hash: ExecutionBlockHash) -> (BeaconState, SignedBeaconBlock) { + let mut state = test_state::with_validators_at(ForkName::Fulu, 1); + let mut signed = fulu_block(state.slot(), Root::ZERO); + let SignedBeaconBlock::Fulu(inner) = &mut signed else { + unreachable!("`fulu_block` builds a fulu signed block"); + }; + inner.message.body.execution_payload.block_hash = el_hash; + *state.latest_block_header_mut() = BeaconBlockHeader { + slot: inner.message.slot, + proposer_index: 0, + parent_root: Root::ZERO, + state_root: Root::ZERO, + body_root: inner.message.body.hash_tree_root(), + }; + inner.message.state_root = state.hash_tree_root(); + (state, signed) + } + + /// The anchor is the one block `on_block` never sees, so its execution hash + /// has to be cached when the store is built: otherwise its first child's + /// parent reads as pre-merge and waits out the age horizon. + #[test] + fn a_post_merge_anchor_makes_its_first_child_an_optimistic_candidate() { + let el_hash = ExecutionBlockHash::repeat_byte(0xaa); + let (state, block) = fulu_anchor_pair(el_hash); + let anchor_root = block.message_hash_tree_root(); + let anchor_slot = block.slot(); + + let store = get_forkchoice_store( + Arc::new(InMemoryBackend::new()), + state, + block, + &Config::active(), + ) + .expect("a fulu anchor is accepted"); + + assert_eq!(store.beacon_el_block_hash(anchor_root), Some(el_hash)); + assert!(is_optimistic_candidate_block( + &store, + anchor_slot + 1, + anchor_slot + 1, + anchor_root, + constants::SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY, + )); + } + + #[test] + fn an_anchor_with_no_execution_payload_caches_no_hash() { + // A zero payload hash is the pre-merge shape: not an execution block. + let (state, block) = fulu_anchor_pair(ExecutionBlockHash::ZERO); + let anchor_root = block.message_hash_tree_root(); + let store = get_forkchoice_store( + Arc::new(InMemoryBackend::new()), + state, + block, + &Config::active(), + ) + .unwrap(); + assert_eq!(store.beacon_el_block_hash(anchor_root), None); + + // Phase0 carries no payload at all. + let (state, block) = anchor_pair(); + let anchor_root = block.message_hash_tree_root(); + let store = get_forkchoice_store( + Arc::new(InMemoryBackend::new()), + state, + block, + &Config::active(), + ) + .unwrap(); + assert_eq!(store.beacon_el_block_hash(anchor_root), None); + } + #[test] fn a_block_on_a_payloadless_parent_needs_the_age_horizon() { let store = empty_store(); @@ -8583,6 +8668,8 @@ mod tests { Some(vec![None; preset::PTC_SIZE]) ); assert!(!store.has_verified_payload(&anchor_root)); + // A gloas block carries a bid, not a payload: nothing to cache. + assert_eq!(store.beacon_el_block_hash(anchor_root), None); // Earlier forks record none of the three for their anchor. let (state, block) = anchor_pair(); diff --git a/crates/common/types/src/beacon/containers/mod.rs b/crates/common/types/src/beacon/containers/mod.rs index 1e8b09e1c..a03dd75ba 100644 --- a/crates/common/types/src/beacon/containers/mod.rs +++ b/crates/common/types/src/beacon/containers/mod.rs @@ -1340,6 +1340,20 @@ impl SignedBeaconBlock { } } + /// This block's payload hash when the specification's `is_execution_block` + /// holds for it, so the one rule that decides which blocks have an + /// execution hash to cache. + /// + /// [`Self::execution_block_hash`] minus the all-zero hash: a pre-merge + /// bellatrix block carries a payload field whose every byte is zero, and + /// that is "not an execution block", not a block with a zero hash. Testing + /// the hash alone is enough, since it is a keccak digest in a real payload + /// and zero in the empty one. `None` for phase0, altair and gloas too (see + /// [`Self::execution_block_hash`]). + pub fn execution_block_hash_if_execution_block(&self) -> Option { + self.execution_block_hash().filter(|hash| !hash.is_zero()) + } + /// How many blob KZG commitments this block's body carries: zero before /// deneb, which introduced them. /// @@ -2247,6 +2261,21 @@ mod tests { assert_eq!(block.execution_block_hash(), Some(expected)); } + #[test] + fn execution_block_hash_if_execution_block_gives_post_merge_hashes_only() { + let expected = ExecutionBlockHash::repeat_byte(7); + let electra = SignedBeaconBlock::Electra(empty_electra_signed_block(expected)); + assert_eq!( + electra.execution_block_hash_if_execution_block(), + Some(expected) + ); + let deneb_like = + SignedBeaconBlock::Electra(empty_electra_signed_block(ExecutionBlockHash::ZERO)); + assert_eq!(deneb_like.execution_block_hash_if_execution_block(), None); + let phase0 = SignedBeaconBlock::Phase0(empty_phase0_signed_block()); + assert_eq!(phase0.execution_block_hash_if_execution_block(), None); + } + // -- body_root -- #[test] diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 3030ae96b..790755be8 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -790,7 +790,9 @@ pub(crate) struct BeaconScratch { /// production path carries its verdict as an `on_block` parameter instead. pub(crate) payload_statuses: HashMap, /// Beacon root to `(slot, execution block hash)`. A cache, not a source of - /// truth: every entry is recoverable by decoding the block. Unlike its + /// truth: every entry is recoverable by decoding the block. Filled at + /// import, for the checkpoint anchor (the one block never imported), and on + /// resume for the unfinalized window. Unlike its /// neighbours it *is* bounded, by `prune_beacon_el_block_hashes`, because /// forkchoiceUpdated reads it once per head move for the whole life of the /// process. @@ -3742,6 +3744,7 @@ impl Store { let mut timeliness = HashMap::new(); let mut payloads = HashSet::new(); let mut gloas_roots = Vec::new(); + let mut el_hashes = Vec::new(); for (key, _) in view .prefix_iterator(Table::LiveChain, &[]) .expect("iterator") @@ -3761,6 +3764,20 @@ impl Store { .expect("a stored timeliness has one byte per deadline"); timeliness.insert(root, bytes.map(|byte| byte != 0)); } + // `el_block_hashes` is a cache of what the block body says, and a + // restart empties it; without it every stored block would read as + // "not an execution block" to `is_optimistic_candidate_block` and + // the `latestValidHash` walk. Cached against the block's own slot, + // so `prune_beacon_el_block_hashes` still bounds it. + if let Some(hash) = view + .read_with(Table::BlockHeaders, &root.to_ssz(), |bytes| { + decode_beacon_block_value(bytes).execution_block_hash_if_execution_block() + }) + .expect("read") + .flatten() + { + el_hashes.push((root, slot, hash)); + } let fork = view .get(Table::BlockHeaders, &root.to_ssz()) .expect("get") @@ -3798,6 +3815,9 @@ impl Store { let mut scratch = self.beacon.lock().unwrap(); scratch.block_timeliness.extend(timeliness); scratch.verified_payloads.extend(payloads); + for (root, slot, hash) in el_hashes { + scratch.el_block_hashes.insert(root, (slot, hash)); + } for root in gloas_roots { scratch .payload_timeliness_vote @@ -8147,6 +8167,63 @@ mod tests { assert_eq!(reopened.payload_timeliness_vote(&phase0_root), None); } + fn fulu_test_block( + slot: u64, + parent_root: H256, + el_hash: ExecutionBlockHash, + ) -> SignedBeaconBlock { + use ethlambda_types::beacon::containers::electra; + + let mut body = electra::BeaconBlockBody::empty(); + body.execution_payload.block_hash = el_hash; + SignedBeaconBlock::Fulu(electra::SignedBeaconBlock { + message: electra::BeaconBlock { + slot, + proposer_index: 0, + parent_root, + state_root: H256::ZERO, + body, + }, + signature: Default::default(), + }) + } + + #[test] + fn resume_rebuilds_execution_hashes_for_the_unfinalized_window() { + let old = H256::from([1u8; 32]); + let finalized = H256::from([2u8; 32]); + let child = H256::from([3u8; 32]); + let premerge = H256::from([4u8; 32]); + let backend = beacon_backend_finalized_at(finalized); + let mut store = reopen(backend.clone()); + let rows = [ + (old, SLOTS_PER_EPOCH - 3, H256::ZERO, 0xa1), + (finalized, SLOTS_PER_EPOCH - 2, old, 0xa2), + (child, SLOTS_PER_EPOCH - 1, finalized, 0xa3), + // A zero payload hash is a pre-merge shape: no entry. + (premerge, SLOTS_PER_EPOCH, child, 0), + ]; + for (root, slot, parent, byte) in rows { + let hash = ExecutionBlockHash::repeat_byte(byte); + store + .insert_signed_block(root, fulu_test_block(slot, parent, hash)) + .expect("insert block"); + } + + let reopened = reopen(backend); + + assert_eq!(reopened.beacon_el_block_hash(old), None); + assert_eq!( + reopened.beacon_el_block_hash(finalized), + Some(ExecutionBlockHash::repeat_byte(0xa2)) + ); + assert_eq!( + reopened.beacon_el_block_hash(child), + Some(ExecutionBlockHash::repeat_byte(0xa3)) + ); + assert_eq!(reopened.beacon_el_block_hash(premerge), None); + } + #[test] fn resume_skips_payloads_below_finality() { // The finalized block sits below its checkpoint's epoch start, and its diff --git a/docs/beacon_engine.md b/docs/beacon_engine.md index c849cffd0..1aa13b184 100644 --- a/docs/beacon_engine.md +++ b/docs/beacon_engine.md @@ -148,29 +148,24 @@ A `NOT_VALIDATED` verdict is not enough on its own. The block must also either 1. have a parent this node has already recorded an execution hash for, or 2. be at least `--safe-slots-to-import-optimistically` behind the wall clock. -Condition 1 is `store.beacon_el_block_hash(parent_root).is_some()`, and that hash -is written when a block is *imported*. So it asks whether this node imported the -parent and saw its payload, not whether the parent is post-merge. +Condition 1 is `store.beacon_el_block_hash(parent_root).is_some()`: the +specification's `is_execution_block(parent)`, read off a cache of the parent's +payload hash rather than by decoding the block. The cache holds a hash for every +post-merge block the store has, and none for a pre-merge block (a zero payload +hash is not an execution block). It is filled at import, for the checkpoint +anchor when the store is built, and on resume for the unfinalized window. The horizon guards against a poisoned *merge transition* block, whose parent hash -names an execution block nobody can produce. Once a node is importing a -post-merge chain every block's parent carries a recorded hash, so condition 1 -carries each one and condition 2 stops mattering. - -The exception is the first block after a checkpoint-sync anchor. The anchor is -never imported and never asked about (see [The anchor is assumed -valid](#the-anchor-is-assumed-valid)), so no execution hash is recorded for it -and its child fails condition 1. Paired with an execution client that is itself -still syncing, and so answers `SYNCING` to every `newPayload`, that child has -only condition 2 left and the follower's head sits at the anchor until the -horizon admits it. - -The wait is `--safe-slots-to-import-optimistically` minus however far the anchor -already trails the wall clock, which for a finalized anchor is most of it: -measured against mainnet on 2026-09-15, an anchor 85 slots back cleared in about -9 minutes rather than the full 128 slots. It costs that once per sync. Importing -that block records its hash, so every descendant takes condition 1 and the -backlog cascades. +names an execution block nobody can produce. Once a node is on a post-merge chain +every block's parent has a recorded hash, so condition 1 carries each one and +condition 2 stops mattering. + +The anchor is never imported and never asked about (see [The anchor is assumed +valid](#the-anchor-is-assumed-valid)), so its hash is cached when the store is +built; only the hash, not a verdict. That is what lets the first block after a +checkpoint-sync anchor take condition 1 even while the execution client is still +syncing and answers `SYNCING` to every `newPayload`. A restart empties the cache, +so resume rebuilds it from the stored blocks. ### `latestValidHash`