From f44af84fe914112e0312f96dcb3f0e0af6b7830b Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 21 Aug 2026 22:44:08 +0300 Subject: [PATCH 1/4] fix(wallet): exclude unconfirmable outputs from CoinJoin accounting setWalletUTXO holds every unspent output the wallet owns, including outputs of transactions that cannot confirm as they stand: conflicted ones, and ones that were abandoned, never broadcast, or rejected from the mempool. AvailableCoins() filters those out, but the CoinJoin functions that read setWalletUTXO directly checked at most `depth < 0`, so such outputs were counted as spendable wallet funds. CountInputsWithAmount() feeds CCoinJoinClientSession::CreateDenominated(), which then under-creates denominations, and GetAverageAnonymizedRounds() and GetNormalizedAnonymizedBalance() surface the same skew in the GUI. Give all four direct readers of setWalletUTXO the liveness filter AvailableCoins() uses, via IsWalletUTXOSpendable(). Co-Authored-By: Claude Opus 5 --- src/wallet/coinjoin.cpp | 9 ++++++--- src/wallet/test/availablecoins_tests.cpp | 22 ++++++++++++++++++++++ src/wallet/wallet.cpp | 8 ++++++++ src/wallet/wallet.h | 7 +++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/wallet/coinjoin.cpp b/src/wallet/coinjoin.cpp index 0a2b6705d49b..28545e6a6287 100644 --- a/src/wallet/coinjoin.cpp +++ b/src/wallet/coinjoin.cpp @@ -162,7 +162,7 @@ std::vector CWallet::SelectCoinsGroupedByAddresses(bool fSkipD if (wtx.IsCoinBase() && GetTxBlocksToMaturity(wtx) > 0) continue; if (fSkipUnconfirmed && !CachedTxIsTrusted(*this, wtx)) continue; - if (GetTxDepthInMainChain(wtx) < 0) continue; + if (!IsWalletUTXOSpendable(wtx)) continue; for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { CTxDestination txdest; @@ -253,7 +253,7 @@ int CWallet::CountInputsWithAmount(CAmount nInputAmount) const const auto it{mapWallet.find(outpoint.hash)}; if (it == mapWallet.end()) continue; if (it->second.tx->vout[outpoint.n].nValue != nInputAmount) continue; - if (GetTxDepthInMainChain(it->second) < 0) continue; + if (!IsWalletUTXOSpendable(it->second)) continue; nTotal++; } @@ -542,6 +542,9 @@ float CWallet::GetAverageAnonymizedRounds() const LOCK(cs_wallet); for (const auto& outpoint : setWalletUTXO) { + const auto it{mapWallet.find(outpoint.hash)}; + if (it == mapWallet.end()) continue; + if (!IsWalletUTXOSpendable(it->second)) continue; if (!IsDenominated(outpoint)) continue; nTotal += GetCappedOutpointCoinJoinRounds(outpoint); @@ -568,7 +571,7 @@ CAmount CWallet::GetNormalizedAnonymizedBalance() const CAmount nValue = it->second.tx->vout[outpoint.n].nValue; if (!CoinJoin::IsDenominatedAmount(nValue)) continue; - if (GetTxDepthInMainChain(it->second) < 0) continue; + if (!IsWalletUTXOSpendable(it->second)) continue; int nRounds = GetCappedOutpointCoinJoinRounds(outpoint); nTotal += nValue * nRounds / CCoinJoinClientOptions::GetRounds(); diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp index 0afb81d062e0..eb0aa2cdedcd 100644 --- a/src/wallet/test/availablecoins_tests.cpp +++ b/src/wallet/test/availablecoins_tests.cpp @@ -89,5 +89,27 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U); } +BOOST_FIXTURE_TEST_CASE(UnconfirmableOutputsAreNotWalletFunds, AvailableCoinsTestingSetup) +{ + LOCK(wallet->cs_wallet); + + const auto dest{wallet->GetNewDestination("")}; + BOOST_ASSERT(dest); + + CMutableTransaction mtx; + mtx.vin.emplace_back(COutPoint{uint256::ONE, 0}); + mtx.vout.emplace_back(1 * COIN, GetScriptForDestination(*dest)); + const CTransactionRef tx{MakeTransactionRef(mtx)}; + + // A transaction the wallet knows about but that never reached the mempool cannot + // confirm as it stands, so its outputs are not funds the wallet can spend or mix. + BOOST_CHECK(wallet->AddToWallet(tx, TxStateInactive{})); + BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(1 * COIN), 0); + + // Once it is in the mempool they count. + BOOST_CHECK(wallet->AddToWallet(tx, TxStateInMempool{})); + BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(1 * COIN), 1); +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index da4a2e08d885..c7b8a81b7580 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -762,6 +762,14 @@ bool CWallet::IsSpent(const COutPoint& outpoint) const return false; } +bool CWallet::IsWalletUTXOSpendable(const CWalletTx& wtx) const +{ + AssertLockHeld(cs_wallet); + const int depth{GetTxDepthInMainChain(wtx)}; + if (depth < 0) return false; + return depth > 0 || wtx.InMempool(); +} + void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid, WalletBatch* batch) { mapTxSpends.insert(std::make_pair(outpoint, wtxid)); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 641065321e1e..e45658e45f8e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -338,6 +338,13 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void AddToSpends(const CWalletTx& wtx, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::set setWalletUTXO; + /** May `wtx`'s outputs in the wallet UTXO set be counted as wallet funds? + * + * setWalletUTXO holds every unspent output the wallet owns, including outputs of + * transactions that will never confirm as they stand: conflicted ones, and ones that + * were abandoned, never broadcast or rejected from the mempool. AvailableCoins() + * filters those out, so consumers reading setWalletUTXO directly must do the same. */ + bool IsWalletUTXOSpendable(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Add new UTXOs to the wallet UTXO set * * @param[in] tx Transaction to scan eligible UTXOs from From 72ad8cbab7962488224ac6dd5d171bb595c1a114 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 22 Aug 2026 10:59:07 +0300 Subject: [PATCH 2/4] doc: add release notes for #7634 Co-Authored-By: Claude Opus 5 --- doc/release-notes-7634.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 doc/release-notes-7634.md diff --git a/doc/release-notes-7634.md b/doc/release-notes-7634.md new file mode 100644 index 000000000000..464d5c842c4d --- /dev/null +++ b/doc/release-notes-7634.md @@ -0,0 +1,10 @@ +Wallet changes +-------------- + +- CoinJoin denomination counts, average mixing rounds and the normalized + anonymized balance no longer include outputs of transactions that cannot + confirm as they stand: conflicted ones, and ones that were abandoned, never + broadcast, or rejected from the mempool. Previously such outputs inflated + these figures, which could make the wallet create fewer denominations than + intended and report mixing progress that did not match the coins it could + actually use. (#7634) From 1255064943f7046261a2078f24577e784e09a4de Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 22 Aug 2026 11:19:02 +0300 Subject: [PATCH 3/4] fix(wallet): apply the liveness filter to CoinJoin balances too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CachedTxGetAvailableCoinJoinCredits() and CachedTxGetAnonymizedCredit() value a transaction's outputs for GetBalance() and the Qt figures, and both rejected only conflicted transactions. An inactive transaction that never reached the mempool therefore still inflated getbalances.mine.coinjoin and the denominated and anonymized balances, while the counts and the normalized balance no longer counted it — the two disagreed about the same output. Reuse IsWalletUTXOSpendable() there as well, so everything that values wallet outputs applies the same rule. Co-Authored-By: Claude Opus 5 --- src/wallet/coinjoin.cpp | 8 ++++---- src/wallet/test/availablecoins_tests.cpp | 16 +++++++++++++--- src/wallet/wallet.h | 14 +++++++------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/wallet/coinjoin.cpp b/src/wallet/coinjoin.cpp index 28545e6a6287..6fab8d0c4b84 100644 --- a/src/wallet/coinjoin.cpp +++ b/src/wallet/coinjoin.cpp @@ -584,8 +584,8 @@ CAmount CachedTxGetAnonymizedCredit(const CWallet& wallet, const CWalletTx& wtx, { AssertLockHeld(wallet.cs_wallet); - // Exclude coinbase and conflicted txes - if (wtx.IsCoinBase() || wallet.GetTxDepthInMainChain(wtx) < 0) return 0; + // Exclude coinbase transactions, and any that cannot confirm as they stand + if (wtx.IsCoinBase() || !wallet.IsWalletUTXOSpendable(wtx)) return 0; CAmount nCredit = 0; uint256 hashTx = wtx.GetHash(); @@ -623,9 +623,9 @@ CoinJoinCredits CachedTxGetAvailableCoinJoinCredits(const CWallet& wallet, const // Must wait until coinbase is safely deep enough in the chain before valuing it if (wtx.IsCoinBase() && wallet.GetTxBlocksToMaturity(wtx) > 0) return ret; - int nDepth = wallet.GetTxDepthInMainChain(wtx); - if (nDepth < 0) return ret; + if (!wallet.IsWalletUTXOSpendable(wtx)) return ret; + const int nDepth{wallet.GetTxDepthInMainChain(wtx)}; ret.is_unconfirmed = CachedTxIsTrusted(wallet, wtx) && nDepth == 0; if (wtx.m_amounts[CWalletTx::ANON_CREDIT].m_cached[ISMINE_SPENDABLE]) { diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp index eb0aa2cdedcd..4984085b1943 100644 --- a/src/wallet/test/availablecoins_tests.cpp +++ b/src/wallet/test/availablecoins_tests.cpp @@ -2,8 +2,10 @@ // 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 @@ -96,19 +98,27 @@ BOOST_FIXTURE_TEST_CASE(UnconfirmableOutputsAreNotWalletFunds, AvailableCoinsTes const auto dest{wallet->GetNewDestination("")}; BOOST_ASSERT(dest); + // Use a real CoinJoin denomination so the denominated-credit paths apply. + const CAmount denom{CoinJoin::GetSmallestDenomination()}; CMutableTransaction mtx; mtx.vin.emplace_back(COutPoint{uint256::ONE, 0}); - mtx.vout.emplace_back(1 * COIN, GetScriptForDestination(*dest)); + mtx.vout.emplace_back(denom, GetScriptForDestination(*dest)); const CTransactionRef tx{MakeTransactionRef(mtx)}; // A transaction the wallet knows about but that never reached the mempool cannot // confirm as it stands, so its outputs are not funds the wallet can spend or mix. BOOST_CHECK(wallet->AddToWallet(tx, TxStateInactive{})); - BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(1 * COIN), 0); + BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(denom), 0); + + // The aggregate CoinJoin balances have to agree: an output that is not wallet funds + // is not denominated or anonymized funds either. + const CWalletTx& wtx{wallet->mapWallet.at(tx->GetHash())}; + BOOST_CHECK_EQUAL(CachedTxGetAvailableCoinJoinCredits(*wallet, wtx).m_denominated, 0); // Once it is in the mempool they count. BOOST_CHECK(wallet->AddToWallet(tx, TxStateInMempool{})); - BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(1 * COIN), 1); + BOOST_CHECK_EQUAL(wallet->CountInputsWithAmount(denom), 1); + BOOST_CHECK_EQUAL(CachedTxGetAvailableCoinJoinCredits(*wallet, wtx).m_denominated, denom); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e45658e45f8e..0c0b9bb55b5a 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -338,13 +338,6 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void AddToSpends(const CWalletTx& wtx, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::set setWalletUTXO; - /** May `wtx`'s outputs in the wallet UTXO set be counted as wallet funds? - * - * setWalletUTXO holds every unspent output the wallet owns, including outputs of - * transactions that will never confirm as they stand: conflicted ones, and ones that - * were abandoned, never broadcast or rejected from the mempool. AvailableCoins() - * filters those out, so consumers reading setWalletUTXO directly must do the same. */ - bool IsWalletUTXOSpendable(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Add new UTXOs to the wallet UTXO set * * @param[in] tx Transaction to scan eligible UTXOs from @@ -645,6 +638,13 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati std::vector SelectFullyMixedForPromotion(int nDenom, int nCount) const; bool IsSpent(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + /** May `wtx`'s outputs be counted as wallet funds? + * + * The wallet knows about transactions that cannot confirm as they stand: conflicted + * ones, and ones that were abandoned, never broadcast or rejected from the mempool. + * AvailableCoins() filters their outputs out, so anything else that values wallet + * outputs must do the same. */ + bool IsWalletUTXOSpendable(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); // Whether this or any known UTXO with the same single key has been spent. bool IsSpentKey(const CScript& scriptPubKey) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); From 6b8b0b8000c3ee0f351fd5a556c6d6b8e8ba6ccb Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 22 Aug 2026 12:24:01 +0300 Subject: [PATCH 4/4] fix(wallet): invalidate the anonymizable tallies on mempool removal SelectCoinsGroupedByAddresses() serves its result from vecAnonymizableTallyCached before it looks at any transaction, so the liveness filter never runs on a cached call. A wallet transaction is trusted at depth zero while it sits in the mempool, so its outputs enter that cache; when it later leaves the mempool for any reason other than a conflict, transactionRemovedFromMempool() refreshes its state but left both cache flags set. The tally then kept handing out outputs of a transaction that can no longer confirm as it stands, until some unrelated event cleared the cache. Clear both flags on that transition, as the conflict path already does through SyncTransaction(). Co-Authored-By: Claude Opus 5 --- src/wallet/test/availablecoins_tests.cpp | 33 ++++++++++++++++++++++++ src/wallet/wallet.cpp | 5 ++++ 2 files changed, 38 insertions(+) diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp index 4984085b1943..b00faa631749 100644 --- a/src/wallet/test/availablecoins_tests.cpp +++ b/src/wallet/test/availablecoins_tests.cpp @@ -3,6 +3,7 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -121,5 +122,37 @@ BOOST_FIXTURE_TEST_CASE(UnconfirmableOutputsAreNotWalletFunds, AvailableCoinsTes BOOST_CHECK_EQUAL(CachedTxGetAvailableCoinJoinCredits(*wallet, wtx).m_denominated, denom); } +BOOST_FIXTURE_TEST_CASE(MempoolRemovalInvalidatesAnonymizableTally, AvailableCoinsTestingSetup) +{ + LOCK(wallet->cs_wallet); + + const auto dest{wallet->GetNewDestination("")}; + BOOST_ASSERT(dest); + + // A wallet transaction in the mempool is trusted at depth zero, so its outputs count + // towards the anonymizable tally and that tally is cacheable. + auto created{CreateTransaction(*wallet, {CRecipient{GetScriptForDestination(*dest), 1 * COIN, + /*fSubtractFeeFromAmount=*/false}}, + RANDOM_CHANGE_POSITION, CCoinControl{})}; + BOOST_REQUIRE(created); + const CTransactionRef tx{created->tx}; + BOOST_CHECK(wallet->AddToWallet(tx, TxStateInMempool{})); + + const auto tallied = [&](const CTxDestination& target) { + for (const auto& item : wallet->SelectCoinsGroupedByAddresses()) { + if (item.txdest == target) return true; + } + return false; + }; + + // Prime the cache, so that the check below cannot pass by recomputing the tally. + BOOST_REQUIRE(tallied(*dest)); + + // Leaving the mempool makes the transaction unconfirmable as it stands; the cached + // tally must not keep handing out its outputs. + wallet->transactionRemovedFromMempool(tx, MemPoolRemovalReason::EXPIRY); + BOOST_CHECK(!tallied(*dest)); +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c7b8a81b7580..5a2540b59aea 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1448,6 +1448,11 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRe auto it = mapWallet.find(tx->GetHash()); if (it != mapWallet.end()) { RefreshMempoolStatus(it->second, chain()); + // The transaction is inactive now, so its outputs stop counting as wallet + // funds. The anonymizable tallies are served from a cache that would keep + // handing out the old answer until some unrelated event cleared it. + fAnonymizableTallyCached = false; + fAnonymizableTallyCachedNonDenom = false; } } // Handle transactions that were removed from the mempool because they