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); /**