From 5d383ecccb46c77e167f4cf47c8bb6e455ba9d07 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Thu, 19 Mar 2026 18:59:54 +0200 Subject: [PATCH 01/25] FCU to canonical ancestor silently ignored, leaving descendants canonical --- .../BlockTreeTests.cs | 596 ++++++++++++++++++ .../EngineModuleTests.V1.cs | 54 ++ .../Handlers/ForkchoiceUpdatedHandler.cs | 5 - 3 files changed, 650 insertions(+), 5 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index d2f2a332753e..214e93762a61 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2201,4 +2201,600 @@ public void On_UpdateMainBranch_UpdateSyncPivot_ToHeaderUnderReorgDepth() } } } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_by_number_after_reorg_returns_new_canonical_with_and_without_RequireCanonical() + { + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + // Block A at height 1 — initially canonical + Block blockA = Build.A.Block + .WithNumber(1) + .WithParent(genesis) + .WithExtraData(new byte[] { 1 }) + .TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + + // Block B at height 1 — same parent, different block (reorg) + Block blockB = Build.A.Block + .WithNumber(1) + .WithParent(genesis) + .WithExtraData(new byte[] { 2 }) + .TestObject; + blockTree.SuggestBlock(blockB); + blockTree.UpdateMainChain(new[] { blockB }, true); + + // Without RequireCanonical (old BlockParameter(long) behavior) + Block? withoutCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.None); + + // With RequireCanonical (new BlockParameter(long) behavior after Ahmad's change) + Block? withCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); + + withoutCanonical.Should().NotBeNull(); + withCanonical.Should().NotBeNull(); + + withoutCanonical!.Hash.Should().Be(blockB.Hash!, + "FindBlock without RequireCanonical must return the current canonical block B after reorg"); + withCanonical!.Hash.Should().Be(blockB.Hash!, + "FindBlock with RequireCanonical must return the current canonical block B after reorg, not the old canonical A"); + + withCanonical.Hash.Should().Be(withoutCanonical.Hash, + "RequireCanonical and non-canonical number lookups must agree on which block is canonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_after_double_reorg_A_to_B_back_to_A_returns_A() + { + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.SuggestBlock(blockB); + + // A canonical → reorg to B → reorg back to A + blockTree.UpdateMainChain(new[] { blockA }, true); + blockTree.UpdateMainChain(new[] { blockB }, true); + blockTree.UpdateMainChain(new[] { blockA }, true); + + Block? withoutCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.None); + Block? withCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); + + withoutCanonical!.Hash.Should().Be(blockA.Hash!, "after reorg back to A, None lookup must return A"); + withCanonical!.Hash.Should().Be(blockA.Hash!, "after reorg back to A, RequireCanonical lookup must return A"); + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "B must not be canonical after reorg back to A"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_after_multi_block_reorg_both_heights_updated() + { + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + // Original chain: A1 → A2 + Block a1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + Block a2 = Build.A.Block.WithNumber(2).WithParent(a1).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(a1); + blockTree.SuggestBlock(a2); + blockTree.UpdateMainChain(new[] { a1, a2 }, true); + + // Reorg chain: B1 → B2 + Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 3 }).TestObject; + Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).WithExtraData(new byte[] { 4 }).TestObject; + blockTree.SuggestBlock(b1); + blockTree.SuggestBlock(b2); + blockTree.UpdateMainChain(new[] { b1, b2 }, true); + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(b1.Hash!, + "height 1 must be B1 after reorg"); + blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(b2.Hash!, + "height 2 must be B2 after reorg"); + + blockTree.FindBlock(a1.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("A1 must not be canonical"); + blockTree.FindBlock(a2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("A2 must not be canonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_after_reorg_to_lower_height_unmarks_higher_levels() + { + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + // Chain: A1 → A2 (head at height 2) + Block a1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + Block a2 = Build.A.Block.WithNumber(2).WithParent(a1).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(a1); + blockTree.SuggestBlock(a2); + blockTree.UpdateMainChain(new[] { a1, a2 }, true); + + // Reorg to B1 at height 1 (lower than current head) + Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 3 }).TestObject; + blockTree.SuggestBlock(b1); + blockTree.UpdateMainChain(new[] { b1 }, true); + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(b1.Hash!, + "height 1 must be B1 after reorg to lower height"); + + // Height 2 must be unmarked — no canonical block there anymore + blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "height 2 must be unmarked after reorging to height 1"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_canonical_and_non_canonical_siblings_are_both_accessible_by_hash() + { + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.SuggestBlock(blockB); + blockTree.UpdateMainChain(new[] { blockA }, true); + blockTree.UpdateMainChain(new[] { blockB }, true); + + // B is canonical — accessible both by hash and by number + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.None)!.Hash.Should().Be(blockB.Hash!); + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockB.Hash!); + + // A is not canonical — accessible by hash without RequireCanonical, null with RequireCanonical + blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.None)!.Hash.Should().Be(blockA.Hash!, + "non-canonical block A must still be accessible by hash without RequireCanonical"); + blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "non-canonical block A must not be returned when RequireCanonical is set"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_UpdateMainChain_called_twice_for_same_block_is_idempotent() + { + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + blockTree.UpdateMainChain(new[] { blockA }, true); // second call — must not corrupt state + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockA.Hash!, + "calling UpdateMainChain twice for the same block must not corrupt canonical state"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_canonical_state_consistent_after_reorg_to_fork_with_different_parent() + { + // Scenario matching the Gnosis node bug: + // P (height 1) → A (height 2, 20 txs) → canonical + // B (height 2, 32 txs, same parent P) → FCU makes B canonical + // C (height 3, child of A) → FCU makes A canonical again at height 2 + // + // After this, A is correctly canonical at height 2 from Nethermind's perspective. + // Both RequireCanonical and None lookups must agree. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block p = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0 }).TestObject; + blockTree.SuggestBlock(p); + blockTree.UpdateMainChain(new[] { p }, true); + + // A at height 2 — initially canonical (20 txs equivalent) + Block blockA = Build.A.Block.WithNumber(2).WithParent(p).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + + // B at height 2 — sibling of A (32 txs equivalent), becomes canonical via reorg + Block blockB = Build.A.Block.WithNumber(2).WithParent(p).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + blockTree.UpdateMainChain(new[] { blockB }, true); + + // C at height 3 — child of A (not B), FCU to C re-canonicalizes A + Block blockC = Build.A.Block.WithNumber(3).WithParent(blockA).WithExtraData(new byte[] { 3 }).TestObject; + blockTree.SuggestBlock(blockC); + blockTree.UpdateMainChain(new[] { blockA, blockC }, true); + + // A must now be canonical at height 2 (FCU to C forced A back) + Block? byNumberNoCanonical = blockTree.FindBlock(2, BlockTreeLookupOptions.None); + Block? byNumberWithCanonical = blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical); + + byNumberNoCanonical!.Hash.Should().Be(blockA.Hash!, "height 2 canonical must be A after FCU to C"); + byNumberWithCanonical!.Hash.Should().Be(blockA.Hash!, "RequireCanonical lookup at height 2 must return A"); + + // Both lookups must agree + byNumberNoCanonical.Hash.Should().Be(byNumberWithCanonical!.Hash, + "None and RequireCanonical lookups must return the same block"); + + // B is no longer canonical + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "B must not be canonical after FCU re-canonicalized A"); + + // C is canonical at height 3 + blockTree.FindBlock(3, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockC.Hash!, + "C must be canonical at height 3"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_three_siblings_third_becomes_canonical_SwapToMain_index_greater_than_one() + { + // Exercises SwapToMain with index > 1 (three blocks at the same height, + // the third one — index=2 — is made canonical last). + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + blockTree.UpdateMainChain(new[] { blockB }, true); + + Block blockC = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 3 }).TestObject; + blockTree.SuggestBlock(blockC); + blockTree.UpdateMainChain(new[] { blockC }, true); + + // C (the third sibling, originally at index 2) must now be canonical + Block? byNumber = blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); + byNumber.Should().NotBeNull("RequireCanonical lookup must find C"); + byNumber!.Hash.Should().Be(blockC.Hash!, "C is the last canonical, SwapToMain must have moved it to index 0"); + + // A and B must not be canonical + blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "A must not be canonical after C was set"); + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "B must not be canonical after C was set"); + + // All three are still findable by hash (non-canonical lookup) + blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.None).Should().NotBeNull("A findable by hash"); + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.None).Should().NotBeNull("B findable by hash"); + blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.None).Should().NotBeNull("C findable by hash"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_canonical_state_survives_BlockTree_reload_from_same_db() + { + // Verifies that the canonical marker (HasBlockOnMainChain / BlockInfos[0]) is + // persisted to the DB and correctly restored when a fresh BlockTree is constructed + // over the same DB instances — i.e. a node restart preserves canonical state. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + blockTree.UpdateMainChain(new[] { blockB }, true); + + // B is canonical before reload + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockB.Hash!, + "B must be canonical before reload"); + + // Reload: new BlockTree over the same DB instances (simulates node restart) + BlockTree reloadedTree = Build.A.BlockTree() + .WithBlocksDb(_blocksDb) + .WithHeadersDb(_headersDb) + .WithBlockInfoDb(_blocksInfosDb) + .WithoutSettingHead + .TestObject; + + Block? afterReload = reloadedTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); + afterReload.Should().NotBeNull("canonical block must be findable after reload"); + afterReload!.Hash.Should().Be(blockB.Hash!, + "B must still be canonical after BlockTree reload from the same DB"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_gap_in_canonical_chain_height_one_not_marked_when_height_two_is() + { + // Gap scenario: UpdateMainChain is called with a block at height 2 (C), + // but height 1 (B, C's parent) was never made canonical. + // The canonical chain is therefore inconsistent: height 2 is marked, + // height 1 is not. This documents the current behavior so that any + // accidental regression (e.g. silently marking height 1) is caught. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockB); + // Intentionally NOT calling UpdateMainChain for blockB + + Block blockC = Build.A.Block.WithNumber(2).WithParent(blockB).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockC); + blockTree.UpdateMainChain(new[] { blockC }, true); + + // C is marked canonical at height 2 + blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockC.Hash!, + "C must be canonical at height 2"); + + // B was never passed to UpdateMainChain, so height 1 has no canonical marker + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "B must NOT be canonical at height 1 — it was never passed to UpdateMainChain"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void IsMainChain_returns_correct_values_after_reorg() + { + // After A→B reorg: IsMainChain(B)=true, IsMainChain(A)=false. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + blockTree.UpdateMainChain(new[] { blockB }, true); + + blockTree.IsMainChain(blockB.Header).Should().BeTrue("B is canonical after reorg"); + blockTree.IsMainChain(blockA.Header).Should().BeFalse("A is no longer canonical after reorg"); + blockTree.IsMainChain(blockB.Hash!).Should().BeTrue("hash-based IsMainChain must agree for B"); + blockTree.IsMainChain(blockA.Hash!).Should().BeFalse("hash-based IsMainChain must agree for A"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_by_hash_with_RequireCanonical_returns_null_for_non_canonical_hash() + { + // FindBlock(Hash, RequireCanonical) uses a separate code path from number-based lookup. + // It must return null when the hash belongs to a non-canonical block. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + blockTree.UpdateMainChain(new[] { blockB }, true); + + // B canonical: hash lookup with RequireCanonical must succeed + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull( + "B is canonical — hash lookup with RequireCanonical must return it"); + + // A non-canonical: hash lookup with RequireCanonical must return null + blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "A is not canonical — hash lookup with RequireCanonical must return null"); + + // Without RequireCanonical both are still findable + blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.None).Should().NotBeNull("A findable without RequireCanonical"); + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.None).Should().NotBeNull("B findable without RequireCanonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_by_number_without_RequireCanonical_uses_best_difficulty_fallback_when_no_canonical_marked() + { + // When HasBlockOnMainChain=false (no UpdateMainChain called), GetBlockHashOnMainOrBestDifficultyHash + // falls back to the best-TD loop using >=, so the last SuggestBlock call wins (TD=0 in PoS). + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + // Suggest two siblings but do NOT call UpdateMainChain for either + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + + // Neither is canonical — RequireCanonical must return null + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "no canonical block at height 1 — RequireCanonical must return null"); + + // Without RequireCanonical the best-difficulty fallback returns a block. + // With TD=0 and >= comparison, the last SuggestBlock (B) wins. + Block? fallback = blockTree.FindBlock(1, BlockTreeLookupOptions.None); + fallback.Should().NotBeNull("best-difficulty fallback must return a block when no canonical is set"); + fallback!.Hash.Should().Be(blockB.Hash!, + "last SuggestBlock (B) wins the TD=0 >= fallback tie-break"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_UpdateMainChain_with_wereProcessed_false_still_marks_canonical() + { + // wereProcessed=false is used during sync to set canonical without updating Head. + // The canonical marker (HasBlockOnMainChain / BlockInfos[0]) must be set regardless. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, wereProcessed: true); + + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + + // Reorg to B with wereProcessed=false (sync path) + blockTree.UpdateMainChain(new[] { blockB }, wereProcessed: false); + + // Canonical marker must be updated even without wereProcessed + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockB.Hash!, + "B must be canonical at height 1 even when UpdateMainChain was called with wereProcessed=false"); + + blockTree.IsMainChain(blockB.Header).Should().BeTrue("B is canonical"); + blockTree.IsMainChain(blockA.Header).Should().BeFalse("A is no longer canonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void Canonical_chain_walk_every_ancestor_is_IsMainChain_true() + { + // Walk from canonical head back to genesis via ParentHash. + // Every block in the chain must satisfy IsMainChain=true. + // This catches the Gnosis-style bug where a child is canonical but its parent is not. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; + blockTree.SuggestBlock(b1); + blockTree.UpdateMainChain(new[] { b1 }, true); + + Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; + blockTree.SuggestBlock(b2); + blockTree.UpdateMainChain(new[] { b2 }, true); + + Block b3 = Build.A.Block.WithNumber(3).WithParent(b2).TestObject; + blockTree.SuggestBlock(b3); + blockTree.UpdateMainChain(new[] { b3 }, true); + + // Walk canonical chain from head down to genesis + BlockHeader? current = blockTree.FindHeader(3, BlockTreeLookupOptions.RequireCanonical); + current.Should().NotBeNull("canonical head must exist at height 3"); + + while (current!.Number > 0) + { + blockTree.IsMainChain(current).Should().BeTrue( + $"block at height {current.Number} (hash {current.Hash}) must be IsMainChain=true"); + current = blockTree.FindHeader(current.ParentHash!, BlockTreeLookupOptions.TotalDifficultyNotNeeded); + current.Should().NotBeNull($"parent must exist when walking canonical chain"); + } + + blockTree.IsMainChain(current!).Should().BeTrue("genesis must be IsMainChain=true"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_long_branch_reorg_all_new_heights_canonical_old_height_unmarked() + { + // Head at A(1). Reorg to B(1)→C(2)→D(3). + // After reorg: B, C, D canonical; A unmarked; heights 2 and 3 newly marked. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + + // Competing branch: B sibling of A, then C and D on top of B + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + + Block blockC = Build.A.Block.WithNumber(2).WithParent(blockB).TestObject; + blockTree.SuggestBlock(blockC); + + Block blockD = Build.A.Block.WithNumber(3).WithParent(blockC).TestObject; + blockTree.SuggestBlock(blockD); + + blockTree.UpdateMainChain(new[] { blockB, blockC, blockD }, true); + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockB.Hash!, "B canonical at height 1"); + blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockC.Hash!, "C canonical at height 2"); + blockTree.FindBlock(3, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockD.Hash!, "D canonical at height 3"); + blockTree.IsMainChain(blockA.Header).Should().BeFalse("A must be unmarked after reorg to longer branch"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_reorg_to_shorter_chain_higher_levels_unmarked() + { + // Head at height 3 (A1→A2→A3). Reorg to B1 (height 1 only). + // Heights 2 and 3 must be unmarked; B1 must be canonical at height 1. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block a1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(a1); + Block a2 = Build.A.Block.WithNumber(2).WithParent(a1).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(a2); + Block a3 = Build.A.Block.WithNumber(3).WithParent(a2).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(a3); + blockTree.UpdateMainChain(new[] { a1, a2, a3 }, true); + + // Reorg to a competing block at height 1 only + Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(b1); + blockTree.UpdateMainChain(new[] { b1 }, true); + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(b1.Hash!, "B1 canonical at height 1"); + blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("height 2 must be unmarked after reorg to shorter chain"); + blockTree.FindBlock(3, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("height 3 must be unmarked after reorg to shorter chain"); + blockTree.IsMainChain(a1.Header).Should().BeFalse("A1 no longer canonical"); + blockTree.IsMainChain(a2.Header).Should().BeFalse("A2 no longer canonical"); + blockTree.IsMainChain(a3.Header).Should().BeFalse("A3 no longer canonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindHeader_by_number_with_RequireCanonical_mirrors_FindBlock_behavior() + { + // FindHeader(long, RequireCanonical) uses GetBlockHashOnMainOrBestDifficultyHash then + // checks level.MainChainBlock — same logic as FindBlock but returning only the header. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(blockA); + blockTree.UpdateMainChain(new[] { blockA }, true); + + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(blockB); + blockTree.UpdateMainChain(new[] { blockB }, true); + + // B canonical: FindHeader with RequireCanonical must return B's header + BlockHeader? headerCanonical = blockTree.FindHeader(1, BlockTreeLookupOptions.RequireCanonical); + headerCanonical.Should().NotBeNull("canonical header must be found at height 1"); + headerCanonical!.Hash.Should().Be(blockB.Hash!, "FindHeader(RequireCanonical) must return B after reorg"); + + // A non-canonical: FindHeader(hash, RequireCanonical) must return null + blockTree.FindHeader(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "FindHeader by A's hash with RequireCanonical must return null — A is not canonical"); + + // FindHeader and FindBlock must agree on canonical hash + Block? blockCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); + blockCanonical!.Hash.Should().Be(headerCanonical.Hash, + "FindHeader and FindBlock must return the same canonical block at height 1"); + } } diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs index 8179adfee29b..8e4a2204cf80 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs @@ -273,6 +273,60 @@ public async Task block_should_not_be_canonical_before_forkchoiceUpdatedV1() chain.BlockTree.FindBlock(newHead, BlockTreeLookupOptions.None).Should().NotBeNull(); } + [Test] + public async Task fcu_to_ancestor_below_head_must_update_head_and_decanonicalise_descendants() + { + // Reproduces the IsOnMainChainBehindHead early-return bug: + // When FCU points to a block that is already canonical but BELOW the current head, + // the handler returns Valid immediately and skips UpdateMainChain. + // This leaves the head unchanged and the descendant blocks still canonical. + // + // Steps: + // FCU(A, height 1) → head = A + // newPayload(C, child of A, height 2) + FCU(C) → head = C + // FCU(A) again → should move head back to A and de-canonicalise C + // + // With the bug: FCU(A) is ignored (A.Number < Head.Number && IsMainChain(A)), + // leaving head = C and C still canonical. + using MergeTestBlockchain chain = await CreateBlockchain(); + IEngineRpcModule rpc = chain.EngineRpcModule; + Hash256 genesisHash = chain.BlockTree.HeadHash; + Hash256 zero = Keccak.Zero; + ulong ts = 1000; + + // Build and submit block A (height 1, parent = genesis) + ExecutionPayload payloadA = await BuildAndGetPayloadResult(rpc, chain, + genesisHash, zero, genesisHash, ts, Keccak.Zero, Address.Zero); + Hash256 hashA = payloadA.BlockHash!; + (await rpc.engine_newPayloadV1(payloadA)).Data.Status.Should().Be(PayloadStatus.Valid); + (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(hashA, zero, zero))) + .Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid); + chain.BlockTree.Head!.Hash.Should().Be(hashA, "head must be A after FCU(A)"); + + // Build and submit block C (height 2, parent = A) + ExecutionPayload payloadC = await BuildAndGetPayloadResult(rpc, chain, + hashA, zero, hashA, ts + 12, Keccak.Zero, Address.Zero); + Hash256 hashC = payloadC.BlockHash!; + (await rpc.engine_newPayloadV1(payloadC)).Data.Status.Should().Be(PayloadStatus.Valid); + (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(hashC, zero, zero))) + .Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid); + chain.BlockTree.Head!.Hash.Should().Be(hashC, "head must be C after FCU(C)"); + chain.BlockTree.IsMainChain(chain.BlockTree.FindHeader(hashA, BlockTreeLookupOptions.None)!).Should().BeTrue("A must be canonical as ancestor of C"); + chain.BlockTree.IsMainChain(chain.BlockTree.FindHeader(hashC, BlockTreeLookupOptions.None)!).Should().BeTrue("C must be canonical"); + + // FCU back to A — A is canonical but below the current head (C at height 2) + // Expected: head moves back to A, C is de-canonicalised + (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(hashA, zero, zero))) + .Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid); + + chain.BlockTree.Head!.Hash.Should().Be(hashA, + "head must return to A after FCU(A) — currently fails due to IsOnMainChainBehindHead early-return bug"); + chain.BlockTree.FindBlock(hashC, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "C must not be canonical after FCU reorged back to A"); + chain.BlockTree.FindBlock(hashA, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull( + "A must still be canonical"); + } + [Test] public async Task block_should_not_be_canonical_after_reorg() { diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs index 856dfa0cb570..00f3367fc3ba 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs @@ -246,11 +246,6 @@ protected virtual bool IsOnMainChainBehindHead(Block newHeadBlock, ForkchoiceSta return ForkchoiceUpdatedV1Result.Error(setHeadErrorMsg, ErrorCodes.InvalidParams); } - if (!IsOnMainChainBehindHead(newHeadBlock, forkchoiceState, out ResultWrapper? result)) - { - return result; - } - bool newHeadTheSameAsCurrentHead = _blockTree.Head!.Hash == newHeadBlock.Hash; bool shouldUpdateHead = !newHeadTheSameAsCurrentHead && blocks is not null; if (shouldUpdateHead) From 4de70a8a63feb0bae23fbb33a745e91808733e8f Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Thu, 19 Mar 2026 19:38:16 +0200 Subject: [PATCH 02/25] fix comments --- .../EngineModuleTests.V1.cs | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs index 8e4a2204cf80..0a2b27345f09 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs @@ -276,18 +276,22 @@ public async Task block_should_not_be_canonical_before_forkchoiceUpdatedV1() [Test] public async Task fcu_to_ancestor_below_head_must_update_head_and_decanonicalise_descendants() { - // Reproduces the IsOnMainChainBehindHead early-return bug: - // When FCU points to a block that is already canonical but BELOW the current head, - // the handler returns Valid immediately and skips UpdateMainChain. - // This leaves the head unchanged and the descendant blocks still canonical. + // Regression test for the Gnosis canonical-mismatch bug. // - // Steps: + // The bug: IsOnMainChainBehindHead returned true when FCU pointed to a block that + // was already canonical but below the current head. The handler returned Valid + // immediately, skipping UpdateMainChain. This left the head unchanged and the + // descendant (C, the wrong block) still marked canonical — so by-number RPC calls + // returned C instead of the correct canonical block A. + // + // Scenario: // FCU(A, height 1) → head = A - // newPayload(C, child of A, height 2) + FCU(C) → head = C - // FCU(A) again → should move head back to A and de-canonicalise C + // newPayload(C, child of A, height 2) + FCU(C) → head = C (C is now canonical) + // FCU(A) → the CL considers A the correct head (e.g. after a reorg on its side) + // A is already canonical and behind C, so the bug silently ignored it, + // leaving C canonical and the head stuck at C. // - // With the bug: FCU(A) is ignored (A.Number < Head.Number && IsMainChain(A)), - // leaving head = C and C still canonical. + // After the fix: FCU(A) moves the head back to A and de-canonicalises C. using MergeTestBlockchain chain = await CreateBlockchain(); IEngineRpcModule rpc = chain.EngineRpcModule; Hash256 genesisHash = chain.BlockTree.HeadHash; @@ -303,7 +307,7 @@ public async Task fcu_to_ancestor_below_head_must_update_head_and_decanonicalise .Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid); chain.BlockTree.Head!.Hash.Should().Be(hashA, "head must be A after FCU(A)"); - // Build and submit block C (height 2, parent = A) + // Build and submit block C (height 2, child of A) — C becomes the head ExecutionPayload payloadC = await BuildAndGetPayloadResult(rpc, chain, hashA, zero, hashA, ts + 12, Keccak.Zero, Address.Zero); Hash256 hashC = payloadC.BlockHash!; @@ -314,17 +318,15 @@ public async Task fcu_to_ancestor_below_head_must_update_head_and_decanonicalise chain.BlockTree.IsMainChain(chain.BlockTree.FindHeader(hashA, BlockTreeLookupOptions.None)!).Should().BeTrue("A must be canonical as ancestor of C"); chain.BlockTree.IsMainChain(chain.BlockTree.FindHeader(hashC, BlockTreeLookupOptions.None)!).Should().BeTrue("C must be canonical"); - // FCU back to A — A is canonical but below the current head (C at height 2) - // Expected: head moves back to A, C is de-canonicalised + // CL reorgs: FCU(A) — A is the correct canonical head, C should be de-canonicalised (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(hashA, zero, zero))) .Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid); - chain.BlockTree.Head!.Hash.Should().Be(hashA, - "head must return to A after FCU(A) — currently fails due to IsOnMainChainBehindHead early-return bug"); + chain.BlockTree.Head!.Hash.Should().Be(hashA, "head must return to A"); chain.BlockTree.FindBlock(hashC, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( "C must not be canonical after FCU reorged back to A"); chain.BlockTree.FindBlock(hashA, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull( - "A must still be canonical"); + "A must be canonical"); } [Test] From 3e6e6b9a4d70f4108d1b66b8a488a05124cf3d26 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Thu, 19 Mar 2026 20:52:49 +0200 Subject: [PATCH 03/25] fix comment --- src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index 214e93762a61..9fc8566e2a0b 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2232,7 +2232,7 @@ public void FindBlock_by_number_after_reorg_returns_new_canonical_with_and_witho // Without RequireCanonical (old BlockParameter(long) behavior) Block? withoutCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.None); - // With RequireCanonical (new BlockParameter(long) behavior after Ahmad's change) + // With RequireCanonical (new BlockParameter(long) behavior after RequireCanonical-by-number behavior change) Block? withCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); withoutCanonical.Should().NotBeNull(); From 4f89834a76dd4584599c912ac141224bcaf2a78f Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Thu, 19 Mar 2026 22:26:27 +0200 Subject: [PATCH 04/25] change approach --- .../BlockTreeTests.cs | 54 ++++++++++++++++++ .../Nethermind.Blockchain/BlockTree.cs | 19 +++++++ .../EngineModuleTests.V1.cs | 55 ------------------- .../Handlers/ForkchoiceUpdatedHandler.cs | 5 ++ 4 files changed, 78 insertions(+), 55 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index 9fc8566e2a0b..5bb3ab673d81 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2660,6 +2660,60 @@ public void FindBlock_UpdateMainChain_with_wereProcessed_false_still_marks_canon blockTree.IsMainChain(blockA.Header).Should().BeFalse("A is no longer canonical"); } + [Test, MaxTime(Timeout.MaxTestTime)] + public void UpdateMainChain_sync_marks_descendant_canonical_then_reorg_to_sibling_decanonalizes_it() + { + // Regression test for the Gnosis canonical-mismatch bug. + // + // BlockDownloader calls UpdateMainChain(block, wereProcessed: false) to mark synced + // blocks canonical without updating Head. When a reorg then arrives at the same height + // as the stale Head (previousHeadNumber == lastNumber), the old unmark loop + // (previousHeadNumber > lastNumber) is skipped, leaving the orphaned descendant + // wrongly canonical so eth_getBlockByNumber returns the wrong block. + // + // The fix adds an else-branch that scans upward from lastNumber+1 to clear any + // stale canonical markers left by the sync path. + // + // Scenario: + // UpdateMainChain([A], wereProcessed: true) — FCU(A): head = A at H=1. + // UpdateMainChain([C], wereProcessed: false) — sync: C canonical at H=2, head stays at A. + // UpdateMainChain([B], wereProcessed: true) — FCU(B, H=1, sibling of A): + // previousHeadNumber(1) == lastNumber(1) → else-branch fires → C at H=2 decanonalized. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + Block blockC = Build.A.Block.WithNumber(2).WithParent(blockA).TestObject; + + blockTree.SuggestBlock(blockA); + blockTree.SuggestBlock(blockB); + blockTree.SuggestBlock(blockC); + + // FCU(A): A becomes head at H=1. + blockTree.UpdateMainChain(new[] { blockA }, wereProcessed: true, forceUpdateHeadBlock: true); + blockTree.Head!.Hash.Should().Be(blockA.Hash!, "head must be A"); + + // Sync marks C canonical at H=2 without updating Head (BlockDownloader path). + blockTree.UpdateMainChain(new[] { blockC }, wereProcessed: false); + blockTree.Head!.Hash.Should().Be(blockA.Hash!, "head must stay at A — wereProcessed=false"); + blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical) + .Should().NotBeNull("C must be canonical at H=2 after sync marks it"); + + // FCU(B): reorg to sibling at the same height as the stale Head. + // previousHeadNumber(1) == lastNumber(1) → old loop skipped → else-branch must clear C. + blockTree.UpdateMainChain(new[] { blockB }, wereProcessed: true, forceUpdateHeadBlock: true); + + blockTree.Head!.Hash.Should().Be(blockB.Hash!, "head must be B after reorg"); + blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( + "C must not be canonical — its parent A was replaced by B"); + blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull( + "B must be canonical"); + } + [Test, MaxTime(Timeout.MaxTestTime)] public void Canonical_chain_walk_every_ancestor_is_IsMainChain_true() { diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index c273abf46dbe..8ebca91ce2bf 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -1013,6 +1013,25 @@ public void UpdateMainChain(IReadOnlyList blocks, bool wereProcessed, boo } } } + else + { + // Head can be stale when BlockchainProcessor calls UpdateMainChain with + // forceUpdateHeadBlock=false (e.g. during newPayload processing). In that case + // previousHeadNumber reflects a height we already moved past, so the loop above + // never runs and canonical markers above lastNumber are left unreachable. + // Scan upward from lastNumber+1 and clear any stale canonical levels we find. + for (long levelNumber = lastNumber + 1; ; levelNumber++) + { + ChainLevelInfo? level = LoadLevel(levelNumber); + if (level is null || !level.HasBlockOnMainChain) + { + break; + } + + level.HasBlockOnMainChain = false; + _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); + } + } for (int i = 0; i < blocks.Count; i++) { diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs index 0a2b27345f09..748894085b77 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs @@ -273,61 +273,6 @@ public async Task block_should_not_be_canonical_before_forkchoiceUpdatedV1() chain.BlockTree.FindBlock(newHead, BlockTreeLookupOptions.None).Should().NotBeNull(); } - [Test] - public async Task fcu_to_ancestor_below_head_must_update_head_and_decanonicalise_descendants() - { - // Regression test for the Gnosis canonical-mismatch bug. - // - // The bug: IsOnMainChainBehindHead returned true when FCU pointed to a block that - // was already canonical but below the current head. The handler returned Valid - // immediately, skipping UpdateMainChain. This left the head unchanged and the - // descendant (C, the wrong block) still marked canonical — so by-number RPC calls - // returned C instead of the correct canonical block A. - // - // Scenario: - // FCU(A, height 1) → head = A - // newPayload(C, child of A, height 2) + FCU(C) → head = C (C is now canonical) - // FCU(A) → the CL considers A the correct head (e.g. after a reorg on its side) - // A is already canonical and behind C, so the bug silently ignored it, - // leaving C canonical and the head stuck at C. - // - // After the fix: FCU(A) moves the head back to A and de-canonicalises C. - using MergeTestBlockchain chain = await CreateBlockchain(); - IEngineRpcModule rpc = chain.EngineRpcModule; - Hash256 genesisHash = chain.BlockTree.HeadHash; - Hash256 zero = Keccak.Zero; - ulong ts = 1000; - - // Build and submit block A (height 1, parent = genesis) - ExecutionPayload payloadA = await BuildAndGetPayloadResult(rpc, chain, - genesisHash, zero, genesisHash, ts, Keccak.Zero, Address.Zero); - Hash256 hashA = payloadA.BlockHash!; - (await rpc.engine_newPayloadV1(payloadA)).Data.Status.Should().Be(PayloadStatus.Valid); - (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(hashA, zero, zero))) - .Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid); - chain.BlockTree.Head!.Hash.Should().Be(hashA, "head must be A after FCU(A)"); - - // Build and submit block C (height 2, child of A) — C becomes the head - ExecutionPayload payloadC = await BuildAndGetPayloadResult(rpc, chain, - hashA, zero, hashA, ts + 12, Keccak.Zero, Address.Zero); - Hash256 hashC = payloadC.BlockHash!; - (await rpc.engine_newPayloadV1(payloadC)).Data.Status.Should().Be(PayloadStatus.Valid); - (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(hashC, zero, zero))) - .Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid); - chain.BlockTree.Head!.Hash.Should().Be(hashC, "head must be C after FCU(C)"); - chain.BlockTree.IsMainChain(chain.BlockTree.FindHeader(hashA, BlockTreeLookupOptions.None)!).Should().BeTrue("A must be canonical as ancestor of C"); - chain.BlockTree.IsMainChain(chain.BlockTree.FindHeader(hashC, BlockTreeLookupOptions.None)!).Should().BeTrue("C must be canonical"); - - // CL reorgs: FCU(A) — A is the correct canonical head, C should be de-canonicalised - (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(hashA, zero, zero))) - .Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid); - - chain.BlockTree.Head!.Hash.Should().Be(hashA, "head must return to A"); - chain.BlockTree.FindBlock(hashC, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "C must not be canonical after FCU reorged back to A"); - chain.BlockTree.FindBlock(hashA, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull( - "A must be canonical"); - } [Test] public async Task block_should_not_be_canonical_after_reorg() diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs index 00f3367fc3ba..856dfa0cb570 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs @@ -246,6 +246,11 @@ protected virtual bool IsOnMainChainBehindHead(Block newHeadBlock, ForkchoiceSta return ForkchoiceUpdatedV1Result.Error(setHeadErrorMsg, ErrorCodes.InvalidParams); } + if (!IsOnMainChainBehindHead(newHeadBlock, forkchoiceState, out ResultWrapper? result)) + { + return result; + } + bool newHeadTheSameAsCurrentHead = _blockTree.Head!.Hash == newHeadBlock.Hash; bool shouldUpdateHead = !newHeadTheSameAsCurrentHead && blocks is not null; if (shouldUpdateHead) From 9dad4011f7ff7f2e1da01d5e08720f7d840741ec Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Thu, 19 Mar 2026 22:30:05 +0200 Subject: [PATCH 05/25] remove empty line --- .../Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs index 748894085b77..8179adfee29b 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs @@ -273,7 +273,6 @@ public async Task block_should_not_be_canonical_before_forkchoiceUpdatedV1() chain.BlockTree.FindBlock(newHead, BlockTreeLookupOptions.None).Should().NotBeNull(); } - [Test] public async Task block_should_not_be_canonical_after_reorg() { From c090be2cbb14939ed1910644713477c516e04725 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Thu, 19 Mar 2026 23:28:51 +0200 Subject: [PATCH 06/25] HealCanonicalChain implementation & tests --- src/Nethermind/Nethermind.Api/IInitConfig.cs | 13 ++ src/Nethermind/Nethermind.Api/InitConfig.cs | 2 + .../BlockTreeTests.cs | 217 ++++++++++++++++++ .../Nethermind.Blockchain/BlockTree.cs | 80 +++++++ .../Nethermind.Blockchain/BlockTreeOverlay.cs | 1 + .../Nethermind.Blockchain/IBlockTree.cs | 8 + .../ReadOnlyBlockTree.cs | 1 + .../Stateless/StatelessBlockTree.cs | 3 + .../Nethermind.Init/Steps/ReviewBlockTree.cs | 15 ++ 9 files changed, 340 insertions(+) diff --git a/src/Nethermind/Nethermind.Api/IInitConfig.cs b/src/Nethermind/Nethermind.Api/IInitConfig.cs index 525c066d1886..7915247c3eea 100644 --- a/src/Nethermind/Nethermind.Api/IInitConfig.cs +++ b/src/Nethermind/Nethermind.Api/IInitConfig.cs @@ -98,6 +98,19 @@ public interface IInitConfig : IConfig [ConfigItem(Description = "[TECHNICAL] True when in runner test. Disable some wait.", DefaultValue = "false", HiddenFromDocs = true)] bool InRunnerTest { get; set; } + + [ConfigItem( + Description = "On startup, walk backward from the current head for up to `HealCanonicalChainDepth` " + + "blocks via parentHash and repair any incorrect `HasBlockOnMainChain` markers left by " + + "the beacon-sync path. Also clears stale canonical markers above the head. " + + "Use this flag once after observing a canonical-mismatch (wrong `eth_getBlockByNumber` results).", + DefaultValue = "false")] + bool HealCanonicalChainOnStartup { get; set; } + + [ConfigItem( + Description = "Number of blocks to walk back from the head when `HealCanonicalChain` is enabled.", + DefaultValue = "8192")] + long HealCanonicalChainDepth { get; set; } } public enum DiagnosticMode diff --git a/src/Nethermind/Nethermind.Api/InitConfig.cs b/src/Nethermind/Nethermind.Api/InitConfig.cs index 914849cc7be8..0a4d31541a2b 100644 --- a/src/Nethermind/Nethermind.Api/InitConfig.cs +++ b/src/Nethermind/Nethermind.Api/InitConfig.cs @@ -41,6 +41,8 @@ public class InitConfig : IInitConfig public int BackgroundTaskMaxNumber { get; set; } = 2048; public bool InRunnerTest { get; set; } = false; public string? DataDir { get; set; } + public bool HealCanonicalChainOnStartup { get; set; } = false; + public long HealCanonicalChainDepth { get; set; } = 8192; [Obsolete("Use DiagnosticMode with MemDb instead")] public bool UseMemDb diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index 5bb3ab673d81..e9ea58b2d272 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2714,6 +2714,223 @@ public void UpdateMainChain_sync_marks_descendant_canonical_then_reorg_to_siblin "B must be canonical"); } + [Test, MaxTime(Timeout.MaxTestTime)] + public void HealCanonicalChain_clears_stale_marker_above_head_left_by_sync() + { + // Scenario: sync (wereProcessed=false) marks C canonical at H=2 without updating Head. + // HealCanonicalChain(head=A) must scan upward and clear C's stale marker. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; + Block blockC = Build.A.Block.WithNumber(2).WithParent(blockA).TestObject; + + blockTree.SuggestBlock(blockA); + blockTree.SuggestBlock(blockC); + + // FCU: head = A at H=1 + blockTree.UpdateMainChain(new[] { blockA }, wereProcessed: true, forceUpdateHeadBlock: true); + + // Sync marks C canonical at H=2 without updating Head + blockTree.UpdateMainChain(new[] { blockC }, wereProcessed: false); + blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical) + .Should().NotBeNull("precondition: C is canonical before heal"); + + blockTree.HealCanonicalChain(blockA.Hash!, maxBlockDepth: 10); + + blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical) + .Should().BeNull("C must be decanonalized after heal"); + blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical) + .Should().NotBeNull("A must remain canonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void HealCanonicalChain_fixes_wrong_canonical_block_at_height() + { + // Scenario: A and B are siblings at H=1. B was swapped to index 0 by accident + // (e.g. a stale write), but the real canonical chain goes through A. + // HealCanonicalChain walking from A must swap A back to index 0. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + + blockTree.SuggestBlock(blockA); + blockTree.SuggestBlock(blockB); + + // Make A canonical first, then B (leaving B at index 0, A at index 1) + blockTree.UpdateMainChain(new[] { blockA }, wereProcessed: true, forceUpdateHeadBlock: true); + blockTree.UpdateMainChain(new[] { blockB }, wereProcessed: false); // B wrongly becomes canonical + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash + .Should().Be(blockB.Hash!, "precondition: B is wrongly canonical"); + + blockTree.HealCanonicalChain(blockA.Hash!, maxBlockDepth: 10); + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash + .Should().Be(blockA.Hash!, "A must be canonical after heal"); + blockTree.IsMainChain(blockB.Header).Should().BeFalse("B must not be canonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void HealCanonicalChain_does_not_alter_already_consistent_chain() + { + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; + Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; + + blockTree.SuggestBlock(b1); + blockTree.SuggestBlock(b2); + blockTree.UpdateMainChain(new[] { b1, b2 }, wereProcessed: true, forceUpdateHeadBlock: true); + + blockTree.HealCanonicalChain(b2.Hash!, maxBlockDepth: 10); + + blockTree.FindBlock(b2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("b2 must remain canonical"); + blockTree.FindBlock(b1.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("b1 must remain canonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void HealCanonicalChain_does_nothing_for_unknown_start_hash() + { + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + // Should not throw — unknown hash is treated as a no-op. + blockTree.Invoking(bt => bt.HealCanonicalChain(TestItem.KeccakA, maxBlockDepth: 10)) + .Should().NotThrow(); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void HealCanonicalChain_clears_multiple_stale_levels_above_head() + { + // Sync left THREE levels above head canonical — heal must clear all of them. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; + Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; + Block b3 = Build.A.Block.WithNumber(3).WithParent(b2).TestObject; + Block b4 = Build.A.Block.WithNumber(4).WithParent(b3).TestObject; + + blockTree.SuggestBlock(b1); + blockTree.SuggestBlock(b2); + blockTree.SuggestBlock(b3); + blockTree.SuggestBlock(b4); + + // FCU: head = b1 at H=1 + blockTree.UpdateMainChain(new[] { b1 }, wereProcessed: true, forceUpdateHeadBlock: true); + + // Sync marks b2, b3, b4 canonical without updating Head + blockTree.UpdateMainChain(new[] { b2 }, wereProcessed: false); + blockTree.UpdateMainChain(new[] { b3 }, wereProcessed: false); + blockTree.UpdateMainChain(new[] { b4 }, wereProcessed: false); + + blockTree.HealCanonicalChain(b1.Hash!, maxBlockDepth: 10); + + blockTree.FindBlock(b2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("H=2 stale marker must be cleared"); + blockTree.FindBlock(b3.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("H=3 stale marker must be cleared"); + blockTree.FindBlock(b4.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("H=4 stale marker must be cleared"); + blockTree.FindBlock(b1.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("b1 must remain canonical"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void HealCanonicalChain_fixes_both_directions_in_one_pass() + { + // Combined scenario: wrong canonical at H=1 AND stale markers at H=2,3 above head. + // This is the realistic production scenario: FCU moved head to B at H=1, + // but C (child of A) was left canonical at H=2 by sync, and A is still canonical at H=1 + // when it shouldn't be. + // + // genesis → A(H=1) → C(H=2) ← sync left C canonical, A at H=1 + // → B(H=1) ← heal starts from B, the correct head + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + Block blockC = Build.A.Block.WithNumber(2).WithParent(blockA).TestObject; + + blockTree.SuggestBlock(blockA); + blockTree.SuggestBlock(blockB); + blockTree.SuggestBlock(blockC); + + // FCU(A): A is canonical at H=1, B is known but not canonical. + // Sync marks C canonical at H=2 without updating Head. + // No FCU for B — the heal is told B is the correct head (e.g. via the CL reorg). + blockTree.UpdateMainChain(new[] { blockA }, wereProcessed: true, forceUpdateHeadBlock: true); + blockTree.UpdateMainChain(new[] { blockC }, wereProcessed: false); // sync: C canonical at H=2, head stays A + + // Preconditions: A canonical at H=1, C stale-canonical at H=2, B suggested but not canonical + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash + .Should().Be(blockA.Hash!, "precondition: A is canonical at H=1"); + blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical) + .Should().NotBeNull("precondition: C is stale-canonical at H=2"); + + // Heal from B — the CL says B is the correct head. + // Must both: clear C at H=2 (upward scan) and swap B to index 0 at H=1 (downward walk). + blockTree.HealCanonicalChain(blockB.Hash!, maxBlockDepth: 10); + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash + .Should().Be(blockB.Hash!, "B must be canonical at H=1 after heal"); + blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical) + .Should().BeNull("C must not be canonical — it was orphaned when B replaced A"); + } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void HealCanonicalChain_respects_maxBlockDepth() + { + // With maxBlockDepth=1, the walk only checks the start block and one parent. + // A broken marker two levels below must NOT be repaired. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + Block b1Alt = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; + + blockTree.SuggestBlock(b1); + blockTree.SuggestBlock(b1Alt); + blockTree.SuggestBlock(b2); + + // b1Alt wrongly canonical at H=1, b2 canonical at H=2 (head) + blockTree.UpdateMainChain(new[] { b1 }, wereProcessed: true, forceUpdateHeadBlock: true); + blockTree.UpdateMainChain(new[] { b2 }, wereProcessed: true, forceUpdateHeadBlock: true); + blockTree.UpdateMainChain(new[] { b1Alt }, wereProcessed: false); // breaks H=1 + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash + .Should().Be(b1Alt.Hash!, "precondition: H=1 is broken"); + + // Heal from b2 with depth=0: only checks b2, does not reach H=1 + blockTree.HealCanonicalChain(b2.Hash!, maxBlockDepth: 0); + + blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash + .Should().Be(b1Alt.Hash!, "H=1 must remain broken — it is beyond maxBlockDepth"); + } + [Test, MaxTime(Timeout.MaxTestTime)] public void Canonical_chain_walk_every_ancestor_is_IsMainChain_true() { diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index 8ebca91ce2bf..54e8a153055e 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -1054,6 +1054,86 @@ public void UpdateMainChain(IReadOnlyList blocks, bool wereProcessed, boo OnUpdateMainChain?.Invoke(this, new OnUpdateMainChainArgs(blocks, wereProcessed)); } + public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) + { + BlockHeader? start = FindHeader(startHash, BlockTreeLookupOptions.None); + if (start is null) return; + + // BatchWrite is a write-buffered, deferred batch: PersistLevel only enqueues writes + // and nothing is flushed to the underlying DB until Dispose(). Both phases therefore + // commit atomically — a crash before Dispose leaves the DB unchanged. + using BatchWrite batch = _chainLevelInfoRepository.StartBatch(); + + long repairedAbove = ClearStaleMarkersAbove(start.Number, batch); + long repairedBelow = RepairMarkersBelow(start, maxBlockDepth, batch); + long repaired = repairedAbove + repairedBelow; + + if (Logger.IsInfo) Logger.Info($"Canonical chain heal complete: {repaired} level(s) repaired ({repairedAbove} stale above head cleared, {repairedBelow} incorrect markers fixed)."); + } + + private long ClearStaleMarkersAbove(long headNumber, BatchWrite batch) + { + long cleared = 0L; + for (long levelNumber = headNumber + 1; ; levelNumber++) + { + ChainLevelInfo? level = LoadLevel(levelNumber); + if (level is null || !level.HasBlockOnMainChain) break; + level.HasBlockOnMainChain = false; + _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); + cleared++; + } + return cleared; + } + + private long RepairMarkersBelow(BlockHeader start, long maxBlockDepth, BatchWrite batch) + { + long repairedCount = 0L; + long blocksWalked = 0L; + BlockHeader? current = start; + + while (current is not null && blocksWalked < maxBlockDepth) + { + ChainLevelInfo? level = LoadLevel(current.Number); + if (level is not null) + { + int? index = level.FindIndex(current.Hash!); + if (index is null) + { + if (Logger.IsWarn) Logger.Warn($"Canonical heal: block {current.Hash} at height {current.Number} not found in any BlockInfo slot — repair halted."); + break; + } + + bool needsPersist = index.Value != 0 || !level.HasBlockOnMainChain; + + if (index.Value != 0) + level.SwapToMain(index.Value); + + if (!level.HasBlockOnMainChain) + level.HasBlockOnMainChain = true; + + if (needsPersist) + { + _chainLevelInfoRepository.PersistLevel(current.Number, level, batch); + repairedCount++; + } + } + + if (current.IsGenesis) break; + + BlockHeader? parent = FindHeader(current.ParentHash!, BlockTreeLookupOptions.None); + if (parent is null) + { + if (Logger.IsWarn) Logger.Warn($"Canonical heal: parent {current.ParentHash} of block {current.Number} not found — chain may be pruned, repair halted."); + break; + } + + current = parent; + blocksWalked++; + } + + return repairedCount; + } + private void TryUpdateSyncPivot() { BlockHeader? newPivotHeader = null; diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs b/src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs index d776bc7895fa..ae862b499cfd 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs @@ -245,6 +245,7 @@ public void UpdateBeaconMainChain(BlockInfo[]? blockInfos, long clearBeaconMainC _overlayTree.UpdateBeaconMainChain(blockInfos, clearBeaconMainChainStartPoint); public void RecalculateTreeLevels() => _overlayTree.RecalculateTreeLevels(); + public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) => _overlayTree.HealCanonicalChain(startHash, maxBlockDepth); public (long BlockNumber, Hash256 BlockHash) SyncPivot { get => _baseTree.SyncPivot; diff --git a/src/Nethermind/Nethermind.Blockchain/IBlockTree.cs b/src/Nethermind/Nethermind.Blockchain/IBlockTree.cs index f3531b6f8f91..67fa3cf9286f 100644 --- a/src/Nethermind/Nethermind.Blockchain/IBlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/IBlockTree.cs @@ -191,6 +191,14 @@ AddBlockResult Insert(Block block, BlockTreeInsertBlockOptions insertBlockOption void RecalculateTreeLevels(); + /// + /// Repairs canonical chain markers starting from , walking backward + /// up to blocks. Also removes stale canonical markers above + /// left by the beacon-sync path. Safe to run at startup after + /// observing canonical mismatches (wrong eth_getBlockByNumber results). + /// + void HealCanonicalChain(Hash256 startHash, long maxBlockDepth); + /// /// Sync pivot is mainly concerned with old blocks and receipts. /// After sync pivot, blocks and headers should be continuous. diff --git a/src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs b/src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs index 867c44dc267b..3db94ffc941b 100644 --- a/src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs @@ -193,6 +193,7 @@ public int DeleteChainSlice(in long startNumber, long? endNumber = null, bool fo public bool IsBetterThanHead(BlockHeader? header) => _wrapped.IsBetterThanHead(header); public void UpdateBeaconMainChain(BlockInfo[]? blockInfos, long clearBeaconMainChainStartPoint) => throw new InvalidOperationException($"{nameof(ReadOnlyBlockTree)} does not expect {nameof(UpdateBeaconMainChain)} calls"); public void RecalculateTreeLevels() => throw new InvalidOperationException($"{nameof(ReadOnlyBlockTree)} does not expect {nameof(RecalculateTreeLevels)} calls"); + public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) => throw new InvalidOperationException($"{nameof(ReadOnlyBlockTree)} does not expect {nameof(HealCanonicalChain)} calls"); public (long BlockNumber, Hash256 BlockHash) SyncPivot { get => _wrapped.SyncPivot; diff --git a/src/Nethermind/Nethermind.Consensus/Stateless/StatelessBlockTree.cs b/src/Nethermind/Nethermind.Consensus/Stateless/StatelessBlockTree.cs index a21bc941b58b..acef3e8f5c87 100644 --- a/src/Nethermind/Nethermind.Consensus/Stateless/StatelessBlockTree.cs +++ b/src/Nethermind/Nethermind.Consensus/Stateless/StatelessBlockTree.cs @@ -192,6 +192,9 @@ public void UpdateBeaconMainChain(BlockInfo[]? blockInfos, long clearBeaconMainC public void RecalculateTreeLevels() => throw new NotSupportedException(); + public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) + => throw new NotSupportedException(); + public (long BlockNumber, Hash256 BlockHash) SyncPivot { get => throw new NotSupportedException(); diff --git a/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs b/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs index 74abe0545993..c4170dcb0a0a 100644 --- a/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs +++ b/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs @@ -10,6 +10,7 @@ using Nethermind.Blockchain.Synchronization; using Nethermind.Blockchain.Visitors; using Nethermind.Consensus.Processing; +using Nethermind.Core.Crypto; using Nethermind.Logging; using Nethermind.State; @@ -29,6 +30,20 @@ ILogManager logManager public Task Execute(CancellationToken cancellationToken) { + if (initConfig.HealCanonicalChainOnStartup) + { + Hash256? startHash = blockTree.Head?.Hash; + if (startHash is not null) + { + if (_logger.IsInfo) _logger.Info($"Healing canonical chain from head {startHash} (depth {initConfig.HealCanonicalChainDepth})..."); + blockTree.HealCanonicalChain(startHash, initConfig.HealCanonicalChainDepth); + } + else + { + if (_logger.IsWarn) _logger.Warn("HealCanonicalChainOnStartup requested but no head block found — skipping."); + } + } + if (initConfig.ProcessingEnabled) { return RunBlockTreeInitTasks(cancellationToken); From 55d1487f9f47c709eb3b258b37068f30f1ef5cc9 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Thu, 19 Mar 2026 23:48:34 +0200 Subject: [PATCH 07/25] fix build --- src/Nethermind/Nethermind.PerfTest/Program.cs | 2 ++ src/Nethermind/Nethermind.TxPool.Test/TestBlockTree.cs | 1 + 2 files changed, 3 insertions(+) diff --git a/src/Nethermind/Nethermind.PerfTest/Program.cs b/src/Nethermind/Nethermind.PerfTest/Program.cs index cb29f8d7ab9f..cb7b734ba3d9 100644 --- a/src/Nethermind/Nethermind.PerfTest/Program.cs +++ b/src/Nethermind/Nethermind.PerfTest/Program.cs @@ -219,6 +219,8 @@ public int DeleteChainSlice(in long startNumber, long? endNumber) { return _blockTree.DeleteChainSlice(startNumber, endNumber); } + + public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) { } } private const string DbBasePath = @"C:\perf_db"; diff --git a/src/Nethermind/Nethermind.TxPool.Test/TestBlockTree.cs b/src/Nethermind/Nethermind.TxPool.Test/TestBlockTree.cs index a8473dc82663..f49c5824e705 100644 --- a/src/Nethermind/Nethermind.TxPool.Test/TestBlockTree.cs +++ b/src/Nethermind/Nethermind.TxPool.Test/TestBlockTree.cs @@ -105,4 +105,5 @@ public void ForkChoiceUpdated(Hash256? finalizedBlockHash, Hash256? safeBlockBlo public bool IsBetterThanHead(BlockHeader? header) => false; public void UpdateBeaconMainChain(BlockInfo[]? blockInfos, long clearBeaconMainChainStartPoint) { } public void RecalculateTreeLevels() { } + public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) { } } From 28b3961510b08b8d52b966c1eca5f42f506e0e99 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Fri, 20 Mar 2026 15:03:47 +0200 Subject: [PATCH 08/25] ePBS FCU fix --- .../BlockTreeTests.cs | 59 +++++++++++++++++++ .../Nethermind.Blockchain/BlockTree.cs | 31 +++++----- 2 files changed, 75 insertions(+), 15 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index e9ea58b2d272..f051dc71eb73 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2714,6 +2714,65 @@ public void UpdateMainChain_sync_marks_descendant_canonical_then_reorg_to_siblin "B must be canonical"); } + [Test, MaxTime(Timeout.MaxTestTime)] + public void UpdateMainChain_fcu_to_ancestor_with_stale_head_clears_all_beacon_synced_descendants() + { + // ePBS scenario: FCU can reorg to an ancestor (not just a sibling at the same height). + // If head is stale because beacon sync marked descendants canonical without updating Head, + // a subsequent FCU to an ancestor must clear ALL canonical markers above the ancestor — + // including beacon-synced blocks above the stale head that the IF branch cannot reach. + // + // Scenario: + // genesis → b1(H=1) → b2(H=2) → b3(H=3) → b4(H=4) + // + // UpdateMainChain([b1], wereProcessed: true) — FCU(b1): head = b1 at H=1. + // UpdateMainChain([b2], wereProcessed: false) — beacon sync: b2 canonical, head stays at b1. + // UpdateMainChain([b3], wereProcessed: false) — beacon sync: b3 canonical, head stays at b1. + // UpdateMainChain([b4], wereProcessed: false) — beacon sync: b4 canonical, head stays at b1. + // UpdateMainChain([genesis], wereProcessed: true) — ePBS FCU to ancestor at H=0: + // previousHeadNumber(1) > lastNumber(0) → IF branch clears H=1 only. + // b2, b3, b4 are NOT cleared — they are above the stale head and invisible to the IF branch. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + + Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; + Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; + Block b3 = Build.A.Block.WithNumber(3).WithParent(b2).TestObject; + Block b4 = Build.A.Block.WithNumber(4).WithParent(b3).TestObject; + + blockTree.SuggestBlock(b1); + blockTree.SuggestBlock(b2); + blockTree.SuggestBlock(b3); + blockTree.SuggestBlock(b4); + + // FCU(b1): head = b1 at H=1. + blockTree.UpdateMainChain(new[] { b1 }, wereProcessed: true, forceUpdateHeadBlock: true); + + // Beacon sync: b2, b3, b4 marked canonical without updating Head. + blockTree.UpdateMainChain(new[] { b2 }, wereProcessed: false); + blockTree.UpdateMainChain(new[] { b3 }, wereProcessed: false); + blockTree.UpdateMainChain(new[] { b4 }, wereProcessed: false); + + // Preconditions: head stale at b1, b2-b4 canonical via beacon sync. + blockTree.Head!.Hash.Should().Be(b1.Hash!, "precondition: head stale at b1"); + blockTree.FindBlock(b2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("precondition: b2 beacon-synced canonical"); + blockTree.FindBlock(b4.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("precondition: b4 beacon-synced canonical"); + + // ePBS FCU to ancestor: reorg back to genesis at H=0. + // The IF branch (previousHeadNumber=1 > lastNumber=0) clears b1 at H=1. + // b2, b3, b4 must also be cleared — they are above the stale head. + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true, forceUpdateHeadBlock: true); + + blockTree.FindBlock(genesis.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("genesis must be canonical"); + blockTree.FindBlock(b1.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("b1 must be de-canonicalized"); + blockTree.FindBlock(b2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("b2 must be de-canonicalized — beacon sync stale marker above stale head"); + blockTree.FindBlock(b3.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("b3 must be de-canonicalized — beacon sync stale marker above stale head"); + blockTree.FindBlock(b4.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("b4 must be de-canonicalized — beacon sync stale marker above stale head"); + } + [Test, MaxTime(Timeout.MaxTestTime)] public void HealCanonicalChain_clears_stale_marker_above_head_left_by_sync() { diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index 54e8a153055e..731e43b57023 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -999,6 +999,8 @@ public void UpdateMainChain(IReadOnlyList blocks, bool wereProcessed, boo long lastNumber = ascendingOrder ? blocks[^1].Number : blocks[0].Number; long previousHeadNumber = Head?.Number ?? 0L; using BatchWrite batch = _chainLevelInfoRepository.StartBatch(); + // Downward unmark: clear levels from previousHead down to lastNumber+1. + // Handles the normal reorg case where the new chain is shorter than the old head. if (previousHeadNumber > lastNumber) { for (long i = 0; i < previousHeadNumber - lastNumber; i++) @@ -1013,24 +1015,23 @@ public void UpdateMainChain(IReadOnlyList blocks, bool wereProcessed, boo } } } - else + + // Upward scan: clear any stale canonical markers above max(previousHead, lastNumber). + // Beacon sync (wereProcessed=false) marks blocks canonical without updating Head, + // leaving stale markers the downward loop cannot reach. This covers two cases: + // - FCU at same/higher height (previousHeadNumber <= lastNumber): scans from lastNumber+1. + // - ePBS FCU to ancestor with stale head: scans from previousHeadNumber+1, clearing + // beacon-synced markers above the stale head that the downward loop misses. + for (long levelNumber = Math.Max(previousHeadNumber, lastNumber) + 1; ; levelNumber++) { - // Head can be stale when BlockchainProcessor calls UpdateMainChain with - // forceUpdateHeadBlock=false (e.g. during newPayload processing). In that case - // previousHeadNumber reflects a height we already moved past, so the loop above - // never runs and canonical markers above lastNumber are left unreachable. - // Scan upward from lastNumber+1 and clear any stale canonical levels we find. - for (long levelNumber = lastNumber + 1; ; levelNumber++) + ChainLevelInfo? level = LoadLevel(levelNumber); + if (level is null || !level.HasBlockOnMainChain) { - ChainLevelInfo? level = LoadLevel(levelNumber); - if (level is null || !level.HasBlockOnMainChain) - { - break; - } - - level.HasBlockOnMainChain = false; - _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); + break; } + + level.HasBlockOnMainChain = false; + _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); } for (int i = 0; i < blocks.Count; i++) From 3f6cb8aec6e3057e86e92b45aa9b21c7bbb1db9e Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Mon, 23 Mar 2026 21:46:59 +0200 Subject: [PATCH 09/25] geth parity with https://github.com/ethereum/go-ethereum/blob/745b0a8c09ad9d0866da67403ffa99d11ba70ec3/core/rawdb/accessors_chain.go#L47 return null for orphaned heights post-merge in GetBlockHashOnMainOrBestDifficultyHash --- .../BlockTreeTests.cs | 48 +++++++++++++++++++ .../Nethermind.Blockchain/BlockTree.cs | 9 ++++ 2 files changed, 57 insertions(+) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index f051dc71eb73..b9b06996acaa 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -3127,4 +3127,52 @@ public void FindHeader_by_number_with_RequireCanonical_mirrors_FindBlock_behavio blockCanonical!.Hash.Should().Be(headerCanonical.Hash, "FindHeader and FindBlock must return the same canonical block at height 1"); } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_by_number_after_reorg_returns_null_not_orphaned_block_in_pos() + { + // In PoS all blocks share the same cumulative TotalDifficulty (difficulty=0 per block). + // The PoW-era "best difficulty" fallback in GetBlockHashOnMainOrBestDifficultyHash + // fires when HasBlockOnMainChain=false and returns whichever block it finds first — + // which after a reorg is the now-orphaned block. FindBlock(height, None) must return + // null for a decanonized height in PoS, not the orphaned block. + CustomSpecProvider specProvider = new(((ForkActivation)0, London.Instance)) + { + TerminalTotalDifficulty = UInt256.Zero // pure PoS from genesis (e.g. Gnosis) + }; + + _blocksDb = new TestMemDb(); + _headersDb = new TestMemDb(); + _blocksInfosDb = new TestMemDb(); + BlockTree blockTree = Build.A.BlockTree(specProvider) + .WithBlocksDb(_blocksDb) + .WithHeadersDb(_headersDb) + .WithBlockInfoDb(_blocksInfosDb) + .WithoutSettingHead + .TestObject; + + Block genesis = Build.A.Block.WithNumber(0).WithDifficulty(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + // Old chain: genesis → A1 → A2 (head at height 2) + Block a1 = Build.A.Block.WithNumber(1).WithDifficulty(0).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(a1); + Block a2 = Build.A.Block.WithNumber(2).WithDifficulty(0).WithParent(a1).WithExtraData(new byte[] { 1 }).TestObject; + blockTree.SuggestBlock(a2); + blockTree.UpdateMainChain(new[] { a1, a2 }, true); + + // Reorg: genesis → B1 (head drops from height 2 to height 1, different block) + Block b1 = Build.A.Block.WithNumber(1).WithDifficulty(0).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; + blockTree.SuggestBlock(b1); + blockTree.UpdateMainChain(new[] { b1 }, true); + + // Height 2 was orphaned: must return null, not the stale A2 + blockTree.FindBlock(2, BlockTreeLookupOptions.None).Should().BeNull( + "orphaned height 2 must return null after reorg in PoS — not the stale A2 block"); + + // Height 1 must return the new canonical B1 + blockTree.FindBlock(1, BlockTreeLookupOptions.None)!.Hash.Should().Be(b1.Hash!, + "height 1 must return B1 after reorg"); + } } diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index 731e43b57023..8f91734132ec 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -767,6 +767,15 @@ as it does not require the step of resolving number -> hash */ return level.BlockInfos[0].BlockHash; } + // Post-merge: block difficulty is 0, so TotalDifficulty never increases and all + // blocks at the same height share the same cumulative TD. The best-difficulty + // fallback below cannot distinguish canonical from orphaned — it would return + // a stale block after a reorg. Return null: there is no canonical block here. + bool isPostMerge = SpecProvider.TerminalTotalDifficulty is not null + && (Head?.TotalDifficulty ?? UInt256.Zero) >= SpecProvider.TerminalTotalDifficulty; + if (isPostMerge) return null; + + // Pre-merge: the block with the highest total difficulty is canonical by PoW rule. UInt256 bestDifficultySoFar = UInt256.Zero; Hash256 bestHash = null; for (int i = 0; i < level.BlockInfos.Length; i++) From 883eafdb5837a3bd00877d63c664471ce22eec93 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Tue, 24 Mar 2026 12:42:29 +0200 Subject: [PATCH 10/25] GetBlockHashOnMainOrBestDifficultyHash now returns null when HasBlockOnMainChain=true but WasProcessed=false and blockNumber > Head.Number. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Beacon sync calls UpdateMainChain(wereProcessed=false), setting HasBlockOnMainChain without advancing Head. If this races with a cleanup FCU where previousHeadNumber == lastNumber, the upward scan runs before the marker is set and cannot clear it — leaving a stale marker that eth_getBlockByNumber would return as canonical. The write-time scan cannot close this window; a read-time guard can. WasProcessed=false precisely identifies beacon-sync markers: processed canonical blocks always have WasProcessed=true, so startup/reload paths are unaffected. --- .../BlockTreeTests.cs | 42 +++++++++++++++++++ .../Nethermind.Blockchain/BlockTree.cs | 12 +++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index b9b06996acaa..1734a84ddb03 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -3175,4 +3175,46 @@ public void FindBlock_by_number_after_reorg_returns_null_not_orphaned_block_in_p blockTree.FindBlock(1, BlockTreeLookupOptions.None)!.Hash.Should().Be(b1.Hash!, "height 1 must return B1 after reorg"); } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void FindBlock_by_number_returns_null_when_beacon_sync_marks_block_canonical_above_head() + { + // Simulates the race condition where beacon sync calls UpdateMainChain(wereProcessed=false) + // for a block above the current head, leaving a stale HasBlockOnMainChain marker. + // GetBlockHashOnMainOrBestDifficultyHash must detect the unprocessed marker above Head + // and return null rather than the stale block. + CustomSpecProvider specProvider = new(((ForkActivation)0, London.Instance)) + { + TerminalTotalDifficulty = UInt256.Zero + }; + _blocksDb = new TestMemDb(); + _headersDb = new TestMemDb(); + _blocksInfosDb = new TestMemDb(); + BlockTree blockTree = Build.A.BlockTree(specProvider) + .WithBlocksDb(_blocksDb) + .WithHeadersDb(_headersDb) + .WithBlockInfoDb(_blocksInfosDb) + .WithoutSettingHead + .TestObject; + + Block genesis = Build.A.Block.WithNumber(0).WithDifficulty(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, true); + + Block head = Build.A.Block.WithNumber(1).WithDifficulty(0).WithParent(genesis).TestObject; + blockTree.SuggestBlock(head); + blockTree.UpdateMainChain(new[] { head }, true); // Head = 1, WasProcessed = true + + Block beaconBlock = Build.A.Block.WithNumber(2).WithDifficulty(0).WithParent(head).TestObject; + blockTree.SuggestBlock(beaconBlock); + blockTree.UpdateMainChain(new[] { beaconBlock }, false); // HasBlockOnMainChain=true, WasProcessed=false + + // Height 2 is a stale beacon marker — must not be returned as canonical + blockTree.FindBlock(2, BlockTreeLookupOptions.None).Should().BeNull( + "height 2 is above Head and was only beacon-synced (WasProcessed=false) — must not be returned"); + + // Height 1 is the real head and must still be found + blockTree.FindBlock(1, BlockTreeLookupOptions.None)!.Hash.Should().Be(head.Hash!, + "height 1 is the canonical head and must still be findable"); + } } diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index 8f91734132ec..5365654f7edf 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -764,7 +764,17 @@ as it does not require the step of resolving number -> hash */ if (level.HasBlockOnMainChain) { - return level.BlockInfos[0].BlockHash; + BlockInfo blockInfo = level.BlockInfos[0]; + // A beacon-sync marker has HasBlockOnMainChain=true but WasProcessed=false. + // Such markers can be left above the real head when the upward-scan in + // UpdateMainChain races with beacon sync (scan runs before the marker is + // set, so it cannot clear it). Any unprocessed marker above Head is stale. + if (!blockInfo.WasProcessed && Head is not null && blockNumber > Head.Number) + { + return null; + } + + return blockInfo.BlockHash; } // Post-merge: block difficulty is 0, so TotalDifficulty never increases and all From e83b46d445be4cb18284cf667a4fb8482a530568 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Tue, 24 Mar 2026 15:14:11 +0200 Subject: [PATCH 11/25] revert change for WasProcessed=false --- .../BlockTreeTests.cs | 42 ------------------- .../Nethermind.Blockchain/BlockTree.cs | 12 +----- 2 files changed, 1 insertion(+), 53 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index 1734a84ddb03..b9b06996acaa 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -3175,46 +3175,4 @@ public void FindBlock_by_number_after_reorg_returns_null_not_orphaned_block_in_p blockTree.FindBlock(1, BlockTreeLookupOptions.None)!.Hash.Should().Be(b1.Hash!, "height 1 must return B1 after reorg"); } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_by_number_returns_null_when_beacon_sync_marks_block_canonical_above_head() - { - // Simulates the race condition where beacon sync calls UpdateMainChain(wereProcessed=false) - // for a block above the current head, leaving a stale HasBlockOnMainChain marker. - // GetBlockHashOnMainOrBestDifficultyHash must detect the unprocessed marker above Head - // and return null rather than the stale block. - CustomSpecProvider specProvider = new(((ForkActivation)0, London.Instance)) - { - TerminalTotalDifficulty = UInt256.Zero - }; - _blocksDb = new TestMemDb(); - _headersDb = new TestMemDb(); - _blocksInfosDb = new TestMemDb(); - BlockTree blockTree = Build.A.BlockTree(specProvider) - .WithBlocksDb(_blocksDb) - .WithHeadersDb(_headersDb) - .WithBlockInfoDb(_blocksInfosDb) - .WithoutSettingHead - .TestObject; - - Block genesis = Build.A.Block.WithNumber(0).WithDifficulty(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block head = Build.A.Block.WithNumber(1).WithDifficulty(0).WithParent(genesis).TestObject; - blockTree.SuggestBlock(head); - blockTree.UpdateMainChain(new[] { head }, true); // Head = 1, WasProcessed = true - - Block beaconBlock = Build.A.Block.WithNumber(2).WithDifficulty(0).WithParent(head).TestObject; - blockTree.SuggestBlock(beaconBlock); - blockTree.UpdateMainChain(new[] { beaconBlock }, false); // HasBlockOnMainChain=true, WasProcessed=false - - // Height 2 is a stale beacon marker — must not be returned as canonical - blockTree.FindBlock(2, BlockTreeLookupOptions.None).Should().BeNull( - "height 2 is above Head and was only beacon-synced (WasProcessed=false) — must not be returned"); - - // Height 1 is the real head and must still be found - blockTree.FindBlock(1, BlockTreeLookupOptions.None)!.Hash.Should().Be(head.Hash!, - "height 1 is the canonical head and must still be findable"); - } } diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index 5365654f7edf..8f91734132ec 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -764,17 +764,7 @@ as it does not require the step of resolving number -> hash */ if (level.HasBlockOnMainChain) { - BlockInfo blockInfo = level.BlockInfos[0]; - // A beacon-sync marker has HasBlockOnMainChain=true but WasProcessed=false. - // Such markers can be left above the real head when the upward-scan in - // UpdateMainChain races with beacon sync (scan runs before the marker is - // set, so it cannot clear it). Any unprocessed marker above Head is stale. - if (!blockInfo.WasProcessed && Head is not null && blockNumber > Head.Number) - { - return null; - } - - return blockInfo.BlockHash; + return level.BlockInfos[0].BlockHash; } // Post-merge: block difficulty is 0, so TotalDifficulty never increases and all From 590c37baed2bd0cbc061be0bcdb1384aa7aca8d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Chodo=C5=82a?= Date: Tue, 24 Mar 2026 14:25:00 +0100 Subject: [PATCH 12/25] test: add beacon sync + reorg stale marker reproduction test Reproduction of the stale canonical markers bug from the Engine API test generator: beacon sync marks H+1, H+2, H+3 canonical without advancing Head, then FCU reorgs to a sibling at the same height as Head. Verifies all orphaned levels are de-canonicalized. --- .../BlockTreeTests.cs | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index b9b06996acaa..1713de1af89d 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2714,6 +2714,80 @@ public void UpdateMainChain_sync_marks_descendant_canonical_then_reorg_to_siblin "B must be canonical"); } + [Test, MaxTime(Timeout.MaxTestTime)] + public void UpdateMainChain_beacon_sync_multiple_descendants_then_reorg_to_sibling_clears_all() + { + // Exact reproduction of the stale canonical markers bug from the Engine API test generator. + // + // Beacon sync marks H+1, H+2, H+3 canonical without updating Head (wereProcessed: false). + // FCU reorgs to a sibling of Head at the SAME height H. + // previousHeadNumber == lastNumber → downward unmark skipped entirely. + // Upward scan must clear all three orphaned levels. + // + // This differs from the single-descendant test above: with multiple levels, the bounded + // scan (not break-on-first-gap) is critical — a concurrent MoveToMain could create a gap. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true, forceUpdateHeadBlock: true); + + // Chain: genesis → headBlock(H=1) → d1(H=2) → d2(H=3) → d3(H=4) + Block headBlock = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0xAA }).TestObject; + Block d1 = Build.A.Block.WithNumber(2).WithParent(headBlock).TestObject; + Block d2 = Build.A.Block.WithNumber(3).WithParent(d1).TestObject; + Block d3 = Build.A.Block.WithNumber(4).WithParent(d2).TestObject; + + blockTree.SuggestBlock(headBlock); + blockTree.SuggestBlock(d1); + blockTree.SuggestBlock(d2); + blockTree.SuggestBlock(d3); + + // FCU sets Head to headBlock at H=1 + blockTree.UpdateMainChain(new[] { headBlock }, wereProcessed: true, forceUpdateHeadBlock: true); + blockTree.Head!.Hash.Should().Be(headBlock.Hash!); + + // Beacon sync: d1, d2, d3 marked canonical without advancing Head + blockTree.UpdateMainChain(new[] { d1 }, wereProcessed: false); + blockTree.UpdateMainChain(new[] { d2 }, wereProcessed: false); + blockTree.UpdateMainChain(new[] { d3 }, wereProcessed: false); + + // Step 2: verify stale Head + blockTree.Head!.Number.Should().Be(1, "Head must stay at H=1 — wereProcessed=false"); + blockTree.IsMainChain(d1.Header).Should().BeTrue("precondition: d1 canonical via beacon sync"); + blockTree.IsMainChain(d2.Header).Should().BeTrue("precondition: d2 canonical via beacon sync"); + blockTree.IsMainChain(d3.Header).Should().BeTrue("precondition: d3 canonical via beacon sync"); + + // Step 3: FCU reorg to sibling at same height H=1 + Block sibling = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0xBB }).TestObject; + blockTree.SuggestBlock(sibling); + blockTree.UpdateMainChain(new[] { sibling }, wereProcessed: true, forceUpdateHeadBlock: true); + + // Step 4: verify reorg correctness + blockTree.Head!.Number.Should().Be(1); + blockTree.Head!.Hash.Should().Be(sibling.Hash!); + blockTree.IsMainChain(sibling.Header).Should().BeTrue("sibling must be canonical"); + blockTree.IsMainChain(d1.Header).Should().BeFalse("d1 must be de-canonicalized after reorg"); + blockTree.IsMainChain(d2.Header).Should().BeFalse("d2 must be de-canonicalized after reorg"); + blockTree.IsMainChain(d3.Header).Should().BeFalse("d3 must be de-canonicalized after reorg"); + + // Step 5: verify user-visible impact — FindCanonicalBlockInfo must return null + blockTree.FindCanonicalBlockInfo(2).Should().BeNull("H+1 must return null — orphaned after reorg"); + blockTree.FindCanonicalBlockInfo(3).Should().BeNull("H+2 must return null — orphaned after reorg"); + blockTree.FindCanonicalBlockInfo(4).Should().BeNull("H+3 must return null — orphaned after reorg"); + + // Step 6: verify block hashes — canonical lookup returns correct hash at H=1 + BlockInfo? infoAt1 = blockTree.FindCanonicalBlockInfo(1); + infoAt1.Should().NotBeNull(); + infoAt1!.BlockHash.Should().Be(sibling.Hash!, "H=1 must return sibling's hash, not old headBlock"); + infoAt1.BlockHash.Should().NotBe(headBlock.Hash!, "H=1 must NOT return old headBlock's hash"); + + // Orphaned heights must not return old block hashes via canonical lookup + blockTree.FindCanonicalBlockInfo(2)?.BlockHash.Should().NotBe(d1.Hash!, "H+1 canonical hash must not be d1"); + blockTree.FindCanonicalBlockInfo(3)?.BlockHash.Should().NotBe(d2.Hash!, "H+2 canonical hash must not be d2"); + blockTree.FindCanonicalBlockInfo(4)?.BlockHash.Should().NotBe(d3.Hash!, "H+3 canonical hash must not be d3"); + } + [Test, MaxTime(Timeout.MaxTestTime)] public void UpdateMainChain_fcu_to_ancestor_with_stale_head_clears_all_beacon_synced_descendants() { From 1da5cf6fa906d0b461873a56eec41b1b34942ffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Chodo=C5=82a?= Date: Tue, 24 Mar 2026 14:42:26 +0100 Subject: [PATCH 13/25] test: add failing gap test for beacon sync race condition Adds UpdateMainChain_beacon_sync_gap_in_stale_markers_leaves_orphan_after_reorg which reproduces the scenario where a concurrent MoveToMain creates a gap in stale canonical markers during beacon sync. The break-on-first-gap upward scan stops at the gap and leaves d3 orphaned as stale canonical. This test FAILS on canonical-fix (expected) and PASSES on bounded-scan. --- .../BlockTreeTests.cs | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index 1713de1af89d..5ee4c419557d 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -3249,4 +3249,73 @@ public void FindBlock_by_number_after_reorg_returns_null_not_orphaned_block_in_p blockTree.FindBlock(1, BlockTreeLookupOptions.None)!.Hash.Should().Be(b1.Hash!, "height 1 must return B1 after reorg"); } + + [Test, MaxTime(Timeout.MaxTestTime)] + public void UpdateMainChain_beacon_sync_gap_in_stale_markers_leaves_orphan_after_reorg() + { + // Reproduces the race-condition gap scenario on top of the beacon sync path: + // + // genesis → headBlock(H=1) → d1(H=2) → d2(H=3) → d3(H=4) + // + // 1. FCU(headBlock): Head = headBlock at H=1. + // 2. Beacon sync marks d1, d2, d3 canonical (wereProcessed=false, Head stays at H=1). + // 3. Concurrent MoveToMain clears d2's HasBlockOnMainChain → gap at H=3. + // 4. FCU(sibling) at H=1: previousHeadNumber==lastNumber==1 → Phase 1 skips. + // Phase 2 upward scan: clears d1(H=2), hits gap at d2(H=3), breaks. + // d3(H=4) remains stale canonical — BUG. + // + // This test FAILS on canonical-fix (break-on-first-gap) and PASSES on bounded-scan. + BlockTree blockTree = BuildBlockTree(); + + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true, forceUpdateHeadBlock: true); + + Block headBlock = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0xAA }).TestObject; + Block d1 = Build.A.Block.WithNumber(2).WithParent(headBlock).TestObject; + Block d2 = Build.A.Block.WithNumber(3).WithParent(d1).TestObject; + Block d3 = Build.A.Block.WithNumber(4).WithParent(d2).TestObject; + + blockTree.SuggestBlock(headBlock); + blockTree.SuggestBlock(d1); + blockTree.SuggestBlock(d2); + blockTree.SuggestBlock(d3); + + // FCU(headBlock): head = headBlock at H=1 + blockTree.UpdateMainChain(new[] { headBlock }, wereProcessed: true, forceUpdateHeadBlock: true); + blockTree.Head!.Hash.Should().Be(headBlock.Hash!); + + // Beacon sync: d1, d2, d3 marked canonical, Head stays at H=1 + blockTree.UpdateMainChain(new[] { d1 }, wereProcessed: false); + blockTree.UpdateMainChain(new[] { d2 }, wereProcessed: false); + blockTree.UpdateMainChain(new[] { d3 }, wereProcessed: false); + + blockTree.Head!.Number.Should().Be(1, "Head must stay at H=1 — wereProcessed=false"); + blockTree.IsMainChain(d1.Header).Should().BeTrue("precondition: d1 canonical via beacon sync"); + blockTree.IsMainChain(d2.Header).Should().BeTrue("precondition: d2 canonical via beacon sync"); + blockTree.IsMainChain(d3.Header).Should().BeTrue("precondition: d3 canonical via beacon sync"); + + // Simulate race: concurrent MoveToMain clears d2's marker, creating a gap at H=3 + ChainLevelInfo? levelD2 = blockTree.FindLevel(d2.Number); + levelD2!.HasBlockOnMainChain = false; + + // Precondition: gap exists + blockTree.IsMainChain(d2.Header).Should().BeFalse("precondition: d2 gap — cleared by simulated race"); + blockTree.IsMainChain(d3.Header).Should().BeTrue("precondition: d3 still stale canonical past the gap"); + + // FCU(sibling) at H=1: reorg to sibling, previousHeadNumber==lastNumber==1 + Block sibling = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0xBB }).TestObject; + blockTree.SuggestBlock(sibling); + blockTree.UpdateMainChain(new[] { sibling }, wereProcessed: true, forceUpdateHeadBlock: true); + + blockTree.Head!.Hash.Should().Be(sibling.Hash!); + blockTree.IsMainChain(sibling.Header).Should().BeTrue("sibling must be canonical"); + blockTree.IsMainChain(d1.Header).Should().BeFalse("d1 must be de-canonicalized after reorg"); + blockTree.IsMainChain(d2.Header).Should().BeFalse("d2 must stay non-canonical (gap)"); + blockTree.IsMainChain(d3.Header).Should().BeFalse("d3 must be de-canonicalized — bounded scan must reach past the gap"); + + blockTree.FindCanonicalBlockInfo(2).Should().BeNull("H+1 must be null — orphaned after reorg"); + blockTree.FindCanonicalBlockInfo(3).Should().BeNull("H+2 must be null — gap, non-canonical"); + blockTree.FindCanonicalBlockInfo(4).Should().BeNull("H+3 must be null — orphaned, must not survive the gap"); + } } From a8cf7c792addbd84f9f74066b4e9480e6ce2826b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Chodo=C5=82a?= Date: Tue, 24 Mar 2026 14:53:31 +0100 Subject: [PATCH 14/25] fix: skip gaps in upward scan instead of breaking Change Phase 2 upward scan and ClearStaleMarkersAbove to continue past levels where HasBlockOnMainChain is false, breaking only when the level does not exist. This handles gaps left by concurrent MoveToMain without needing a BestKnownNumber bound. --- .../Nethermind.Blockchain/BlockTree.cs | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index 8f91734132ec..20d087bb8a34 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -1034,13 +1034,12 @@ public void UpdateMainChain(IReadOnlyList blocks, bool wereProcessed, boo for (long levelNumber = Math.Max(previousHeadNumber, lastNumber) + 1; ; levelNumber++) { ChainLevelInfo? level = LoadLevel(levelNumber); - if (level is null || !level.HasBlockOnMainChain) + if (level is null) break; + if (level.HasBlockOnMainChain) { - break; + level.HasBlockOnMainChain = false; + _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); } - - level.HasBlockOnMainChain = false; - _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); } for (int i = 0; i < blocks.Count; i++) @@ -1087,10 +1086,13 @@ private long ClearStaleMarkersAbove(long headNumber, BatchWrite batch) for (long levelNumber = headNumber + 1; ; levelNumber++) { ChainLevelInfo? level = LoadLevel(levelNumber); - if (level is null || !level.HasBlockOnMainChain) break; - level.HasBlockOnMainChain = false; - _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); - cleared++; + if (level is null) break; + if (level.HasBlockOnMainChain) + { + level.HasBlockOnMainChain = false; + _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); + cleared++; + } } return cleared; } From 292985c034ac996aedcee69deeca686fa6d9c386 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Wed, 25 Mar 2026 18:47:45 +0200 Subject: [PATCH 15/25] PR cleanup --- src/Nethermind/Nethermind.Api/IInitConfig.cs | 2 +- src/Nethermind/Nethermind.Api/InitConfig.cs | 2 +- .../BlockTreeTests.cs | 555 +----------------- .../Nethermind.Blockchain/BlockTree.cs | 51 +- .../Nethermind.Init/Steps/ReviewBlockTree.cs | 30 +- 5 files changed, 45 insertions(+), 595 deletions(-) diff --git a/src/Nethermind/Nethermind.Api/IInitConfig.cs b/src/Nethermind/Nethermind.Api/IInitConfig.cs index 7915247c3eea..849e0cb0d270 100644 --- a/src/Nethermind/Nethermind.Api/IInitConfig.cs +++ b/src/Nethermind/Nethermind.Api/IInitConfig.cs @@ -105,7 +105,7 @@ public interface IInitConfig : IConfig "the beacon-sync path. Also clears stale canonical markers above the head. " + "Use this flag once after observing a canonical-mismatch (wrong `eth_getBlockByNumber` results).", DefaultValue = "false")] - bool HealCanonicalChainOnStartup { get; set; } + bool HealCanonicalChain { get; set; } [ConfigItem( Description = "Number of blocks to walk back from the head when `HealCanonicalChain` is enabled.", diff --git a/src/Nethermind/Nethermind.Api/InitConfig.cs b/src/Nethermind/Nethermind.Api/InitConfig.cs index 0a4d31541a2b..946af2a0f16a 100644 --- a/src/Nethermind/Nethermind.Api/InitConfig.cs +++ b/src/Nethermind/Nethermind.Api/InitConfig.cs @@ -41,7 +41,7 @@ public class InitConfig : IInitConfig public int BackgroundTaskMaxNumber { get; set; } = 2048; public bool InRunnerTest { get; set; } = false; public string? DataDir { get; set; } - public bool HealCanonicalChainOnStartup { get; set; } = false; + public bool HealCanonicalChain { get; set; } = false; public long HealCanonicalChainDepth { get; set; } = 8192; [Obsolete("Use DiagnosticMode with MemDb instead")] diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index 5ee4c419557d..e523d256d009 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2203,241 +2203,7 @@ public void On_UpdateMainBranch_UpdateSyncPivot_ToHeaderUnderReorgDepth() } [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_by_number_after_reorg_returns_new_canonical_with_and_without_RequireCanonical() - { - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - // Block A at height 1 — initially canonical - Block blockA = Build.A.Block - .WithNumber(1) - .WithParent(genesis) - .WithExtraData(new byte[] { 1 }) - .TestObject; - blockTree.SuggestBlock(blockA); - blockTree.UpdateMainChain(new[] { blockA }, true); - - // Block B at height 1 — same parent, different block (reorg) - Block blockB = Build.A.Block - .WithNumber(1) - .WithParent(genesis) - .WithExtraData(new byte[] { 2 }) - .TestObject; - blockTree.SuggestBlock(blockB); - blockTree.UpdateMainChain(new[] { blockB }, true); - - // Without RequireCanonical (old BlockParameter(long) behavior) - Block? withoutCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.None); - - // With RequireCanonical (new BlockParameter(long) behavior after RequireCanonical-by-number behavior change) - Block? withCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); - - withoutCanonical.Should().NotBeNull(); - withCanonical.Should().NotBeNull(); - - withoutCanonical!.Hash.Should().Be(blockB.Hash!, - "FindBlock without RequireCanonical must return the current canonical block B after reorg"); - withCanonical!.Hash.Should().Be(blockB.Hash!, - "FindBlock with RequireCanonical must return the current canonical block B after reorg, not the old canonical A"); - - withCanonical.Hash.Should().Be(withoutCanonical.Hash, - "RequireCanonical and non-canonical number lookups must agree on which block is canonical"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_after_double_reorg_A_to_B_back_to_A_returns_A() - { - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.SuggestBlock(blockB); - - // A canonical → reorg to B → reorg back to A - blockTree.UpdateMainChain(new[] { blockA }, true); - blockTree.UpdateMainChain(new[] { blockB }, true); - blockTree.UpdateMainChain(new[] { blockA }, true); - - Block? withoutCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.None); - Block? withCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); - - withoutCanonical!.Hash.Should().Be(blockA.Hash!, "after reorg back to A, None lookup must return A"); - withCanonical!.Hash.Should().Be(blockA.Hash!, "after reorg back to A, RequireCanonical lookup must return A"); - blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "B must not be canonical after reorg back to A"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_after_multi_block_reorg_both_heights_updated() - { - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - // Original chain: A1 → A2 - Block a1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - Block a2 = Build.A.Block.WithNumber(2).WithParent(a1).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(a1); - blockTree.SuggestBlock(a2); - blockTree.UpdateMainChain(new[] { a1, a2 }, true); - - // Reorg chain: B1 → B2 - Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 3 }).TestObject; - Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).WithExtraData(new byte[] { 4 }).TestObject; - blockTree.SuggestBlock(b1); - blockTree.SuggestBlock(b2); - blockTree.UpdateMainChain(new[] { b1, b2 }, true); - - blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(b1.Hash!, - "height 1 must be B1 after reorg"); - blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(b2.Hash!, - "height 2 must be B2 after reorg"); - - blockTree.FindBlock(a1.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("A1 must not be canonical"); - blockTree.FindBlock(a2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("A2 must not be canonical"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_after_reorg_to_lower_height_unmarks_higher_levels() - { - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - // Chain: A1 → A2 (head at height 2) - Block a1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - Block a2 = Build.A.Block.WithNumber(2).WithParent(a1).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(a1); - blockTree.SuggestBlock(a2); - blockTree.UpdateMainChain(new[] { a1, a2 }, true); - - // Reorg to B1 at height 1 (lower than current head) - Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 3 }).TestObject; - blockTree.SuggestBlock(b1); - blockTree.UpdateMainChain(new[] { b1 }, true); - - blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(b1.Hash!, - "height 1 must be B1 after reorg to lower height"); - - // Height 2 must be unmarked — no canonical block there anymore - blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "height 2 must be unmarked after reorging to height 1"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_canonical_and_non_canonical_siblings_are_both_accessible_by_hash() - { - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.SuggestBlock(blockB); - blockTree.UpdateMainChain(new[] { blockA }, true); - blockTree.UpdateMainChain(new[] { blockB }, true); - - // B is canonical — accessible both by hash and by number - blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.None)!.Hash.Should().Be(blockB.Hash!); - blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockB.Hash!); - - // A is not canonical — accessible by hash without RequireCanonical, null with RequireCanonical - blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.None)!.Hash.Should().Be(blockA.Hash!, - "non-canonical block A must still be accessible by hash without RequireCanonical"); - blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "non-canonical block A must not be returned when RequireCanonical is set"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_UpdateMainChain_called_twice_for_same_block_is_idempotent() - { - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.UpdateMainChain(new[] { blockA }, true); - blockTree.UpdateMainChain(new[] { blockA }, true); // second call — must not corrupt state - - blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockA.Hash!, - "calling UpdateMainChain twice for the same block must not corrupt canonical state"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_canonical_state_consistent_after_reorg_to_fork_with_different_parent() - { - // Scenario matching the Gnosis node bug: - // P (height 1) → A (height 2, 20 txs) → canonical - // B (height 2, 32 txs, same parent P) → FCU makes B canonical - // C (height 3, child of A) → FCU makes A canonical again at height 2 - // - // After this, A is correctly canonical at height 2 from Nethermind's perspective. - // Both RequireCanonical and None lookups must agree. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block p = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0 }).TestObject; - blockTree.SuggestBlock(p); - blockTree.UpdateMainChain(new[] { p }, true); - - // A at height 2 — initially canonical (20 txs equivalent) - Block blockA = Build.A.Block.WithNumber(2).WithParent(p).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.UpdateMainChain(new[] { blockA }, true); - - // B at height 2 — sibling of A (32 txs equivalent), becomes canonical via reorg - Block blockB = Build.A.Block.WithNumber(2).WithParent(p).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockB); - blockTree.UpdateMainChain(new[] { blockB }, true); - - // C at height 3 — child of A (not B), FCU to C re-canonicalizes A - Block blockC = Build.A.Block.WithNumber(3).WithParent(blockA).WithExtraData(new byte[] { 3 }).TestObject; - blockTree.SuggestBlock(blockC); - blockTree.UpdateMainChain(new[] { blockA, blockC }, true); - - // A must now be canonical at height 2 (FCU to C forced A back) - Block? byNumberNoCanonical = blockTree.FindBlock(2, BlockTreeLookupOptions.None); - Block? byNumberWithCanonical = blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical); - - byNumberNoCanonical!.Hash.Should().Be(blockA.Hash!, "height 2 canonical must be A after FCU to C"); - byNumberWithCanonical!.Hash.Should().Be(blockA.Hash!, "RequireCanonical lookup at height 2 must return A"); - - // Both lookups must agree - byNumberNoCanonical.Hash.Should().Be(byNumberWithCanonical!.Hash, - "None and RequireCanonical lookups must return the same block"); - - // B is no longer canonical - blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "B must not be canonical after FCU re-canonicalized A"); - - // C is canonical at height 3 - blockTree.FindBlock(3, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockC.Hash!, - "C must be canonical at height 3"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_three_siblings_third_becomes_canonical_SwapToMain_index_greater_than_one() + public void FindBlock_WhenThirdOfThreeSiblingsIsCanonical_ReturnsThatSibling() { // Exercises SwapToMain with index > 1 (three blocks at the same height, // the third one — index=2 — is made canonical last). @@ -2477,162 +2243,7 @@ public void FindBlock_three_siblings_third_becomes_canonical_SwapToMain_index_gr } [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_canonical_state_survives_BlockTree_reload_from_same_db() - { - // Verifies that the canonical marker (HasBlockOnMainChain / BlockInfos[0]) is - // persisted to the DB and correctly restored when a fresh BlockTree is constructed - // over the same DB instances — i.e. a node restart preserves canonical state. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.UpdateMainChain(new[] { blockA }, true); - - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockB); - blockTree.UpdateMainChain(new[] { blockB }, true); - - // B is canonical before reload - blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockB.Hash!, - "B must be canonical before reload"); - - // Reload: new BlockTree over the same DB instances (simulates node restart) - BlockTree reloadedTree = Build.A.BlockTree() - .WithBlocksDb(_blocksDb) - .WithHeadersDb(_headersDb) - .WithBlockInfoDb(_blocksInfosDb) - .WithoutSettingHead - .TestObject; - - Block? afterReload = reloadedTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); - afterReload.Should().NotBeNull("canonical block must be findable after reload"); - afterReload!.Hash.Should().Be(blockB.Hash!, - "B must still be canonical after BlockTree reload from the same DB"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_gap_in_canonical_chain_height_one_not_marked_when_height_two_is() - { - // Gap scenario: UpdateMainChain is called with a block at height 2 (C), - // but height 1 (B, C's parent) was never made canonical. - // The canonical chain is therefore inconsistent: height 2 is marked, - // height 1 is not. This documents the current behavior so that any - // accidental regression (e.g. silently marking height 1) is caught. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockB); - // Intentionally NOT calling UpdateMainChain for blockB - - Block blockC = Build.A.Block.WithNumber(2).WithParent(blockB).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockC); - blockTree.UpdateMainChain(new[] { blockC }, true); - - // C is marked canonical at height 2 - blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockC.Hash!, - "C must be canonical at height 2"); - - // B was never passed to UpdateMainChain, so height 1 has no canonical marker - blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "B must NOT be canonical at height 1 — it was never passed to UpdateMainChain"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void IsMainChain_returns_correct_values_after_reorg() - { - // After A→B reorg: IsMainChain(B)=true, IsMainChain(A)=false. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.UpdateMainChain(new[] { blockA }, true); - - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockB); - blockTree.UpdateMainChain(new[] { blockB }, true); - - blockTree.IsMainChain(blockB.Header).Should().BeTrue("B is canonical after reorg"); - blockTree.IsMainChain(blockA.Header).Should().BeFalse("A is no longer canonical after reorg"); - blockTree.IsMainChain(blockB.Hash!).Should().BeTrue("hash-based IsMainChain must agree for B"); - blockTree.IsMainChain(blockA.Hash!).Should().BeFalse("hash-based IsMainChain must agree for A"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_by_hash_with_RequireCanonical_returns_null_for_non_canonical_hash() - { - // FindBlock(Hash, RequireCanonical) uses a separate code path from number-based lookup. - // It must return null when the hash belongs to a non-canonical block. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.UpdateMainChain(new[] { blockA }, true); - - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockB); - blockTree.UpdateMainChain(new[] { blockB }, true); - - // B canonical: hash lookup with RequireCanonical must succeed - blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull( - "B is canonical — hash lookup with RequireCanonical must return it"); - - // A non-canonical: hash lookup with RequireCanonical must return null - blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "A is not canonical — hash lookup with RequireCanonical must return null"); - - // Without RequireCanonical both are still findable - blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.None).Should().NotBeNull("A findable without RequireCanonical"); - blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.None).Should().NotBeNull("B findable without RequireCanonical"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_by_number_without_RequireCanonical_uses_best_difficulty_fallback_when_no_canonical_marked() - { - // When HasBlockOnMainChain=false (no UpdateMainChain called), GetBlockHashOnMainOrBestDifficultyHash - // falls back to the best-TD loop using >=, so the last SuggestBlock call wins (TD=0 in PoS). - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - // Suggest two siblings but do NOT call UpdateMainChain for either - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockA); - - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockB); - - // Neither is canonical — RequireCanonical must return null - blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "no canonical block at height 1 — RequireCanonical must return null"); - - // Without RequireCanonical the best-difficulty fallback returns a block. - // With TD=0 and >= comparison, the last SuggestBlock (B) wins. - Block? fallback = blockTree.FindBlock(1, BlockTreeLookupOptions.None); - fallback.Should().NotBeNull("best-difficulty fallback must return a block when no canonical is set"); - fallback!.Hash.Should().Be(blockB.Hash!, - "last SuggestBlock (B) wins the TD=0 >= fallback tie-break"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_UpdateMainChain_with_wereProcessed_false_still_marks_canonical() + public void UpdateMainChain_WhenCalledWithWereProcessedFalse_MarksBlockCanonical() { // wereProcessed=false is used during sync to set canonical without updating Head. // The canonical marker (HasBlockOnMainChain / BlockInfos[0]) must be set regardless. @@ -2661,7 +2272,7 @@ public void FindBlock_UpdateMainChain_with_wereProcessed_false_still_marks_canon } [Test, MaxTime(Timeout.MaxTestTime)] - public void UpdateMainChain_sync_marks_descendant_canonical_then_reorg_to_sibling_decanonalizes_it() + public void UpdateMainChain_WhenBeaconSyncMarksThenReorgsToSibling_DecanonalizesDescendant() { // Regression test for the Gnosis canonical-mismatch bug. // @@ -2715,7 +2326,7 @@ public void UpdateMainChain_sync_marks_descendant_canonical_then_reorg_to_siblin } [Test, MaxTime(Timeout.MaxTestTime)] - public void UpdateMainChain_beacon_sync_multiple_descendants_then_reorg_to_sibling_clears_all() + public void UpdateMainChain_WhenBeaconSyncMarksMultipleDescendantsThenReorgs_ClearsAllStaleMarkers() { // Exact reproduction of the stale canonical markers bug from the Engine API test generator. // @@ -2789,7 +2400,7 @@ public void UpdateMainChain_beacon_sync_multiple_descendants_then_reorg_to_sibli } [Test, MaxTime(Timeout.MaxTestTime)] - public void UpdateMainChain_fcu_to_ancestor_with_stale_head_clears_all_beacon_synced_descendants() + public void UpdateMainChain_WhenFcuToAncestorWithStaleBeaconSyncedDescendants_ClearsAll() { // ePBS scenario: FCU can reorg to an ancestor (not just a sibling at the same height). // If head is stale because beacon sync marked descendants canonical without updating Head, @@ -2848,7 +2459,7 @@ public void UpdateMainChain_fcu_to_ancestor_with_stale_head_clears_all_beacon_sy } [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_clears_stale_marker_above_head_left_by_sync() + public void HealCanonicalChain_WhenStaleMarkerAboveHeadFromSync_ClearsMarker() { // Scenario: sync (wereProcessed=false) marks C canonical at H=2 without updating Head. // HealCanonicalChain(head=A) must scan upward and clear C's stale marker. @@ -2881,7 +2492,7 @@ public void HealCanonicalChain_clears_stale_marker_above_head_left_by_sync() } [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_fixes_wrong_canonical_block_at_height() + public void HealCanonicalChain_WhenWrongBlockIsMarkedCanonical_FixesMarker() { // Scenario: A and B are siblings at H=1. B was swapped to index 0 by accident // (e.g. a stale write), but the real canonical chain goes through A. @@ -2913,7 +2524,7 @@ public void HealCanonicalChain_fixes_wrong_canonical_block_at_height() } [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_does_not_alter_already_consistent_chain() + public void HealCanonicalChain_WhenChainIsAlreadyConsistent_MakesNoChanges() { BlockTree blockTree = BuildBlockTree(); @@ -2935,7 +2546,7 @@ public void HealCanonicalChain_does_not_alter_already_consistent_chain() } [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_does_nothing_for_unknown_start_hash() + public void HealCanonicalChain_WhenStartHashIsUnknown_DoesNothing() { BlockTree blockTree = BuildBlockTree(); @@ -2949,7 +2560,7 @@ public void HealCanonicalChain_does_nothing_for_unknown_start_hash() } [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_clears_multiple_stale_levels_above_head() + public void HealCanonicalChain_WhenMultipleStaleLevelsAboveHead_ClearsAll() { // Sync left THREE levels above head canonical — heal must clear all of them. BlockTree blockTree = BuildBlockTree(); @@ -2985,7 +2596,7 @@ public void HealCanonicalChain_clears_multiple_stale_levels_above_head() } [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_fixes_both_directions_in_one_pass() + public void HealCanonicalChain_WhenStaleMarkersAboveAndIncorrectMarkersBelow_FixesBothDirections() { // Combined scenario: wrong canonical at H=1 AND stale markers at H=2,3 above head. // This is the realistic production scenario: FCU moved head to B at H=1, @@ -3031,7 +2642,7 @@ public void HealCanonicalChain_fixes_both_directions_in_one_pass() } [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_respects_maxBlockDepth() + public void HealCanonicalChain_WhenDepthExceedsMaxBlockDepth_StopsAtLimit() { // With maxBlockDepth=1, the walk only checks the start block and one parent. // A broken marker two levels below must NOT be repaired. @@ -3065,145 +2676,7 @@ public void HealCanonicalChain_respects_maxBlockDepth() } [Test, MaxTime(Timeout.MaxTestTime)] - public void Canonical_chain_walk_every_ancestor_is_IsMainChain_true() - { - // Walk from canonical head back to genesis via ParentHash. - // Every block in the chain must satisfy IsMainChain=true. - // This catches the Gnosis-style bug where a child is canonical but its parent is not. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; - blockTree.SuggestBlock(b1); - blockTree.UpdateMainChain(new[] { b1 }, true); - - Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; - blockTree.SuggestBlock(b2); - blockTree.UpdateMainChain(new[] { b2 }, true); - - Block b3 = Build.A.Block.WithNumber(3).WithParent(b2).TestObject; - blockTree.SuggestBlock(b3); - blockTree.UpdateMainChain(new[] { b3 }, true); - - // Walk canonical chain from head down to genesis - BlockHeader? current = blockTree.FindHeader(3, BlockTreeLookupOptions.RequireCanonical); - current.Should().NotBeNull("canonical head must exist at height 3"); - - while (current!.Number > 0) - { - blockTree.IsMainChain(current).Should().BeTrue( - $"block at height {current.Number} (hash {current.Hash}) must be IsMainChain=true"); - current = blockTree.FindHeader(current.ParentHash!, BlockTreeLookupOptions.TotalDifficultyNotNeeded); - current.Should().NotBeNull($"parent must exist when walking canonical chain"); - } - - blockTree.IsMainChain(current!).Should().BeTrue("genesis must be IsMainChain=true"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_long_branch_reorg_all_new_heights_canonical_old_height_unmarked() - { - // Head at A(1). Reorg to B(1)→C(2)→D(3). - // After reorg: B, C, D canonical; A unmarked; heights 2 and 3 newly marked. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.UpdateMainChain(new[] { blockA }, true); - - // Competing branch: B sibling of A, then C and D on top of B - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockB); - - Block blockC = Build.A.Block.WithNumber(2).WithParent(blockB).TestObject; - blockTree.SuggestBlock(blockC); - - Block blockD = Build.A.Block.WithNumber(3).WithParent(blockC).TestObject; - blockTree.SuggestBlock(blockD); - - blockTree.UpdateMainChain(new[] { blockB, blockC, blockD }, true); - - blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockB.Hash!, "B canonical at height 1"); - blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockC.Hash!, "C canonical at height 2"); - blockTree.FindBlock(3, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(blockD.Hash!, "D canonical at height 3"); - blockTree.IsMainChain(blockA.Header).Should().BeFalse("A must be unmarked after reorg to longer branch"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_reorg_to_shorter_chain_higher_levels_unmarked() - { - // Head at height 3 (A1→A2→A3). Reorg to B1 (height 1 only). - // Heights 2 and 3 must be unmarked; B1 must be canonical at height 1. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block a1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(a1); - Block a2 = Build.A.Block.WithNumber(2).WithParent(a1).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(a2); - Block a3 = Build.A.Block.WithNumber(3).WithParent(a2).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(a3); - blockTree.UpdateMainChain(new[] { a1, a2, a3 }, true); - - // Reorg to a competing block at height 1 only - Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(b1); - blockTree.UpdateMainChain(new[] { b1 }, true); - - blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical)!.Hash.Should().Be(b1.Hash!, "B1 canonical at height 1"); - blockTree.FindBlock(2, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("height 2 must be unmarked after reorg to shorter chain"); - blockTree.FindBlock(3, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("height 3 must be unmarked after reorg to shorter chain"); - blockTree.IsMainChain(a1.Header).Should().BeFalse("A1 no longer canonical"); - blockTree.IsMainChain(a2.Header).Should().BeFalse("A2 no longer canonical"); - blockTree.IsMainChain(a3.Header).Should().BeFalse("A3 no longer canonical"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindHeader_by_number_with_RequireCanonical_mirrors_FindBlock_behavior() - { - // FindHeader(long, RequireCanonical) uses GetBlockHashOnMainOrBestDifficultyHash then - // checks level.MainChainBlock — same logic as FindBlock but returning only the header. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - blockTree.SuggestBlock(blockA); - blockTree.UpdateMainChain(new[] { blockA }, true); - - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - blockTree.SuggestBlock(blockB); - blockTree.UpdateMainChain(new[] { blockB }, true); - - // B canonical: FindHeader with RequireCanonical must return B's header - BlockHeader? headerCanonical = blockTree.FindHeader(1, BlockTreeLookupOptions.RequireCanonical); - headerCanonical.Should().NotBeNull("canonical header must be found at height 1"); - headerCanonical!.Hash.Should().Be(blockB.Hash!, "FindHeader(RequireCanonical) must return B after reorg"); - - // A non-canonical: FindHeader(hash, RequireCanonical) must return null - blockTree.FindHeader(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "FindHeader by A's hash with RequireCanonical must return null — A is not canonical"); - - // FindHeader and FindBlock must agree on canonical hash - Block? blockCanonical = blockTree.FindBlock(1, BlockTreeLookupOptions.RequireCanonical); - blockCanonical!.Hash.Should().Be(headerCanonical.Hash, - "FindHeader and FindBlock must return the same canonical block at height 1"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void FindBlock_by_number_after_reorg_returns_null_not_orphaned_block_in_pos() + public void FindBlock_WhenBlockOrphanedAfterReorgInPoS_ReturnsNull() { // In PoS all blocks share the same cumulative TotalDifficulty (difficulty=0 per block). // The PoW-era "best difficulty" fallback in GetBlockHashOnMainOrBestDifficultyHash @@ -3251,7 +2724,7 @@ public void FindBlock_by_number_after_reorg_returns_null_not_orphaned_block_in_p } [Test, MaxTime(Timeout.MaxTestTime)] - public void UpdateMainChain_beacon_sync_gap_in_stale_markers_leaves_orphan_after_reorg() + public void UpdateMainChain_WhenGapInBeaconSyncMarkersAndReorging_ClearsStaleMarkersAcrossGap() { // Reproduces the race-condition gap scenario on top of the beacon sync path: // diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index 20d087bb8a34..c22ae21d6070 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -767,13 +767,12 @@ as it does not require the step of resolving number -> hash */ return level.BlockInfos[0].BlockHash; } - // Post-merge: block difficulty is 0, so TotalDifficulty never increases and all - // blocks at the same height share the same cumulative TD. The best-difficulty - // fallback below cannot distinguish canonical from orphaned — it would return - // a stale block after a reorg. Return null: there is no canonical block here. - bool isPostMerge = SpecProvider.TerminalTotalDifficulty is not null - && (Head?.TotalDifficulty ?? UInt256.Zero) >= SpecProvider.TerminalTotalDifficulty; - if (isPostMerge) return null; + // Post-merge: TD never increases, so the best-TD fallback cannot distinguish canonical from orphaned. + if (SpecProvider.TerminalTotalDifficulty is not null + && (Head?.TotalDifficulty ?? UInt256.Zero) >= SpecProvider.TerminalTotalDifficulty) + { + return null; + } // Pre-merge: the block with the highest total difficulty is canonical by PoW rule. UInt256 bestDifficultySoFar = UInt256.Zero; @@ -1008,8 +1007,6 @@ public void UpdateMainChain(IReadOnlyList blocks, bool wereProcessed, boo long lastNumber = ascendingOrder ? blocks[^1].Number : blocks[0].Number; long previousHeadNumber = Head?.Number ?? 0L; using BatchWrite batch = _chainLevelInfoRepository.StartBatch(); - // Downward unmark: clear levels from previousHead down to lastNumber+1. - // Handles the normal reorg case where the new chain is shorter than the old head. if (previousHeadNumber > lastNumber) { for (long i = 0; i < previousHeadNumber - lastNumber; i++) @@ -1025,22 +1022,9 @@ public void UpdateMainChain(IReadOnlyList blocks, bool wereProcessed, boo } } - // Upward scan: clear any stale canonical markers above max(previousHead, lastNumber). - // Beacon sync (wereProcessed=false) marks blocks canonical without updating Head, - // leaving stale markers the downward loop cannot reach. This covers two cases: - // - FCU at same/higher height (previousHeadNumber <= lastNumber): scans from lastNumber+1. - // - ePBS FCU to ancestor with stale head: scans from previousHeadNumber+1, clearing - // beacon-synced markers above the stale head that the downward loop misses. - for (long levelNumber = Math.Max(previousHeadNumber, lastNumber) + 1; ; levelNumber++) - { - ChainLevelInfo? level = LoadLevel(levelNumber); - if (level is null) break; - if (level.HasBlockOnMainChain) - { - level.HasBlockOnMainChain = false; - _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); - } - } + // Clear stale canonical markers above the new head left by beacon sync. + // Covers both same-height FCU (previousHeadNumber <= lastNumber) and ePBS FCU to ancestor. + ClearStaleMarkersAbove(Math.Max(previousHeadNumber, lastNumber), batch); for (int i = 0; i < blocks.Count; i++) { @@ -1068,22 +1052,18 @@ public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) BlockHeader? start = FindHeader(startHash, BlockTreeLookupOptions.None); if (start is null) return; - // BatchWrite is a write-buffered, deferred batch: PersistLevel only enqueues writes - // and nothing is flushed to the underlying DB until Dispose(). Both phases therefore - // commit atomically — a crash before Dispose leaves the DB unchanged. using BatchWrite batch = _chainLevelInfoRepository.StartBatch(); long repairedAbove = ClearStaleMarkersAbove(start.Number, batch); long repairedBelow = RepairMarkersBelow(start, maxBlockDepth, batch); - long repaired = repairedAbove + repairedBelow; - if (Logger.IsInfo) Logger.Info($"Canonical chain heal complete: {repaired} level(s) repaired ({repairedAbove} stale above head cleared, {repairedBelow} incorrect markers fixed)."); + if (Logger.IsInfo) Logger.Info($"Canonical chain heal complete: {repairedAbove + repairedBelow} level(s) repaired ({repairedAbove} stale above head cleared, {repairedBelow} incorrect markers fixed)."); } - private long ClearStaleMarkersAbove(long headNumber, BatchWrite batch) + private long ClearStaleMarkersAbove(long fromExclusive, BatchWrite batch) { long cleared = 0L; - for (long levelNumber = headNumber + 1; ; levelNumber++) + for (long levelNumber = fromExclusive + 1; ; levelNumber++) { ChainLevelInfo? level = LoadLevel(levelNumber); if (level is null) break; @@ -1115,15 +1095,14 @@ private long RepairMarkersBelow(BlockHeader start, long maxBlockDepth, BatchWrit break; } - bool needsPersist = index.Value != 0 || !level.HasBlockOnMainChain; + bool needsRepair = index.Value != 0 || !level.HasBlockOnMainChain; if (index.Value != 0) level.SwapToMain(index.Value); - if (!level.HasBlockOnMainChain) - level.HasBlockOnMainChain = true; + level.HasBlockOnMainChain = true; - if (needsPersist) + if (needsRepair) { _chainLevelInfoRepository.PersistLevel(current.Number, level, batch); repairedCount++; diff --git a/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs b/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs index c4170dcb0a0a..151078286a19 100644 --- a/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs +++ b/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs @@ -30,27 +30,25 @@ ILogManager logManager public Task Execute(CancellationToken cancellationToken) { - if (initConfig.HealCanonicalChainOnStartup) - { - Hash256? startHash = blockTree.Head?.Hash; - if (startHash is not null) - { - if (_logger.IsInfo) _logger.Info($"Healing canonical chain from head {startHash} (depth {initConfig.HealCanonicalChainDepth})..."); - blockTree.HealCanonicalChain(startHash, initConfig.HealCanonicalChainDepth); - } - else - { - if (_logger.IsWarn) _logger.Warn("HealCanonicalChainOnStartup requested but no head block found — skipping."); - } - } + HealCanonicalChainIfEnabled(); + return initConfig.ProcessingEnabled + ? RunBlockTreeInitTasks(cancellationToken) + : Task.CompletedTask; + } + + private void HealCanonicalChainIfEnabled() + { + if (!initConfig.HealCanonicalChain) return; - if (initConfig.ProcessingEnabled) + Hash256? startHash = blockTree.Head?.Hash; + if (startHash is not null) { - return RunBlockTreeInitTasks(cancellationToken); + if (_logger.IsInfo) _logger.Info($"Healing canonical chain from head {startHash} (depth {initConfig.HealCanonicalChainDepth})..."); + blockTree.HealCanonicalChain(startHash, initConfig.HealCanonicalChainDepth); } else { - return Task.CompletedTask; + if (_logger.IsWarn) _logger.Warn("HealCanonicalChain requested but no head block found — skipping."); } } From de523da8f9881d83dac2d61ade61aacd35782ebc Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Wed, 25 Mar 2026 19:15:16 +0200 Subject: [PATCH 16/25] remove misleading comment --- src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index e523d256d009..0be6bd9461cc 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2737,7 +2737,6 @@ public void UpdateMainChain_WhenGapInBeaconSyncMarkersAndReorging_ClearsStaleMar // Phase 2 upward scan: clears d1(H=2), hits gap at d2(H=3), breaks. // d3(H=4) remains stale canonical — BUG. // - // This test FAILS on canonical-fix (break-on-first-gap) and PASSES on bounded-scan. BlockTree blockTree = BuildBlockTree(); Block genesis = Build.A.Block.WithNumber(0).TestObject; From 06a15672ee2c79757774f4ad332ce3e1e933c1e2 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Wed, 25 Mar 2026 19:23:13 +0200 Subject: [PATCH 17/25] minor copilot comments --- .../BlockTreeTests.cs | 25 ++++++------------- .../Nethermind.Blockchain/BlockTree.cs | 2 +- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index 0be6bd9461cc..c1166d9c47ac 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2272,24 +2272,13 @@ public void UpdateMainChain_WhenCalledWithWereProcessedFalse_MarksBlockCanonical } [Test, MaxTime(Timeout.MaxTestTime)] - public void UpdateMainChain_WhenBeaconSyncMarksThenReorgsToSibling_DecanonalizesDescendant() + public void UpdateMainChain_WhenBeaconSyncMarksThenReorgsToSibling_DecanonicalizesDescendant() { // Regression test for the Gnosis canonical-mismatch bug. // - // BlockDownloader calls UpdateMainChain(block, wereProcessed: false) to mark synced - // blocks canonical without updating Head. When a reorg then arrives at the same height - // as the stale Head (previousHeadNumber == lastNumber), the old unmark loop - // (previousHeadNumber > lastNumber) is skipped, leaving the orphaned descendant - // wrongly canonical so eth_getBlockByNumber returns the wrong block. - // - // The fix adds an else-branch that scans upward from lastNumber+1 to clear any - // stale canonical markers left by the sync path. - // - // Scenario: - // UpdateMainChain([A], wereProcessed: true) — FCU(A): head = A at H=1. - // UpdateMainChain([C], wereProcessed: false) — sync: C canonical at H=2, head stays at A. - // UpdateMainChain([B], wereProcessed: true) — FCU(B, H=1, sibling of A): - // previousHeadNumber(1) == lastNumber(1) → else-branch fires → C at H=2 decanonalized. + // Beacon sync marks C canonical at H=2 (wereProcessed=false, Head stays at H=1). + // FCU reorgs to sibling B at H=1: previousHeadNumber == lastNumber, so the downward + // unmark loop is skipped. The unconditional upward scan clears C at H=2. BlockTree blockTree = BuildBlockTree(); Block genesis = Build.A.Block.WithNumber(0).TestObject; @@ -2315,7 +2304,7 @@ public void UpdateMainChain_WhenBeaconSyncMarksThenReorgsToSibling_Decanonalizes .Should().NotBeNull("C must be canonical at H=2 after sync marks it"); // FCU(B): reorg to sibling at the same height as the stale Head. - // previousHeadNumber(1) == lastNumber(1) → old loop skipped → else-branch must clear C. + // previousHeadNumber(1) == lastNumber(1) → downward loop skipped → upward scan clears C. blockTree.UpdateMainChain(new[] { blockB }, wereProcessed: true, forceUpdateHeadBlock: true); blockTree.Head!.Hash.Should().Be(blockB.Hash!, "head must be B after reorg"); @@ -2644,8 +2633,8 @@ public void HealCanonicalChain_WhenStaleMarkersAboveAndIncorrectMarkersBelow_Fix [Test, MaxTime(Timeout.MaxTestTime)] public void HealCanonicalChain_WhenDepthExceedsMaxBlockDepth_StopsAtLimit() { - // With maxBlockDepth=1, the walk only checks the start block and one parent. - // A broken marker two levels below must NOT be repaired. + // maxBlockDepth=0 repairs only the start block; maxBlockDepth=N repairs start + N parents. + // A broken marker one level below start must NOT be repaired when maxBlockDepth=0. BlockTree blockTree = BuildBlockTree(); Block genesis = Build.A.Block.WithNumber(0).TestObject; diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index c22ae21d6070..0401f912bce1 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -1083,7 +1083,7 @@ private long RepairMarkersBelow(BlockHeader start, long maxBlockDepth, BatchWrit long blocksWalked = 0L; BlockHeader? current = start; - while (current is not null && blocksWalked < maxBlockDepth) + while (current is not null && blocksWalked <= maxBlockDepth) { ChainLevelInfo? level = LoadLevel(current.Number); if (level is not null) From 07dfcc0559706feb9e868b53191ce648ffdbda86 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Fri, 27 Mar 2026 02:07:41 +0200 Subject: [PATCH 18/25] review comments --- src/Nethermind/Nethermind.Api/IInitConfig.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Nethermind/Nethermind.Api/IInitConfig.cs b/src/Nethermind/Nethermind.Api/IInitConfig.cs index 849e0cb0d270..22956857fc85 100644 --- a/src/Nethermind/Nethermind.Api/IInitConfig.cs +++ b/src/Nethermind/Nethermind.Api/IInitConfig.cs @@ -99,17 +99,13 @@ public interface IInitConfig : IConfig [ConfigItem(Description = "[TECHNICAL] True when in runner test. Disable some wait.", DefaultValue = "false", HiddenFromDocs = true)] bool InRunnerTest { get; set; } - [ConfigItem( - Description = "On startup, walk backward from the current head for up to `HealCanonicalChainDepth` " + - "blocks via parentHash and repair any incorrect `HasBlockOnMainChain` markers left by " + - "the beacon-sync path. Also clears stale canonical markers above the head. " + - "Use this flag once after observing a canonical-mismatch (wrong `eth_getBlockByNumber` results).", - DefaultValue = "false")] + [ConfigItem(Description = "Whether to repair canonical-chain markers on startup after a canonical mismatch.", DefaultValue = "false", HiddenFromDocs = true)] bool HealCanonicalChain { get; set; } [ConfigItem( - Description = "Number of blocks to walk back from the head when `HealCanonicalChain` is enabled.", - DefaultValue = "8192")] + Description = $"The number of blocks to walk back from the head when the `{nameof(HealCanonicalChain)}` is set to `true`.", + DefaultValue = "8192", + HiddenFromDocs = true)] long HealCanonicalChainDepth { get; set; } } From e2a932293550b06d5c2d97e38e20e64a2036a26d Mon Sep 17 00:00:00 2001 From: Lukasz Rozmej Date: Fri, 27 Mar 2026 13:44:44 +0100 Subject: [PATCH 19/25] Canonical fix refactor (#10972) * small refactors in BlockTree * de-duplicate tests --- .../BlockTreeTests.cs | 406 +++++------------- .../Nethermind.Blockchain/BlockTree.cs | 20 +- 2 files changed, 121 insertions(+), 305 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index c1166d9c47ac..793925ff26aa 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -66,6 +66,27 @@ private static void AddToMain(BlockTree blockTree, Block block0) blockTree.UpdateMainChain(new[] { block0 }, true); } + private (BlockTree blockTree, Block genesis) BuildBlockTreeWithGenesis(bool forceUpdateHead = false) + { + BlockTree blockTree = BuildBlockTree(); + Block genesis = Build.A.Block.WithNumber(0).TestObject; + blockTree.SuggestBlock(genesis); + blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true, forceUpdateHeadBlock: forceUpdateHead); + return (blockTree, genesis); + } + + private static Block[] BuildAndSuggestChain(BlockTree blockTree, Block parent, int count) + { + Block[] chain = new Block[count]; + for (int i = 0; i < count; i++) + { + chain[i] = Build.A.Block.WithNumber(parent.Number + 1).WithParent(parent).TestObject; + blockTree.SuggestBlock(chain[i]); + parent = chain[i]; + } + return chain; + } + [Test, MaxTime(Timeout.MaxTestTime)] public void Add_genesis_shall_notify() { @@ -2207,11 +2228,7 @@ public void FindBlock_WhenThirdOfThreeSiblingsIsCanonical_ReturnsThatSibling() { // Exercises SwapToMain with index > 1 (three blocks at the same height, // the third one — index=2 — is made canonical last). - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, true); + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(); Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; blockTree.SuggestBlock(blockA); @@ -2247,11 +2264,7 @@ public void UpdateMainChain_WhenCalledWithWereProcessedFalse_MarksBlockCanonical { // wereProcessed=false is used during sync to set canonical without updating Head. // The canonical marker (HasBlockOnMainChain / BlockInfos[0]) must be set regardless. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(); Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; blockTree.SuggestBlock(blockA); @@ -2271,121 +2284,69 @@ public void UpdateMainChain_WhenCalledWithWereProcessedFalse_MarksBlockCanonical blockTree.IsMainChain(blockA.Header).Should().BeFalse("A is no longer canonical"); } - [Test, MaxTime(Timeout.MaxTestTime)] - public void UpdateMainChain_WhenBeaconSyncMarksThenReorgsToSibling_DecanonicalizesDescendant() - { - // Regression test for the Gnosis canonical-mismatch bug. - // - // Beacon sync marks C canonical at H=2 (wereProcessed=false, Head stays at H=1). - // FCU reorgs to sibling B at H=1: previousHeadNumber == lastNumber, so the downward - // unmark loop is skipped. The unconditional upward scan clears C at H=2. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); - - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; - Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; - Block blockC = Build.A.Block.WithNumber(2).WithParent(blockA).TestObject; - - blockTree.SuggestBlock(blockA); - blockTree.SuggestBlock(blockB); - blockTree.SuggestBlock(blockC); - - // FCU(A): A becomes head at H=1. - blockTree.UpdateMainChain(new[] { blockA }, wereProcessed: true, forceUpdateHeadBlock: true); - blockTree.Head!.Hash.Should().Be(blockA.Hash!, "head must be A"); - - // Sync marks C canonical at H=2 without updating Head (BlockDownloader path). - blockTree.UpdateMainChain(new[] { blockC }, wereProcessed: false); - blockTree.Head!.Hash.Should().Be(blockA.Hash!, "head must stay at A — wereProcessed=false"); - blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical) - .Should().NotBeNull("C must be canonical at H=2 after sync marks it"); - - // FCU(B): reorg to sibling at the same height as the stale Head. - // previousHeadNumber(1) == lastNumber(1) → downward loop skipped → upward scan clears C. - blockTree.UpdateMainChain(new[] { blockB }, wereProcessed: true, forceUpdateHeadBlock: true); - - blockTree.Head!.Hash.Should().Be(blockB.Hash!, "head must be B after reorg"); - blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull( - "C must not be canonical — its parent A was replaced by B"); - blockTree.FindBlock(blockB.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull( - "B must be canonical"); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void UpdateMainChain_WhenBeaconSyncMarksMultipleDescendantsThenReorgs_ClearsAllStaleMarkers() + [TestCase(1, false, TestName = "SingleDescendant")] + [TestCase(3, false, TestName = "MultipleDescendants")] + [TestCase(3, true, TestName = "MultipleDescendantsWithGap")] + [MaxTime(Timeout.MaxTestTime)] + public void UpdateMainChain_WhenBeaconSyncMarksThenReorgsToSibling_ClearsStaleMarkers(int descendantCount, bool simulateGap) { - // Exact reproduction of the stale canonical markers bug from the Engine API test generator. - // - // Beacon sync marks H+1, H+2, H+3 canonical without updating Head (wereProcessed: false). - // FCU reorgs to a sibling of Head at the SAME height H. - // previousHeadNumber == lastNumber → downward unmark skipped entirely. - // Upward scan must clear all three orphaned levels. - // - // This differs from the single-descendant test above: with multiple levels, the bounded - // scan (not break-on-first-gap) is critical — a concurrent MoveToMain could create a gap. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true, forceUpdateHeadBlock: true); - - // Chain: genesis → headBlock(H=1) → d1(H=2) → d2(H=3) → d3(H=4) - Block headBlock = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0xAA }).TestObject; - Block d1 = Build.A.Block.WithNumber(2).WithParent(headBlock).TestObject; - Block d2 = Build.A.Block.WithNumber(3).WithParent(d1).TestObject; - Block d3 = Build.A.Block.WithNumber(4).WithParent(d2).TestObject; + // Beacon sync marks N descendants canonical (wereProcessed=false, Head stays stale at H=1). + // FCU reorgs to sibling at the same height. All stale markers must be cleared. + // When simulateGap=true, a concurrent MoveToMain clears one intermediate marker, + // creating a gap that the bounded scan must handle. + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(forceUpdateHead: true); + Block headBlock = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData([0xAA]).TestObject; blockTree.SuggestBlock(headBlock); - blockTree.SuggestBlock(d1); - blockTree.SuggestBlock(d2); - blockTree.SuggestBlock(d3); + + Block[] descendants = BuildAndSuggestChain(blockTree, headBlock, descendantCount); // FCU sets Head to headBlock at H=1 blockTree.UpdateMainChain(new[] { headBlock }, wereProcessed: true, forceUpdateHeadBlock: true); blockTree.Head!.Hash.Should().Be(headBlock.Hash!); - // Beacon sync: d1, d2, d3 marked canonical without advancing Head - blockTree.UpdateMainChain(new[] { d1 }, wereProcessed: false); - blockTree.UpdateMainChain(new[] { d2 }, wereProcessed: false); - blockTree.UpdateMainChain(new[] { d3 }, wereProcessed: false); + // Beacon sync: mark descendants canonical without advancing Head + foreach (Block d in descendants) + { + blockTree.UpdateMainChain(new[] { d }, wereProcessed: false); + } - // Step 2: verify stale Head blockTree.Head!.Number.Should().Be(1, "Head must stay at H=1 — wereProcessed=false"); - blockTree.IsMainChain(d1.Header).Should().BeTrue("precondition: d1 canonical via beacon sync"); - blockTree.IsMainChain(d2.Header).Should().BeTrue("precondition: d2 canonical via beacon sync"); - blockTree.IsMainChain(d3.Header).Should().BeTrue("precondition: d3 canonical via beacon sync"); + foreach (Block d in descendants) + { + blockTree.IsMainChain(d.Header).Should().BeTrue($"precondition: block at H={d.Number} canonical via beacon sync"); + } - // Step 3: FCU reorg to sibling at same height H=1 + if (simulateGap && descendantCount >= 3) + { + // Simulate race: concurrent MoveToMain clears middle marker, creating a gap + ChainLevelInfo? gapLevel = blockTree.FindLevel(descendants[1].Number); + gapLevel!.HasBlockOnMainChain = false; + blockTree.IsMainChain(descendants[1].Header).Should().BeFalse("precondition: gap exists"); + } + + // FCU reorg to sibling at H=1 Block sibling = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0xBB }).TestObject; blockTree.SuggestBlock(sibling); blockTree.UpdateMainChain(new[] { sibling }, wereProcessed: true, forceUpdateHeadBlock: true); - // Step 4: verify reorg correctness - blockTree.Head!.Number.Should().Be(1); blockTree.Head!.Hash.Should().Be(sibling.Hash!); blockTree.IsMainChain(sibling.Header).Should().BeTrue("sibling must be canonical"); - blockTree.IsMainChain(d1.Header).Should().BeFalse("d1 must be de-canonicalized after reorg"); - blockTree.IsMainChain(d2.Header).Should().BeFalse("d2 must be de-canonicalized after reorg"); - blockTree.IsMainChain(d3.Header).Should().BeFalse("d3 must be de-canonicalized after reorg"); + foreach (Block d in descendants) + { + blockTree.IsMainChain(d.Header).Should().BeFalse($"block at H={d.Number} must be de-canonicalized after reorg"); + } - // Step 5: verify user-visible impact — FindCanonicalBlockInfo must return null - blockTree.FindCanonicalBlockInfo(2).Should().BeNull("H+1 must return null — orphaned after reorg"); - blockTree.FindCanonicalBlockInfo(3).Should().BeNull("H+2 must return null — orphaned after reorg"); - blockTree.FindCanonicalBlockInfo(4).Should().BeNull("H+3 must return null — orphaned after reorg"); + // FindCanonicalBlockInfo must return null for all orphaned heights + for (int h = 2; h <= descendantCount + 1; h++) + { + blockTree.FindCanonicalBlockInfo(h).Should().BeNull($"H={h} must return null — orphaned after reorg"); + } - // Step 6: verify block hashes — canonical lookup returns correct hash at H=1 + // Canonical lookup at H=1 must return sibling BlockInfo? infoAt1 = blockTree.FindCanonicalBlockInfo(1); infoAt1.Should().NotBeNull(); - infoAt1!.BlockHash.Should().Be(sibling.Hash!, "H=1 must return sibling's hash, not old headBlock"); - infoAt1.BlockHash.Should().NotBe(headBlock.Hash!, "H=1 must NOT return old headBlock's hash"); - - // Orphaned heights must not return old block hashes via canonical lookup - blockTree.FindCanonicalBlockInfo(2)?.BlockHash.Should().NotBe(d1.Hash!, "H+1 canonical hash must not be d1"); - blockTree.FindCanonicalBlockInfo(3)?.BlockHash.Should().NotBe(d2.Hash!, "H+2 canonical hash must not be d2"); - blockTree.FindCanonicalBlockInfo(4)?.BlockHash.Should().NotBe(d3.Hash!, "H+3 canonical hash must not be d3"); + infoAt1!.BlockHash.Should().Be(sibling.Hash!, "H=1 must return sibling's hash"); } [Test, MaxTime(Timeout.MaxTestTime)] @@ -2406,78 +2367,65 @@ public void UpdateMainChain_WhenFcuToAncestorWithStaleBeaconSyncedDescendants_Cl // UpdateMainChain([genesis], wereProcessed: true) — ePBS FCU to ancestor at H=0: // previousHeadNumber(1) > lastNumber(0) → IF branch clears H=1 only. // b2, b3, b4 are NOT cleared — they are above the stale head and invisible to the IF branch. - BlockTree blockTree = BuildBlockTree(); + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(); - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); - - Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; - Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; - Block b3 = Build.A.Block.WithNumber(3).WithParent(b2).TestObject; - Block b4 = Build.A.Block.WithNumber(4).WithParent(b3).TestObject; - - blockTree.SuggestBlock(b1); - blockTree.SuggestBlock(b2); - blockTree.SuggestBlock(b3); - blockTree.SuggestBlock(b4); + Block[] chain = BuildAndSuggestChain(blockTree, genesis, 4); // FCU(b1): head = b1 at H=1. - blockTree.UpdateMainChain(new[] { b1 }, wereProcessed: true, forceUpdateHeadBlock: true); + blockTree.UpdateMainChain(new[] { chain[0] }, wereProcessed: true, forceUpdateHeadBlock: true); // Beacon sync: b2, b3, b4 marked canonical without updating Head. - blockTree.UpdateMainChain(new[] { b2 }, wereProcessed: false); - blockTree.UpdateMainChain(new[] { b3 }, wereProcessed: false); - blockTree.UpdateMainChain(new[] { b4 }, wereProcessed: false); + for (int i = 1; i < chain.Length; i++) + { + blockTree.UpdateMainChain(new[] { chain[i] }, wereProcessed: false); + } // Preconditions: head stale at b1, b2-b4 canonical via beacon sync. - blockTree.Head!.Hash.Should().Be(b1.Hash!, "precondition: head stale at b1"); - blockTree.FindBlock(b2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("precondition: b2 beacon-synced canonical"); - blockTree.FindBlock(b4.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("precondition: b4 beacon-synced canonical"); + blockTree.Head!.Hash.Should().Be(chain[0].Hash!, "precondition: head stale at b1"); + blockTree.FindBlock(chain[1].Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("precondition: b2 beacon-synced canonical"); + blockTree.FindBlock(chain[3].Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("precondition: b4 beacon-synced canonical"); // ePBS FCU to ancestor: reorg back to genesis at H=0. - // The IF branch (previousHeadNumber=1 > lastNumber=0) clears b1 at H=1. - // b2, b3, b4 must also be cleared — they are above the stale head. blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true, forceUpdateHeadBlock: true); blockTree.FindBlock(genesis.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("genesis must be canonical"); - blockTree.FindBlock(b1.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("b1 must be de-canonicalized"); - blockTree.FindBlock(b2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("b2 must be de-canonicalized — beacon sync stale marker above stale head"); - blockTree.FindBlock(b3.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("b3 must be de-canonicalized — beacon sync stale marker above stale head"); - blockTree.FindBlock(b4.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("b4 must be de-canonicalized — beacon sync stale marker above stale head"); + foreach (Block b in chain) + { + blockTree.FindBlock(b.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull($"b{b.Number} must be de-canonicalized"); + } } - [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_WhenStaleMarkerAboveHeadFromSync_ClearsMarker() + [TestCase(1, TestName = "SingleStaleLevel")] + [TestCase(3, TestName = "MultipleStaleLevel")] + [MaxTime(Timeout.MaxTestTime)] + public void HealCanonicalChain_WhenStaleLevelsAboveHead_ClearsAll(int staleLevelCount) { - // Scenario: sync (wereProcessed=false) marks C canonical at H=2 without updating Head. - // HealCanonicalChain(head=A) must scan upward and clear C's stale marker. - BlockTree blockTree = BuildBlockTree(); + // Sync (wereProcessed=false) marks N levels above head canonical without updating Head. + // HealCanonicalChain must scan upward and clear all stale markers. + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(); - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + Block head = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; + blockTree.SuggestBlock(head); - Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; - Block blockC = Build.A.Block.WithNumber(2).WithParent(blockA).TestObject; + Block[] descendants = BuildAndSuggestChain(blockTree, head, staleLevelCount); - blockTree.SuggestBlock(blockA); - blockTree.SuggestBlock(blockC); + // FCU: head at H=1 + blockTree.UpdateMainChain(new[] { head }, wereProcessed: true, forceUpdateHeadBlock: true); - // FCU: head = A at H=1 - blockTree.UpdateMainChain(new[] { blockA }, wereProcessed: true, forceUpdateHeadBlock: true); + // Sync marks descendants canonical without updating Head + foreach (Block d in descendants) + { + blockTree.UpdateMainChain(new[] { d }, wereProcessed: false); + } - // Sync marks C canonical at H=2 without updating Head - blockTree.UpdateMainChain(new[] { blockC }, wereProcessed: false); - blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical) - .Should().NotBeNull("precondition: C is canonical before heal"); + blockTree.HealCanonicalChain(head.Hash!, maxBlockDepth: 10); - blockTree.HealCanonicalChain(blockA.Hash!, maxBlockDepth: 10); + foreach (Block d in descendants) + { + blockTree.FindBlock(d.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull($"H={d.Number} stale marker must be cleared"); + } - blockTree.FindBlock(blockC.Hash!, BlockTreeLookupOptions.RequireCanonical) - .Should().BeNull("C must be decanonalized after heal"); - blockTree.FindBlock(blockA.Hash!, BlockTreeLookupOptions.RequireCanonical) - .Should().NotBeNull("A must remain canonical"); + blockTree.FindBlock(head.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("head must remain canonical"); } [Test, MaxTime(Timeout.MaxTestTime)] @@ -2486,11 +2434,7 @@ public void HealCanonicalChain_WhenWrongBlockIsMarkedCanonical_FixesMarker() // Scenario: A and B are siblings at H=1. B was swapped to index 0 by accident // (e.g. a stale write), but the real canonical chain goes through A. // HealCanonicalChain walking from A must swap A back to index 0. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(); Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; @@ -2515,73 +2459,24 @@ public void HealCanonicalChain_WhenWrongBlockIsMarkedCanonical_FixesMarker() [Test, MaxTime(Timeout.MaxTestTime)] public void HealCanonicalChain_WhenChainIsAlreadyConsistent_MakesNoChanges() { - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); - - Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; - Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(); - blockTree.SuggestBlock(b1); - blockTree.SuggestBlock(b2); - blockTree.UpdateMainChain(new[] { b1, b2 }, wereProcessed: true, forceUpdateHeadBlock: true); + Block[] chain = BuildAndSuggestChain(blockTree, genesis, 2); + blockTree.UpdateMainChain(chain, wereProcessed: true, forceUpdateHeadBlock: true); - blockTree.HealCanonicalChain(b2.Hash!, maxBlockDepth: 10); + blockTree.HealCanonicalChain(chain[1].Hash!, maxBlockDepth: 10); - blockTree.FindBlock(b2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("b2 must remain canonical"); - blockTree.FindBlock(b1.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("b1 must remain canonical"); + blockTree.FindBlock(chain[1].Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("b2 must remain canonical"); + blockTree.FindBlock(chain[0].Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("b1 must remain canonical"); } [Test, MaxTime(Timeout.MaxTestTime)] public void HealCanonicalChain_WhenStartHashIsUnknown_DoesNothing() { - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + (BlockTree blockTree, _) = BuildBlockTreeWithGenesis(); // Should not throw — unknown hash is treated as a no-op. - blockTree.Invoking(bt => bt.HealCanonicalChain(TestItem.KeccakA, maxBlockDepth: 10)) - .Should().NotThrow(); - } - - [Test, MaxTime(Timeout.MaxTestTime)] - public void HealCanonicalChain_WhenMultipleStaleLevelsAboveHead_ClearsAll() - { - // Sync left THREE levels above head canonical — heal must clear all of them. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); - - Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).TestObject; - Block b2 = Build.A.Block.WithNumber(2).WithParent(b1).TestObject; - Block b3 = Build.A.Block.WithNumber(3).WithParent(b2).TestObject; - Block b4 = Build.A.Block.WithNumber(4).WithParent(b3).TestObject; - - blockTree.SuggestBlock(b1); - blockTree.SuggestBlock(b2); - blockTree.SuggestBlock(b3); - blockTree.SuggestBlock(b4); - - // FCU: head = b1 at H=1 - blockTree.UpdateMainChain(new[] { b1 }, wereProcessed: true, forceUpdateHeadBlock: true); - - // Sync marks b2, b3, b4 canonical without updating Head - blockTree.UpdateMainChain(new[] { b2 }, wereProcessed: false); - blockTree.UpdateMainChain(new[] { b3 }, wereProcessed: false); - blockTree.UpdateMainChain(new[] { b4 }, wereProcessed: false); - - blockTree.HealCanonicalChain(b1.Hash!, maxBlockDepth: 10); - - blockTree.FindBlock(b2.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("H=2 stale marker must be cleared"); - blockTree.FindBlock(b3.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("H=3 stale marker must be cleared"); - blockTree.FindBlock(b4.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().BeNull("H=4 stale marker must be cleared"); - blockTree.FindBlock(b1.Hash!, BlockTreeLookupOptions.RequireCanonical).Should().NotBeNull("b1 must remain canonical"); + blockTree.Invoking(bt => bt.HealCanonicalChain(TestItem.KeccakA, maxBlockDepth: 10)).Should().NotThrow(); } [Test, MaxTime(Timeout.MaxTestTime)] @@ -2594,11 +2489,7 @@ public void HealCanonicalChain_WhenStaleMarkersAboveAndIncorrectMarkersBelow_Fix // // genesis → A(H=1) → C(H=2) ← sync left C canonical, A at H=1 // → B(H=1) ← heal starts from B, the correct head - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(); Block blockA = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; Block blockB = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; @@ -2635,11 +2526,7 @@ public void HealCanonicalChain_WhenDepthExceedsMaxBlockDepth_StopsAtLimit() { // maxBlockDepth=0 repairs only the start block; maxBlockDepth=N repairs start + N parents. // A broken marker one level below start must NOT be repaired when maxBlockDepth=0. - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true); + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(); Block b1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 1 }).TestObject; Block b1Alt = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 2 }).TestObject; @@ -2712,71 +2599,4 @@ public void FindBlock_WhenBlockOrphanedAfterReorgInPoS_ReturnsNull() "height 1 must return B1 after reorg"); } - [Test, MaxTime(Timeout.MaxTestTime)] - public void UpdateMainChain_WhenGapInBeaconSyncMarkersAndReorging_ClearsStaleMarkersAcrossGap() - { - // Reproduces the race-condition gap scenario on top of the beacon sync path: - // - // genesis → headBlock(H=1) → d1(H=2) → d2(H=3) → d3(H=4) - // - // 1. FCU(headBlock): Head = headBlock at H=1. - // 2. Beacon sync marks d1, d2, d3 canonical (wereProcessed=false, Head stays at H=1). - // 3. Concurrent MoveToMain clears d2's HasBlockOnMainChain → gap at H=3. - // 4. FCU(sibling) at H=1: previousHeadNumber==lastNumber==1 → Phase 1 skips. - // Phase 2 upward scan: clears d1(H=2), hits gap at d2(H=3), breaks. - // d3(H=4) remains stale canonical — BUG. - // - BlockTree blockTree = BuildBlockTree(); - - Block genesis = Build.A.Block.WithNumber(0).TestObject; - blockTree.SuggestBlock(genesis); - blockTree.UpdateMainChain(new[] { genesis }, wereProcessed: true, forceUpdateHeadBlock: true); - - Block headBlock = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0xAA }).TestObject; - Block d1 = Build.A.Block.WithNumber(2).WithParent(headBlock).TestObject; - Block d2 = Build.A.Block.WithNumber(3).WithParent(d1).TestObject; - Block d3 = Build.A.Block.WithNumber(4).WithParent(d2).TestObject; - - blockTree.SuggestBlock(headBlock); - blockTree.SuggestBlock(d1); - blockTree.SuggestBlock(d2); - blockTree.SuggestBlock(d3); - - // FCU(headBlock): head = headBlock at H=1 - blockTree.UpdateMainChain(new[] { headBlock }, wereProcessed: true, forceUpdateHeadBlock: true); - blockTree.Head!.Hash.Should().Be(headBlock.Hash!); - - // Beacon sync: d1, d2, d3 marked canonical, Head stays at H=1 - blockTree.UpdateMainChain(new[] { d1 }, wereProcessed: false); - blockTree.UpdateMainChain(new[] { d2 }, wereProcessed: false); - blockTree.UpdateMainChain(new[] { d3 }, wereProcessed: false); - - blockTree.Head!.Number.Should().Be(1, "Head must stay at H=1 — wereProcessed=false"); - blockTree.IsMainChain(d1.Header).Should().BeTrue("precondition: d1 canonical via beacon sync"); - blockTree.IsMainChain(d2.Header).Should().BeTrue("precondition: d2 canonical via beacon sync"); - blockTree.IsMainChain(d3.Header).Should().BeTrue("precondition: d3 canonical via beacon sync"); - - // Simulate race: concurrent MoveToMain clears d2's marker, creating a gap at H=3 - ChainLevelInfo? levelD2 = blockTree.FindLevel(d2.Number); - levelD2!.HasBlockOnMainChain = false; - - // Precondition: gap exists - blockTree.IsMainChain(d2.Header).Should().BeFalse("precondition: d2 gap — cleared by simulated race"); - blockTree.IsMainChain(d3.Header).Should().BeTrue("precondition: d3 still stale canonical past the gap"); - - // FCU(sibling) at H=1: reorg to sibling, previousHeadNumber==lastNumber==1 - Block sibling = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData(new byte[] { 0xBB }).TestObject; - blockTree.SuggestBlock(sibling); - blockTree.UpdateMainChain(new[] { sibling }, wereProcessed: true, forceUpdateHeadBlock: true); - - blockTree.Head!.Hash.Should().Be(sibling.Hash!); - blockTree.IsMainChain(sibling.Header).Should().BeTrue("sibling must be canonical"); - blockTree.IsMainChain(d1.Header).Should().BeFalse("d1 must be de-canonicalized after reorg"); - blockTree.IsMainChain(d2.Header).Should().BeFalse("d2 must stay non-canonical (gap)"); - blockTree.IsMainChain(d3.Header).Should().BeFalse("d3 must be de-canonicalized — bounded scan must reach past the gap"); - - blockTree.FindCanonicalBlockInfo(2).Should().BeNull("H+1 must be null — orphaned after reorg"); - blockTree.FindCanonicalBlockInfo(3).Should().BeNull("H+2 must be null — gap, non-canonical"); - blockTree.FindCanonicalBlockInfo(4).Should().BeNull("H+3 must be null — orphaned, must not survive the gap"); - } } diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index 0401f912bce1..c341c69376e2 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -767,9 +767,11 @@ as it does not require the step of resolving number -> hash */ return level.BlockInfos[0].BlockHash; } + bool IsPostMerge(Block? block) => SpecProvider.TerminalTotalDifficulty is { } ttd + && (block?.TotalDifficulty ?? UInt256.Zero) >= ttd; + // Post-merge: TD never increases, so the best-TD fallback cannot distinguish canonical from orphaned. - if (SpecProvider.TerminalTotalDifficulty is not null - && (Head?.TotalDifficulty ?? UInt256.Zero) >= SpecProvider.TerminalTotalDifficulty) + if (IsPostMerge(Head)) { return null; } @@ -1083,7 +1085,7 @@ private long RepairMarkersBelow(BlockHeader start, long maxBlockDepth, BatchWrit long blocksWalked = 0L; BlockHeader? current = start; - while (current is not null && blocksWalked <= maxBlockDepth) + while (blocksWalked <= maxBlockDepth) { ChainLevelInfo? level = LoadLevel(current.Number); if (level is not null) @@ -1127,15 +1129,9 @@ private long RepairMarkersBelow(BlockHeader start, long maxBlockDepth, BatchWrit private void TryUpdateSyncPivot() { - BlockHeader? newPivotHeader = null; - if (FinalizedHash is not null) - { - newPivotHeader = FindHeader(FinalizedHash, BlockTreeLookupOptions.RequireCanonical); - } - else - { - newPivotHeader = FindHeader(Math.Max(0, (Head?.Number ?? 0) - Reorganization.MaxDepth), BlockTreeLookupOptions.RequireCanonical); - } + BlockHeader? newPivotHeader = FinalizedHash is not null + ? FindHeader(FinalizedHash, BlockTreeLookupOptions.RequireCanonical) + : FindHeader(Math.Max(0, (Head?.Number ?? 0) - Reorganization.MaxDepth), BlockTreeLookupOptions.RequireCanonical); if (newPivotHeader is null) { From 402195da683a01a13a6f386e0783cf04c4414de1 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Fri, 27 Mar 2026 15:02:03 +0200 Subject: [PATCH 20/25] Lukasz review --- .../BlockTreeTests.cs | 75 ++++++++++++++++ .../BlockTree.Healing.cs | 90 +++++++++++++++++++ .../Nethermind.Blockchain/BlockTree.cs | 77 ---------------- .../Nethermind.Blockchain/BlockTreeOverlay.cs | 2 +- .../Nethermind.Blockchain/IBlockTree.cs | 8 -- .../Nethermind.Init/Steps/ReviewBlockTree.cs | 3 +- 6 files changed, 168 insertions(+), 87 deletions(-) create mode 100644 src/Nethermind/Nethermind.Blockchain/BlockTree.Healing.cs diff --git a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs index 793925ff26aa..8d0d91469757 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/BlockTreeTests.cs @@ -2551,6 +2551,81 @@ public void HealCanonicalChain_WhenDepthExceedsMaxBlockDepth_StopsAtLimit() .Should().Be(b1Alt.Hash!, "H=1 must remain broken — it is beyond maxBlockDepth"); } + [Test, MaxTime(Timeout.MaxTestTime)] + public void UpdateMainChain_WhenBeaconSyncAndFcuCycleRepeatedTwice_ClearsStaleMarkersEachRound() + { + // Two full beacon-sync + FCU cycles at the same head height (H=1). + // Each round: beacon sync marks descendants canonical, then FCU reorgs to a new sibling. + // After each FCU, all stale markers from that round must be cleared before the next round. + // + // Round 1: + // FCU(head): head=H=1 + // Beacon sync: desc1[0] (H=2), desc1[1] (H=3) marked canonical without updating Head. + // FCU(sibling1): reorg to sibling1 at H=1 — stale desc1 markers must be cleared. + // + // Round 2 (starting from sibling1 as new head): + // Beacon sync: desc2[0] (H=2), desc2[1] (H=3) from sibling1's chain, marked canonical. + // FCU(sibling2): reorg to sibling2 at H=1 — stale desc2 markers must be cleared. + (BlockTree blockTree, Block genesis) = BuildBlockTreeWithGenesis(forceUpdateHead: true); + + Block head = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData([0xAA]).TestObject; + blockTree.SuggestBlock(head); + blockTree.UpdateMainChain(new[] { head }, wereProcessed: true, forceUpdateHeadBlock: true); + + // Round 1 — beacon sync marks two descendants of head canonical + Block[] desc1 = BuildAndSuggestChain(blockTree, head, 2); + foreach (Block d in desc1) + { + blockTree.UpdateMainChain(new[] { d }, wereProcessed: false); + } + + blockTree.Head!.Hash.Should().Be(head.Hash!, "precondition: head stale at H=1 after round-1 beacon sync"); + foreach (Block d in desc1) + { + blockTree.IsMainChain(d.Header).Should().BeTrue($"precondition: round-1 desc at H={d.Number} canonical via beacon sync"); + } + + // Round 1 FCU — reorg to sibling1 at H=1 + Block sibling1 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData([0xBB]).TestObject; + blockTree.SuggestBlock(sibling1); + blockTree.UpdateMainChain(new[] { sibling1 }, wereProcessed: true, forceUpdateHeadBlock: true); + + blockTree.Head!.Hash.Should().Be(sibling1.Hash!, "after round-1 FCU head must be sibling1"); + foreach (Block d in desc1) + { + blockTree.IsMainChain(d.Header).Should().BeFalse($"round-1 stale marker at H={d.Number} must be cleared after FCU to sibling1"); + } + + // Round 2 — beacon sync marks two descendants of sibling1 canonical + Block[] desc2 = BuildAndSuggestChain(blockTree, sibling1, 2); + foreach (Block d in desc2) + { + blockTree.UpdateMainChain(new[] { d }, wereProcessed: false); + } + + blockTree.Head!.Hash.Should().Be(sibling1.Hash!, "precondition: head stale at sibling1 after round-2 beacon sync"); + foreach (Block d in desc2) + { + blockTree.IsMainChain(d.Header).Should().BeTrue($"precondition: round-2 desc at H={d.Number} canonical via beacon sync"); + } + + // Round 2 FCU — reorg to sibling2 at H=1 + Block sibling2 = Build.A.Block.WithNumber(1).WithParent(genesis).WithExtraData([0xCC]).TestObject; + blockTree.SuggestBlock(sibling2); + blockTree.UpdateMainChain(new[] { sibling2 }, wereProcessed: true, forceUpdateHeadBlock: true); + + blockTree.Head!.Hash.Should().Be(sibling2.Hash!, "after round-2 FCU head must be sibling2"); + foreach (Block d in desc2) + { + blockTree.IsMainChain(d.Header).Should().BeFalse($"round-2 stale marker at H={d.Number} must be cleared after FCU to sibling2"); + } + + // Sibling2 is canonical at H=1; head, sibling1 are orphaned + blockTree.IsMainChain(sibling2.Header).Should().BeTrue("sibling2 must be canonical at H=1"); + blockTree.IsMainChain(head.Header).Should().BeFalse("original head must be orphaned"); + blockTree.IsMainChain(sibling1.Header).Should().BeFalse("sibling1 must be orphaned"); + } + [Test, MaxTime(Timeout.MaxTestTime)] public void FindBlock_WhenBlockOrphanedAfterReorgInPoS_ReturnsNull() { diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.Healing.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.Healing.cs new file mode 100644 index 000000000000..b6abba4d72e5 --- /dev/null +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.Healing.cs @@ -0,0 +1,90 @@ +// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using Nethermind.Core; +using Nethermind.Core.Crypto; +using Nethermind.State.Repositories; + +namespace Nethermind.Blockchain +{ + public partial class BlockTree + { + public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) + { + BlockHeader? start = FindHeader(startHash, BlockTreeLookupOptions.None); + if (start is null) return; + + using BatchWrite batch = _chainLevelInfoRepository.StartBatch(); + + long repairedAbove = ClearStaleMarkersAbove(start.Number, batch); + long repairedBelow = RepairMarkersBelow(start, maxBlockDepth, batch); + + if (Logger.IsInfo) Logger.Info($"Canonical chain heal complete: {repairedAbove + repairedBelow} level(s) repaired ({repairedAbove} stale above head cleared, {repairedBelow} incorrect markers fixed)."); + } + + private long ClearStaleMarkersAbove(long fromExclusive, BatchWrite batch) + { + long cleared = 0L; + for (long levelNumber = fromExclusive + 1; ; levelNumber++) + { + ChainLevelInfo? level = LoadLevel(levelNumber); + if (level is null) break; + if (level.HasBlockOnMainChain) + { + level.HasBlockOnMainChain = false; + _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); + cleared++; + } + } + return cleared; + } + + private long RepairMarkersBelow(BlockHeader start, long maxBlockDepth, BatchWrite batch) + { + long repairedCount = 0L; + long blocksWalked = 0L; + BlockHeader current = start; + + while (blocksWalked <= maxBlockDepth) + { + ChainLevelInfo? level = LoadLevel(current.Number); + if (level is not null) + { + int? index = level.FindIndex(current.Hash!); + if (index is null) + { + if (Logger.IsWarn) Logger.Warn($"Canonical heal: block {current.Hash} at height {current.Number} not found in any BlockInfo slot — repair halted."); + break; + } + + bool needsRepair = index.Value != 0 || !level.HasBlockOnMainChain; + + if (index.Value != 0) + level.SwapToMain(index.Value); + + level.HasBlockOnMainChain = true; + + if (needsRepair) + { + _chainLevelInfoRepository.PersistLevel(current.Number, level, batch); + repairedCount++; + } + } + + if (current.IsGenesis) break; + + BlockHeader? parent = FindHeader(current.ParentHash!, BlockTreeLookupOptions.None); + if (parent is null) + { + if (Logger.IsWarn) Logger.Warn($"Canonical heal: parent {current.ParentHash} of block {current.Number} not found — chain may be pruned, repair halted."); + break; + } + + current = parent; + blocksWalked++; + } + + return repairedCount; + } + } +} diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs index c341c69376e2..42c493d714a0 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.cs @@ -1049,83 +1049,6 @@ public void UpdateMainChain(IReadOnlyList blocks, bool wereProcessed, boo OnUpdateMainChain?.Invoke(this, new OnUpdateMainChainArgs(blocks, wereProcessed)); } - public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) - { - BlockHeader? start = FindHeader(startHash, BlockTreeLookupOptions.None); - if (start is null) return; - - using BatchWrite batch = _chainLevelInfoRepository.StartBatch(); - - long repairedAbove = ClearStaleMarkersAbove(start.Number, batch); - long repairedBelow = RepairMarkersBelow(start, maxBlockDepth, batch); - - if (Logger.IsInfo) Logger.Info($"Canonical chain heal complete: {repairedAbove + repairedBelow} level(s) repaired ({repairedAbove} stale above head cleared, {repairedBelow} incorrect markers fixed)."); - } - - private long ClearStaleMarkersAbove(long fromExclusive, BatchWrite batch) - { - long cleared = 0L; - for (long levelNumber = fromExclusive + 1; ; levelNumber++) - { - ChainLevelInfo? level = LoadLevel(levelNumber); - if (level is null) break; - if (level.HasBlockOnMainChain) - { - level.HasBlockOnMainChain = false; - _chainLevelInfoRepository.PersistLevel(levelNumber, level, batch); - cleared++; - } - } - return cleared; - } - - private long RepairMarkersBelow(BlockHeader start, long maxBlockDepth, BatchWrite batch) - { - long repairedCount = 0L; - long blocksWalked = 0L; - BlockHeader? current = start; - - while (blocksWalked <= maxBlockDepth) - { - ChainLevelInfo? level = LoadLevel(current.Number); - if (level is not null) - { - int? index = level.FindIndex(current.Hash!); - if (index is null) - { - if (Logger.IsWarn) Logger.Warn($"Canonical heal: block {current.Hash} at height {current.Number} not found in any BlockInfo slot — repair halted."); - break; - } - - bool needsRepair = index.Value != 0 || !level.HasBlockOnMainChain; - - if (index.Value != 0) - level.SwapToMain(index.Value); - - level.HasBlockOnMainChain = true; - - if (needsRepair) - { - _chainLevelInfoRepository.PersistLevel(current.Number, level, batch); - repairedCount++; - } - } - - if (current.IsGenesis) break; - - BlockHeader? parent = FindHeader(current.ParentHash!, BlockTreeLookupOptions.None); - if (parent is null) - { - if (Logger.IsWarn) Logger.Warn($"Canonical heal: parent {current.ParentHash} of block {current.Number} not found — chain may be pruned, repair halted."); - break; - } - - current = parent; - blocksWalked++; - } - - return repairedCount; - } private void TryUpdateSyncPivot() { diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs b/src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs index ae862b499cfd..b1b862c2449f 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs @@ -245,7 +245,7 @@ public void UpdateBeaconMainChain(BlockInfo[]? blockInfos, long clearBeaconMainC _overlayTree.UpdateBeaconMainChain(blockInfos, clearBeaconMainChainStartPoint); public void RecalculateTreeLevels() => _overlayTree.RecalculateTreeLevels(); - public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) => _overlayTree.HealCanonicalChain(startHash, maxBlockDepth); + public (long BlockNumber, Hash256 BlockHash) SyncPivot { get => _baseTree.SyncPivot; diff --git a/src/Nethermind/Nethermind.Blockchain/IBlockTree.cs b/src/Nethermind/Nethermind.Blockchain/IBlockTree.cs index 67fa3cf9286f..f3531b6f8f91 100644 --- a/src/Nethermind/Nethermind.Blockchain/IBlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/IBlockTree.cs @@ -191,14 +191,6 @@ AddBlockResult Insert(Block block, BlockTreeInsertBlockOptions insertBlockOption void RecalculateTreeLevels(); - /// - /// Repairs canonical chain markers starting from , walking backward - /// up to blocks. Also removes stale canonical markers above - /// left by the beacon-sync path. Safe to run at startup after - /// observing canonical mismatches (wrong eth_getBlockByNumber results). - /// - void HealCanonicalChain(Hash256 startHash, long maxBlockDepth); - /// /// Sync pivot is mainly concerned with old blocks and receipts. /// After sync pivot, blocks and headers should be continuous. diff --git a/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs b/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs index 151078286a19..5a72925bb22a 100644 --- a/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs +++ b/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs @@ -39,12 +39,13 @@ public Task Execute(CancellationToken cancellationToken) private void HealCanonicalChainIfEnabled() { if (!initConfig.HealCanonicalChain) return; + if (blockTree is not BlockTree concreteBlockTree) return; Hash256? startHash = blockTree.Head?.Hash; if (startHash is not null) { if (_logger.IsInfo) _logger.Info($"Healing canonical chain from head {startHash} (depth {initConfig.HealCanonicalChainDepth})..."); - blockTree.HealCanonicalChain(startHash, initConfig.HealCanonicalChainDepth); + concreteBlockTree.HealCanonicalChain(startHash, initConfig.HealCanonicalChainDepth); } else { From b7a54d1cf4057cddcd2cd995733cf0c0b833176a Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 27 Mar 2026 14:02:16 +0100 Subject: [PATCH 21/25] fixes --- src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs | 1 + .../Nethermind.Network/Rlpx/NettyHandshakeHandler.cs | 3 ++- .../Nethermind.Shutter/ShutterBlockImprovementContext.cs | 7 ++++--- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs b/src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs index 169a841af8e3..242264612d5a 100644 --- a/src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs +++ b/src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs @@ -83,6 +83,7 @@ private async Task CleanOldRequests() if (_requests.TryRemove(requestIdValues.Key, out Request request)) { Interlocked.Decrement(ref _requestCount); + request.Message.TryDispose(); // Unblock waiting thread. request.CompletionSource.TrySetException(new TimeoutException("No response received")); } diff --git a/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs b/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs index 4e5a09a5e28b..c3843034d703 100644 --- a/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs +++ b/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs @@ -201,7 +201,7 @@ private async Task CheckHandshakeInitTimeout() try { Task receivedInitMsgTask = _initCompletionSource.Task; - CancellationTokenSource delayCancellation = new(); + using CancellationTokenSource delayCancellation = new(); Task firstTask = await Task.WhenAny(receivedInitMsgTask, Task.Delay(Timeouts.Handshake, delayCancellation.Token)); if (firstTask != receivedInitMsgTask) @@ -209,6 +209,7 @@ private async Task CheckHandshakeInitTimeout() Metrics.HandshakeTimeouts++; if (_logger.IsTrace) _logger.Trace($"Disconnecting due to timeout for handshake: {_session.RemoteNodeId}@{_session.RemoteHost}:{_session.RemotePort}"); //It will trigger channel.CloseCompletion which will trigger DisconnectAsync on the session + _initCompletionSource.TrySetCanceled(); await _channel.DisconnectAsync(); } else diff --git a/src/Nethermind/Nethermind.Shutter/ShutterBlockImprovementContext.cs b/src/Nethermind/Nethermind.Shutter/ShutterBlockImprovementContext.cs index bc213677c2ee..a5f0db786cf4 100644 --- a/src/Nethermind/Nethermind.Shutter/ShutterBlockImprovementContext.cs +++ b/src/Nethermind/Nethermind.Shutter/ShutterBlockImprovementContext.cs @@ -142,12 +142,13 @@ public void Dispose() if (_logger.IsDebug) _logger.Debug($"Awaiting Shutter decryption keys for {slot} at offset {offset}ms. Timeout in {waitTime}ms..."); using var txTimeout = new CancellationTokenSource((int)waitTime); - _linkedCancellation = CancellationTokenSource.CreateLinkedTokenSource(_improvementCancellation.Token, txTimeout.Token); + CancellationTokenSource linkedCancellation = CancellationTokenSource.CreateLinkedTokenSource(_improvementCancellation.Token, txTimeout.Token); + _linkedCancellation = linkedCancellation; try { - _linkedCancellation.Token.ThrowIfCancellationRequested(); - await _txSignal.WaitForTransactions(slot, _linkedCancellation.Token); + linkedCancellation.Token.ThrowIfCancellationRequested(); + await _txSignal.WaitForTransactions(slot, linkedCancellation.Token); } catch (Exception ex) when (ex is OperationCanceledException or ObjectDisposedException) { From 62ba845ba346f96671e0613f533dc30a56638f22 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 27 Mar 2026 14:02:51 +0100 Subject: [PATCH 22/25] Revert "fixes" This reverts commit b7a54d1cf4057cddcd2cd995733cf0c0b833176a. --- src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs | 1 - .../Nethermind.Network/Rlpx/NettyHandshakeHandler.cs | 3 +-- .../Nethermind.Shutter/ShutterBlockImprovementContext.cs | 7 +++---- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs b/src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs index 242264612d5a..169a841af8e3 100644 --- a/src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs +++ b/src/Nethermind/Nethermind.Network/P2P/MessageDictionary.cs @@ -83,7 +83,6 @@ private async Task CleanOldRequests() if (_requests.TryRemove(requestIdValues.Key, out Request request)) { Interlocked.Decrement(ref _requestCount); - request.Message.TryDispose(); // Unblock waiting thread. request.CompletionSource.TrySetException(new TimeoutException("No response received")); } diff --git a/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs b/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs index c3843034d703..4e5a09a5e28b 100644 --- a/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs +++ b/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs @@ -201,7 +201,7 @@ private async Task CheckHandshakeInitTimeout() try { Task receivedInitMsgTask = _initCompletionSource.Task; - using CancellationTokenSource delayCancellation = new(); + CancellationTokenSource delayCancellation = new(); Task firstTask = await Task.WhenAny(receivedInitMsgTask, Task.Delay(Timeouts.Handshake, delayCancellation.Token)); if (firstTask != receivedInitMsgTask) @@ -209,7 +209,6 @@ private async Task CheckHandshakeInitTimeout() Metrics.HandshakeTimeouts++; if (_logger.IsTrace) _logger.Trace($"Disconnecting due to timeout for handshake: {_session.RemoteNodeId}@{_session.RemoteHost}:{_session.RemotePort}"); //It will trigger channel.CloseCompletion which will trigger DisconnectAsync on the session - _initCompletionSource.TrySetCanceled(); await _channel.DisconnectAsync(); } else diff --git a/src/Nethermind/Nethermind.Shutter/ShutterBlockImprovementContext.cs b/src/Nethermind/Nethermind.Shutter/ShutterBlockImprovementContext.cs index a5f0db786cf4..bc213677c2ee 100644 --- a/src/Nethermind/Nethermind.Shutter/ShutterBlockImprovementContext.cs +++ b/src/Nethermind/Nethermind.Shutter/ShutterBlockImprovementContext.cs @@ -142,13 +142,12 @@ public void Dispose() if (_logger.IsDebug) _logger.Debug($"Awaiting Shutter decryption keys for {slot} at offset {offset}ms. Timeout in {waitTime}ms..."); using var txTimeout = new CancellationTokenSource((int)waitTime); - CancellationTokenSource linkedCancellation = CancellationTokenSource.CreateLinkedTokenSource(_improvementCancellation.Token, txTimeout.Token); - _linkedCancellation = linkedCancellation; + _linkedCancellation = CancellationTokenSource.CreateLinkedTokenSource(_improvementCancellation.Token, txTimeout.Token); try { - linkedCancellation.Token.ThrowIfCancellationRequested(); - await _txSignal.WaitForTransactions(slot, linkedCancellation.Token); + _linkedCancellation.Token.ThrowIfCancellationRequested(); + await _txSignal.WaitForTransactions(slot, _linkedCancellation.Token); } catch (Exception ex) when (ex is OperationCanceledException or ObjectDisposedException) { From b646a6a4ecc6a1d473bbbc8dbf961965ea25548f Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Fri, 27 Mar 2026 15:12:05 +0200 Subject: [PATCH 23/25] Lukasz review - Refactoring --- .../Nethermind.Blockchain/BlockTree.Healing.cs | 2 +- .../Nethermind.Blockchain/IBlockTreeHealer.cs | 17 +++++++++++++++++ .../Nethermind.Init/Modules/BlockTreeModule.cs | 1 + .../Nethermind.Init/Steps/ReviewBlockTree.cs | 4 ++-- 4 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 src/Nethermind/Nethermind.Blockchain/IBlockTreeHealer.cs diff --git a/src/Nethermind/Nethermind.Blockchain/BlockTree.Healing.cs b/src/Nethermind/Nethermind.Blockchain/BlockTree.Healing.cs index b6abba4d72e5..791e1089eeb7 100644 --- a/src/Nethermind/Nethermind.Blockchain/BlockTree.Healing.cs +++ b/src/Nethermind/Nethermind.Blockchain/BlockTree.Healing.cs @@ -7,7 +7,7 @@ namespace Nethermind.Blockchain { - public partial class BlockTree + public partial class BlockTree : IBlockTreeHealer { public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) { diff --git a/src/Nethermind/Nethermind.Blockchain/IBlockTreeHealer.cs b/src/Nethermind/Nethermind.Blockchain/IBlockTreeHealer.cs new file mode 100644 index 000000000000..5200d2d1d29b --- /dev/null +++ b/src/Nethermind/Nethermind.Blockchain/IBlockTreeHealer.cs @@ -0,0 +1,17 @@ +// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using Nethermind.Core.Crypto; + +namespace Nethermind.Blockchain +{ + public interface IBlockTreeHealer + { + /// + /// Walks backward from up to blocks, + /// repairing incorrect HasBlockOnMainChain markers. Also removes stale canonical markers + /// above left by the beacon-sync path. + /// + void HealCanonicalChain(Hash256 startHash, long maxBlockDepth); + } +} diff --git a/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs b/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs index 56f4eb3edac0..99a20d7ae3df 100644 --- a/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs +++ b/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs @@ -46,6 +46,7 @@ protected override void Load(ContainerBuilder builder) .AddSingleton() .AddSingleton() .Bind() + .AddSingleton((bt) => (IBlockTreeHealer)bt) .AddSingleton((bt) => bt.AsReadOnly()); builder.AddSingleton() diff --git a/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs b/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs index 5a72925bb22a..d71a46617a6d 100644 --- a/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs +++ b/src/Nethermind/Nethermind.Init/Steps/ReviewBlockTree.cs @@ -23,6 +23,7 @@ public class ReviewBlockTree( ISyncConfig syncConfig, IBlockProcessingQueue blockProcessingQueue, IBlockTree blockTree, + IBlockTreeHealer blockTreeHealer, ILogManager logManager ) : IStep { @@ -39,13 +40,12 @@ public Task Execute(CancellationToken cancellationToken) private void HealCanonicalChainIfEnabled() { if (!initConfig.HealCanonicalChain) return; - if (blockTree is not BlockTree concreteBlockTree) return; Hash256? startHash = blockTree.Head?.Hash; if (startHash is not null) { if (_logger.IsInfo) _logger.Info($"Healing canonical chain from head {startHash} (depth {initConfig.HealCanonicalChainDepth})..."); - concreteBlockTree.HealCanonicalChain(startHash, initConfig.HealCanonicalChainDepth); + blockTreeHealer.HealCanonicalChain(startHash, initConfig.HealCanonicalChainDepth); } else { From d1bd0214f0c19505bb1f1d45aa4d919bcf176c87 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Fri, 27 Mar 2026 15:27:36 +0200 Subject: [PATCH 24/25] fixes --- src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs | 1 - .../Nethermind.Consensus/Stateless/StatelessBlockTree.cs | 3 --- src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs | 7 ++++--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs b/src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs index 3db94ffc941b..867c44dc267b 100644 --- a/src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs +++ b/src/Nethermind/Nethermind.Blockchain/ReadOnlyBlockTree.cs @@ -193,7 +193,6 @@ public int DeleteChainSlice(in long startNumber, long? endNumber = null, bool fo public bool IsBetterThanHead(BlockHeader? header) => _wrapped.IsBetterThanHead(header); public void UpdateBeaconMainChain(BlockInfo[]? blockInfos, long clearBeaconMainChainStartPoint) => throw new InvalidOperationException($"{nameof(ReadOnlyBlockTree)} does not expect {nameof(UpdateBeaconMainChain)} calls"); public void RecalculateTreeLevels() => throw new InvalidOperationException($"{nameof(ReadOnlyBlockTree)} does not expect {nameof(RecalculateTreeLevels)} calls"); - public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) => throw new InvalidOperationException($"{nameof(ReadOnlyBlockTree)} does not expect {nameof(HealCanonicalChain)} calls"); public (long BlockNumber, Hash256 BlockHash) SyncPivot { get => _wrapped.SyncPivot; diff --git a/src/Nethermind/Nethermind.Consensus/Stateless/StatelessBlockTree.cs b/src/Nethermind/Nethermind.Consensus/Stateless/StatelessBlockTree.cs index acef3e8f5c87..a21bc941b58b 100644 --- a/src/Nethermind/Nethermind.Consensus/Stateless/StatelessBlockTree.cs +++ b/src/Nethermind/Nethermind.Consensus/Stateless/StatelessBlockTree.cs @@ -192,9 +192,6 @@ public void UpdateBeaconMainChain(BlockInfo[]? blockInfos, long clearBeaconMainC public void RecalculateTreeLevels() => throw new NotSupportedException(); - public void HealCanonicalChain(Hash256 startHash, long maxBlockDepth) - => throw new NotSupportedException(); - public (long BlockNumber, Hash256 BlockHash) SyncPivot { get => throw new NotSupportedException(); diff --git a/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs b/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs index 99a20d7ae3df..d0523e3cd772 100644 --- a/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs +++ b/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs @@ -44,9 +44,10 @@ protected override void Load(ContainerBuilder builder) ) .AddSingleton() .AddSingleton() - .AddSingleton() - .Bind() - .AddSingleton((bt) => (IBlockTreeHealer)bt) + .AddSingleton() + .Bind() + .Bind() + .Bind() .AddSingleton((bt) => bt.AsReadOnly()); builder.AddSingleton() From fb43b3ba7a0ede7a9e8748c4b5c7b3121a2f6c93 Mon Sep 17 00:00:00 2001 From: stavrosvl7 Date: Fri, 27 Mar 2026 16:37:55 +0200 Subject: [PATCH 25/25] revert blocktree registration --- src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs b/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs index d0523e3cd772..0c0c2020d7c9 100644 --- a/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs +++ b/src/Nethermind/Nethermind.Init/Modules/BlockTreeModule.cs @@ -45,9 +45,9 @@ protected override void Load(ContainerBuilder builder) .AddSingleton() .AddSingleton() .AddSingleton() - .Bind() - .Bind() - .Bind() + .AddSingleton() + .Bind() + .AddSingleton((bt) => (IBlockTreeHealer)bt) .AddSingleton((bt) => bt.AsReadOnly()); builder.AddSingleton()