zecfoundation
/
zcashd
mirror of https://github.com/ZcashFoundation/zcashd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Change ZIP 401 mempool limiting to use constants decided in zcash/zips#565. 

fixes #6518

In a ZIP sync meeting we decided that:

* The minimum cost should be changed to 10000, in order to avoid
  penalizing Orchard-using transactions too much relative to other
  transactions.
* `low_fee_penalty` should be changed to 40000. This preserves the
  property that a transaction paying less than the ZIP 317 conventional
  fee is deprioritized relative to a min-cost, conventional-fee
  transaction by a factor of 5, as in the original design.
* The recommended default for `mempooltxcostlimit` should remain at
  80000000. Rationale: 80000000 was chosen so that the worst-case size
  of the mempool would be equal to the worst-case size of 40 blocks,
  which is the current default transaction expiry delta. That reasoning
  still holds even with the above changes.
* `eviction_memory_entries` remains at 40000. It could have been lowered
  given that there will now be at most 80000000/10000 = 8000 transactions
  "in-flight", but it doesn't need to be because the rationale that
  "40000 [RecentlyEvicted queue] entries can be stored in ~1.6 MB,
  which is small compared to other node memory usage" still holds.

Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>
This commit is contained in:
Daira Emma Hopwood 2023-04-16 15:17:05 +01:00
parent 31c0dcf790
commit 7c7e8645bc
3 changed files with 51 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
qa/rpc-tests/mempool_limit.py
View File

@ -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()

51
src/gtest/test_mempoollimit.cpp
View File

@ -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<uint256> 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();

4
src/mempool_limit.h
View File

@ -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