Bad blocks - potential fix (#6212)

* Potential bad blocks fix * Revert "Potential bad blocks fix" This reverts commit 9be4bd3. * Potential fix * Integration tests with iInvalid block and fix * cleanup tests * notInForceProcessing
NethermindEth · Oct 23, 2023 · 645717b · 645717b
1 parent bb9b72c
commit 645717b
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 3 deletions.
diff --git a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs
@@ -581,7 +581,7 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin
         BlockHeader branchingPoint = null;
         List<Block> blocksToBeAddedToMain = new();
 
-        bool preMergeFinishBranchingCondition;
+        bool branchingCondition;
         bool suggestedBlockIsPostMerge = suggestedBlock.IsPostMerge;
 
         Block toBeProcessed = suggestedBlock;
@@ -665,12 +665,13 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin
             // otherwise some nodes would be missing
             bool notFoundTheBranchingPointYet = !_blockTree.IsMainChain(branchingPoint.Hash!);
             bool notReachedTheReorgBoundary = branchingPoint.Number > (_blockTree.Head?.Header.Number ?? 0);
-            preMergeFinishBranchingCondition = (notFoundTheBranchingPointYet || notReachedTheReorgBoundary);
+            bool notInForceProcessing = !options.ContainsFlag(ProcessingOptions.ForceProcessing);
+            branchingCondition = (notFoundTheBranchingPointYet || notReachedTheReorgBoundary) && notInForceProcessing;
             if (_logger.IsTrace)
                 _logger.Trace(
                     $" Current branching point: {branchingPoint.Number}, {branchingPoint.Hash} TD: {branchingPoint.TotalDifficulty} Processing conditions notFoundTheBranchingPointYet {notFoundTheBranchingPointYet}, notReachedTheReorgBoundary: {notReachedTheReorgBoundary}, suggestedBlockIsPostMerge {suggestedBlockIsPostMerge}");
 
-        } while (preMergeFinishBranchingCondition);
+        } while (branchingCondition);
 
         if (branchingPoint is not null && branchingPoint.Hash != _blockTree.Head?.Hash)
         {

diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.PayloadProduction.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.PayloadProduction.cs
@@ -27,6 +27,7 @@
 using Nethermind.Specs;
 using Nethermind.Specs.Forks;
 using Nethermind.State;
+using Nethermind.State.Repositories;
 using NSubstitute;
 using NUnit.Framework;
 
@@ -398,6 +399,80 @@ public async Task getPayloadV1_doesnt_wait_for_improvement_when_block_is_not_emp
         cancelledContext?.Disposed.Should().BeTrue();
     }
 
+    [Test]
+    public async Task Cannot_build_invalid_block_with_the_branch()
+    {
+        using SemaphoreSlim blockImprovementLock = new(0);
+        using MergeTestBlockchain chain = await CreateBlockchain(new TestSingleReleaseSpecProvider(London.Instance));
+        IEngineRpcModule rpc = CreateEngineModule(chain);
+
+        // creating chain with 30 blocks
+        await ProduceBranchV1(rpc, chain, 30, CreateParentBlockRequestOnHead(chain.BlockTree), true);
+        TimeSpan delay = TimeSpan.FromMilliseconds(10);
+        TimeSpan timePerSlot = 4 * delay;
+        StoringBlockImprovementContextFactory improvementContextFactory = new(new BlockImprovementContextFactory(chain.BlockProductionTrigger, TimeSpan.FromSeconds(chain.MergeConfig.SecondsPerSlot)));
+        chain.PayloadPreparationService = new PayloadPreparationService(
+            chain.PostMergeBlockProducer!,
+            improvementContextFactory,
+            TimerFactory.Default,
+            chain.LogManager,
+            timePerSlot,
+            improvementDelay: delay,
+            minTimeForProduction: delay);
+        chain.PayloadPreparationService!.BlockImproved += (_, _) => { blockImprovementLock.Release(1); };
+
+        Block block30 = chain.BlockTree.Head!;
+
+        // we added transactions
+        chain.AddTransactions(BuildTransactions(chain, block30.CalculateHash(), TestItem.PrivateKeyB, TestItem.AddressF, 3, 10, out _, out _));
+        PayloadAttributes payloadAttributesBlock31A = new PayloadAttributes
+        {
+            Timestamp = (ulong)DateTime.UtcNow.AddDays(3).Ticks,
+            PrevRandao = TestItem.KeccakA,
+            SuggestedFeeRecipient = Address.Zero
+        };
+        string? payloadIdBlock31A = rpc.engine_forkchoiceUpdatedV1(
+                new ForkchoiceStateV1(block30.GetOrCalculateHash(), Keccak.Zero, block30.GetOrCalculateHash()),
+                payloadAttributesBlock31A)
+            .Result.Data.PayloadId!;
+        await blockImprovementLock.WaitAsync(delay * 1000);
+
+        ExecutionPayload getPayloadResultBlock31A = (await rpc.engine_getPayloadV1(Bytes.FromHexString(payloadIdBlock31A))).Data!;
+        getPayloadResultBlock31A.Should().NotBeNull();
+        await rpc.engine_newPayloadV1(getPayloadResultBlock31A);
+
+        // current main chain block 30->31A, we start building payload 32A
+        await rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(getPayloadResultBlock31A.BlockHash, Keccak.Zero, getPayloadResultBlock31A.BlockHash));
+        Block block31A = chain.BlockTree.Head!;
+        PayloadAttributes payloadAttributes = new()
+        {
+            Timestamp = (ulong)DateTime.UtcNow.AddDays(4).Ticks,
+            PrevRandao = TestItem.KeccakA,
+            SuggestedFeeRecipient = Address.Zero
+        };
+
+        // we build one more block on the same level
+        Block block31B = chain.PostMergeBlockProducer!.PrepareEmptyBlock(block30.Header, payloadAttributes);
+        await rpc.engine_newPayloadV1(new ExecutionPayload(block31B));
+
+        // ...and we change the main chain, so main chain now is 30->31B, block improvement for block 32A is still in progress
+        string? payloadId = rpc.engine_forkchoiceUpdatedV1(
+                new ForkchoiceStateV1(block31A.GetOrCalculateHash(), Keccak.Zero, block31A.GetOrCalculateHash()),
+                payloadAttributes)
+            .Result.Data.PayloadId!;
+        ForkchoiceUpdatedV1Result result = rpc.engine_forkchoiceUpdatedV1(
+            new ForkchoiceStateV1(block31B.GetOrCalculateHash(), Keccak.Zero, block31B.GetOrCalculateHash())).Result.Data;
+
+        // we added same transactions, so if we build on incorrect state root, we will end up with invalid block
+        chain.AddTransactions(BuildTransactions(chain, block31A.GetOrCalculateHash(), TestItem.PrivateKeyC, TestItem.AddressF, 3, 10, out _, out _));
+        await blockImprovementLock.WaitAsync(delay * 1000);
+        ExecutionPayload getPayloadResult = (await rpc.engine_getPayloadV1(Bytes.FromHexString(payloadId))).Data!;
+        getPayloadResult.Should().NotBeNull();
+
+        ResultWrapper<PayloadStatusV1> finalResult = await rpc.engine_newPayloadV1(getPayloadResult);
+        finalResult.Data.Status.Should().Be(PayloadStatus.Valid);
+    }
+
     [Test, Repeat(100)]
     public async Task Cannot_produce_bad_blocks()
     {