diff --git a/src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs b/src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs index c8cd8e1b4af..6cd66531054 100644 --- a/src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs +++ b/src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs @@ -361,8 +361,8 @@ public void should_not_broadcast_tx_with_MaxFeePerGas_lower_than_70_percent_of_C // tx should be immediately broadcasted only if MaxFeePerGas is equal at least 70% of current base fee peer.Received(shouldBroadcast ? 1 : 0).SendNewTransaction(Arg.Any()); - // tx should always be added to persistent collection, without any fee restrictions - _broadcaster.GetSnapshot().Length.Should().Be(1); + // tx should only be added to persistent collection, if it is above the fee restriction + _broadcaster.GetSnapshot().Length.Should().Be(shouldBroadcast ? 1 : 0); } [Test] diff --git a/src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs b/src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs index e2555cd1bd1..b39e3a937bb 100644 --- a/src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs +++ b/src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs @@ -505,7 +505,7 @@ public void should_add_underpaid_txs_to_full_TxPool_only_if_local(bool isLocal) AcceptTxResult result = _txPool.SubmitTx(tx, txHandlingOptions); _txPool.GetPendingTransactionsCount().Should().Be(30); _txPool.GetOwnPendingTransactions().Length.Should().Be(isLocal ? 1 : 0); - result.ToString().Should().Contain(isLocal ? nameof(AcceptTxResult.FeeTooLowToCompete) : nameof(AcceptTxResult.FeeTooLow)); + result.ToString().Should().Contain(isLocal ? nameof(AcceptTxResult.Accepted) : nameof(AcceptTxResult.FeeTooLow)); } [TestCase(0)] @@ -1381,7 +1381,7 @@ public void should_increase_nonce_when_transaction_not_included_in_txPool_but_br .WithMaxFeePerGas(1) .WithMaxPriorityFeePerGas(1) .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; - _txPool.SubmitTx(cheapTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.FeeTooLowToCompete); + _txPool.SubmitTx(cheapTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.Accepted); _txPool.GetPendingTransactions().Should().NotContain(cheapTx); _txPool.GetOwnPendingTransactions().Should().Contain(cheapTx); peer.Received().SendNewTransaction(cheapTx); @@ -1393,7 +1393,7 @@ public void should_increase_nonce_when_transaction_not_included_in_txPool_but_br .WithMaxFeePerGas(1) .WithMaxPriorityFeePerGas(1) .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; - _txPool.SubmitTx(fourthTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.FeeTooLowToCompete); + _txPool.SubmitTx(fourthTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.Accepted); _txPool.GetPendingTransactions().Should().NotContain(fourthTx); _txPool.GetOwnPendingTransactions().Should().Contain(fourthTx); peer.Received().SendNewTransaction(fourthTx); @@ -1638,6 +1638,53 @@ public void Should_not_add_underpaid_tx_even_if_lower_nonces_are_expensive(int g result.Should().Be(expectedResult ? AcceptTxResult.Accepted : AcceptTxResult.FeeTooLowToCompete); } + [Test] + public void Should_correctly_add_tx_to_local_pool_when_underpaid([Values] TxType txType) + { + // Should only add non-blob transactions to local pool when underpaid + bool expectedResult = txType != TxType.Blob; + + // No need to check for deposit tx + if (txType == TxType.DepositTx) return; + + ISpecProvider specProvider = GetCancunSpecProvider(); + TxPoolConfig txPoolConfig = new TxPoolConfig { Size = 30, PersistentBlobStorageSize = 0 }; + _txPool = CreatePool(txPoolConfig, specProvider); + + Transaction[] transactions = GetTransactions(GetPeers(3), true, false); + + foreach (Address address in transactions.Select(t => t.SenderAddress).Distinct()) + { + EnsureSenderBalance(address, UInt256.MaxValue); + } + + // setup full tx pool + foreach (Transaction transaction in transactions) + { + transaction.GasPrice = 10.GWei(); + _txPool.SubmitTx(transaction, TxHandlingOptions.None); + } + + _txPool.GetPendingTransactionsCount().Should().Be(30); + + Transaction testTx = Build.A.Transaction + .WithNonce(0) + .WithType(txType) + .WithShardBlobTxTypeAndFieldsIfBlobTx() + .WithMaxFeePerGas(9.GWei()) + .WithMaxPriorityFeePerGas(9.GWei()) + .WithTo(TestItem.AddressB) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; + + EnsureSenderBalance(TestItem.PrivateKeyA.Address, UInt256.MaxValue); + + AcceptTxResult result = _txPool.SubmitTx(testTx, TxHandlingOptions.PersistentBroadcast); + result.Should().Be(expectedResult ? AcceptTxResult.Accepted : AcceptTxResult.FeeTooLowToCompete); + _txPool.GetOwnPendingTransactions().Length.Should().Be(expectedResult ? 1 : 0); + _txPool.GetPendingBlobTransactionsCount().Should().Be(0); + _txPool.GetPendingTransactions().Should().NotContain(testTx); + } + private IDictionary GetPeers(int limit = 100) { var peers = new Dictionary(); diff --git a/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs b/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs index 1505b21e9fb..de7eb80c9d0 100644 --- a/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs +++ b/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs @@ -97,31 +97,33 @@ internal class TxBroadcaster : IDisposable // only for testing reasons internal Transaction[] GetSnapshot() => _persistentTxs.GetSnapshot(); - public void Broadcast(Transaction tx, bool isPersistent) + public bool Broadcast(Transaction tx, bool isPersistent) { if (isPersistent) { - StartBroadcast(tx); - } - else - { - BroadcastOnce(tx); + return StartBroadcast(tx); } + + BroadcastOnce(tx); + + return true; } - private void StartBroadcast(Transaction tx) + private bool StartBroadcast(Transaction tx) { // broadcast local tx only if MaxFeePerGas is not lower than configurable percent of current base fee // (70% by default). Otherwise only add to persistent txs and broadcast when tx will be ready for inclusion - if (tx.MaxFeePerGas >= _baseFeeThreshold || tx.IsFree()) + + if (tx is not null + && (tx.MaxFeePerGas >= _baseFeeThreshold || tx.IsFree()) + && _persistentTxs.TryInsert(tx.Hash, tx.SupportsBlobs ? new LightTransaction(tx) : tx, out Transaction? removed) + && removed?.Hash != tx.Hash) { NotifyPeersAboutLocalTx(tx); + return true; } - if (tx.Hash is not null) - { - _persistentTxs.TryInsert(tx.Hash, tx.SupportsBlobs ? new LightTransaction(tx) : tx); - } + return false; } private void BroadcastOnce(Transaction tx) diff --git a/src/Nethermind/Nethermind.TxPool/TxPool.cs b/src/Nethermind/Nethermind.TxPool/TxPool.cs index 7841ea39993..538e95b1588 100644 --- a/src/Nethermind/Nethermind.TxPool/TxPool.cs +++ b/src/Nethermind/Nethermind.TxPool/TxPool.cs @@ -450,14 +450,17 @@ private AcceptTxResult AddCore(Transaction tx, TxFilteringState state, bool isPe if (tx.Hash == removed?.Hash) { // it means it was added and immediately evicted - pool was full of better txs - if (isPersistentBroadcast) + if (!isPersistentBroadcast || tx.SupportsBlobs || !_broadcaster.Broadcast(tx, true)) { // we are adding only to persistent broadcast - not good enough for standard pool, // but can be good enough for TxBroadcaster pool - for local txs only - _broadcaster.Broadcast(tx, isPersistentBroadcast); + Metrics.PendingTransactionsPassedFiltersButCannotCompeteOnFees++; + return AcceptTxResult.FeeTooLowToCompete; + } + else + { + return AcceptTxResult.Accepted; } - Metrics.PendingTransactionsPassedFiltersButCannotCompeteOnFees++; - return AcceptTxResult.FeeTooLowToCompete; } relevantPool.UpdateGroup(tx.SenderAddress!, state.SenderAccount, UpdateBucketWithAddedTransaction);