From 9c02367479889daafb61da1553db0503af6d268f Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Tue, 17 Mar 2026 13:11:53 -0500 Subject: [PATCH 1/6] fix: add IsLockedCoin/IsSpent check in SelectFullyMixedForPromotion Add IsSpent/IsLockedCoin filtering to both CountCoinsByDenomination() and SelectFullyMixedForPromotion() to prevent concurrent CoinJoin sessions from selecting the same UTXOs. This matches the existing filtering pattern used in the denomination selector. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/wallet/coinjoin.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/coinjoin.cpp b/src/wallet/coinjoin.cpp index f9f5d772d3e2..aaa56dd7efd2 100644 --- a/src/wallet/coinjoin.cpp +++ b/src/wallet/coinjoin.cpp @@ -425,6 +425,8 @@ int CWallet::CountCoinsByDenomination(int nDenom, bool fFullyMixedOnly) const const auto it{mapWallet.find(outpoint.hash)}; if (it == mapWallet.end()) continue; + if (IsSpent(outpoint) || IsLockedCoin(outpoint)) continue; + const CAmount nValue = it->second.tx->vout[outpoint.n].nValue; if (nValue != nDenomAmount) continue; @@ -455,6 +457,8 @@ std::vector CWallet::SelectFullyMixedForPromotion(int nDenom, int nCo const auto it{mapWallet.find(outpoint.hash)}; if (it == mapWallet.end()) continue; + if (IsSpent(outpoint) || IsLockedCoin(outpoint)) continue; + const CAmount nValue = it->second.tx->vout[outpoint.n].nValue; if (nValue != nDenomAmount) continue; From 1ead9a5ebecce3c421ced3b5772873377f5fc383 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Tue, 17 Mar 2026 13:12:27 -0500 Subject: [PATCH 2/6] fix: add bounds checking for outpoint.n in coinjoin client Add bounds check for outpoint.n before accessing wtx.tx->vout in JoinExistingQueue and StartNewQueue promotion paths. This prevents potential out-of-bounds access if wallet state changes between UTXO selection and input construction. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/coinjoin/client.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 671fb5bce09b..d2e513987544 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -1209,6 +1209,10 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, const auto it = m_wallet->mapWallet.find(outpoint.hash); if (it != m_wallet->mapWallet.end()) { const wallet::CWalletTx& wtx = it->second; + if (outpoint.n >= wtx.tx->vout.size()) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- invalid outpoint index %u for tx %s\n", outpoint.n, outpoint.hash.ToString()); + continue; + } CTxDSIn txdsin(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); vecTxDSInTmp.push_back(txdsin); @@ -1411,6 +1415,10 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon const auto it = m_wallet->mapWallet.find(outpoint.hash); if (it != m_wallet->mapWallet.end()) { const wallet::CWalletTx& wtx = it->second; + if (outpoint.n >= wtx.tx->vout.size()) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- invalid outpoint index %u for tx %s\n", outpoint.n, outpoint.hash.ToString()); + continue; + } CTxDSIn txdsin(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); vecTxDSInTmp.push_back(txdsin); From 2578855688380f42fb098b23173e22e3c720de4c Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Tue, 17 Mar 2026 13:12:52 -0500 Subject: [PATCH 3/6] fix: acquire cs_main for V24 activation checks in server Wrap m_chainman.ActiveChain().Tip() accesses in LOCK(::cs_main) in ProcessDSVIN and AddEntry to prevent race conditions during V24 activation state checks. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/coinjoin/server.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index eff2980a11c0..59e04359795f 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -228,8 +228,12 @@ void CCoinJoinServer::ProcessDSVIN(CNode& peer, CDataStream& vRecv) // Post-V24: Check if unbalanced entries (promotion/demotion) are allowed if (entry.vecTxDSIn.size() != entry.vecTxOut.size()) { // This is a promotion or demotion entry - requires V24 activation - const CBlockIndex* pindex = m_chainman.ActiveChain().Tip(); - const bool fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + bool fV24Active{false}; + { + LOCK(::cs_main); + const CBlockIndex* pindex = m_chainman.ActiveChain().Tip(); + fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + } if (!fV24Active) { LogPrint(BCLog::COINJOIN, "DSVIN -- promotion/demotion entry rejected: V24 not active\n"); PushStatus(peer, STATUS_REJECTED, ERR_MODE); @@ -617,8 +621,12 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag // Post-V24: allow up to PROMOTION_RATIO (10) inputs for promotion entries // Pre-V24: max COINJOIN_ENTRY_MAX_SIZE (9) inputs - const CBlockIndex* pindex = m_chainman.ActiveChain().Tip(); - const bool fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + bool fV24Active{false}; + { + LOCK(::cs_main); + const CBlockIndex* pindex = m_chainman.ActiveChain().Tip(); + fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + } const size_t nMaxEntryInputs = fV24Active ? CoinJoin::PROMOTION_RATIO : COINJOIN_ENTRY_MAX_SIZE; if (entry.vecTxDSIn.size() > nMaxEntryInputs) { From 3af9ac2fa5e46861e6ab7c53e013ca3003d9609f Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Tue, 17 Mar 2026 13:13:13 -0500 Subject: [PATCH 4/6] fix: enforce standard mixer minimum on full-session finalization The full-session finalization path in CheckPool was creating the final transaction without verifying the minimum standard mixer count. This could allow a session with only promotion/demotion entries (which don't provide privacy) to finalize. Add the same GetStandardEntriesCount() check that the timeout path already uses. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/coinjoin/server.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 59e04359795f..f5af994a9a8b 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -298,9 +298,12 @@ void CCoinJoinServer::CheckPool() // If we have an entry for each collateral, then create final tx if (nState == POOL_STATE_ACCEPTING_ENTRIES && size_t(GetEntriesCount()) == vecSessionCollaterals.size()) { - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- FINALIZE TRANSACTIONS\n"); - CreateFinalTransaction(); - return; + if (GetStandardEntriesCount() >= CoinJoin::GetMinPoolParticipants()) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- FINALIZE TRANSACTIONS\n"); + CreateFinalTransaction(); + return; + } + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- all entries received but insufficient standard mixers (%d), waiting for timeout\n", GetStandardEntriesCount()); } // Check for Time Out From d9428e1064624949af70ec3ca05ab342a96f6483 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Tue, 17 Mar 2026 13:15:05 -0500 Subject: [PATCH 5/6] refactor: use ValidatePromotionEntry/ValidateDemotionEntry in IsValidInOuts Replace the inline promotion/demotion validation in IsValidInOuts() with calls to the dedicated CoinJoin::ValidatePromotionEntry() and CoinJoin::ValidateDemotionEntry() functions. These functions were already declared, implemented, and tested but never called from production code. This ensures the tested code IS the production code. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/coinjoin/coinjoin.cpp | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index cf7157097426..24545212edfb 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -256,32 +256,23 @@ bool CCoinJoinBaseSession::IsValidInOuts(CChainState& active_chainstate, const l return false; } - const int nLargerAdjacentDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); - - // Determine expected denominations based on entry type + // Validate promotion/demotion entries using dedicated validators + // and determine expected denominations for UTXO input validation int nExpectedInputDenom = nSessionDenom; int nExpectedOutputDenom = nSessionDenom; if (entryType == EntryType::PROMOTION) { - // Promotion: inputs = session denom (smaller), output = larger adjacent - nExpectedInputDenom = nSessionDenom; - nExpectedOutputDenom = nLargerAdjacentDenom; - if (nLargerAdjacentDenom == 0) { - LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- ERROR: no larger adjacent denom for promotion\n", __func__); - nMessageIDRet = ERR_DENOM; + if (!CoinJoin::ValidatePromotionEntry(vin, vout, nSessionDenom, nMessageIDRet)) { if (fConsumeCollateralRet) *fConsumeCollateralRet = true; return false; } + nExpectedOutputDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); } else if (entryType == EntryType::DEMOTION) { - // Demotion: input = larger adjacent, outputs = session denom (smaller) - nExpectedInputDenom = nLargerAdjacentDenom; - nExpectedOutputDenom = nSessionDenom; - if (nLargerAdjacentDenom == 0) { - LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- ERROR: no larger adjacent denom for demotion\n", __func__); - nMessageIDRet = ERR_DENOM; + if (!CoinJoin::ValidateDemotionEntry(vin, vout, nSessionDenom, nMessageIDRet)) { if (fConsumeCollateralRet) *fConsumeCollateralRet = true; return false; } + nExpectedInputDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); } auto checkTxOut = [&](const CTxOut& txout, int nExpectedDenom) { From cc50997142055a13b982cc2b88256b21989c8c55 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Tue, 17 Mar 2026 13:15:38 -0500 Subject: [PATCH 6/6] refactor: rename m_vecPromotionInputs to m_vecRebalanceInputs The member stores inputs for both promotion AND demotion operations, so the name m_vecPromotionInputs was misleading. Rename to m_vecRebalanceInputs to better reflect its dual purpose. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/coinjoin/client.cpp | 36 ++++++++++++++++++------------------ src/coinjoin/client.h | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index d2e513987544..d5a169c7f3d9 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -296,10 +296,10 @@ void CCoinJoinClientSession::SetNull() // Post-V24: Unlock promotion/demotion inputs before clearing state // These coins were locked in JoinExistingQueue/StartNewQueue but may not // have been added to vecOutPointLocked yet if the session failed early - if (!m_vecPromotionInputs.empty()) { + if (!m_vecRebalanceInputs.empty()) { // Add to vecOutPointLocked so UnlockCoins() will handle them properly // with its retry mechanism if the wallet is locked - for (const auto& outpoint : m_vecPromotionInputs) { + for (const auto& outpoint : m_vecRebalanceInputs) { // Only add if not already in the list (avoid duplicates) if (std::find(vecOutPointLocked.begin(), vecOutPointLocked.end(), outpoint) == vecOutPointLocked.end()) { vecOutPointLocked.push_back(outpoint); @@ -308,7 +308,7 @@ void CCoinJoinClientSession::SetNull() } m_fPromotion = false; m_fDemotion = false; - m_vecPromotionInputs.clear(); + m_vecRebalanceInputs.clear(); CCoinJoinBaseSession::SetNull(); } @@ -1282,23 +1282,23 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, // Store promotion inputs for use in PreparePromotionEntry if (fPromotion) { - m_vecPromotionInputs.clear(); + m_vecRebalanceInputs.clear(); for (const auto& txdsin : vecTxDSInTmp) { - m_vecPromotionInputs.push_back(txdsin.prevout); + m_vecRebalanceInputs.push_back(txdsin.prevout); } WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- pending PROMOTION connection, masternode=%s, nSessionDenom=%d (%s), %d inputs\n", dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), - m_vecPromotionInputs.size()); + m_vecRebalanceInputs.size()); } else if (fDemotion) { // For demotion, store the single input - m_vecPromotionInputs.clear(); + m_vecRebalanceInputs.clear(); if (!vecTxDSInTmp.empty()) { - m_vecPromotionInputs.push_back(vecTxDSInTmp[0].prevout); + m_vecRebalanceInputs.push_back(vecTxDSInTmp[0].prevout); } WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- pending DEMOTION connection, masternode=%s, nSessionDenom=%d (%s)\n", dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom)); } else { - m_vecPromotionInputs.clear(); + m_vecRebalanceInputs.clear(); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- pending connection, masternode=%s, nSessionDenom=%d (%s)\n", dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom)); } @@ -1508,15 +1508,15 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon // Store promotion/demotion state and inputs m_fPromotion = fPromotion; m_fDemotion = fDemotion; - m_vecPromotionInputs.clear(); + m_vecRebalanceInputs.clear(); for (const auto& txdsin : vecTxDSInTmp) { - m_vecPromotionInputs.push_back(txdsin.prevout); + m_vecRebalanceInputs.push_back(txdsin.prevout); } WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- pending %s connection, masternode=%s, nSessionDenom=%d (%s), %zu inputs\n", fPromotion ? "PROMOTION" : "DEMOTION", dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), - m_vecPromotionInputs.size()); + m_vecRebalanceInputs.size()); strAutoDenomResult = _("Trying to connect…"); return true; } @@ -1759,8 +1759,8 @@ bool CCoinJoinClientSession::PreparePromotionEntry(std::string& strErrorRet, std vecPSInOutPairsRet.clear(); - if (m_vecPromotionInputs.size() != static_cast(CoinJoin::PROMOTION_RATIO)) { - strErrorRet = strprintf("Invalid promotion input count: %d (expected %d)", m_vecPromotionInputs.size(), CoinJoin::PROMOTION_RATIO); + if (m_vecRebalanceInputs.size() != static_cast(CoinJoin::PROMOTION_RATIO)) { + strErrorRet = strprintf("Invalid promotion input count: %d (expected %d)", m_vecRebalanceInputs.size(), CoinJoin::PROMOTION_RATIO); return false; } @@ -1773,7 +1773,7 @@ bool CCoinJoinClientSession::PreparePromotionEntry(std::string& strErrorRet, std const CAmount nLargerAmount = CoinJoin::DenominationToAmount(nLargerDenom); // Create 10 inputs from stored promotion inputs - for (const auto& outpoint : m_vecPromotionInputs) { + for (const auto& outpoint : m_vecRebalanceInputs) { const auto it = m_wallet->mapWallet.find(outpoint.hash); if (it == m_wallet->mapWallet.end()) { strErrorRet = "Promotion input not found in wallet"; @@ -1830,8 +1830,8 @@ bool CCoinJoinClientSession::PrepareDemotionEntry(std::string& strErrorRet, std: vecPSInOutPairsRet.clear(); - if (m_vecPromotionInputs.size() != 1) { - strErrorRet = strprintf("Invalid demotion input count: %d (expected 1)", m_vecPromotionInputs.size()); + if (m_vecRebalanceInputs.size() != 1) { + strErrorRet = strprintf("Invalid demotion input count: %d (expected 1)", m_vecRebalanceInputs.size()); return false; } @@ -1844,7 +1844,7 @@ bool CCoinJoinClientSession::PrepareDemotionEntry(std::string& strErrorRet, std: } // Get the single input (larger denom) - const COutPoint& outpoint = m_vecPromotionInputs[0]; + const COutPoint& outpoint = m_vecRebalanceInputs[0]; const auto it = m_wallet->mapWallet.find(outpoint.hash); if (it == m_wallet->mapWallet.end()) { strErrorRet = "Demotion input not found in wallet"; diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 70ea99892dc3..f3672978e9d8 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -146,7 +146,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession // Post-V24: Promotion/demotion session state bool m_fPromotion{false}; // True if this session is promoting smaller -> larger denom bool m_fDemotion{false}; // True if this session is demoting larger -> smaller denom - std::vector m_vecPromotionInputs; // Selected inputs for promotion (10 coins) + std::vector m_vecRebalanceInputs; // Selected inputs for promotion/demotion rebalancing /// Create denominations bool CreateDenominated(CAmount nBalanceToDenominate);