Avoid bucket snapshot allocation in BalanceTooLowFilter (#11686)

* Avoid bucket snapshot allocation in BalanceTooLowFilter

* fix BucketBalanceState nonce type, hoist null checks, add VisitBucket tests

- BucketBalanceState fields are UInt256 to match Transaction.Nonce / AccountStruct.Nonce; the previous long fields did not compile.
- Move ArgumentNullException.ThrowIfNull calls in VisitBucket before lock acquisition.
- Add SortedPoolTests covering VisitBucket: missing group, full iteration, early stop, ref-state mutation, null-visitor guard.

* drop cumulativeCost local; parameterize VisitBucket tests

- Write the final tx-cost accumulation directly into BucketBalanceState.CumulativeCost instead of a separate local.
- Collapse VisitBucket iteration tests into a single [TestCaseSource] (missing group / full iteration / early stop) and drop the redundant ref-state-mutation test (already covered by accumulating into List<int>).

---------

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

Author: jschnelder

src/Nethermind/Nethermind.TxPool.Test/Collections/SortedPoolTests.cs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 
 using System;
+using System.Collections.Generic;
 using FluentAssertions;
 using Nethermind.Blockchain;
 using Nethermind.Consensus.Comparers;
@@ -104,5 +105,52 @@ public void should_remove_empty_buckets()
             _sortedPool.TryRemove(tx.Hash);
             _sortedPool.TryGetBucket(tx.SenderAddress, out _).Should().BeFalse();
         }
+
+        private static IEnumerable<TestCaseData> VisitBucketCases()
+        {
+            yield return new TestCaseData(Array.Empty<int>(), int.MaxValue, Array.Empty<int>())
+                .SetName("VisitBucket_missing_group_visits_nothing");
+            yield return new TestCaseData(new[] { 0, 1, 2, 3 }, int.MaxValue, new[] { 0, 1, 2, 3 })
+                .SetName("VisitBucket_iterates_all_items_in_ascending_nonce_order");
+            yield return new TestCaseData(new[] { 0, 1, 2, 3 }, 2, new[] { 0, 1, 2 })
+                .SetName("VisitBucket_stops_after_visitor_returns_false");
+        }
+
+        [TestCaseSource(nameof(VisitBucketCases))]
+        public void VisitBucket_visits_expected_nonces(int[] insertNonces, int stopAfterNonce, int[] expectedVisited)
+        {
+            InsertNonces(TestItem.AddressA, insertNonces);
+
+            (List<int> Visited, int StopAfter) state = (new List<int>(), stopAfterNonce);
+            _sortedPool.VisitBucket(TestItem.AddressA, ref state, static (Transaction tx, ref (List<int> Visited, int StopAfter) s) =>
+            {
+                s.Visited.Add((int)tx.Nonce);
+                return (int)tx.Nonce < s.StopAfter;
+            });
+
+            state.Visited.Should().Equal(expectedVisited);
+        }
+
+        [Test]
+        public void VisitBucket_throws_on_null_visitor()
+        {
+            int unused = 0;
+            Action act = () => _sortedPool.VisitBucket(TestItem.AddressA, ref unused, null!);
+
+            act.Should().Throw<ArgumentNullException>();
+        }
+
+        private void InsertNonces(Address sender, ReadOnlySpan<int> nonces)
+        {
+            foreach (int nonce in nonces)
+            {
+                Transaction tx = Build.A.Transaction
+                    .WithNonce((UInt256)nonce)
+                    .WithSenderAddress(sender)
+                    .TestObject;
+                tx.Hash = tx.CalculateHash();
+                _sortedPool.TryInsert(tx.Hash, tx);
+            }
+        }
     }
 }

src/Nethermind/Nethermind.TxPool/Collections/SortedPool.cs

@@ -23,6 +23,8 @@ public abstract partial class SortedPool<TKey, TValue, TGroupKey>
         where TKey : notnull
         where TGroupKey : notnull
     {
+        internal delegate bool BucketVisitor<TState>(TValue value, ref TState state);
+
         protected McsLock Lock { get; } = new();
 
         private readonly int _capacity;
@@ -548,6 +550,34 @@ public bool TryGetBucketsWorstValue(TGroupKey groupKey, out TValue? item)
             return false;
         }
 
+        /// <summary>
+        /// Iterates over bucket items under lock until visitor returns false.
+        /// </summary>
+        /// <remarks>
+        /// The visitor runs while the pool lock is held, so it must be short and must not call back into the pool.
+        /// Items are visited in the pool's group comparer order (ascending nonce for the tx pool).
+        /// </remarks>
+        internal void VisitBucket<TState>(TGroupKey groupKey, ref TState state, BucketVisitor<TState> visitor)
+        {
+            ArgumentNullException.ThrowIfNull(groupKey);
+            ArgumentNullException.ThrowIfNull(visitor);
+
+            using McsLock.Disposable lockRelease = Lock.Acquire();
+
+            if (!_buckets.TryGetValue(groupKey, out EnhancedSortedSet<TValue>? bucket))
+            {
+                return;
+            }
+
+            foreach (TValue value in bucket)
+            {
+                if (!visitor(value, ref state))
+                {
+                    break;
+                }
+            }
+        }
+
         public bool BucketAny(TGroupKey groupKey, Func<TValue, bool> predicate)
         {
             using McsLock.Disposable lockRelease = Lock.Acquire();

src/Nethermind/Nethermind.TxPool/Filters/BalanceTooLowFilter.cs

@@ -13,6 +13,14 @@ namespace Nethermind.TxPool.Filters
     /// </summary>
     internal sealed class BalanceTooLowFilter(TxDistinctSortedPool txs, TxDistinctSortedPool blobTxs, ILogger logger) : IIncomingTxFilter
     {
+        private struct BucketBalanceState(UInt256 accountNonce, UInt256 txNonce)
+        {
+            public readonly UInt256 AccountNonce = accountNonce;
+            public readonly UInt256 TxNonce = txNonce;
+            public UInt256 CumulativeCost = UInt256.Zero;
+            public bool Overflow = false;
+        }
+
         private readonly TxDistinctSortedPool _txs = txs;
         private readonly TxDistinctSortedPool _blobTxs = blobTxs;
         private readonly ILogger _logger = logger;
@@ -27,32 +35,27 @@ public AcceptTxResult Accept(Transaction tx, ref TxFilteringState state, TxHandl
             AccountStruct account = state.SenderAccount;
             UInt256 balance = account.Balance;
 
-            UInt256 cumulativeCost = UInt256.Zero;
-            bool overflow = false;
-            Transaction[] sameTypeTxs = tx.SupportsBlobs
-                ? _blobTxs.GetBucketSnapshot(tx.SenderAddress!) // it will create a snapshot of light txs (without actual blobs)
-                : _txs.GetBucketSnapshot(tx.SenderAddress!);
+            BucketBalanceState bucketBalanceState = new(account.Nonce, tx.Nonce);
+            TxDistinctSortedPool pool = tx.SupportsBlobs ? _blobTxs : _txs;
             // tx.SenderAddress! as unknownSenderFilter will run before this one
-
-            for (int i = 0; i < sameTypeTxs.Length; i++)
+            pool.VisitBucket(tx.SenderAddress!, ref bucketBalanceState, static (Transaction otherTx, ref BucketBalanceState bucketState) =>
             {
-                Transaction otherTx = sameTypeTxs[i];
-                if (otherTx.Nonce < account.Nonce)
+                if (otherTx.Nonce < bucketState.AccountNonce)
                 {
-                    continue;
+                    return true;
                 }
 
-                if (otherTx.Nonce < tx.Nonce)
+                if (otherTx.Nonce >= bucketState.TxNonce)
                 {
-                    overflow |= otherTx.IsOverflowWhenAddingTxCostToCumulative(cumulativeCost, out cumulativeCost);
+                    return false;
                 }
-                else
-                {
-                    break;
-                }
-            }
 
-            overflow |= tx.IsOverflowWhenAddingTxCostToCumulative(cumulativeCost, out cumulativeCost);
+                bucketState.Overflow |= otherTx.IsOverflowWhenAddingTxCostToCumulative(bucketState.CumulativeCost, out bucketState.CumulativeCost);
+                return true;
+            });
+
+            bool overflow = bucketBalanceState.Overflow;
+            overflow |= tx.IsOverflowWhenAddingTxCostToCumulative(bucketBalanceState.CumulativeCost, out bucketBalanceState.CumulativeCost);
 
             if (overflow)
             {
@@ -61,7 +64,7 @@ public AcceptTxResult Accept(Transaction tx, ref TxFilteringState state, TxHandl
                 return AcceptTxResult.Int256Overflow;
             }
 
-            if (balance < cumulativeCost)
+            if (balance < bucketBalanceState.CumulativeCost)
             {
                 Metrics.PendingTransactionsTooLowBalance++;
 
@@ -73,7 +76,7 @@ public AcceptTxResult Accept(Transaction tx, ref TxFilteringState state, TxHandl
                 bool isNotLocal = (handlingOptions & TxHandlingOptions.PersistentBroadcast) == 0;
                 return isNotLocal ?
                     AcceptTxResult.InsufficientFunds :
-                    AcceptTxResult.InsufficientFunds.WithMessage($"Account balance: {balance}, cumulative cost: {cumulativeCost}");
+                    AcceptTxResult.InsufficientFunds.WithMessage($"Account balance: {balance}, cumulative cost: {bucketBalanceState.CumulativeCost}");
             }
 
             return AcceptTxResult.Accepted;