Skip to content

refactor: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0 #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 7, 2024
Merged

refactor: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0 #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3a118e1
test: Directly constructing 2 entry map for getprioritisedtransactions
kevkevinpal Nov 13, 2023
2fca6c2
rpc: changed prioritisation-map -> ""
kevkevinpal Nov 13, 2023
252a867
rpc: renaming txid -> transactionid
kevkevinpal Jan 9, 2024
cfdbcd1
rpc: exposing modified_fee in getprioritisedtransactions
kevkevinpal Jan 9, 2024
0eebd6f
test: Assert that a new tx with a delta of 0 is never added
kevkevinpal Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/rpc/mining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,11 +487,12 @@ static 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_DYN, "", "prioritisation keyed by txid",
{
{RPCResult::Type::OBJ, "txid", "", {
{RPCResult::Type::OBJ, "<transactionid>", "", {
{RPCResult::Type::NUM, "fee_delta", "transaction fee delta in satoshis"},
{RPCResult::Type::BOOL, "in_mempool", "whether this transaction is currently in mempool"},
{RPCResult::Type::NUM, "modified_fee", /*optional=*/true, "modified fee in satoshis. Only returned if in_mempool=true"},
}}
},
},
Expand All @@ -508,6 +509,9 @@ static RPCHelpMan getprioritisedtransactions()
UniValue result_inner{UniValue::VOBJ};
result_inner.pushKV("fee_delta", delta_info.delta);
result_inner.pushKV("in_mempool", delta_info.in_mempool);
if (delta_info.in_mempool) {
result_inner.pushKV("modified_fee", *delta_info.modified_fee);
}
rpc_result.pushKV(delta_info.txid.GetHex(), result_inner);
}
return rpc_result;
Expand Down
5 changes: 3 additions & 2 deletions test/functional/mempool_expiry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ def test_transaction_expiry(self, timeout):
node = self.nodes[0]

# Send a parent transaction that will expire.
parent_txid = self.wallet.send_self_transfer(from_node=node)['txid']
parent = self.wallet.send_self_transfer(from_node=node)
parent_txid = parent["txid"]
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})
assert_equal(node.getprioritisedtransactions()[parent_txid], { "fee_delta" : COIN, "in_mempool" : True, "modified_fee": COIN + COIN * parent["fee"] })

# Ensure the transactions we send to trigger the mempool check spend utxos that are independent of
# the transactions being tested for expiration.
Expand Down
28 changes: 14 additions & 14 deletions test/functional/mining_prioritisetransaction.py
View file
Open in desktop
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": int(tx_replacee["fee"] * COIN + 100)}})
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": int(tx_replacee["fee"] * COIN + COIN + 100)}})
self.generate(self.nodes[0], 1)
assert_equal(self.nodes[0].getprioritisedtransactions(), {})

Expand Down Expand Up @@ -111,9 +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)
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})
assert_equal(self.nodes[0].getprioritisedtransactions(), {txid_b: {"fee_delta" : fee_delta_b*COIN, "in_mempool" : True, "modified_fee": int(fee_delta_b*COIN + COIN * tx_o_b["fee"])}, txid_c: {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : True, "modified_fee": int((fee_delta_c_1 + fee_delta_c_2 ) * COIN + COIN * tx_o_c["fee"])}})
# 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 @@ -126,17 +124,13 @@ def test_diamond(self):
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})
assert_equal(self.nodes[0].getprioritisedtransactions(), {txid_b: {"fee_delta" : fee_delta_b*COIN, "in_mempool" : False}, 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})
assert_equal(self.nodes[0].getprioritisedtransactions(), {txid_b: {"fee_delta" : fee_delta_b*COIN, "in_mempool" : True, "modified_fee": int(fee_delta_b*COIN + COIN * tx_o_b["fee"])}, txid_c: {"fee_delta" : (fee_delta_c_1 + fee_delta_c_2)*COIN, "in_mempool" : True, "modified_fee": int((fee_delta_c_1 + fee_delta_c_2 ) * COIN + COIN * tx_o_c["fee"])}})

# Clear mempool
self.generate(self.nodes[0], 1)
Expand Down Expand Up @@ -217,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": int(3*base_fee*COIN + COIN * 1 * base_fee)}})

# 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 @@ -264,11 +258,17 @@ 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": int(-2*base_fee*COIN + COIN * 3 * base_fee)})
for x in txids[2]:
if (x != high_fee_tx):
assert x not in mempool


self.log.info("Assert that 0 delta is never added to mapDeltas")
tx_id_zero_del = self.wallet.create_self_transfer()['txid']
self.nodes[0].prioritisetransaction(txid=tx_id_zero_del, fee_delta=0)
assert tx_id_zero_del not in self.nodes[0].getprioritisedtransactions()

# Create a free transaction. Should be rejected.
tx_res = self.wallet.create_self_transfer(fee_rate=0)
tx_hex = tx_res['hex']
Expand All @@ -287,7 +287,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": int(self.relayfee*COIN + COIN * tx_res["fee"])})

# Test that calling prioritisetransaction is sufficient to trigger
# getblocktemplate to (eventually) return a new block.
Expand Down