diff --git a/qa/rpc-tests/mempool_limit.py b/qa/rpc-tests/mempool_limit.py index 466444006..874f238b5 100755 --- a/qa/rpc-tests/mempool_limit.py +++ b/qa/rpc-tests/mempool_limit.py @@ -10,8 +10,7 @@ from test_framework.util import ( get_coinbase_address, fail, start_nodes, - wait_and_assert_operationid_status, - DEFAULT_FEE + wait_and_assert_operationid_status ) from test_framework.zip317 import conventional_fee @@ -28,11 +27,11 @@ BASE_ARGS = [ class MempoolLimit(BitcoinTestFramework): def setup_nodes(self): extra_args = [ - BASE_ARGS + ['-mempooltxcostlimit=8000'], # 2 transactions at min cost - BASE_ARGS + ['-mempooltxcostlimit=8000'], # 2 transactions at min cost - BASE_ARGS + ['-mempooltxcostlimit=8000'], # 2 transactions at min cost + BASE_ARGS + ['-mempooltxcostlimit=20000'], # 2 transactions at min cost + BASE_ARGS + ['-mempooltxcostlimit=20000'], # 2 transactions at min cost + BASE_ARGS + ['-mempooltxcostlimit=20000'], # 2 transactions at min cost # Let node 3 hold one more transaction - BASE_ARGS + ['-mempooltxcostlimit=12000'], # 3 transactions at min cost + BASE_ARGS + ['-mempooltxcostlimit=30000'], # 3 transactions at min cost ] return start_nodes(self.num_nodes, self.options.tmpdir, extra_args) @@ -68,17 +67,18 @@ class MempoolLimit(BitcoinTestFramework): zaddr1 = self.nodes[0].z_getnewaddress('sapling') zaddr2 = self.nodes[0].z_getnewaddress('sapling') zaddr3 = self.nodes[0].z_getnewaddress('sapling') + fee = conventional_fee(2) print("Filling mempool...") opid1 = self.nodes[1].z_sendmany( get_coinbase_address(self.nodes[1]), - [{"address": zaddr1, "amount": Decimal('10.0') - DEFAULT_FEE}], - 1, DEFAULT_FEE, 'AllowRevealedSenders') + [{"address": zaddr1, "amount": Decimal('10.0') - fee}], + 1, fee, 'AllowRevealedSenders') wait_and_assert_operationid_status(self.nodes[1], opid1) opid2 = self.nodes[2].z_sendmany( get_coinbase_address(self.nodes[2]), - [{"address": zaddr2, "amount": Decimal('10.0') - DEFAULT_FEE}], - 1, DEFAULT_FEE, 'AllowRevealedSenders') + [{"address": zaddr2, "amount": Decimal('10.0') - fee}], + 1, fee, 'AllowRevealedSenders') wait_and_assert_operationid_status(self.nodes[2], opid2) self.sync_all() @@ -87,8 +87,8 @@ class MempoolLimit(BitcoinTestFramework): print("Adding one more transaction...") opid3 = self.nodes[3].z_sendmany( get_coinbase_address(self.nodes[3]), - [{"address": zaddr3, "amount": Decimal('10.0') - DEFAULT_FEE}], - 1, DEFAULT_FEE, 'AllowRevealedSenders') + [{"address": zaddr3, "amount": Decimal('10.0') - fee}], + 1, fee, 'AllowRevealedSenders') wait_and_assert_operationid_status(self.nodes[3], opid3) # The mempools are no longer guaranteed to be in a consistent state, so we cannot sync sleep(5) @@ -106,9 +106,9 @@ class MempoolLimit(BitcoinTestFramework): print("Checking mempool size reset after block mined...") self.check_mempool_sizes(0) zaddr4 = self.nodes[0].z_getnewaddress('sapling') - opid4 = self.nodes[0].z_sendmany(zaddr1, [{"address": zaddr4, "amount": Decimal('10.0') - 2*conventional_fee(2)}], 1) + opid4 = self.nodes[0].z_sendmany(zaddr1, [{"address": zaddr4, "amount": Decimal('10.0') - 2*fee}], 1) wait_and_assert_operationid_status(self.nodes[0], opid4) - opid5 = self.nodes[0].z_sendmany(zaddr2, [{"address": zaddr4, "amount": Decimal('10.0') - 2*conventional_fee(2)}], 1) + opid5 = self.nodes[0].z_sendmany(zaddr2, [{"address": zaddr4, "amount": Decimal('10.0') - 2*fee}], 1) wait_and_assert_operationid_status(self.nodes[0], opid5) self.sync_all() diff --git a/src/gtest/test_mempoollimit.cpp b/src/gtest/test_mempoollimit.cpp index 3b47497e0..fb08bb115 100644 --- a/src/gtest/test_mempoollimit.cpp +++ b/src/gtest/test_mempoollimit.cpp @@ -11,6 +11,8 @@ #include "util/time.h" #include "util/test.h" #include "transaction_builder.h" +#include "zip317.h" +#include "core_memusage.h" const uint256 TX_ID1 = ArithToUint256(1); @@ -89,18 +91,18 @@ TEST(MempoolLimitTests, MempoolLimitTxSetCheckSizeAfterDropping) MempoolLimitTxSet limitSet(MIN_TX_COST * 2); EXPECT_EQ(0, limitSet.getTotalWeight()); limitSet.add(TX_ID1, MIN_TX_COST, MIN_TX_COST); - EXPECT_EQ(4000, limitSet.getTotalWeight()); + EXPECT_EQ(10000, limitSet.getTotalWeight()); limitSet.add(TX_ID2, MIN_TX_COST, MIN_TX_COST); - EXPECT_EQ(8000, limitSet.getTotalWeight()); + EXPECT_EQ(20000, limitSet.getTotalWeight()); EXPECT_FALSE(limitSet.maybeDropRandom().has_value()); limitSet.add(TX_ID3, MIN_TX_COST, MIN_TX_COST + LOW_FEE_PENALTY); - EXPECT_EQ(12000 + LOW_FEE_PENALTY, limitSet.getTotalWeight()); + EXPECT_EQ(30000 + LOW_FEE_PENALTY, limitSet.getTotalWeight()); std::optional drop = limitSet.maybeDropRandom(); ASSERT_TRUE(drop.has_value()); uint256 txid = drop.value(); testedDropping.insert(txid); // Do not continue to test if a particular trial fails - ASSERT_EQ(txid == TX_ID3 ? 8000 : 8000 + LOW_FEE_PENALTY, limitSet.getTotalWeight()); + ASSERT_EQ(txid == TX_ID3 ? 20000 : 20000 + LOW_FEE_PENALTY, limitSet.getTotalWeight()); } } @@ -113,7 +115,8 @@ TEST(MempoolLimitTests, MempoolCostAndEvictionWeight) auto consensusParams = RegtestActivateSapling(); auto sk = libzcash::SaplingSpendingKey::random(); - auto testNote = GetTestSaplingNote(sk.default_address(), 50000); + CAmount funds = 66000; + auto testNote = GetTestSaplingNote(sk.default_address(), funds); // Default fee { @@ -121,7 +124,7 @@ TEST(MempoolLimitTests, MempoolCostAndEvictionWeight) builder.AddSaplingSpend(sk.expanded_spending_key(), testNote.note, testNote.tree.root(), testNote.tree.witness()); builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 25000, {}); - auto [cost, evictionWeight] = MempoolCostAndEvictionWeight(builder.Build().GetTxOrThrow(), DEFAULT_FEE); + auto [cost, evictionWeight] = MempoolCostAndEvictionWeight(builder.Build().GetTxOrThrow(), MINIMUM_FEE); EXPECT_EQ(MIN_TX_COST, cost); EXPECT_EQ(MIN_TX_COST, evictionWeight); } @@ -131,10 +134,10 @@ TEST(MempoolLimitTests, MempoolCostAndEvictionWeight) auto builder = TransactionBuilder(consensusParams, 1, std::nullopt); builder.AddSaplingSpend(sk.expanded_spending_key(), testNote.note, testNote.tree.root(), testNote.tree.witness()); builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 25000, {}); - static_assert(DEFAULT_FEE == 1000); - builder.SetFee(DEFAULT_FEE-1); + static_assert(MINIMUM_FEE == 10000); + builder.SetFee(MINIMUM_FEE-1); - auto [cost, evictionWeight] = MempoolCostAndEvictionWeight(builder.Build().GetTxOrThrow(), DEFAULT_FEE-1); + auto [cost, evictionWeight] = MempoolCostAndEvictionWeight(builder.Build().GetTxOrThrow(), MINIMUM_FEE-1); EXPECT_EQ(MIN_TX_COST, cost); EXPECT_EQ(MIN_TX_COST + LOW_FEE_PENALTY, evictionWeight); } @@ -143,18 +146,34 @@ TEST(MempoolLimitTests, MempoolCostAndEvictionWeight) { auto builder = TransactionBuilder(consensusParams, 1, std::nullopt); builder.AddSaplingSpend(sk.expanded_spending_key(), testNote.note, testNote.tree.root(), testNote.tree.witness()); - builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 5000, {}); - builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 5000, {}); - builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 5000, {}); - builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 5000, {}); + for (int i = 0; i < 10; i++) { + builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 1000, {}); + } auto result = builder.Build(); if (result.IsError()) { std::cerr << result.GetError() << std::endl; } - auto [cost, evictionWeight] = MempoolCostAndEvictionWeight(result.GetTxOrThrow(), DEFAULT_FEE); - EXPECT_EQ(5168, cost); - EXPECT_EQ(5168, evictionWeight); + // max(1 input, 10 outputs + 1 change output) => 11 logical actions. + CAmount zip317_fee = CalculateConventionalFee(11); + ASSERT_GT(funds, 1000*10 + zip317_fee); + const CTransaction tx {result.GetTxOrThrow()}; + EXPECT_EQ(11, tx.GetLogicalActionCount()); + + // For the test to be valid, we want the memory usage of this transaction to be more than MIN_TX_COST. + // Avoid hard-coding the usage because it might be platform-dependent. + ASSERT_GT(GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION), MIN_TX_COST); + size_t tx_usage = RecursiveDynamicUsage(tx); + EXPECT_GT(tx_usage, MIN_TX_COST); + + auto [cost, evictionWeight] = MempoolCostAndEvictionWeight(tx, zip317_fee); + EXPECT_EQ(tx_usage, cost); + EXPECT_EQ(tx_usage, evictionWeight); + + // If we pay less than the conventional fee for 11 actions, we should incur a low fee penalty. + auto [cost2, evictionWeight2] = MempoolCostAndEvictionWeight(tx, zip317_fee-1); + EXPECT_EQ(tx_usage, cost2); + EXPECT_EQ(tx_usage + LOW_FEE_PENALTY, evictionWeight2); } RegtestDeactivateSapling(); diff --git a/src/mempool_limit.h b/src/mempool_limit.h index b9cc314df..b9a7ffee5 100644 --- a/src/mempool_limit.h +++ b/src/mempool_limit.h @@ -22,8 +22,8 @@ const size_t DEFAULT_MEMPOOL_TOTAL_COST_LIMIT = 80000000; const int64_t DEFAULT_MEMPOOL_EVICTION_MEMORY_MINUTES = 60; const size_t EVICTION_MEMORY_ENTRIES = 40000; -const int64_t MIN_TX_COST = 4000; -const int64_t LOW_FEE_PENALTY = 16000; +const int64_t MIN_TX_COST = 10000; +const int64_t LOW_FEE_PENALTY = 40000; // This class keeps track of transactions which have been recently evicted from the mempool