diff --git a/src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs b/src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs index 67e31270f0a..9a4d08d1612 100644 --- a/src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs +++ b/src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs @@ -65,6 +65,7 @@ public interface IApiWithBlockchain : IApiWithStores, IBlockchainBridgeFactory ITransactionProcessor? TransactionProcessor { get; set; } ITrieStore? TrieStore { get; set; } ITxSender? TxSender { get; set; } + INonceManager? NonceManager { get; set; } ITxPool? TxPool { get; set; } ITxPoolInfoProvider? TxPoolInfoProvider { get; set; } IWitnessCollector? WitnessCollector { get; set; } diff --git a/src/Nethermind/Nethermind.Api/NethermindApi.cs b/src/Nethermind/Nethermind.Api/NethermindApi.cs index f229c5d1c9d..77140420124 100644 --- a/src/Nethermind/Nethermind.Api/NethermindApi.cs +++ b/src/Nethermind/Nethermind.Api/NethermindApi.cs @@ -197,6 +197,7 @@ public ISealEngine SealEngine public ITrieStore? TrieStore { get; set; } public IReadOnlyTrieStore? ReadOnlyTrieStore { get; set; } public ITxSender? TxSender { get; set; } + public INonceManager? NonceManager { get; set; } public ITxPool? TxPool { get; set; } public ITxPoolInfoProvider? TxPoolInfoProvider { get; set; } public IHealthHintService? HealthHintService { get; set; } diff --git a/src/Nethermind/Nethermind.Consensus.AuRa/InitializationSteps/InitializeBlockchainAuRa.cs b/src/Nethermind/Nethermind.Consensus.AuRa/InitializationSteps/InitializeBlockchainAuRa.cs index 6ddc0dc1ea7..fa81d144ba7 100644 --- a/src/Nethermind/Nethermind.Consensus.AuRa/InitializationSteps/InitializeBlockchainAuRa.cs +++ b/src/Nethermind/Nethermind.Consensus.AuRa/InitializationSteps/InitializeBlockchainAuRa.cs @@ -116,6 +116,7 @@ private IAuRaValidator CreateAuRaValidator(IBlockProcessor processor, IReadOnlyT if (_api.BlockTree is null) throw new StepDependencyException(nameof(_api.BlockTree)); if (_api.EngineSigner is null) throw new StepDependencyException(nameof(_api.EngineSigner)); if (_api.SpecProvider is null) throw new StepDependencyException(nameof(_api.SpecProvider)); + if (_api.NonceManager is null) throw new StepDependencyException(nameof(_api.NonceManager)); var chainSpecAuRa = _api.ChainSpec.AuRa; @@ -136,8 +137,7 @@ private IAuRaValidator CreateAuRaValidator(IBlockProcessor processor, IReadOnlyT _api.ReceiptStorage, _api.ValidatorStore, _api.FinalizationManager, - new TxPoolSender(_api.TxPool, - new NonceReservingTxSealer(_api.EngineSigner, _api.Timestamper, _api.TxPool, _api.EthereumEcdsa)), + new TxPoolSender(_api.TxPool, new TxSealer(_api.EngineSigner, _api.Timestamper), _api.NonceManager, _api.EthereumEcdsa), _api.TxPool, NethermindApi.Config(), _api.LogManager, diff --git a/src/Nethermind/Nethermind.Core.Test/Blockchain/TestBlockchain.cs b/src/Nethermind/Nethermind.Core.Test/Blockchain/TestBlockchain.cs index e8e5d5fb974..8d10db08bd0 100644 --- a/src/Nethermind/Nethermind.Core.Test/Blockchain/TestBlockchain.cs +++ b/src/Nethermind/Nethermind.Core.Test/Blockchain/TestBlockchain.cs @@ -43,6 +43,7 @@ public class TestBlockchain : IDisposable public const int DefaultTimeout = 4000; public IStateReader StateReader { get; private set; } = null!; public IEthereumEcdsa EthereumEcdsa { get; private set; } = null!; + public INonceManager NonceManager { get; private set; } = null!; public TransactionProcessor TxProcessor { get; set; } = null!; public IStorageProvider Storage { get; set; } = null!; public IReceiptStorage ReceiptStorage { get; set; } = null!; @@ -142,6 +143,11 @@ protected virtual async Task Build(ISpecProvider? specProvider = TransactionComparerProvider = new TransactionComparerProvider(SpecProvider, BlockTree); TxPool = CreateTxPool(); + IChainHeadInfoProvider chainHeadInfoProvider = + new ChainHeadInfoProvider(SpecProvider, BlockTree, StateReader); + + NonceManager = new NonceManager(chainHeadInfoProvider.AccountStateProvider); + _trieStoreWatcher = new TrieStoreBoundaryWatcher(TrieStore, BlockTree, LogManager); ReceiptStorage = new InMemoryReceiptStorage(); diff --git a/src/Nethermind/Nethermind.Facade.Test/TxPoolBridgeTests.cs b/src/Nethermind/Nethermind.Facade.Test/TxPoolBridgeTests.cs index e71e732cc99..0dacce0af55 100644 --- a/src/Nethermind/Nethermind.Facade.Test/TxPoolBridgeTests.cs +++ b/src/Nethermind/Nethermind.Facade.Test/TxPoolBridgeTests.cs @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using System.Threading; using FluentAssertions; using Nethermind.Consensus; using Nethermind.Core; @@ -19,19 +20,23 @@ public class TxPoolBridgeTests private ITxSender _txSender; private ITxPool _txPool; private ITxSigner _txSigner; + private INonceManager _nonceManager; + private IEthereumEcdsa _ecdsa; [SetUp] public void SetUp() { _txPool = Substitute.For(); _txSigner = Substitute.For(); - _txSender = new TxPoolSender(_txPool, new TxSealer(_txSigner, Timestamper.Default)); + _nonceManager = new NonceManager(Substitute.For()); + _ecdsa = Substitute.For(); + _txSender = new TxPoolSender(_txPool, new TxSealer(_txSigner, Timestamper.Default), _nonceManager, _ecdsa); } [Test] public void Timestamp_is_set_on_transactions() { - Transaction tx = Build.A.Transaction.Signed(new EthereumEcdsa(TestBlockchainIds.ChainId, LimboLogs.Instance), TestItem.PrivateKeyA).TestObject; + Transaction tx = Build.A.Transaction.WithSenderAddress(TestItem.AddressA).Signed(new EthereumEcdsa(TestBlockchainIds.ChainId, LimboLogs.Instance), TestItem.PrivateKeyA).TestObject; _txSender.SendTransaction(tx, TxHandlingOptions.PersistentBroadcast); _txPool.Received().SubmitTx(Arg.Is(tx => tx.Timestamp != UInt256.Zero), TxHandlingOptions.PersistentBroadcast); } diff --git a/src/Nethermind/Nethermind.Init/Steps/InitializeBlockchain.cs b/src/Nethermind/Nethermind.Init/Steps/InitializeBlockchain.cs index 9f4c61d756c..74d00e8ae46 100644 --- a/src/Nethermind/Nethermind.Init/Steps/InitializeBlockchain.cs +++ b/src/Nethermind/Nethermind.Init/Steps/InitializeBlockchain.cs @@ -245,6 +245,15 @@ private Task InitBlockchain() IChainHeadInfoProvider chainHeadInfoProvider = new ChainHeadInfoProvider(getApi.SpecProvider, getApi.BlockTree, stateReader); + + // TODO: can take the tx sender from plugin here maybe + ITxSigner txSigner = new WalletTxSigner(getApi.Wallet, getApi.SpecProvider.ChainId); + TxSealer nonceReservingTxSealer = + new(txSigner, getApi.Timestamper); + INonceManager nonceManager = new NonceManager(chainHeadInfoProvider.AccountStateProvider); + setApi.NonceManager = nonceManager; + setApi.TxSender = new TxPoolSender(txPool, nonceReservingTxSealer, nonceManager, getApi.EthereumEcdsa!); + setApi.TxPoolInfoProvider = new TxPoolInfoProvider(chainHeadInfoProvider.AccountStateProvider, txPool); setApi.GasPriceOracle = new GasPriceOracle(getApi.BlockTree, getApi.SpecProvider, _api.LogManager, blocksConfig.MinGasPrice); IBlockProcessor mainBlockProcessor = setApi.MainBlockProcessor = CreateBlockProcessor(); @@ -265,12 +274,6 @@ private Task InitBlockchain() setApi.BlockchainProcessor = blockchainProcessor; setApi.EthSyncingInfo = new EthSyncingInfo(getApi.BlockTree, getApi.ReceiptStorage!, syncConfig, getApi.LogManager); - // TODO: can take the tx sender from plugin here maybe - ITxSigner txSigner = new WalletTxSigner(getApi.Wallet, getApi.SpecProvider.ChainId); - NonceReservingTxSealer nonceReservingTxSealer = - new(txSigner, getApi.Timestamper, txPool, getApi.EthereumEcdsa!); - setApi.TxSender = new TxPoolSender(txPool, nonceReservingTxSealer); - IFilterStore filterStore = setApi.FilterStore = new FilterStore(); setApi.FilterManager = new FilterManager(filterStore, mainBlockProcessor, txPool, getApi.LogManager); setApi.HealthHintService = CreateHealthHintService(); diff --git a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.cs b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.cs index 3f4b6a63f69..0267b7ebfcf 100644 --- a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.cs +++ b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.cs @@ -971,7 +971,8 @@ public async Task Send_transaction_with_signature_will_not_try_to_sign() public async Task Send_raw_transaction_will_send_transaction(string rawTransaction) { using Context ctx = await Context.Create(); - ITxSender txSender = Substitute.ForPartsOf(ctx.Test.TxPool, ctx.Test.TxSealer); + ITxSender txSender = Substitute.ForPartsOf(ctx.Test.TxPool, ctx.Test.TxSealer, + ctx.Test.NonceManager, ctx.Test.EthereumEcdsa); IBlockchainBridge bridge = Substitute.For(); ctx.Test = await TestRpcBlockchain.ForTest(SealEngineType.NethDev).WithBlockchainBridge(bridge).WithTxSender(txSender).Build(); string serialized = ctx.Test.TestEthRpc("eth_sendRawTransaction", rawTransaction); diff --git a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs index d55c545e604..fe7996211c8 100644 --- a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs +++ b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs @@ -126,8 +126,8 @@ protected override async Task Build(ISpecProvider? specProvider ITxSigner txSigner = new WalletTxSigner(TestWallet, specProvider.ChainId); - TxSealer = new NonceReservingTxSealer(txSigner, Timestamper, TxPool, EthereumEcdsa ?? new EthereumEcdsa(specProvider.ChainId, LogManager)); - TxSender ??= new TxPoolSender(TxPool, TxSealer); + TxSealer = new TxSealer(txSigner, Timestamper); + TxSender ??= new TxPoolSender(TxPool, TxSealer, NonceManager, EthereumEcdsa ?? new EthereumEcdsa(specProvider.ChainId, LogManager)); GasPriceOracle ??= new GasPriceOracle(BlockFinder, SpecProvider, LogManager); FeeHistoryOracle ??= new FeeHistoryOracle(BlockFinder, ReceiptStorage, SpecProvider); ISyncConfig syncConfig = new SyncConfig(); diff --git a/src/Nethermind/Nethermind.TxPool.Test/NonceManagerTests.cs b/src/Nethermind/Nethermind.TxPool.Test/NonceManagerTests.cs new file mode 100644 index 00000000000..9d38cb756b3 --- /dev/null +++ b/src/Nethermind/Nethermind.TxPool.Test/NonceManagerTests.cs @@ -0,0 +1,225 @@ +// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Threading.Tasks; +using FluentAssertions; +using Nethermind.Blockchain; +using Nethermind.Core; +using Nethermind.Core.Specs; +using Nethermind.Core.Test.Builders; +using Nethermind.Db; +using Nethermind.Int256; +using Nethermind.Logging; +using Nethermind.Specs; +using Nethermind.State; +using Nethermind.Trie.Pruning; +using NSubstitute; +using NUnit.Framework; + +namespace Nethermind.TxPool.Test; + +public class NonceManagerTests +{ + private ISpecProvider _specProvider; + private IStateProvider _stateProvider; + private IBlockTree _blockTree; + private ChainHeadInfoProvider _headInfo; + private INonceManager _nonceManager; + + [SetUp] + public void Setup() + { + ILogManager logManager = LimboLogs.Instance; + _specProvider = RopstenSpecProvider.Instance; + var trieStore = new TrieStore(new MemDb(), logManager); + var codeDb = new MemDb(); + _stateProvider = new StateProvider(trieStore, codeDb, logManager); + _blockTree = Substitute.For(); + Block block = Build.A.Block.WithNumber(0).TestObject; + _blockTree.Head.Returns(block); + _blockTree.FindBestSuggestedHeader().Returns(Build.A.BlockHeader.WithNumber(10000000).TestObject); + + _headInfo = new ChainHeadInfoProvider(_specProvider, _blockTree, _stateProvider); + _nonceManager = new NonceManager(_headInfo.AccountStateProvider); + } + + [Test] + public void should_increment_own_transaction_nonces_locally_when_requesting_reservations() + { + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(0); + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(1); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(1); + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressB, out UInt256 nonce)) + { + nonce.Should().Be(0); + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressB, out UInt256 nonce)) + { + nonce.Should().Be(1); + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressB, out UInt256 nonce)) + { + nonce.Should().Be(2); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressB, out UInt256 nonce)) + { + nonce.Should().Be(2); + locker.Accept(); + } + } + + [Test] + [Repeat(10)] + public void should_increment_own_transaction_nonces_locally_when_requesting_reservations_in_parallel() + { + const int reservationsCount = 1000; + + ConcurrentQueue nonces = new(); + + var result = Parallel.For(0, reservationsCount, i => + { + using NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce); + locker.Accept(); + nonces.Enqueue(nonce); + }); + + result.IsCompleted.Should().BeTrue(); + using NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce); + nonces.Enqueue(nonce); + nonce.Should().Be(new UInt256(reservationsCount)); + nonces.OrderBy(n => n).Should().BeEquivalentTo(Enumerable.Range(0, reservationsCount + 1).Select(i => new UInt256((uint)i))); + } + + [Test] + public void should_pick_account_nonce_as_initial_value() + { + IAccountStateProvider accountStateProvider = Substitute.For(); + Account account = new(0); + accountStateProvider.GetAccount(TestItem.AddressA).Returns(account); + _nonceManager = new NonceManager(accountStateProvider); + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(0); + } + + accountStateProvider.GetAccount(TestItem.AddressA).Returns(account.WithChangedNonce(10)); + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(10); + } + } + + [Test] + public void ReserveNonce_should_skip_nonce_if_TxWithNonceReceived() + { + using (NonceLocker locker = _nonceManager.TxWithNonceReceived(TestItem.AddressA, 4)) + { + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(0); + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(1); + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.TxWithNonceReceived(TestItem.AddressA, 2)) + { + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(3); + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(5); + locker.Accept(); + } + } + + [Test] + public void should_reuse_nonce_if_tx_rejected() + { + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(0); + } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(0); + locker.Accept(); + } + + using (NonceLocker locker = _nonceManager.TxWithNonceReceived(TestItem.AddressA, 1)) { } + + using (NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce)) + { + nonce.Should().Be(1); + locker.Accept(); + } + } + + [Test] + [Repeat(10)] + public void should_lock_on_same_account() + { + using NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce); + nonce.Should().Be(0); + Task task = Task.Run(() => + { + using NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 _); + }); + TimeSpan ts = TimeSpan.FromMilliseconds(1000); + task.Wait(ts); + task.IsCompleted.Should().Be(false); + } + + [Test] + [Repeat(10)] + public void should_not_lock_on_different_accounts() + { + using NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce); + nonce.Should().Be(0); + Task task = Task.Run(() => + { + using NonceLocker locker2 = _nonceManager.ReserveNonce(TestItem.AddressB, out UInt256 nonce2); + nonce2.Should().Be(0); + }); + TimeSpan ts = TimeSpan.FromMilliseconds(1000); + task.Wait(ts); + task.IsCompleted.Should().Be(true); + } +} diff --git a/src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs b/src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs index 7156e08a2b8..3b4e8f7ee4e 100644 --- a/src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs +++ b/src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs @@ -980,46 +980,7 @@ public void should_delete_pending_transactions() } [Test] - public void should_increment_own_transaction_nonces_locally_when_requesting_reservations() - { - _txPool = CreatePool(); - var nonceA1 = _txPool.ReserveOwnTransactionNonce(TestItem.AddressA); - var nonceA2 = _txPool.ReserveOwnTransactionNonce(TestItem.AddressA); - var nonceA3 = _txPool.ReserveOwnTransactionNonce(TestItem.AddressA); - var nonceB1 = _txPool.ReserveOwnTransactionNonce(TestItem.AddressB); - var nonceB2 = _txPool.ReserveOwnTransactionNonce(TestItem.AddressB); - var nonceB3 = _txPool.ReserveOwnTransactionNonce(TestItem.AddressB); - - nonceA1.Should().Be(0); - nonceA2.Should().Be(1); - nonceA3.Should().Be(2); - nonceB1.Should().Be(0); - nonceB2.Should().Be(1); - nonceB3.Should().Be(2); - } - - [Test] - public void should_increment_own_transaction_nonces_locally_when_requesting_reservations_in_parallel() - { - TransactionBuilder builder = new TransactionBuilder(); - const int reservationsCount = 1000; - _txPool = CreatePool(); - ConcurrentQueue nonces = new(); - - var result = Parallel.For(0, reservationsCount, i => - { - nonces.Enqueue(_txPool.ReserveOwnTransactionNonce(TestItem.AddressA)); - }); - - result.IsCompleted.Should().BeTrue(); - UInt256 nonce = _txPool.ReserveOwnTransactionNonce(TestItem.AddressA); - nonces.Enqueue(nonce); - nonce.Should().Be(new UInt256(reservationsCount)); - nonces.OrderBy(n => n).Should().BeEquivalentTo(Enumerable.Range(0, reservationsCount + 1).Select(i => new UInt256((uint)i))); - } - - [Test] - public void should_return_own_nonce_already_used_result_when_trying_to_send_transaction_with_same_nonce_for_same_address() + public void should_return_feeTooLowTooCompete_result_when_trying_to_send_transaction_with_same_nonce_for_same_address() { _txPool = CreatePool(); var result1 = _txPool.SubmitTx(GetTransaction(TestItem.PrivateKeyA, TestItem.AddressA), TxHandlingOptions.PersistentBroadcast | TxHandlingOptions.ManagedNonce); @@ -1027,7 +988,7 @@ public void should_return_own_nonce_already_used_result_when_trying_to_send_tran _txPool.GetOwnPendingTransactions().Length.Should().Be(1); _txPool.GetPendingTransactions().Length.Should().Be(1); var result2 = _txPool.SubmitTx(GetTransaction(TestItem.PrivateKeyA, TestItem.AddressB), TxHandlingOptions.PersistentBroadcast | TxHandlingOptions.ManagedNonce); - result2.Should().Be(AcceptTxResult.OwnNonceAlreadyUsed); + result2.Should().Be(AcceptTxResult.FeeTooLowToCompete); _txPool.GetOwnPendingTransactions().Length.Should().Be(1); _txPool.GetPendingTransactions().Length.Should().Be(1); } @@ -1381,6 +1342,105 @@ public void TooExpensiveTxFilter_correctly_calculates_cumulative_cost() _txPool.SubmitTx(expensiveTx, TxHandlingOptions.None).Should().Be(AcceptTxResult.Accepted); } + [Test] + public void should_increase_nonce_when_transaction_not_included_in_txPool_but_broadcasted() + { + ISpecProvider specProvider = GetLondonSpecProvider(); + _txPool = CreatePool(new TxPoolConfig { Size = 2 }, specProvider); + + ITxPoolPeer peer = Substitute.For(); + peer.Id.Returns(TestItem.PublicKeyA); + + _txPool.AddPeer(peer); + + // Add two transactions with high gas price + Transaction firstTx = Build.A.Transaction + .WithNonce(0) + .WithType(TxType.EIP1559) + .WithMaxFeePerGas(100) + .WithMaxPriorityFeePerGas(100) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; + Transaction secondTx = Build.A.Transaction + .WithNonce(1) + .WithType(TxType.EIP1559) + .WithMaxFeePerGas(100) + .WithMaxPriorityFeePerGas(100) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; + EnsureSenderBalance(TestItem.AddressA, UInt256.MaxValue); + + _txPool.SubmitTx(firstTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.Accepted); + peer.Received().SendNewTransaction(firstTx); + _txPool.SubmitTx(secondTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.Accepted); + peer.Received().SendNewTransaction(secondTx); + + // Send cheap transaction => Not included in txPool + Transaction cheapTx = Build.A.Transaction + .WithNonce(2) + .WithType(TxType.EIP1559) + .WithMaxFeePerGas(1) + .WithMaxPriorityFeePerGas(1) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; + _txPool.SubmitTx(cheapTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.Accepted); + peer.Received().SendNewTransaction(cheapTx); + + // Send transaction with increased nonce => NonceGap should not appear as previous transaction is broadcasted, should be accepted + Transaction fourthTx = Build.A.Transaction + .WithNonce(3) + .WithType(TxType.EIP1559) + .WithMaxFeePerGas(1) + .WithMaxPriorityFeePerGas(1) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; + _txPool.SubmitTx(fourthTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.Accepted); + peer.Received().SendNewTransaction(fourthTx); + } + + [Test] + public async Task should_include_transaction_after_removal() + { + ISpecProvider specProvider = GetLondonSpecProvider(); + _txPool = CreatePool(new TxPoolConfig { Size = 2 }, specProvider); + + // Send cheap transaction + Transaction txA = Build.A.Transaction + .WithNonce(0) + .WithType(TxType.EIP1559) + .WithMaxFeePerGas(1) + .WithMaxPriorityFeePerGas(1) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyB).TestObject; + EnsureSenderBalance(TestItem.AddressB, UInt256.MaxValue); + + _txPool.SubmitTx(txA, TxHandlingOptions.None).Should().Be(AcceptTxResult.Accepted); + + Transaction expensiveTx1 = Build.A.Transaction + .WithNonce(0) + .WithType(TxType.EIP1559) + .WithMaxFeePerGas(100) + .WithMaxPriorityFeePerGas(100) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; + Transaction expensiveTx2 = Build.A.Transaction + .WithNonce(1) + .WithType(TxType.EIP1559) + .WithMaxFeePerGas(100) + .WithMaxPriorityFeePerGas(100) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; + EnsureSenderBalance(TestItem.AddressA, UInt256.MaxValue); + + // Send two transactions with high gas price => txA removed from pool + _txPool.SubmitTx(expensiveTx1, TxHandlingOptions.None).Should().Be(AcceptTxResult.Accepted); + _txPool.SubmitTx(expensiveTx2, TxHandlingOptions.None).Should().Be(AcceptTxResult.Accepted); + + // Rise new block event to cleanup cash and remove one expensive tx + _blockTree.BlockAddedToMain += + Raise.Event>(this, + new BlockReplacementEventArgs(Build.A.Block.WithTransactions(expensiveTx1).TestObject)); + + // Wait four event processing + await Task.Delay(100); + + // Send txA again => should be Accepted + _txPool.SubmitTx(txA, TxHandlingOptions.None).Should().Be(AcceptTxResult.Accepted); + } + private IDictionary GetPeers(int limit = 100) { var peers = new Dictionary(); diff --git a/src/Nethermind/Nethermind.TxPool/Filters/GapNonceFilter.cs b/src/Nethermind/Nethermind.TxPool/Filters/GapNonceFilter.cs index 1cd95e6e667..be2cbc90853 100644 --- a/src/Nethermind/Nethermind.TxPool/Filters/GapNonceFilter.cs +++ b/src/Nethermind/Nethermind.TxPool/Filters/GapNonceFilter.cs @@ -25,12 +25,13 @@ public GapNonceFilter(TxDistinctSortedPool txs, ILogger logger) public AcceptTxResult Accept(Transaction tx, TxFilteringState state, TxHandlingOptions handlingOptions) { + bool isNotLocal = (handlingOptions & TxHandlingOptions.PersistentBroadcast) != TxHandlingOptions.PersistentBroadcast; int numberOfSenderTxsInPending = _txs.GetBucketCount(tx.SenderAddress!); // since unknownSenderFilter will run before this one bool isTxPoolFull = _txs.IsFull(); UInt256 currentNonce = state.SenderAccount.Nonce; long nextNonceInOrder = (long)currentNonce + numberOfSenderTxsInPending; bool isTxNonceNextInOrder = tx.Nonce <= nextNonceInOrder; - if (isTxPoolFull && !isTxNonceNextInOrder) + if (isNotLocal && isTxPoolFull && !isTxNonceNextInOrder) { Metrics.PendingTransactionsNonceGap++; if (_logger.IsTrace) diff --git a/src/Nethermind/Nethermind.TxPool/Filters/ReusedOwnNonceTxFilter.cs b/src/Nethermind/Nethermind.TxPool/Filters/ReusedOwnNonceTxFilter.cs deleted file mode 100644 index f52dba334a2..00000000000 --- a/src/Nethermind/Nethermind.TxPool/Filters/ReusedOwnNonceTxFilter.cs +++ /dev/null @@ -1,82 +0,0 @@ -// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using System.Collections.Concurrent; -using Nethermind.Core; -using Nethermind.Int256; -using Nethermind.Logging; - -namespace Nethermind.TxPool.Filters -{ - /// - /// Filters out transactions that were generated at this machine and were already signed with the same nonce. - /// TODO: review if cancel by replace is still possible with this! - /// - internal class ReusedOwnNonceTxFilter : IIncomingTxFilter - { - private readonly object _locker = new(); - private readonly ConcurrentDictionary _nonces; - private readonly ILogger _logger; - - public ReusedOwnNonceTxFilter(ConcurrentDictionary nonces, ILogger logger) - { - _nonces = nonces; - _logger = logger; - } - - public AcceptTxResult Accept(Transaction tx, TxFilteringState state, TxHandlingOptions handlingOptions) - { - bool managedNonce = (handlingOptions & TxHandlingOptions.ManagedNonce) == TxHandlingOptions.ManagedNonce; - Account account = state.SenderAccount; - UInt256 currentNonce = account.Nonce; - - if (managedNonce && CheckOwnTransactionAlreadyUsed(tx, currentNonce)) - { - if (_logger.IsTrace) - _logger.Trace($"Skipped adding transaction {tx.ToString(" ")}, nonce already used."); - return AcceptTxResult.OwnNonceAlreadyUsed; - } - - return AcceptTxResult.Accepted; - } - - /// - /// Nonce manager needed - /// - /// - /// - /// - private bool CheckOwnTransactionAlreadyUsed(Transaction transaction, in UInt256 currentNonce) - { - Address address = transaction.SenderAddress!; // since unknownSenderFilter will run before this one - lock (_locker) - { - if (!_nonces.TryGetValue(address, out var addressNonces)) - { - addressNonces = new TxPool.AddressNonces(currentNonce); - _nonces.TryAdd(address, addressNonces); - } - - if (!addressNonces.Nonces.TryGetValue(transaction.Nonce, out var nonce)) - { - nonce = new TxPool.NonceInfo(transaction.Nonce); - addressNonces.Nonces.TryAdd(transaction.Nonce, new TxPool.NonceInfo(transaction.Nonce)); - } - - if (!(nonce.TransactionHash is null && nonce.TransactionHash != transaction.Hash)) - { - // Nonce conflict - if (_logger.IsDebug) - _logger.Debug( - $"Nonce: {nonce.Value} was already used in transaction: '{nonce.TransactionHash}' and cannot be reused by transaction: '{transaction.Hash}'."); - - return true; - } - - nonce.SetTransactionHash(transaction.Hash!); - } - - return false; - } - } -} diff --git a/src/Nethermind/Nethermind.TxPool/INonceManager.cs b/src/Nethermind/Nethermind.TxPool/INonceManager.cs new file mode 100644 index 00000000000..3803e87e76a --- /dev/null +++ b/src/Nethermind/Nethermind.TxPool/INonceManager.cs @@ -0,0 +1,14 @@ +// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using Nethermind.Core; +using Nethermind.Int256; + +namespace Nethermind.TxPool; + +public interface INonceManager +{ + NonceLocker ReserveNonce(Address address, out UInt256 reservedNonce); + NonceLocker TxWithNonceReceived(Address address, UInt256 nonce); +} diff --git a/src/Nethermind/Nethermind.TxPool/ITxPool.cs b/src/Nethermind/Nethermind.TxPool/ITxPool.cs index 73d06492617..8cd41cec842 100644 --- a/src/Nethermind/Nethermind.TxPool/ITxPool.cs +++ b/src/Nethermind/Nethermind.TxPool/ITxPool.cs @@ -31,7 +31,6 @@ public interface ITxPool bool RemoveTransaction(Keccak? hash); bool IsKnown(Keccak hash); bool TryGetPendingTransaction(Keccak hash, out Transaction? transaction); - UInt256 ReserveOwnTransactionNonce(Address address); // TODO: this should be moved to a signer component, outside of TX pool UInt256 GetLatestPendingNonce(Address address); event EventHandler NewDiscovered; event EventHandler NewPending; diff --git a/src/Nethermind/Nethermind.TxPool/NonceLocker.cs b/src/Nethermind/Nethermind.TxPool/NonceLocker.cs new file mode 100644 index 00000000000..f97139295cf --- /dev/null +++ b/src/Nethermind/Nethermind.TxPool/NonceLocker.cs @@ -0,0 +1,37 @@ +// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Threading; +using Nethermind.Int256; + +namespace Nethermind.TxPool; + +public ref struct NonceLocker +{ + private readonly SemaphoreSlim _accountLock; + private readonly Action _acceptAction; + private int _disposed; + + internal NonceLocker( + SemaphoreSlim accountLock, + Action acceptAction) + { + _accountLock = accountLock; + _acceptAction = acceptAction; + _accountLock.Wait(); + } + + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) == 0) + { + _accountLock.Release(); + } + } + + public void Accept() + { + _acceptAction(); + } +} diff --git a/src/Nethermind/Nethermind.TxPool/NonceManager.cs b/src/Nethermind/Nethermind.TxPool/NonceManager.cs new file mode 100644 index 00000000000..709f2a8f605 --- /dev/null +++ b/src/Nethermind/Nethermind.TxPool/NonceManager.cs @@ -0,0 +1,81 @@ +// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Threading; +using Nethermind.Core; +using Nethermind.Int256; + +namespace Nethermind.TxPool; + +public class NonceManager : INonceManager +{ + private readonly ConcurrentDictionary _addressNonceManagers = new(); + private readonly IAccountStateProvider _accounts; + + public NonceManager(IAccountStateProvider accounts) + { + _accounts = accounts; + } + + public NonceLocker ReserveNonce(Address address, out UInt256 reservedNonce) + { + AddressNonceManager addressNonceManager = + _addressNonceManagers.GetOrAdd(address, _ => new AddressNonceManager()); + return addressNonceManager.ReserveNonce(_accounts.GetAccount(address).Nonce, out reservedNonce); + } + + public NonceLocker TxWithNonceReceived(Address address, UInt256 nonce) + { + AddressNonceManager addressNonceManager = + _addressNonceManagers.GetOrAdd(address, _ => new AddressNonceManager()); + return addressNonceManager.TxWithNonceReceived(nonce); + } + + private class AddressNonceManager + { + private readonly HashSet _usedNonces = new(); + private UInt256 _currentNonce; + private UInt256 _reservedNonce; + private UInt256 _previousAccountNonce; + + private readonly SemaphoreSlim _accountLock = new(1); + + public NonceLocker ReserveNonce(UInt256 accountNonce, out UInt256 reservedNonce) + { + NonceLocker locker = new(_accountLock, TxAccepted); + ReleaseNonces(accountNonce); + _currentNonce = UInt256.Max(_currentNonce, accountNonce); + _reservedNonce = _currentNonce; + reservedNonce = _currentNonce; + return locker; + } + + private void TxAccepted() + { + _usedNonces.Add(_reservedNonce); + while (_usedNonces.Contains(_currentNonce)) + { + _currentNonce++; + } + } + + public NonceLocker TxWithNonceReceived(UInt256 nonce) + { + _reservedNonce = nonce; + return new(_accountLock, TxAccepted); + } + + private void ReleaseNonces(UInt256 accountNonce) + { + for (UInt256 i = _previousAccountNonce; i < accountNonce; i++) + { + _usedNonces.Remove(i); + } + + _previousAccountNonce = accountNonce; + } + } +} diff --git a/src/Nethermind/Nethermind.TxPool/TxNonceTxPoolReserveSealer.cs b/src/Nethermind/Nethermind.TxPool/TxNonceTxPoolReserveSealer.cs index 58000d43ea8..e69de29bb2d 100644 --- a/src/Nethermind/Nethermind.TxPool/TxNonceTxPoolReserveSealer.cs +++ b/src/Nethermind/Nethermind.TxPool/TxNonceTxPoolReserveSealer.cs @@ -1,44 +0,0 @@ -// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using System; -using System.Threading.Tasks; -using Nethermind.Core; -using Nethermind.Core.Specs; -using Nethermind.Crypto; -using Nethermind.Logging; - -namespace Nethermind.TxPool -{ - // TODO: this should be nonce reserving tx sender, not sealer? - public class NonceReservingTxSealer : TxSealer - { - private readonly ITxPool _txPool; - private readonly IEthereumEcdsa _ecdsa; - - public NonceReservingTxSealer(ITxSigner txSigner, - ITimestamper timestamper, - ITxPool txPool, - IEthereumEcdsa ecdsa) - : base(txSigner, timestamper) - { - _txPool = txPool ?? throw new ArgumentNullException(nameof(txPool)); - _ecdsa = ecdsa ?? throw new ArgumentNullException(nameof(ecdsa)); - } - - public override ValueTask Seal(Transaction tx, TxHandlingOptions txHandlingOptions) - { - bool manageNonce = (txHandlingOptions & TxHandlingOptions.ManagedNonce) == TxHandlingOptions.ManagedNonce; - if (manageNonce) - { - tx.SenderAddress ??= _ecdsa.RecoverAddress(tx); - if (tx.SenderAddress is null) - throw new ArgumentNullException(nameof(tx.SenderAddress)); - tx.Nonce = _txPool.ReserveOwnTransactionNonce(tx.SenderAddress); - txHandlingOptions |= TxHandlingOptions.AllowReplacingSignature; - } - - return base.Seal(tx, txHandlingOptions); - } - } -} diff --git a/src/Nethermind/Nethermind.TxPool/TxPool.AddressNonces.cs b/src/Nethermind/Nethermind.TxPool/TxPool.AddressNonces.cs deleted file mode 100644 index 40f6215bcde..00000000000 --- a/src/Nethermind/Nethermind.TxPool/TxPool.AddressNonces.cs +++ /dev/null @@ -1,54 +0,0 @@ -// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using System.Collections.Concurrent; -using System.Linq; -using System.Threading; -using Nethermind.Int256; - -namespace Nethermind.TxPool -{ - public partial class TxPool - { - internal class AddressNonces - { - private NonceInfo _currentNonceInfo; - - public ConcurrentDictionary Nonces { get; } = new(); - - public AddressNonces(in UInt256 startNonce) - { - _currentNonceInfo = new NonceInfo(startNonce); - Nonces.TryAdd(_currentNonceInfo.Value, _currentNonceInfo); - } - - public NonceInfo ReserveNonce() - { - UInt256 nonce = _currentNonceInfo.Value; - NonceInfo newNonce; - bool added = false; - - do - { - nonce += 1; - newNonce = Nonces.AddOrUpdate(nonce, v => - { - added = true; - return new NonceInfo(v); - }, (v, n) => - { - added = false; - return n; - }); - } while (!added); - - if (_currentNonceInfo.Value < newNonce.Value) - { - Interlocked.Exchange(ref _currentNonceInfo, newNonce); - } - - return newNonce; - } - } - } -} diff --git a/src/Nethermind/Nethermind.TxPool/TxPool.NonceInfo.cs b/src/Nethermind/Nethermind.TxPool/TxPool.NonceInfo.cs index 1f89720a118..e69de29bb2d 100644 --- a/src/Nethermind/Nethermind.TxPool/TxPool.NonceInfo.cs +++ b/src/Nethermind/Nethermind.TxPool/TxPool.NonceInfo.cs @@ -1,28 +0,0 @@ -// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using Nethermind.Core.Crypto; -using Nethermind.Int256; - -namespace Nethermind.TxPool -{ - public partial class TxPool - { - internal class NonceInfo - { - public UInt256 Value { get; } - - public Keccak? TransactionHash { get; private set; } - - public NonceInfo(in UInt256 value) - { - Value = value; - } - - public void SetTransactionHash(Keccak transactionHash) - { - TransactionHash = transactionHash; - } - } - } -} diff --git a/src/Nethermind/Nethermind.TxPool/TxPool.cs b/src/Nethermind/Nethermind.TxPool/TxPool.cs index 8aeb2ed697b..4a9949109ed 100644 --- a/src/Nethermind/Nethermind.TxPool/TxPool.cs +++ b/src/Nethermind/Nethermind.TxPool/TxPool.cs @@ -28,12 +28,10 @@ namespace Nethermind.TxPool /// Stores all pending transactions. These will be used by block producer if this node is a miner / validator /// or simply for broadcasting and tracing in other cases. /// - public partial class TxPool : ITxPool, IDisposable + public class TxPool : ITxPool, IDisposable { private readonly object _locker = new(); - private readonly ConcurrentDictionary _nonces = new(); - private readonly List _filterPipeline = new(); private readonly HashCache _hashCache = new(); @@ -88,7 +86,6 @@ public TxPool( _specProvider = _headInfo.SpecProvider; _ecdsa = ecdsa; - MemoryAllowance.MemPoolSize = txPoolConfig.Size; AddNodeInfoEntryForTxPool(); @@ -106,7 +103,6 @@ public TxPool( _filterPipeline.Add(new GapNonceFilter(_transactions, _logger)); _filterPipeline.Add(new TooExpensiveTxFilter(_headInfo, _transactions, _logger)); _filterPipeline.Add(new FeeTooLowFilter(_headInfo, _transactions, logManager)); - _filterPipeline.Add(new ReusedOwnNonceTxFilter(_nonces, _logger)); if (incomingTxFilter is not null) { _filterPipeline.Add(incomingTxFilter); @@ -459,17 +455,6 @@ public bool RemoveTransaction(Keccak? hash) return false; if (hasBeenRemoved) { - Address? address = transaction.SenderAddress; - - if (address != null && _nonces.TryGetValue(address, out AddressNonces? addressNonces)) - { - addressNonces.Nonces.TryRemove(transaction.Nonce, out _); - if (addressNonces.Nonces.IsEmpty) - { - _nonces.Remove(address, out _); - } - } - RemovedPending?.Invoke(this, new TxEventArgs(transaction)); } @@ -501,23 +486,6 @@ public bool TryGetPendingTransaction(Keccak hash, out Transaction? transaction) return transaction is not null; } - // TODO: Ensure that nonce is always valid in case of sending own transactions from different nodes. - public UInt256 ReserveOwnTransactionNonce(Address address) - { - UInt256 currentNonce = 0; - _nonces.AddOrUpdate(address, a => - { - currentNonce = _accounts.GetAccount(address).Nonce; - return new AddressNonces(currentNonce); - }, (a, n) => - { - currentNonce = n.ReserveNonce().Value; - return n; - }); - - return currentNonce; - } - public UInt256 GetLatestPendingNonce(Address address) { UInt256 maxPendingNonce = _accounts.GetAccount(address).Nonce; diff --git a/src/Nethermind/Nethermind.TxPool/TxPoolSender.cs b/src/Nethermind/Nethermind.TxPool/TxPoolSender.cs index 0d78c6737b0..bd1ad9aa594 100644 --- a/src/Nethermind/Nethermind.TxPool/TxPoolSender.cs +++ b/src/Nethermind/Nethermind.TxPool/TxPoolSender.cs @@ -5,6 +5,8 @@ using System.Threading.Tasks; using Nethermind.Core; using Nethermind.Core.Crypto; +using Nethermind.Crypto; +using Nethermind.Int256; namespace Nethermind.TxPool { @@ -12,18 +14,56 @@ public class TxPoolSender : ITxSender { private readonly ITxPool _txPool; private readonly ITxSealer _sealer; + private readonly INonceManager _nonceManager; + private readonly IEthereumEcdsa _ecdsa; - public TxPoolSender(ITxPool txPool, ITxSealer sealer) + public TxPoolSender(ITxPool txPool, ITxSealer sealer, INonceManager nonceManager, IEthereumEcdsa ecdsa) { _txPool = txPool ?? throw new ArgumentNullException(nameof(txPool)); _sealer = sealer ?? throw new ArgumentNullException(nameof(sealer)); + _nonceManager = nonceManager ?? throw new ArgumentNullException(nameof(nonceManager)); + _ecdsa = ecdsa ?? throw new ArgumentException(nameof(ecdsa)); } public ValueTask<(Keccak, AcceptTxResult?)> SendTransaction(Transaction tx, TxHandlingOptions txHandlingOptions) + { + bool manageNonce = (txHandlingOptions & TxHandlingOptions.ManagedNonce) == TxHandlingOptions.ManagedNonce; + tx.SenderAddress ??= _ecdsa.RecoverAddress(tx); + if (tx.SenderAddress is null) + throw new ArgumentNullException(nameof(tx.SenderAddress)); + + AcceptTxResult result = manageNonce + ? SubmitTxWithManagedNonce(tx, txHandlingOptions) + : SubmitTxWithNonce(tx, txHandlingOptions); + + return new ValueTask<(Keccak, AcceptTxResult?)>((tx.Hash!, result)); // The sealer calculates the hash + } + + private AcceptTxResult SubmitTxWithNonce(Transaction tx, TxHandlingOptions txHandlingOptions) + { + using NonceLocker locker = _nonceManager.TxWithNonceReceived(tx.SenderAddress!, tx.Nonce); + return SubmitTx(locker, tx, txHandlingOptions); + } + + private AcceptTxResult SubmitTxWithManagedNonce(Transaction tx, TxHandlingOptions txHandlingOptions) + { + using NonceLocker locker = _nonceManager.ReserveNonce(tx.SenderAddress!, out UInt256 reservedNonce); + txHandlingOptions |= TxHandlingOptions.AllowReplacingSignature; + tx.Nonce = reservedNonce; + return SubmitTx(locker, tx, txHandlingOptions); + } + + private AcceptTxResult SubmitTx(NonceLocker locker, Transaction tx, TxHandlingOptions txHandlingOptions) { _sealer.Seal(tx, txHandlingOptions); AcceptTxResult result = _txPool.SubmitTx(tx, txHandlingOptions); - return new ValueTask<(Keccak, AcceptTxResult?)>((tx.Hash!, result)); // The sealer calculates the hash + + if (result == AcceptTxResult.Accepted) + { + locker.Accept(); + } + + return result; } } }