From ce587e3ced614f104159e6e2f7820acd7d32f026 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 14 Aug 2020 15:52:04 -0500 Subject: [PATCH 01/13] Use GetRealOut... instead of Capped Signed-off-by: pasta --- src/wallet/wallet.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 81ba4a4658a5..6361108cf167 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2830,11 +2830,11 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const bool found = false; if (nCoinType == CoinType::ONLY_FULLY_MIXED) { if (!CPrivateSend::IsDenominatedAmount(pcoin->tx->vout[i].nValue)) continue; - int nRounds = GetCappedOutpointPrivateSendRounds(COutPoint(wtxid, i)); + int nRounds = GetRealOutpointPrivateSendRounds(COutPoint(wtxid, i)); found = nRounds >= CPrivateSendClientOptions::GetRounds(); } else if(nCoinType == CoinType::ONLY_READY_TO_MIX) { if (!CPrivateSend::IsDenominatedAmount(pcoin->tx->vout[i].nValue)) continue; - int nRounds = GetCappedOutpointPrivateSendRounds(COutPoint(wtxid, i)); + int nRounds = GetRealOutpointPrivateSendRounds(COutPoint(wtxid, i)); found = nRounds < CPrivateSendClientOptions::GetRounds(); } else if(nCoinType == CoinType::ONLY_NONDENOMINATED) { if (CPrivateSend::IsCollateralAmount(pcoin->tx->vout[i].nValue)) continue; // do not use collateral amounts From 47704a756a4199c946f327f47af58c0beeabef22 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 17 Aug 2020 14:05:48 -0500 Subject: [PATCH 02/13] Add "ps_salt" value to walletdb This value is used to deterministically pick a random number of rounds to mix, between N and N + GetRandomRounds. A salt is needed in addition to the inputs hash to ensure that an attacker learns nothing from looking at the blockchain. Signed-off-by: pasta --- src/privatesend/privatesend-client.cpp | 5 +++++ src/privatesend/privatesend-client.h | 8 ++++++-- src/wallet/wallet.cpp | 14 ++++++++++++++ src/wallet/wallet.h | 10 ++++++++++ src/wallet/walletdb.cpp | 10 ++++++++++ src/wallet/walletdb.h | 3 +++ 6 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp index 775db8b1bc9a..f3557b8877d1 100644 --- a/src/privatesend/privatesend-client.cpp +++ b/src/privatesend/privatesend-client.cpp @@ -244,6 +244,11 @@ bool CPrivateSendClientManager::StartMixing(CWallet* pwallet) { } assert(pwallet != nullptr); mixingWallet = pwallet; + nSalt = mixingWallet->GetPrivateSendSalt(); + if (nSalt.IsNull()) { + nSalt = GetRandHash(); + pwallet->WritePrivateSendSalt(nSalt); + } return true; } diff --git a/src/privatesend/privatesend-client.h b/src/privatesend/privatesend-client.h index 6b979f94a745..bbc4959d7455 100644 --- a/src/privatesend/privatesend-client.h +++ b/src/privatesend/privatesend-client.h @@ -224,8 +224,12 @@ class CPrivateSendClientManager bool CheckAutomaticBackup(); public: - int nCachedNumBlocks; //used for the overview screen - bool fCreateAutoBackups; //builtin support for automatic backups + // used for the overview screen + int nCachedNumBlocks; + // builtin support for automatic backups + bool fCreateAutoBackups; + // Pulled from wallet DB ("ps_salt") and used when mixing a random number of rounds + uint256 nSalt; CPrivateSendClientManager() : vecMasternodesUsed(), diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6361108cf167..86358eb2d2af 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2951,6 +2951,20 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } +const uint256 CWallet::GetPrivateSendSalt() +{ + uint256 ps_salt; + WalletBatch batch(*database); + batch.ReadPrivateSendSalt(ps_salt); + return ps_salt; +} + +bool CWallet::WritePrivateSendSalt(uint256 &salt) +{ + WalletBatch batch(*database); + return batch.WritePrivateSendSalt(salt); +} + static void ApproximateBestSubset(const std::vector& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, std::vector& vfBest, CAmount& nBest, int iterations = 1000) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d665c36872e4..a0c6be9d38c2 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1221,6 +1221,16 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface void NotifyTransactionLock(const CTransaction &tx, const llmq::CInstantSendLock& islock) override; void NotifyChainLock(const CBlockIndex* pindexChainLock, const llmq::CChainLockSig& clsig) override; + /** + * Fetches PrivateSend salt from database + */ + const uint256 GetPrivateSendSalt(); + + /** + * Writes PrivateSend salt to database + */ + bool WritePrivateSendSalt(uint256& salt); + /** * Blocks until the wallet state is up-to-date to /at least/ the current * chain at the time this function is entered diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 61c751b9b228..7f8dac75e5e4 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -168,6 +168,16 @@ bool WalletBatch::WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccou return WriteIC(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry); } +bool WalletBatch::ReadPrivateSendSalt(uint256& salt) +{ + return m_batch.Read(std::string("ps_salt"), salt); +} + +bool WalletBatch::WritePrivateSendSalt(uint256& salt) +{ + return WriteIC(std::string("ps_salt"), salt); +} + CAmount WalletBatch::GetAccountCreditDebit(const std::string& strAccount) { std::list entries; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index d6a2a9e92a80..3f9d55330293 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -160,6 +160,9 @@ class WalletBatch bool ReadAccount(const std::string& strAccount, CAccount& account); bool WriteAccount(const std::string& strAccount, const CAccount& account); + bool WritePrivateSendSalt(uint256& salt); + bool ReadPrivateSendSalt(uint256& salt); + /// Write destination data key,value tuple to database bool WriteDestData(const std::string &address, const std::string &key, const std::string &value); /// Erase destination data tuple from wallet database From da40adf7b0257feb25ce0e3c6940315d19eaaa68 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 17 Aug 2020 14:16:24 -0500 Subject: [PATCH 03/13] Implement Random Round Mixing This implements "Random Round Mixing." Previously, attempted attacks on PrivateSend assumed that all inputs had been mixed for the same number of rounds. Noramlly assuming 2,4,8 or 16. While none of these attacks have been successful, they still teach what we can do to make our system more robust, and one of those ways is to implement "Random Round Mixing". Under the previous system, inputs were mixed up until N rounds (configured by user). At this point, the input was considered mixed, and could be private-sent. Under this new system, an input will be mixed to N rounds like prior. However, at this point, Sha256d(input, salt) will be calculated (note: this likely could be a more efficient hash function than double sha256, but that can be done in another PR / version if needed). If (hash % 2 == 0), then the input will be mixed again. This results in an exponential decay where if you mix a set of inputs, half of those inputs will be mixed for N rounds, 1/4 will be mixed N+1, 1/8 will be mixed N+2, etc. This current implementation caps it at N+2. This results in mixing an average of N+0.875 rounds. If you removed the cap, you would mix on average N+1 rounds. Signed-off-by: pasta --- src/privatesend/privatesend-client.cpp | 2 +- src/privatesend/privatesend-client.h | 5 +++++ src/wallet/wallet.cpp | 19 ++++++++++++++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp index f3557b8877d1..dfcc289bdd3a 100644 --- a/src/privatesend/privatesend-client.cpp +++ b/src/privatesend/privatesend-client.cpp @@ -1259,7 +1259,7 @@ bool CPrivateSendClientSession::SubmitDenominate(CConnman& connman) std::vector > vecInputsByRounds; - for (int i = 0; i < CPrivateSendClientOptions::GetRounds(); i++) { + for (int i = 0; i < CPrivateSendClientOptions::GetRounds() + CPrivateSendClientOptions::GetRandomRounds(); i++) { if (PrepareDenominate(i, i, strError, vecPSInOutPairs, vecPSInOutPairsTmp, true)) { LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::SubmitDenominate -- Running PrivateSend denominate for %d rounds, success\n", i); vecInputsByRounds.emplace_back(i, vecPSInOutPairsTmp.size()); diff --git a/src/privatesend/privatesend-client.h b/src/privatesend/privatesend-client.h index bbc4959d7455..0835f67af1d0 100644 --- a/src/privatesend/privatesend-client.h +++ b/src/privatesend/privatesend-client.h @@ -54,6 +54,8 @@ static const int PRIVATESEND_DENOM_OUTPUTS_THRESHOLD = 500; static const int PRIVATESEND_KEYS_THRESHOLD_WARNING = 100; // Stop mixing completely, it's too dangerous to continue when we have only this many keys left static const int PRIVATESEND_KEYS_THRESHOLD_STOP = 50; +// Pseudorandomly mix up to this many times in addition to base round count +static const int PRIVATESEND_RANDOM_ROUNDS = 3; // The main object for accessing mixing extern std::map privateSendClientManagers; @@ -284,6 +286,7 @@ class CPrivateSendClientOptions public: static int GetSessions() { return CPrivateSendClientOptions::Get().nPrivateSendSessions; } static int GetRounds() { return CPrivateSendClientOptions::Get().nPrivateSendRounds; } + static int GetRandomRounds() { return CPrivateSendClientOptions::Get().nPrivateSendRandomRounds; } static int GetAmount() { return CPrivateSendClientOptions::Get().nPrivateSendAmount; } static int GetDenomsGoal() { return CPrivateSendClientOptions::Get().nPrivateSendDenomsGoal; } static int GetDenomsHardCap() { return CPrivateSendClientOptions::Get().nPrivateSendDenomsHardCap; } @@ -305,6 +308,7 @@ class CPrivateSendClientOptions CCriticalSection cs_ps_options; int nPrivateSendSessions; int nPrivateSendRounds; + int nPrivateSendRandomRounds; int nPrivateSendAmount; int nPrivateSendDenomsGoal; int nPrivateSendDenomsHardCap; @@ -313,6 +317,7 @@ class CPrivateSendClientOptions CPrivateSendClientOptions() : nPrivateSendRounds(DEFAULT_PRIVATESEND_ROUNDS), + nPrivateSendRandomRounds(PRIVATESEND_RANDOM_ROUNDS), nPrivateSendAmount(DEFAULT_PRIVATESEND_AMOUNT), nPrivateSendDenomsGoal(DEFAULT_PRIVATESEND_DENOMS_GOAL), nPrivateSendDenomsHardCap(DEFAULT_PRIVATESEND_DENOMS_HARDCAP), diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 86358eb2d2af..6a4d87c9e34e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2835,7 +2835,24 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const } else if(nCoinType == CoinType::ONLY_READY_TO_MIX) { if (!CPrivateSend::IsDenominatedAmount(pcoin->tx->vout[i].nValue)) continue; int nRounds = GetRealOutpointPrivateSendRounds(COutPoint(wtxid, i)); - found = nRounds < CPrivateSendClientOptions::GetRounds(); + + // Mix again if we don't have N rounds yet + if (nRounds < CPrivateSendClientOptions::GetRounds()) found = true; + // try to mix a "random" number of rounds more than minimum + // If we have already mixed N + MaxOffset rounds, don't mix again. + // Otherwise, we should mix again 50% of the time, this results in an exponential decay + // N rounds 50% N+1 25% N+2 12.5%... until we reach N + GetRandomRounds() rounds where we stop + else if (nRounds < CPrivateSendClientOptions::GetRounds() + CPrivateSendClientOptions::GetRandomRounds()) { + // This salt is needed to prevent an attacker from learning how many extra times this input was mixed + // based only on information in the blockchain + auto psMan = privateSendClientManagers.at(GetName()); + uint256 salt; + if (psMan) salt = psMan->nSalt; + if (Hash(wtxid.begin(), wtxid.end(), salt.begin(), salt.end()).GetCheapHash() % 2 == 0) { + found = true; + } + } + } else if(nCoinType == CoinType::ONLY_NONDENOMINATED) { if (CPrivateSend::IsCollateralAmount(pcoin->tx->vout[i].nValue)) continue; // do not use collateral amounts found = !CPrivateSend::IsDenominatedAmount(pcoin->tx->vout[i].nValue); From b9f2486f05e862b9720bd7b9039e1542cbda59f2 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 20 Aug 2020 14:21:26 +0300 Subject: [PATCH 04/13] Make PS salt a private member of CWallet, tweak the way it's initialized --- src/privatesend/privatesend-client.cpp | 5 ----- src/privatesend/privatesend-client.h | 2 -- src/wallet/wallet.cpp | 28 ++++++++++++-------------- src/wallet/wallet.h | 20 +++++++++--------- src/wallet/walletdb.cpp | 2 +- src/wallet/walletdb.h | 2 +- 6 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp index dfcc289bdd3a..e0aac8c470a7 100644 --- a/src/privatesend/privatesend-client.cpp +++ b/src/privatesend/privatesend-client.cpp @@ -244,11 +244,6 @@ bool CPrivateSendClientManager::StartMixing(CWallet* pwallet) { } assert(pwallet != nullptr); mixingWallet = pwallet; - nSalt = mixingWallet->GetPrivateSendSalt(); - if (nSalt.IsNull()) { - nSalt = GetRandHash(); - pwallet->WritePrivateSendSalt(nSalt); - } return true; } diff --git a/src/privatesend/privatesend-client.h b/src/privatesend/privatesend-client.h index 0835f67af1d0..22126ee96c74 100644 --- a/src/privatesend/privatesend-client.h +++ b/src/privatesend/privatesend-client.h @@ -230,8 +230,6 @@ class CPrivateSendClientManager int nCachedNumBlocks; // builtin support for automatic backups bool fCreateAutoBackups; - // Pulled from wallet DB ("ps_salt") and used when mixing a random number of rounds - uint256 nSalt; CPrivateSendClientManager() : vecMasternodesUsed(), diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6a4d87c9e34e..de0bfd2e67ff 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2843,12 +2843,7 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const // Otherwise, we should mix again 50% of the time, this results in an exponential decay // N rounds 50% N+1 25% N+2 12.5%... until we reach N + GetRandomRounds() rounds where we stop else if (nRounds < CPrivateSendClientOptions::GetRounds() + CPrivateSendClientOptions::GetRandomRounds()) { - // This salt is needed to prevent an attacker from learning how many extra times this input was mixed - // based only on information in the blockchain - auto psMan = privateSendClientManagers.at(GetName()); - uint256 salt; - if (psMan) salt = psMan->nSalt; - if (Hash(wtxid.begin(), wtxid.end(), salt.begin(), salt.end()).GetCheapHash() % 2 == 0) { + if (Hash(wtxid.begin(), wtxid.end(), nPrivateSendSalt.begin(), nPrivateSendSalt.end()).GetCheapHash() % 2 == 0) { found = true; } } @@ -2968,18 +2963,19 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } -const uint256 CWallet::GetPrivateSendSalt() +void CWallet::GetPrivateSendSalt() { - uint256 ps_salt; - WalletBatch batch(*database); - batch.ReadPrivateSendSalt(ps_salt); - return ps_salt; -} + // Avoid fetching it multiple times + assert(nPrivateSendSalt.IsNull()); -bool CWallet::WritePrivateSendSalt(uint256 &salt) -{ WalletBatch batch(*database); - return batch.WritePrivateSendSalt(salt); + batch.ReadPrivateSendSalt(nPrivateSendSalt); + + while (nPrivateSendSalt.IsNull()) { + // We never generated/saved it + nPrivateSendSalt = GetRandHash(); + batch.WritePrivateSendSalt(nPrivateSendSalt); + } } static void ApproximateBestSubset(const std::vector& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, @@ -4225,6 +4221,8 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) } } + GetPrivateSendSalt(); + if (nLoadWalletRet != DBErrors::LOAD_OK) return nLoadWalletRet; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a0c6be9d38c2..645880dc2a72 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -802,6 +802,16 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface */ const CBlockIndex* m_last_block_processed = nullptr; + // Pulled from wallet DB ("ps_salt") and used when mixing a random number of rounds. + // This salt is needed to prevent an attacker from learning how many extra times + // the input was mixed based only on information in the blockchain. + uint256 nPrivateSendSalt; + + /** + * Fetches PrivateSend salt from database or generates and saves a new one if no salt was found in the db + */ + void GetPrivateSendSalt(); + public: /* * Main wallet lock. @@ -1221,16 +1231,6 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface void NotifyTransactionLock(const CTransaction &tx, const llmq::CInstantSendLock& islock) override; void NotifyChainLock(const CBlockIndex* pindexChainLock, const llmq::CChainLockSig& clsig) override; - /** - * Fetches PrivateSend salt from database - */ - const uint256 GetPrivateSendSalt(); - - /** - * Writes PrivateSend salt to database - */ - bool WritePrivateSendSalt(uint256& salt); - /** * Blocks until the wallet state is up-to-date to /at least/ the current * chain at the time this function is entered diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 7f8dac75e5e4..2bc01ac01c98 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -173,7 +173,7 @@ bool WalletBatch::ReadPrivateSendSalt(uint256& salt) return m_batch.Read(std::string("ps_salt"), salt); } -bool WalletBatch::WritePrivateSendSalt(uint256& salt) +bool WalletBatch::WritePrivateSendSalt(const uint256& salt) { return WriteIC(std::string("ps_salt"), salt); } diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 3f9d55330293..3477c1eb1b68 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -160,8 +160,8 @@ class WalletBatch bool ReadAccount(const std::string& strAccount, CAccount& account); bool WriteAccount(const std::string& strAccount, const CAccount& account); - bool WritePrivateSendSalt(uint256& salt); bool ReadPrivateSendSalt(uint256& salt); + bool WritePrivateSendSalt(const uint256& salt); /// Write destination data key,value tuple to database bool WriteDestData(const std::string &address, const std::string &key, const std::string &value); From 5b643d04cda7a729540febefd4d7bfe0eeca7d75 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 20 Aug 2020 14:28:19 +0300 Subject: [PATCH 05/13] Introduce `CWallet::IsFullyMixed` and use it everywhere instead of comparing rounds directly to ensure consistency between coin selection logic, balance calculations and gui --- src/qt/coincontroldialog.cpp | 4 ++-- src/qt/walletmodel.cpp | 5 ++++ src/qt/walletmodel.h | 1 + src/wallet/wallet.cpp | 45 +++++++++++++++++++----------------- src/wallet/wallet.h | 1 + 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 9f1dee92ac74..b4bb19efe97e 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -706,9 +706,8 @@ void CoinControlDialog::updateView() int nChildren = 0; for (const COutput& out : coins.second) { COutPoint outpoint = COutPoint(out.tx->tx->GetHash(), out.i); - int nRounds = model->getRealOutpointPrivateSendRounds(outpoint); - if ((coinControl()->IsUsingPrivateSend() && nRounds >= CPrivateSendClientOptions::GetRounds()) || !(coinControl()->IsUsingPrivateSend())) { + if ((coinControl()->IsUsingPrivateSend() && model->isFullyMixed(outpoint)) || !(coinControl()->IsUsingPrivateSend())) { nSum += out.tx->tx->vout[out.i].nValue; nChildren++; @@ -759,6 +758,7 @@ void CoinControlDialog::updateView() itemOutput->setData(COLUMN_DATE, Qt::UserRole, QVariant((qlonglong)out.tx->GetTxTime())); // PrivateSend rounds + int nRounds = model->getRealOutpointPrivateSendRounds(outpoint); if (nRounds >= 0 || LogAcceptCategory(BCLog::PRIVATESEND)) { itemOutput->setText(COLUMN_PRIVATESEND_ROUNDS, QString::number(nRounds)); } else { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index b2f330dcf8f9..728cb239a7da 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -229,6 +229,11 @@ int WalletModel::getRealOutpointPrivateSendRounds(const COutPoint& outpoint) con return wallet->GetRealOutpointPrivateSendRounds(outpoint); } +bool WalletModel::isFullyMixed(const COutPoint& outpoint) const +{ + return wallet->IsFullyMixed(outpoint); +} + void WalletModel::updateAddressBook(const QString &address, const QString &label, bool isMine, const QString &purpose, int status) { diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index f18814d20e70..28cbc6c0bf56 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -232,6 +232,7 @@ class WalletModel : public QObject int getNumISLocks() const; int getRealOutpointPrivateSendRounds(const COutPoint& outpoint) const; + bool isFullyMixed(const COutPoint& outpoint) const; QString getWalletName() const; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index de0bfd2e67ff..578c16046b9a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1681,6 +1681,25 @@ bool CWallet::IsDenominated(const COutPoint& outpoint) const return CPrivateSend::IsDenominatedAmount(it->second.tx->vout[outpoint.n].nValue); } +bool CWallet::IsFullyMixed(const COutPoint& outpoint) const +{ + // Mix again if we don't have N rounds yet + int nRounds = GetRealOutpointPrivateSendRounds(outpoint); + if (nRounds < CPrivateSendClientOptions::GetRounds()) return false; + + // Try to mix a "random" number of rounds more than minimum. + // If we have already mixed N + MaxOffset rounds, don't mix again. + // Otherwise, we should mix again 50% of the time, this results in an exponential decay + // N rounds 50% N+1 25% N+2 12.5%... until we reach N + GetRandomRounds() rounds where we stop. + if (nRounds < CPrivateSendClientOptions::GetRounds() + CPrivateSendClientOptions::GetRandomRounds()) { + if (Hash(outpoint.hash.begin(), outpoint.hash.end(), nPrivateSendSalt.begin(), nPrivateSendSalt.end()).GetCheapHash() % 2 == 0) { + return false; + } + } + + return true; +} + isminetype CWallet::IsMine(const CTxOut& txout) const { return ::IsMine(*this, txout.scriptPubKey); @@ -2363,8 +2382,7 @@ CAmount CWalletTx::GetAnonymizedCredit(bool fUseCache) const if (pwallet->IsSpent(hashTx, i) || !CPrivateSend::IsDenominatedAmount(txout.nValue)) continue; - const int nRounds = pwallet->GetCappedOutpointPrivateSendRounds(outpoint); - if (nRounds >= CPrivateSendClientOptions::GetRounds()) { + if (pwallet->IsFullyMixed(outpoint)) { nCredit += pwallet->GetCredit(txout, ISMINE_SPENDABLE); if (!MoneyRange(nCredit)) throw std::runtime_error(std::string(__func__) + ": value out of range"); @@ -2830,24 +2848,10 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const bool found = false; if (nCoinType == CoinType::ONLY_FULLY_MIXED) { if (!CPrivateSend::IsDenominatedAmount(pcoin->tx->vout[i].nValue)) continue; - int nRounds = GetRealOutpointPrivateSendRounds(COutPoint(wtxid, i)); - found = nRounds >= CPrivateSendClientOptions::GetRounds(); + found = IsFullyMixed(COutPoint(wtxid, i)); } else if(nCoinType == CoinType::ONLY_READY_TO_MIX) { if (!CPrivateSend::IsDenominatedAmount(pcoin->tx->vout[i].nValue)) continue; - int nRounds = GetRealOutpointPrivateSendRounds(COutPoint(wtxid, i)); - - // Mix again if we don't have N rounds yet - if (nRounds < CPrivateSendClientOptions::GetRounds()) found = true; - // try to mix a "random" number of rounds more than minimum - // If we have already mixed N + MaxOffset rounds, don't mix again. - // Otherwise, we should mix again 50% of the time, this results in an exponential decay - // N rounds 50% N+1 25% N+2 12.5%... until we reach N + GetRandomRounds() rounds where we stop - else if (nRounds < CPrivateSendClientOptions::GetRounds() + CPrivateSendClientOptions::GetRandomRounds()) { - if (Hash(wtxid.begin(), wtxid.end(), nPrivateSendSalt.begin(), nPrivateSendSalt.end()).GetCheapHash() % 2 == 0) { - found = true; - } - } - + found = !IsFullyMixed(COutPoint(wtxid, i)); } else if(nCoinType == CoinType::ONLY_NONDENOMINATED) { if (CPrivateSend::IsCollateralAmount(pcoin->tx->vout[i].nValue)) continue; // do not use collateral amounts found = !CPrivateSend::IsDenominatedAmount(pcoin->tx->vout[i].nValue); @@ -3243,8 +3247,7 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm if (nCoinType == CoinType::ONLY_FULLY_MIXED) { // Make sure to include mixed preset inputs only, // even if some non-mixed inputs were manually selected via CoinControl - int nRounds = GetRealOutpointPrivateSendRounds(outpoint); - if (nRounds < CPrivateSendClientOptions::GetRounds()) continue; + if (!IsFullyMixed(outpoint)) continue; } nValueFromPresetInputs += pcoin->tx->vout[outpoint.n].nValue; setPresetCoins.insert(CInputCoin(pcoin, outpoint.n)); @@ -3447,7 +3450,7 @@ bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTa // otherwise they will just lead to higher fee / lower priority if(wtx.tx->vout[i].nValue <= nSmallestDenom/10) continue; // ignore mixed - if (GetCappedOutpointPrivateSendRounds(COutPoint(outpoint.hash, i)) >= CPrivateSendClientOptions::GetRounds()) continue; + if (IsFullyMixed(COutPoint(outpoint.hash, i))) continue; } if (itTallyItem == mapTally.end()) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 645880dc2a72..5c4312672051 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -922,6 +922,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface int GetCappedOutpointPrivateSendRounds(const COutPoint& outpoint) const; bool IsDenominated(const COutPoint& outpoint) const; + bool IsFullyMixed(const COutPoint& outpoint) const; bool IsSpent(const uint256& hash, unsigned int n) const; From c5d1b9f413537bef1e85284ba745c0aa3ba5fbda Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 20 Aug 2020 14:29:33 +0300 Subject: [PATCH 06/13] Tweak `GetRealOutpointPrivateSendRounds` to respect random rounds --- src/wallet/wallet.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 578c16046b9a..d05b2082e0ab 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1583,9 +1583,11 @@ int CWallet::GetRealOutpointPrivateSendRounds(const COutPoint& outpoint, int nRo { LOCK(cs_wallet); - if (nRounds >= MAX_PRIVATESEND_ROUNDS) { - // there can only be MAX_PRIVATESEND_ROUNDS rounds max - return MAX_PRIVATESEND_ROUNDS - 1; + const int nRoundsMax = MAX_PRIVATESEND_ROUNDS + CPrivateSendClientOptions::GetRandomRounds(); + + if (nRounds >= nRoundsMax) { + // there can only be nRoundsMax rounds max + return nRoundsMax - 1; } auto pair = mapOutpointRoundsCache.emplace(outpoint, -10); @@ -1651,7 +1653,7 @@ int CWallet::GetRealOutpointPrivateSendRounds(const COutPoint& outpoint, int nRo } } *nRoundsRef = fDenomFound - ? (nShortest >= MAX_PRIVATESEND_ROUNDS - 1 ? MAX_PRIVATESEND_ROUNDS : nShortest + 1) // good, we a +1 to the shortest one but only MAX_PRIVATESEND_ROUNDS rounds max allowed + ? (nShortest >= nRoundsMax - 1 ? nRoundsMax : nShortest + 1) // good, we a +1 to the shortest one but only nRoundsMax rounds max allowed : 0; // too bad, we are the fist one in that chain LogPrint(BCLog::PRIVATESEND, "%s UPDATED %-70s %3d\n", __func__, outpoint.ToStringShort(), *nRoundsRef); return *nRoundsRef; From 969e3033a746a0f034fdc48c4ecde60e4ea32783 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 21 Aug 2020 11:45:21 +0300 Subject: [PATCH 07/13] Tweak IsFullyMixed to make decision on a per-outpoint basis instead of a per-tx one --- src/wallet/wallet.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d05b2082e0ab..6e71ccaf3c09 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1693,8 +1693,9 @@ bool CWallet::IsFullyMixed(const COutPoint& outpoint) const // If we have already mixed N + MaxOffset rounds, don't mix again. // Otherwise, we should mix again 50% of the time, this results in an exponential decay // N rounds 50% N+1 25% N+2 12.5%... until we reach N + GetRandomRounds() rounds where we stop. + uint256 outpointHash = SerializeHash(outpoint); if (nRounds < CPrivateSendClientOptions::GetRounds() + CPrivateSendClientOptions::GetRandomRounds()) { - if (Hash(outpoint.hash.begin(), outpoint.hash.end(), nPrivateSendSalt.begin(), nPrivateSendSalt.end()).GetCheapHash() % 2 == 0) { + if (Hash(outpointHash.begin(), outpointHash.end(), nPrivateSendSalt.begin(), nPrivateSendSalt.end()).GetCheapHash() % 2 == 0) { return false; } } From a8e6270ca6a45da9f4e4226cb69bb5c04042fbfd Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 21 Aug 2020 15:08:03 -0500 Subject: [PATCH 08/13] make a comment doxygen Signed-off-by: pasta --- src/wallet/wallet.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5c4312672051..8c51b2339fae 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -802,9 +802,10 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface */ const CBlockIndex* m_last_block_processed = nullptr; - // Pulled from wallet DB ("ps_salt") and used when mixing a random number of rounds. - // This salt is needed to prevent an attacker from learning how many extra times - // the input was mixed based only on information in the blockchain. + /** Pulled from wallet DB ("ps_salt") and used when mixing a random number of rounds. + * This salt is needed to prevent an attacker from learning how many extra times + * the input was mixed based only on information in the blockchain. + */ uint256 nPrivateSendSalt; /** From 1fa2553f86566f249ad09428b468b78ca50b12f5 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 21 Aug 2020 15:11:01 -0500 Subject: [PATCH 09/13] Rename GetPrivateSendSalt InitPrivateSendSalt, since it is not a getter Signed-off-by: pasta --- src/wallet/wallet.cpp | 4 ++-- src/wallet/wallet.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6e71ccaf3c09..ac8c4af4518a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2970,7 +2970,7 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } -void CWallet::GetPrivateSendSalt() +void CWallet::InitPrivateSendSalt() { // Avoid fetching it multiple times assert(nPrivateSendSalt.IsNull()); @@ -4227,7 +4227,7 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) } } - GetPrivateSendSalt(); + InitPrivateSendSalt(); if (nLoadWalletRet != DBErrors::LOAD_OK) return nLoadWalletRet; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 8c51b2339fae..c94ff9b771f5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -811,7 +811,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface /** * Fetches PrivateSend salt from database or generates and saves a new one if no salt was found in the db */ - void GetPrivateSendSalt(); + void InitPrivateSendSalt(); public: /* From 3a4b11242e2aa0935defae1a71aff8c0c92e0ca0 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 21 Aug 2020 15:13:17 -0500 Subject: [PATCH 10/13] move the comment below GetRounds call Signed-off-by: pasta --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ac8c4af4518a..108cc13f62d1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1685,8 +1685,8 @@ bool CWallet::IsDenominated(const COutPoint& outpoint) const bool CWallet::IsFullyMixed(const COutPoint& outpoint) const { - // Mix again if we don't have N rounds yet int nRounds = GetRealOutpointPrivateSendRounds(outpoint); + // Mix again if we don't have N rounds yet if (nRounds < CPrivateSendClientOptions::GetRounds()) return false; // Try to mix a "random" number of rounds more than minimum. From e64de32c0b27371b8b256c8b3d5323408eee546e Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 21 Aug 2020 15:17:38 -0500 Subject: [PATCH 11/13] don't use GetCappedOutpointPrivateSendRounds when printing to RPC Signed-off-by: pasta --- src/wallet/rpcwallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index db4e100dd9e7..89be10b19857 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3167,7 +3167,7 @@ UniValue listunspent(const JSONRPCRequest& request) entry.pushKV("spendable", out.fSpendable); entry.pushKV("solvable", out.fSolvable); entry.pushKV("safe", out.fSafe); - entry.pushKV("ps_rounds", pwallet->GetCappedOutpointPrivateSendRounds(COutPoint(out.tx->GetHash(), out.i))); + entry.pushKV("ps_rounds", pwallet->GetRealOutpointPrivateSendRounds(COutPoint(out.tx->GetHash(), out.i))); results.push_back(entry); } From 8d2de22f9d0fc9d6dc05b3a2262119f688352fcb Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 26 Aug 2020 22:36:21 +0300 Subject: [PATCH 12/13] Simplify hashing in IsFullyMixed Uses just 1 sha256 instead of 3 (1 in SerializeHash + 2 in Hash) --- src/wallet/wallet.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 108cc13f62d1..4ee537219fbf 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1693,9 +1693,12 @@ bool CWallet::IsFullyMixed(const COutPoint& outpoint) const // If we have already mixed N + MaxOffset rounds, don't mix again. // Otherwise, we should mix again 50% of the time, this results in an exponential decay // N rounds 50% N+1 25% N+2 12.5%... until we reach N + GetRandomRounds() rounds where we stop. - uint256 outpointHash = SerializeHash(outpoint); if (nRounds < CPrivateSendClientOptions::GetRounds() + CPrivateSendClientOptions::GetRandomRounds()) { - if (Hash(outpointHash.begin(), outpointHash.end(), nPrivateSendSalt.begin(), nPrivateSendSalt.end()).GetCheapHash() % 2 == 0) { + CDataStream ss(SER_GETHASH, PROTOCOL_VERSION); + ss << outpoint << nPrivateSendSalt; + uint256 nHash; + CSHA256().Write((const unsigned char*)ss.data(), ss.size()).Finalize(nHash.begin()); + if (nHash.GetCheapHash() % 2 == 0) { return false; } } From 7d2d5de67654083b383b358fc243f07b86c3c2f4 Mon Sep 17 00:00:00 2001 From: pasta Date: Sun, 30 Aug 2020 00:42:43 -0400 Subject: [PATCH 13/13] undo comment change Signed-off-by: pasta --- src/privatesend/privatesend-client.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/privatesend/privatesend-client.h b/src/privatesend/privatesend-client.h index 22126ee96c74..74ac2d95b956 100644 --- a/src/privatesend/privatesend-client.h +++ b/src/privatesend/privatesend-client.h @@ -226,10 +226,8 @@ class CPrivateSendClientManager bool CheckAutomaticBackup(); public: - // used for the overview screen - int nCachedNumBlocks; - // builtin support for automatic backups - bool fCreateAutoBackups; + int nCachedNumBlocks; // used for the overview screen + bool fCreateAutoBackups; // builtin support for automatic backups CPrivateSendClientManager() : vecMasternodesUsed(),