diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4efd4f73857f6f..8da19812f1a636 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1476,34 +1476,16 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, const Mem } } + auto* conflict_reason = std::get_if(&reason); + // Handle transactions that were removed from the mempool because they // conflict with transactions in a newly connected block. - if (std::get_if(&reason)) { - // Trigger external -walletnotify notifications for these transactions. - // Set Status::UNCONFIRMED instead of Status::CONFLICTED for a few reasons: - // - // 1. The transactionRemovedFromMempool callback does not currently - // provide the conflicting block's hash and height, and for backwards - // compatibility reasons it may not be not safe to store conflicted - // wallet transactions with a null block hash. See - // https://github.com/bitcoin/bitcoin/pull/18600#discussion_r420195993. - // 2. For most of these transactions, the wallet's internal conflict - // detection in the blockConnected handler will subsequently call - // MarkConflicted and update them with CONFLICTED status anyway. This - // applies to any wallet transaction that has inputs spent in the - // block, or that has ancestors in the wallet with inputs spent by - // the block. - // 3. Longstanding behavior since the sync implementation in - // https://github.com/bitcoin/bitcoin/pull/9371 and the prior sync - // implementation before that was to mark these transactions - // unconfirmed rather than conflicted. - // - // Nothing described above should be seen as an unchangeable requirement - // when improving this code in the future. The wallet's heuristics for + if (conflict_reason != nullptr && IsFromMe(*tx)) { + // The wallet's heuristics for // distinguishing between conflicted and unconfirmed transactions are // imperfect, and could be improved in general, see // https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking - SyncTransaction(tx, TxStateInactive{}); + SyncTransaction(tx, TxStateBlockConflicted(conflict_reason->conflicting_block_hash, conflict_reason->conflicting_block_height)); } const Txid& txid = tx->GetHash(); diff --git a/test/functional/wallet_conflicts.py b/test/functional/wallet_conflicts.py index dbf435ab6d9637..f5ee7fe97c6aa4 100755 --- a/test/functional/wallet_conflicts.py +++ b/test/functional/wallet_conflicts.py @@ -10,6 +10,7 @@ from decimal import Decimal from test_framework.blocktools import COINBASE_MATURITY +from test_framework.messages import COIN from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -428,56 +429,77 @@ def test_descendants_with_mempool_conflicts(self): def test_unknown_parent_conflict(self): self.log.info("Test that a conflict with parent not belonging to the wallet causes in-wallet children to also conflict") + self.nodes[0].createwallet("parent_conflict") + self.nodes[1].createwallet("external_wallet_conflict") wallet = self.nodes[0].get_wallet_rpc("parent_conflict") + external_wallet = self.nodes[1].get_wallet_rpc("external_wallet_conflict") def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) - # one utxo to target wallet - addr = wallet.getnewaddress() - addr_desc = wallet.getaddressinfo(addr)["desc"] - def_wallet.sendtoaddress(addr, 10) - self.generate(self.nodes[0], 1) - utxo = wallet.listunspent()[0] - - # Make utxo in grandparent that will be double spent - gp_addr = def_wallet.getnewaddress() - gp_txid = def_wallet.sendtoaddress(gp_addr, 7) - gp_utxo = {"txid": gp_txid, "vout": find_vout_for_address(self.nodes[0], gp_txid, gp_addr)} - self.generate(self.nodes[0], 1) - - assert_equal(self.nodes[0].getrawmempool(), []) - - # make unconfirmed parent tx - parent_addr = def_wallet.getnewaddress() - parent_txid = def_wallet.send(outputs=[{parent_addr: 5}], inputs=[gp_utxo])["txid"] - parent_utxo = {"txid": parent_txid, "vout": find_vout_for_address(self.nodes[0], parent_txid, parent_addr)} - parent_me = self.nodes[0].getmempoolentry(parent_txid) - parent_feerate = parent_me["fees"]["base"] * 100000000 / parent_me["vsize"] - assert_equal(self.nodes[0].getrawmempool(), [parent_txid]) - - # make child tx that has one input belonging to target wallet, and one input not - psbt = def_wallet.walletcreatefundedpsbt(inputs=[utxo, parent_utxo], outputs=[{def_wallet.getnewaddress(): 13}], solving_data={"descriptors":[addr_desc]})["psbt"] - psbt = def_wallet.walletprocesspsbt(psbt)["psbt"] - psbt_proc = wallet.walletprocesspsbt(psbt) - psbt = psbt_proc["psbt"] - assert_equal(psbt_proc["complete"], True) - child_txid = self.nodes[0].sendrawtransaction(psbt_proc["hex"]) - assert_equal(set(self.nodes[0].getrawmempool()), {child_txid, parent_txid}) - - assert_equal(def_wallet.gettransaction(parent_txid)["confirmations"], 0) - assert_equal(wallet.gettransaction(child_txid)["confirmations"], 0) - - # Make a conflict spending parent - conflict_psbt = def_wallet.walletcreatefundedpsbt(inputs=[gp_utxo], outputs=[{def_wallet.getnewaddress(): 2}], fee_rate=parent_feerate*3)["psbt"] - conflict_proc = def_wallet.walletprocesspsbt(conflict_psbt) - assert_equal(conflict_proc["complete"], True) - conflict_txid = self.nodes[0].sendrawtransaction(conflict_proc["hex"]) - assert_equal(set(self.nodes[0].getrawmempool()), {conflict_txid}) - - self.generate(self.nodes[0], 1) - - assert_equal(def_wallet.gettransaction(parent_txid)["confirmations"], -1) - assert_equal(wallet.gettransaction(child_txid)["confirmations"], -1) + child_txids = [] + for current_wallet in [wallet, external_wallet]: + # one utxo to target wallet + addr = current_wallet.getnewaddress() + addr_desc = current_wallet.getaddressinfo(addr)["desc"] + def_wallet.sendtoaddress(addr, 10) + + self.generate(self.nodes[0], 1) + + utxo = current_wallet.listunspent()[0] + + # Make utxo in grandparent that will be double spent + gp_addr = def_wallet.getnewaddress() + gp_txid = def_wallet.sendtoaddress(gp_addr, 7) + gp_utxo = {"txid": gp_txid, "vout": find_vout_for_address(self.nodes[0], gp_txid, gp_addr)} + + self.generate(self.nodes[0], 1) + assert_equal(self.nodes[0].getrawmempool(), []) + assert_equal(self.nodes[1].getrawmempool(), []) + + # make unconfirmed parent tx + parent_addr = def_wallet.getnewaddress() + parent_txid = def_wallet.send(outputs=[{parent_addr: 5}], inputs=[gp_utxo])["txid"] + parent_utxo = {"txid": parent_txid, "vout": find_vout_for_address(self.nodes[0], parent_txid, parent_addr)} + parent_me = self.nodes[0].getmempoolentry(parent_txid) + parent_feerate = parent_me["fees"]["base"] * COIN / parent_me["vsize"] + self.nodes[1].sendrawtransaction(def_wallet.gettransaction(parent_txid)["hex"]) + + assert_equal(self.nodes[0].getrawmempool(), [parent_txid]) + assert_equal(self.nodes[1].getrawmempool(), [parent_txid]) + + # make child tx that has one input belonging to target wallet, and one input not + psbt = def_wallet.walletcreatefundedpsbt(inputs=[utxo, parent_utxo], outputs=[{def_wallet.getnewaddress(): 13}], solving_data={"descriptors":[addr_desc]})["psbt"] + psbt = def_wallet.walletprocesspsbt(psbt)["psbt"] + psbt_proc = current_wallet.walletprocesspsbt(psbt) + psbt = psbt_proc["psbt"] + assert_equal(psbt_proc["complete"], True) + child_txid = self.nodes[0].sendrawtransaction(psbt_proc["hex"]) + self.nodes[1].sendrawtransaction(psbt_proc["hex"]) + child_txids.append(child_txid) + + assert_equal(set(self.nodes[0].getrawmempool()), {child_txid, parent_txid}) + assert_equal(set(self.nodes[1].getrawmempool()), {child_txid, parent_txid}) + + assert_equal(def_wallet.gettransaction(parent_txid)["confirmations"], 0) + assert_equal(current_wallet.gettransaction(child_txid)["confirmations"], 0) + + # Make a conflict spending parent + conflict_psbt = def_wallet.walletcreatefundedpsbt(inputs=[gp_utxo], outputs=[{def_wallet.getnewaddress(): 2}], fee_rate="{0:.8}".format(Decimal(parent_feerate*3)))["psbt"] + conflict_proc = def_wallet.walletprocesspsbt(conflict_psbt) + assert_equal(conflict_proc["complete"], True) + # Only send conflict to node 0 + conflict_txid = self.nodes[0].sendrawtransaction(conflict_proc["hex"]) + assert_equal(set(self.nodes[0].getrawmempool()), {conflict_txid}) + + self.generate(self.nodes[0], 1, sync_fun=self.no_op) + assert_equal(def_wallet.gettransaction(parent_txid)["confirmations"], -1) + + assert_equal(wallet.gettransaction(child_txids[0])["confirmations"], -4) + + # Sync Blocks so that node 1 gets conflicted block + self.sync_blocks([self.nodes[0], self.nodes[1]]) + assert_equal(external_wallet.gettransaction(child_txids[1])["confirmations"], -1) + external_wallet.unloadwallet() if __name__ == '__main__': TxConflicts().main()