Remove the wait for GetPayload if payload is not ready to follow the …

…spec (#4180)
NethermindEth · Jun 22, 2022 · c0a89c1 · c0a89c1
1 parent aaa4bcd
commit c0a89c1
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 43 deletions.
diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.RelayBuilder.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.RelayBuilder.cs
@@ -78,11 +78,15 @@ public async Task forkchoiceUpdatedV1_should_communicate_with_boost_relay()
         boostRelay.When(b => b.SendPayload(Arg.Any<BoostExecutionPayloadV1>(), Arg.Any<CancellationToken>()))
             .Do(c => sentItem = c.Arg<BoostExecutionPayloadV1>());
 
+        ManualResetEvent wait = new(false);
+        chain.PayloadPreparationService.BlockImproved += (_, _) => wait.Set();
+
         string payloadId = rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(startingHead, Keccak.Zero, startingHead),
                 new PayloadAttributes { Timestamp = timestamp, SuggestedFeeRecipient = feeRecipient, PrevRandao = random }).Result.Data
             .PayloadId!;
 
-
+        await wait.WaitOneAsync(100, CancellationToken.None);
+
         ResultWrapper<ExecutionPayloadV1?> response = await rpc.engine_getPayloadV1(Bytes.FromHexString(payloadId));
 
         ExecutionPayloadV1 executionPayloadV1 = response.Data!;
@@ -126,11 +130,16 @@ public virtual async Task forkchoiceUpdatedV1_should_communicate_with_boost_rela
 
         IEngineRpcModule rpc = CreateEngineModule(chain);
         Keccak startingHead = chain.BlockTree.HeadHash;
+
+        ManualResetEvent wait = new(false);
+        chain.PayloadPreparationService.BlockImproved += (_, _) => wait.Set();
 
         string payloadId = rpc.engine_forkchoiceUpdatedV1(new ForkchoiceStateV1(startingHead, Keccak.Zero, startingHead),
                 payloadAttributes).Result.Data
             .PayloadId!;
 
+        await wait.WaitOneAsync(100, CancellationToken.None);
+
         ResultWrapper<ExecutionPayloadV1?> response = await rpc.engine_getPayloadV1(Bytes.FromHexString(payloadId));
 
         ExecutionPayloadV1 executionPayloadV1 = response.Data!;

diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs
@@ -994,39 +994,6 @@ public async Task getPayloadV1_picks_transactions_from_pool_v1()
             UInt256 totalValue = ((int)(count * value)).GWei();
             chain.StateReader.GetBalance(getPayloadResult.StateRoot, recipient).Should().Be(totalValue);
         }
-
-        [TestCase(PayloadPreparationService.GetPayloadWaitForFullBlockMillisecondsDelay / 10, ExpectedResult = 3)]
-        [TestCase(PayloadPreparationService.GetPayloadWaitForFullBlockMillisecondsDelay * 2, ExpectedResult = 0)]
-        public async Task<int> getPayloadV1_waits_for_block_production(int delay)
-        {
-            using MergeTestBlockchain chain = await CreateBlockChain();
-
-            DelayBlockImprovementContextFactory improvementContextFactory = new(chain.BlockProductionTrigger, TimeSpan.FromSeconds(10), delay);
-            chain.PayloadPreparationService = new PayloadPreparationService(
-                chain.PostMergeBlockProducer!,
-                improvementContextFactory,
-                chain.SealEngine, 
-                TimerFactory.Default, 
-                chain.LogManager, 
-                TimeSpan.FromSeconds(10));
-
-            IEngineRpcModule rpc = CreateEngineModule(chain);
-            Keccak startingHead = chain.BlockTree.HeadHash;
-            uint count = 3;
-            int value = 10;
-            Address recipient = TestItem.AddressF;
-            PrivateKey sender = TestItem.PrivateKeyB;
-            Transaction[] transactions = BuildTransactions(chain, startingHead, sender, recipient, count, value, out _, out _);
-            chain.AddTransactions(transactions);
-            string payloadId = rpc.engine_forkchoiceUpdatedV1(
-                new ForkchoiceStateV1(startingHead, Keccak.Zero, startingHead),
-                new PayloadAttributes { Timestamp = 100, PrevRandao = TestItem.KeccakA, SuggestedFeeRecipient = Address.Zero })
-                .Result.Data.PayloadId!;
-
-            ExecutionPayloadV1 getPayloadResult = (await rpc.engine_getPayloadV1(Bytes.FromHexString(payloadId))).Data!;
-
-            return getPayloadResult.Transactions.Length;
-        }
 
         [Test]
         public async Task getPayloadV1_return_correct_block_values_for_empty_block()

diff --git a/src/Nethermind/Nethermind.Merge.Plugin/BlockProduction/IPayloadPreparationService.cs b/src/Nethermind/Nethermind.Merge.Plugin/BlockProduction/IPayloadPreparationService.cs
@@ -26,7 +26,7 @@ public interface IPayloadPreparationService
     {
         string? StartPreparingPayload(BlockHeader parentHeader, PayloadAttributes payloadAttributes);
 
-        ValueTask<Block?> GetPayload(string payloadId);
+        Block? GetPayload(string payloadId);
 
         event EventHandler<BlockEventArgs>? BlockImproved;
     }

diff --git a/src/Nethermind/Nethermind.Merge.Plugin/BlockProduction/PayloadPreparationService.cs b/src/Nethermind/Nethermind.Merge.Plugin/BlockProduction/PayloadPreparationService.cs
@@ -48,7 +48,6 @@ public class PayloadPreparationService : IPayloadPreparationService
 
         // by default we will cleanup the old payload once per six slot. There is no need to fire it more often
         public const int SlotsPerOldPayloadCleanup = 6;
-        public const int GetPayloadWaitForFullBlockMillisecondsDelay = 500;
         private readonly TimeSpan _cleanupOldPayloadDelay;
 
         // first ExecutionPayloadV1 is empty (without txs), second one is the ideal one
@@ -163,17 +162,12 @@ private void CleanupOldPayloads(object? sender, EventArgs e)
             return t.Result;
         }
 
-        public async ValueTask<Block?> GetPayload(string payloadId)
+        public Block? GetPayload(string payloadId)
         {
             if (_payloadStorage.TryRemove(payloadId, out IBlockImprovementContext? blockContext))
             {
                 using (blockContext)
                 {
-                    if (!blockContext.ImprovementTask.IsCompleted)
-                    {
-                        await Task.WhenAny(blockContext.ImprovementTask, Task.Delay(GetPayloadWaitForFullBlockMillisecondsDelay));
-                    }
-
                     return blockContext.CurrentBestBlock;
                 }
             }

diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/GetPayloadV1Handler.cs b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/GetPayloadV1Handler.cs
@@ -53,7 +53,7 @@ public GetPayloadV1Handler(IPayloadPreparationService payloadPreparationService,
         public async Task<ResultWrapper<ExecutionPayloadV1?>> HandleAsync(byte[] payloadId)
         {
             string payloadStr = payloadId.ToHexString(true);
-            Block? block = await _payloadPreparationService.GetPayload(payloadStr);
+            Block? block = _payloadPreparationService.GetPayload(payloadStr);
 
             if (block == null)
             {