Skip to content

fix(wallet): exclude unconfirmable outputs from CoinJoin accounting #7634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 4 commits into from
Aug 23, 2026
Merged

fix(wallet): exclude unconfirmable outputs from CoinJoin accounting #7634

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f44af84
fix(wallet): exclude unconfirmable outputs from CoinJoin accounting
UdjinM6 Aug 21, 2026
72ad8cb
doc: add release notes for #7634
UdjinM6 Aug 22, 2026
1255064
fix(wallet): apply the liveness filter to CoinJoin balances too
UdjinM6 Aug 22, 2026
6b8b0b8
fix(wallet): invalidate the anonymizable tallies on mempool removal
UdjinM6 Aug 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/wallet/coinjoin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ std::vector<CompactTallyItem> 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;

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Invalidate tallies when a transaction leaves the mempool

When a trusted wallet transaction is in the mempool, the default GetAnonymizableBalance() path can cache it before reaching this check on later calls. For non-conflict mempool removals, transactionRemovedFromMempool() only calls RefreshMempoolStatus() and does not clear fAnonymizableTallyCached or fAnonymizableTallyCachedNonDenom, so after eviction/expiry this function returns the cached tally at lines 137–147 without evaluating the new liveness condition. Automatic CoinJoin accounting can therefore continue using the unconfirmable outputs until an unrelated cache reset; invalidate these caches on the in-mempool-to-inactive transition or avoid caching zero-depth entries.

Useful? React with 👍 / 👎.

@thepastaclaw thepastaclaw Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Invalidate cached tallies when a transaction leaves the mempool

SelectCoinsGroupedByAddresses() returns vecAnonymizableTallyCached or vecAnonymizableTallyCachedNonDenom at lines 137-147 before reaching this new liveness predicate. A wallet-created zero-depth transaction can pass CachedTxIsTrusted() while it is in the mempool and populate either cache at lines 214-220. On non-conflict removals such as EXPIRY, SIZELIMIT, or MANUAL, CWallet::transactionRemovedFromMempool() changes that transaction from TxStateInMempool to TxStateInactive through RefreshMempoolStatus(), but does not clear either cache flag. Subsequent GetAnonymizableBalance() calls therefore continue counting the now-unconfirmable outputs until a block or unrelated wallet event invalidates the cache. Clear both tally caches on this mempool-state transition and add a regression test that primes the cache before removing the transaction from the mempool.

source: ['codex']

@thepastaclaw thepastaclaw Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in this update — Invalidate cached tallies when a transaction leaves the mempool no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.


for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) {
CTxDestination txdest;
Expand Down Expand Up @@ -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++;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Apply the liveness check to aggregate CoinJoin balances

Filtering only the normalized/rounds paths leaves the primary CoinJoin balances unchanged: GetBalance() still calls CachedTxGetAvailableCoinJoinCredits(), whose nDepth < 0 guard accepts an inactive depth-zero transaction and then classifies its denominations as denominated_trusted; CachedTxGetAnonymizedCredit() has the same gap for CoinJoin-only spending. Thus the transaction reproduced by the new test still inflates getbalances.mine.coinjoin, coinjoin_balance, the Qt denomination/anonymized figures, and—if its output has enough rounds—the CoinJoin send balance, while this newly filtered normalized value excludes it. Apply the same inactive/non-mempool rejection in both aggregate credit functions.

Useful? React with 👍 / 👎.


int nRounds = GetCappedOutpointCoinJoinRounds(outpoint);
nTotal += nValue * nRounds / CCoinJoinClientOptions::GetRounds();
Expand Down
22 changes: 22 additions & 0 deletions src/wallet/test/availablecoins_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 8 additions & 0 deletions src/wallet/wallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
7 changes: 7 additions & 0 deletions src/wallet/wallet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<COutPoint> 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
Expand Down
Loading