From 66a7db4337a127cc454a05a8fa456d561834392c Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 8 Dec 2025 08:42:24 -0600 Subject: [PATCH 01/12] feat: coinjoin promotion / demotion --- src/coinjoin/client.cpp | 514 +++++++++++++++- src/coinjoin/client.h | 35 +- src/coinjoin/coinjoin.cpp | 226 ++++++- src/coinjoin/coinjoin.h | 25 +- src/coinjoin/common.h | 51 ++ src/coinjoin/server.cpp | 27 + src/net_processing.cpp | 9 +- src/test/coinjoin_inouts_tests.cpp | 943 ++++++++++++++++++++++++++++- src/wallet/coinjoin.cpp | 68 ++- src/wallet/wallet.h | 18 + 10 files changed, 1871 insertions(+), 45 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index df14f62e717f..fbe04c142b2d 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -22,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -176,6 +178,11 @@ void CCoinJoinClientSession::SetNull() mixingMasternode = nullptr; pendingDsaRequest = CPendingDsaRequest(); + // Post-V24: Clear promotion/demotion session state + m_fPromotion = false; + m_fDemotion = false; + m_vecPromotionInputs.clear(); + CCoinJoinBaseSession::SetNull(); } @@ -375,13 +382,21 @@ bool CCoinJoinClientSession::SendDenominate(const std::vector vecTxOutTmp; for (const auto& [txDsIn, txOut] : vecPSInOutPairsIn) { - vecTxDSInTmp.emplace_back(txDsIn); - vecTxOutTmp.emplace_back(txOut); - tx.vin.emplace_back(txDsIn); - tx.vout.emplace_back(txOut); + // For promotion/demotion, filter out empty inputs/outputs + // Promotion: 10 inputs with only 1 real output (others are empty) + // Demotion: 1 input with 10 outputs (only first has real input) + if (!txDsIn.prevout.IsNull()) { + vecTxDSInTmp.emplace_back(txDsIn); + tx.vin.emplace_back(txDsIn); + } + if (txOut.nValue > 0) { + vecTxOutTmp.emplace_back(txOut); + tx.vout.emplace_back(txOut); + } } - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::SendDenominate -- Submitting partial tx %s", tx.ToString()); /* Continued */ + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::SendDenominate -- Submitting partial tx with %d inputs, %d outputs: %s", + vecTxDSInTmp.size(), vecTxOutTmp.size(), tx.ToString()); /* Continued */ // store our entry for later use LOCK(cs_coinjoin); @@ -846,6 +861,64 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman } } // LOCK(m_wallet->cs_wallet); + // Post-V24: Check if we should promote or demote denominations + // This helps maintain optimal denomination distribution as coins are spent + bool fV24Active{false}; + { + LOCK(::cs_main); + const CBlockIndex* pindex = chainman.ActiveChain().Tip(); + fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + } + + if (fV24Active) { + // Check all adjacent denomination pairs for promotion/demotion opportunities + // Denominations: 10, 1, 0.1, 0.01, 0.001 (indices 0-4, smaller index = larger denom) + for (int i = 0; i < 4; ++i) { + int nLargerDenom = 1 << i; // Larger denomination (e.g., 10 DASH) + int nSmallerDenom = 1 << (i + 1); // Smaller denomination (e.g., 1 DASH) + + // Check if we should promote smaller -> larger + if (m_clientman.ShouldPromote(nSmallerDenom, nLargerDenom)) { + // Verify we have enough fully-mixed coins for promotion + auto vecCoins = m_wallet->SelectFullyMixedForPromotion(nSmallerDenom, CoinJoin::PROMOTION_RATIO); + if (static_cast(vecCoins.size()) >= CoinJoin::PROMOTION_RATIO) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Promotion opportunity: " + "%d x %s -> 1 x %s\n", + CoinJoin::PROMOTION_RATIO, + CoinJoin::DenominationToString(nSmallerDenom), + CoinJoin::DenominationToString(nLargerDenom)); + + // Try to join an existing queue for promotion + if (JoinExistingQueue(nBalanceNeedsAnonymized, connman, nSmallerDenom, /*fPromotion=*/true)) { + return true; + } + // No existing queue found - try to start a new one for promotion + if (StartNewQueue(nBalanceNeedsAnonymized, connman, nSmallerDenom, /*fPromotion=*/true, /*fDemotion=*/false)) { + return true; + } + } + } + + // Check if we should demote larger -> smaller + if (m_clientman.ShouldDemote(nLargerDenom, nSmallerDenom)) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Demotion opportunity: " + "1 x %s -> %d x %s\n", + CoinJoin::DenominationToString(nLargerDenom), + CoinJoin::PROMOTION_RATIO, + CoinJoin::DenominationToString(nSmallerDenom)); + + // Try to join an existing queue for demotion + if (JoinExistingQueue(nBalanceNeedsAnonymized, connman, nSmallerDenom, /*fPromotion=*/false, /*fDemotion=*/true)) { + return true; + } + // No existing queue found - try to start a new one for demotion + if (StartNewQueue(nBalanceNeedsAnonymized, connman, nSmallerDenom, /*fPromotion=*/false, /*fDemotion=*/true)) { + return true; + } + } + } + } + // Always attempt to join an existing queue if (JoinExistingQueue(nBalanceNeedsAnonymized, connman)) { return true; @@ -957,10 +1030,14 @@ static int WinnersToSkip() ? 1 : 8; } -bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman) +bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman, + int nTargetDenom, bool fPromotion, bool fDemotion) { if (!CCoinJoinClientOptions::IsEnabled()) return false; + // Promotion and demotion are mutually exclusive + assert(!(fPromotion && fDemotion)); + const auto mnList = m_dmnman.GetListAtChainTip(); const int nWeightedMnCount = mnList.GetCounts().m_valid_weighted; @@ -986,12 +1063,64 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- trying queue: %s\n", dsq.ToString()); + // For promotion/demotion, we need a queue with the target denomination + if ((fPromotion || fDemotion) && nTargetDenom != 0) { + if (dsq.nDenom != nTargetDenom) { + continue; // Skip queues with wrong denomination + } + } + std::vector vecTxDSInTmp; - // Try to match their denominations if possible, select exact number of denominations - if (!m_wallet->SelectTxDSInsByDenomination(dsq.nDenom, nBalanceNeedsAnonymized, vecTxDSInTmp)) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Couldn't match denomination %d (%s)\n", dsq.nDenom, CoinJoin::DenominationToString(dsq.nDenom)); - continue; + if (fPromotion && nTargetDenom != 0) { + // Promotion: select 10 fully-mixed coins of the smaller denomination + auto vecCoins = m_wallet->SelectFullyMixedForPromotion(nTargetDenom, CoinJoin::PROMOTION_RATIO); + if (static_cast(vecCoins.size()) < CoinJoin::PROMOTION_RATIO) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Not enough fully-mixed coins for promotion\n"); + continue; + } + // Convert COutPoints to CTxDSIn + LOCK(m_wallet->cs_wallet); + for (const auto& outpoint : vecCoins) { + const auto it = m_wallet->mapWallet.find(outpoint.hash); + if (it != m_wallet->mapWallet.end()) { + const wallet::CWalletTx& wtx = it->second; + CTxDSIn txdsin(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, + m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); + vecTxDSInTmp.push_back(txdsin); + } + } + if (static_cast(vecTxDSInTmp.size()) < CoinJoin::PROMOTION_RATIO) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Failed to build promotion inputs\n"); + continue; + } + } else if (fDemotion && nTargetDenom != 0) { + // Demotion: select 1 coin of the larger denomination + const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nTargetDenom); + if (nLargerDenom == 0) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- No larger adjacent denom for demotion\n"); + continue; + } + // Select 1 coin of the larger denomination + // Prefer fully-mixed coins first (they're "done" mixing), then fall back to ready-to-mix + // According to plan: "Splitting allows unmixed" - demotion can use any denominated coin + if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInTmp, CoinType::ONLY_FULLY_MIXED)) { + // No fully-mixed coins available, try ready-to-mix + if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInTmp, CoinType::ONLY_READY_TO_MIX)) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Couldn't find coin for demotion\n"); + continue; + } + } + // Keep only 1 input for demotion + if (vecTxDSInTmp.size() > 1) { + vecTxDSInTmp.resize(1); + } + } else { + // Standard mixing: try to match their denominations if possible + if (!m_wallet->SelectTxDSInsByDenomination(dsq.nDenom, nBalanceNeedsAnonymized, vecTxDSInTmp)) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Couldn't match denomination %d (%s)\n", dsq.nDenom, CoinJoin::DenominationToString(dsq.nDenom)); + continue; + } } m_clientman.AddUsedMasternode(dmn->proTxHash); @@ -1008,9 +1137,36 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, connman.AddPendingMasternode(dmn->proTxHash); SetState(POOL_STATE_QUEUE); nTimeLastSuccessfulStep = GetTime(); - WalletCJLogPrint(m_wallet, /* Continued */ - "CCoinJoinClientSession::JoinExistingQueue -- pending connection, masternode=%s, nSessionDenom=%d (%s)\n", - dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom)); + + // Set promotion/demotion session state + m_fPromotion = fPromotion; + m_fDemotion = fDemotion; + + // Store promotion inputs for use in PreparePromotionEntry + if (fPromotion) { + m_vecPromotionInputs.clear(); + for (const auto& txdsin : vecTxDSInTmp) { + m_vecPromotionInputs.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()); + } else if (fDemotion) { + // For demotion, store the single input + m_vecPromotionInputs.clear(); + if (!vecTxDSInTmp.empty()) { + m_vecPromotionInputs.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(); + WalletCJLogPrint(m_wallet, /* Continued */ + "CCoinJoinClientSession::JoinExistingQueue -- pending connection, masternode=%s, nSessionDenom=%d (%s)\n", + dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom)); + } strAutoDenomResult = _("Trying to connect…"); return true; } @@ -1100,6 +1256,127 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon return false; } +bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman, + int nTargetDenom, bool fPromotion, bool fDemotion) +{ + assert(m_mn_metaman.IsValid()); + + if (!CCoinJoinClientOptions::IsEnabled()) return false; + if (nTargetDenom == 0) return false; + + // For promotion/demotion, verify we have the required coins before starting a queue + std::vector vecTxDSInTmp; + + if (fPromotion) { + // Promotion: need 10 fully-mixed coins of the target (smaller) denomination + auto vecCoins = m_wallet->SelectFullyMixedForPromotion(nTargetDenom, CoinJoin::PROMOTION_RATIO); + if (static_cast(vecCoins.size()) < CoinJoin::PROMOTION_RATIO) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- Not enough fully-mixed coins for promotion\n"); + return false; + } + // Convert to CTxDSIn for storage + LOCK(m_wallet->cs_wallet); + for (const auto& outpoint : vecCoins) { + const auto it = m_wallet->mapWallet.find(outpoint.hash); + if (it != m_wallet->mapWallet.end()) { + const wallet::CWalletTx& wtx = it->second; + CTxDSIn txdsin(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, + m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); + vecTxDSInTmp.push_back(txdsin); + } + } + if (static_cast(vecTxDSInTmp.size()) < CoinJoin::PROMOTION_RATIO) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- Failed to build promotion inputs\n"); + return false; + } + } else if (fDemotion) { + // Demotion: need 1 coin of the larger denomination + const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nTargetDenom); + if (nLargerDenom == 0) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- No larger adjacent denom for demotion\n"); + return false; + } + // Prefer fully-mixed coins first, then fall back to ready-to-mix + if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInTmp, CoinType::ONLY_FULLY_MIXED)) { + if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInTmp, CoinType::ONLY_READY_TO_MIX)) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- Couldn't find coin for demotion\n"); + return false; + } + } + if (vecTxDSInTmp.size() > 1) { + vecTxDSInTmp.resize(1); + } + } else { + // Neither promotion nor demotion - shouldn't use this overload + return false; + } + + int nTries = 0; + const auto mnList = m_dmnman.GetListAtChainTip(); + const int nMnCount = mnList.GetValidMNsCount(); + const int nWeightedMnCount = mnList.GetValidWeightedMNsCount(); + + while (nTries < 10) { + auto dmn = m_clientman.GetRandomNotUsedMasternode(); + if (!dmn) { + strAutoDenomResult = _("Can't find random Masternode."); + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- %s\n", strAutoDenomResult.original); + return false; + } + + m_clientman.AddUsedMasternode(dmn->proTxHash); + + // skip next mn payments winners + if (dmn->pdmnState->nLastPaidHeight + nWeightedMnCount < mnList.GetHeight() + WinnersToSkip()) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- skipping winner, masternode=%s\n", dmn->proTxHash.ToString()); + nTries++; + continue; + } + + if (m_mn_metaman.IsMixingThresholdExceeded(dmn->proTxHash, nMnCount)) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- too early to mix with node masternode=%s\n", + dmn->proTxHash.ToString()); + nTries++; + continue; + } + + if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->netInfo->GetPrimary())) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- skipping connection, masternode=%s\n", + dmn->proTxHash.ToString()); + nTries++; + continue; + } + + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- attempting %s connection, masternode=%s, tries=%d\n", + fPromotion ? "PROMOTION" : "DEMOTION", dmn->proTxHash.ToString(), nTries); + + nSessionDenom = nTargetDenom; + mixingMasternode = dmn; + connman.AddPendingMasternode(dmn->proTxHash); + pendingDsaRequest = CPendingDsaRequest(dmn->proTxHash, CCoinJoinAccept(nSessionDenom, txMyCollateral)); + SetState(POOL_STATE_QUEUE); + nTimeLastSuccessfulStep = GetTime(); + + // Store promotion/demotion state and inputs + m_fPromotion = fPromotion; + m_fDemotion = fDemotion; + m_vecPromotionInputs.clear(); + for (const auto& txdsin : vecTxDSInTmp) { + m_vecPromotionInputs.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()); + strAutoDenomResult = _("Trying to connect…"); + return true; + } + strAutoDenomResult = _("Failed to start a new mixing queue"); + return false; +} + bool CCoinJoinClientSession::ProcessPendingDsaRequest(CConnman& connman) { if (!pendingDsaRequest) return false; @@ -1182,9 +1459,32 @@ bool CCoinJoinClientSession::SubmitDenominate(CConnman& connman) LOCK(m_wallet->cs_wallet); std::string strError; - std::vector vecTxDSIn; std::vector > vecPSInOutPairsTmp; + // Post-V24: Handle promotion/demotion entries + if (m_fPromotion) { + if (PreparePromotionEntry(strError, vecPSInOutPairsTmp)) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::SubmitDenominate -- Promotion entry prepared, sending\n"); + return SendDenominate(vecPSInOutPairsTmp, connman); + } + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::SubmitDenominate -- PreparePromotionEntry failed: %s\n", strError); + strAutoDenomResult = Untranslated(strError); + return false; + } + + if (m_fDemotion) { + if (PrepareDemotionEntry(strError, vecPSInOutPairsTmp)) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::SubmitDenominate -- Demotion entry prepared, sending\n"); + return SendDenominate(vecPSInOutPairsTmp, connman); + } + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::SubmitDenominate -- PrepareDemotionEntry failed: %s\n", strError); + strAutoDenomResult = Untranslated(strError); + return false; + } + + // Standard 1:1 mixing + std::vector vecTxDSIn; + if (!SelectDenominate(strError, vecTxDSIn)) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::SubmitDenominate -- SelectDenominate failed, error: %s\n", strError); return false; @@ -1311,6 +1611,128 @@ bool CCoinJoinClientSession::PrepareDenominate(int nMinRounds, int nMaxRounds, s return true; } +bool CCoinJoinClientSession::PreparePromotionEntry(std::string& strErrorRet, std::vector>& vecPSInOutPairsRet) +{ + AssertLockHeld(m_wallet->cs_wallet); + + 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); + return false; + } + + // Session denom is the smaller denom (inputs), get the larger adjacent denom for output + const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); + if (nLargerDenom == 0) { + strErrorRet = "No larger adjacent denomination for promotion"; + return false; + } + const CAmount nLargerAmount = CoinJoin::DenominationToAmount(nLargerDenom); + + // Create 10 inputs from stored promotion inputs + for (const auto& outpoint : m_vecPromotionInputs) { + const auto it = m_wallet->mapWallet.find(outpoint.hash); + if (it == m_wallet->mapWallet.end()) { + strErrorRet = "Promotion input not found in wallet"; + return false; + } + const wallet::CWalletTx& wtx = it->second; + if (outpoint.n >= wtx.tx->vout.size()) { + strErrorRet = "Invalid promotion input index"; + return false; + } + + CTxDSIn txdsin(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, + m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); + + // For promotion, outputs are created but only 1 matters (the larger denom) + // We'll use empty CTxOut for all but the first to signal "no output for this input" + // Actually, for promotion entry: 10 inputs, 1 output + // We need to pair each input with an "empty" output, except the last gets the real output + vecPSInOutPairsRet.emplace_back(txdsin, CTxOut()); + } + + // Now set the single output (larger denomination) on the last entry + CScript scriptDenom = keyHolderStorage.AddKey(m_wallet.get()); + if (!vecPSInOutPairsRet.empty()) { + // Replace the last output with the actual promotion output + vecPSInOutPairsRet.back().second = CTxOut(nLargerAmount, scriptDenom); + } + + // Lock all inputs + for (const auto& [txDsIn, txDsOut] : vecPSInOutPairsRet) { + m_wallet->LockCoin(txDsIn.prevout); + vecOutPointLocked.push_back(txDsIn.prevout); + } + + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::PreparePromotionEntry -- Prepared %d inputs for promotion to %s\n", + vecPSInOutPairsRet.size(), CoinJoin::DenominationToString(nLargerDenom)); + + return true; +} + +bool CCoinJoinClientSession::PrepareDemotionEntry(std::string& strErrorRet, std::vector>& vecPSInOutPairsRet) +{ + AssertLockHeld(m_wallet->cs_wallet); + + vecPSInOutPairsRet.clear(); + + if (m_vecPromotionInputs.size() != 1) { + strErrorRet = strprintf("Invalid demotion input count: %d (expected 1)", m_vecPromotionInputs.size()); + return false; + } + + // Session denom is the smaller denom (outputs) + const CAmount nSmallerAmount = CoinJoin::DenominationToAmount(nSessionDenom); + const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); + if (nLargerDenom == 0) { + strErrorRet = "No larger adjacent denomination for demotion"; + return false; + } + + // Get the single input (larger denom) + const COutPoint& outpoint = m_vecPromotionInputs[0]; + const auto it = m_wallet->mapWallet.find(outpoint.hash); + if (it == m_wallet->mapWallet.end()) { + strErrorRet = "Demotion input not found in wallet"; + return false; + } + const wallet::CWalletTx& wtx = it->second; + if (outpoint.n >= wtx.tx->vout.size()) { + strErrorRet = "Invalid demotion input index"; + return false; + } + + CTxDSIn txdsin(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, + m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); + + // Create 10 outputs of smaller denomination + // For demotion: 1 input, 10 outputs + // The first pair has the real input, subsequent pairs have empty inputs + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + CScript scriptDenom = keyHolderStorage.AddKey(m_wallet.get()); + CTxOut txout(nSmallerAmount, scriptDenom); + + if (i == 0) { + // First entry has the real input + vecPSInOutPairsRet.emplace_back(txdsin, txout); + } else { + // Subsequent entries have empty inputs (will be filtered out when building entry) + vecPSInOutPairsRet.emplace_back(CTxDSIn(), txout); + } + } + + // Lock the input + m_wallet->LockCoin(txdsin.prevout); + vecOutPointLocked.push_back(txdsin.prevout); + + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::PrepareDemotionEntry -- Prepared 1 input for demotion to %d x %s\n", + CoinJoin::PROMOTION_RATIO, CoinJoin::DenominationToString(nSessionDenom)); + + return true; +} + // Create collaterals by looping through inputs grouped by addresses bool CCoinJoinClientSession::MakeCollateralAmounts() { @@ -1783,3 +2205,67 @@ UniValue CCoinJoinClientManager::getJsonInfo() const return obj; } +bool CCoinJoinClientManager::ShouldPromote(int nSmallerDenom, int nLargerDenom) const +{ + // Validate denominations are adjacent + if (!CoinJoin::AreAdjacentDenominations(nSmallerDenom, nLargerDenom)) { + return false; + } + + const int nGoal = CCoinJoinClientOptions::GetDenomsGoal(); + const int nHalfGoal = nGoal / 2; + + const int nSmallerCount = m_wallet->CountCoinsByDenomination(nSmallerDenom, /*fFullyMixedOnly=*/false); + const int nLargerCount = m_wallet->CountCoinsByDenomination(nLargerDenom, /*fFullyMixedOnly=*/false); + + // Don't sacrifice a denomination that's still being built up + if (nSmallerCount < nHalfGoal) { + return false; + } + + // Calculate how far each is from goal (0 if at or above goal) + const int nSmallerDeficit = std::max(0, nGoal - nSmallerCount); + const int nLargerDeficit = std::max(0, nGoal - nLargerCount); + + // Promote if: + // 1. Smaller denom has at least half the goal (above check) + // 2. Larger denomination is further from goal (needs more help) + // 3. Gap exceeds threshold to prevent oscillation + // 4. Have 10 fully-mixed coins to promote + const int nFullyMixedCount = m_wallet->CountCoinsByDenomination(nSmallerDenom, /*fFullyMixedOnly=*/true); + if (nFullyMixedCount < CoinJoin::PROMOTION_RATIO) { + return false; + } + + return (nLargerDeficit > nSmallerDeficit + CoinJoin::GAP_THRESHOLD); +} + +bool CCoinJoinClientManager::ShouldDemote(int nLargerDenom, int nSmallerDenom) const +{ + // Validate denominations are adjacent + if (!CoinJoin::AreAdjacentDenominations(nLargerDenom, nSmallerDenom)) { + return false; + } + + const int nGoal = CCoinJoinClientOptions::GetDenomsGoal(); + const int nHalfGoal = nGoal / 2; + + const int nLargerCount = m_wallet->CountCoinsByDenomination(nLargerDenom, /*fFullyMixedOnly=*/false); + const int nSmallerCount = m_wallet->CountCoinsByDenomination(nSmallerDenom, /*fFullyMixedOnly=*/false); + + // Don't sacrifice a denomination that's still being built up + if (nLargerCount < nHalfGoal) { + return false; + } + + // Calculate how far each is from goal (0 if at or above goal) + const int nSmallerDeficit = std::max(0, nGoal - nSmallerCount); + const int nLargerDeficit = std::max(0, nGoal - nLargerCount); + + // Demote if: + // 1. Larger denom has at least half the goal (above check) + // 2. Smaller denomination is further from goal (needs more help) + // 3. Gap exceeds threshold to prevent oscillation + return (nSmallerDeficit > nLargerDeficit + CoinJoin::GAP_THRESHOLD); +} + diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 9e4c514f23b8..cb7a719699df 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -84,6 +84,11 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession CKeyHolderStorage keyHolderStorage; // storage for keys used in PrepareDenominate + // 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) + /// Create denominations bool CreateDenominated(CAmount nBalanceToDenominate); bool CreateDenominated(CAmount nBalanceToDenominate, const wallet::CompactTallyItem& tallyItem, bool fCreateMixingCollaterals) @@ -97,8 +102,11 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession bool CreateCollateralTransaction(CMutableTransaction& txCollateral, std::string& strReason) EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet); - bool JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman); + bool JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman, + int nTargetDenom = 0, bool fPromotion = false, bool fDemotion = false); bool StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman); + bool StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman, + int nTargetDenom, bool fPromotion, bool fDemotion); /// step 0: select denominated inputs and txouts bool SelectDenominate(std::string& strErrorRet, std::vector& vecTxDSInRet); @@ -106,6 +114,15 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession bool PrepareDenominate(int nMinRounds, int nMaxRounds, std::string& strErrorRet, const std::vector& vecTxDSIn, std::vector>& vecPSInOutPairsRet, bool fDryRun = false) EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet); + + /// Post-V24: prepare promotion entry (10 inputs of smaller denom -> 1 output of larger denom) + bool PreparePromotionEntry(std::string& strErrorRet, std::vector>& vecPSInOutPairsRet) + EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet); + + /// Post-V24: prepare demotion entry (1 input of larger denom -> 10 outputs of smaller denom) + bool PrepareDemotionEntry(std::string& strErrorRet, std::vector>& vecPSInOutPairsRet) + EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet); + /// step 2: send denominated inputs and outputs prepared in step 1 bool SendDenominate(const std::vector >& vecPSInOutPairsIn, CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); @@ -231,6 +248,22 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client bool isMixing() const override; bool startMixing() override; void stopMixing() override; + + /** + * Post-V24: Check if we should promote smaller denominations into larger ones + * @param nSmallerDenom The smaller denomination to promote from + * @param nLargerDenom The larger denomination to promote into + * @return true if promotion is recommended + */ + bool ShouldPromote(int nSmallerDenom, int nLargerDenom) const; + + /** + * Post-V24: Check if we should demote larger denominations into smaller ones + * @param nLargerDenom The larger denomination to demote from + * @param nSmallerDenom The smaller denomination to demote into + * @return true if demotion is recommended + */ + bool ShouldDemote(int nLargerDenom, int nSmallerDenom) const; }; #endif // BITCOIN_COINJOIN_CLIENT_H diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index ae0506c39fea..60c651ca5301 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -83,24 +84,55 @@ bool CCoinJoinBroadcastTx::CheckSignature(const CBLSPublicKey& blsPubKey) const return true; } -bool CCoinJoinBroadcastTx::IsValidStructure() const +bool CCoinJoinBroadcastTx::IsExpired(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler) const +{ + // expire confirmed DSTXes after ~1h since confirmation or chainlocked confirmation + if (!nConfirmedHeight.has_value() || pindex->nHeight < *nConfirmedHeight) return false; // not mined yet + if (pindex->nHeight - *nConfirmedHeight > 24) return true; // mined more than an hour ago + return clhandler.HasChainLock(pindex->nHeight, *pindex->phashBlock); +} + +bool CCoinJoinBroadcastTx::IsValidStructure(const CBlockIndex* pindex) const { // some trivial checks only if (masternodeOutpoint.IsNull() && m_protxHash.IsNull()) { return false; } - if (tx->vin.size() != tx->vout.size()) { - return false; + + const bool fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + + // Pre-V24: require balanced input/output counts (1:1 mixing only) + // Post-V24: allow unbalanced counts (promotion/demotion) + if (!fV24Active) { + if (tx->vin.size() != tx->vout.size()) { + return false; + } } + if (tx->vin.size() < size_t(CoinJoin::GetMinPoolParticipants())) { return false; } - if (tx->vin.size() > CoinJoin::GetMaxPoolInputOutputCount()) { + // Post-V24: allow up to 200 inputs (20 participants * 10 inputs for promotions) + // Pre-V24: max 180 inputs (20 participants * 9 entries) + const size_t nMaxInputs = fV24Active + ? CoinJoin::GetMaxPoolParticipants() * CoinJoin::PROMOTION_RATIO + : CoinJoin::GetMaxPoolInputOutputCount(); + + if (tx->vin.size() > nMaxInputs) { return false; } - return std::ranges::all_of(tx->vout, [](const auto& txOut) { + + if (!std::ranges::all_of(tx->vout, [](const auto& txOut) { return CoinJoin::IsDenominatedAmount(txOut.nValue) && txOut.scriptPubKey.IsPayToPublicKeyHash(); - }); + })) { + return false; + } + + // Note: For post-V24 unbalanced transactions (promotion/demotion), + // value sum validation (inputs == outputs) requires UTXO access and + // is performed in IsValidInOuts() when the transaction is processed. + + return true; } void CCoinJoinBaseSession::SetNull() @@ -208,17 +240,80 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll nMessageIDRet = MSG_NOERR; if (fConsumeCollateralRet) *fConsumeCollateralRet = false; - if (vin.size() != vout.size()) { + // Check if V24 is active for promotion/demotion support + bool fV24Active{false}; + { + LOCK(::cs_main); + const CBlockIndex* pindex = active_chainstate.m_chain.Tip(); + fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + } + + // Determine entry type based on input/output counts + // Standard: N inputs, N outputs (same denom) + // Promotion: PROMOTION_RATIO inputs of session denom, 1 output of larger adjacent denom + // Demotion: 1 input of larger adjacent denom, PROMOTION_RATIO outputs of session denom + enum class EntryType { STANDARD, PROMOTION, DEMOTION, INVALID }; + EntryType entryType = EntryType::STANDARD; + + if (vin.size() == vout.size()) { + entryType = EntryType::STANDARD; + } else if (fV24Active) { + if (vin.size() == static_cast(CoinJoin::PROMOTION_RATIO) && vout.size() == 1) { + entryType = EntryType::PROMOTION; + } else if (vin.size() == 1 && vout.size() == static_cast(CoinJoin::PROMOTION_RATIO)) { + entryType = EntryType::DEMOTION; + } else { + entryType = EntryType::INVALID; + } + } else { + // Pre-V24: only standard entries allowed LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- ERROR: inputs vs outputs size mismatch! %d vs %d\n", __func__, vin.size(), vout.size()); nMessageIDRet = ERR_SIZE_MISMATCH; if (fConsumeCollateralRet) *fConsumeCollateralRet = true; return false; } - auto checkTxOut = [&](const CTxOut& txout) { - if (int nDenom = CoinJoin::AmountToDenomination(txout.nValue); nDenom != nSessionDenom) { - LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::IsValidInOuts -- ERROR: incompatible denom %d (%s) != nSessionDenom %d (%s)\n", - nDenom, CoinJoin::DenominationToString(nDenom), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom)); + if (entryType == EntryType::INVALID) { + LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- ERROR: invalid entry structure! %d inputs, %d outputs\n", __func__, vin.size(), vout.size()); + nMessageIDRet = ERR_SIZE_MISMATCH; + if (fConsumeCollateralRet) *fConsumeCollateralRet = true; + return false; + } + + const int nLargerAdjacentDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); + + // Determine expected denominations based on entry type + 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 (fConsumeCollateralRet) *fConsumeCollateralRet = true; + return false; + } + } 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 (fConsumeCollateralRet) *fConsumeCollateralRet = true; + return false; + } + } + + auto checkTxOut = [&](const CTxOut& txout, int nExpectedDenom) { + const int nDenom = CoinJoin::AmountToDenomination(txout.nValue); + + if (nDenom != nExpectedDenom) { + LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::IsValidInOuts -- ERROR: incompatible denom %d (%s) != expected %d (%s)\n", + nDenom, CoinJoin::DenominationToString(nDenom), nExpectedDenom, CoinJoin::DenominationToString(nExpectedDenom)); nMessageIDRet = ERR_DENOM; if (fConsumeCollateralRet) *fConsumeCollateralRet = true; return false; @@ -229,21 +324,20 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll if (fConsumeCollateralRet) *fConsumeCollateralRet = true; return false; } + // Check for duplicate scripts across all inputs and outputs (privacy requirement) if (!setScripPubKeys.insert(txout.scriptPubKey).second) { LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::IsValidInOuts -- ERROR: already have this script! scriptPubKey=%s\n", ScriptToAsmStr(txout.scriptPubKey)); nMessageIDRet = ERR_ALREADY_HAVE; if (fConsumeCollateralRet) *fConsumeCollateralRet = true; return false; } - // IsPayToPublicKeyHash() above already checks for scriptPubKey size, - // no need to double-check, hence no usage of ERR_NON_STANDARD_PUBKEY return true; }; CAmount nFees{0}; for (const auto& txout : vout) { - if (!checkTxOut(txout)) { + if (!checkTxOut(txout, nExpectedOutputDenom)) { return false; } nFees -= txout.nValue; @@ -269,21 +363,26 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll return false; } - if (!checkTxOut(coin.out)) { + if (!checkTxOut(coin.out, nExpectedInputDenom)) { return false; } nFees += coin.out.nValue; } - // The same size and denom for inputs and outputs ensures their total value is also the same, - // no need to double-check. If not, we are doing something wrong, bail out. + // Value sum must match: inputs == outputs (no fees in CoinJoin) + // This holds for standard mixing (same denom) and promotion/demotion (value preserved) if (nFees != 0) { LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- ERROR: non-zero fees! fees: %lld\n", __func__, nFees); nMessageIDRet = ERR_FEES; return false; } + LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- Valid %s entry: %d inputs, %d outputs\n", + __func__, + entryType == EntryType::PROMOTION ? "PROMOTION" : (entryType == EntryType::DEMOTION ? "DEMOTION" : "STANDARD"), + vin.size(), vout.size()); + return true; } @@ -507,3 +606,96 @@ void CDSTXManager::BlockDisconnected(const std::shared_ptr& pblock int CoinJoin::GetMinPoolParticipants() { return Params().PoolMinParticipants(); } int CoinJoin::GetMaxPoolParticipants() { return Params().PoolMaxParticipants(); } + +bool CoinJoin::ValidatePromotionEntry(const std::vector& vecTxIn, const std::vector& vecTxOut, + int nSessionDenom, PoolMessage& nMessageIDRet) +{ + // Promotion: 10 inputs of smaller denom → 1 output of larger denom + // Session denom is the smaller denom (inputs) + nMessageIDRet = MSG_NOERR; + + // Check input count + if (vecTxIn.size() != static_cast(PROMOTION_RATIO)) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidatePromotionEntry -- ERROR: wrong input count %zu, expected %d\n", + vecTxIn.size(), PROMOTION_RATIO); + nMessageIDRet = ERR_SIZE_MISMATCH; + return false; + } + + // Check output count + if (vecTxOut.size() != 1) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidatePromotionEntry -- ERROR: wrong output count %zu, expected 1\n", + vecTxOut.size()); + nMessageIDRet = ERR_SIZE_MISMATCH; + return false; + } + + // Get the larger adjacent denomination + const int nLargerDenom = GetLargerAdjacentDenom(nSessionDenom); + if (nLargerDenom == 0) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidatePromotionEntry -- ERROR: no larger adjacent denom for %s\n", + DenominationToString(nSessionDenom)); + nMessageIDRet = ERR_DENOM; + return false; + } + + // Validate output is at larger denomination + const int nOutputDenom = AmountToDenomination(vecTxOut[0].nValue); + if (nOutputDenom != nLargerDenom) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidatePromotionEntry -- ERROR: output denom %s != expected %s\n", + DenominationToString(nOutputDenom), DenominationToString(nLargerDenom)); + nMessageIDRet = ERR_DENOM; + return false; + } + + // Validate output is P2PKH + if (!vecTxOut[0].scriptPubKey.IsPayToPublicKeyHash()) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidatePromotionEntry -- ERROR: output is not P2PKH\n"); + nMessageIDRet = ERR_INVALID_SCRIPT; + return false; + } + + return true; +} + +bool CoinJoin::ValidateDemotionEntry(const std::vector& vecTxIn, const std::vector& vecTxOut, + int nSessionDenom, PoolMessage& nMessageIDRet) +{ + // Demotion: 1 input of larger denom → 10 outputs of smaller denom + // Session denom is the smaller denom (outputs) + nMessageIDRet = MSG_NOERR; + + // Check input count + if (vecTxIn.size() != 1) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidateDemotionEntry -- ERROR: wrong input count %zu, expected 1\n", + vecTxIn.size()); + nMessageIDRet = ERR_SIZE_MISMATCH; + return false; + } + + // Check output count + if (vecTxOut.size() != static_cast(PROMOTION_RATIO)) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidateDemotionEntry -- ERROR: wrong output count %zu, expected %d\n", + vecTxOut.size(), PROMOTION_RATIO); + nMessageIDRet = ERR_SIZE_MISMATCH; + return false; + } + + // Validate all outputs are at session denomination and P2PKH + for (const auto& txout : vecTxOut) { + const int nDenom = AmountToDenomination(txout.nValue); + if (nDenom != nSessionDenom) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidateDemotionEntry -- ERROR: output denom %s != session denom %s\n", + DenominationToString(nDenom), DenominationToString(nSessionDenom)); + nMessageIDRet = ERR_DENOM; + return false; + } + if (!txout.scriptPubKey.IsPayToPublicKeyHash()) { + LogPrint(BCLog::COINJOIN, "CoinJoin::ValidateDemotionEntry -- ERROR: output is not P2PKH\n"); + nMessageIDRet = ERR_INVALID_SCRIPT; + return false; + } + } + + return true; +} diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index 801ce3b43611..461cc4ef9218 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -310,7 +310,8 @@ class CCoinJoinBroadcastTx [[nodiscard]] const std::optional& GetConfirmedHeight() const { return nConfirmedHeight; } void SetConfirmedHeight(std::optional nConfirmedHeightIn) { assert(nConfirmedHeightIn == std::nullopt || *nConfirmedHeightIn > 0); nConfirmedHeight = nConfirmedHeightIn; } - [[nodiscard]] bool IsValidStructure() const; + [[nodiscard]] bool IsExpired(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler) const; + [[nodiscard]] bool IsValidStructure(const CBlockIndex* pindex) const; }; // base class @@ -411,6 +412,28 @@ namespace CoinJoin /// If the collateral is valid given by a client bool IsCollateralValid(ChainstateManager& chainman, const llmq::CInstantSendManager& isman, const CTxMemPool& mempool, const CTransaction& txCollateral); + + /** + * Validate a promotion entry: 10 inputs of smaller denom → 1 output of larger denom + * @param vecTxIn The inputs for this entry + * @param vecTxOut The outputs for this entry + * @param nSessionDenom The session denomination (the smaller denom for promotion) + * @param nMessageIDRet Error message if validation fails + * @return true if valid promotion entry + */ + bool ValidatePromotionEntry(const std::vector& vecTxIn, const std::vector& vecTxOut, + int nSessionDenom, PoolMessage& nMessageIDRet); + + /** + * Validate a demotion entry: 1 input of larger denom → 10 outputs of smaller denom + * @param vecTxIn The inputs for this entry + * @param vecTxOut The outputs for this entry + * @param nSessionDenom The session denomination (the smaller denom for demotion outputs) + * @param nMessageIDRet Error message if validation fails + * @return true if valid demotion entry + */ + bool ValidateDemotionEntry(const std::vector& vecTxIn, const std::vector& vecTxOut, + int nSessionDenom, PoolMessage& nMessageIDRet); } class CDSTXManager diff --git a/src/coinjoin/common.h b/src/coinjoin/common.h index 3d97d639434d..404a415348d5 100644 --- a/src/coinjoin/common.h +++ b/src/coinjoin/common.h @@ -110,6 +110,57 @@ std::string DenominationToString(int nDenom); constexpr CAmount GetCollateralAmount() { return GetSmallestDenomination() / 10; } constexpr CAmount GetMaxCollateralAmount() { return GetCollateralAmount() * 4; } +// Promotion/demotion constants (post-V24 feature) +constexpr int PROMOTION_RATIO = 10; // 10 smaller denomination coins = 1 larger denomination coin +constexpr int GAP_THRESHOLD = 10; // Deficit gap required to trigger promotion/demotion + +/** + * Get the index of a denomination in vecStandardDenominations (0=largest, 4=smallest) + * Returns -1 if not a valid denomination + */ +constexpr int GetDenominationIndex(int nDenom) +{ + if (nDenom <= 0) return -1; + for (size_t i = 0; i < vecStandardDenominations.size(); ++i) { + if (nDenom == (1 << i)) { + return static_cast(i); + } + } + return -1; +} + +/** + * Check if two denominations are adjacent (one step apart in the denom list) + * Used for validating promotion/demotion entries post-V24 + */ +constexpr bool AreAdjacentDenominations(int nDenom1, int nDenom2) +{ + int idx1 = GetDenominationIndex(nDenom1); + int idx2 = GetDenominationIndex(nDenom2); + if (idx1 < 0 || idx2 < 0) return false; + return (idx1 == idx2 + 1) || (idx1 == idx2 - 1); +} + +/** + * Get the larger adjacent denomination (returns 0 if none exists or invalid) + */ +constexpr int GetLargerAdjacentDenom(int nDenom) +{ + int idx = GetDenominationIndex(nDenom); + if (idx <= 0) return 0; // Already largest or invalid + return 1 << (idx - 1); +} + +/** + * Get the smaller adjacent denomination (returns 0 if none exists or invalid) + */ +constexpr int GetSmallerAdjacentDenom(int nDenom) +{ + int idx = GetDenominationIndex(nDenom); + if (idx < 0 || idx >= static_cast(vecStandardDenominations.size()) - 1) return 0; + return 1 << (idx + 1); +} + constexpr bool IsCollateralAmount(CAmount nInputAmount) { // collateral input can be anything between 1x and "max" (including both) diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index d30a7bd376ad..134d01d3e631 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -5,6 +5,8 @@ #include #include +#include +#include #include #include #include @@ -210,6 +212,18 @@ void CCoinJoinServer::ProcessDSVIN(CNode& peer, CDataStream& vRecv) LogPrint(BCLog::COINJOIN, "DSVIN -- txCollateral %s", entry.txCollateral->ToString()); /* Continued */ + // 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); + if (!fV24Active) { + LogPrint(BCLog::COINJOIN, "DSVIN -- promotion/demotion entry rejected: V24 not active\n"); + PushStatus(peer, STATUS_REJECTED, ERR_MODE); + return; + } + } + PoolMessage nMessageID = MSG_NOERR; entry.addr = peer.addr; @@ -638,6 +652,19 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag return false; } + // 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); + const size_t nMaxEntryInputs = fV24Active ? CoinJoin::PROMOTION_RATIO : COINJOIN_ENTRY_MAX_SIZE; + + if (entry.vecTxDSIn.size() > nMaxEntryInputs) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: too many inputs! %d/%d\n", __func__, entry.vecTxDSIn.size(), nMaxEntryInputs); + nMessageIDRet = ERR_MAXIMUM; + ConsumeCollateral(entry.txCollateral); + return false; + } + std::vector vin; for (const auto& txin : entry.vecTxDSIn) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- txin=%s\n", __func__, txin.ToString()); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b8d059ae1abd..9d71a051e949 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3586,10 +3586,6 @@ static DSTXValidationResult ValidateDSTX(CDeterministicMNManager& dmnman, CDSTXM { assert(mn_metaman.IsValid()); - if (!dstx.IsValidStructure()) { - LogPrint(BCLog::COINJOIN, "DSTX -- Invalid DSTX structure: %s\n", hashTx.ToString()); - return {DSTXValidationScore::INVALID, true}; - } if (dstxman.GetDSTX(hashTx)) { LogPrint(BCLog::COINJOIN, "DSTX -- Already have %s, skipping...\n", hashTx.ToString()); return {DSTXValidationScore::NONE, true}; // not an error @@ -3601,6 +3597,11 @@ static DSTXValidationResult ValidateDSTX(CDeterministicMNManager& dmnman, CDSTXM LOCK(cs_main); pindex = chainman.ActiveChain().Tip(); } + + if (!dstx.IsValidStructure(pindex)) { + LogPrint(BCLog::COINJOIN, "DSTX -- Invalid DSTX structure: %s\n", hashTx.ToString()); + return {DSTXValidationScore::INVALID, true}; + } // It could be that a MN is no longer in the list but its DSTX is not yet mined. // Try to find a MN up to 24 blocks deep to make sure such dstx-es are relayed and processed correctly. if (dstx.masternodeOutpoint.IsNull()) { diff --git a/src/test/coinjoin_inouts_tests.cpp b/src/test/coinjoin_inouts_tests.cpp index 70c81c87ce6c..121a6289a8de 100644 --- a/src/test/coinjoin_inouts_tests.cpp +++ b/src/test/coinjoin_inouts_tests.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -64,22 +65,23 @@ BOOST_AUTO_TEST_CASE(broadcasttx_isvalidstructure_good_and_bad) good.tx = MakeTransactionRef(mtx); good.m_protxHash = uint256::ONE; // at least one of (outpoint, protxhash) must be set } - BOOST_CHECK(good.IsValidStructure()); + // Pre-V24 behavior (nullptr pindex = pre-fork) + BOOST_CHECK(good.IsValidStructure(nullptr)); // Bad: both identifiers null CCoinJoinBroadcastTx bad_ids = good; bad_ids.m_protxHash = uint256{}; bad_ids.masternodeOutpoint.SetNull(); - BOOST_CHECK(!bad_ids.IsValidStructure()); + BOOST_CHECK(!bad_ids.IsValidStructure(nullptr)); - // Bad: vin/vout size mismatch + // Bad: vin/vout size mismatch (invalid pre-V24) CCoinJoinBroadcastTx bad_sizes = good; { CMutableTransaction mtx(*good.tx); mtx.vout.pop_back(); bad_sizes.tx = MakeTransactionRef(mtx); } - BOOST_CHECK(!bad_sizes.IsValidStructure()); + BOOST_CHECK(!bad_sizes.IsValidStructure(nullptr)); // Bad: non-P2PKH output CCoinJoinBroadcastTx bad_script = good; @@ -88,7 +90,7 @@ BOOST_AUTO_TEST_CASE(broadcasttx_isvalidstructure_good_and_bad) mtx.vout[0].scriptPubKey = CScript() << OP_RETURN << std::vector{'x'}; bad_script.tx = MakeTransactionRef(mtx); } - BOOST_CHECK(!bad_script.IsValidStructure()); + BOOST_CHECK(!bad_script.IsValidStructure(nullptr)); // Bad: non-denominated amount CCoinJoinBroadcastTx bad_amount = good; @@ -97,7 +99,7 @@ BOOST_AUTO_TEST_CASE(broadcasttx_isvalidstructure_good_and_bad) mtx.vout[0].nValue = 42; // not a valid denom bad_amount.tx = MakeTransactionRef(mtx); } - BOOST_CHECK(!bad_amount.IsValidStructure()); + BOOST_CHECK(!bad_amount.IsValidStructure(nullptr)); } BOOST_AUTO_TEST_CASE(entry_addscriptsig_matches_and_rejects) @@ -355,4 +357,933 @@ BOOST_AUTO_TEST_CASE(queue_timeout_bounds) // Reset mock time SetMockTime(0); } +BOOST_AUTO_TEST_CASE(broadcasttx_expiry_height_logic) +{ + // Build a valid-looking CCoinJoinBroadcastTx with confirmed height + CCoinJoinBroadcastTx dstx; + { + CMutableTransaction mtx; + const int participants = std::max(3, CoinJoin::GetMinPoolParticipants()); + for (int i = 0; i < participants; ++i) { + mtx.vin.emplace_back(COutPoint(uint256::TWO, i)); + mtx.vout.emplace_back(CoinJoin::GetSmallestDenomination(), P2PKHScript(static_cast(i))); + } + dstx.tx = MakeTransactionRef(mtx); + dstx.m_protxHash = uint256::ONE; + // mark as confirmed at height 100 + dstx.SetConfirmedHeight(100); + } + + // Minimal CBlockIndex with required fields + // Create a minimal block index to satisfy the interface + CBlockIndex index; + uint256 blk_hash = uint256S("03"); + index.nHeight = 125; // 125 - 100 == 25 > 24 → expired by height + index.phashBlock = &blk_hash; + BOOST_CHECK(dstx.IsExpired(&index, *Assert(m_node.llmq_ctx->clhandler))); +} + +// Helper to create a denominated CTxIn with a specific denomination value +static CTxIn MakeDenomInput(uint8_t index) +{ + CTxIn in; + in.prevout = COutPoint(uint256::ONE, index); + return in; +} + +// Helper to create a denominated CTxOut +static CTxOut MakeDenomOutput(CAmount nAmount, uint8_t tag = 0x01) +{ + return CTxOut{nAmount, P2PKHScript(tag)}; +} + +BOOST_AUTO_TEST_CASE(validate_promotion_entry_valid) +{ + // Valid promotion: 10 inputs of 0.1 DASH → 1 output of 1.0 DASH + std::vector vecTxIn; + std::vector vecTxOut; + + // Get the 0.1 DASH denomination (index 2: 1 << 2 = 4) + const int nSmallerDenom = 1 << 2; // 0.1 DASH + const int nLargerDenom = 1 << 1; // 1.0 DASH + const CAmount nLargerAmount = CoinJoin::DenominationToAmount(nLargerDenom); + + BOOST_CHECK(CoinJoin::IsValidDenomination(nSmallerDenom)); + BOOST_CHECK(CoinJoin::IsValidDenomination(nLargerDenom)); + + // Create 10 inputs of smaller denomination + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + vecTxIn.push_back(MakeDenomInput(static_cast(i))); + } + + // Create 1 output of larger denomination + vecTxOut.push_back(MakeDenomOutput(nLargerAmount, 0x01)); + + PoolMessage nMessageID = MSG_NOERR; + BOOST_CHECK(CoinJoin::ValidatePromotionEntry(vecTxIn, vecTxOut, nSmallerDenom, nMessageID)); +} + +BOOST_AUTO_TEST_CASE(validate_promotion_entry_wrong_input_count) +{ + // Invalid: only 9 inputs instead of 10 + std::vector vecTxIn; + std::vector vecTxOut; + + const int nSmallerDenom = 1 << 2; // 0.1 DASH + const int nLargerDenom = 1 << 1; // 1.0 DASH + const CAmount nLargerAmount = CoinJoin::DenominationToAmount(nLargerDenom); + + // Create only 9 inputs + for (int i = 0; i < CoinJoin::PROMOTION_RATIO - 1; ++i) { + vecTxIn.push_back(MakeDenomInput(static_cast(i))); + } + + vecTxOut.push_back(MakeDenomOutput(nLargerAmount, 0x01)); + + PoolMessage nMessageID = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidatePromotionEntry(vecTxIn, vecTxOut, nSmallerDenom, nMessageID)); +} + +BOOST_AUTO_TEST_CASE(validate_promotion_entry_wrong_output_count) +{ + // Invalid: 2 outputs instead of 1 + std::vector vecTxIn; + std::vector vecTxOut; + + const int nSmallerDenom = 1 << 2; + const int nLargerDenom = 1 << 1; + const CAmount nLargerAmount = CoinJoin::DenominationToAmount(nLargerDenom); + + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + vecTxIn.push_back(MakeDenomInput(static_cast(i))); + } + + // Two outputs instead of one + vecTxOut.push_back(MakeDenomOutput(nLargerAmount, 0x01)); + vecTxOut.push_back(MakeDenomOutput(nLargerAmount, 0x02)); + + PoolMessage nMessageID = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidatePromotionEntry(vecTxIn, vecTxOut, nSmallerDenom, nMessageID)); +} + +BOOST_AUTO_TEST_CASE(validate_promotion_entry_non_adjacent_denoms) +{ + // Invalid: trying to promote 0.01 to 1.0 (not adjacent) + std::vector vecTxIn; + std::vector vecTxOut; + + const int nSmallerDenom = 1 << 3; // 0.01 DASH + const int nLargerDenom = 1 << 1; // 1.0 DASH (not adjacent to 0.01) + const CAmount nLargerAmount = CoinJoin::DenominationToAmount(nLargerDenom); + + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + vecTxIn.push_back(MakeDenomInput(static_cast(i))); + } + + vecTxOut.push_back(MakeDenomOutput(nLargerAmount, 0x01)); + + PoolMessage nMessageID = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidatePromotionEntry(vecTxIn, vecTxOut, nSmallerDenom, nMessageID)); +} + +BOOST_AUTO_TEST_CASE(validate_demotion_entry_valid) +{ + // Valid demotion: 1 input of 1.0 DASH → 10 outputs of 0.1 DASH + std::vector vecTxIn; + std::vector vecTxOut; + + const int nSmallerDenom = 1 << 2; // 0.1 DASH + const CAmount nSmallerAmount = CoinJoin::DenominationToAmount(nSmallerDenom); + const int nLargerDenom = 1 << 1; // 1.0 DASH + + BOOST_CHECK(CoinJoin::IsValidDenomination(nSmallerDenom)); + BOOST_CHECK(CoinJoin::IsValidDenomination(nLargerDenom)); + BOOST_CHECK(CoinJoin::AreAdjacentDenominations(nSmallerDenom, nLargerDenom)); + + // 1 input of larger denomination + vecTxIn.push_back(MakeDenomInput(0)); + + // 10 outputs of smaller denomination + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + vecTxOut.push_back(MakeDenomOutput(nSmallerAmount, static_cast(i))); + } + + PoolMessage nMessageID = MSG_NOERR; + BOOST_CHECK(CoinJoin::ValidateDemotionEntry(vecTxIn, vecTxOut, nSmallerDenom, nMessageID)); +} + +BOOST_AUTO_TEST_CASE(validate_demotion_entry_wrong_input_count) +{ + // Invalid: 2 inputs instead of 1 + std::vector vecTxIn; + std::vector vecTxOut; + + const int nSmallerDenom = 1 << 2; + const CAmount nSmallerAmount = CoinJoin::DenominationToAmount(nSmallerDenom); + + // 2 inputs instead of 1 + vecTxIn.push_back(MakeDenomInput(0)); + vecTxIn.push_back(MakeDenomInput(1)); + + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + vecTxOut.push_back(MakeDenomOutput(nSmallerAmount, static_cast(i))); + } + + PoolMessage nMessageID = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidateDemotionEntry(vecTxIn, vecTxOut, nSmallerDenom, nMessageID)); +} + +BOOST_AUTO_TEST_CASE(validate_demotion_entry_wrong_output_count) +{ + // Invalid: 9 outputs instead of 10 + std::vector vecTxIn; + std::vector vecTxOut; + + const int nSmallerDenom = 1 << 2; + const CAmount nSmallerAmount = CoinJoin::DenominationToAmount(nSmallerDenom); + + vecTxIn.push_back(MakeDenomInput(0)); + + // Only 9 outputs + for (int i = 0; i < CoinJoin::PROMOTION_RATIO - 1; ++i) { + vecTxOut.push_back(MakeDenomOutput(nSmallerAmount, static_cast(i))); + } + + PoolMessage nMessageID = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidateDemotionEntry(vecTxIn, vecTxOut, nSmallerDenom, nMessageID)); +} + +BOOST_AUTO_TEST_CASE(denomination_adjacency_checks) +{ + // Test AreAdjacentDenominations function + // Denomination indices: 0=10.0, 1=1.0, 2=0.1, 3=0.01, 4=0.001 + + // Adjacent pairs should return true + BOOST_CHECK(CoinJoin::AreAdjacentDenominations(1 << 0, 1 << 1)); // 10.0 and 1.0 + BOOST_CHECK(CoinJoin::AreAdjacentDenominations(1 << 1, 1 << 2)); // 1.0 and 0.1 + BOOST_CHECK(CoinJoin::AreAdjacentDenominations(1 << 2, 1 << 3)); // 0.1 and 0.01 + BOOST_CHECK(CoinJoin::AreAdjacentDenominations(1 << 3, 1 << 4)); // 0.01 and 0.001 + + // Non-adjacent pairs should return false + BOOST_CHECK(!CoinJoin::AreAdjacentDenominations(1 << 0, 1 << 2)); // 10.0 and 0.1 (skip 1.0) + BOOST_CHECK(!CoinJoin::AreAdjacentDenominations(1 << 1, 1 << 3)); // 1.0 and 0.01 (skip 0.1) + BOOST_CHECK(!CoinJoin::AreAdjacentDenominations(1 << 0, 1 << 4)); // 10.0 and 0.001 + + // Same denomination is not adjacent to itself + BOOST_CHECK(!CoinJoin::AreAdjacentDenominations(1 << 2, 1 << 2)); + + // Invalid denominations + BOOST_CHECK(!CoinJoin::AreAdjacentDenominations(0, 1 << 1)); + BOOST_CHECK(!CoinJoin::AreAdjacentDenominations(1 << 1, 0)); + BOOST_CHECK(!CoinJoin::AreAdjacentDenominations(999, 1 << 1)); +} + +BOOST_AUTO_TEST_CASE(get_adjacent_denomination_helpers) +{ + // Test GetLargerAdjacentDenom and GetSmallerAdjacentDenom + // Indices: 0=10.0, 1=1.0, 2=0.1, 3=0.01, 4=0.001 + + // GetLargerAdjacentDenom + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(1 << 1), 1 << 0); // 1.0 → 10.0 + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(1 << 2), 1 << 1); // 0.1 → 1.0 + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(1 << 3), 1 << 2); // 0.01 → 0.1 + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(1 << 4), 1 << 3); // 0.001 → 0.01 + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(1 << 0), 0); // 10.0 has no larger + + // GetSmallerAdjacentDenom + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 0), 1 << 1); // 10.0 → 1.0 + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 1), 1 << 2); // 1.0 → 0.1 + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 2), 1 << 3); // 0.1 → 0.01 + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 3), 1 << 4); // 0.01 → 0.001 + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 4), 0); // 0.001 has no smaller +} + +BOOST_AUTO_TEST_CASE(promotion_ratio_constant) +{ + // Verify the PROMOTION_RATIO constant is 10 (10 smaller = 1 larger) + BOOST_CHECK_EQUAL(CoinJoin::PROMOTION_RATIO, 10); +} + +BOOST_AUTO_TEST_CASE(gap_threshold_constant) +{ + // Verify GAP_THRESHOLD is set to prevent oscillation + BOOST_CHECK_EQUAL(CoinJoin::GAP_THRESHOLD, 10); +} + +BOOST_AUTO_TEST_CASE(isvalidstructure_postfork_unbalanced_valid) +{ + // Post-V24: Unbalanced vin/vout (promotion: 10 inputs, 1 output) should be valid + // We need a mock pindex that signals V24 active - for this test we use nullptr which means pre-fork + // This test validates that the structure check correctly identifies promotion structure + + CCoinJoinBroadcastTx promo; + { + CMutableTransaction mtx; + // Promotion: 10 inputs of smaller denom -> 1 output of larger denom + const int nInputCount = CoinJoin::PROMOTION_RATIO; + const CAmount nLargerAmount = CoinJoin::DenominationToAmount(1 << 1); // 1.0 DASH + + for (int i = 0; i < nInputCount; ++i) { + CTxIn in; + in.prevout = COutPoint(uint256::ONE, static_cast(i)); + mtx.vin.push_back(in); + } + // 1 output of larger denom + CTxOut out{nLargerAmount, P2PKHScript(0x01)}; + mtx.vout.push_back(out); + + promo.tx = MakeTransactionRef(mtx); + promo.m_protxHash = uint256::ONE; + } + + // Pre-V24 (nullptr): unbalanced should fail + BOOST_CHECK(!promo.IsValidStructure(nullptr)); + + // Note: Post-V24 test would require a valid CBlockIndex with V24 deployment active + // which requires more setup. The above confirms pre-fork rejection works. +} + +BOOST_AUTO_TEST_CASE(isvalidstructure_demotion_structure) +{ + // Demotion: 1 input of larger denom -> 10 outputs of smaller denom + CCoinJoinBroadcastTx demo; + { + CMutableTransaction mtx; + const CAmount nSmallerAmount = CoinJoin::DenominationToAmount(1 << 2); // 0.1 DASH + + // 1 input + CTxIn in; + in.prevout = COutPoint(uint256::ONE, 0); + mtx.vin.push_back(in); + + // 10 outputs of smaller denom + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + CTxOut out{nSmallerAmount, P2PKHScript(static_cast(i))}; + mtx.vout.push_back(out); + } + + demo.tx = MakeTransactionRef(mtx); + demo.m_protxHash = uint256::ONE; + } + + // Pre-V24 (nullptr): unbalanced should fail + BOOST_CHECK(!demo.IsValidStructure(nullptr)); +} + +BOOST_AUTO_TEST_CASE(input_limit_prefork) +{ + // Pre-V24: max inputs = GetMaxPoolParticipants() * COINJOIN_ENTRY_MAX_SIZE + // Typically 20 * 9 = 180 + const size_t nMaxPreFork = CoinJoin::GetMaxPoolParticipants() * COINJOIN_ENTRY_MAX_SIZE; + BOOST_CHECK_EQUAL(nMaxPreFork, 180); + + // Transaction with exactly max inputs should be valid + CCoinJoinBroadcastTx maxValid; + { + CMutableTransaction mtx; + for (size_t i = 0; i < nMaxPreFork; ++i) { + CTxIn in; + in.prevout = COutPoint(uint256::ONE, static_cast(i)); + mtx.vin.push_back(in); + CTxOut out{CoinJoin::GetSmallestDenomination(), P2PKHScript(static_cast(i % 256))}; + mtx.vout.push_back(out); + } + maxValid.tx = MakeTransactionRef(mtx); + maxValid.m_protxHash = uint256::ONE; + } + BOOST_CHECK(maxValid.IsValidStructure(nullptr)); + + // Transaction with max+1 inputs should be invalid + CCoinJoinBroadcastTx tooMany; + { + CMutableTransaction mtx; + for (size_t i = 0; i < nMaxPreFork + 1; ++i) { + CTxIn in; + in.prevout = COutPoint(uint256::ONE, static_cast(i)); + mtx.vin.push_back(in); + CTxOut out{CoinJoin::GetSmallestDenomination(), P2PKHScript(static_cast(i % 256))}; + mtx.vout.push_back(out); + } + tooMany.tx = MakeTransactionRef(mtx); + tooMany.m_protxHash = uint256::ONE; + } + BOOST_CHECK(!tooMany.IsValidStructure(nullptr)); +} + +BOOST_AUTO_TEST_CASE(input_limit_postfork_constants) +{ + // Post-V24: max inputs = GetMaxPoolParticipants() * PROMOTION_RATIO + // Typically 20 * 10 = 200 + const size_t nMaxPostFork = CoinJoin::GetMaxPoolParticipants() * CoinJoin::PROMOTION_RATIO; + BOOST_CHECK_EQUAL(nMaxPostFork, 200); + + // Verify the increase from pre-fork + const size_t nMaxPreFork = CoinJoin::GetMaxPoolParticipants() * COINJOIN_ENTRY_MAX_SIZE; + BOOST_CHECK(nMaxPostFork > nMaxPreFork); + BOOST_CHECK_EQUAL(nMaxPostFork - nMaxPreFork, 20); // Increase of 20 +} + +BOOST_AUTO_TEST_CASE(promotion_demotion_value_preservation) +{ + // Verify that 10 smaller = 1 larger (value is preserved exactly) + // CoinJoin denominations are designed so that 10 * smaller == larger + // e.g., 10 * (0.1 DASH + 100 sat) = 1.0 DASH + 1000 sat + + const CAmount nSmallerAmount = CoinJoin::DenominationToAmount(1 << 2); // 0.1 DASH + const CAmount nLargerAmount = CoinJoin::DenominationToAmount(1 << 1); // 1.0 DASH + + // Value must match EXACTLY for promotion/demotion to preserve value + BOOST_CHECK_EQUAL(nSmallerAmount * CoinJoin::PROMOTION_RATIO, nLargerAmount); + + // Verify for all adjacent denomination pairs + for (int i = 0; i < 4; ++i) { + const int nLargerDenom = 1 << i; + const int nSmallerDenom = 1 << (i + 1); + const CAmount nLarger = CoinJoin::DenominationToAmount(nLargerDenom); + const CAmount nSmaller = CoinJoin::DenominationToAmount(nSmallerDenom); + + BOOST_CHECK(nLarger > nSmaller); + // The denominations are designed so 10 * smaller == larger exactly + BOOST_CHECK_EQUAL(nSmaller * CoinJoin::PROMOTION_RATIO, nLarger); + } +} + +BOOST_AUTO_TEST_CASE(decision_logic_mutual_exclusivity) +{ + // Verify that at any given count distribution, at most one of promote/demote is true + // This is a property-based test of the algorithm + + // Test case 1: Equal counts - neither should trigger + // With goal=100, halfGoal=50, if both at 100, deficits are 0, 0 + // Neither exceeds the other + GAP_THRESHOLD + // This verifies the mutual exclusivity property + + // Test case 2: Large imbalance toward smaller + // 0 larger, 100 smaller -> largerDeficit=100, smallerDeficit=0 + // Should promote (if smallerCount >= halfGoal) + + // Test case 3: Large imbalance toward larger + // 100 larger, 0 smaller -> largerDeficit=0, smallerDeficit=100 + // Should demote (if largerCount >= halfGoal) + + // The actual ShouldPromote/ShouldDemote require wallet mocking which is complex + // This test documents the expected behavior based on the algorithm + + // Verify the GAP_THRESHOLD prevents oscillation + // If |deficit1 - deficit2| <= GAP_THRESHOLD, neither action should occur + BOOST_CHECK_EQUAL(CoinJoin::GAP_THRESHOLD, 10); + + // The algorithm guarantees: for any pair of counts, + // ShouldPromote XOR ShouldDemote OR neither (but never both) + // This is enforced by the > (not >=) comparison with GAP_THRESHOLD +} + +BOOST_AUTO_TEST_CASE(isvalidstructure_mixed_session_postfork) +{ + // Post-V24: A transaction with mixed entry types should be valid + // (some participants doing 1:1, others doing promotion/demotion) + // This tests that the structure validation allows mixed input/output counts + + CCoinJoinBroadcastTx mixed; + { + CMutableTransaction mtx; + // Simulate a mixed session: + // - 3 standard participants: 3 inputs, 3 outputs (smallest denom) + // - 1 promotion participant: 10 inputs, 1 output + // Total: 13 inputs, 4 outputs + + const CAmount nSmallestDenom = CoinJoin::GetSmallestDenomination(); + const CAmount nSecondSmallest = CoinJoin::DenominationToAmount(1 << 3); // 0.01 DASH + + // Standard participants (3 x 1:1) + for (int i = 0; i < 3; ++i) { + CTxIn in; + in.prevout = COutPoint(uint256::ONE, static_cast(i)); + mtx.vin.push_back(in); + mtx.vout.push_back(CTxOut{nSmallestDenom, P2PKHScript(static_cast(i))}); + } + + // Promotion participant (10 inputs -> 1 output) + for (int i = 3; i < 13; ++i) { + CTxIn in; + in.prevout = COutPoint(uint256::ONE, static_cast(i)); + mtx.vin.push_back(in); + } + // One output for the promotion (larger denom) + mtx.vout.push_back(CTxOut{nSecondSmallest, P2PKHScript(0x0D)}); + + mixed.tx = MakeTransactionRef(mtx); + mixed.m_protxHash = uint256::ONE; + } + + // Pre-V24: Should fail (unbalanced) + BOOST_CHECK(!mixed.IsValidStructure(nullptr)); + + // Note: Post-V24 testing requires a valid CBlockIndex with V24 deployment active + // which requires more complex test setup with chainstate. The above verifies + // that pre-fork rejection works correctly. +} + +BOOST_AUTO_TEST_CASE(entry_type_detection_logic) +{ + // Test the entry type detection logic used in IsValidInOuts + // Entry types: + // - STANDARD: vin.size() == vout.size() + // - PROMOTION: vin.size() == PROMOTION_RATIO && vout.size() == 1 + // - DEMOTION: vin.size() == 1 && vout.size() == PROMOTION_RATIO + // - INVALID: anything else + + auto detectEntryType = [](size_t vinSize, size_t voutSize) -> std::string { + if (vinSize == voutSize) { + return "STANDARD"; + } else if (vinSize == static_cast(CoinJoin::PROMOTION_RATIO) && voutSize == 1) { + return "PROMOTION"; + } else if (vinSize == 1 && voutSize == static_cast(CoinJoin::PROMOTION_RATIO)) { + return "DEMOTION"; + } + return "INVALID"; + }; + + // Standard entries + BOOST_CHECK_EQUAL(detectEntryType(1, 1), "STANDARD"); + BOOST_CHECK_EQUAL(detectEntryType(5, 5), "STANDARD"); + BOOST_CHECK_EQUAL(detectEntryType(9, 9), "STANDARD"); // Max standard entry + + // Promotion entries + BOOST_CHECK_EQUAL(detectEntryType(10, 1), "PROMOTION"); + + // Demotion entries + BOOST_CHECK_EQUAL(detectEntryType(1, 10), "DEMOTION"); + + // Invalid entries (not valid pre or post fork) + BOOST_CHECK_EQUAL(detectEntryType(9, 1), "INVALID"); // Wrong ratio + BOOST_CHECK_EQUAL(detectEntryType(1, 9), "INVALID"); // Wrong ratio + BOOST_CHECK_EQUAL(detectEntryType(5, 3), "INVALID"); // Random mismatch + BOOST_CHECK_EQUAL(detectEntryType(2, 10), "INVALID"); // Wrong input count for demotion + BOOST_CHECK_EQUAL(detectEntryType(10, 2), "INVALID"); // Wrong output count for promotion + BOOST_CHECK_EQUAL(detectEntryType(20, 2), "INVALID"); // 20:2 is not valid + BOOST_CHECK_EQUAL(detectEntryType(11, 1), "INVALID"); // 11:1 is not valid promotion +} + +BOOST_AUTO_TEST_CASE(validate_entry_session_denom_consistency) +{ + // Test that promotion/demotion validation enforces session denomination consistency + // For promotion: inputs must be session denom (smaller), output must be larger adjacent + // For demotion: input must be larger adjacent, outputs must be session denom (smaller) + + const int nSessionDenom = 1 << 2; // 0.1 DASH (session denom) + const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); + + BOOST_CHECK(nLargerDenom != 0); + BOOST_CHECK_EQUAL(nLargerDenom, 1 << 1); // 1.0 DASH + + // Create a valid promotion entry structure + std::vector promoVin; + std::vector promoVout; + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + promoVin.push_back(MakeDenomInput(static_cast(i))); + } + promoVout.push_back(MakeDenomOutput(CoinJoin::DenominationToAmount(nLargerDenom))); + + PoolMessage msg = MSG_NOERR; + BOOST_CHECK(CoinJoin::ValidatePromotionEntry(promoVin, promoVout, nSessionDenom, msg)); + + // Test with wrong session denom (largest denom can't promote) + const int nLargestDenom = 1 << 0; // 10 DASH + msg = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidatePromotionEntry(promoVin, promoVout, nLargestDenom, msg)); + BOOST_CHECK(msg == ERR_DENOM); // No larger adjacent for 10 DASH + + // Create a valid demotion entry structure + std::vector demoVin; + std::vector demoVout; + demoVin.push_back(MakeDenomInput(0)); + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + demoVout.push_back(MakeDenomOutput(CoinJoin::DenominationToAmount(nSessionDenom), static_cast(i))); + } + + msg = MSG_NOERR; + BOOST_CHECK(CoinJoin::ValidateDemotionEntry(demoVin, demoVout, nSessionDenom, msg)); +} + +BOOST_AUTO_TEST_CASE(isvalidstructure_boundary_input_counts) +{ + // Test boundary conditions for input counts at the pre/post V24 limits + // Pre-V24: max 180 inputs (20 * 9) + // Post-V24: max 200 inputs (20 * 10) + + const size_t nPreForkMax = CoinJoin::GetMaxPoolParticipants() * COINJOIN_ENTRY_MAX_SIZE; + const size_t nPostForkMax = CoinJoin::GetMaxPoolParticipants() * CoinJoin::PROMOTION_RATIO; + + BOOST_CHECK_EQUAL(nPreForkMax, 180); + BOOST_CHECK_EQUAL(nPostForkMax, 200); + + // Create transaction with exactly 180 inputs (valid pre and post fork) + CCoinJoinBroadcastTx tx180; + { + CMutableTransaction mtx; + for (size_t i = 0; i < 180; ++i) { + mtx.vin.emplace_back(COutPoint(uint256::ONE, static_cast(i))); + mtx.vout.emplace_back(CoinJoin::GetSmallestDenomination(), P2PKHScript(static_cast(i % 256))); + } + tx180.tx = MakeTransactionRef(mtx); + tx180.m_protxHash = uint256::ONE; + } + BOOST_CHECK(tx180.IsValidStructure(nullptr)); // Pre-fork: valid at boundary + + // Create transaction with 181 inputs (invalid pre-fork, would be valid post-fork) + CCoinJoinBroadcastTx tx181; + { + CMutableTransaction mtx; + for (size_t i = 0; i < 181; ++i) { + mtx.vin.emplace_back(COutPoint(uint256::ONE, static_cast(i))); + mtx.vout.emplace_back(CoinJoin::GetSmallestDenomination(), P2PKHScript(static_cast(i % 256))); + } + tx181.tx = MakeTransactionRef(mtx); + tx181.m_protxHash = uint256::ONE; + } + BOOST_CHECK(!tx181.IsValidStructure(nullptr)); // Pre-fork: over limit + + // Transaction at post-fork boundary (200 inputs) + CCoinJoinBroadcastTx tx200; + { + CMutableTransaction mtx; + for (size_t i = 0; i < 200; ++i) { + mtx.vin.emplace_back(COutPoint(uint256::ONE, static_cast(i))); + mtx.vout.emplace_back(CoinJoin::GetSmallestDenomination(), P2PKHScript(static_cast(i % 256))); + } + tx200.tx = MakeTransactionRef(mtx); + tx200.m_protxHash = uint256::ONE; + } + // Pre-fork: over limit (200 > 180) + BOOST_CHECK(!tx200.IsValidStructure(nullptr)); + + // Note: Testing post-fork behavior requires a CBlockIndex with V24 active via EHF +} + +// ============================================================================ +// Tests for ShouldPromote/ShouldDemote algorithm logic +// These test the decision algorithm independently of wallet access +// ============================================================================ + +// Helper to simulate the ShouldPromote/ShouldDemote algorithm logic +// This mirrors the actual implementation in client.cpp +namespace { + +bool TestShouldPromote(int smallerCount, int largerCount, int goal) { + const int halfGoal = goal / 2; + + // Don't sacrifice a denomination that's still being built up + if (smallerCount < halfGoal) { + return false; + } + + // Calculate how far each is from goal (0 if at or above goal) + int smallerDeficit = std::max(0, goal - smallerCount); + int largerDeficit = std::max(0, goal - largerCount); + + // Promote if larger denomination is further from goal by more than threshold + return (largerDeficit > smallerDeficit + CoinJoin::GAP_THRESHOLD); +} + +bool TestShouldDemote(int largerCount, int smallerCount, int goal) { + const int halfGoal = goal / 2; + + // Don't sacrifice a denomination that's still being built up + if (largerCount < halfGoal) { + return false; + } + + // Calculate how far each is from goal (0 if at or above goal) + int smallerDeficit = std::max(0, goal - smallerCount); + int largerDeficit = std::max(0, goal - largerCount); + + // Demote if smaller denomination is further from goal by more than threshold + return (smallerDeficit > largerDeficit + CoinJoin::GAP_THRESHOLD); +} + +} // anonymous namespace + +BOOST_AUTO_TEST_CASE(should_promote_larger_deficit_greater) +{ + const int goal = 100; + + // 60 of smaller, 0 of larger -> should promote (0.1 has surplus, 1.0 needs help) + // smallerDeficit = 40, largerDeficit = 100, gap = 60 > GAP_THRESHOLD + BOOST_CHECK(TestShouldPromote(60, 0, goal)); + + // 100 of smaller, 0 of larger -> should promote (0.1 full, 1.0 empty) + // smallerDeficit = 0, largerDeficit = 100, gap = 100 > GAP_THRESHOLD + BOOST_CHECK(TestShouldPromote(100, 0, goal)); +} + +BOOST_AUTO_TEST_CASE(should_promote_below_half_goal_false) +{ + const int goal = 100; + + // 30 of smaller, 0 of larger -> should NOT promote (30 < 50 = halfGoal) + BOOST_CHECK(!TestShouldPromote(30, 0, goal)); + + // 49 of smaller, 0 of larger -> should NOT promote (49 < 50 = halfGoal) + BOOST_CHECK(!TestShouldPromote(49, 0, goal)); + + // 50 of smaller, 0 of larger -> CAN promote (50 >= 50 = halfGoal) + // smallerDeficit = 50, largerDeficit = 100, gap = 50 > GAP_THRESHOLD + BOOST_CHECK(TestShouldPromote(50, 0, goal)); +} + +BOOST_AUTO_TEST_CASE(should_promote_small_gap_false) +{ + const int goal = 100; + + // 90 smaller, 80 larger -> deficits are 10 and 20, gap = 10 + // 20 > 10 + 10 is false, so no promotion + BOOST_CHECK(!TestShouldPromote(90, 80, goal)); + + // 85 smaller, 80 larger -> deficits are 15 and 20, gap = 5 + // 20 > 15 + 10 is false, so no promotion + BOOST_CHECK(!TestShouldPromote(85, 80, goal)); +} + +BOOST_AUTO_TEST_CASE(should_demote_smaller_deficit_greater) +{ + const int goal = 100; + + // 60 of larger, 0 of smaller -> should demote (1.0 has surplus, 0.1 needs help) + // largerDeficit = 40, smallerDeficit = 100, gap = 60 > GAP_THRESHOLD + BOOST_CHECK(TestShouldDemote(60, 0, goal)); + + // 99 of larger, 0 of smaller -> should demote + // largerDeficit = 1, smallerDeficit = 100, gap = 99 > GAP_THRESHOLD + BOOST_CHECK(TestShouldDemote(99, 0, goal)); +} + +BOOST_AUTO_TEST_CASE(should_demote_below_half_goal_false) +{ + const int goal = 100; + + // 30 of larger, 0 of smaller -> should NOT demote (30 < 50 = halfGoal) + BOOST_CHECK(!TestShouldDemote(30, 0, goal)); + + // 49 of larger, 0 of smaller -> should NOT demote (49 < 50 = halfGoal) + BOOST_CHECK(!TestShouldDemote(49, 0, goal)); + + // 50 of larger, 0 of smaller -> CAN demote (50 >= 50 = halfGoal) + // largerDeficit = 50, smallerDeficit = 100, gap = 50 > GAP_THRESHOLD + BOOST_CHECK(TestShouldDemote(50, 0, goal)); +} + +BOOST_AUTO_TEST_CASE(promote_demote_mutually_exclusive) +{ + // Test various distributions - at most one should be true + auto testMutualExclusivity = [](int smallerCount, int largerCount) { + const int testGoal = 100; + bool promote = TestShouldPromote(smallerCount, largerCount, testGoal); + bool demote = TestShouldDemote(largerCount, smallerCount, testGoal); + // At most one can be true (XOR or neither) + BOOST_CHECK(!(promote && demote)); + return std::make_pair(promote, demote); + }; + + // Equal counts - neither + auto [p1, d1] = testMutualExclusivity(100, 100); + BOOST_CHECK(!p1 && !d1); + + // 90/90 - neither (close to goal, small gap) + auto [p2, d2] = testMutualExclusivity(90, 90); + BOOST_CHECK(!p2 && !d2); + + // Large imbalance toward smaller - promote only + auto [p3, d3] = testMutualExclusivity(100, 0); + BOOST_CHECK(p3 && !d3); + + // Large imbalance toward larger - demote only + auto [p4, d4] = testMutualExclusivity(0, 100); + BOOST_CHECK(!p4 && d4); + + // Mid-range cases from the plan + auto [p5, d5] = testMutualExclusivity(0, 60); // 0.1=0, 1.0=60 -> should demote + BOOST_CHECK(!p5 && d5); + + auto [p6, d6] = testMutualExclusivity(60, 0); // 0.1=60, 1.0=0 -> should promote + BOOST_CHECK(p6 && !d6); +} + +BOOST_AUTO_TEST_CASE(decision_logic_example_cases_from_plan) +{ + // Test the specific examples from the implementation plan + const int goal = 100; + + // | 1.0 count | 0.1 count | Action | + // | 99 | 0 | Demote | + BOOST_CHECK(TestShouldDemote(99, 0, goal)); + BOOST_CHECK(!TestShouldPromote(0, 99, goal)); // 0.1 has 0, can't promote + + // | 60 | 0 | Demote | + BOOST_CHECK(TestShouldDemote(60, 0, goal)); + + // | 30 | 0 | Nothing (1.0 < halfGoal) | + BOOST_CHECK(!TestShouldDemote(30, 0, goal)); + BOOST_CHECK(!TestShouldPromote(0, 30, goal)); + + // | 0 | 60 | Promote | + BOOST_CHECK(TestShouldPromote(60, 0, goal)); + + // | 0 | 30 | Nothing (0.1 < halfGoal) | + BOOST_CHECK(!TestShouldPromote(30, 0, goal)); + + // | 100 | 100 | Nothing (both at goal) | + BOOST_CHECK(!TestShouldPromote(100, 100, goal)); + BOOST_CHECK(!TestShouldDemote(100, 100, goal)); + + // | 90 | 90 | Nothing (deficits equal) | + BOOST_CHECK(!TestShouldPromote(90, 90, goal)); + BOOST_CHECK(!TestShouldDemote(90, 90, goal)); +} + +BOOST_AUTO_TEST_CASE(adjacent_denomination_helpers) +{ + // Test GetLargerAdjacentDenom and GetSmallerAdjacentDenom + + // Denomination bits: 0=10DASH, 1=1DASH, 2=0.1DASH, 3=0.01DASH, 4=0.001DASH + const int denom_10 = 1 << 0; // 10 DASH + const int denom_1 = 1 << 1; // 1 DASH + const int denom_01 = 1 << 2; // 0.1 DASH + const int denom_001 = 1 << 3; // 0.01 DASH + const int denom_0001 = 1 << 4; // 0.001 DASH + + // GetLargerAdjacentDenom: returns the next larger denomination (10x value) + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_1), denom_10); + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_01), denom_1); + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_001), denom_01); + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_0001), denom_001); + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_10), 0); // No larger + + // GetSmallerAdjacentDenom: returns the next smaller denomination (0.1x value) + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_10), denom_1); + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_1), denom_01); + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_01), denom_001); + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_001), denom_0001); + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_0001), 0); // No smaller + + // Invalid denominations + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(0), 0); + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(0), 0); + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(999), 0); + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(999), 0); +} + +BOOST_AUTO_TEST_CASE(validate_promotion_entry_edge_cases) +{ + // Additional edge cases for ValidatePromotionEntry + + const int nSessionDenom = 1 << 2; // 0.1 DASH + const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); + const CAmount nLargerAmount = CoinJoin::DenominationToAmount(nLargerDenom); + + // Valid case: exactly 10 inputs, 1 output of correct larger denom + std::vector validVin; + std::vector validVout; + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + validVin.push_back(MakeDenomInput(static_cast(i))); + } + validVout.push_back(MakeDenomOutput(nLargerAmount)); + + PoolMessage msg = MSG_NOERR; + BOOST_CHECK(CoinJoin::ValidatePromotionEntry(validVin, validVout, nSessionDenom, msg)); + + // Invalid: 9 inputs (wrong count) + std::vector wrongCountVin; + for (int i = 0; i < 9; ++i) { + wrongCountVin.push_back(MakeDenomInput(static_cast(i))); + } + msg = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidatePromotionEntry(wrongCountVin, validVout, nSessionDenom, msg)); + + // Invalid: 11 inputs (wrong count) + std::vector tooManyVin; + for (int i = 0; i < 11; ++i) { + tooManyVin.push_back(MakeDenomInput(static_cast(i))); + } + msg = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidatePromotionEntry(tooManyVin, validVout, nSessionDenom, msg)); + + // Invalid: 2 outputs (should be 1) + std::vector twoOutputs; + twoOutputs.push_back(MakeDenomOutput(nLargerAmount / 2)); + twoOutputs.push_back(MakeDenomOutput(nLargerAmount / 2)); + msg = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidatePromotionEntry(validVin, twoOutputs, nSessionDenom, msg)); + + // Invalid: output is wrong denomination + std::vector wrongDenomOut; + wrongDenomOut.push_back(MakeDenomOutput(CoinJoin::DenominationToAmount(nSessionDenom))); // Same denom, not larger + msg = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidatePromotionEntry(validVin, wrongDenomOut, nSessionDenom, msg)); +} + +BOOST_AUTO_TEST_CASE(validate_demotion_entry_edge_cases) +{ + // Additional edge cases for ValidateDemotionEntry + + const int nSessionDenom = 1 << 2; // 0.1 DASH (output denom) + const CAmount nSessionAmount = CoinJoin::DenominationToAmount(nSessionDenom); + + // Verify the larger adjacent denom exists for this test to be meaningful + BOOST_CHECK(CoinJoin::GetLargerAdjacentDenom(nSessionDenom) != 0); + + // Valid case: 1 input of larger denom, 10 outputs of session denom + std::vector validVin; + validVin.push_back(MakeDenomInput(0)); + + std::vector validVout; + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + validVout.push_back(MakeDenomOutput(nSessionAmount, static_cast(i))); + } + + PoolMessage msg = MSG_NOERR; + BOOST_CHECK(CoinJoin::ValidateDemotionEntry(validVin, validVout, nSessionDenom, msg)); + + // Invalid: 2 inputs (should be 1) + std::vector twoInputs; + twoInputs.push_back(MakeDenomInput(0)); + twoInputs.push_back(MakeDenomInput(1)); + msg = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidateDemotionEntry(twoInputs, validVout, nSessionDenom, msg)); + + // Invalid: 9 outputs (wrong count) + std::vector nineOutputs; + for (int i = 0; i < 9; ++i) { + nineOutputs.push_back(MakeDenomOutput(nSessionAmount, static_cast(i))); + } + msg = MSG_NOERR; + BOOST_CHECK(!CoinJoin::ValidateDemotionEntry(validVin, nineOutputs, nSessionDenom, msg)); + + // Test that demotion from smallest denomination (0.001 DASH) fails + // because there's no smaller denomination to demote to + const int nSmallestDenom = 1 << 4; // 0.001 DASH + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(nSmallestDenom), 0); // No smaller exists +} + +// ============================================================================ +// Documentation: Post-V24 Integration Testing Requirements +// ============================================================================ +// The following tests cannot be performed as unit tests because V24 uses +// EHF (Extended Hard Fork) activation which requires: +// 1. A valid blockchain with proper masternode infrastructure +// 2. EHF signaling from masternodes +// 3. Proper chainstate with V24 deployment active +// +// These tests should be performed as functional tests: +// - test/functional/coinjoin_promotion.py (to be added) +// - test/functional/coinjoin_demotion.py (to be added) +// +// Post-V24 behaviors that require functional testing: +// 1. IsValidStructure accepts unbalanced vin/vout with proper promo/demo structure +// 2. IsValidInOuts accepts promotion/demotion entries within sessions +// 3. Full promotion flow: 10 inputs -> 1 output mixing +// 4. Full demotion flow: 1 input -> 10 outputs mixing +// 5. Mixed sessions: some participants 1:1, others promoting/demoting +// 6. Input limit increased from 180 to 200 post-V24 +// ============================================================================ BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/coinjoin.cpp b/src/wallet/coinjoin.cpp index 5731e0ab6691..f9f5d772d3e2 100644 --- a/src/wallet/coinjoin.cpp +++ b/src/wallet/coinjoin.cpp @@ -46,6 +46,11 @@ bool CWallet::SetCoinJoinSalt(const uint256& cj_salt) } bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector& vecTxDSInRet) +{ + return SelectTxDSInsByDenomination(nDenom, nValueMax, vecTxDSInRet, CoinType::ONLY_READY_TO_MIX); +} + +bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector& vecTxDSInRet, CoinType nCoinType) { LOCK(cs_wallet); @@ -56,11 +61,11 @@ bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::ve CAmount nDenomAmount{CoinJoin::DenominationToAmount(nDenom)}; CAmount nValueTotal{0}; - CCoinControl coin_control(CoinType::ONLY_READY_TO_MIX); + CCoinControl coin_control(nCoinType); std::set setRecentTxIds; std::vector vCoins{AvailableCoinsListUnspent(*this, &coin_control).all()}; - WalletCJLogPrint(this, "CWallet::%s -- vCoins.size(): %d\n", __func__, vCoins.size()); + WalletCJLogPrint(this, "CWallet::%s -- vCoins.size(): %d, CoinType: %d\n", __func__, vCoins.size(), static_cast(nCoinType)); Shuffle(vCoins.rbegin(), vCoins.rend(), FastRandomContext()); @@ -406,6 +411,65 @@ bool CWallet::IsFullyMixed(const COutPoint& outpoint) const return true; } +int CWallet::CountCoinsByDenomination(int nDenom, bool fFullyMixedOnly) const +{ + if (!CCoinJoinClientOptions::IsEnabled()) return 0; + + const CAmount nDenomAmount = CoinJoin::DenominationToAmount(nDenom); + if (nDenomAmount <= 0) return 0; + + int nCount = 0; + + LOCK(cs_wallet); + for (const auto& outpoint : setWalletUTXO) { + const auto it{mapWallet.find(outpoint.hash)}; + if (it == mapWallet.end()) continue; + + const CAmount nValue = it->second.tx->vout[outpoint.n].nValue; + if (nValue != nDenomAmount) continue; + + // Skip unconfirmed or conflicted + if (GetTxDepthInMainChain(it->second) < 1) continue; + + // Skip if we need fully mixed and this isn't + if (fFullyMixedOnly && !IsFullyMixed(outpoint)) continue; + + nCount++; + } + + return nCount; +} + +std::vector CWallet::SelectFullyMixedForPromotion(int nDenom, int nCount) const +{ + std::vector vecRet; + if (!CCoinJoinClientOptions::IsEnabled()) return vecRet; + + const CAmount nDenomAmount = CoinJoin::DenominationToAmount(nDenom); + if (nDenomAmount <= 0) return vecRet; + + LOCK(cs_wallet); + for (const auto& outpoint : setWalletUTXO) { + if (static_cast(vecRet.size()) >= nCount) break; + + const auto it{mapWallet.find(outpoint.hash)}; + if (it == mapWallet.end()) continue; + + const CAmount nValue = it->second.tx->vout[outpoint.n].nValue; + if (nValue != nDenomAmount) continue; + + // Skip unconfirmed or conflicted + if (GetTxDepthInMainChain(it->second) < 1) continue; + + // Must be fully mixed for promotion + if (!IsFullyMixed(outpoint)) continue; + + vecRet.push_back(outpoint); + } + + return vecRet; +} + void CWallet::RecalculateMixedCredit(const uint256 hash) { AssertLockHeld(cs_wallet); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6665af8ac35c..b443dcfddac5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -580,6 +581,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // Coin selection bool SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector& vecTxDSInRet); + bool SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector& vecTxDSInRet, CoinType nCoinType); bool SelectDenominatedAmounts(CAmount nValueMax, std::set& setAmountsRet) const; std::vector SelectCoinsGroupedByAddresses(bool fSkipDenominated = true, bool fAnonymizable = true, bool fSkipUnconfirmed = true, int nMaxOupointsPerAddress = -1) const; @@ -597,6 +599,22 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool IsDenominated(const COutPoint& outpoint) const; bool IsFullyMixed(const COutPoint& outpoint) const; + /** + * Count coins of a specific denomination (post-V24 promotion/demotion feature) + * @param nDenom The denomination to count (bitshifted integer) + * @param fFullyMixedOnly If true, only count fully-mixed coins + * @return Number of coins matching the criteria + */ + int CountCoinsByDenomination(int nDenom, bool fFullyMixedOnly = false) const; + + /** + * Select fully-mixed coins for promotion (post-V24 feature) + * @param nDenom The denomination to select (bitshifted integer) + * @param nCount Maximum number of coins to select + * @return Vector of outpoints for selected coins + */ + std::vector SelectFullyMixedForPromotion(int nDenom, int nCount) const; + bool IsSpent(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); // Whether this or any known UTXO with the same single key has been spent. From 40d75da7ffd17a13b9cac00cfcff6600bf09a97f Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 8 Dec 2025 09:48:03 -0600 Subject: [PATCH 02/12] fix: lint-logs issues --- src/coinjoin/client.cpp | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index fbe04c142b2d..c98e035a55d0 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -395,8 +395,8 @@ bool CCoinJoinClientSession::SendDenominate(const std::vectorSelectFullyMixedForPromotion(nSmallerDenom, CoinJoin::PROMOTION_RATIO); if (static_cast(vecCoins.size()) >= CoinJoin::PROMOTION_RATIO) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Promotion opportunity: " - "%d x %s -> 1 x %s\n", + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Promotion opportunity: %d x %s -> 1 x %s\n", CoinJoin::PROMOTION_RATIO, CoinJoin::DenominationToString(nSmallerDenom), CoinJoin::DenominationToString(nLargerDenom)); @@ -901,8 +900,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman // Check if we should demote larger -> smaller if (m_clientman.ShouldDemote(nLargerDenom, nSmallerDenom)) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Demotion opportunity: " - "1 x %s -> %d x %s\n", + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Demotion opportunity: 1 x %s -> %d x %s\n", CoinJoin::DenominationToString(nLargerDenom), CoinJoin::PROMOTION_RATIO, CoinJoin::DenominationToString(nSmallerDenom)); @@ -1148,8 +1146,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, for (const auto& txdsin : vecTxDSInTmp) { m_vecPromotionInputs.push_back(txdsin.prevout); } - WalletCJLogPrint(m_wallet, - "CCoinJoinClientSession::JoinExistingQueue -- pending PROMOTION connection, masternode=%s, nSessionDenom=%d (%s), %d inputs\n", + 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()); } else if (fDemotion) { @@ -1158,13 +1155,11 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, if (!vecTxDSInTmp.empty()) { m_vecPromotionInputs.push_back(vecTxDSInTmp[0].prevout); } - WalletCJLogPrint(m_wallet, - "CCoinJoinClientSession::JoinExistingQueue -- pending DEMOTION connection, masternode=%s, nSessionDenom=%d (%s)\n", + 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(); - WalletCJLogPrint(m_wallet, /* Continued */ - "CCoinJoinClientSession::JoinExistingQueue -- pending connection, masternode=%s, nSessionDenom=%d (%s)\n", + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- pending connection, masternode=%s, nSessionDenom=%d (%s)\n", dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom)); } strAutoDenomResult = _("Trying to connect…"); @@ -1365,8 +1360,7 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon m_vecPromotionInputs.push_back(txdsin.prevout); } - WalletCJLogPrint(m_wallet, - "CCoinJoinClientSession::StartNewQueue -- pending %s connection, masternode=%s, nSessionDenom=%d (%s), %zu inputs\n", + 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()); From cbd56b59538f1008d20fcb8b1a232af815527eed Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 9 Dec 2025 18:14:17 +0300 Subject: [PATCH 03/12] fix: address critical issues in CoinJoin promotion/demotion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes for PR #7052: Issue #1 (CRITICAL): Fix race condition in coin selection - Lock coins immediately after selection in JoinExistingQueue/StartNewQueue - Prevents concurrent sessions from selecting the same coins - Add defensive IsLockedCoin check in Prepare functions Issue #2 (CRITICAL): Fix resource leak on session failure - Add promotion inputs to vecOutPointLocked in SetNull() - Ensures coins locked early are properly unlocked if session fails - Leverages existing UnlockCoins() retry mechanism Issue #3: Add UTXO validation before use - Check IsSpent() before using promotion/demotion inputs - Prevents using externally spent or transferred coins 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- src/coinjoin/client.cpp | 61 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index c98e035a55d0..9094023c616b 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -178,7 +178,19 @@ void CCoinJoinClientSession::SetNull() mixingMasternode = nullptr; pendingDsaRequest = CPendingDsaRequest(); - // Post-V24: Clear promotion/demotion session state + // 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()) { + // Add to vecOutPointLocked so UnlockCoins() will handle them properly + // with its retry mechanism if the wallet is locked + for (const auto& outpoint : m_vecPromotionInputs) { + // Only add if not already in the list (avoid duplicates) + if (std::find(vecOutPointLocked.begin(), vecOutPointLocked.end(), outpoint) == vecOutPointLocked.end()) { + vecOutPointLocked.push_back(outpoint); + } + } + } m_fPromotion = false; m_fDemotion = false; m_vecPromotionInputs.clear(); @@ -1092,6 +1104,11 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Failed to build promotion inputs\n"); continue; } + // Lock promotion inputs immediately to prevent race conditions + // with other concurrent CoinJoin sessions + for (const auto& outpoint : vecCoins) { + m_wallet->LockCoin(outpoint); + } } else if (fDemotion && nTargetDenom != 0) { // Demotion: select 1 coin of the larger denomination const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nTargetDenom); @@ -1113,6 +1130,11 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, if (vecTxDSInTmp.size() > 1) { vecTxDSInTmp.resize(1); } + // Lock demotion input immediately to prevent race conditions + if (!vecTxDSInTmp.empty()) { + LOCK(m_wallet->cs_wallet); + m_wallet->LockCoin(vecTxDSInTmp[0].prevout); + } } else { // Standard mixing: try to match their denominations if possible if (!m_wallet->SelectTxDSInsByDenomination(dsq.nDenom, nBalanceNeedsAnonymized, vecTxDSInTmp)) { @@ -1284,6 +1306,10 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- Failed to build promotion inputs\n"); return false; } + // Lock promotion inputs immediately to prevent race conditions + for (const auto& outpoint : vecCoins) { + m_wallet->LockCoin(outpoint); + } } else if (fDemotion) { // Demotion: need 1 coin of the larger denomination const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nTargetDenom); @@ -1301,6 +1327,11 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon if (vecTxDSInTmp.size() > 1) { vecTxDSInTmp.resize(1); } + // Lock demotion input immediately to prevent race conditions + if (!vecTxDSInTmp.empty()) { + LOCK(m_wallet->cs_wallet); + m_wallet->LockCoin(vecTxDSInTmp[0].prevout); + } } else { // Neither promotion nor demotion - shouldn't use this overload return false; @@ -1637,6 +1668,12 @@ bool CCoinJoinClientSession::PreparePromotionEntry(std::string& strErrorRet, std return false; } + // Validate the UTXO is still spendable + if (m_wallet->IsSpent(outpoint)) { + strErrorRet = "Promotion input has been spent"; + return false; + } + CTxDSIn txdsin(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); @@ -1654,9 +1691,13 @@ bool CCoinJoinClientSession::PreparePromotionEntry(std::string& strErrorRet, std vecPSInOutPairsRet.back().second = CTxOut(nLargerAmount, scriptDenom); } - // Lock all inputs + // Lock all inputs (should already be locked from JoinExistingQueue/StartNewQueue) + // Add to vecOutPointLocked for proper cleanup in SetNull() for (const auto& [txDsIn, txDsOut] : vecPSInOutPairsRet) { - m_wallet->LockCoin(txDsIn.prevout); + if (!m_wallet->IsLockedCoin(txDsIn.prevout)) { + // Defensive: lock if not already locked + m_wallet->LockCoin(txDsIn.prevout); + } vecOutPointLocked.push_back(txDsIn.prevout); } @@ -1698,6 +1739,12 @@ bool CCoinJoinClientSession::PrepareDemotionEntry(std::string& strErrorRet, std: return false; } + // Validate the UTXO is still spendable + if (m_wallet->IsSpent(outpoint)) { + strErrorRet = "Demotion input has been spent"; + return false; + } + CTxDSIn txdsin(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); @@ -1717,8 +1764,12 @@ bool CCoinJoinClientSession::PrepareDemotionEntry(std::string& strErrorRet, std: } } - // Lock the input - m_wallet->LockCoin(txdsin.prevout); + // Lock the input (should already be locked from JoinExistingQueue/StartNewQueue) + // Add to vecOutPointLocked for proper cleanup in SetNull() + if (!m_wallet->IsLockedCoin(txdsin.prevout)) { + // Defensive: lock if not already locked + m_wallet->LockCoin(txdsin.prevout); + } vecOutPointLocked.push_back(txdsin.prevout); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::PrepareDemotionEntry -- Prepared 1 input for demotion to %d x %s\n", From b14d42d8e4e72e94e41e40e28f33f0ed3a9cac0b Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 9 Dec 2025 20:55:41 +0300 Subject: [PATCH 04/12] fix: privacy - require minimum standard mixers for promotion/demotion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure promotion/demotion sessions have adequate privacy by requiring the minimum participant threshold to be met by STANDARD mixing entries only. Implementation: - Add IsStandardMixingEntry() method to CCoinJoinEntry Detects entry type: standard (equal in/out), promotion (10:1), demotion (1:10) - Add GetStandardEntriesCount() to CCoinJoinBaseSession Counts only standard mixing entries for threshold checks - Modify server CheckPool timeout logic to use GetStandardEntriesCount() Only standard entries count toward minimum; promotion/demotion get free cover Why this matters: - Promotion/demotion creates unique patterns (10→1 or 1→10) - Small sessions where you're the only rebalancer = weak anonymity (33%) - Requiring minimum standard mixers provides cover traffic Benefits: - Promotion/demotion can still start queues (better UX, doesn't block feature) - Privacy protected by requiring standard mixer cover traffic - Session won't finalize until adequate anonymity set exists Example: Before: 1 promotion + 2 standard = proceeds (weak: 33% chance you're identified) After: 1 promotion + 2 standard = times out (needs 3 standard minimum) After: 2 promotion + 4 standard = proceeds (good: 6 participants, better cover!) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- src/coinjoin/coinjoin.h | 23 +++++++++++++++++++++++ src/coinjoin/server.cpp | 6 ++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index 461cc4ef9218..7724e975d2b3 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -199,6 +199,15 @@ class CCoinJoinEntry } bool AddScriptSig(const CTxIn& txin); + + // Check if this is a standard mixing entry (not promotion/demotion) + // Standard: equal number of inputs and outputs + // Promotion: PROMOTION_RATIO inputs, 1 output + // Demotion: 1 input, PROMOTION_RATIO outputs + bool IsStandardMixingEntry() const + { + return vecTxDSIn.size() == vecTxOut.size(); + } }; @@ -346,6 +355,20 @@ class CCoinJoinBaseSession int GetEntriesCount() const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin) { LOCK(cs_coinjoin); return vecEntries.size(); } int GetEntriesCountLocked() const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin) { return vecEntries.size(); } + + // Count only standard mixing entries (not promotion/demotion) for privacy threshold + int GetStandardEntriesCount() const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin) + { + LOCK(cs_coinjoin); + return std::count_if(vecEntries.begin(), vecEntries.end(), + [](const CCoinJoinEntry& entry) { return entry.IsStandardMixingEntry(); }); + } + + int GetStandardEntriesCountLocked() const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin) + { + return std::count_if(vecEntries.begin(), vecEntries.end(), + [](const CCoinJoinEntry& entry) { return entry.IsStandardMixingEntry(); }); + } }; class CoinJoinQueueManager diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 134d01d3e631..194b0cc58dcc 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -318,8 +318,10 @@ void CCoinJoinServer::CheckPool() // Check for Time Out // If we timed out while accepting entries, then if we have more than minimum, create final tx - if (nState == POOL_STATE_ACCEPTING_ENTRIES && CCoinJoinServer::HasTimedOut() && - GetEntriesCount() >= CoinJoin::GetMinPoolParticipants()) { + // PRIVACY: Only count standard mixing entries toward minimum participant threshold + // Promotion/demotion entries don't count - they get privacy from standard mixers + if (nState == POOL_STATE_ACCEPTING_ENTRIES && CCoinJoinServer::HasTimedOut() + && GetStandardEntriesCount() >= CoinJoin::GetMinPoolParticipants()) { // Punish misbehaving participants ChargeFees(); // Try to complete this session ignoring the misbehaving ones From 4f068df1b149a8e3fce181b4303355fdd5a69cf6 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 9 Dec 2025 21:38:20 +0300 Subject: [PATCH 05/12] test: improve CoinJoin test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove 4 redundant/low-value tests: - promotion_ratio_constant (trivial constant check) - gap_threshold_constant (trivial constant check) - decision_logic_mutual_exclusivity (documentation only) - adjacent_denomination_helpers (duplicate) Add 2 critical tests for privacy protection: - is_standard_mixing_entry (validates IsStandardMixingEntry method) - get_standard_entries_count (validates threshold enforcement) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- src/test/coinjoin_inouts_tests.cpp | 172 +++++++++++++---------------- 1 file changed, 77 insertions(+), 95 deletions(-) diff --git a/src/test/coinjoin_inouts_tests.cpp b/src/test/coinjoin_inouts_tests.cpp index 121a6289a8de..7a25ab2c0d54 100644 --- a/src/test/coinjoin_inouts_tests.cpp +++ b/src/test/coinjoin_inouts_tests.cpp @@ -589,6 +589,8 @@ BOOST_AUTO_TEST_CASE(get_adjacent_denomination_helpers) BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(1 << 3), 1 << 2); // 0.01 → 0.1 BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(1 << 4), 1 << 3); // 0.001 → 0.01 BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(1 << 0), 0); // 10.0 has no larger + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(0), 0); // Invalid denominations + BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(999), 0); // Invalid denominations // GetSmallerAdjacentDenom BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 0), 1 << 1); // 10.0 → 1.0 @@ -596,18 +598,8 @@ BOOST_AUTO_TEST_CASE(get_adjacent_denomination_helpers) BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 2), 1 << 3); // 0.1 → 0.01 BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 3), 1 << 4); // 0.01 → 0.001 BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(1 << 4), 0); // 0.001 has no smaller -} - -BOOST_AUTO_TEST_CASE(promotion_ratio_constant) -{ - // Verify the PROMOTION_RATIO constant is 10 (10 smaller = 1 larger) - BOOST_CHECK_EQUAL(CoinJoin::PROMOTION_RATIO, 10); -} - -BOOST_AUTO_TEST_CASE(gap_threshold_constant) -{ - // Verify GAP_THRESHOLD is set to prevent oscillation - BOOST_CHECK_EQUAL(CoinJoin::GAP_THRESHOLD, 10); + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(0), 0); // Invalid denominations + BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(999), 0); // Invalid denominations } BOOST_AUTO_TEST_CASE(isvalidstructure_postfork_unbalanced_valid) @@ -748,36 +740,6 @@ BOOST_AUTO_TEST_CASE(promotion_demotion_value_preservation) } } -BOOST_AUTO_TEST_CASE(decision_logic_mutual_exclusivity) -{ - // Verify that at any given count distribution, at most one of promote/demote is true - // This is a property-based test of the algorithm - - // Test case 1: Equal counts - neither should trigger - // With goal=100, halfGoal=50, if both at 100, deficits are 0, 0 - // Neither exceeds the other + GAP_THRESHOLD - // This verifies the mutual exclusivity property - - // Test case 2: Large imbalance toward smaller - // 0 larger, 100 smaller -> largerDeficit=100, smallerDeficit=0 - // Should promote (if smallerCount >= halfGoal) - - // Test case 3: Large imbalance toward larger - // 100 larger, 0 smaller -> largerDeficit=0, smallerDeficit=100 - // Should demote (if largerCount >= halfGoal) - - // The actual ShouldPromote/ShouldDemote require wallet mocking which is complex - // This test documents the expected behavior based on the algorithm - - // Verify the GAP_THRESHOLD prevents oscillation - // If |deficit1 - deficit2| <= GAP_THRESHOLD, neither action should occur - BOOST_CHECK_EQUAL(CoinJoin::GAP_THRESHOLD, 10); - - // The algorithm guarantees: for any pair of counts, - // ShouldPromote XOR ShouldDemote OR neither (but never both) - // This is enforced by the > (not >=) comparison with GAP_THRESHOLD -} - BOOST_AUTO_TEST_CASE(isvalidstructure_mixed_session_postfork) { // Post-V24: A transaction with mixed entry types should be valid @@ -1141,38 +1103,6 @@ BOOST_AUTO_TEST_CASE(decision_logic_example_cases_from_plan) BOOST_CHECK(!TestShouldDemote(90, 90, goal)); } -BOOST_AUTO_TEST_CASE(adjacent_denomination_helpers) -{ - // Test GetLargerAdjacentDenom and GetSmallerAdjacentDenom - - // Denomination bits: 0=10DASH, 1=1DASH, 2=0.1DASH, 3=0.01DASH, 4=0.001DASH - const int denom_10 = 1 << 0; // 10 DASH - const int denom_1 = 1 << 1; // 1 DASH - const int denom_01 = 1 << 2; // 0.1 DASH - const int denom_001 = 1 << 3; // 0.01 DASH - const int denom_0001 = 1 << 4; // 0.001 DASH - - // GetLargerAdjacentDenom: returns the next larger denomination (10x value) - BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_1), denom_10); - BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_01), denom_1); - BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_001), denom_01); - BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_0001), denom_001); - BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(denom_10), 0); // No larger - - // GetSmallerAdjacentDenom: returns the next smaller denomination (0.1x value) - BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_10), denom_1); - BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_1), denom_01); - BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_01), denom_001); - BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_001), denom_0001); - BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(denom_0001), 0); // No smaller - - // Invalid denominations - BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(0), 0); - BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(0), 0); - BOOST_CHECK_EQUAL(CoinJoin::GetLargerAdjacentDenom(999), 0); - BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(999), 0); -} - BOOST_AUTO_TEST_CASE(validate_promotion_entry_edge_cases) { // Additional edge cases for ValidatePromotionEntry @@ -1265,25 +1195,77 @@ BOOST_AUTO_TEST_CASE(validate_demotion_entry_edge_cases) BOOST_CHECK_EQUAL(CoinJoin::GetSmallerAdjacentDenom(nSmallestDenom), 0); // No smaller exists } -// ============================================================================ -// Documentation: Post-V24 Integration Testing Requirements -// ============================================================================ -// The following tests cannot be performed as unit tests because V24 uses -// EHF (Extended Hard Fork) activation which requires: -// 1. A valid blockchain with proper masternode infrastructure -// 2. EHF signaling from masternodes -// 3. Proper chainstate with V24 deployment active -// -// These tests should be performed as functional tests: -// - test/functional/coinjoin_promotion.py (to be added) -// - test/functional/coinjoin_demotion.py (to be added) -// -// Post-V24 behaviors that require functional testing: -// 1. IsValidStructure accepts unbalanced vin/vout with proper promo/demo structure -// 2. IsValidInOuts accepts promotion/demotion entries within sessions -// 3. Full promotion flow: 10 inputs -> 1 output mixing -// 4. Full demotion flow: 1 input -> 10 outputs mixing -// 5. Mixed sessions: some participants 1:1, others promoting/demoting -// 6. Input limit increased from 180 to 200 post-V24 -// ============================================================================ +BOOST_AUTO_TEST_CASE(is_standard_mixing_entry) +{ + // Test IsStandardMixingEntry() method added for privacy protection + + // Standard entry: equal inputs and outputs + CCoinJoinEntry standard; + standard.vecTxDSIn.resize(3); + standard.vecTxOut.resize(3); + BOOST_CHECK(standard.IsStandardMixingEntry()); + + // Promotion entry: PROMOTION_RATIO inputs, 1 output + CCoinJoinEntry promotion; + promotion.vecTxDSIn.resize(CoinJoin::PROMOTION_RATIO); // 10 inputs + promotion.vecTxOut.resize(1); // 1 output + BOOST_CHECK(!promotion.IsStandardMixingEntry()); + + // Demotion entry: 1 input, PROMOTION_RATIO outputs + CCoinJoinEntry demotion; + demotion.vecTxDSIn.resize(1); // 1 input + demotion.vecTxOut.resize(CoinJoin::PROMOTION_RATIO); // 10 outputs + BOOST_CHECK(!demotion.IsStandardMixingEntry()); + + // Edge case: empty entry + CCoinJoinEntry empty; + BOOST_CHECK(empty.IsStandardMixingEntry()); // 0 == 0, technically standard + + // Edge case: 1:1 is standard + CCoinJoinEntry one_to_one; + one_to_one.vecTxDSIn.resize(1); + one_to_one.vecTxOut.resize(1); + BOOST_CHECK(one_to_one.IsStandardMixingEntry()); +} + +BOOST_AUTO_TEST_CASE(get_standard_entries_count) +{ + // Test GetStandardEntriesCount() methods for privacy threshold enforcement + // This is tested through CCoinJoinBaseSession which vecEntries is part of + // We verify the logic by testing IsStandardMixingEntry on various entry types + + // Mix of standard and promotion/demotion entries + std::vector entries; + + // 3 standard entries + for (int i = 0; i < 3; ++i) { + CCoinJoinEntry standard; + standard.vecTxDSIn.resize(5); + standard.vecTxOut.resize(5); + entries.push_back(standard); + } + + // 1 promotion entry + CCoinJoinEntry promotion; + promotion.vecTxDSIn.resize(CoinJoin::PROMOTION_RATIO); + promotion.vecTxOut.resize(1); + entries.push_back(promotion); + + // 1 demotion entry + CCoinJoinEntry demotion; + demotion.vecTxDSIn.resize(1); + demotion.vecTxOut.resize(CoinJoin::PROMOTION_RATIO); + entries.push_back(demotion); + + // Count standard entries manually (what GetStandardEntriesCount should return) + int standard_count = std::count_if(entries.begin(), entries.end(), + [](const CCoinJoinEntry& e) { return e.IsStandardMixingEntry(); }); + + BOOST_CHECK_EQUAL(standard_count, 3); // Only 3 standard, not 5 total + BOOST_CHECK_EQUAL(entries.size(), 5); // Total is 5 + + // Verify promotion and demotion are NOT counted + BOOST_CHECK(!promotion.IsStandardMixingEntry()); + BOOST_CHECK(!demotion.IsStandardMixingEntry()); +} BOOST_AUTO_TEST_SUITE_END() From 1f015efdab86c0c554560d600856ed2289cdece6 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 9 Dec 2025 21:45:49 +0300 Subject: [PATCH 06/12] trivial refactor --- src/coinjoin/client.cpp | 6 ++---- src/coinjoin/coinjoin.cpp | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 9094023c616b..ffd3edd0b969 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -1074,10 +1074,8 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- trying queue: %s\n", dsq.ToString()); // For promotion/demotion, we need a queue with the target denomination - if ((fPromotion || fDemotion) && nTargetDenom != 0) { - if (dsq.nDenom != nTargetDenom) { - continue; // Skip queues with wrong denomination - } + if ((fPromotion || fDemotion) && nTargetDenom != 0 && dsq.nDenom != nTargetDenom) { + continue; // Skip queues with wrong denomination } std::vector vecTxDSInTmp; diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 60c651ca5301..1b0f5ec23379 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -103,10 +103,8 @@ bool CCoinJoinBroadcastTx::IsValidStructure(const CBlockIndex* pindex) const // Pre-V24: require balanced input/output counts (1:1 mixing only) // Post-V24: allow unbalanced counts (promotion/demotion) - if (!fV24Active) { - if (tx->vin.size() != tx->vout.size()) { - return false; - } + if (!fV24Active && tx->vin.size() != tx->vout.size()) { + return false; } if (tx->vin.size() < size_t(CoinJoin::GetMinPoolParticipants())) { From 79ae06435d5f15d0d62aab5707cdaedba36e1213 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 17 Mar 2026 09:46:10 -0500 Subject: [PATCH 07/12] docs: add release notes for pr 7052 --- doc/release-notes-7052.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 doc/release-notes-7052.md diff --git a/doc/release-notes-7052.md b/doc/release-notes-7052.md new file mode 100644 index 000000000000..d800e3a509b3 --- /dev/null +++ b/doc/release-notes-7052.md @@ -0,0 +1,9 @@ +Wallet +------ + +- CoinJoin can now promote and demote between adjacent standard + denominations within a mixing session after V24 activation. + Promotion combines 10 inputs of one denomination into 1 output of the + next larger denomination, while demotion splits 1 input into 10 + outputs of the next smaller denomination. Pre-V24 behavior remains + unchanged. (#7052) From 1141bf49cbe1fe1dcf8c6f261bae297a3d4b7dfb Mon Sep 17 00:00:00 2001 From: Pasta Lil Claw Date: Tue, 17 Mar 2026 14:43:29 -0500 Subject: [PATCH 08/12] fix: address review feedback for CoinJoin promotion/demotion * 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) * 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) * 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) * 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) * 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) * 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) --------- Co-authored-by: PastaClaw Co-authored-by: Claude Opus 4.6 (1M context) --- src/coinjoin/client.cpp | 44 +++++++++++++++++++++++---------------- src/coinjoin/client.h | 2 +- src/coinjoin/coinjoin.cpp | 21 ++++++------------- src/coinjoin/server.cpp | 25 +++++++++++++++------- src/wallet/coinjoin.cpp | 4 ++++ 5 files changed, 55 insertions(+), 41 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index ffd3edd0b969..ad013af506a8 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -181,10 +181,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); @@ -193,7 +193,7 @@ void CCoinJoinClientSession::SetNull() } m_fPromotion = false; m_fDemotion = false; - m_vecPromotionInputs.clear(); + m_vecRebalanceInputs.clear(); CCoinJoinBaseSession::SetNull(); } @@ -1093,6 +1093,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); @@ -1162,23 +1166,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)); } @@ -1295,6 +1299,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); @@ -1384,15 +1392,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; } @@ -1640,8 +1648,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; } @@ -1654,7 +1662,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"; @@ -1711,8 +1719,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; } @@ -1725,7 +1733,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 cb7a719699df..b7698a9f918a 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -87,7 +87,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); diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 1b0f5ec23379..87c7552fe0f7 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -278,32 +278,23 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll 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) { diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 194b0cc58dcc..8d8f2a7bcecc 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -215,8 +215,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); @@ -311,9 +315,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 @@ -656,8 +663,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) { 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 23a55bc8cf7450207190be6331918d555b2d8549 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 4 Jul 2026 11:13:52 -0500 Subject: [PATCH 09/12] fix: adapt CoinJoin promotion/demotion to develop API changes Post-rebase fixups: llmq::CChainLocksHandler was replaced by chainlock::Chainlocks, DEPLOYMENT_V24 became a versionbits deployment so DeploymentActiveAt needs the ChainstateManager, CDeterministicMNList count getters moved to GetCounts(), and the chainlocks handler moved from LLMQContext to NodeContext. Co-Authored-By: Claude Fable 5 --- src/coinjoin/client.cpp | 7 ++++--- src/coinjoin/coinjoin.cpp | 8 ++++---- src/coinjoin/coinjoin.h | 4 ++-- src/coinjoin/server.cpp | 4 ++-- src/net_processing.cpp | 2 +- src/test/coinjoin_inouts_tests.cpp | 28 ++++++++++++++-------------- 6 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index ad013af506a8..3169dca555fa 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -879,7 +879,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman { LOCK(::cs_main); const CBlockIndex* pindex = chainman.ActiveChain().Tip(); - fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + fV24Active = pindex && DeploymentActiveAt(*pindex, chainman, Consensus::DEPLOYMENT_V24); } if (fV24Active) { @@ -1345,8 +1345,9 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon int nTries = 0; const auto mnList = m_dmnman.GetListAtChainTip(); - const int nMnCount = mnList.GetValidMNsCount(); - const int nWeightedMnCount = mnList.GetValidWeightedMNsCount(); + const auto mnCounts = mnList.GetCounts(); + const int nMnCount = mnCounts.enabled(); + const int nWeightedMnCount = mnCounts.m_valid_weighted; while (nTries < 10) { auto dmn = m_clientman.GetRandomNotUsedMasternode(); diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 87c7552fe0f7..34baba6addef 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -84,7 +84,7 @@ bool CCoinJoinBroadcastTx::CheckSignature(const CBLSPublicKey& blsPubKey) const return true; } -bool CCoinJoinBroadcastTx::IsExpired(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler) const +bool CCoinJoinBroadcastTx::IsExpired(const CBlockIndex* pindex, const chainlock::Chainlocks& clhandler) const { // expire confirmed DSTXes after ~1h since confirmation or chainlocked confirmation if (!nConfirmedHeight.has_value() || pindex->nHeight < *nConfirmedHeight) return false; // not mined yet @@ -92,14 +92,14 @@ bool CCoinJoinBroadcastTx::IsExpired(const CBlockIndex* pindex, const llmq::CCha return clhandler.HasChainLock(pindex->nHeight, *pindex->phashBlock); } -bool CCoinJoinBroadcastTx::IsValidStructure(const CBlockIndex* pindex) const +bool CCoinJoinBroadcastTx::IsValidStructure(const CBlockIndex* pindex, const ChainstateManager& chainman) const { // some trivial checks only if (masternodeOutpoint.IsNull() && m_protxHash.IsNull()) { return false; } - const bool fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + const bool fV24Active = pindex && DeploymentActiveAt(*pindex, chainman, Consensus::DEPLOYMENT_V24); // Pre-V24: require balanced input/output counts (1:1 mixing only) // Post-V24: allow unbalanced counts (promotion/demotion) @@ -243,7 +243,7 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll { LOCK(::cs_main); const CBlockIndex* pindex = active_chainstate.m_chain.Tip(); - fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + fV24Active = pindex && DeploymentActiveAt(*pindex, active_chainstate.m_chainman, Consensus::DEPLOYMENT_V24); } // Determine entry type based on input/output counts diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index 7724e975d2b3..589d9772e8bc 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -319,8 +319,8 @@ class CCoinJoinBroadcastTx [[nodiscard]] const std::optional& GetConfirmedHeight() const { return nConfirmedHeight; } void SetConfirmedHeight(std::optional nConfirmedHeightIn) { assert(nConfirmedHeightIn == std::nullopt || *nConfirmedHeightIn > 0); nConfirmedHeight = nConfirmedHeightIn; } - [[nodiscard]] bool IsExpired(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler) const; - [[nodiscard]] bool IsValidStructure(const CBlockIndex* pindex) const; + [[nodiscard]] bool IsExpired(const CBlockIndex* pindex, const chainlock::Chainlocks& clhandler) const; + [[nodiscard]] bool IsValidStructure(const CBlockIndex* pindex, const ChainstateManager& chainman) const; }; // base class diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 8d8f2a7bcecc..1f26e9a08507 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -219,7 +219,7 @@ void CCoinJoinServer::ProcessDSVIN(CNode& peer, CDataStream& vRecv) { LOCK(::cs_main); const CBlockIndex* pindex = m_chainman.ActiveChain().Tip(); - fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + fV24Active = pindex && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_V24); } if (!fV24Active) { LogPrint(BCLog::COINJOIN, "DSVIN -- promotion/demotion entry rejected: V24 not active\n"); @@ -667,7 +667,7 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag { LOCK(::cs_main); const CBlockIndex* pindex = m_chainman.ActiveChain().Tip(); - fV24Active = pindex && DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V24); + fV24Active = pindex && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_V24); } const size_t nMaxEntryInputs = fV24Active ? CoinJoin::PROMOTION_RATIO : COINJOIN_ENTRY_MAX_SIZE; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9d71a051e949..13198063c7da 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3598,7 +3598,7 @@ static DSTXValidationResult ValidateDSTX(CDeterministicMNManager& dmnman, CDSTXM pindex = chainman.ActiveChain().Tip(); } - if (!dstx.IsValidStructure(pindex)) { + if (!dstx.IsValidStructure(pindex, chainman)) { LogPrint(BCLog::COINJOIN, "DSTX -- Invalid DSTX structure: %s\n", hashTx.ToString()); return {DSTXValidationScore::INVALID, true}; } diff --git a/src/test/coinjoin_inouts_tests.cpp b/src/test/coinjoin_inouts_tests.cpp index 7a25ab2c0d54..5abf5fa1f6cc 100644 --- a/src/test/coinjoin_inouts_tests.cpp +++ b/src/test/coinjoin_inouts_tests.cpp @@ -66,13 +66,13 @@ BOOST_AUTO_TEST_CASE(broadcasttx_isvalidstructure_good_and_bad) good.m_protxHash = uint256::ONE; // at least one of (outpoint, protxhash) must be set } // Pre-V24 behavior (nullptr pindex = pre-fork) - BOOST_CHECK(good.IsValidStructure(nullptr)); + BOOST_CHECK(good.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Bad: both identifiers null CCoinJoinBroadcastTx bad_ids = good; bad_ids.m_protxHash = uint256{}; bad_ids.masternodeOutpoint.SetNull(); - BOOST_CHECK(!bad_ids.IsValidStructure(nullptr)); + BOOST_CHECK(!bad_ids.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Bad: vin/vout size mismatch (invalid pre-V24) CCoinJoinBroadcastTx bad_sizes = good; @@ -81,7 +81,7 @@ BOOST_AUTO_TEST_CASE(broadcasttx_isvalidstructure_good_and_bad) mtx.vout.pop_back(); bad_sizes.tx = MakeTransactionRef(mtx); } - BOOST_CHECK(!bad_sizes.IsValidStructure(nullptr)); + BOOST_CHECK(!bad_sizes.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Bad: non-P2PKH output CCoinJoinBroadcastTx bad_script = good; @@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE(broadcasttx_isvalidstructure_good_and_bad) mtx.vout[0].scriptPubKey = CScript() << OP_RETURN << std::vector{'x'}; bad_script.tx = MakeTransactionRef(mtx); } - BOOST_CHECK(!bad_script.IsValidStructure(nullptr)); + BOOST_CHECK(!bad_script.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Bad: non-denominated amount CCoinJoinBroadcastTx bad_amount = good; @@ -99,7 +99,7 @@ BOOST_AUTO_TEST_CASE(broadcasttx_isvalidstructure_good_and_bad) mtx.vout[0].nValue = 42; // not a valid denom bad_amount.tx = MakeTransactionRef(mtx); } - BOOST_CHECK(!bad_amount.IsValidStructure(nullptr)); + BOOST_CHECK(!bad_amount.IsValidStructure(nullptr, *Assert(m_node.chainman))); } BOOST_AUTO_TEST_CASE(entry_addscriptsig_matches_and_rejects) @@ -380,7 +380,7 @@ BOOST_AUTO_TEST_CASE(broadcasttx_expiry_height_logic) uint256 blk_hash = uint256S("03"); index.nHeight = 125; // 125 - 100 == 25 > 24 → expired by height index.phashBlock = &blk_hash; - BOOST_CHECK(dstx.IsExpired(&index, *Assert(m_node.llmq_ctx->clhandler))); + BOOST_CHECK(dstx.IsExpired(&index, *Assert(m_node.chainlocks))); } // Helper to create a denominated CTxIn with a specific denomination value @@ -629,7 +629,7 @@ BOOST_AUTO_TEST_CASE(isvalidstructure_postfork_unbalanced_valid) } // Pre-V24 (nullptr): unbalanced should fail - BOOST_CHECK(!promo.IsValidStructure(nullptr)); + BOOST_CHECK(!promo.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Note: Post-V24 test would require a valid CBlockIndex with V24 deployment active // which requires more setup. The above confirms pre-fork rejection works. @@ -659,7 +659,7 @@ BOOST_AUTO_TEST_CASE(isvalidstructure_demotion_structure) } // Pre-V24 (nullptr): unbalanced should fail - BOOST_CHECK(!demo.IsValidStructure(nullptr)); + BOOST_CHECK(!demo.IsValidStructure(nullptr, *Assert(m_node.chainman))); } BOOST_AUTO_TEST_CASE(input_limit_prefork) @@ -683,7 +683,7 @@ BOOST_AUTO_TEST_CASE(input_limit_prefork) maxValid.tx = MakeTransactionRef(mtx); maxValid.m_protxHash = uint256::ONE; } - BOOST_CHECK(maxValid.IsValidStructure(nullptr)); + BOOST_CHECK(maxValid.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Transaction with max+1 inputs should be invalid CCoinJoinBroadcastTx tooMany; @@ -699,7 +699,7 @@ BOOST_AUTO_TEST_CASE(input_limit_prefork) tooMany.tx = MakeTransactionRef(mtx); tooMany.m_protxHash = uint256::ONE; } - BOOST_CHECK(!tooMany.IsValidStructure(nullptr)); + BOOST_CHECK(!tooMany.IsValidStructure(nullptr, *Assert(m_node.chainman))); } BOOST_AUTO_TEST_CASE(input_limit_postfork_constants) @@ -779,7 +779,7 @@ BOOST_AUTO_TEST_CASE(isvalidstructure_mixed_session_postfork) } // Pre-V24: Should fail (unbalanced) - BOOST_CHECK(!mixed.IsValidStructure(nullptr)); + BOOST_CHECK(!mixed.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Note: Post-V24 testing requires a valid CBlockIndex with V24 deployment active // which requires more complex test setup with chainstate. The above verifies @@ -891,7 +891,7 @@ BOOST_AUTO_TEST_CASE(isvalidstructure_boundary_input_counts) tx180.tx = MakeTransactionRef(mtx); tx180.m_protxHash = uint256::ONE; } - BOOST_CHECK(tx180.IsValidStructure(nullptr)); // Pre-fork: valid at boundary + BOOST_CHECK(tx180.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Pre-fork: valid at boundary // Create transaction with 181 inputs (invalid pre-fork, would be valid post-fork) CCoinJoinBroadcastTx tx181; @@ -904,7 +904,7 @@ BOOST_AUTO_TEST_CASE(isvalidstructure_boundary_input_counts) tx181.tx = MakeTransactionRef(mtx); tx181.m_protxHash = uint256::ONE; } - BOOST_CHECK(!tx181.IsValidStructure(nullptr)); // Pre-fork: over limit + BOOST_CHECK(!tx181.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Pre-fork: over limit // Transaction at post-fork boundary (200 inputs) CCoinJoinBroadcastTx tx200; @@ -918,7 +918,7 @@ BOOST_AUTO_TEST_CASE(isvalidstructure_boundary_input_counts) tx200.m_protxHash = uint256::ONE; } // Pre-fork: over limit (200 > 180) - BOOST_CHECK(!tx200.IsValidStructure(nullptr)); + BOOST_CHECK(!tx200.IsValidStructure(nullptr, *Assert(m_node.chainman))); // Note: Testing post-fork behavior requires a CBlockIndex with V24 active via EHF } From a424f84e6c1d041240c67c9e3a35a424c7ea521d Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 4 Jul 2026 11:42:02 -0500 Subject: [PATCH 10/12] fix: address code review findings for CoinJoin promotion/demotion - unlock promotion/demotion inputs when a pending masternode connection expires or the masternode can't be found, instead of leaving them permanently locked - replace repeated per-denomination wallet scans in ShouldPromote/ShouldDemote with a single CWallet::GetDenominationCounts() scan shared across all four adjacent-pair checks per DoAutomaticDenominating pass - consolidate the duplicated V24-activation check into CoinJoin::IsPromotionDemotionActive(), used at all call sites in coinjoin.cpp/server.cpp/client.cpp - make DenominationToAmount reuse GetDenominationIndex instead of re-scanning vecStandardDenominations with its own loop Co-Authored-By: Claude Fable 5 --- src/coinjoin/client.cpp | 38 +++++++++++++++++--------------- src/coinjoin/client.h | 6 +++-- src/coinjoin/coinjoin.cpp | 14 +++++++----- src/coinjoin/coinjoin.h | 3 +++ src/coinjoin/common.h | 46 +++++++++++++++------------------------ src/coinjoin/server.cpp | 26 +++++----------------- src/wallet/coinjoin.cpp | 29 ++++++++++++++++++++++++ src/wallet/wallet.h | 16 ++++++++++++++ 8 files changed, 104 insertions(+), 74 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 3169dca555fa..886a2aa669c1 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -5,7 +5,6 @@ #include #include -#include #include #include #include @@ -875,14 +874,11 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman // Post-V24: Check if we should promote or demote denominations // This helps maintain optimal denomination distribution as coins are spent - bool fV24Active{false}; - { - LOCK(::cs_main); - const CBlockIndex* pindex = chainman.ActiveChain().Tip(); - fV24Active = pindex && DeploymentActiveAt(*pindex, chainman, Consensus::DEPLOYMENT_V24); - } + if (CoinJoin::IsPromotionDemotionActive(chainman)) { + // Single wallet scan covering all denominations, reused across every + // adjacent-pair check below instead of re-scanning per pair. + const auto denomCounts = m_wallet->GetDenominationCounts(); - if (fV24Active) { // Check all adjacent denomination pairs for promotion/demotion opportunities // Denominations: 10, 1, 0.1, 0.01, 0.001 (indices 0-4, smaller index = larger denom) for (int i = 0; i < 4; ++i) { @@ -890,7 +886,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman int nSmallerDenom = 1 << (i + 1); // Smaller denomination (e.g., 1 DASH) // Check if we should promote smaller -> larger - if (m_clientman.ShouldPromote(nSmallerDenom, nLargerDenom)) { + if (m_clientman.ShouldPromote(nSmallerDenom, nLargerDenom, denomCounts)) { // Verify we have enough fully-mixed coins for promotion auto vecCoins = m_wallet->SelectFullyMixedForPromotion(nSmallerDenom, CoinJoin::PROMOTION_RATIO); if (static_cast(vecCoins.size()) >= CoinJoin::PROMOTION_RATIO) { @@ -911,7 +907,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman } // Check if we should demote larger -> smaller - if (m_clientman.ShouldDemote(nLargerDenom, nSmallerDenom)) { + if (m_clientman.ShouldDemote(nLargerDenom, nSmallerDenom, denomCounts)) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Demotion opportunity: 1 x %s -> %d x %s\n", CoinJoin::DenominationToString(nLargerDenom), CoinJoin::PROMOTION_RATIO, @@ -1419,6 +1415,7 @@ bool CCoinJoinClientSession::ProcessPendingDsaRequest(CConnman& connman) } else { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- cannot find address to connect, masternode=%s\n", __func__, pendingDsaRequest.GetProTxHash().ToString()); + UnlockCoins(); WITH_LOCK(cs_coinjoin, SetNull()); return false; } @@ -1436,6 +1433,7 @@ bool CCoinJoinClientSession::ProcessPendingDsaRequest(CConnman& connman) } else if (pendingDsaRequest.IsExpired()) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- failed to connect, masternode=%s\n", __func__, pendingDsaRequest.GetProTxHash().ToString()); + UnlockCoins(); WITH_LOCK(cs_coinjoin, SetNull()); } @@ -2257,18 +2255,21 @@ UniValue CCoinJoinClientManager::getJsonInfo() const return obj; } -bool CCoinJoinClientManager::ShouldPromote(int nSmallerDenom, int nLargerDenom) const +bool CCoinJoinClientManager::ShouldPromote(int nSmallerDenom, int nLargerDenom, const wallet::CoinJoinDenomCounts& counts) const { // Validate denominations are adjacent if (!CoinJoin::AreAdjacentDenominations(nSmallerDenom, nLargerDenom)) { return false; } + const int idxSmaller = CoinJoin::GetDenominationIndex(nSmallerDenom); + const int idxLarger = CoinJoin::GetDenominationIndex(nLargerDenom); + const int nGoal = CCoinJoinClientOptions::GetDenomsGoal(); const int nHalfGoal = nGoal / 2; - const int nSmallerCount = m_wallet->CountCoinsByDenomination(nSmallerDenom, /*fFullyMixedOnly=*/false); - const int nLargerCount = m_wallet->CountCoinsByDenomination(nLargerDenom, /*fFullyMixedOnly=*/false); + const int nSmallerCount = counts.total[idxSmaller]; + const int nLargerCount = counts.total[idxLarger]; // Don't sacrifice a denomination that's still being built up if (nSmallerCount < nHalfGoal) { @@ -2284,7 +2285,7 @@ bool CCoinJoinClientManager::ShouldPromote(int nSmallerDenom, int nLargerDenom) // 2. Larger denomination is further from goal (needs more help) // 3. Gap exceeds threshold to prevent oscillation // 4. Have 10 fully-mixed coins to promote - const int nFullyMixedCount = m_wallet->CountCoinsByDenomination(nSmallerDenom, /*fFullyMixedOnly=*/true); + const int nFullyMixedCount = counts.fully_mixed[idxSmaller]; if (nFullyMixedCount < CoinJoin::PROMOTION_RATIO) { return false; } @@ -2292,18 +2293,21 @@ bool CCoinJoinClientManager::ShouldPromote(int nSmallerDenom, int nLargerDenom) return (nLargerDeficit > nSmallerDeficit + CoinJoin::GAP_THRESHOLD); } -bool CCoinJoinClientManager::ShouldDemote(int nLargerDenom, int nSmallerDenom) const +bool CCoinJoinClientManager::ShouldDemote(int nLargerDenom, int nSmallerDenom, const wallet::CoinJoinDenomCounts& counts) const { // Validate denominations are adjacent if (!CoinJoin::AreAdjacentDenominations(nLargerDenom, nSmallerDenom)) { return false; } + const int idxLarger = CoinJoin::GetDenominationIndex(nLargerDenom); + const int idxSmaller = CoinJoin::GetDenominationIndex(nSmallerDenom); + const int nGoal = CCoinJoinClientOptions::GetDenomsGoal(); const int nHalfGoal = nGoal / 2; - const int nLargerCount = m_wallet->CountCoinsByDenomination(nLargerDenom, /*fFullyMixedOnly=*/false); - const int nSmallerCount = m_wallet->CountCoinsByDenomination(nSmallerDenom, /*fFullyMixedOnly=*/false); + const int nLargerCount = counts.total[idxLarger]; + const int nSmallerCount = counts.total[idxSmaller]; // Don't sacrifice a denomination that's still being built up if (nLargerCount < nHalfGoal) { diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index b7698a9f918a..20fa1cb6a612 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -253,17 +253,19 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client * Post-V24: Check if we should promote smaller denominations into larger ones * @param nSmallerDenom The smaller denomination to promote from * @param nLargerDenom The larger denomination to promote into + * @param counts Wallet denomination counts, e.g. from CWallet::GetDenominationCounts() * @return true if promotion is recommended */ - bool ShouldPromote(int nSmallerDenom, int nLargerDenom) const; + bool ShouldPromote(int nSmallerDenom, int nLargerDenom, const wallet::CoinJoinDenomCounts& counts) const; /** * Post-V24: Check if we should demote larger denominations into smaller ones * @param nLargerDenom The larger denomination to demote from * @param nSmallerDenom The smaller denomination to demote into + * @param counts Wallet denomination counts, e.g. from CWallet::GetDenominationCounts() * @return true if demotion is recommended */ - bool ShouldDemote(int nLargerDenom, int nSmallerDenom) const; + bool ShouldDemote(int nLargerDenom, int nSmallerDenom, const wallet::CoinJoinDenomCounts& counts) const; }; #endif // BITCOIN_COINJOIN_CLIENT_H diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 34baba6addef..79f91255d04e 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -229,6 +229,13 @@ std::string CCoinJoinBaseSession::GetStateString() const } } +bool CoinJoin::IsPromotionDemotionActive(const ChainstateManager& chainman) +{ + LOCK(::cs_main); + const CBlockIndex* pindex = chainman.ActiveChain().Tip(); + return pindex && DeploymentActiveAt(*pindex, chainman, Consensus::DEPLOYMENT_V24); +} + bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const llmq::CInstantSendManager& isman, const CTxMemPool& mempool, const std::vector& vin, const std::vector& vout, PoolMessage& nMessageIDRet, @@ -239,12 +246,7 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll if (fConsumeCollateralRet) *fConsumeCollateralRet = false; // Check if V24 is active for promotion/demotion support - bool fV24Active{false}; - { - LOCK(::cs_main); - const CBlockIndex* pindex = active_chainstate.m_chain.Tip(); - fV24Active = pindex && DeploymentActiveAt(*pindex, active_chainstate.m_chainman, Consensus::DEPLOYMENT_V24); - } + const bool fV24Active = CoinJoin::IsPromotionDemotionActive(active_chainstate.m_chainman); // Determine entry type based on input/output counts // Standard: N inputs, N outputs (same denom) diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index 589d9772e8bc..dc967b632a37 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -432,6 +432,9 @@ namespace CoinJoin constexpr CAmount GetMaxPoolAmount() { return COINJOIN_ENTRY_MAX_SIZE * vecStandardDenominations.front(); } + /// Whether denomination promotion/demotion (post-V24) is active at the current chain tip + bool IsPromotionDemotionActive(const ChainstateManager& chainman); + /// If the collateral is valid given by a client bool IsCollateralValid(ChainstateManager& chainman, const llmq::CInstantSendManager& isman, const CTxMemPool& mempool, const CTransaction& txCollateral); diff --git a/src/coinjoin/common.h b/src/coinjoin/common.h index 404a415348d5..149cefb3cd49 100644 --- a/src/coinjoin/common.h +++ b/src/coinjoin/common.h @@ -47,6 +47,21 @@ constexpr std::array vecStandardDenominations{ constexpr std::array GetStandardDenominations() { return vecStandardDenominations; } constexpr CAmount GetSmallestDenomination() { return vecStandardDenominations.back(); } +/** + * Get the index of a denomination in vecStandardDenominations (0=largest, 4=smallest) + * Returns -1 if not a valid denomination + */ +constexpr int GetDenominationIndex(int nDenom) +{ + if (nDenom <= 0) return -1; + for (size_t i = 0; i < vecStandardDenominations.size(); ++i) { + if (nDenom == (1 << i)) { + return static_cast(i); + } + } + return -1; +} + /* Return a bitshifted integer representing a denomination in vecStandardDenominations or 0 if none was found @@ -74,9 +89,7 @@ constexpr CAmount DenominationToAmount(int nDenom) return 0; } - size_t nMaxDenoms = vecStandardDenominations.size(); - - if (nDenom >= (1 << nMaxDenoms) || nDenom < 0) { + if (nDenom >= (1 << vecStandardDenominations.size()) || nDenom < 0) { // out of bounds return -1; } @@ -86,16 +99,8 @@ constexpr CAmount DenominationToAmount(int nDenom) return -2; } - CAmount nDenomAmount{-3}; - - for (size_t i = 0; i < nMaxDenoms; ++i) { - if (nDenom & (1 << i)) { - nDenomAmount = vecStandardDenominations[i]; - break; - } - } - - return nDenomAmount; + const int idx = GetDenominationIndex(nDenom); + return idx >= 0 ? vecStandardDenominations[idx] : -3; } @@ -114,21 +119,6 @@ constexpr CAmount GetMaxCollateralAmount() { return GetCollateralAmount() * 4; } constexpr int PROMOTION_RATIO = 10; // 10 smaller denomination coins = 1 larger denomination coin constexpr int GAP_THRESHOLD = 10; // Deficit gap required to trigger promotion/demotion -/** - * Get the index of a denomination in vecStandardDenominations (0=largest, 4=smallest) - * Returns -1 if not a valid denomination - */ -constexpr int GetDenominationIndex(int nDenom) -{ - if (nDenom <= 0) return -1; - for (size_t i = 0; i < vecStandardDenominations.size(); ++i) { - if (nDenom == (1 << i)) { - return static_cast(i); - } - } - return -1; -} - /** * Check if two denominations are adjacent (one step apart in the denom list) * Used for validating promotion/demotion entries post-V24 diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 1f26e9a08507..4828b057d783 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -213,19 +212,10 @@ void CCoinJoinServer::ProcessDSVIN(CNode& peer, CDataStream& vRecv) LogPrint(BCLog::COINJOIN, "DSVIN -- txCollateral %s", entry.txCollateral->ToString()); /* Continued */ // 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 - bool fV24Active{false}; - { - LOCK(::cs_main); - const CBlockIndex* pindex = m_chainman.ActiveChain().Tip(); - fV24Active = pindex && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_V24); - } - if (!fV24Active) { - LogPrint(BCLog::COINJOIN, "DSVIN -- promotion/demotion entry rejected: V24 not active\n"); - PushStatus(peer, STATUS_REJECTED, ERR_MODE); - return; - } + if (entry.vecTxDSIn.size() != entry.vecTxOut.size() && !CoinJoin::IsPromotionDemotionActive(m_chainman)) { + LogPrint(BCLog::COINJOIN, "DSVIN -- promotion/demotion entry rejected: V24 not active\n"); + PushStatus(peer, STATUS_REJECTED, ERR_MODE); + return; } PoolMessage nMessageID = MSG_NOERR; @@ -663,13 +653,7 @@ 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 - bool fV24Active{false}; - { - LOCK(::cs_main); - const CBlockIndex* pindex = m_chainman.ActiveChain().Tip(); - fV24Active = pindex && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_V24); - } - const size_t nMaxEntryInputs = fV24Active ? CoinJoin::PROMOTION_RATIO : COINJOIN_ENTRY_MAX_SIZE; + const size_t nMaxEntryInputs = CoinJoin::IsPromotionDemotionActive(m_chainman) ? CoinJoin::PROMOTION_RATIO : COINJOIN_ENTRY_MAX_SIZE; if (entry.vecTxDSIn.size() > nMaxEntryInputs) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: too many inputs! %d/%d\n", __func__, entry.vecTxDSIn.size(), nMaxEntryInputs); diff --git a/src/wallet/coinjoin.cpp b/src/wallet/coinjoin.cpp index aaa56dd7efd2..c842be985445 100644 --- a/src/wallet/coinjoin.cpp +++ b/src/wallet/coinjoin.cpp @@ -442,6 +442,35 @@ int CWallet::CountCoinsByDenomination(int nDenom, bool fFullyMixedOnly) const return nCount; } +CoinJoinDenomCounts CWallet::GetDenominationCounts() const +{ + CoinJoinDenomCounts counts; + if (!CCoinJoinClientOptions::IsEnabled()) return counts; + + const auto& denoms = CoinJoin::GetStandardDenominations(); + + LOCK(cs_wallet); + for (const auto& outpoint : setWalletUTXO) { + 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; + const auto denom_it = std::find(denoms.begin(), denoms.end(), nValue); + if (denom_it == denoms.end()) continue; + + // Skip unconfirmed or conflicted + if (GetTxDepthInMainChain(it->second) < 1) continue; + + const size_t idx = static_cast(denom_it - denoms.begin()); + counts.total[idx]++; + if (IsFullyMixed(outpoint)) counts.fully_mixed[idx]++; + } + + return counts; +} + std::vector CWallet::SelectFullyMixedForPromotion(int nDenom, int nCount) const { std::vector vecRet; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b443dcfddac5..7c2006f21bc4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -35,6 +35,7 @@ #include #include +#include #include #include #include @@ -281,6 +282,14 @@ enum class RescanStatus : uint8_t { }; class WalletRescanReserver; //forward declarations for ScanForWalletTransactions/RescanFromTime + +//! Per-denomination coin counts (post-V24 promotion/demotion feature), indexed the same way as +//! CoinJoin::vecStandardDenominations (index 0 = largest denomination). +struct CoinJoinDenomCounts { + std::array total{}; + std::array fully_mixed{}; +}; + /** * A CWallet maintains a set of transactions and balances, and provides the ability to create new transactions. */ @@ -607,6 +616,13 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati */ int CountCoinsByDenomination(int nDenom, bool fFullyMixedOnly = false) const; + /** + * Count coins across all standard denominations in a single wallet scan + * (post-V24 promotion/demotion feature). Prefer this over repeated + * CountCoinsByDenomination() calls when counts for multiple denominations are needed. + */ + CoinJoinDenomCounts GetDenominationCounts() const; + /** * Select fully-mixed coins for promotion (post-V24 feature) * @param nDenom The denomination to select (bitshifted integer) From ff95d445a9e7c9c60c9e0b7ee3c13cba11a5fff2 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 4 Jul 2026 12:22:13 -0500 Subject: [PATCH 11/12] fix: address review findings for CoinJoin promotion/demotion - validate the aggregated final transaction with new aggregate rules (allowed denominations, value balance, promotion/demotion input/output consistency) instead of per-entry shape classification, which rejected every final tx containing a promotion/demotion entry and exposed honest participants to collateral charges - consolidate promotion/demotion input selection into SelectRebalanceInputs(), record locked outpoints in vecOutPointLocked at lock time and unlock on all JoinExistingQueue/StartNewQueue failure paths, fixing coin-lock leaks and the UnlockCoins-before-SetNull ordering bug that left rebalance inputs locked until restart - check promotion/demotion opportunities before the 'nothing to do' / 'not enough funds' early exits so rebalancing works for wallets that reached their anonymization target (the feature's primary use case) - cap promotion/demotion entries per session on the server so the frozen participant set can always reach the standard-mixer minimum, and reset a stuck session immediately instead of stalling everyone until timeout - drop unbalanced DSTXes without a Misbehaving penalty when V24 is not active at the local tip, so honest relayers are not punished around the activation boundary - implement CDSTXManager::IsTxExpired via CCoinJoinBroadcastTx::IsExpired so the unit test covers the production expiry rule Co-Authored-By: Claude Fable 5 --- src/coinjoin/client.cpp | 370 +++++++++++++++++--------------------- src/coinjoin/client.h | 7 + src/coinjoin/coinjoin.cpp | 54 ++++-- src/coinjoin/coinjoin.h | 9 +- src/coinjoin/server.cpp | 23 ++- src/net_processing.cpp | 7 + 6 files changed, 252 insertions(+), 218 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 886a2aa669c1..a1522b17f5d5 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -177,19 +177,8 @@ void CCoinJoinClientSession::SetNull() mixingMasternode = nullptr; pendingDsaRequest = CPendingDsaRequest(); - // 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_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_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); - } - } - } + // Post-V24: rebalance inputs are recorded in vecOutPointLocked at lock time + // (SelectRebalanceInputs), so UnlockCoins() releases them - just clear the bookkeeping m_fPromotion = false; m_fDemotion = false; m_vecRebalanceInputs.clear(); @@ -498,7 +487,7 @@ bool CCoinJoinClientSession::SignFinalTransaction(CNode& peer, Chainstate& activ // Make sure all inputs/outputs are valid PoolMessage nMessageID{MSG_NOERR}; if (!IsValidInOuts(active_chainstate, m_isman, mempool, finalMutableTransaction.vin, finalMutableTransaction.vout, - nMessageID, nullptr)) { + nMessageID, nullptr, /*fFinalTx=*/true)) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- ERROR! IsValidInOuts() failed: %s\n", __func__, CoinJoin::GetMessageByID(nMessageID).translated); UnlockCoins(); keyHolderStorage.ReturnAll(); @@ -716,7 +705,13 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman if (!CCoinJoinClientOptions::IsEnabled()) return false; + // Post-V24: whether denomination promotion/demotion is active. Evaluated before taking + // cs_wallet (IsPromotionDemotionActive locks cs_main). + const bool fV24Active = CoinJoin::IsPromotionDemotionActive(chainman); + CAmount nBalanceNeedsAnonymized; + wallet::CoinJoinDenomCounts denomCounts; + bool fRebalanceOpportunity{false}; { LOCK(m_wallet->cs_wallet); @@ -750,7 +745,22 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman CAmount nBalanceAnonymized = bal.m_anonymized; nBalanceNeedsAnonymized = CCoinJoinClientOptions::GetAmount() * COIN - nBalanceAnonymized; - if (nBalanceNeedsAnonymized < 0) { + // Post-V24: check for promotion/demotion opportunities before the "nothing to do" + // exits below - rebalancing matters precisely when the anonymization target is + // reached and all coins are already fully mixed + if (fV24Active) { + // Single wallet scan covering all denominations, reused across every + // adjacent-pair check instead of re-scanning per pair. + denomCounts = m_wallet->GetDenominationCounts(); + for (size_t i = 0; i + 1 < CoinJoin::vecStandardDenominations.size() && !fRebalanceOpportunity; ++i) { + const int nLargerDenom = 1 << i; + const int nSmallerDenom = 1 << (i + 1); + fRebalanceOpportunity = m_clientman.ShouldPromote(nSmallerDenom, nLargerDenom, denomCounts) || + m_clientman.ShouldDemote(nLargerDenom, nSmallerDenom, denomCounts); + } + } + + if (nBalanceNeedsAnonymized < 0 && !fRebalanceOpportunity) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Nothing to do\n"); // nothing to do, just keep it in idle mode return false; @@ -767,8 +777,9 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman // including denoms but applying some restrictions CAmount nBalanceAnonymizable = m_wallet->GetAnonymizableBalance(); - // mixable balance is way too small - if (nBalanceAnonymizable < nValueMin) { + // mixable balance is way too small; note that fully-mixed coins don't count as + // anonymizable, so a rebalance opportunity must bypass this check too + if (nBalanceAnonymizable < nValueMin && !fRebalanceOpportunity) { strAutoDenomResult = _("Not enough funds to mix."); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- %s\n", strAutoDenomResult.original); return false; @@ -874,35 +885,28 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman // Post-V24: Check if we should promote or demote denominations // This helps maintain optimal denomination distribution as coins are spent - if (CoinJoin::IsPromotionDemotionActive(chainman)) { - // Single wallet scan covering all denominations, reused across every - // adjacent-pair check below instead of re-scanning per pair. - const auto denomCounts = m_wallet->GetDenominationCounts(); - + if (fRebalanceOpportunity) { // Check all adjacent denomination pairs for promotion/demotion opportunities // Denominations: 10, 1, 0.1, 0.01, 0.001 (indices 0-4, smaller index = larger denom) - for (int i = 0; i < 4; ++i) { - int nLargerDenom = 1 << i; // Larger denomination (e.g., 10 DASH) - int nSmallerDenom = 1 << (i + 1); // Smaller denomination (e.g., 1 DASH) + for (size_t i = 0; i + 1 < CoinJoin::vecStandardDenominations.size(); ++i) { + const int nLargerDenom = 1 << i; // Larger denomination (e.g., 10 DASH) + const int nSmallerDenom = 1 << (i + 1); // Smaller denomination (e.g., 1 DASH) - // Check if we should promote smaller -> larger + // Check if we should promote smaller -> larger; ShouldPromote() requires + // PROMOTION_RATIO fully-mixed coins, the queue functions re-verify on selection if (m_clientman.ShouldPromote(nSmallerDenom, nLargerDenom, denomCounts)) { - // Verify we have enough fully-mixed coins for promotion - auto vecCoins = m_wallet->SelectFullyMixedForPromotion(nSmallerDenom, CoinJoin::PROMOTION_RATIO); - if (static_cast(vecCoins.size()) >= CoinJoin::PROMOTION_RATIO) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Promotion opportunity: %d x %s -> 1 x %s\n", - CoinJoin::PROMOTION_RATIO, - CoinJoin::DenominationToString(nSmallerDenom), - CoinJoin::DenominationToString(nLargerDenom)); - - // Try to join an existing queue for promotion - if (JoinExistingQueue(nBalanceNeedsAnonymized, connman, nSmallerDenom, /*fPromotion=*/true)) { - return true; - } - // No existing queue found - try to start a new one for promotion - if (StartNewQueue(nBalanceNeedsAnonymized, connman, nSmallerDenom, /*fPromotion=*/true, /*fDemotion=*/false)) { - return true; - } + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- Promotion opportunity: %d x %s -> 1 x %s\n", + CoinJoin::PROMOTION_RATIO, + CoinJoin::DenominationToString(nSmallerDenom), + CoinJoin::DenominationToString(nLargerDenom)); + + // Try to join an existing queue for promotion + if (JoinExistingQueue(nBalanceNeedsAnonymized, connman, nSmallerDenom, /*fPromotion=*/true)) { + return true; + } + // No existing queue found - try to start a new one for promotion + if (StartNewQueue(nBalanceNeedsAnonymized, connman, nSmallerDenom, /*fPromotion=*/true, /*fDemotion=*/false)) { + return true; } } @@ -925,13 +929,17 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman } } - // Always attempt to join an existing queue - if (JoinExistingQueue(nBalanceNeedsAnonymized, connman)) { - return true; - } + // Standard mixing only makes sense while there is still balance to anonymize; a + // rebalance-only pass (target reached) must not fall through to standard queues + if (nBalanceNeedsAnonymized > 0) { + // Always attempt to join an existing queue + if (JoinExistingQueue(nBalanceNeedsAnonymized, connman)) { + return true; + } - // If we were unable to find/join an existing queue then start a new one. - if (StartNewQueue(nBalanceNeedsAnonymized, connman)) return true; + // If we were unable to find/join an existing queue then start a new one. + if (StartNewQueue(nBalanceNeedsAnonymized, connman)) return true; + } strAutoDenomResult = _("No compatible Masternode found."); return false; @@ -1036,6 +1044,83 @@ static int WinnersToSkip() ? 1 : 8; } +bool CCoinJoinClientSession::SelectRebalanceInputs(int nTargetDenom, bool fPromotion, std::vector& vecTxDSInRet) +{ + vecTxDSInRet.clear(); + m_vecRebalanceInputs.clear(); + + if (fPromotion) { + // Promotion: select 10 fully-mixed coins of the smaller denomination + auto vecCoins = m_wallet->SelectFullyMixedForPromotion(nTargetDenom, CoinJoin::PROMOTION_RATIO); + if (static_cast(vecCoins.size()) < CoinJoin::PROMOTION_RATIO) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- Not enough fully-mixed coins for promotion\n", __func__); + return false; + } + // Convert COutPoints to CTxDSIn + LOCK(m_wallet->cs_wallet); + for (const auto& outpoint : vecCoins) { + const auto it = m_wallet->mapWallet.find(outpoint.hash); + if (it == m_wallet->mapWallet.end()) continue; + const wallet::CWalletTx& wtx = it->second; + if (outpoint.n >= wtx.tx->vout.size()) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- invalid outpoint index %u for tx %s\n", __func__, outpoint.n, outpoint.hash.ToString()); + continue; + } + vecTxDSInRet.emplace_back(CTxIn(outpoint), wtx.tx->vout[outpoint.n].scriptPubKey, + m_wallet->GetRealOutpointCoinJoinRounds(outpoint)); + } + if (static_cast(vecTxDSInRet.size()) < CoinJoin::PROMOTION_RATIO) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- Failed to build promotion inputs\n", __func__); + vecTxDSInRet.clear(); + return false; + } + } else { + // Demotion: select 1 coin of the larger adjacent denomination. + // Prefer fully-mixed coins first (they're "done" mixing), then fall back to ready-to-mix. + // According to plan: "Splitting allows unmixed" - demotion can use any denominated coin + const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nTargetDenom); + if (nLargerDenom == 0) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- No larger adjacent denom for demotion\n", __func__); + return false; + } + if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInRet, CoinType::ONLY_FULLY_MIXED) && + !m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInRet, CoinType::ONLY_READY_TO_MIX)) { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- Couldn't find coin for demotion\n", __func__); + return false; + } + // Keep only 1 input for demotion + vecTxDSInRet.resize(1); + } + + // Lock the selected coins immediately to prevent races with other concurrent CoinJoin + // sessions and record them so UnlockCoins() releases them on any failure path + LOCK(m_wallet->cs_wallet); + for (const auto& txdsin : vecTxDSInRet) { + m_wallet->LockCoin(txdsin.prevout); + m_vecRebalanceInputs.push_back(txdsin.prevout); + vecOutPointLocked.push_back(txdsin.prevout); + } + return true; +} + +void CCoinJoinClientSession::UnlockRebalanceInputs() +{ + if (m_vecRebalanceInputs.empty()) return; + { + LOCK(m_wallet->cs_wallet); + for (const auto& outpoint : m_vecRebalanceInputs) { + m_wallet->UnlockCoin(outpoint); + } + } + vecOutPointLocked.erase(std::remove_if(vecOutPointLocked.begin(), vecOutPointLocked.end(), + [&](const COutPoint& outpoint) { + return std::find(m_vecRebalanceInputs.begin(), m_vecRebalanceInputs.end(), + outpoint) != m_vecRebalanceInputs.end(); + }), + vecOutPointLocked.end()); + m_vecRebalanceInputs.clear(); +} + bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman, int nTargetDenom, bool fPromotion, bool fDemotion) { @@ -1044,6 +1129,15 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, // Promotion and demotion are mutually exclusive assert(!(fPromotion && fDemotion)); + // For promotion/demotion select and lock the inputs up front: the selection only depends + // on the target denomination, not on the queue, so there is no need to re-select per queue + std::vector vecRebalanceTxDSIn; + const bool fRebalance = (fPromotion || fDemotion) && nTargetDenom != 0; + if (fRebalance && !SelectRebalanceInputs(nTargetDenom, fPromotion, vecRebalanceTxDSIn)) { + strAutoDenomResult = _("Failed to find mixing queue to join"); + return false; + } + const auto mnList = m_dmnman.GetListAtChainTip(); const int nWeightedMnCount = mnList.GetCounts().m_valid_weighted; @@ -1076,64 +1170,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, std::vector vecTxDSInTmp; - if (fPromotion && nTargetDenom != 0) { - // Promotion: select 10 fully-mixed coins of the smaller denomination - auto vecCoins = m_wallet->SelectFullyMixedForPromotion(nTargetDenom, CoinJoin::PROMOTION_RATIO); - if (static_cast(vecCoins.size()) < CoinJoin::PROMOTION_RATIO) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Not enough fully-mixed coins for promotion\n"); - continue; - } - // Convert COutPoints to CTxDSIn - LOCK(m_wallet->cs_wallet); - for (const auto& outpoint : vecCoins) { - 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); - } - } - if (static_cast(vecTxDSInTmp.size()) < CoinJoin::PROMOTION_RATIO) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Failed to build promotion inputs\n"); - continue; - } - // Lock promotion inputs immediately to prevent race conditions - // with other concurrent CoinJoin sessions - for (const auto& outpoint : vecCoins) { - m_wallet->LockCoin(outpoint); - } - } else if (fDemotion && nTargetDenom != 0) { - // Demotion: select 1 coin of the larger denomination - const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nTargetDenom); - if (nLargerDenom == 0) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- No larger adjacent denom for demotion\n"); - continue; - } - // Select 1 coin of the larger denomination - // Prefer fully-mixed coins first (they're "done" mixing), then fall back to ready-to-mix - // According to plan: "Splitting allows unmixed" - demotion can use any denominated coin - if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInTmp, CoinType::ONLY_FULLY_MIXED)) { - // No fully-mixed coins available, try ready-to-mix - if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInTmp, CoinType::ONLY_READY_TO_MIX)) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Couldn't find coin for demotion\n"); - continue; - } - } - // Keep only 1 input for demotion - if (vecTxDSInTmp.size() > 1) { - vecTxDSInTmp.resize(1); - } - // Lock demotion input immediately to prevent race conditions - if (!vecTxDSInTmp.empty()) { - LOCK(m_wallet->cs_wallet); - m_wallet->LockCoin(vecTxDSInTmp[0].prevout); - } - } else { + if (!fRebalance) { // Standard mixing: try to match their denominations if possible if (!m_wallet->SelectTxDSInsByDenomination(dsq.nDenom, nBalanceNeedsAnonymized, vecTxDSInTmp)) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- Couldn't match denomination %d (%s)\n", dsq.nDenom, CoinJoin::DenominationToString(dsq.nDenom)); @@ -1156,36 +1193,21 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, SetState(POOL_STATE_QUEUE); nTimeLastSuccessfulStep = GetTime(); - // Set promotion/demotion session state + // Set promotion/demotion session state; the rebalance inputs (if any) were already + // selected, locked and recorded by SelectRebalanceInputs above m_fPromotion = fPromotion; m_fDemotion = fDemotion; + if (!fRebalance) m_vecRebalanceInputs.clear(); - // Store promotion inputs for use in PreparePromotionEntry - if (fPromotion) { - m_vecRebalanceInputs.clear(); - for (const auto& txdsin : vecTxDSInTmp) { - 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_vecRebalanceInputs.size()); - } else if (fDemotion) { - // For demotion, store the single input - m_vecRebalanceInputs.clear(); - if (!vecTxDSInTmp.empty()) { - 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_vecRebalanceInputs.clear(); - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- pending connection, masternode=%s, nSessionDenom=%d (%s)\n", - dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom)); - } + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- pending %s connection, masternode=%s, nSessionDenom=%d (%s), %d inputs\n", + fPromotion ? "PROMOTION" : fDemotion ? "DEMOTION" : "mixing", + dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), + vecTxDSInTmp.size() + m_vecRebalanceInputs.size()); strAutoDenomResult = _("Trying to connect…"); return true; } strAutoDenomResult = _("Failed to find mixing queue to join"); + UnlockRebalanceInputs(); return false; } @@ -1279,63 +1301,14 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon if (!CCoinJoinClientOptions::IsEnabled()) return false; if (nTargetDenom == 0) return false; - // For promotion/demotion, verify we have the required coins before starting a queue - std::vector vecTxDSInTmp; + // Promotion and demotion are mutually exclusive, and this overload needs one of them + assert(!(fPromotion && fDemotion)); + if (!fPromotion && !fDemotion) return false; - if (fPromotion) { - // Promotion: need 10 fully-mixed coins of the target (smaller) denomination - auto vecCoins = m_wallet->SelectFullyMixedForPromotion(nTargetDenom, CoinJoin::PROMOTION_RATIO); - if (static_cast(vecCoins.size()) < CoinJoin::PROMOTION_RATIO) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- Not enough fully-mixed coins for promotion\n"); - return false; - } - // Convert to CTxDSIn for storage - LOCK(m_wallet->cs_wallet); - for (const auto& outpoint : vecCoins) { - 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); - } - } - if (static_cast(vecTxDSInTmp.size()) < CoinJoin::PROMOTION_RATIO) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- Failed to build promotion inputs\n"); - return false; - } - // Lock promotion inputs immediately to prevent race conditions - for (const auto& outpoint : vecCoins) { - m_wallet->LockCoin(outpoint); - } - } else if (fDemotion) { - // Demotion: need 1 coin of the larger denomination - const int nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nTargetDenom); - if (nLargerDenom == 0) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- No larger adjacent denom for demotion\n"); - return false; - } - // Prefer fully-mixed coins first, then fall back to ready-to-mix - if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInTmp, CoinType::ONLY_FULLY_MIXED)) { - if (!m_wallet->SelectTxDSInsByDenomination(nLargerDenom, CoinJoin::DenominationToAmount(nLargerDenom), vecTxDSInTmp, CoinType::ONLY_READY_TO_MIX)) { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- Couldn't find coin for demotion\n"); - return false; - } - } - if (vecTxDSInTmp.size() > 1) { - vecTxDSInTmp.resize(1); - } - // Lock demotion input immediately to prevent race conditions - if (!vecTxDSInTmp.empty()) { - LOCK(m_wallet->cs_wallet); - m_wallet->LockCoin(vecTxDSInTmp[0].prevout); - } - } else { - // Neither promotion nor demotion - shouldn't use this overload + // For promotion/demotion, verify we have the required coins before starting a queue. + // SelectRebalanceInputs locks them and records them in vecOutPointLocked/m_vecRebalanceInputs. + std::vector vecTxDSInTmp; + if (!SelectRebalanceInputs(nTargetDenom, fPromotion, vecTxDSInTmp)) { return false; } @@ -1350,6 +1323,7 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon if (!dmn) { strAutoDenomResult = _("Can't find random Masternode."); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- %s\n", strAutoDenomResult.original); + UnlockRebalanceInputs(); return false; } @@ -1386,13 +1360,10 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon SetState(POOL_STATE_QUEUE); nTimeLastSuccessfulStep = GetTime(); - // Store promotion/demotion state and inputs + // Store promotion/demotion state; the inputs were already selected, locked and + // recorded in m_vecRebalanceInputs by SelectRebalanceInputs above m_fPromotion = fPromotion; m_fDemotion = fDemotion; - m_vecRebalanceInputs.clear(); - for (const auto& txdsin : vecTxDSInTmp) { - 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", @@ -1402,6 +1373,7 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon return true; } strAutoDenomResult = _("Failed to start a new mixing queue"); + UnlockRebalanceInputs(); return false; } @@ -1696,15 +1668,7 @@ bool CCoinJoinClientSession::PreparePromotionEntry(std::string& strErrorRet, std vecPSInOutPairsRet.back().second = CTxOut(nLargerAmount, scriptDenom); } - // Lock all inputs (should already be locked from JoinExistingQueue/StartNewQueue) - // Add to vecOutPointLocked for proper cleanup in SetNull() - for (const auto& [txDsIn, txDsOut] : vecPSInOutPairsRet) { - if (!m_wallet->IsLockedCoin(txDsIn.prevout)) { - // Defensive: lock if not already locked - m_wallet->LockCoin(txDsIn.prevout); - } - vecOutPointLocked.push_back(txDsIn.prevout); - } + // NOTE: all inputs were locked and recorded in vecOutPointLocked by SelectRebalanceInputs WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::PreparePromotionEntry -- Prepared %d inputs for promotion to %s\n", vecPSInOutPairsRet.size(), CoinJoin::DenominationToString(nLargerDenom)); @@ -1769,13 +1733,7 @@ bool CCoinJoinClientSession::PrepareDemotionEntry(std::string& strErrorRet, std: } } - // Lock the input (should already be locked from JoinExistingQueue/StartNewQueue) - // Add to vecOutPointLocked for proper cleanup in SetNull() - if (!m_wallet->IsLockedCoin(txdsin.prevout)) { - // Defensive: lock if not already locked - m_wallet->LockCoin(txdsin.prevout); - } - vecOutPointLocked.push_back(txdsin.prevout); + // NOTE: the input was locked and recorded in vecOutPointLocked by SelectRebalanceInputs WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::PrepareDemotionEntry -- Prepared 1 input for demotion to %d x %s\n", CoinJoin::PROMOTION_RATIO, CoinJoin::DenominationToString(nSessionDenom)); diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 20fa1cb6a612..9194c90279de 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -108,6 +108,13 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession bool StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman, int nTargetDenom, bool fPromotion, bool fDemotion); + /// Post-V24: select and lock inputs for a promotion/demotion session. Locked outpoints are + /// recorded in m_vecRebalanceInputs and vecOutPointLocked so UnlockCoins() releases them on + /// any failure path. + bool SelectRebalanceInputs(int nTargetDenom, bool fPromotion, std::vector& vecTxDSInRet); + /// Post-V24: unlock and forget the inputs selected by SelectRebalanceInputs (session setup failed) + void UnlockRebalanceInputs(); + /// step 0: select denominated inputs and txouts bool SelectDenominate(std::string& strErrorRet, std::vector& vecTxDSInRet); /// step 1: prepare denominated inputs and outputs diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 79f91255d04e..e2c87957b8c1 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -239,7 +239,7 @@ bool CoinJoin::IsPromotionDemotionActive(const ChainstateManager& chainman) bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const llmq::CInstantSendManager& isman, const CTxMemPool& mempool, const std::vector& vin, const std::vector& vout, PoolMessage& nMessageIDRet, - bool* fConsumeCollateralRet) const + bool* fConsumeCollateralRet, bool fFinalTx) const { std::set setScripPubKeys; nMessageIDRet = MSG_NOERR; @@ -252,10 +252,13 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll // Standard: N inputs, N outputs (same denom) // Promotion: PROMOTION_RATIO inputs of session denom, 1 output of larger adjacent denom // Demotion: 1 input of larger adjacent denom, PROMOTION_RATIO outputs of session denom - enum class EntryType { STANDARD, PROMOTION, DEMOTION, INVALID }; + // FinalTx: aggregate of the above across all participants - per-entry shapes don't apply + enum class EntryType { STANDARD, PROMOTION, DEMOTION, FINAL_TX, INVALID }; EntryType entryType = EntryType::STANDARD; - if (vin.size() == vout.size()) { + if (fFinalTx && fV24Active) { + entryType = EntryType::FINAL_TX; + } else if (vin.size() == vout.size()) { entryType = EntryType::STANDARD; } else if (fV24Active) { if (vin.size() == static_cast(CoinJoin::PROMOTION_RATIO) && vout.size() == 1) { @@ -284,6 +287,10 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll // and determine expected denominations for UTXO input validation int nExpectedInputDenom = nSessionDenom; int nExpectedOutputDenom = nSessionDenom; + // Final tx only: promotion outputs and demotion inputs use the larger adjacent + // denomination, so it is allowed in addition to the session denomination. + // 0 (never matched) for per-entry validation or when nSessionDenom is the largest denom. + int nLargerDenom{0}; if (entryType == EntryType::PROMOTION) { if (!CoinJoin::ValidatePromotionEntry(vin, vout, nSessionDenom, nMessageIDRet)) { @@ -297,12 +304,14 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll return false; } nExpectedInputDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); + } else if (entryType == EntryType::FINAL_TX) { + nLargerDenom = CoinJoin::GetLargerAdjacentDenom(nSessionDenom); } auto checkTxOut = [&](const CTxOut& txout, int nExpectedDenom) { const int nDenom = CoinJoin::AmountToDenomination(txout.nValue); - if (nDenom != nExpectedDenom) { + if (nDenom != nExpectedDenom && (nLargerDenom == 0 || nDenom != nLargerDenom)) { LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::IsValidInOuts -- ERROR: incompatible denom %d (%s) != expected %d (%s)\n", nDenom, CoinJoin::DenominationToString(nDenom), nExpectedDenom, CoinJoin::DenominationToString(nExpectedDenom)); nMessageIDRet = ERR_DENOM; @@ -326,11 +335,16 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll }; CAmount nFees{0}; + size_t nLargerInputs{0}; + size_t nLargerOutputs{0}; for (const auto& txout : vout) { if (!checkTxOut(txout, nExpectedOutputDenom)) { return false; } + if (nLargerDenom != 0 && CoinJoin::AmountToDenomination(txout.nValue) == nLargerDenom) { + ++nLargerOutputs; + } nFees -= txout.nValue; } @@ -358,6 +372,10 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll return false; } + if (nLargerDenom != 0 && CoinJoin::AmountToDenomination(coin.out.nValue) == nLargerDenom) { + ++nLargerInputs; + } + nFees += coin.out.nValue; } @@ -369,9 +387,29 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll return false; } + if (entryType == EntryType::FINAL_TX) { + // Aggregate consistency: every larger-denom output consumes PROMOTION_RATIO session-denom + // inputs (promotion) and every larger-denom input produces PROMOTION_RATIO session-denom + // outputs (demotion); the remainder on both sides are standard 1:1 inputs/outputs and + // must not be negative. Together with the zero-fee check above this guarantees the final + // tx is composable from valid standard/promotion/demotion entries. + const size_t nSessionInputs = vin.size() - nLargerInputs; + const size_t nSessionOutputs = vout.size() - nLargerOutputs; + if (nSessionInputs < nLargerOutputs * size_t(CoinJoin::PROMOTION_RATIO) || + nSessionOutputs < nLargerInputs * size_t(CoinJoin::PROMOTION_RATIO)) { + LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- ERROR: inconsistent final tx composition! %d/%d session denom, %d/%d larger denom inputs/outputs\n", + __func__, nSessionInputs, nSessionOutputs, nLargerInputs, nLargerOutputs); + nMessageIDRet = ERR_SIZE_MISMATCH; + return false; + } + } + LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- Valid %s entry: %d inputs, %d outputs\n", __func__, - entryType == EntryType::PROMOTION ? "PROMOTION" : (entryType == EntryType::DEMOTION ? "DEMOTION" : "STANDARD"), + entryType == EntryType::PROMOTION ? "PROMOTION" : + entryType == EntryType::DEMOTION ? "DEMOTION" : + entryType == EntryType::FINAL_TX ? "FINAL_TX" : + "STANDARD", vin.size(), vout.size()); return true; @@ -520,11 +558,7 @@ CCoinJoinBroadcastTx CDSTXManager::GetDSTX(const uint256& hash) bool CDSTXManager::IsTxExpired(const CCoinJoinBroadcastTx& tx, const CBlockIndex* pindex) const { - // expire confirmed DSTXes after ~1h since confirmation or chainlocked - const auto& opt_confirmed_height = tx.GetConfirmedHeight(); - if (!opt_confirmed_height.has_value() || pindex->nHeight < *opt_confirmed_height) return false; // not mined yet - return (pindex->nHeight - *opt_confirmed_height > 24) || - m_chainlocks.HasChainLock(pindex->nHeight, *pindex->phashBlock); // mined more than an hour ago or chainlocked + return tx.IsExpired(pindex, m_chainlocks); } void CDSTXManager::CheckDSTXes(const CBlockIndex* pindex) diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index dc967b632a37..cfd0121f8142 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -340,9 +340,16 @@ class CCoinJoinBaseSession virtual void SetNull() EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); + /** + * Validate inputs/outputs of a single mixing entry or, with fFinalTx=true, of the + * aggregated final transaction. Post-V24 a final transaction concatenates standard (N:N), + * promotion (10:1) and demotion (1:10) entries, so per-entry shape rules don't apply to it; + * aggregate rules (allowed denominations, value balance, input/output consistency) are + * checked instead. + */ bool IsValidInOuts(Chainstate& active_chainstate, const llmq::CInstantSendManager& isman, const CTxMemPool& mempool, const std::vector& vin, const std::vector& vout, - PoolMessage& nMessageIDRet, bool* fConsumeCollateralRet) const; + PoolMessage& nMessageIDRet, bool* fConsumeCollateralRet, bool fFinalTx = false) const; public: int nSessionDenom{0}; // Users must submit a denom matching this diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 4828b057d783..42b9fd7889d2 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -310,7 +310,12 @@ void CCoinJoinServer::CheckPool() CreateFinalTransaction(); return; } - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- all entries received but insufficient standard mixers (%d), waiting for timeout\n", GetStandardEntriesCount()); + // The participant set is frozen once entries are being accepted, so this session can + // never reach the standard-mixer minimum anymore - reset instead of stalling everyone + // until timeout. Shouldn't happen given the promotion/demotion entry cap in AddEntry. + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- all entries received but insufficient standard mixers (%d), resetting session\n", GetStandardEntriesCount()); + WITH_LOCK(cs_coinjoin, SetNull()); + return; } // Check for Time Out @@ -662,6 +667,22 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag return false; } + // Post-V24: cap promotion/demotion entries so the session can still reach + // GetMinPoolParticipants() standard entries once every collateral is matched by an + // entry. The participant set is frozen while accepting entries, so without this cap + // the pool could fill up but never satisfy the standard-mixer minimum in CheckPool. + if (!entry.IsStandardMixingEntry()) { + const int nMaxNonStandardEntries = int(vecSessionCollaterals.size()) - CoinJoin::GetMinPoolParticipants(); + const int nNonStandardEntries = GetEntriesCount() - GetStandardEntriesCount(); + if (nNonStandardEntries >= nMaxNonStandardEntries) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- rejecting promotion/demotion entry, not enough standard mixer slots left! %d/%d\n", + __func__, nNonStandardEntries, nMaxNonStandardEntries); + // the entry itself is valid, the session just can't accept it - don't punish + nMessageIDRet = ERR_ENTRIES_FULL; + return false; + } + } + std::vector vin; for (const auto& txin : entry.vecTxDSIn) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- txin=%s\n", __func__, txin.ToString()); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 13198063c7da..6be4a0e52388 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3600,6 +3600,13 @@ static DSTXValidationResult ValidateDSTX(CDeterministicMNManager& dmnman, CDSTXM if (!dstx.IsValidStructure(pindex, chainman)) { LogPrint(BCLog::COINJOIN, "DSTX -- Invalid DSTX structure: %s\n", hashTx.ToString()); + // An unbalanced (promotion/demotion) DSTX is only valid once V24 is active. Around the + // activation boundary our tip may lag the masternode that built it, so - like the + // 24-block-deep masternode scan below - tolerate the skew: drop it without penalizing + // the relaying peer. Honest peers re-relay it once their tip catches up. + if (dstx.tx->vin.size() != dstx.tx->vout.size() && !CoinJoin::IsPromotionDemotionActive(chainman)) { + return {DSTXValidationScore::NONE, true}; + } return {DSTXValidationScore::INVALID, true}; } // It could be that a MN is no longer in the list but its DSTX is not yet mined. From 5294f2c859c369e73c979a7db72e09106c8d83bc Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 4 Jul 2026 12:37:36 -0500 Subject: [PATCH 12/12] fix: narrow the DSTX fork-boundary penalty exemption The previous commit exempted every unbalanced DSTX from the Misbehaving penalty while V24 is inactive at the local tip, which also forgave structurally malformed transactions and weakened the anti-spam pairing with the recent-rejects exemption for DSTX messages. IsValidStructure now reports whether a failing tx would be structurally valid under post-V24 rules (activation-independent checks still enforced), and ValidateDSTX applies a small score-1 penalty (same rationale as UNKNOWN_MASTERNODE) only in that case, keeping the full penalty for garbage. Adds unit coverage for the new flag. Co-Authored-By: Claude Fable 5 --- src/coinjoin/coinjoin.cpp | 51 ++++++++++++++++-------------- src/coinjoin/coinjoin.h | 9 +++++- src/net_processing.cpp | 17 ++++++---- src/test/coinjoin_inouts_tests.cpp | 34 +++++++++++++++++--- 4 files changed, 75 insertions(+), 36 deletions(-) diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index e2c87957b8c1..09b88b2fd0f0 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -92,45 +92,48 @@ bool CCoinJoinBroadcastTx::IsExpired(const CBlockIndex* pindex, const chainlock: return clhandler.HasChainLock(pindex->nHeight, *pindex->phashBlock); } -bool CCoinJoinBroadcastTx::IsValidStructure(const CBlockIndex* pindex, const ChainstateManager& chainman) const +bool CCoinJoinBroadcastTx::IsValidStructure(const CBlockIndex* pindex, const ChainstateManager& chainman, + bool* fPossiblyValidPostV24Ret) const { - // some trivial checks only - if (masternodeOutpoint.IsNull() && m_protxHash.IsNull()) { - return false; - } - - const bool fV24Active = pindex && DeploymentActiveAt(*pindex, chainman, Consensus::DEPLOYMENT_V24); + if (fPossiblyValidPostV24Ret) *fPossiblyValidPostV24Ret = false; - // Pre-V24: require balanced input/output counts (1:1 mixing only) - // Post-V24: allow unbalanced counts (promotion/demotion) - if (!fV24Active && tx->vin.size() != tx->vout.size()) { + // some trivial checks only, activation-independent ones first + if (masternodeOutpoint.IsNull() && m_protxHash.IsNull()) { return false; } if (tx->vin.size() < size_t(CoinJoin::GetMinPoolParticipants())) { return false; } - // Post-V24: allow up to 200 inputs (20 participants * 10 inputs for promotions) - // Pre-V24: max 180 inputs (20 participants * 9 entries) - const size_t nMaxInputs = fV24Active - ? CoinJoin::GetMaxPoolParticipants() * CoinJoin::PROMOTION_RATIO - : CoinJoin::GetMaxPoolInputOutputCount(); - - if (tx->vin.size() > nMaxInputs) { - return false; - } - if (!std::ranges::all_of(tx->vout, [](const auto& txOut) { return CoinJoin::IsDenominatedAmount(txOut.nValue) && txOut.scriptPubKey.IsPayToPublicKeyHash(); })) { return false; } - // Note: For post-V24 unbalanced transactions (promotion/demotion), - // value sum validation (inputs == outputs) requires UTXO access and - // is performed in IsValidInOuts() when the transaction is processed. + // Post-V24: allow unbalanced counts (promotion/demotion) and up to 200 inputs + // (20 participants * 10 inputs for promotions) + // Pre-V24: require balanced input/output counts (1:1 mixing only), max 180 inputs + // (20 participants * 9 entries) + // Note: For post-V24 unbalanced transactions (promotion/demotion), value sum validation + // (inputs == outputs) requires UTXO access and is performed in IsValidInOuts() when the + // transaction is processed. + const size_t nMaxInputsPreV24 = CoinJoin::GetMaxPoolParticipants() * COINJOIN_ENTRY_MAX_SIZE; + const size_t nMaxInputsPostV24 = CoinJoin::GetMaxPoolParticipants() * CoinJoin::PROMOTION_RATIO; + const bool fValidPreV24 = tx->vin.size() == tx->vout.size() && tx->vin.size() <= nMaxInputsPreV24; + const bool fValidPostV24 = tx->vin.size() <= nMaxInputsPostV24; - return true; + const bool fV24Active = pindex && DeploymentActiveAt(*pindex, chainman, Consensus::DEPLOYMENT_V24); + if (fV24Active) { + return fValidPostV24; + } + + // Pre-V24: report whether the tx would be valid on a post-V24 tip so callers can avoid + // fully punishing relayers whose tip is ahead of ours around the activation boundary + if (!fValidPreV24 && fValidPostV24 && fPossiblyValidPostV24Ret) { + *fPossiblyValidPostV24Ret = true; + } + return fValidPreV24; } void CCoinJoinBaseSession::SetNull() diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index cfd0121f8142..d77ab146ff18 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -320,7 +320,14 @@ class CCoinJoinBroadcastTx [[nodiscard]] const std::optional& GetConfirmedHeight() const { return nConfirmedHeight; } void SetConfirmedHeight(std::optional nConfirmedHeightIn) { assert(nConfirmedHeightIn == std::nullopt || *nConfirmedHeightIn > 0); nConfirmedHeight = nConfirmedHeightIn; } [[nodiscard]] bool IsExpired(const CBlockIndex* pindex, const chainlock::Chainlocks& clhandler) const; - [[nodiscard]] bool IsValidStructure(const CBlockIndex* pindex, const ChainstateManager& chainman) const; + /** + * Trivial structural checks against the V24-dependent rules at pindex. When the tx fails + * only because V24 is not active at pindex but would be structurally valid on a post-V24 + * tip, fPossiblyValidPostV24Ret (if provided) is set to true so callers can tolerate tip + * skew around the activation boundary. + */ + [[nodiscard]] bool IsValidStructure(const CBlockIndex* pindex, const ChainstateManager& chainman, + bool* fPossiblyValidPostV24Ret = nullptr) const; }; // base class diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6be4a0e52388..81fe78ae3c4d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3572,6 +3572,7 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& node, Peer& peer, CDataStream& enum class DSTXValidationScore : int { NONE = 0, UNKNOWN_MASTERNODE = 1, + PREMATURE = 1, INVALID = 10, }; @@ -3598,14 +3599,16 @@ static DSTXValidationResult ValidateDSTX(CDeterministicMNManager& dmnman, CDSTXM pindex = chainman.ActiveChain().Tip(); } - if (!dstx.IsValidStructure(pindex, chainman)) { + bool fPossiblyValidPostV24{false}; + if (!dstx.IsValidStructure(pindex, chainman, &fPossiblyValidPostV24)) { LogPrint(BCLog::COINJOIN, "DSTX -- Invalid DSTX structure: %s\n", hashTx.ToString()); - // An unbalanced (promotion/demotion) DSTX is only valid once V24 is active. Around the - // activation boundary our tip may lag the masternode that built it, so - like the - // 24-block-deep masternode scan below - tolerate the skew: drop it without penalizing - // the relaying peer. Honest peers re-relay it once their tip catches up. - if (dstx.tx->vin.size() != dstx.tx->vout.size() && !CoinJoin::IsPromotionDemotionActive(chainman)) { - return {DSTXValidationScore::NONE, true}; + // A tx that is structurally valid under post-V24 rules may come from a masternode + // whose tip is ahead of ours around the activation boundary. Like the 24-block-deep + // masternode scan below, tolerate the skew: drop it with only a small penalty so + // honest relayers are unaffected while a peer flooding us still gets discouraged + // eventually. Anything else malformed keeps the full penalty. + if (fPossiblyValidPostV24) { + return {DSTXValidationScore::PREMATURE, true}; } return {DSTXValidationScore::INVALID, true}; } diff --git a/src/test/coinjoin_inouts_tests.cpp b/src/test/coinjoin_inouts_tests.cpp index 5abf5fa1f6cc..3a290eeb700c 100644 --- a/src/test/coinjoin_inouts_tests.cpp +++ b/src/test/coinjoin_inouts_tests.cpp @@ -628,8 +628,11 @@ BOOST_AUTO_TEST_CASE(isvalidstructure_postfork_unbalanced_valid) promo.m_protxHash = uint256::ONE; } - // Pre-V24 (nullptr): unbalanced should fail - BOOST_CHECK(!promo.IsValidStructure(nullptr, *Assert(m_node.chainman))); + // Pre-V24 (nullptr): unbalanced should fail, but be reported as possibly valid post-V24 + // so relaying peers aren't fully punished around the activation boundary + bool fPossiblyValidPostV24{false}; + BOOST_CHECK(!promo.IsValidStructure(nullptr, *Assert(m_node.chainman), &fPossiblyValidPostV24)); + BOOST_CHECK(fPossiblyValidPostV24); // Note: Post-V24 test would require a valid CBlockIndex with V24 deployment active // which requires more setup. The above confirms pre-fork rejection works. @@ -658,8 +661,31 @@ BOOST_AUTO_TEST_CASE(isvalidstructure_demotion_structure) demo.m_protxHash = uint256::ONE; } - // Pre-V24 (nullptr): unbalanced should fail - BOOST_CHECK(!demo.IsValidStructure(nullptr, *Assert(m_node.chainman))); + // Pre-V24 (nullptr): unbalanced should fail. A single-entry demotion DSTX has just 1 + // input - below GetMinPoolParticipants() under either ruleset - so it must not be + // reported as possibly valid post-V24 either + bool fPossiblyValidPostV24{true}; + BOOST_CHECK(!demo.IsValidStructure(nullptr, *Assert(m_node.chainman), &fPossiblyValidPostV24)); + BOOST_CHECK(!fPossiblyValidPostV24); + + // An unbalanced tx that is garbage under post-V24 rules too (non-denominated output) + // must not be reported as possibly valid + CCoinJoinBroadcastTx garbage; + { + CMutableTransaction mtx; + for (int i = 0; i < CoinJoin::PROMOTION_RATIO; ++i) { + CTxIn in; + in.prevout = COutPoint(uint256::ONE, static_cast(i)); + mtx.vin.push_back(in); + } + CTxOut out{1234567, P2PKHScript(0x01)}; // not a denomination + mtx.vout.push_back(out); + garbage.tx = MakeTransactionRef(mtx); + garbage.m_protxHash = uint256::ONE; + } + fPossiblyValidPostV24 = true; + BOOST_CHECK(!garbage.IsValidStructure(nullptr, *Assert(m_node.chainman), &fPossiblyValidPostV24)); + BOOST_CHECK(!fPossiblyValidPostV24); } BOOST_AUTO_TEST_CASE(input_limit_prefork)