From 8108303bffdf67c0ad73b73f233cd6b06622626e Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 19 Aug 2026 14:46:59 -0500 Subject: [PATCH] fix(wallet): restore spendability of inputs freed by abandoning a transaction setWalletUTXO holds every unspent output the wallet owns and is what AvailableCoins() walks. AddToSpends() drops an outpoint from it as soon as some wallet transaction spends it, but nothing ever puts it back when that transaction stops spending it, so abandoning a transaction left its inputs invisible to coin selection for the rest of the session. Balances recovered immediately, because those are recomputed from the transaction states, which made the coins look present while every attempt to spend them failed for lack of funds. Restarting cleared it, since the set is rebuilt from scratch at load. Reconcile the outpoints a transaction consumed whenever its state changes: restore them when they are no longer spent (abandonment, or being conflicted away by a competing transaction) and drop them again when a reactivated transaction spends them once more. The reconciliation also maintains the automatic protections AddToSpends() tore down with the spend: restored masternode collaterals and dust outputs are relocked exactly as a wallet reload would, and outpoints spent again are unlocked. --- src/wallet/test/availablecoins_tests.cpp | 216 +++++++++++++++++++++++ src/wallet/wallet.cpp | 34 ++++ src/wallet/wallet.h | 13 ++ 3 files changed, 263 insertions(+) diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp index 0afb81d062e0..7c09a3e16706 100644 --- a/src/wallet/test/availablecoins_tests.cpp +++ b/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 +#include +#include +#include +#include +#include #include #include #include @@ -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(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 = █ + 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 = █ + 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 diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6a941af05ca2..b5ad726a9501 100644 --- a/src/wallet/wallet.cpp +++ b/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 CWallet::AddWalletUTXOs(CTransactionRef tx, bool ret_dups) return ret; } +void CWallet::ReconcileWalletUTXOs(const CTransactionRef& tx, WalletBatch& batch) +{ + AssertLockHeld(cs_wallet); + std::set 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); } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 059e7712c6c4..a03de44dd248 100644 --- a/src/wallet/wallet.h +++ b/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 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 mapOutpointRoundsCache GUARDED_BY(cs_wallet); /**