highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-26 17:52:13 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#28885: mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0

Browse Source 
0eebd6fe7d01ddc7f6b7f13a6ed6e705c7aeae4e test: Assert that a new tx with a delta of 0 is never added (kevkevin)
cfdbcd19b32fd63954d7947dcc639aef291fb6b2 rpc: exposing modified_fee in getprioritisedtransactions (kevkevin)
252a86729a15e47ed168d8da7c4a8d6113673909 rpc: renaming txid -> transactionid (kevkevin)
2fca6c2dd03c3955d86efb0b8d2a7961e42115fd rpc: changed prioritisation-map -> "" (kevkevin)
3a118e19e100110300d3290d4c1434f963721d94 test: Directly constructing 2 entry map for getprioritisedtransactions (kevkevin)

Pull request description:

  In this PR I am addressing some comments in https://github.com/bitcoin/bitcoin/pull/27501 as a followup.
  - changed `prioritisation-map` in the `RPCResult` to `""`
  - Directly constructing 2 entry map for getprioritisedtransactions in functional tests
  - renamed `txid` to `transactionid` in `RPCResult` to be more consistent with naming elsewhere
  - exposed the `modified_fee` field instead of having it be a useless arg
  - Created a new test that asserts when `prioritisedtransaction` is called with a fee_delta of 0 it is not added to mempool

ACKs for top commit:
  glozow:
    reACK 0eebd6fe7d01ddc7f6b7f13a6ed6e705c7aeae4e, only change is the doc suggestion

Tree-SHA512: e99056e37a8b1cfc511d87c83edba7c928b50d2cd6c2fd7c038976779850677ad37fddeb2b983e8bc007ca8567eb21ebb78d7eae9b773657c2b297299993ec05
This commit is contained in:
glozow 2024-01-12 11:51:44 +00:00
commit cd603361a4
No known key found for this signature in database
GPG Key ID: BA03F4DBE0C63FB4
3 changed files with 23 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/rpc/mining.cpp
View File

@ -495,11 +495,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"},
}}
},
},
@ -516,6 +517,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;

5
test/functional/mempool_expiry.py
View File

@ -36,13 +36,14 @@ class MempoolExpiryTest(BitcoinTestFramework):
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.

28
test/functional/mining_prioritisetransaction.py
View File

@ -45,7 +45,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework):
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}})
@ -53,7 +53,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework):
# 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(), {})
@ -111,9 +111,7 @@ 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})
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.
@ -126,17 +124,13 @@ 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})
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)
@ -217,7 +211,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework):
# 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))
@ -264,11 +258,17 @@ 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})
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']
@ -287,7 +287,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework):
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.