Skip to content

Commit fe6f9b0

Browse files
svlachakissvlachakis
committed
engine api changes as per ethereum/execution-apis#770
ethereum/execution-apis#760
1 parent a2168c7 commit fe6f9b0

3 files changed

Lines changed: 179 additions & 5 deletions

File tree

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

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,170 @@ public async Task forkchoiceUpdatedV1_should_update_safe_block_hash()
563563
}
564564

565565

566+
[Test]
567+
public async Task forkchoiceUpdatedV1_WhenHeadIsAncestorWithinDepthLimit_ReorgsToAncestor()
568+
{
569+
// FCU to an ancestor within 32 blocks must execute the reorg, not skip it.
570+
// Builds a chain: genesis → b1 → b2 → b3 (head at H=3), then sends FCU(b1).
571+
// b1 is 2 blocks behind head — within the 32-block limit — so head must move back to b1.
572+
using MergeTestBlockchain chain = await CreateBlockchain();
573+
IEngineRpcModule rpc = chain.EngineRpcModule;
574+
575+
IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 3, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
576+
Hash256 b1Hash = branch[0].BlockHash;
577+
Hash256 b3Hash = branch[2].BlockHash;
578+
579+
chain.BlockTree.HeadHash.Should().Be(b3Hash, "precondition: head is at H=3");
580+
581+
ForkchoiceStateV1 fcuToAncestor = new(b1Hash, Keccak.Zero, Keccak.Zero);
582+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuToAncestor);
583+
584+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
585+
chain.BlockTree.HeadHash.Should().Be(b1Hash, "head must reorg back to b1 — within 32-block ancestor depth limit");
586+
}
587+
588+
[Test]
589+
public async Task forkchoiceUpdatedV1_WhenHeadIsAncestorBeyondDepthLimit_SkipsUpdate()
590+
{
591+
// FCU to an ancestor MORE than 32 blocks behind head must be skipped (treated as already canonical).
592+
// Builds a chain of 34 blocks, then sends FCU to block at H=1 (33 blocks behind head at H=34).
593+
using MergeTestBlockchain chain = await CreateBlockchain();
594+
IEngineRpcModule rpc = chain.EngineRpcModule;
595+
596+
IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 34, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
597+
Hash256 b1Hash = branch[0].BlockHash;
598+
Hash256 b34Hash = branch[33].BlockHash;
599+
600+
chain.BlockTree.HeadHash.Should().Be(b34Hash, "precondition: head is at H=34");
601+
602+
ForkchoiceStateV1 fcuToDeepAncestor = new(b1Hash, Keccak.Zero, Keccak.Zero);
603+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuToDeepAncestor);
604+
605+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
606+
chain.BlockTree.HeadHash.Should().Be(b34Hash, "head must stay at H=34 — ancestor is beyond the 32-block depth limit");
607+
}
608+
609+
[Test]
610+
public async Task forkchoiceUpdatedV1_WhenHeadIsAncestorAtExactDepthLimit_ReorgsToAncestor()
611+
{
612+
// FCU to an ancestor exactly 32 blocks behind head must execute the reorg (boundary condition).
613+
// Builds 33 blocks. b1 is exactly 32 blocks behind H=33 — must reorg.
614+
using MergeTestBlockchain chain = await CreateBlockchain();
615+
IEngineRpcModule rpc = chain.EngineRpcModule;
616+
617+
IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 33, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
618+
Hash256 b1Hash = branch[0].BlockHash;
619+
Hash256 b33Hash = branch[32].BlockHash;
620+
621+
chain.BlockTree.HeadHash.Should().Be(b33Hash, "precondition: head is at H=33");
622+
623+
ForkchoiceStateV1 fcuToAncestorAtLimit = new(b1Hash, Keccak.Zero, Keccak.Zero);
624+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuToAncestorAtLimit);
625+
626+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
627+
chain.BlockTree.HeadHash.Should().Be(b1Hash, "head must reorg to b1 — exactly at the 32-block depth limit");
628+
}
629+
630+
[Test]
631+
public async Task forkchoiceUpdatedV1_WhenHeadIsOnDifferentBranch_ReorgsRegardlessOfDepth()
632+
{
633+
// Spec: the 32-block depth limit only applies to ancestors of the canonical chain.
634+
// A block on a different (non-canonical) branch must always trigger a reorg — no depth limit.
635+
// Builds a canonical chain of 34 blocks, then a side branch of 1 block off genesis.
636+
// The side block is 34 levels "behind" but is NOT a canonical ancestor — it's on a fork.
637+
using MergeTestBlockchain chain = await CreateBlockchain();
638+
IEngineRpcModule rpc = chain.EngineRpcModule;
639+
640+
// Build canonical chain: genesis → b1 → b2 → ... → b34 (head at H=34)
641+
IReadOnlyList<ExecutionPayload> canonical = await ProduceBranchV1(rpc, chain, 34, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
642+
Hash256 b34Hash = canonical[33].BlockHash;
643+
chain.BlockTree.HeadHash.Should().Be(b34Hash, "precondition: canonical head is at H=34");
644+
645+
// Build a side block off genesis (H=1, different branch)
646+
BlockHeader genesis = chain.BlockTree.Genesis!;
647+
ExecutionPayload genesisAsParent = new ExecutionPayload
648+
{
649+
BlockNumber = genesis.Number,
650+
BlockHash = genesis.Hash!,
651+
StateRoot = genesis.StateRoot!,
652+
ReceiptsRoot = genesis.ReceiptsRoot!,
653+
GasLimit = genesis.GasLimit,
654+
Timestamp = genesis.Timestamp,
655+
BaseFeePerGas = genesis.BaseFeePerGas,
656+
};
657+
ExecutionPayload sideBlock = CreateBlockRequest(chain, genesisAsParent, TestItem.AddressA);
658+
await rpc.engine_newPayloadV1(sideBlock);
659+
Hash256 sideHash = sideBlock.BlockHash;
660+
chain.BlockTree.IsMainChain(chain.BlockTree.FindHeader(sideHash, BlockTreeLookupOptions.None)!).Should().BeFalse("precondition: side block is not on canonical chain");
661+
662+
// FCU to the side block — it's on a different branch, so it must reorg regardless of depth
663+
ForkchoiceStateV1 fcuToSide = new(sideHash, Keccak.Zero, Keccak.Zero);
664+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuToSide);
665+
666+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
667+
chain.BlockTree.HeadHash.Should().Be(sideHash, "different-branch FCU must always reorg — depth limit does not apply");
668+
}
669+
670+
[Test]
671+
public async Task forkchoiceUpdatedV1_WhenZeroFinalizedAndSafeHash_ReturnsValidWithoutError()
672+
{
673+
// Spec: zero safeBlockHash and finalizedBlockHash mean "unknown" — must not return -38002.
674+
// Models CL checkpoint-syncing from a non-finalized state where safe/finalized are unknown.
675+
using MergeTestBlockchain chain = await CreateBlockchain();
676+
IEngineRpcModule rpc = chain.EngineRpcModule;
677+
678+
IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 3, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
679+
Hash256 headHash = branch[2].BlockHash;
680+
681+
ForkchoiceStateV1 fcuWithUnknownFinality = new(headHash, Keccak.Zero, Keccak.Zero);
682+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuWithUnknownFinality);
683+
684+
result.ErrorCode.Should().Be(0, "zero safe/finalized hashes must not produce an error");
685+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
686+
chain.BlockTree.HeadHash.Should().Be(headHash);
687+
}
688+
689+
[Test]
690+
public async Task forkchoiceUpdatedV1_WhenZeroFinalizedHash_PreservesKnownFinalizedHash()
691+
{
692+
// Spec PR #760: when finalizedBlockHash is zero, client MUST use the latest known finalized hash — not overwrite it with zero.
693+
using MergeTestBlockchain chain = await CreateBlockchain();
694+
IEngineRpcModule rpc = chain.EngineRpcModule;
695+
696+
IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 2, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
697+
Hash256 b1Hash = branch[0].BlockHash;
698+
Hash256 b2Hash = branch[1].BlockHash;
699+
700+
// First FCU: finalize b1
701+
ForkchoiceStateV1 fcuWithFinalized = new(b2Hash, b1Hash, b1Hash);
702+
await rpc.engine_forkchoiceUpdatedV1(fcuWithFinalized);
703+
chain.BlockTree.FinalizedHash.Should().Be(b1Hash, "precondition: b1 is finalized after first FCU");
704+
705+
// Second FCU: zero finalizedBlockHash — must preserve b1 as finalized
706+
ForkchoiceStateV1 fcuWithZeroFinalized = new(b2Hash, Keccak.Zero, Keccak.Zero);
707+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuWithZeroFinalized);
708+
709+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
710+
chain.BlockTree.FinalizedHash.Should().Be(b1Hash, "zero finalizedBlockHash must preserve the previously known finalized hash");
711+
}
712+
713+
[Test]
714+
public async Task forkchoiceUpdatedV1_WhenNonZeroUnknownFinalizedHash_ReturnsInvalidForkchoiceState()
715+
{
716+
// Spec: -38002 must only fire for non-zero hashes that are unknown, not for zero hashes.
717+
using MergeTestBlockchain chain = await CreateBlockchain();
718+
IEngineRpcModule rpc = chain.EngineRpcModule;
719+
720+
IReadOnlyList<ExecutionPayload> branch = await ProduceBranchV1(rpc, chain, 1, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
721+
Hash256 headHash = branch[0].BlockHash;
722+
723+
ForkchoiceStateV1 fcuWithUnknownFinalized = new(headHash, TestItem.KeccakA, Keccak.Zero);
724+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcuWithUnknownFinalized);
725+
726+
result.ErrorCode.Should().Be(MergeErrorCodes.InvalidForkchoiceState,
727+
"non-zero unknown finalizedBlockHash must return -38002");
728+
}
729+
566730
[Test]
567731
public async Task forkchoiceUpdatedV1_should_work_with_zero_keccak_as_safe_block()
568732
{

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ namespace Nethermind.Merge.Plugin;
88

99
public static class BlockTreeExtensions
1010
{
11+
public const int AncestorReorgDepthLimit = 32;
12+
1113
public static bool IsOnMainChainBehindOrEqualHead(this IBlockTree blockTree, Block block) =>
1214
block.Number <= (blockTree.Head?.Number ?? 0) && blockTree.IsMainChain(block.Header);
1315

1416
public static bool IsOnMainChainBehindHead(this IBlockTree blockTree, Block block) =>
15-
block.Number < (blockTree.Head?.Number ?? 0) && blockTree.IsMainChain(block.Header);
17+
(blockTree.Head?.Number ?? 0) - block.Number > AncestorReorgDepthLimit && blockTree.IsMainChain(block.Header);
1618
}

‎src/Nethermind/Nethermind.Merge.Plugin/Handlers/ForkchoiceUpdatedHandler.cs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,10 @@ protected virtual bool IsOnMainChainBehindHead(Block newHeadBlock, ForkchoiceSta
167167

168168
if (!blockInfo.WasProcessed)
169169
{
170-
if (!IsOnMainChainBehindHead(newHeadBlock, forkchoiceState, out ResultWrapper<ForkchoiceUpdatedV1Result>? errorResult))
170+
if (_blockTree.IsOnMainChainBehindOrEqualHead(newHeadBlock))
171171
{
172-
return errorResult;
172+
if (_logger.IsInfo) _logger.Info($"Valid. ForkChoiceUpdated ignored - already in canonical chain.");
173+
return ForkchoiceUpdatedV1Result.Valid(null, forkchoiceState.HeadBlockHash);
173174
}
174175

175176
BlockHeader? blockParent = _blockTree.FindHeader(newHeadBlock.ParentHash!, blockNumber: newHeadBlock.Number - 1);
@@ -284,7 +285,9 @@ protected virtual bool IsOnMainChainBehindHead(Block newHeadBlock, ForkchoiceSta
284285
if (_logger.IsInfo) _logger.Info($"Synced Chain Head to {newHeadBlock.ToString(Block.Format.Short)}");
285286
}
286287

287-
_blockTree.ForkChoiceUpdated(forkchoiceState.FinalizedBlockHash, forkchoiceState.SafeBlockHash);
288+
_blockTree.ForkChoiceUpdated(
289+
ResolveZeroHash(forkchoiceState.FinalizedBlockHash, _blockTree.FinalizedHash),
290+
ResolveZeroHash(forkchoiceState.SafeBlockHash, _blockTree.SafeHash));
288291
return null;
289292
}
290293

@@ -332,7 +335,9 @@ private ResultWrapper<ForkchoiceUpdatedV1Result> StartBuildingPayload(Block newH
332335
payloadId = _payloadPreparationService.StartPreparingPayload(newHeadBlock.Header, payloadAttributes);
333336
}
334337

335-
_blockTree.ForkChoiceUpdated(forkchoiceState.FinalizedBlockHash, forkchoiceState.SafeBlockHash);
338+
_blockTree.ForkChoiceUpdated(
339+
ResolveZeroHash(forkchoiceState.FinalizedBlockHash, _blockTree.FinalizedHash),
340+
ResolveZeroHash(forkchoiceState.SafeBlockHash, _blockTree.SafeHash));
336341
return ForkchoiceUpdatedV1Result.Valid(isPayloadSimulated ? null : payloadId, forkchoiceState.HeadBlockHash);
337342
}
338343

@@ -362,6 +367,9 @@ private void StartNewBeaconHeaderSync(ForkchoiceStateV1 forkchoiceState, BlockHe
362367

363368
private bool IsInconsistent(Hash256 blockHash) => blockHash != Keccak.Zero && !_blockTree.IsMainChain(blockHash);
364369

370+
private static Hash256 ResolveZeroHash(Hash256 hash, Hash256? knownHash) =>
371+
hash == Keccak.Zero ? knownHash ?? Keccak.Zero : hash;
372+
365373
private Block? GetBlock(Hash256 headBlockHash)
366374
{
367375
Block? block = _blockTree.FindBlock(headBlockHash, BlockTreeLookupOptions.DoNotCreateLevelIfMissing);

0 commit comments

Comments
 (0)