fix(wallet): restore spendability of inputs freed by abandoning a transaction

src/wallet/test/availablecoins_tests.cpp

@@ -2,6 +2,12 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or https://www.opensource.org/licenses/mit-license.php.
 
+#include <algorithm>
+#include <bls/bls.h>
+#include <evo/deterministicmns.h>
+#include <evo/dmn_types.h>
+#include <interfaces/chain.h>
+#include <test/util/masternode.h>
 #include <validation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/spend.h>
@@ -89,5 +95,215 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup)
     BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U);
 }
 
+BOOST_FIXTURE_TEST_CASE(AbandonedSpendReleasesItsInputs, AvailableCoinsTestingSetup)
+{
+    LOCK(wallet->cs_wallet);
+
+    const CoinsResult before{AvailableCoins(*wallet)};
+    BOOST_CHECK(before.size() > 0);
+
+    CCoinControl coin_control;
+    auto created{CreateTransaction(*wallet, {CRecipient{{GetScriptForRawPubKey(coinbaseKey.GetPubKey())}, 1 * COIN,
+                                                        /*fSubtractFeeFromAmount=*/false}},
+                                   RANDOM_CHANGE_POSITION, coin_control)};
+    BOOST_CHECK(created);
+    const CTransactionRef tx{created->tx};
+    BOOST_REQUIRE(!tx->vin.empty());
+    const CTxIn& input{tx->vin.front()};
+    const CAmount input_amount{wallet->mapWallet.at(input.prevout.hash).tx->vout.at(input.prevout.n).nValue};
+    const int inputs_before{wallet->CountInputsWithAmount(input_amount)};
+    BOOST_CHECK(inputs_before > 0);
+
+    // The transaction is only in the wallet: never broadcast, never mined.
+    BOOST_CHECK(wallet->AddToWallet(tx, TxStateInactive{}));
+    BOOST_CHECK(AvailableCoins(*wallet).size() < before.size());
+    BOOST_CHECK(wallet->CountInputsWithAmount(input_amount) < inputs_before);
+
+    // Abandoning it makes the coins it spent available again, without a reload.
+    BOOST_CHECK(wallet->AbandonTransaction(tx->GetHash()));
+    const CoinsResult after{AvailableCoins(*wallet)};
+    BOOST_CHECK_EQUAL(after.size(), before.size());
+    BOOST_CHECK_EQUAL(after.total_amount, before.total_amount);
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(input_amount), inputs_before);
+
+    // The abandoned transaction re-entering the mempool spends the inputs
+    // again: the restored outpoints must leave the wallet UTXO set, or
+    // functions that trust it directly (CountInputsWithAmount and the
+    // CoinJoin rounds accounting) would count spent coins.
+    BOOST_CHECK(wallet->AddToWallet(tx, TxStateInMempool{}));
+    BOOST_CHECK(wallet->IsSpent(input.prevout));
+    BOOST_CHECK(wallet->CountInputsWithAmount(input_amount) < inputs_before);
+}
+
+BOOST_FIXTURE_TEST_CASE(ConflictedDescendantReactivationReconcilesInputs, AvailableCoinsTestingSetup)
+{
+    const CScript wallet_script{GetScriptForRawPubKey(coinbaseKey.GetPubKey())};
+    auto created{CreateTransaction(*wallet, {CRecipient{wallet_script, 1 * COIN, /*fSubtractFeeFromAmount=*/false}},
+                                   RANDOM_CHANGE_POSITION, CCoinControl{})};
+    BOOST_REQUIRE(created);
+    const CTransactionRef parent{created->tx};
+
+    CKey external_key;
+    external_key.MakeNewKey(true);
+    auto conflict_created{CreateTransaction(*wallet, {CRecipient{GetScriptForRawPubKey(external_key.GetPubKey()), COIN / 4,
+                                                                 /*fSubtractFeeFromAmount=*/false}},
+                                            RANDOM_CHANGE_POSITION, CCoinControl{})};
+    BOOST_REQUIRE(conflict_created);
+    const CTransactionRef conflict{conflict_created->tx};
+    BOOST_REQUIRE(parent->vin.front().prevout == conflict->vin.front().prevout);
+
+    BOOST_REQUIRE(wallet->AddToWallet(parent, TxStateInactive{}));
+
+    const auto parent_output_it{std::ranges::find_if(parent->vout, [&](const CTxOut& output) {
+        return output.nValue == 1 * COIN && output.scriptPubKey == wallet_script;
+    })};
+    BOOST_REQUIRE(parent_output_it != parent->vout.end());
+    const COutPoint parent_outpoint{parent->GetHash(), static_cast<uint32_t>(parent_output_it - parent->vout.begin())};
+
+    CMutableTransaction child_mtx;
+    child_mtx.vin.emplace_back(parent_outpoint);
+    child_mtx.vout.emplace_back(COIN / 2, wallet_script);
+    const CTransactionRef child{MakeTransactionRef(child_mtx)};
+    BOOST_REQUIRE(wallet->AddToWallet(child, TxStateInactive{}));
+    {
+        LOCK(wallet->cs_wallet);
+        BOOST_CHECK(wallet->IsSpent(parent_outpoint));
+    }
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(1 * COIN), 0);
+
+    // A block transaction conflicts the parent and recursively conflicts the
+    // child. The child's input is temporarily unspent and returns to the UTXO
+    // set, although CountInputsWithAmount() ignores it while its parent is
+    // conflicted.
+    const CBlock block{CreateAndProcessBlock({CMutableTransaction{*conflict}}, GetScriptForRawPubKey({}))};
+    const uint256 block_hash{block.GetHash()};
+    const CBlockIndex* tip{WITH_LOCK(m_node.chainman->GetMutex(), return m_node.chainman->ActiveChain().Tip())};
+    BOOST_REQUIRE_EQUAL(tip->GetBlockHash(), block_hash);
+
+    interfaces::BlockInfo block_info{block_hash};
+    block_info.prev_hash = &block.hashPrevBlock;
+    block_info.height = tip->nHeight;
+    block_info.data = &block;
+    wallet->blockConnected(block_info);
+    {
+        LOCK(wallet->cs_wallet);
+        BOOST_REQUIRE(wallet->mapWallet.at(child->GetHash()).isConflicted());
+        BOOST_CHECK(!wallet->IsSpent(parent_outpoint));
+    }
+
+    // Disconnecting the conflicting block makes the parent and descendant
+    // inactive again. The child therefore spends parent_outpoint again, and
+    // the public CoinJoin counter must not observe a stale UTXO-set entry.
+    wallet->blockDisconnected(block_info);
+    {
+        LOCK(wallet->cs_wallet);
+        BOOST_CHECK(!wallet->mapWallet.at(child->GetHash()).isConflicted());
+        BOOST_CHECK(wallet->IsSpent(parent_outpoint));
+    }
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(1 * COIN), 0);
+}
+
+BOOST_FIXTURE_TEST_CASE(AbandonedSpendRestoresDustLock, AvailableCoinsTestingSetup)
+{
+    LOCK(wallet->cs_wallet);
+    wallet->m_dust_protection_threshold = 1 * COIN;
+
+    const auto dest{wallet->GetNewDestination("")};
+    BOOST_ASSERT(dest);
+
+    // An external transaction (no input is ours) pays us a dust-protection
+    // target; AddToWallet() locks the output on insertion.
+    CMutableTransaction dust_mtx;
+    dust_mtx.vin.emplace_back(COutPoint{uint256::ONE, 0});
+    dust_mtx.vout.emplace_back(COIN / 100, GetScriptForDestination(*dest));
+    const CTransactionRef dust_tx{MakeTransactionRef(dust_mtx)};
+    const COutPoint dust_outpoint{dust_tx->GetHash(), 0};
+    BOOST_CHECK(wallet->AddToWallet(dust_tx, TxStateInMempool{}));
+    BOOST_CHECK(wallet->IsLockedCoin(dust_outpoint));
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(COIN / 100), 1);
+
+    // A wallet transaction spending it unlocks it and removes it from the
+    // wallet UTXO set.
+    CMutableTransaction spend_mtx;
+    spend_mtx.vin.emplace_back(dust_outpoint);
+    spend_mtx.vout.emplace_back(COIN / 200, GetScriptForDestination(*dest));
+    const CTransactionRef spend_tx{MakeTransactionRef(spend_mtx)};
+    BOOST_CHECK(wallet->AddToWallet(spend_tx, TxStateInactive{}));
+    BOOST_CHECK(!wallet->IsLockedCoin(dust_outpoint));
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(COIN / 100), 0);
+
+    // Abandoning the spend restores the outpoint together with the automatic
+    // dust lock a wallet reload would apply.
+    BOOST_CHECK(wallet->AbandonTransaction(spend_tx->GetHash()));
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(COIN / 100), 1);
+    BOOST_CHECK(wallet->IsLockedCoin(dust_outpoint));
+
+    BOOST_CHECK(wallet->AddToWallet(spend_tx, TxStateInMempool{}));
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(COIN / 100), 0);
+    BOOST_CHECK(!wallet->IsLockedCoin(dust_outpoint));
+}
+
+BOOST_FIXTURE_TEST_CASE(AbandonedSpendRestoresActiveMasternodeCollateralLock, AvailableCoinsTestingSetup)
+{
+    const CScript wallet_script{GetScriptForDestination(PKHash(coinbaseKey.GetPubKey()))};
+    while (WITH_LOCK(m_node.chainman->GetMutex(), return m_node.chainman->ActiveChain().Height()) <
+           Params().GetConsensus().DIP0003Height) {
+        CreateAndProcessBlock({}, wallet_script);
+    }
+
+    CKey owner_key;
+    CBLSSecretKey operator_key;
+    auto utxos{BuildSimpleUtxoMap(m_coinbase_txns)};
+    CMutableTransaction pro_reg_mtx{CreateProRegTx(*m_node.chainman, utxos, /*port=*/1, wallet_script,
+                                                   coinbaseKey, owner_key, operator_key)};
+    const CTransactionRef pro_reg_tx{MakeTransactionRef(pro_reg_mtx)};
+    const CBlock block{CreateAndProcessBlock({pro_reg_mtx}, wallet_script)};
+    const CBlockIndex* tip{WITH_LOCK(m_node.chainman->GetMutex(), return m_node.chainman->ActiveChain().Tip())};
+    {
+        LOCK(::cs_main);
+        m_node.dmnman->UpdatedBlockTip(tip);
+        BOOST_REQUIRE(m_node.dmnman->GetListAtChainTip().HasMN(pro_reg_tx->GetHash()));
+    }
+
+    const uint256 block_hash{block.GetHash()};
+    interfaces::BlockInfo block_info{block_hash};
+    block_info.prev_hash = &block.hashPrevBlock;
+    block_info.height = tip->nHeight;
+    block_info.data = &block;
+    wallet->blockConnected(block_info);
+
+    const COutPoint collateral{pro_reg_tx->GetHash(), 0};
+    {
+        LOCK(wallet->cs_wallet);
+        BOOST_CHECK(wallet->IsLockedCoin(collateral));
+    }
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(dmn_types::Regular.collat_amount), 1);
+
+    CMutableTransaction spend_mtx;
+    spend_mtx.vin.emplace_back(collateral);
+    spend_mtx.vout.emplace_back(1 * COIN, wallet_script);
+    const CTransactionRef spend_tx{MakeTransactionRef(spend_mtx)};
+    BOOST_REQUIRE(wallet->AddToWallet(spend_tx, TxStateInactive{}));
+    {
+        LOCK(wallet->cs_wallet);
+        BOOST_CHECK(!wallet->IsLockedCoin(collateral));
+    }
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(dmn_types::Regular.collat_amount), 0);
+
+    BOOST_REQUIRE(wallet->AbandonTransaction(spend_tx->GetHash()));
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(dmn_types::Regular.collat_amount), 1);
+    {
+        LOCK(wallet->cs_wallet);
+        BOOST_CHECK(wallet->IsLockedCoin(collateral));
+    }
+
+    BOOST_CHECK(wallet->AddToWallet(spend_tx, TxStateInMempool{}));
+    BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(dmn_types::Regular.collat_amount), 0);
+    {
+        LOCK(wallet->cs_wallet);
+        BOOST_CHECK(!wallet->IsLockedCoin(collateral));
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet

src/wallet/wallet.cpp

@@ -1071,6 +1071,11 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
         if (state.index() != wtx.m_state.index()) {
             wtx.m_state = state;
             fUpdated = true;
+            // An abandoned transaction re-entering the mempool or a block
+            // spends its inputs again; take the outpoints
+            // ReconcileWalletUTXOs() restored on abandonment back out of the
+            // wallet UTXO set (AddToSpends() only ran on first insertion).
+            ReconcileWalletUTXOs(wtx.tx, batch);
         } else {
             assert(TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state));
             assert(TxStateSerializedBlockHash(wtx.m_state) == TxStateSerializedBlockHash(state));
@@ -1193,6 +1198,34 @@ std::set<COutPoint> CWallet::AddWalletUTXOs(CTransactionRef tx, bool ret_dups)
     return ret;
 }
 
+void CWallet::ReconcileWalletUTXOs(const CTransactionRef& tx, WalletBatch& batch)
+{
+    AssertLockHeld(cs_wallet);
+    std::set<COutPoint> restored;
+    for (const CTxIn& txin : tx->vin) {
+        const auto it{mapWallet.find(txin.prevout.hash)};
+        if (it == mapWallet.end() || txin.prevout.n >= it->second.tx->vout.size()) continue;
+        if (!IsMine(it->second.tx->vout[txin.prevout.n])) continue;
+        if (IsSpent(txin.prevout)) {
+            setWalletUTXO.erase(txin.prevout);
+            UnlockCoin(txin.prevout, &batch);
+        } else if (setWalletUTXO.insert(txin.prevout).second) {
+            restored.insert(txin.prevout);
+        }
+    }
+    if (restored.empty()) return;
+    // AddToSpends() unlocked these outpoints when the spend appeared; reapply
+    // the automatic protections a wallet reload would.
+    LockProTxCoins(restored, &batch);
+    if (m_dust_protection_threshold > 0) {
+        for (const COutPoint& outpoint : restored) {
+            if (IsDustProtectionTarget(mapWallet.at(outpoint.hash), outpoint.n)) {
+                LockCoin(outpoint, &batch);
+            }
+        }
+    }
+}
+
 bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxState& state, WalletBatch& batch, bool fUpdate, bool rescanning_old_block)
 {
     const CTransaction& tx = *ptx;
@@ -1401,6 +1434,7 @@ void CWallet::RecursiveUpdateTxState(const uint256& tx_hash, const TryUpdatingSt
             // If a transaction changes its tx state, that usually changes the balance
             // available of the outputs it spends. So force those to be recomputed
             MarkInputsDirty(wtx.tx);
+            ReconcileWalletUTXOs(wtx.tx, batch);
         }
     }
 

src/wallet/wallet.h

@@ -334,6 +334,19 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
      *  @param[in] ret_dups   Allow UTXOs already in set to be included in return value
      *  @returns              Set of all new UTXOs (eligible to be) added to set */
     std::set<COutPoint> AddWalletUTXOs(CTransactionRef tx, bool ret_dups) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    /** Reconcile the wallet UTXO set with `tx`'s inputs after a state change of `tx`
+     *
+     *  AddToSpends() removes an outpoint from the set (and unlocks it) as soon as
+     *  some wallet transaction spends it, but nothing maintained the set across the
+     *  spender's later state changes. Whenever `tx` changes state, re-evaluate each
+     *  outpoint it consumes: an outpoint no wallet transaction spends any more goes
+     *  back in the set, with the masternode-collateral and dust locks a wallet
+     *  reload would apply; an outpoint that became spent again — the abandoned
+     *  spender re-entered the mempool or a block — is erased and unlocked.
+     *
+     *  @param[in] tx         Transaction whose inputs to reconsider
+     *  @param[in] batch      Batch to write coin-lock updates to */
+    void ReconcileWalletUTXOs(const CTransactionRef& tx, WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     mutable std::map<COutPoint, int> mapOutpointRoundsCache GUARDED_BY(cs_wallet);
 
     /**