diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 1a79d7db4ed25..9c68947fb002c 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -43,6 +43,10 @@ struct TxStateConflicted { explicit TxStateConflicted(const uint256& block_hash, int height) : conflicting_block_hash(block_hash), conflicting_block_height(height) {} }; +//! State of transaction with one or more mempool conflicts. +struct TxStateMempoolConflicted { +}; + //! State of transaction not confirmed or conflicting with a known block and //! not in the mempool. May conflict with the mempool, or with an unknown block, //! or be abandoned, never broadcast, or rejected from the mempool for another @@ -65,7 +69,7 @@ struct TxStateUnrecognized { }; //! All possible CWalletTx states -using TxState = std::variant; +using TxState = std::variant; //! Subset of states transaction sync logic is implemented to handle. using SyncTxState = std::variant; @@ -91,6 +95,7 @@ static inline uint256 TxStateSerializedBlockHash(const TxState& state) return std::visit(util::Overloaded{ [](const TxStateInactive& inactive) { return inactive.abandoned ? uint256::ONE : uint256::ZERO; }, [](const TxStateInMempool& in_mempool) { return uint256::ZERO; }, + [](const TxStateMempoolConflicted& mempool_conflicted) { return uint256::ZERO; }, [](const TxStateConfirmed& confirmed) { return confirmed.confirmed_block_hash; }, [](const TxStateConflicted& conflicted) { return conflicted.conflicting_block_hash; }, [](const TxStateUnrecognized& unrecognized) { return unrecognized.block_hash; } @@ -103,6 +108,7 @@ static inline int TxStateSerializedIndex(const TxState& state) return std::visit(util::Overloaded{ [](const TxStateInactive& inactive) { return inactive.abandoned ? -1 : 0; }, [](const TxStateInMempool& in_mempool) { return 0; }, + [](const TxStateMempoolConflicted& mempool_conflicted) { return 0; }, [](const TxStateConfirmed& confirmed) { return confirmed.position_in_block; }, [](const TxStateConflicted& conflicted) { return -1; }, [](const TxStateUnrecognized& unrecognized) { return unrecognized.index; } @@ -315,7 +321,9 @@ class CWalletTx template T* state() { return std::get_if(&m_state); } bool isAbandoned() const { return state() && state()->abandoned; } - bool isConflicted() const { return state(); } + bool isConflicted() const { return state() || state(); } + bool isMempoolConflicted() const { return state(); } + bool isBlockConflicted() const { return state(); } bool isInactive() const { return state(); } bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); } bool isConfirmed() const { return state(); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 552939c2dc56b..f2f8b1c9c7416 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1279,7 +1279,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) assert(!wtx.isConfirmed()); assert(!wtx.InMempool()); // If already conflicted or abandoned, no need to set abandoned - if (!wtx.isConflicted() && !wtx.isAbandoned()) { + if (!wtx.isBlockConflicted() && !wtx.isAbandoned()) { wtx.m_state = TxStateInactive{/*abandoned=*/true}; return TxUpdate::NOTIFY_CHANGED; } @@ -1385,6 +1385,36 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& tx) { if (it != mapWallet.end()) { RefreshMempoolStatus(it->second, chain()); } + + const uint256& conflict_hash = tx->GetHash(); + + for (const CTxIn& tx_in : tx->vin) { + // If the wallet contains no transactions that spend the same prevout, continue + if (mapTxSpends.count(tx_in.prevout) == 0) continue; + + std::pair range; + range = mapTxSpends.equal_range(tx_in.prevout); + + // For each wallet transaction spending this prevout... + for (TxSpends::const_iterator _it = range.first; _it != range.second; ++_it) { + // if this tx is the same as the tx which was just added to the mempool, continue + if (_it->second == conflict_hash) continue; + + auto try_updating_state = [&](CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { + // Add this conflict to the wtx's entry (create one if one does not already exist) + MempoolConflicts[wtx.tx->GetHash()].insert(conflict_hash); + + if (wtx.isInactive() && !wtx.isAbandoned()) { + wtx.m_state = TxStateMempoolConflicted{}; + return TxUpdate::CHANGED; + } else { + return TxUpdate::UNCHANGED; + } + }; + + RecursiveUpdateTxState(_it->second, try_updating_state); + } + } } void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) { @@ -1422,6 +1452,40 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRe // https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking SyncTransaction(tx, TxStateInactive{}); } + + const uint256& conflict_hash = tx->GetHash(); + + for (const CTxIn& tx_in : tx->vin) { + // If the wallet contains no transactions that spend the same prevout, continue + if (mapTxSpends.count(tx_in.prevout) == 0) continue; + + std::pair range; + range = mapTxSpends.equal_range(tx_in.prevout); + + // For each wallet transaction spending this prevout... + for (TxSpends::const_iterator _it = range.first; _it != range.second; ++_it) { + + auto try_updating_state = [&](CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { + if (MempoolConflicts.find(wtx.tx->GetHash()) != MempoolConflicts.end()) { + + // Remove the previously conflicting transaction from this wtx's entry + MempoolConflicts.at(wtx.tx->GetHash()).erase(conflict_hash); + + if (MempoolConflicts.at(wtx.tx->GetHash()).empty()) { + // If this wtx has no other mempool conflicts, erase this wtx entry + MempoolConflicts.erase(wtx.tx->GetHash()); + if (wtx.isMempoolConflicted()) { + wtx.m_state = TxStateInactive{}; + return TxUpdate::CHANGED; + } + } + } + return TxUpdate::UNCHANGED; + }; + + RecursiveUpdateTxState(_it->second, try_updating_state); + } + } } void CWallet::blockConnected(const interfaces::BlockInfo& block) @@ -1438,8 +1502,8 @@ void CWallet::blockConnected(const interfaces::BlockInfo& block) // Scan block for (size_t index = 0; index < block.data->vtx.size(); index++) { - SyncTransaction(block.data->vtx[index], TxStateConfirmed{block.hash, block.height, static_cast(index)}); transactionRemovedFromMempool(block.data->vtx[index], MemPoolRemovalReason::BLOCK); + SyncTransaction(block.data->vtx[index], TxStateConfirmed{block.hash, block.height, static_cast(index)}); } } @@ -1470,12 +1534,18 @@ void CWallet::blockDisconnected(const interfaces::BlockInfo& block) for (TxSpends::const_iterator _it = range.first; _it != range.second; ++_it) { CWalletTx& wtx = mapWallet.find(_it->second)->second; - if (!wtx.isConflicted()) continue; + if (!wtx.isBlockConflicted()) continue; - auto try_updating_state = [&](CWalletTx& tx) { - if (!tx.isConflicted()) return TxUpdate::UNCHANGED; + auto try_updating_state = [&](CWalletTx& tx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { + if (!tx.isBlockConflicted()) return TxUpdate::UNCHANGED; if (tx.state()->conflicting_block_height >= disconnect_height) { - tx.m_state = TxStateInactive{}; + // Check if this transaction still has conflicts in the mempool + if (MempoolConflicts.find(tx.tx->GetHash()) != MempoolConflicts.end()) { + assert(!MempoolConflicts.at(tx.tx->GetHash()).empty()); + tx.m_state = TxStateMempoolConflicted{}; + } else { + tx.m_state = TxStateInactive{}; + } return TxUpdate::CHANGED; } return TxUpdate::UNCHANGED; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 422ecf531b5a2..e197eed9a8f23 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -313,6 +313,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void AddToSpends(const COutPoint& outpoint, const uint256& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void AddToSpends(const CWalletTx& wtx, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + // Map of wtx hash to a set of mempool conflict hashes + std::map> MempoolConflicts GUARDED_BY(cs_wallet); + /** * Add a transaction to the wallet, or update it. confirm.block_* should * be set when the transaction was known to be included in a block. When diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 26915077736d5..83403fabb49bc 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -232,7 +232,9 @@ def run_test(self): balance = newbalance # Invalidate the block with the double spend. B & C's 10 BTC outputs should no longer be available - self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) + blk = self.nodes[0].getbestblockhash() + self.generate(self.nodes[1], 10) + self.nodes[0].invalidateblock(blk) assert_equal(alice.gettransaction(txAB1)["confirmations"], 0) newbalance = alice.getbalance() assert_equal(newbalance, balance - Decimal("20"))