From 0a9a69e702859510c39452e820b280154b960427 Mon Sep 17 00:00:00 2001 From: kevkevin Date: Mon, 13 Nov 2023 16:17:07 -0600 Subject: [PATCH] refactor: exposing modified_fee in getprioritisedtransactions Instead of having modified_fee be hidden we are now exposing it to avoid having useless code --- src/rpc/mining.cpp | 6 ++++++ test/functional/mempool_expiry.py | 2 +- test/functional/mining_prioritisetransaction.py | 14 +++++++------- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index c06b28024f040e..805ee96c957bc6 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -491,6 +491,7 @@ static RPCHelpMan getprioritisedtransactions() { {RPCResult::Type::OBJ, "transactionid", "", { {RPCResult::Type::NUM, "fee_delta", "transaction fee delta in satoshis"}, + {RPCResult::Type::NUM, "modified_fee", /*optional=*/true, "new fee for this transactionid if in mempool"}, {RPCResult::Type::BOOL, "in_mempool", "whether this transaction is currently in mempool"}, }} }, @@ -507,7 +508,12 @@ static RPCHelpMan getprioritisedtransactions() for (const auto& delta_info : mempool.GetPrioritisedTransactions()) { UniValue result_inner{UniValue::VOBJ}; result_inner.pushKV("fee_delta", delta_info.delta); + CAmount modified_fee; result_inner.pushKV("in_mempool", delta_info.in_mempool); + if(delta_info.in_mempool){ + modified_fee = *delta_info.modified_fee; + result_inner.pushKV("modified_fee", (uint64_t)modified_fee); + } rpc_result.pushKV(delta_info.txid.GetHex(), result_inner); } return rpc_result; diff --git a/test/functional/mempool_expiry.py b/test/functional/mempool_expiry.py index 18b3a8def43d81..b10f44960419cc 100755 --- a/test/functional/mempool_expiry.py +++ b/test/functional/mempool_expiry.py @@ -42,7 +42,7 @@ def test_transaction_expiry(self, timeout): # 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}) + assert_equal(node.getprioritisedtransactions()[parent_txid], { "fee_delta" : COIN, "in_mempool" : True, "modified_fee": 100031200}) # Ensure the transactions we send to trigger the mempool check spend utxos that are independent of # the transactions being tested for expiration. diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 12be53af6b6dec..66c07bdde8d9d7 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -45,7 +45,7 @@ def test_replacement(self): 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}}) + assert_equal(self.nodes[0].getprioritisedtransactions(), { tx_replacee["txid"] : { "fee_delta" : 100, "in_mempool" : True, "modified_fee": 1140}}) 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}}) @@ -53,7 +53,7 @@ def test_replacement(self): # 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}}) + assert_equal(self.nodes[0].getprioritisedtransactions(), { tx_replacee["txid"] : { "fee_delta" : COIN + 100, "in_mempool" : True, "modified_fee": 100001140}}) self.generate(self.nodes[0], 1) assert_equal(self.nodes[0].getprioritisedtransactions(), {}) @@ -111,7 +111,7 @@ def test_diamond(self): raw_after = self.nodes[0].getrawmempool(verbose=True) assert_equal(raw_before[txid_a], raw_after[txid_a]) assert_equal(raw_before, raw_after) - assert_equal(self.nodes[0].getprioritisedtransactions(), {txid_b: {"fee_delta" : fee_delta_b*COIN, "in_mempool" : True}, txid_c: {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : True}}) + assert_equal(self.nodes[0].getprioritisedtransactions(), {txid_b: {"fee_delta" : fee_delta_b*COIN, "in_mempool" : True, "modified_fee": 41199}, txid_c: {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : True, "modified_fee": 38854}}) # 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. @@ -130,7 +130,7 @@ def test_diamond(self): raw_after = self.nodes[0].getrawmempool(verbose=True) assert_equal(raw_before[txid_a], raw_after[txid_a]) assert_equal(raw_before, raw_after) - assert_equal(self.nodes[0].getprioritisedtransactions(), {txid_b: {"fee_delta" : fee_delta_b*COIN, "in_mempool" : True}, txid_c: {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : True}}) + assert_equal(self.nodes[0].getprioritisedtransactions(), {txid_b: {"fee_delta" : fee_delta_b*COIN, "in_mempool" : True, "modified_fee": 41199}, txid_c: {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : True, "modified_fee": 38854}}) # Clear mempool self.generate(self.nodes[0], 1) @@ -211,7 +211,7 @@ def run_test(self): # 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}}) + assert_equal(self.nodes[0].getprioritisedtransactions(), {txids[0][0] : { "fee_delta" : 3*base_fee*COIN, "in_mempool" : True, "modified_fee": 400000}}) # 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)) @@ -258,7 +258,7 @@ def run_test(self): 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}) + assert_equal(self.nodes[0].getprioritisedtransactions()[high_fee_tx], { "fee_delta" : -2*base_fee*COIN, "in_mempool" : True, "modified_fee": 100000}) for x in txids[2]: if (x != high_fee_tx): assert x not in mempool @@ -281,7 +281,7 @@ def run_test(self): 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}) + assert_equal(self.nodes[0].getprioritisedtransactions()[tx_id], { "fee_delta" : self.relayfee*COIN, "in_mempool" : True, "modified_fee": 1000}) # Test that calling prioritisetransaction is sufficient to trigger # getblocktemplate to (eventually) return a new block.