From bde74425262c05652d871994a66be901fb654036 Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Sat, 25 Sep 2021 17:52:03 +0200 Subject: [PATCH 1/2] mempool: tune incremental fee to make more sense for Dogecoin The value of DEFAULT_INCREMENTAL_RELAY_FEE has never been tuned for Dogecoin since porting from Bitcoin Core 0.14. Even though the dual meaning of this parameter is suboptimal, it can still be tuned. This commit sets the value to 1/10th of DEFAULT_MIN_RELAY_TX_FEE from validation.h, which causes: 1. Mempool limiting to be performed in steps of 0.0001 DOGE/kb instead of 0.00001 DOGE/kb 2. RBF to be accepted by the mempool if the new fee is at least 0.0001 DOGE/kb higher than the previous fee known to the mempool 3. RBF to be cheaper than CPFP by a factor 10 (as the latter would require a fee of more than 0.001 DOGE/kb on a subsequent bumping transaction), to encourage mempool replacement over prioritizing through additional transactions that need to be mined. 4. Mempool limiting to be 10x faster to reset to zero than before, because for bitcoin, fee increments equaled their minimum fee, but for us this was 1/100th. mempool_tests.cpp has been reworked a bit to reflect the reality of having a lower increment than the minimum fee, as even though this already was the case, this was not tested correctly due to the static values in the unit test. --- src/policy/policy.h | 18 +++++++++++++-- src/test/mempool_tests.cpp | 46 +++++++++++++++++++------------------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/policy/policy.h b/src/policy/policy.h index ad5fbdabc..619fab69a 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -37,8 +37,22 @@ static const unsigned int MAX_P2SH_SIGOPS = 15; static const unsigned int MAX_STANDARD_TX_SIGOPS_COST = MAX_BLOCK_SIGOPS_COST/5; /** Default for -maxmempool, maximum megabytes of mempool memory usage */ static const unsigned int DEFAULT_MAX_MEMPOOL_SIZE = 300; -/** Default for -incrementalrelayfee, which sets the minimum feerate increase for mempool limiting or BIP 125 replacement **/ -static const CAmount DEFAULT_INCREMENTAL_RELAY_FEE = RECOMMENDED_MIN_TX_FEE / 1000; +/** Default for -incrementalrelayfee, which sets the minimum feerate increase + * for mempool limiting or BIP 125 replacement + * + * Dogecoin: Increment mempool limits and accept RBF in steps of 0.0001 DOGE + * Calculation: DEFAULT_MIN_RELAY_TX_FEE = RECOMMENDED_MIN_TX_FEE / 10 + * DEFAULT_INCREMENTAL_RELAY_FEE = DEFAULT_MIN_RELAY_TX_FEE / 10 + * + * Rationale: This implements a smaller granularity than the wallet + * implementation for fee increments by default, leaving room for + * alternative increment strategies, yet limiting the amount of + * ineffective RBF spam we expose the network to. This also makes + * an RBF fee bump 10x cheaper than a CPFP transaction, because + * RBF leaves no on-chain waste, whereas CPFP adds another + * transaction to the chain. + */ +static const CAmount DEFAULT_INCREMENTAL_RELAY_FEE = RECOMMENDED_MIN_TX_FEE / 100; /** Default for -bytespersigop */ static const unsigned int DEFAULT_BYTES_PER_SIGOP = 20; /** The maximum number of witness stack items in a standard P2WSH script */ diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 91f549fe4..32f24d46b 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -432,7 +432,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) { - CTxMemPool pool(CFeeRate(1000)); + CTxMemPool pool(CFeeRate(COIN / 1000)); TestMemPoolEntryHelper entry; entry.dPriority = 10.0; @@ -442,7 +442,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx1.vout.resize(1); tx1.vout[0].scriptPubKey = CScript() << OP_1 << OP_EQUAL; tx1.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).FromTx(tx1, &pool)); + pool.addUnchecked(tx1.GetHash(), entry.Fee(COIN / 100).FromTx(tx1, &pool)); CMutableTransaction tx2 = CMutableTransaction(); tx2.vin.resize(1); @@ -450,7 +450,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx2.vout.resize(1); tx2.vout[0].scriptPubKey = CScript() << OP_2 << OP_EQUAL; tx2.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx2.GetHash(), entry.Fee(5000LL).FromTx(tx2, &pool)); + pool.addUnchecked(tx2.GetHash(), entry.Fee(COIN / 200).FromTx(tx2, &pool)); pool.TrimToSize(pool.DynamicMemoryUsage()); // should do nothing BOOST_CHECK(pool.exists(tx1.GetHash())); @@ -468,7 +468,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx3.vout.resize(1); tx3.vout[0].scriptPubKey = CScript() << OP_3 << OP_EQUAL; tx3.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx3.GetHash(), entry.Fee(20000LL).FromTx(tx3, &pool)); + pool.addUnchecked(tx3.GetHash(), entry.Fee(COIN / 50).FromTx(tx3, &pool)); pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // tx3 should pay for tx2 (CPFP) BOOST_CHECK(!pool.exists(tx1.GetHash())); @@ -480,8 +480,8 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(!pool.exists(tx2.GetHash())); BOOST_CHECK(!pool.exists(tx3.GetHash())); - CFeeRate maxFeeRateRemoved(25000, GetVirtualTransactionSize(tx3) + GetVirtualTransactionSize(tx2)); - BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000); + CFeeRate maxFeeRateRemoved(COIN / 1000 * 25, GetVirtualTransactionSize(tx3) + GetVirtualTransactionSize(tx2)); + BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + DEFAULT_INCREMENTAL_RELAY_FEE); CMutableTransaction tx4 = CMutableTransaction(); tx4.vin.resize(2); @@ -531,10 +531,10 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx7.vout[1].scriptPubKey = CScript() << OP_7 << OP_EQUAL; tx7.vout[1].nValue = 10 * COIN; - pool.addUnchecked(tx4.GetHash(), entry.Fee(7000LL).FromTx(tx4, &pool)); - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5, &pool)); - pool.addUnchecked(tx6.GetHash(), entry.Fee(1100LL).FromTx(tx6, &pool)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); + pool.addUnchecked(tx4.GetHash(), entry.Fee(COIN / 1000 * 7).FromTx(tx4, &pool)); + pool.addUnchecked(tx5.GetHash(), entry.Fee(COIN / 1000).FromTx(tx5, &pool)); + pool.addUnchecked(tx6.GetHash(), entry.Fee(COIN / 10000 * 11).FromTx(tx6, &pool)); + pool.addUnchecked(tx7.GetHash(), entry.Fee(COIN / 1000 * 9).FromTx(tx7, &pool)); // we only require this remove, at max, 2 txn, because its not clear what we're really optimizing for aside from that pool.TrimToSize(pool.DynamicMemoryUsage() - 1); @@ -543,8 +543,8 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(!pool.exists(tx7.GetHash())); if (!pool.exists(tx5.GetHash())) - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5, &pool)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); + pool.addUnchecked(tx5.GetHash(), entry.Fee(COIN / 1000).FromTx(tx5, &pool)); + pool.addUnchecked(tx7.GetHash(), entry.Fee(COIN / 1000 * 9).FromTx(tx7, &pool)); pool.TrimToSize(pool.DynamicMemoryUsage() / 2); // should maximize mempool size by only removing 5/7 BOOST_CHECK(pool.exists(tx4.GetHash())); @@ -552,34 +552,34 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(pool.exists(tx6.GetHash())); BOOST_CHECK(!pool.exists(tx7.GetHash())); - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5, &pool)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); + pool.addUnchecked(tx5.GetHash(), entry.Fee(COIN / 1000).FromTx(tx5, &pool)); + pool.addUnchecked(tx7.GetHash(), entry.Fee(COIN / 1000 * 9).FromTx(tx7, &pool)); std::vector vtx; SetMockTime(42); SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE); - BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000); + BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + DEFAULT_INCREMENTAL_RELAY_FEE); // ... we should keep the same min fee until we get a block pool.removeForBlock(vtx, 1); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE); - BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/2); + BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + DEFAULT_INCREMENTAL_RELAY_FEE)/2); // ... then feerate should drop 1/2 each halflife SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2); - BOOST_CHECK_EQUAL(pool.GetMinFee(pool.DynamicMemoryUsage() * 5 / 2).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/4); + BOOST_CHECK_EQUAL(pool.GetMinFee(pool.DynamicMemoryUsage() * 5 / 2).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + DEFAULT_INCREMENTAL_RELAY_FEE)/4); // ... with a 1/2 halflife when mempool is < 1/2 its target size SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4); - BOOST_CHECK_EQUAL(pool.GetMinFee(pool.DynamicMemoryUsage() * 9 / 2).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/8); + BOOST_CHECK_EQUAL(pool.GetMinFee(pool.DynamicMemoryUsage() * 9 / 2).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + DEFAULT_INCREMENTAL_RELAY_FEE)/8); // ... with a 1/4 halflife when mempool is < 1/4 its target size - SetMockTime(42 + 7*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4); - BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), 1000); - // ... but feerate should never drop below 1000 + SetMockTime(42 + 10*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4); + BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), DEFAULT_INCREMENTAL_RELAY_FEE); + // ... but feerate should never drop below DEFAULT_INCREMENTAL_RELAY_FEE - SetMockTime(42 + 8*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4); + SetMockTime(42 + 11*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), 0); - // ... unless it has gone all the way to 0 (after getting past 1000/2) + // ... unless it has gone all the way to 0 (after getting past DEFAULT_INCREMENTAL_RELAY_FEE/2) SetMockTime(0); } From c31ff0f0956c418ac80c9d847b2844e7d6e93b0c Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Sat, 25 Sep 2021 20:01:34 +0200 Subject: [PATCH 2/2] wallet: change incremental fee for RBF Sets WALLET_INCREMENTAL_RELAY_FEE to 1/10th of the default recommended minimum fee, to encourage using RBF over CPFP and saving the additional blockspace required for child transactions spending parent transactions. The previous value was based off a 1 DOGE recommended fee and did not make sense with the current minimum fee recommendation. --- qa/rpc-tests/bumpfee.py | 12 ++++++------ src/wallet/wallet.h | 10 +++++++++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/qa/rpc-tests/bumpfee.py b/qa/rpc-tests/bumpfee.py index 1b6feb1c1..65aea4508 100755 --- a/qa/rpc-tests/bumpfee.py +++ b/qa/rpc-tests/bumpfee.py @@ -202,7 +202,7 @@ def test_settxfee(rbf_node, dest_address): rbftx = rbf_node.gettransaction(rbfid) rbf_node.settxfee(Decimal("5.00000000")) bumped_tx = rbf_node.bumpfee(rbfid) - assert_equal(bumped_tx["fee"], abs(rbftx["fee"]) + Decimal("0.50000000")) + assert_equal(bumped_tx["fee"], abs(rbftx["fee"]) + Decimal("0.00100000")) rbf_node.settxfee(Decimal("0.00000000")) # unset paytxfee @@ -210,18 +210,18 @@ def test_rebumping(rbf_node, dest_address): # check that re-bumping the original tx fails, but bumping the bumper succeeds rbf_node.settxfee(Decimal("10.00000000")) rbfid = create_fund_sign_send(rbf_node, {dest_address: 8.00000000}) - bumped = rbf_node.bumpfee(rbfid, {"totalFee": 1050000000}) - assert_raises_jsonrpc(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 11000}) - rbf_node.bumpfee(bumped["txid"], {"totalFee": 1100000000}) + bumped = rbf_node.bumpfee(rbfid, {"totalFee": 1000100000}) + assert_raises_jsonrpc(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 1000100000}) + rbf_node.bumpfee(bumped["txid"], {"totalFee": 1000200000}) rbf_node.settxfee(Decimal("0.00000000")) def test_rebumping_not_replaceable(rbf_node, dest_address): # check that re-bumping a non-replaceable bump tx fails rbfid = create_fund_sign_send(rbf_node, {dest_address: 7.00000000}) - bumped = rbf_node.bumpfee(rbfid, {"totalFee": 150000000, "replaceable": False}) + bumped = rbf_node.bumpfee(rbfid, {"totalFee": 100100000, "replaceable": False}) assert_raises_jsonrpc(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"], - {"totalFee": 200000000}) + {"totalFee": 100200000}) def test_unconfirmed_not_spendable(rbf_node, rbf_node_address): diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 97157819b..f83695e6d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -57,7 +57,15 @@ static const CAmount DEFAULT_FALLBACK_FEE = RECOMMENDED_MIN_TX_FEE; //! -mintxfee default static const CAmount DEFAULT_TRANSACTION_MINFEE = RECOMMENDED_MIN_TX_FEE; //! minimum recommended increment for BIP 125 replacement txs -static const CAmount WALLET_INCREMENTAL_RELAY_FEE = RECOMMENDED_MIN_TX_FEE * 50; +/* + * Dogecoin: Scaled to 1/10th of the recommended transaction fee to make RBF + * cheaper than CPFP. This reduces onchain pollution by encouraging transactions + * to be replaced in the mempool, rather than be respent by another transaction + * which then both would have to be mined, taking up block space and increasing + * the amount of data that needs to be synchronized when validating the chain. + * This way, replacements for fee bumps are transient rather than persisted. + */ +static const CAmount WALLET_INCREMENTAL_RELAY_FEE = RECOMMENDED_MIN_TX_FEE / 10; //! target minimum change amount static const CAmount MIN_CHANGE = RECOMMENDED_MIN_TX_FEE; //! final minimum change amount after paying for fees