Skip to content

Commit

Permalink
refactor: exposing modified_fee in getprioritisedtransactions
Browse files Browse the repository at this point in the history 
Instead of having modified_fee be hidden we are now exposing it to avoid
having useless code
  • Loading branch information
@kevkevinpal
kevkevinpal committed Nov 22, 2023
1 parent cf848d7 commit 0a9a69e
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
6 changes: 6 additions & 0 deletions src/rpc/mining.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
}}
},
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/functional/mempool_expiry.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 7 additions & 7 deletions test/functional/mining_prioritisetransaction.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ 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}})

# 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(), {})

Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down

0 comments on commit 0a9a69e

Please sign in to comment.