backport: bitcoin#27501, bitcoin#28542

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.

src/rpc/mining.cpp

@@ -527,6 +527,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 duffs"},
+                    {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)
@@ -1110,6 +1144,7 @@ void RegisterMiningRPCCommands(CRPCTable& t)
         {"mining", &getnetworkhashps},
         {"mining", &getmininginfo},
         {"mining", &prioritisetransaction},
+        {"mining", &getprioritisedtransactions},
         {"mining", &getblocktemplate},
         {"mining", &submitblock},
         {"mining", &submitheader},

src/test/fuzz/rpc.cpp

@@ -136,6 +136,7 @@ const std::vector<std::string> RPC_COMMANDS_SAFE_FOR_FUZZING{
     "getnetworkinfo",
     "getnodeaddresses",
     "getpeerinfo",
+    "getprioritisedtransactions",
     "getrawaddrman",
     "getrawmempool",
     "getrawtransaction",

src/txmempool.cpp

@@ -1806,8 +1806,17 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD
             }
             ++nTransactionsUpdated;
         }
+        if (delta == 0) {
+            mapDeltas.erase(hash);
+            LogPrint(BCLog::MEMPOOL, "PrioritiseTransaction: %s (%sin mempool) delta cleared\n", hash.ToString(), it == mapTx.end() ? "not " : "");
+        } else {
+            LogPrint(BCLog::MEMPOOL, "PrioritiseTransaction: %s (%sin mempool) fee += %s, new delta=%s\n",
+                     hash.ToString(),
+                     it == mapTx.end() ? "not " : "",
+                     FormatMoney(nFeeDelta),
+                     FormatMoney(delta));
+        }
     }
-    LogPrint(BCLog::MEMPOOL, "PrioritiseTransaction: %s fee += %s\n", hash.ToString(), FormatMoney(nFeeDelta));
 }
 
 void CTxMemPool::ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const
@@ -1826,6 +1835,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);

src/txmempool.h

@@ -558,6 +558,19 @@ class CTxMemPool
     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);
 

src/wallet/wallet.cpp

@@ -1374,11 +1374,14 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
 {
     LOCK(cs_wallet);
 
-    int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1;
     // If number of conflict confirms cannot be determined, this means
     // that the block is still unknown or not yet part of the main chain,
     // for example when loading the wallet during a reindex. Do nothing in that
     // case.
+    if (m_last_block_processed_height < 0 || conflicting_height < 0) {
+        return;
+    }
+    int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1;
     if (conflictconfirms >= 0)
         return;
 

test/functional/mempool_expiry.py

@@ -12,7 +12,10 @@
 
 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 @@ def test_transaction_expiry(self, timeout):
         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, 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 @@ def test_transaction_expiry(self, timeout):
         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',

test/functional/mining_prioritisetransaction.py

@@ -29,6 +29,12 @@ def set_test_params(self):
         ]] * 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, -delta)
+        assert_equal(node.getprioritisedtransactions(), {})
+
     def test_diamond(self):
         self.log.info("Test diamond-shape package with priority")
         self.nodes[0].setmocktime(self.mocktime)
@@ -82,6 +88,13 @@ 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})
+        # 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"])
@@ -90,17 +103,26 @@ 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})
         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])
@@ -112,6 +134,10 @@ def run_test(self):
         # Test `prioritisetransaction` invalid extra parameters
         assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction, '', 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)
@@ -159,9 +185,18 @@ def run_test(self):
                 sizes[i] += mempool[j]['vsize']
             assert sizes[i] > MAX_BLOCK_SIZE  # 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(txids[0][0], 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)
 
@@ -181,6 +216,7 @@ def run_test(self):
         # 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(high_fee_tx, -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())
@@ -200,6 +236,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})
         for x in txids[2]:
             if (x != high_fee_tx):
                 assert x not in mempool
@@ -217,16 +254,22 @@ def run_test(self):
         # to be the minimum for a 1000-byte transaction and check that it is
         # accepted.
         self.nodes[0].prioritisetransaction(tx_id, 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.
         self.nodes[0].setmocktime(self.mocktime)
         template = self.nodes[0].getblocktemplate()
         self.nodes[0].prioritisetransaction(tx_id, -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(self.mocktime+10)
         new_template = self.nodes[0].getblocktemplate()
 

test/functional/tool_wallet.py

@@ -402,6 +402,65 @@ def test_dump_createfromdump(self):
         self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
         assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "regtest/wallets", "badload"))
 
+    def test_chainless_conflicts(self):
+        self.log.info("Test wallet tool when wallet contains conflicting transactions")
+        self.restart_node(0)
+        self.generate(self.nodes[0], 101)
+
+        def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        self.nodes[0].createwallet("conflicts")
+        wallet = self.nodes[0].get_wallet_rpc("conflicts")
+        def_wallet.sendtoaddress(wallet.getnewaddress(), 10)
+        self.generate(self.nodes[0], 1)
+
+        # parent tx
+        parent_txid = wallet.sendtoaddress(wallet.getnewaddress(), 9)
+        parent_txid_bytes = bytes.fromhex(parent_txid)[::-1]
+        conflict_utxo = wallet.gettransaction(txid=parent_txid, verbose=True)["decoded"]["vin"][0]
+
+        # The specific assertion in MarkConflicted being tested requires that the parent tx is already loaded
+        # by the time the child tx is loaded. Since transactions end up being loaded in txid order due to how both
+        # and sqlite store things, we can just grind the child tx until it has a txid that is greater than the parent's.
+        locktime = 500000000 # Use locktime as nonce, starting at unix timestamp minimum
+        addr = wallet.getnewaddress()
+        while True:
+            child_send_res = wallet.send(outputs=[{addr: 8}], options={"add_to_wallet": False, "locktime": locktime})
+            child_txid = child_send_res["txid"]
+            child_txid_bytes = bytes.fromhex(child_txid)[::-1]
+            if (child_txid_bytes > parent_txid_bytes):
+                wallet.sendrawtransaction(child_send_res["hex"])
+                break
+            locktime += 1
+
+        # conflict with parent
+        conflict_unsigned = self.nodes[0].createrawtransaction(inputs=[conflict_utxo], outputs=[{wallet.getnewaddress(): 9.9999}])
+        conflict_signed = wallet.signrawtransactionwithwallet(conflict_unsigned)["hex"]
+        conflict_txid = self.nodes[0].decoderawtransaction(conflict_signed)["txid"]
+        # Dash has no RBF, so the conflicting tx cannot be broadcast while the parent is in
+        # the mempool. Mine it directly into a block instead, which conflicts the parent and
+        # the child just the same.
+        self.generateblock(self.nodes[0], output=def_wallet.getnewaddress(), transactions=[conflict_signed])
+        assert_equal(wallet.gettransaction(txid=parent_txid)["confirmations"], -1)
+        assert_equal(wallet.gettransaction(txid=child_txid)["confirmations"], -1)
+        assert_equal(wallet.gettransaction(txid=conflict_txid)["confirmations"], 1)
+
+        self.stop_node(0)
+
+        # Wallet tool should successfully give info for this wallet
+        expected_output = textwrap.dedent(f'''\
+            Wallet info
+            ===========
+            Name: conflicts
+            Format: {"sqlite" if self.options.descriptors else "bdb"}
+            Descriptors: {"yes" if self.options.descriptors else "no"}
+            Encrypted: no
+            HD (hd seed available): yes
+            Keypool Size: {"2" if self.options.descriptors else "1"}
+            Transactions: 4
+            Address Book: 4
+        ''')
+        self.assert_tool_output(expected_output, "-wallet=conflicts", "info")
 
     def test_nonhd(self):
         self.log.info('Check non-hd wallet')
@@ -427,6 +486,7 @@ def run_test(self):
             self.test_wipe()
             self.test_nonhd()
         self.test_dump_createfromdump()
+        self.test_chainless_conflicts()
 
 if __name__ == '__main__':
     ToolWalletTest().main()