diff options
| -rw-r--r-- | doc/release-notes-27501.md | 3 | ||||
| -rw-r--r-- | src/rpc/mining.cpp | 35 | ||||
| -rw-r--r-- | src/test/fuzz/rpc.cpp | 1 | ||||
| -rw-r--r-- | src/txmempool.cpp | 27 | ||||
| -rw-r--r-- | src/txmempool.h | 13 | ||||
| -rwxr-xr-x | test/functional/mempool_expiry.py | 12 | ||||
| -rwxr-xr-x | test/functional/mining_prioritisetransaction.py | 65 |
7 files changed, 154 insertions, 2 deletions
diff --git a/doc/release-notes-27501.md b/doc/release-notes-27501.md new file mode 100644 index 0000000000..386a00fb34 --- /dev/null +++ b/doc/release-notes-27501.md @@ -0,0 +1,3 @@ +- A new `getprioritisedtransactions` RPC has been added. It returns a map of all fee deltas created by the + user with prioritisetransaction, indexed by txid. The map also indicates whether each transaction is + present in the mempool. diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index eb61d58a33..074cecadd2 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -480,6 +480,40 @@ static RPCHelpMan prioritisetransaction() }; } +static RPCHelpMan getprioritisedtransactions() +{ + return RPCHelpMan{"getprioritisedtransactions", + "Returns a map of all user-created (see prioritisetransaction) fee deltas by txid, and whether the tx is present in mempool.", + {}, + RPCResult{ + RPCResult::Type::OBJ_DYN, "prioritisation-map", "prioritisation keyed by txid", + { + {RPCResult::Type::OBJ, "txid", "", { + {RPCResult::Type::NUM, "fee_delta", "transaction fee delta in satoshis"}, + {RPCResult::Type::BOOL, "in_mempool", "whether this transaction is currently in mempool"}, + }} + }, + }, + RPCExamples{ + HelpExampleCli("getprioritisedtransactions", "") + + HelpExampleRpc("getprioritisedtransactions", "") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + NodeContext& node = EnsureAnyNodeContext(request.context); + CTxMemPool& mempool = EnsureMemPool(node); + UniValue rpc_result{UniValue::VOBJ}; + for (const auto& delta_info : mempool.GetPrioritisedTransactions()) { + UniValue result_inner{UniValue::VOBJ}; + result_inner.pushKV("fee_delta", delta_info.delta); + result_inner.pushKV("in_mempool", delta_info.in_mempool); + rpc_result.pushKV(delta_info.txid.GetHex(), result_inner); + } + return rpc_result; + }, + }; +} + // NOTE: Assumes a conclusive result; if result is inconclusive, it must be handled by caller static UniValue BIP22ValidationResult(const BlockValidationState& state) @@ -1048,6 +1082,7 @@ void RegisterMiningRPCCommands(CRPCTable& t) {"mining", &getnetworkhashps}, {"mining", &getmininginfo}, {"mining", &prioritisetransaction}, + {"mining", &getprioritisedtransactions}, {"mining", &getblocktemplate}, {"mining", &submitblock}, {"mining", &submitheader}, diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 6424f756a0..b1858a1800 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -136,6 +136,7 @@ const std::vector<std::string> RPC_COMMANDS_SAFE_FOR_FUZZING{ "getnetworkinfo", "getnodeaddresses", "getpeerinfo", + "getprioritisedtransactions", "getrawmempool", "getrawtransaction", "getrpcinfo", diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 1ba110d9cb..9ce4a17c5e 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -876,8 +876,17 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD } ++nTransactionsUpdated; } + if (delta == 0) { + mapDeltas.erase(hash); + LogPrintf("PrioritiseTransaction: %s (%sin mempool) delta cleared\n", hash.ToString(), it == mapTx.end() ? "not " : ""); + } else { + LogPrintf("PrioritiseTransaction: %s (%sin mempool) fee += %s, new delta=%s\n", + hash.ToString(), + it == mapTx.end() ? "not " : "", + FormatMoney(nFeeDelta), + FormatMoney(delta)); + } } - LogPrintf("PrioritiseTransaction: %s fee += %s\n", hash.ToString(), FormatMoney(nFeeDelta)); } void CTxMemPool::ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const @@ -896,6 +905,22 @@ void CTxMemPool::ClearPrioritisation(const uint256& hash) mapDeltas.erase(hash); } +std::vector<CTxMemPool::delta_info> CTxMemPool::GetPrioritisedTransactions() const +{ + AssertLockNotHeld(cs); + LOCK(cs); + std::vector<delta_info> result; + result.reserve(mapDeltas.size()); + for (const auto& [txid, delta] : mapDeltas) { + const auto iter{mapTx.find(txid)}; + const bool in_mempool{iter != mapTx.end()}; + std::optional<CAmount> modified_fee; + if (in_mempool) modified_fee = iter->GetModifiedFee(); + result.emplace_back(delta_info{in_mempool, delta, modified_fee, txid}); + } + return result; +} + const CTransaction* CTxMemPool::GetConflictTx(const COutPoint& prevout) const { const auto it = mapNextTx.find(prevout); diff --git a/src/txmempool.h b/src/txmempool.h index 769b7f69ea..000033086b 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -516,6 +516,19 @@ public: void ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const EXCLUSIVE_LOCKS_REQUIRED(cs); void ClearPrioritisation(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs); + struct delta_info { + /** Whether this transaction is in the mempool. */ + const bool in_mempool; + /** The fee delta added using PrioritiseTransaction(). */ + const CAmount delta; + /** The modified fee (base fee + delta) of this entry. Only present if in_mempool=true. */ + std::optional<CAmount> modified_fee; + /** The prioritised transaction's txid. */ + const uint256 txid; + }; + /** Return a vector of all entries in mapDeltas with their corresponding delta_info. */ + std::vector<delta_info> GetPrioritisedTransactions() const EXCLUSIVE_LOCKS_REQUIRED(!cs); + /** Get the transaction in the pool that spends the same prevout */ const CTransaction* GetConflictTx(const COutPoint& prevout) const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/test/functional/mempool_expiry.py b/test/functional/mempool_expiry.py index 15a5f765df..18b3a8def4 100755 --- a/test/functional/mempool_expiry.py +++ b/test/functional/mempool_expiry.py @@ -12,7 +12,10 @@ definable expiry timeout via the '-mempoolexpiry=<n>' command line argument from datetime import timedelta -from test_framework.messages import DEFAULT_MEMPOOL_EXPIRY_HOURS +from test_framework.messages import ( + COIN, + DEFAULT_MEMPOOL_EXPIRY_HOURS, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -37,6 +40,10 @@ class MempoolExpiryTest(BitcoinTestFramework): parent_utxo = self.wallet.get_utxo(txid=parent_txid) independent_utxo = self.wallet.get_utxo() + # Add prioritisation to this transaction to check that it persists after the expiry + node.prioritisetransaction(parent_txid, 0, COIN) + assert_equal(node.getprioritisedtransactions()[parent_txid], { "fee_delta" : COIN, "in_mempool" : True}) + # Ensure the transactions we send to trigger the mempool check spend utxos that are independent of # the transactions being tested for expiration. trigger_utxo1 = self.wallet.get_utxo() @@ -79,6 +86,9 @@ class MempoolExpiryTest(BitcoinTestFramework): assert_raises_rpc_error(-5, 'Transaction not in mempool', node.getmempoolentry, parent_txid) + # Prioritisation does not disappear when transaction expires + assert_equal(node.getprioritisedtransactions()[parent_txid], { "fee_delta" : COIN, "in_mempool" : False}) + # The child transaction should be removed from the mempool as well. self.log.info('Test child tx is evicted as well.') assert_raises_rpc_error(-5, 'Transaction not in mempool', diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index a4481c15a0..099c0e418c 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -30,6 +30,33 @@ class PrioritiseTransactionTest(BitcoinTestFramework): ]] * self.num_nodes self.supports_cli = False + def clear_prioritisation(self, node): + for txid, info in node.getprioritisedtransactions().items(): + delta = info["fee_delta"] + node.prioritisetransaction(txid, 0, -delta) + assert_equal(node.getprioritisedtransactions(), {}) + + def test_replacement(self): + self.log.info("Test tx prioritisation stays after a tx is replaced") + conflicting_input = self.wallet.get_utxo() + tx_replacee = self.wallet.create_self_transfer(utxo_to_spend=conflicting_input, fee_rate=Decimal("0.0001")) + tx_replacement = self.wallet.create_self_transfer(utxo_to_spend=conflicting_input, fee_rate=Decimal("0.005")) + # Add 1 satoshi fee delta to replacee + self.nodes[0].prioritisetransaction(tx_replacee["txid"], 0, 100) + assert_equal(self.nodes[0].getprioritisedtransactions(), { tx_replacee["txid"] : { "fee_delta" : 100, "in_mempool" : False}}) + self.nodes[0].sendrawtransaction(tx_replacee["hex"]) + assert_equal(self.nodes[0].getprioritisedtransactions(), { tx_replacee["txid"] : { "fee_delta" : 100, "in_mempool" : True}}) + self.nodes[0].sendrawtransaction(tx_replacement["hex"]) + assert tx_replacee["txid"] not in self.nodes[0].getrawmempool() + assert_equal(self.nodes[0].getprioritisedtransactions(), { tx_replacee["txid"] : { "fee_delta" : 100, "in_mempool" : False}}) + + # PrioritiseTransaction is additive + self.nodes[0].prioritisetransaction(tx_replacee["txid"], 0, COIN) + self.nodes[0].sendrawtransaction(tx_replacee["hex"]) + assert_equal(self.nodes[0].getprioritisedtransactions(), { tx_replacee["txid"] : { "fee_delta" : COIN + 100, "in_mempool" : True}}) + self.generate(self.nodes[0], 1) + assert_equal(self.nodes[0].getprioritisedtransactions(), {}) + def test_diamond(self): self.log.info("Test diamond-shape package with priority") mock_time = int(time.time()) @@ -84,6 +111,13 @@ class PrioritiseTransactionTest(BitcoinTestFramework): raw_after = self.nodes[0].getrawmempool(verbose=True) assert_equal(raw_before[txid_a], raw_after[txid_a]) assert_equal(raw_before, raw_after) + prioritisation_map_in_mempool = self.nodes[0].getprioritisedtransactions() + assert_equal(prioritisation_map_in_mempool[txid_b], {"fee_delta" : fee_delta_b*COIN, "in_mempool" : True}) + assert_equal(prioritisation_map_in_mempool[txid_c], {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : True}) + # Clear prioritisation, otherwise the transactions' fee deltas are persisted to mempool.dat and loaded again when the node + # is restarted at the end of this subtest. Deltas are removed when a transaction is mined, but only at that time. We do + # not check whether mapDeltas transactions were mined when loading from mempool.dat. + self.clear_prioritisation(node=self.nodes[0]) self.log.info("Test priority while txs are not in mempool") self.restart_node(0, extra_args=["-nopersistmempool"]) @@ -92,17 +126,26 @@ class PrioritiseTransactionTest(BitcoinTestFramework): self.nodes[0].prioritisetransaction(txid=txid_b, fee_delta=int(fee_delta_b * COIN)) self.nodes[0].prioritisetransaction(txid=txid_c, fee_delta=int(fee_delta_c_1 * COIN)) self.nodes[0].prioritisetransaction(txid=txid_c, fee_delta=int(fee_delta_c_2 * COIN)) + prioritisation_map_not_in_mempool = self.nodes[0].getprioritisedtransactions() + assert_equal(prioritisation_map_not_in_mempool[txid_b], {"fee_delta" : fee_delta_b*COIN, "in_mempool" : False}) + assert_equal(prioritisation_map_not_in_mempool[txid_c], {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : False}) for t in [tx_o_a["hex"], tx_o_b["hex"], tx_o_c["hex"], tx_o_d["hex"]]: self.nodes[0].sendrawtransaction(t) raw_after = self.nodes[0].getrawmempool(verbose=True) assert_equal(raw_before[txid_a], raw_after[txid_a]) assert_equal(raw_before, raw_after) + prioritisation_map_in_mempool = self.nodes[0].getprioritisedtransactions() + assert_equal(prioritisation_map_in_mempool[txid_b], {"fee_delta" : fee_delta_b*COIN, "in_mempool" : True}) + assert_equal(prioritisation_map_in_mempool[txid_c], {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : True}) # Clear mempool self.generate(self.nodes[0], 1) + # Prioritisation for transactions is automatically deleted after they are mined. + assert_equal(self.nodes[0].getprioritisedtransactions(), {}) # Use default extra_args self.restart_node(0) + assert_equal(self.nodes[0].getprioritisedtransactions(), {}) def run_test(self): self.wallet = MiniWallet(self.nodes[0]) @@ -115,6 +158,10 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # Test `prioritisetransaction` invalid extra parameters assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction, '', 0, 0, 0) + # Test `getprioritisedtransactions` invalid parameters + assert_raises_rpc_error(-1, "getprioritisedtransactions", + self.nodes[0].getprioritisedtransactions, True) + # Test `prioritisetransaction` invalid `txid` assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].prioritisetransaction, txid='foo', fee_delta=0) assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000')", self.nodes[0].prioritisetransaction, txid='Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000', fee_delta=0) @@ -127,6 +174,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # Test `prioritisetransaction` invalid `fee_delta` assert_raises_rpc_error(-3, "JSON value of type string is not of expected type number", self.nodes[0].prioritisetransaction, txid=txid, fee_delta='foo') + self.test_replacement() self.test_diamond() self.txouts = gen_return_txouts() @@ -165,9 +213,18 @@ class PrioritiseTransactionTest(BitcoinTestFramework): sizes[i] += mempool[j]['vsize'] assert sizes[i] > MAX_BLOCK_WEIGHT // 4 # Fail => raise utxo_count + assert_equal(self.nodes[0].getprioritisedtransactions(), {}) # add a fee delta to something in the cheapest bucket and make sure it gets mined # also check that a different entry in the cheapest bucket is NOT mined self.nodes[0].prioritisetransaction(txid=txids[0][0], fee_delta=int(3*base_fee*COIN)) + assert_equal(self.nodes[0].getprioritisedtransactions(), {txids[0][0] : { "fee_delta" : 3*base_fee*COIN, "in_mempool" : True}}) + + # Priority disappears when prioritisetransaction is called with an inverse value... + self.nodes[0].prioritisetransaction(txid=txids[0][0], fee_delta=int(-3*base_fee*COIN)) + assert txids[0][0] not in self.nodes[0].getprioritisedtransactions() + # ... and reappears when prioritisetransaction is called again. + self.nodes[0].prioritisetransaction(txid=txids[0][0], fee_delta=int(3*base_fee*COIN)) + assert txids[0][0] in self.nodes[0].getprioritisedtransactions() self.generate(self.nodes[0], 1) @@ -187,6 +244,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # Add a prioritisation before a tx is in the mempool (de-prioritising a # high-fee transaction so that it's now low fee). self.nodes[0].prioritisetransaction(txid=high_fee_tx, fee_delta=-int(2*base_fee*COIN)) + assert_equal(self.nodes[0].getprioritisedtransactions()[high_fee_tx], { "fee_delta" : -2*base_fee*COIN, "in_mempool" : False}) # Add everything back to mempool self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) @@ -206,6 +264,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): mempool = self.nodes[0].getrawmempool() self.log.info("Assert that de-prioritised transaction is still in mempool") assert high_fee_tx in mempool + assert_equal(self.nodes[0].getprioritisedtransactions()[high_fee_tx], { "fee_delta" : -2*base_fee*COIN, "in_mempool" : True}) for x in txids[2]: if (x != high_fee_tx): assert x not in mempool @@ -223,10 +282,12 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # to be the minimum for a 1000-byte transaction and check that it is # accepted. self.nodes[0].prioritisetransaction(txid=tx_id, fee_delta=int(self.relayfee*COIN)) + assert_equal(self.nodes[0].getprioritisedtransactions()[tx_id], { "fee_delta" : self.relayfee*COIN, "in_mempool" : False}) self.log.info("Assert that prioritised free transaction is accepted to mempool") assert_equal(self.nodes[0].sendrawtransaction(tx_hex), tx_id) assert tx_id in self.nodes[0].getrawmempool() + assert_equal(self.nodes[0].getprioritisedtransactions()[tx_id], { "fee_delta" : self.relayfee*COIN, "in_mempool" : True}) # Test that calling prioritisetransaction is sufficient to trigger # getblocktemplate to (eventually) return a new block. @@ -234,6 +295,10 @@ class PrioritiseTransactionTest(BitcoinTestFramework): self.nodes[0].setmocktime(mock_time) template = self.nodes[0].getblocktemplate({'rules': ['segwit']}) self.nodes[0].prioritisetransaction(txid=tx_id, fee_delta=-int(self.relayfee*COIN)) + + # Calling prioritisetransaction with the inverse amount should delete its prioritisation entry + assert tx_id not in self.nodes[0].getprioritisedtransactions() + self.nodes[0].setmocktime(mock_time+10) new_template = self.nodes[0].getblocktemplate({'rules': ['segwit']}) |