wallet: track mempool conflicts

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<TxStateConfirmed, TxStateInMempool, TxStateConflicted, TxStateInactive, TxStateUnrecognized>;
+using TxState = std::variant<TxStateConfirmed, TxStateInMempool, TxStateConflicted, TxStateMempoolConflicted, TxStateInactive, TxStateUnrecognized>;
 
 //! Subset of states transaction sync logic is implemented to handle.
 using SyncTxState = std::variant<TxStateConfirmed, TxStateInMempool, TxStateInactive>;
@@ -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<typename T> T* state() { return std::get_if<T>(&m_state); }
 
     bool isAbandoned() const { return state<TxStateInactive>() && state<TxStateInactive>()->abandoned; }
-    bool isConflicted() const { return state<TxStateConflicted>(); }
+    bool isConflicted() const { return state<TxStateConflicted>() || state<TxStateMempoolConflicted>(); }
+    bool isMempoolConflicted() const { return state<TxStateMempoolConflicted>(); }
+    bool isBlockConflicted() const { return state<TxStateConflicted>(); }
     bool isInactive() const { return state<TxStateInactive>(); }
     bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); }
     bool isConfirmed() const { return state<TxStateConfirmed>(); }

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<TxSpends::const_iterator, TxSpends::const_iterator> 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<TxSpends::const_iterator, TxSpends::const_iterator> 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<int>(index)});
         transactionRemovedFromMempool(block.data->vtx[index], MemPoolRemovalReason::BLOCK);
+        SyncTransaction(block.data->vtx[index], TxStateConfirmed{block.hash, block.height, static_cast<int>(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<TxStateConflicted>()->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;

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<uint256, std::set<uint256>> 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

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"))