Skip to content

Commit 1b311e7

Browse files
MarchhillsvlachakisMarekM25
authored
Add tests for skipping reorg only if ancestor of finalised block (#11439)
* engine api changes as per ethereum/execution-apis#770 ethereum/execution-apis#760 * fix comment * improve test * improve tests matching the specs * cleaner design * taiko fix * claude review * claude review * more fixes * conflicts fix * fix tests * fix taiko tests * remove unused import * update per 786 * fixes * fix tests * fixes * claude review again * fix tests * test fix * fixes * test fixes * minor fixes * Cleanup mess * remove test * fixing * cleanups * revert formatting changes and zero hash stuff * remove tests * fix lint * delete old tests * add tests * refactor tests * fix test * add comment --------- Co-authored-by: stavrosvl7 <stavrosvl7@gmail.com> Co-authored-by: Stavros Vlachakis <89769224+svlachakis@users.noreply.github.com> Co-authored-by: MarekM25 <marekm2504@gmail.com> Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com>
1 parent 0085994 commit 1b311e7

2 files changed

Lines changed: 87 additions & 0 deletions

File tree

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,6 +1553,87 @@ public async Task forkchoiceUpdated_safe_block_that_is_real_ancestor_of_current_
15531553
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
15541554
}
15551555

1556+
private async Task<IReadOnlyList<ExecutionPayload>> BuildChainWithLoweredFinalized(
1557+
MergeTestBlockchain chain, IEngineRpcModule rpc, int oldHead, int lastFinalized)
1558+
{
1559+
IReadOnlyList<ExecutionPayload> blocks = await ProduceBranchV1(rpc, chain, oldHead + 1, CreateParentBlockRequestOnHead(chain.BlockTree), setHead: true);
1560+
1561+
// Lower the finalized marker to blocks[lastFinalized] while keeping the head at blocks[oldHead].
1562+
Hash256 finalized = blocks[lastFinalized].BlockHash;
1563+
ForkchoiceStateV1 setFinalized = new(headBlockHash: blocks[oldHead].BlockHash, finalizedBlockHash: finalized, safeBlockHash: finalized);
1564+
(await rpc.engine_forkchoiceUpdatedV1(setFinalized)).Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
1565+
chain.BlockTree.Head!.Hash.Should().Be(blocks[oldHead].BlockHash);
1566+
return blocks;
1567+
}
1568+
1569+
[TestCase(-1, TestName = "Behind finalized")]
1570+
[TestCase(0, TestName = "Last finalized")]
1571+
[TestCase(1, TestName = "After finalized")]
1572+
public async Task forkchoiceUpdatedV1_processed_skips_reorg_only_when_head_is_ancestor_of_finalized(int offset)
1573+
{
1574+
using MergeTestBlockchain chain =
1575+
await CreateBlockchain(null, new MergeConfig() { TerminalTotalDifficulty = "0" });
1576+
IEngineRpcModule rpc = chain.EngineRpcModule;
1577+
1578+
const int oldHead = 4;
1579+
const int lastFinalized = 2;
1580+
IReadOnlyList<ExecutionPayload> blocks = await BuildChainWithLoweredFinalized(chain, rpc, oldHead, lastFinalized);
1581+
1582+
// Zero request-level finalized/safe so RejectIfInconsistent (which runs before the skip
1583+
// check) does not reject the offset < 0 case where finalized > head. The skip check still
1584+
// fires via the BlockTree's internal FinalizedHash set by the helper.
1585+
int newHead = lastFinalized + offset;
1586+
ForkchoiceStateV1 fcu = new(headBlockHash: blocks[newHead].BlockHash, finalizedBlockHash: Keccak.Zero, safeBlockHash: Keccak.Zero);
1587+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcu);
1588+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
1589+
1590+
if (offset < 0)
1591+
{
1592+
// Skip path: the FCU returns Valid without reorging; the head stays at blocks[oldHead].
1593+
chain.BlockTree.Head!.Hash.Should().Be(blocks[oldHead].BlockHash);
1594+
}
1595+
else
1596+
{
1597+
// No skip: the regular reorg path runs and the head is updated to blocks[newHead].
1598+
chain.BlockTree.Head!.Hash.Should().Be(blocks[newHead].BlockHash);
1599+
}
1600+
}
1601+
1602+
[TestCase(-1, TestName = "Behind finalized")]
1603+
[TestCase(0, TestName = "Last finalized")]
1604+
[TestCase(1, TestName = "After finalized")]
1605+
public async Task forkchoiceUpdatedV1_unprocessed_skips_reorg_only_when_head_is_ancestor_of_finalized(int offset)
1606+
{
1607+
using MergeTestBlockchain chain =
1608+
await CreateBlockchain(null, new MergeConfig() { TerminalTotalDifficulty = "0" });
1609+
IEngineRpcModule rpc = chain.EngineRpcModule;
1610+
1611+
const int oldHead = 4;
1612+
const int lastFinalized = 2;
1613+
IReadOnlyList<ExecutionPayload> blocks = await BuildChainWithLoweredFinalized(chain, rpc, oldHead, lastFinalized);
1614+
Hash256 finalized = blocks[lastFinalized].BlockHash;
1615+
1616+
int newHead = lastFinalized + offset;
1617+
// Reset the candidate's WasProcessed flag (the block stays on the main chain) so the
1618+
// FCU enters the unprocessed branch where the first skip check lives.
1619+
FlipCanonicalMarkerTo(chain, blocks[newHead]);
1620+
1621+
ForkchoiceStateV1 fcu = new(headBlockHash: blocks[newHead].BlockHash, finalizedBlockHash: finalized, safeBlockHash: finalized);
1622+
ResultWrapper<ForkchoiceUpdatedV1Result> result = await rpc.engine_forkchoiceUpdatedV1(fcu);
1623+
1624+
if (offset < 0)
1625+
{
1626+
// Skip path: the unprocessed branch returns Valid early without falling through
1627+
// to the sync logic.
1628+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Valid);
1629+
}
1630+
else
1631+
{
1632+
// No skip: the unprocessed branch falls through and returns Syncing.
1633+
result.Data.PayloadStatus.Status.Should().Be(PayloadStatus.Syncing);
1634+
}
1635+
}
1636+
15561637
[Test]
15571638
public async Task forkchoiceUpdated_accepts_lower_finalized_than_previous_but_rejects_safe_before_finalized()
15581639
{

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ public async Task<ResultWrapper<ForkchoiceUpdatedV1Result>> Handle(ForkchoiceSta
6464
?? StartBuildingPayload(newHeadHeader!, forkchoiceState, payloadAttributes);
6565
}
6666

67+
/// <summary>
68+
/// MAY-skip clause from
69+
/// <see href="https://github.com/ethereum/execution-apis/pull/786">execution-apis#786</see>:
70+
/// returns Valid when <paramref name="newHeadHeader"/> is a canonical ancestor of the latest
71+
/// known finalized block, so the FCU is answered without performing a reorg.
72+
/// </summary>
6773
protected virtual bool IsOnMainChainBehindFinalized(BlockHeader newHeadHeader, ForkchoiceStateV1 forkchoiceState,
6874
[NotNullWhen(true)] out ResultWrapper<ForkchoiceUpdatedV1Result>? result)
6975
{

0 commit comments

Comments
 (0)