From 9d4df466b943ab8ce647bc936f6781d1dc2d5723 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 25 Nov 2018 16:27:31 +0300 Subject: [PATCH] Fix CreateDenominated failure for addresses with huge amount of inputs (#2486) * Allow specifing max number of outpoints to be returned per address in SelectCoinsGroupedByAddresses Fixes CreateDenominated failure for addresses with huge amount of inputs * Apply suggested fixes --- src/privatesend-client.cpp | 6 +++++- src/wallet/wallet.cpp | 23 ++++++++++++++--------- src/wallet/wallet.h | 2 +- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index 5a72ef800f..be9c870f68 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -1444,8 +1444,12 @@ bool CPrivateSendClientSession::CreateDenominated(CConnman& connman) LOCK2(cs_main, pwalletMain->cs_wallet); + // NOTE: We do not allow txes larger than 100kB, so we have to limit number of inputs here. + // We still want to consume a lot of inputs to avoid creating only smaller denoms though. + // Knowing that each CTxIn is at least 148b big, 400 inputs should take 400 x ~148b = ~60kB. + // This still leaves more than enough room for another data of typical CreateDenominated tx. std::vector vecTally; - if (!pwalletMain->SelectCoinsGroupedByAddresses(vecTally)) { + if (!pwalletMain->SelectCoinsGroupedByAddresses(vecTally, true, true, true, 400)) { LogPrint("privatesend", "CPrivateSendClientSession::CreateDenominated -- SelectCoinsGroupedByAddresses can't find any inputs!\n"); return false; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c00f27e701..f74547b556 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3069,14 +3069,14 @@ struct CompareByAmount } }; -bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTallyRet, bool fSkipDenominated, bool fAnonymizable, bool fSkipUnconfirmed) const +bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTallyRet, bool fSkipDenominated, bool fAnonymizable, bool fSkipUnconfirmed, int nMaxOupointsPerAddress) const { LOCK2(cs_main, cs_wallet); isminefilter filter = ISMINE_SPENDABLE; - // try to use cache for already confirmed anonymizable inputs - if(fAnonymizable && fSkipUnconfirmed) { + // try to use cache for already confirmed anonymizable inputs, no cache should be used when the limit is specified + if(nMaxOupointsPerAddress != -1 && fAnonymizable && fSkipUnconfirmed) { if(fSkipDenominated && fAnonymizableTallyCachedNonDenom) { vecTallyRet = vecAnonymizableTallyCachedNonDenom; LogPrint("selectcoins", "SelectCoinsGroupedByAddresses - using cache for non-denom inputs %d\n", vecTallyRet.size()); @@ -3114,6 +3114,9 @@ bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTa isminefilter mine = ::IsMine(*this, txdest); if(!(mine & filter)) continue; + auto itTallyItem = mapTally.find(txdest); + if (nMaxOupointsPerAddress != -1 && itTallyItem != mapTally.end() && itTallyItem->second.vecOutPoints.size() >= nMaxOupointsPerAddress) continue; + if(IsSpent(outpoint.hash, i) || IsLockedCoin(outpoint.hash, i)) continue; if(fSkipDenominated && CPrivateSend::IsDenominatedAmount(wtx.tx->vout[i].nValue)) continue; @@ -3129,10 +3132,12 @@ bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTa if(GetCappedOutpointPrivateSendRounds(COutPoint(outpoint.hash, i)) >= privateSendClient.nPrivateSendRounds) continue; } - CompactTallyItem& item = mapTally[txdest]; - item.txdest = txdest; - item.nAmount += wtx.tx->vout[i].nValue; - item.vecOutPoints.emplace_back(outpoint.hash, i); + if (itTallyItem == mapTally.end()) { + itTallyItem = mapTally.emplace(txdest, CompactTallyItem()).first; + itTallyItem->second.txdest = txdest; + } + itTallyItem->second.nAmount += wtx.tx->vout[i].nValue; + itTallyItem->second.vecOutPoints.emplace_back(outpoint.hash, i); } } @@ -3146,8 +3151,8 @@ bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTa // order by amounts per address, from smallest to largest std::sort(vecTallyRet.rbegin(), vecTallyRet.rend(), CompareByAmount()); - // cache already confirmed anonymizable entries for later use - if(fAnonymizable && fSkipUnconfirmed) { + // cache already confirmed anonymizable entries for later use, no cache should be saved when the limit is specified + if(nMaxOupointsPerAddress != -1 && fAnonymizable && fSkipUnconfirmed) { if(fSkipDenominated) { vecAnonymizableTallyCachedNonDenom = vecTallyRet; fAnonymizableTallyCachedNonDenom = true; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1e55061a66..d17e8e9574 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -810,7 +810,7 @@ public: bool GetCollateralTxDSIn(CTxDSIn& txdsinRet, CAmount& nValueRet) const; bool SelectPrivateCoins(CAmount nValueMin, CAmount nValueMax, std::vector& vecTxInRet, CAmount& nValueRet, int nPrivateSendRoundsMin, int nPrivateSendRoundsMax) const; - bool SelectCoinsGroupedByAddresses(std::vector& vecTallyRet, bool fSkipDenominated = true, bool fAnonymizable = true, bool fSkipUnconfirmed = true) const; + bool SelectCoinsGroupedByAddresses(std::vector& vecTallyRet, bool fSkipDenominated = true, bool fAnonymizable = true, bool fSkipUnconfirmed = true, int nMaxOupointsPerAddress = -1) const; /// Get 1000DASH output and keys which can be used for the Masternode bool GetMasternodeOutpointAndKeys(COutPoint& outpointRet, CPubKey& pubKeyRet, CKey& keyRet, const std::string& strTxHash = "", const std::string& strOutputIndex = "");