From 199d1c443425b3f373198e603837b7bfce02a5d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:06:51 -0300 Subject: [PATCH] fix(blockchain): stop the missing-ancestor walk at an imported block pending_block_parents records each pending block's missing ancestor as it was when the block pended. When that ancestor later imports while a block between it and its descendants is still missing (one dropped for lack of an execution verdict after its own parent imported), every new arrival resolved to the imported ancestor. The DB walk re-queued it as a no-op and returned before requesting anything, so the dropped block was never retried and the head stayed put. Seen on Hoodi behind a snap-syncing geth: stuck at slot 4248640 for three hours. Stop the pointer walk at an ancestor that has a post-state, so the DB walk finds the real gap. --- crates/blockchain/src/lib.rs | 92 +++++++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/crates/blockchain/src/lib.rs b/crates/blockchain/src/lib.rs index 459f6451..251048b7 100644 --- a/crates/blockchain/src/lib.rs +++ b/crates/blockchain/src/lib.rs @@ -753,9 +753,12 @@ pub enum ImportOutcome { /// re-imports it and reaches `collect_pending_children` for real. /// - No verdict from the execution client, or a `NOT_VALIDATED` verdict on /// a block that is not an optimistic candidate. Nothing is recorded and - /// the block is simply dropped: unlike a column hold, nothing re-drives + /// the block is simply dropped: unlike a column hold, no timer re-drives /// it, which is what the terminal retry policy on the engine ladder - /// means. See `EngineClient::call`'s own documentation. + /// means. See `EngineClient::call`'s own documentation. The next arrival + /// whose missing ancestor is this block does: `process_or_pend_block`'s + /// DB walk re-queues it if it was ever pended, and otherwise asks the + /// network for it. /// /// The name reads as "held for columns" for historical reasons; it is the /// general "produced no post-state" outcome. @@ -2777,8 +2780,22 @@ impl BlockChainServer { // Resolve the actual missing ancestor by walking the chain. A stale entry // can occur when a cached ancestor was itself received and became pending // with its own missing parent — the children still point to the old value. + // + // An entry also goes stale the other way: the recorded ancestor gets a + // post-state while a block between it and this one is still missing, + // e.g. one dropped for lack of an execution verdict after its parent + // imported. Following it would land on an imported block, which the + // DB walk below re-queues as a no-op, so the real gap would never be + // retried or requested. Stop there; the DB walk finds the gap. let mut missing_root = parent_root; while let Some(&ancestor) = self.pending_block_parents.get(&missing_root) { + if self + .store + .has_state(&ancestor) + .expect("DB read should succeed") + { + break; + } missing_root = ancestor; } @@ -5404,6 +5421,77 @@ mod tests { ); } + #[tokio::test] + async fn a_block_dropped_below_an_imported_ancestor_is_retried_by_the_next_arrival() { + // A Hoodi follower behind a snap-syncing geth: a run of blocks pended + // while their oldest ancestor was missing, so each recorded that + // ancestor in `pending_block_parents`. It then imported, its child got + // no execution verdict and was dropped, and every later arrival still + // resolved to the imported ancestor. The DB walk re-queued that + // ancestor as a no-op and never reached the dropped block, so the + // head stayed put for three hours. + let mut store = beacon_store(GENESIS_TIME, 0); + store + .insert_signed_block(H256::ZERO, bare_block(0, H256::repeat_byte(0xcc))) + .expect("insert"); + store + .insert_state(H256::ZERO, bare_state()) + .expect("insert"); + let mut server = beacon_server(store); + + // The ancestor the stale pointer names, imported since. + let imported = bare_block(1, H256::ZERO); + let imported_root = imported.message_hash_tree_root(); + server + .store + .insert_pending_block(imported_root, imported) + .expect("insert"); + server + .store + .insert_state(imported_root, bare_state()) + .expect("insert"); + + // Its child, dropped without a verdict: stored, but with no state. + let dropped = bare_block(2, imported_root); + let dropped_root = dropped.message_hash_tree_root(); + server + .store + .insert_pending_block(dropped_root, dropped) + .expect("insert"); + + // The dropped block's child, still pending and still pointing at the + // ancestor that was missing when it arrived. + let pending = bare_block(3, dropped_root); + let pending_root = pending.message_hash_tree_root(); + server + .store + .insert_pending_block(pending_root, pending) + .expect("insert"); + server + .pending_blocks + .insert(dropped_root, HashSet::from([pending_root])); + server + .pending_block_parents + .insert(pending_root, imported_root); + + let arrival = bare_block(4, pending_root); + let mut queue = VecDeque::new(); + let outcome = server + .process_or_pend_block(arrival, ImportTimings::default(), &mut queue) + .await; + + assert_eq!(outcome, None, "the arrival itself still pends"); + let queued: Vec = queue + .iter() + .map(|(block, _)| block.message_hash_tree_root()) + .collect(); + assert_eq!( + queued, + vec![dropped_root], + "the dropped block is retried, not the imported ancestor" + ); + } + // ----------------------------------------------------------------- // The test above proves the cascade cannot livelock; it does not prove // the fix's whole point, that a released parent actually unblocks what