Glamsterdam - Engine API Changes

src/Nethermind/Nethermind.Merge.Plugin.Test/BaseEngineModuleTests.cs

@@ -107,7 +107,9 @@ protected async Task<IReadOnlyList<ExecutionPayload>> ProduceBranchV1(IEngineRpc
             if (setHead)
             {
                 Hash256 newHead = getPayloadResult.BlockHash;
-                ForkchoiceStateV1 forkchoiceStateV1 = new(newHead, newHead, newHead);
+                // Use Keccak.Zero for finalized/safe: ProduceBranchV1 is a chain-building helper,
+                // not a finality-setting one. Tests that need finalized must set it explicitly.
+                ForkchoiceStateV1 forkchoiceStateV1 = new(newHead, Keccak.Zero, Keccak.Zero);
                 ResultWrapper<ForkchoiceUpdatedV1Result> setHeadResponse = await rpc.engine_forkchoiceUpdatedV1(forkchoiceStateV1);
                 setHeadResponse.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
                 setHeadResponse.Data.PayloadId.Should().Be(null);

src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs

@@ -14,6 +14,7 @@
 using FluentAssertions;
 using Nethermind.Blockchain;
 using Nethermind.Blockchain.Find;
+using Nethermind.Blockchain.Synchronization;
 using Nethermind.Consensus.Processing;
 using Nethermind.Consensus.Producers;
 using Nethermind.Core;
@@ -564,6 +565,213 @@ public async Task forkchoiceUpdatedV1_should_update_safe_block_hash()
     }
 
 
+    [TestCase(3, TestName = "2 blocks behind head — within PruningBoundary")]
+    [TestCase(33, TestName = "32 blocks behind head — within PruningBoundary (default 64)")]
+    public async Task forkchoiceUpdatedV1_WhenHeadIsCanonicalAncestorWithinPruningBoundary_ReorgsToAncestor(int chainLength)
+    {
+        // Spec PR #786: MUST reorg to a canonical ancestor when reorg depth <= PruningBoundary (default 64).
+        // No finalized block is set — the MAY-skip (spec point 2) never fires; the reorg proceeds
+        // because depth < PruningBoundary, not because of any old "32-block limit".
+        using MergeTestBlockchain chain = await CreateBlockchain();
+        IEngineRpcModule rpc = chain.EngineRpcModule;
+
+        IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, chainLength, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: false);
+        Hash256 b1Hash = branch[0].BlockHash;
+        Hash256 headHash = branch[chainLength - 1].BlockHash;
+
+        // Advance head to the last block without setting finalized
+        (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(headHash, Keccak.Zero, Keccak.Zero))).Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+        chain.BlockTree.HeadHash.Should().Be(headHash, $"precondition: head is at H={chainLength}");
+
+        ForkchoiceStateV1 fcuToAncestor = new(b1Hash, Keccak.Zero, Keccak.Zero);
+        ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuToAncestor);
+
+        result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+        chain.BlockTree.HeadHash.Should().Be(b1Hash, $"head must reorg to b1 — depth {chainLength - 1} is within PruningBoundary (default 64)");
+    }
+
+    [Test]
+    public async Task forkchoiceUpdatedV1_WhenHeadIsAncestorOfFinalizedBlock_SkipsUpdate()
+    {
+        // Spec PR #786 point 2: client MAY skip the update when headBlockHash is a valid ancestor of the
+        // latest known finalized block. Build 34 blocks, explicitly finalize b34, then FCU to b1
+        // (H=1 <= H=34, canonical) — the MAY-skip fires, not any depth limit.
+        using MergeTestBlockchain chain = await CreateBlockchain();
+        IEngineRpcModule rpc = chain.EngineRpcModule;
+
+        IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 34, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: false);
+        Hash256 b1Hash = branch[0].BlockHash;
+        Hash256 b34Hash = branch[33].BlockHash;
+
+        // Set head to b34 and explicitly finalize it
+        (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(b34Hash, b34Hash, b34Hash))).Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+        chain.BlockTree.HeadHash.Should().Be(b34Hash, "precondition: head is at H=34");
+        chain.BlockTree.FinalizedHash.Should().Be(b34Hash, "precondition: b34 is finalized");
+
+        ForkchoiceStateV1 fcuToAncestorOfFinalized = new(b1Hash, Keccak.Zero, Keccak.Zero);
+        ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuToAncestorOfFinalized);
+
+        result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+        result.Data.PayloadStatus.LatestValidHash.Should().Be(b1Hash, "spec mandates latestValidHash == forkchoiceState.headBlockHash when skipping");
+        result.Data.PayloadId.Should().BeNull("spec mandates payloadId: null when skipping");
+        chain.BlockTree.HeadHash.Should().Be(b34Hash, "Nethermind skips the update — b1 is a canonical ancestor of the finalized block, skip is permitted by spec");
+    }
+
+    [Test]
+    public async Task forkchoiceUpdatedV1_WhenHeadIsOnDifferentBranch_ReorgsRegardlessOfDepth()
+    {
+        // Spec PR #786: the MAY-skip (spec point 2) only fires when headBlockHash is a canonical ancestor
+        // of the finalized block. A non-canonical (fork) block is NOT eligible for the skip.
+        // FindMainChainAncestorNumber returns 0 (genesis) for side-chain blocks, giving a large reported
+        // depth, but here depth (34) < PruningBoundary (64) so -38006 doesn't fire either — the reorg
+        // always proceeds. Builds a canonical chain of 34 blocks, then a side block off genesis.
+        using MergeTestBlockchain chain = await CreateBlockchain();
+        IEngineRpcModule rpc = chain.EngineRpcModule;
+
+        // Capture genesis as parent before building canonical chain
+        ExecutionPayload genesisAsParent = CreateParentBlockRequestOnHead(chain.BlockTree);
+
+        // Build canonical chain: genesis → b1 → b2 → ... → b34 (head at H=34)
+        IReadOnlyList<ExecutionPayload> canonical = await ProduceBranchV1(rpc, chain, 34, genesisAsParent, setHead: true);
+        Hash256 b34Hash = canonical[33].BlockHash;
+        chain.BlockTree.HeadHash.Should().Be(b34Hash, "precondition: canonical head is at H=34");
+
+        // Build a side block off genesis (H=1, different branch)
+        ExecutionPayload sideBlock = CreateBlockRequest(chain, genesisAsParent, TestItem.AddressA);
+        await rpc.engine_newPayloadV1(sideBlock);
+        Hash256 sideHash = sideBlock.BlockHash;
+        chain.BlockTree.IsMainChain(chain.BlockTree.FindHeader(sideHash, BlockTreeLookupOptions.None)!).Should().BeFalse("precondition: side block is not on canonical chain");
+
+        // FCU to the side block — it's on a different branch, so it must reorg regardless of depth
+        ForkchoiceStateV1 fcuToSide = new(sideHash, Keccak.Zero, Keccak.Zero);
+        ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuToSide);
+
+        result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+        chain.BlockTree.HeadHash.Should().Be(sideHash, "different-branch FCU must always reorg — MAY-skip only applies to canonical ancestors of the finalized block");
+    }
+
+    [Test]
+    public async Task forkchoiceUpdatedV1_WhenReorgDepthExceedsPruningBoundary_ReturnsTooDeepReorg()
+    {
+        // Spec PR #786 point 6: MUST return -38006 when reorg depth > client-specific limit.
+        // Disable SnapServing so PruningTrieStateFactory.AdviseConfig doesn't bump the boundary
+        // to SnapServingMaxDepth=128 (TestBlockchain wiring auto-flips SnapServingEnabled on for
+        // HalfPath key schemes via WorldStateModule; setting it to false here keeps the auto-flip
+        // a no-op since `|=` only fires when the value is null).
+        // AdviseConfig also enforces a hard floor of 64 on PruningBoundary, so the smallest
+        // chain that exercises the -38006 path is 66 blocks: reorgDepth = 66 - 1 = 65 > 64 → -38006.
+        using MergeTestBlockchain chain = await CreateBlockchain(configurer: b => b
+            .Intercept<ISyncConfig>(cfg => cfg.SnapServingEnabled = false));
+        IEngineRpcModule rpc = chain.EngineRpcModule;
+
+        IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 66, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: false);
+        Hash256 b1Hash = branch[0].BlockHash;
+        Hash256 b66Hash = branch[65].BlockHash;
+
+        (await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(b66Hash, Keccak.Zero, Keccak.Zero))).Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+
+        chain.BlockTree.HeadHash.Should().Be(b66Hash, "precondition: head is at H=66");
+
+        ForkchoiceStateV1 fcuTooDeep = new(b1Hash, Keccak.Zero, Keccak.Zero);
+        ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuTooDeep);
+
+        result.ErrorCode.Should().Be(MergeErrorCodes.TooDeepReorg, "reorg depth 65 exceeds PruningBoundary 64 — must return -38006");
+        chain.BlockTree.HeadHash.Should().Be(b66Hash, "head must not change when -38006 is returned");
+    }
+
+    [Test]
+    public async Task forkchoiceUpdatedV1_WhenZeroFinalizedAndSafeHash_ReturnsValidWithoutError()
+    {
+        // Spec: zero safeBlockHash and finalizedBlockHash mean "unknown" — must not return -38002.
+        // Models CL checkpoint-syncing from a non-finalized state where safe/finalized are unknown.
+        using MergeTestBlockchain chain = await CreateBlockchain();
+        IEngineRpcModule rpc = chain.EngineRpcModule;
+
+        IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 3, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
+        Hash256 headHash = branch[2].BlockHash;
+
+        ForkchoiceStateV1 fcuWithUnknownFinality = new(headHash, Keccak.Zero, Keccak.Zero);
+        ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuWithUnknownFinality);
+
+        result.ErrorCode.Should().Be(0, "zero safe/finalized hashes must not produce an error");
+        result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+        chain.BlockTree.HeadHash.Should().Be(headHash);
+    }
+
+    [Test]
+    public async Task forkchoiceUpdatedV1_WhenZeroFinalizedHash_PreservesKnownFinalizedHash()
+    {
+        // Spec PR #760: when finalizedBlockHash is zero, client MUST use the latest known finalized hash — not overwrite it with zero.
+        using MergeTestBlockchain chain = await CreateBlockchain();
+        IEngineRpcModule rpc = chain.EngineRpcModule;
+
+        IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 2, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: false);
+        Hash256 b1Hash = branch[0].BlockHash;
+        Hash256 b2Hash = branch[1].BlockHash;
+
+        // First FCU: set head to b2 and finalize b1
+        ForkchoiceStateV1 fcuWithFinalized = new(b2Hash, b1Hash, b1Hash);
+        await rpc.engine_forkchoiceUpdatedV1(fcuWithFinalized);
+        chain.BlockTree.FinalizedHash.Should().Be(b1Hash, "precondition: b1 is finalized after first FCU");
+
+        // Second FCU: zero finalizedBlockHash — must preserve b1 as finalized
+        ForkchoiceStateV1 fcuWithZeroFinalized = new(b2Hash, Keccak.Zero, Keccak.Zero);
+        ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuWithZeroFinalized);
+
+        result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+        chain.BlockTree.FinalizedHash.Should().Be(b1Hash, "zero finalizedBlockHash must preserve the previously known finalized hash");
+        chain.BlockTree.SafeHash.Should().Be(b1Hash, "zero safeBlockHash must preserve the previously known safe hash");
+    }
+
+    [Test]
+    public async Task forkchoiceUpdatedV1_WhenZeroFinalizedHash_PreservesKnownFinalizedHash_WithPayloadAttributes()
+    {
+        // Spec PR #760: ResolveZeroHash is called in StartBuildingPayload too — verify preservation on that path.
+        using MergeTestBlockchain chain = await CreateBlockchain();
+        IEngineRpcModule rpc = chain.EngineRpcModule;
+
+        IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 2, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: false);
+        Hash256 b1Hash = branch[0].BlockHash;
+        Hash256 b2Hash = branch[1].BlockHash;
+
+        // First FCU: set head to b2 and finalize b1
+        ForkchoiceStateV1 fcuWithFinalized = new(b2Hash, b1Hash, b1Hash);
+        await rpc.engine_forkchoiceUpdatedV1(fcuWithFinalized);
+        chain.BlockTree.FinalizedHash.Should().Be(b1Hash, "precondition: b1 is finalized after first FCU");
+
+        // Second FCU with payload attributes — exercises the StartBuildingPayload call site of ResolveZeroHash
+        PayloadAttributes payloadAttributes = new()
+        {
+            Timestamp = branch[1].Timestamp + 1,
+            PrevRandao = Keccak.Zero,
+            SuggestedFeeRecipient = Address.Zero
+        };
+        ForkchoiceStateV1 fcuWithZeroFinalized = new(b2Hash, Keccak.Zero, Keccak.Zero);
+        ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuWithZeroFinalized, payloadAttributes);
+
+        result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
+        result.Data.PayloadId.Should().NotBeNull("payload build must be started when attributes are provided");
+        chain.BlockTree.FinalizedHash.Should().Be(b1Hash, "zero finalizedBlockHash must preserve the previously known finalized hash via StartBuildingPayload");
+        chain.BlockTree.SafeHash.Should().Be(b1Hash, "zero safeBlockHash must preserve the previously known safe hash via StartBuildingPayload");
+    }
+
+    [Test]
+    public async Task forkchoiceUpdatedV1_WhenNonZeroUnknownFinalizedHash_ReturnsInvalidForkchoiceState()
+    {
+        // Spec: -38002 must only fire for non-zero hashes that are unknown, not for zero hashes.
+        using MergeTestBlockchain chain = await CreateBlockchain();
+        IEngineRpcModule rpc = chain.EngineRpcModule;
+
+        IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 1, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
+        Hash256 headHash = branch[0].BlockHash;
+
+        ForkchoiceStateV1 fcuWithUnknownFinalized = new(headHash, TestItem.KeccakA, Keccak.Zero);
+        ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuWithUnknownFinalized);
+
+        result.ErrorCode.Should().Be(MergeErrorCodes.InvalidForkchoiceState,
+            "non-zero unknown finalizedBlockHash must return -38002");
+    }
+
     [Test]
     public async Task forkchoiceUpdatedV1_should_work_with_zero_keccak_as_safe_block()
     {
@@ -1453,6 +1661,7 @@ public async Task forkchoiceUpdated_isInconsistent_takes_fast_path_when_candidat
         // Count FindHeader calls made by the repeated FCU only. Safe=Keccak.Zero skips its
         // ValidateBlockHash lookup, so the baseline calls are: 1 to resolve head, 1 for finalized
         // validation, plus the IsInconsistent walk (1 under the optimization, 2 without).
+        // ShouldProceedWithReorg is skipped because head is unchanged (blocks is null → no reorg).
         spy.ResetCounters();
         ForkchoiceStateV1 repeated = new(headBlockHash: a3.BlockHash, finalizedBlockHash: a1.BlockHash, safeBlockHash: Keccak.Zero);
         ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(repeated);

src/Nethermind/Nethermind.Merge.Plugin/BlockTreeExtensions.cs

@@ -8,9 +8,15 @@ namespace Nethermind.Merge.Plugin;
 
 public static class BlockTreeExtensions
 {
+    /// <summary>
+    /// Returns <c>true</c> when <paramref name="header"/> belongs to the canonical chain
+    /// and is at or behind the current head — i.e. an unprocessed FCU to it can be safely
+    /// answered <c>VALID</c> without reorging.
+    /// </summary>
+    /// <param name="blockTree">The block tree.</param>
+    /// <param name="header">Header to test for canonical-ancestor membership.</param>
+    /// <returns><c>true</c> if <paramref name="header"/> is on the main chain and its
+    /// number does not exceed the current head's number; otherwise <c>false</c>.</returns>
     public static bool IsOnMainChainBehindOrEqualHead(this IBlockTree blockTree, BlockHeader header) =>
         header.Number <= (blockTree.Head?.Number ?? 0) && blockTree.IsMainChain(header);
-
-    public static bool IsOnMainChainBehindHead(this IBlockTree blockTree, BlockHeader header) =>
-        header.Number < (blockTree.Head?.Number ?? 0) && blockTree.IsMainChain(header);
 }