From 24a4ed3f8b1d1317c0ec000662199973b6f835dc Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 17 Aug 2023 07:39:38 +0300 Subject: [PATCH] fix: Update conditions and unify calculations for the number of "winners" to skip when mixing (#5532) ## Issue being fixed or feature implemented `JoinExistingQueue` was tweaked for regtest/devnets in #4394 but we have "skipping winners" logic in `StartNewQueue` too. We should also use weighted count when checking "skip winners" conditions. ## What was done? Add a helper to calculate the number and use it in both methods. Adjust logic. ## How Has This Been Tested? Running a local mixing node on devnet ~- no "skipping winners" in logs anymore.~ and testnet ## Breaking Changes n/a ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone --- src/coinjoin/client.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 1eed993dde..ba238c57f3 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -1042,18 +1042,20 @@ CDeterministicMNCPtr CCoinJoinClientManager::GetRandomNotUsedMasternode() return nullptr; } +static int WinnersToSkip() +{ + return (Params().NetworkIDString() == CBaseChainParams::DEVNET || + Params().NetworkIDString() == CBaseChainParams::REGTEST) + ? 1 : 8; +} + bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman) { if (!CCoinJoinClientOptions::IsEnabled()) return false; if (coinJoinClientQueueManager == nullptr) return false; - auto mnList = deterministicMNManager->GetListAtChainTip(); - - int winners_to_skip{8}; - - if (Params().NetworkIDString() == CBaseChainParams::DEVNET || Params().NetworkIDString() == CBaseChainParams::REGTEST) { - winners_to_skip = 0; - } + const auto mnList = deterministicMNManager->GetListAtChainTip(); + const int nWeightedMnCount = mnList.GetValidWeightedMNsCount(); // Look through the queues and see if anything matches CCoinJoinQueue dsq; @@ -1066,7 +1068,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, } // skip next mn payments winners - if (dmn->pdmnState->nLastPaidHeight + int(mnList.GetValidMNsCount()) < mnList.GetHeight() + winners_to_skip) { + if (dmn->pdmnState->nLastPaidHeight + nWeightedMnCount < mnList.GetHeight() + WinnersToSkip()) { LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::JoinExistingQueue -- skipping winner, masternode=%s\n", dmn->proTxHash.ToString()); continue; } @@ -1113,8 +1115,9 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon if (nBalanceNeedsAnonymized <= 0) return false; int nTries = 0; - auto mnList = deterministicMNManager->GetListAtChainTip(); - int nMnCount = mnList.GetValidMNsCount(); + const auto mnList = deterministicMNManager->GetListAtChainTip(); + const int nMnCount = mnList.GetValidMNsCount(); + const int nWeightedMnCount = mnList.GetValidWeightedMNsCount(); // find available denominated amounts std::set setAmounts; @@ -1138,7 +1141,7 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon coinJoinClientManagers.at(mixingWallet.GetName())->AddUsedMasternode(dmn->collateralOutpoint); // skip next mn payments winners - if (dmn->pdmnState->nLastPaidHeight + nMnCount < mnList.GetHeight() + 8) { + if (dmn->pdmnState->nLastPaidHeight + nWeightedMnCount < mnList.GetHeight() + WinnersToSkip()) { LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::StartNewQueue -- skipping winner, masternode=%s\n", dmn->proTxHash.ToString()); nTries++; continue;