Fix/bad blocks v5 (#5411)

* Different approach

* fix for zero block producer

* added lock

* remove AuRaPostMergeBlockProducer

* tests update

* fix file encoding

* fix Aura tests

* add ForceProcessing -> Trace

* Removed unused usings

---------

Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>

src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs

@@ -557,7 +557,7 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin
             do
             {
                 iterations++;
-                if (!options.ContainsFlag(ProcessingOptions.ForceProcessing))
+                if (!options.ContainsFlag(ProcessingOptions.Trace))
                 {
                     blocksToBeAddedToMain.Add(toBeProcessed);
                 }

src/Nethermind/Nethermind.Consensus/Producers/BlockProducerBase.cs

@@ -48,11 +48,11 @@ public abstract class BlockProducerBase : IBlockProducer
         private readonly ITxSource _txSource;
         private readonly IBlockProductionTrigger _trigger;
         private bool _isRunning;
-        private readonly ManualResetEvent _producingBlockLock = new(true);
+        protected readonly SemaphoreSlim _producingBlockLock = new(1);
         private CancellationTokenSource? _producerCancellationToken;
 
         private DateTime _lastProducedBlockDateTime;
-        private const int BlockProductionTimeout = 1000;
+        protected const int BlockProductionTimeout = 2000;
         protected ILogger Logger { get; }
         protected readonly IBlocksConfig _blocksConfig;
 
@@ -122,7 +122,7 @@ public bool IsProducingBlocks(ulong? maxProducingInterval)
             token = tokenSource.Token;
 
             Block? block = null;
-            if (await _producingBlockLock.WaitOneAsync(BlockProductionTimeout, token))
+            if (await _producingBlockLock.WaitAsync(BlockProductionTimeout, token))
             {
                 try
                 {
@@ -140,7 +140,7 @@ public bool IsProducingBlocks(ulong? maxProducingInterval)
                 }
                 finally
                 {
-                    _producingBlockLock.Set();
+                    _producingBlockLock.Release();
                 }
             }
             else
@@ -229,6 +229,12 @@ public bool IsProducingBlocks(ulong? maxProducingInterval)
             return Task.FromResult((Block?)null);
         }
 
+        /// <summary>
+        /// Sets the state to produce block on
+        /// </summary>
+        /// <param name="parentStateRoot">Parent block state</param>
+        /// <returns>True if succeeded, false otherwise</returns>
+        /// <remarks>Should be called inside <see cref="_producingBlockLock"/> lock.</remarks>
         protected bool TrySetState(Keccak? parentStateRoot)
         {
             bool HasState(Keccak stateRoot)

src/Nethermind/Nethermind.Core.Test/Blockchain/TestBlockchain.cs

@@ -379,8 +379,12 @@ public void AddTransactions(params Transaction[] txs)
     public virtual void Dispose()
     {
         BlockProducer?.StopAsync();
-        CodeDb?.Dispose();
-        StateDb?.Dispose();
+        if (DbProvider != null)
+        {
+            CodeDb?.Dispose();
+            StateDb?.Dispose();
+        }
+
         _trieStoreWatcher?.Dispose();
         DbProvider?.Dispose();
     }

src/Nethermind/Nethermind.Merge.AuRa.Test/AuRaMergePluginTests.cs

@@ -18,6 +18,7 @@
 using Nethermind.Core.Specs;
 using Nethermind.Core.Timers;
 using Nethermind.Facade.Eth;
+using Nethermind.Logging;
 using Nethermind.Merge.Plugin;
 using Nethermind.Merge.Plugin.BlockProduction;
 using Nethermind.Merge.Plugin.Handlers;
@@ -33,7 +34,8 @@ public class AuRaMergeEngineModuleTests : EngineModuleTests
 {
     protected override MergeTestBlockchain CreateBaseBlockChain(
         IMergeConfig? mergeConfig = null,
-        IPayloadPreparationService? mockedPayloadService = null)
+        IPayloadPreparationService? mockedPayloadService = null,
+        ILogManager? logManager = null)
         => new MergeAuRaTestBlockchain(mergeConfig, mockedPayloadService);
 
     protected override Keccak ExpectedBlockHash => new("0x990d377b67dbffee4a60db6f189ae479ffb406e8abea16af55e0469b8524cf46");
@@ -127,7 +129,8 @@ protected override IBlockProducer CreateTestBlockProducer(TxPoolTxSource txPoolT
                 ),
                 TimerFactory.Default,
                 LogManager,
-                TimeSpan.FromSeconds(MergeConfig.SecondsPerSlot)
+                TimeSpan.FromSeconds(MergeConfig.SecondsPerSlot),
+                50000 // by default we want to avoid cleanup payload effects in testing
             );
 
             IAuRaStepCalculator auraStepCalculator = Substitute.For<IAuRaStepCalculator>();

src/Nethermind/Nethermind.Merge.AuRa/AuRaPostMergeBlockProducerFactory.cs

@@ -39,7 +39,7 @@ public override PostMergeBlockProducer Create(
             TargetAdjustedGasLimitCalculator targetAdjustedGasLimitCalculator =
                 new(_specProvider, _blocksConfig);
 
-            return new AuRaPostMergeBlockProducer(
+            return new PostMergeBlockProducer(
                 txSource ?? producerEnv.TxSource,
                 producerEnv.ChainProcessor,
                 producerEnv.BlockTree,

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

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using FluentAssertions;
 using Nethermind.Blockchain;
 using Nethermind.Blockchain.Receipts;
@@ -16,7 +15,6 @@
 using Nethermind.Db.Blooms;
 using Nethermind.Int256;
 using Nethermind.Logging;
-using Nethermind.Merge.Plugin.Handlers;
 using Nethermind.Merge.Plugin.Synchronization;
 using Nethermind.Serialization.Rlp;
 using Nethermind.Specs;

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

@@ -2,12 +2,9 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 
 using System;
-using System.Threading;
 using System.Threading.Tasks;
 using Nethermind.Consensus.Producers;
 using Nethermind.Core;
-using Nethermind.Core.Extensions;
-using Nethermind.Evm.Tracing;
 using Nethermind.Int256;
 using Nethermind.Merge.Plugin.BlockProduction;
 

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

@@ -25,7 +25,6 @@
 using Nethermind.Facade.Eth;
 using Nethermind.HealthChecks;
 using Nethermind.Int256;
-using Nethermind.JsonRpc;
 using Nethermind.Logging;
 using Nethermind.Merge.Plugin.BlockProduction;
 using Nethermind.Merge.Plugin.Handlers;
@@ -39,17 +38,17 @@ namespace Nethermind.Merge.Plugin.Test;
 
 public partial class EngineModuleTests
 {
-    protected virtual MergeTestBlockchain CreateBaseBlockChain(IMergeConfig? mergeConfig = null, IPayloadPreparationService? mockedPayloadService = null) =>
-        new(mergeConfig, mockedPayloadService);
+    protected virtual MergeTestBlockchain CreateBaseBlockChain(IMergeConfig? mergeConfig = null, IPayloadPreparationService? mockedPayloadService = null, ILogManager? logManager = null) =>
+        new(mergeConfig, mockedPayloadService, logManager);
 
     protected async Task<MergeTestBlockchain> CreateShanghaiBlockChain(IMergeConfig? mergeConfig = null, IPayloadPreparationService? mockedPayloadService = null)
         => await CreateBlockChain(mergeConfig, mockedPayloadService, Shanghai.Instance);
 
     protected async Task<MergeTestBlockchain> CreateBlockChain(IMergeConfig? mergeConfig = null, IPayloadPreparationService? mockedPayloadService = null, IReleaseSpec? releaseSpec = null)
         => await CreateBaseBlockChain(mergeConfig, mockedPayloadService).Build(new TestSingleReleaseSpecProvider(releaseSpec ?? London.Instance));
 
-    protected async Task<MergeTestBlockchain> CreateBlockChain(ISpecProvider specProvider)
-        => await CreateBaseBlockChain(null, null).Build(specProvider);
+    protected async Task<MergeTestBlockchain> CreateBlockChain(ISpecProvider specProvider, ILogManager? logManager = null)
+        => await CreateBaseBlockChain(null, null, logManager).Build(specProvider);
 
     private IEngineRpcModule CreateEngineModule(MergeTestBlockchain chain, ISyncConfig? syncConfig = null, TimeSpan? newPayloadTimeout = null, int newPayloadCacheSize = 50)
     {
@@ -133,11 +132,12 @@ public MergeTestBlockchain ThrottleBlockProcessor(int delayMs)
             return this;
         }
 
-        public MergeTestBlockchain(IMergeConfig? mergeConfig = null, IPayloadPreparationService? mockedPayloadPreparationService = null)
+        public MergeTestBlockchain(IMergeConfig? mergeConfig = null, IPayloadPreparationService? mockedPayloadPreparationService = null, ILogManager? logManager = null)
         {
             GenesisBlockBuilder = Core.Test.Builders.Build.A.Block.Genesis.Genesis.WithTimestamp(1UL);
             MergeConfig = mergeConfig ?? new MergeConfig() { TerminalTotalDifficulty = "0" };
             PayloadPreparationService = mockedPayloadPreparationService;
+            LogManager = logManager ?? LogManager;
         }
 
         protected override Task AddBlocksOnStart() => Task.CompletedTask;
@@ -187,7 +187,8 @@ protected override IBlockProducer CreateTestBlockProducer(TxPoolTxSource txPoolT
                 new BlockImprovementContextFactory(BlockProductionTrigger, TimeSpan.FromSeconds(MergeConfig.SecondsPerSlot)),
                 TimerFactory.Default,
                 LogManager,
-                TimeSpan.FromSeconds(MergeConfig.SecondsPerSlot));
+                TimeSpan.FromSeconds(MergeConfig.SecondsPerSlot),
+                50000); // by default we want to avoid cleanup payload effects in testing
             return new MergeBlockProducer(preMergeBlockProducer, postMergeBlockProducer, PoSSwitcher);
         }
 

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

@@ -3,12 +3,9 @@
 
 using System;
 using System.Collections.Generic;
-using System.Threading;
 using System.Threading.Tasks;
 using Nethermind.Consensus.Producers;
 using Nethermind.Core;
-using Nethermind.Core.Extensions;
-using Nethermind.Evm.Tracing;
 using Nethermind.Merge.Plugin.BlockProduction;
 
 namespace Nethermind.Merge.Plugin.Test;