From 82afd0fe078c8e085aef13ecc0eb71eeacfa22f9 Mon Sep 17 00:00:00 2001 From: Marcin Sobczak <77129288+marcindsobczak@users.noreply.github.com> Date: Thu, 14 Dec 2023 09:21:10 +0100 Subject: [PATCH] Broadcast local txs only if MaxFeePerGas is equal at least 70% of current base fee (#6350) * limit initial broadcast * add BaseFeeThreshold to ChainHeadInfoProvider * simplify broadcaster * add simple test * make BaseFeeThreshold configurable in TxPoolConfig * remove from ChainHeadInfoProvider * more tests * cosmetic * cosmetic * fix posdao tests * Update src/Nethermind/Nethermind.TxPool/ITxPoolConfig.cs Co-authored-by: Ruben Buniatyan --------- Co-authored-by: Ruben Buniatyan --- .../TxBroadcasterTests.cs | 63 +++++++++++++++++++ .../Nethermind.TxPool/ITxPoolConfig.cs | 3 + .../Nethermind.TxPool/TxBroadcaster.cs | 42 ++++++++++++- src/Nethermind/Nethermind.TxPool/TxPool.cs | 2 +- .../Nethermind.TxPool/TxPoolConfig.cs | 1 + 5 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs b/src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs index 3a1448b7951..4a1f694bca3 100644 --- a/src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs +++ b/src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs @@ -334,6 +334,36 @@ public void should_not_pick_txs_with_GasPrice_lower_than_CurrentBaseFee([Values( expectedTxs.Should().BeEquivalentTo(pickedTxs); } + [TestCase(0, false)] + [TestCase(69, false)] + [TestCase(70, true)] + [TestCase(100, true)] + [TestCase(150, true)] + public void should_not_broadcast_tx_with_MaxFeePerGas_lower_than_70_percent_of_CurrentBaseFee(int maxFeePerGas, bool shouldBroadcast) + { + _headInfo.CurrentBaseFee.Returns((UInt256)100); + + _broadcaster = new TxBroadcaster(_comparer, TimerFactory.Default, _txPoolConfig, _headInfo, _logManager); + + ITxPoolPeer peer = Substitute.For(); + peer.Id.Returns(TestItem.PublicKeyA); + _broadcaster.AddPeer(peer); + + Transaction transaction = Build.A.Transaction + .WithType(TxType.EIP1559) + .WithMaxFeePerGas((UInt256)maxFeePerGas) + .SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA) + .TestObject; + + _broadcaster.Broadcast(transaction, true); + + // 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); + } + [Test] public void should_not_pick_1559_txs_with_MaxFeePerGas_lower_than_CurrentBaseFee([Values(1, 2, 99, 100, 101, 1000)] int threshold) { @@ -657,6 +687,39 @@ public void should_rebroadcast_all_persistent_transactions_if_PeerNotificationTh } } + [TestCase(0, 0)] + [TestCase(2, 1)] + [TestCase(3, 2)] + [TestCase(7, 4)] + [TestCase(8, 5)] + [TestCase(100, 70)] + [TestCase(9999, 6999)] + [TestCase(10000, 7000)] + public void should_calculate_baseFeeThreshold_correctly(int baseFee, int expectedThreshold) + { + _headInfo.CurrentBaseFee.Returns((UInt256)baseFee); + _broadcaster = new TxBroadcaster(_comparer, TimerFactory.Default, _txPoolConfig, _headInfo, _logManager); + _broadcaster.CalculateBaseFeeThreshold().Should().Be((UInt256)expectedThreshold); + } + + [Test] + public void calculation_of_baseFeeThreshold_should_handle_overflow_correctly([Values(0, 70, 100, 101, 500)] int threshold, [Values(2, 3, 4, 5, 6, 7, 8, 9, 10, 11)] int divisor) + { + UInt256.Divide(UInt256.MaxValue, (UInt256)divisor, out UInt256 baseFee); + _headInfo.CurrentBaseFee.Returns(baseFee); + + _txPoolConfig = new TxPoolConfig() { MinBaseFeeThreshold = threshold }; + _broadcaster = new TxBroadcaster(_comparer, TimerFactory.Default, _txPoolConfig, _headInfo, _logManager); + + UInt256.Divide(baseFee, 100, out UInt256 onePercentOfBaseFee); + bool overflow = UInt256.MultiplyOverflow(onePercentOfBaseFee, (UInt256)threshold, out UInt256 lessAccurateBaseFeeThreshold); + + _broadcaster.CalculateBaseFeeThreshold().Should().Be( + UInt256.MultiplyOverflow(baseFee, (UInt256)threshold, out UInt256 baseFeeThreshold) + ? overflow ? UInt256.MaxValue : lessAccurateBaseFeeThreshold + : baseFeeThreshold); + } + private (IList expectedTxs, IList expectedHashes) GetTxsAndHashesExpectedToBroadcast(Transaction[] transactions, int expectedCountTotal) { List expectedTxs = new(); diff --git a/src/Nethermind/Nethermind.TxPool/ITxPoolConfig.cs b/src/Nethermind/Nethermind.TxPool/ITxPoolConfig.cs index 95ba7cd7ca7..02420aca0b0 100644 --- a/src/Nethermind/Nethermind.TxPool/ITxPoolConfig.cs +++ b/src/Nethermind/Nethermind.TxPool/ITxPoolConfig.cs @@ -10,6 +10,9 @@ public interface ITxPoolConfig : IConfig [ConfigItem(DefaultValue = "5", Description = "The average percentage of transaction hashes from persistent broadcast sent to a peer together with hashes of the last added transactions.")] int PeerNotificationThreshold { get; set; } + [ConfigItem(DefaultValue = "70", Description = "The minimal percentage of the current base fee that must be surpassed by the max fee (`max_fee_per_gas`) for the transaction to be broadcasted.")] + int MinBaseFeeThreshold { get; set; } + [ConfigItem(DefaultValue = "2048", Description = "The max number of transactions held in the mempool (the more transactions in the mempool, the more memory used).")] int Size { get; set; } diff --git a/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs b/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs index ef53ab53208..be55ae965ff 100644 --- a/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs +++ b/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs @@ -52,11 +52,18 @@ internal class TxBroadcaster : IDisposable /// private ResettableList _txsToSend; + + /// + /// Minimal value of MaxFeePerGas of local tx to be broadcasted immediately after receiving it + /// + private UInt256 _baseFeeThreshold; + /// /// Used to throttle tx broadcast. Particularly during forward sync where the head changes a lot which triggers /// a lot of broadcast. There are no transaction in pool but its quite spammy on the log. /// private DateTimeOffset _lastPersistedTxBroadcast = DateTimeOffset.UnixEpoch; + private readonly TimeSpan _minTimeBetweenPersistedTxBroadcast = TimeSpan.FromSeconds(1); private readonly ILogger _logger; @@ -80,6 +87,8 @@ public TxBroadcaster(IComparer comparer, _timer.Elapsed += TimerOnElapsed; _timer.AutoReset = false; _timer.Start(); + + _baseFeeThreshold = CalculateBaseFeeThreshold(); } // only for testing reasons @@ -99,7 +108,13 @@ public void Broadcast(Transaction tx, bool isPersistent) private void StartBroadcast(Transaction tx) { - NotifyPeersAboutLocalTx(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()) + { + NotifyPeersAboutLocalTx(tx); + } + if (tx.Hash is not null) { _persistentTxs.TryInsert(tx.Hash, tx.SupportsBlobs ? new LightTransaction(tx) : tx); @@ -122,7 +137,29 @@ public void AnnounceOnce(ITxPoolPeer peer, Transaction[] txs) } } - public void BroadcastPersistentTxs() + public void OnNewHead() + { + _baseFeeThreshold = CalculateBaseFeeThreshold(); + BroadcastPersistentTxs(); + } + + internal UInt256 CalculateBaseFeeThreshold() + { + bool overflow = UInt256.MultiplyOverflow(_headInfo.CurrentBaseFee, (UInt256)_txPoolConfig.MinBaseFeeThreshold, out UInt256 baseFeeThreshold); + UInt256.Divide(baseFeeThreshold, 100, out baseFeeThreshold); + + if (overflow) + { + UInt256.Divide(_headInfo.CurrentBaseFee, 100, out baseFeeThreshold); + overflow = UInt256.MultiplyOverflow(baseFeeThreshold, (UInt256)_txPoolConfig.MinBaseFeeThreshold, out baseFeeThreshold); + } + + // if there is still an overflow, it means that MinBaseFeeThreshold > 100 + // we are returning max possible value of UInt256.MaxValue + return overflow ? UInt256.MaxValue : baseFeeThreshold; + } + + internal void BroadcastPersistentTxs() { if (_persistentTxs.Count == 0) { @@ -274,7 +311,6 @@ private void Notify(ITxPoolPeer peer, IEnumerable txs, bool sendFul { try { - peer.SendNewTransactions(txs.Where(t => _txGossipPolicy.ShouldGossipTransaction(t)), sendFullTx); if (_logger.IsTrace) _logger.Trace($"Notified {peer} about transactions."); } diff --git a/src/Nethermind/Nethermind.TxPool/TxPool.cs b/src/Nethermind/Nethermind.TxPool/TxPool.cs index f28b42d9ec0..701c2bf9eea 100644 --- a/src/Nethermind/Nethermind.TxPool/TxPool.cs +++ b/src/Nethermind/Nethermind.TxPool/TxPool.cs @@ -201,7 +201,7 @@ private void ProcessNewHeads() ReAddReorganisedTransactions(args.PreviousBlock); RemoveProcessedTransactions(args.Block.Transactions); UpdateBuckets(); - _broadcaster.BroadcastPersistentTxs(); + _broadcaster.OnNewHead(); Metrics.TransactionCount = _transactions.Count; Metrics.BlobTransactionCount = _blobTransactions.Count; } diff --git a/src/Nethermind/Nethermind.TxPool/TxPoolConfig.cs b/src/Nethermind/Nethermind.TxPool/TxPoolConfig.cs index 071ed94dd04..7ae41d8bebb 100644 --- a/src/Nethermind/Nethermind.TxPool/TxPoolConfig.cs +++ b/src/Nethermind/Nethermind.TxPool/TxPoolConfig.cs @@ -6,6 +6,7 @@ namespace Nethermind.TxPool public class TxPoolConfig : ITxPoolConfig { public int PeerNotificationThreshold { get; set; } = 5; + public int MinBaseFeeThreshold { get; set; } = 70; public int Size { get; set; } = 2048; public bool BlobSupportEnabled { get; set; } = false; public bool PersistentBlobStorageEnabled { get; set; } = false;