Fix flaky test in SimpleRetryCacheTests by pooling instead of delay

src/Nethermind/Nethermind.Core.Test/NSubstituteExtensions.cs

@@ -0,0 +1,36 @@
+// SPDX-FileCopyrightText: 2025 Demerzel Solutions Limited
+// SPDX-License-Identifier: LGPL-3.0-only
+
+using System;
+using NSubstitute;
+
+namespace Nethermind.Core.Test;
+
+public static class NSubstituteExtensions
+{
+    public static bool ReceivedBool<T>(this T substitute, Action<T> action, int requiredNumberOfCalls = 1) where T : class
+    {
+        try
+        {
+            action(substitute.Received(requiredNumberOfCalls));
+            return true;
+        }
+        catch
+        {
+            return false;
+        }
+    }
+
+    public static bool DidNotReceiveBool<T>(this T substitute, Action<T> action) where T : class
+    {
+        try
+        {
+            action(substitute.DidNotReceive());
+            return true;
+        }
+        catch
+        {
+            return false;
+        }
+    }
+}

src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/V68/Eth68ProtocolHandler.cs

@@ -253,6 +253,7 @@ protected override ValueTask HandleSlow((IOwnedReadOnlyList<Transaction> txs, in
         {
             if (!ValidateSizeAndType(request.txs[i]))
             {
+                request.txs.Dispose();
                 throw new SubprotocolException("invalid pooled tx type or size");
             }
         }

src/Nethermind/Nethermind.TxPool.Test/SimpleRetryCacheTests.cs → src/Nethermind/Nethermind.TxPool.Test/RetryCacheTests.cs

@@ -8,11 +8,12 @@
 using System;
 using System.Threading;
 using System.Threading.Tasks;
+using Nethermind.Core.Test;
 
 namespace Nethermind.TxPool.Test;
 
 [TestFixture]
-public class SimpleRetryCacheTests
+public class RetryCacheTests
 {
     public readonly struct ResourceRequestMessage : INew<int, ResourceRequestMessage>
     {
@@ -22,17 +23,16 @@ public class SimpleRetryCacheTests
 
     public interface ITestHandler : IMessageHandler<ResourceRequestMessage>;
 
-
     private CancellationTokenSource _cancellationTokenSource;
     private RetryCache<ResourceRequestMessage, int> cache;
 
-    private readonly int Timeout = 3000;
+    private readonly int Timeout = 1000;
 
     [SetUp]
     public void Setup()
     {
         _cancellationTokenSource = new CancellationTokenSource();
-        cache = new(TestLogManager.Instance, token: _cancellationTokenSource.Token);
+        cache = new(TestLogManager.Instance, timeoutMs: Timeout / 2, token: _cancellationTokenSource.Token);
     }
 
     [TearDown]
@@ -53,22 +53,20 @@ public void Announced_SameResourceDifferentNode_ReturnsEnqueued()
     }
 
     [Test]
-    public async Task Announced_AfterTimeout_ExecutesRetryRequests()
+    public void Announced_AfterTimeout_ExecutesRetryRequests()
     {
         ITestHandler request1 = Substitute.For<ITestHandler>();
         ITestHandler request2 = Substitute.For<ITestHandler>();
 
         cache.Announced(1, request1);
         cache.Announced(1, request2);
 
-        await Task.Delay(Timeout, _cancellationTokenSource.Token);
-
-        request1.DidNotReceive().HandleMessage(Arg.Any<ResourceRequestMessage>());
-        request2.Received(1).HandleMessage(Arg.Any<ResourceRequestMessage>());
+        Assert.That(() => request1.DidNotReceiveBool(r => r.HandleMessage(Arg.Any<ResourceRequestMessage>())), Is.True.After(Timeout, 100));
+        Assert.That(() => request2.ReceivedBool(r => r.HandleMessage(Arg.Any<ResourceRequestMessage>())), Is.True.After(Timeout, 100));
     }
 
     [Test]
-    public async Task Announced_MultipleResources_ExecutesAllRetryRequestsExceptInititalOne()
+    public void Announced_MultipleResources_ExecutesAllRetryRequestsExceptInititalOne()
     {
         ITestHandler request1 = Substitute.For<ITestHandler>();
         ITestHandler request2 = Substitute.For<ITestHandler>();
@@ -80,12 +78,10 @@ public async Task Announced_MultipleResources_ExecutesAllRetryRequestsExceptInit
         cache.Announced(2, request3);
         cache.Announced(2, request4);
 
-        await Task.Delay(Timeout, _cancellationTokenSource.Token);
-
-        request1.Received(0).HandleMessage(Arg.Any<ResourceRequestMessage>());
-        request2.Received(1).HandleMessage(Arg.Any<ResourceRequestMessage>());
-        request3.Received(0).HandleMessage(Arg.Any<ResourceRequestMessage>());
-        request4.Received(1).HandleMessage(Arg.Any<ResourceRequestMessage>());
+        Assert.That(() => request1.DidNotReceiveBool(r => r.HandleMessage(Arg.Any<ResourceRequestMessage>())), Is.True.After(Timeout, 100));
+        Assert.That(() => request2.ReceivedBool(r => r.HandleMessage(Arg.Any<ResourceRequestMessage>())), Is.True.After(Timeout, 100));
+        Assert.That(() => request3.DidNotReceiveBool(r => r.HandleMessage(Arg.Any<ResourceRequestMessage>())), Is.True.After(Timeout, 100));
+        Assert.That(() => request4.ReceivedBool(r => r.HandleMessage(Arg.Any<ResourceRequestMessage>())), Is.True.After(Timeout, 100));
     }
 
     [Test]
@@ -99,21 +95,19 @@ public void Received_RemovesResourceFromRetryQueue()
     }
 
     [Test]
-    public async Task Received_BeforeTimeout_PreventsRetryExecution()
+    public void Received_BeforeTimeout_PreventsRetryExecution()
     {
         ITestHandler request = Substitute.For<ITestHandler>();
 
         cache.Announced(1, request);
         cache.Announced(1, request);
         cache.Received(1);
 
-        await Task.Delay(Timeout, _cancellationTokenSource.Token);
-
-        request.DidNotReceive().HandleMessage(ResourceRequestMessage.New(1));
+        Assert.That(() => request.DidNotReceiveBool(r => r.HandleMessage(ResourceRequestMessage.New(1))), Is.True.After(Timeout, 100));
     }
 
     [Test]
-    public async Task RetryExecution_HandlesExceptions()
+    public void RetryExecution_HandlesExceptions()
     {
         ITestHandler faultyRequest = Substitute.For<ITestHandler>();
         ITestHandler normalRequest = Substitute.For<ITestHandler>();
@@ -124,21 +118,17 @@ public async Task RetryExecution_HandlesExceptions()
         cache.Announced(1, faultyRequest);
         cache.Announced(1, normalRequest);
 
-        await Task.Delay(Timeout, _cancellationTokenSource.Token);
-
-        normalRequest.Received(1).HandleMessage(Arg.Any<ResourceRequestMessage>());
+        Assert.That(() => normalRequest.ReceivedBool(r => r.HandleMessage(ResourceRequestMessage.New(1))), Is.True.After(Timeout, 100));
     }
 
     [Test]
-    public async Task CancellationToken_StopsProcessing()
+    public void CancellationToken_StopsProcessing()
     {
         ITestHandler request = Substitute.For<ITestHandler>();
 
         cache.Announced(1, request);
         _cancellationTokenSource.Cancel();
-        await Task.Delay(Timeout);
-
-        request.DidNotReceive().HandleMessage(Arg.Any<ResourceRequestMessage>());
+        Assert.That(() => request.DidNotReceiveBool(r => r.HandleMessage(Arg.Any<ResourceRequestMessage>())), Is.True.After(Timeout, 100));
     }
 
     [Test]
@@ -148,8 +138,7 @@ public async Task Announced_AfterRetryInProgress_ReturnsNew()
 
         await Task.Delay(Timeout, _cancellationTokenSource.Token);
 
-        AnnounceResult result = cache.Announced(1, Substitute.For<ITestHandler>());
-        Assert.That(result, Is.EqualTo(AnnounceResult.New));
+        Assert.That(() => cache.Announced(1, Substitute.For<ITestHandler>()), Is.EqualTo(AnnounceResult.New).After(Timeout, 100));
     }
 
     [Test]