Skip to content

Commit

Permalink
Handle new blocks with unrelated parents conflicts
Browse files Browse the repository at this point in the history 
Watch for wallet transaction conflicts triggered by adding conflicting blocks
  • Loading branch information
@Eunovo
Eunovo committed May 9, 2024
1 parent 8598ffa commit ea66044
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 69 deletions.
28 changes: 5 additions & 23 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1471,34 +1471,16 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, const Mem
}
}

auto* conflict_reason = std::get_if<ConflictReason>(&reason);

// Handle transactions that were removed from the mempool because they
// conflict with transactions in a newly connected block.
if (std::get_if<ConflictReason>(&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();
Expand Down
113 changes: 67 additions & 46 deletions test/functional/wallet_conflicts.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,56 +428,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"] * 100000000 / 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=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()

0 comments on commit ea66044

Please sign in to comment.