foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Handle new blocks with unrelated parents conflicts

Browse source 
Watch for wallet transaction conflicts triggered by adding conflicting blocks
This commit is contained in:
Novo 2024-04-09 17:06:07 +01:00 • committed by Eunovo
parent c162984fbc
commit 6db2961f60
2 changed files with 50 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
src/wallet/wallet.cpp
View file

@ -1492,34 +1492,16 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, const Mem
m_unrelated_conflict_tx_watchlist.erase(iter); m_unrelated_conflict_tx_watchlist.erase(iter);
} }
auto* conflict_reason = std::get_if<ConflictReason>(&reason);
// Handle transactions that were removed from the mempool because they // Handle transactions that were removed from the mempool because they
// conflict with transactions in a newly connected block. // conflict with transactions in a newly connected block.
if (IsReason<ConflictReason>(reason)) { if (conflict_reason != nullptr && IsFromMe(*tx)) {
// Trigger external -walletnotify notifications for these transactions. // The wallet's heuristics for
// 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
// distinguishing between conflicted and unconfirmed transactions are // distinguishing between conflicted and unconfirmed transactions are
// imperfect, and could be improved in general, see // imperfect, and could be improved in general, see
// https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking // 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(); const Txid& txid = tx->GetHash();

42
test/functional/wallet_conflicts.py
View file

@ -9,6 +9,7 @@ Test that wallet correctly tracks transactions that have been conflicted by bloc
from decimal import Decimal from decimal import Decimal
from test_framework.messages import COIN
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import ( from test_framework.util import (
assert_equal, assert_equal,
@ -426,32 +427,36 @@ class TxConflicts(BitcoinTestFramework):
def test_unknown_parent_conflict(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.log.info("Test that a conflict with parent not belonging to the wallet causes in-wallet children to also conflict")
def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
self.nodes[0].createwallet("parent_conflict") self.nodes[0].createwallet("parent_conflict")
wallet = self.nodes[0].get_wallet_rpc("parent_conflict") wallet = self.nodes[0].get_wallet_rpc("parent_conflict")
def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
# one utxo to target wallet # two utxos to target wallet
addr = wallet.getnewaddress() addr = wallet.getnewaddress()
addr_desc = wallet.getaddressinfo(addr)["desc"] addr_desc = wallet.getaddressinfo(addr)["desc"]
def_wallet.sendtoaddress(addr, 10) def_wallet.sendtoaddress(addr, 10)
self.generate(self.nodes[0], 1) def_wallet.sendtoaddress(addr, 10)
utxo = wallet.listunspent()[0]
self.generate(self.nodes[0], 1)
txids = []
for node in [self.nodes[0], self.nodes[1]]:
utxo = wallet.listunspent()[0]
# Make utxo in grandparent that will be double spent # Make utxo in grandparent that will be double spent
gp_addr = def_wallet.getnewaddress() gp_addr = def_wallet.getnewaddress()
gp_txid = def_wallet.sendtoaddress(gp_addr, 7) 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)} 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(), []) self.generate(self.nodes[0], 1)
# make unconfirmed parent tx # make unconfirmed parent tx
parent_addr = def_wallet.getnewaddress() parent_addr = def_wallet.getnewaddress()
parent_txid = def_wallet.send(outputs=[{parent_addr: 5}], inputs=[gp_utxo])["txid"] 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_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_me = self.nodes[0].getmempoolentry(parent_txid)
parent_feerate = parent_me["fees"]["base"] * 100000000 / parent_me["vsize"] parent_feerate = parent_me["fees"]["base"] * COIN / parent_me["vsize"]
assert_equal(self.nodes[0].getrawmempool(), [parent_txid]) self.nodes[1].sendrawtransaction(def_wallet.gettransaction(parent_txid)["hex"])
# make child tx that has one input belonging to target wallet, and one input not # 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.walletcreatefundedpsbt(inputs=[utxo, parent_utxo], outputs=[{def_wallet.getnewaddress(): 13}], solving_data={"descriptors":[addr_desc]})["psbt"]
@ -460,22 +465,21 @@ class TxConflicts(BitcoinTestFramework):
psbt = psbt_proc["psbt"] psbt = psbt_proc["psbt"]
assert_equal(psbt_proc["complete"], True) assert_equal(psbt_proc["complete"], True)
child_txid = self.nodes[0].sendrawtransaction(psbt_proc["hex"]) child_txid = self.nodes[0].sendrawtransaction(psbt_proc["hex"])
assert_equal(set(self.nodes[0].getrawmempool()), {child_txid, parent_txid}) txids.append(child_txid)
self.nodes[1].sendrawtransaction(psbt_proc["hex"])
assert_equal(def_wallet.gettransaction(parent_txid)["confirmations"], 0)
assert_equal(wallet.gettransaction(child_txid)["confirmations"], 0)
# Make a conflict spending parent # 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_psbt = def_wallet.walletcreatefundedpsbt(inputs=[gp_utxo], outputs=[{def_wallet.getnewaddress(): 2}], fee_rate="{:.8f}".format(Decimal(parent_feerate*3)))["psbt"]
conflict_proc = def_wallet.walletprocesspsbt(conflict_psbt) conflict_proc = def_wallet.walletprocesspsbt(conflict_psbt)
assert_equal(conflict_proc["complete"], True) node.sendrawtransaction(conflict_proc["hex"])
conflict_txid = self.nodes[0].sendrawtransaction(conflict_proc["hex"]) self.generate(node, 1, sync_fun=self.no_op)
assert_equal(set(self.nodes[0].getrawmempool()), {conflict_txid})
self.generate(self.nodes[0], 1) assert_equal(wallet.gettransaction(txids[0])["confirmations"], -3)
assert_equal(def_wallet.gettransaction(parent_txid)["confirmations"], -1) self.log.info("Test that receiving a block with a conflict with parent not belonging to the wallet causes in-wallet children to also conflict")
assert_equal(wallet.gettransaction(child_txid)["confirmations"], -1) # Sync block with conflict tx
self.sync_blocks([self.nodes[0], self.nodes[1]])
assert_equal(wallet.gettransaction(txids[1])["confirmations"], -1)
if __name__ == '__main__': if __name__ == '__main__':
TxConflicts(__file__).main() TxConflicts(__file__).main()