From 18f690ec2f7eb1b4aa51825bfed0cbfdadc93ac7 Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Tue, 31 Jul 2018 01:50:43 +0900 Subject: [PATCH] wallet: shuffle coins before grouping, where warranted Issue brought up in https://github.com/bitcoin/bitcoin/pull/12257\#discussion_r204554549 --- src/wallet/wallet.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 597d108ae..73d2b205c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -35,6 +35,8 @@ #include +static const size_t OUTPUT_GROUP_MAX_ENTRIES = 10; + static CCriticalSection cs_wallets; static std::vector> vpwallets GUARDED_BY(cs_wallets); @@ -2525,6 +2527,12 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm // form groups from remaining coins; note that preset coins will not // automatically have their associated (same address) coins included + if (coin_control.m_avoid_partial_spends && vCoins.size() > OUTPUT_GROUP_MAX_ENTRIES) { + // Cases where we have 11+ outputs all pointing to the same destination may result in + // privacy leaks as they will potentially be deterministically sorted. We solve that by + // explicitly shuffling the outputs before processing + std::shuffle(vCoins.begin(), vCoins.end(), FastRandomContext()); + } std::vector groups = GroupOutputs(vCoins, !coin_control.m_avoid_partial_spends); size_t max_ancestors = (size_t)std::max(1, gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT)); @@ -4444,7 +4452,7 @@ std::vector CWallet::GroupOutputs(const std::vector& outpu // Limit output groups to no more than 10 entries, to protect // against inadvertently creating a too-large transaction // when using -avoidpartialspends - if (gmap[dst].m_outputs.size() >= 10) { + if (gmap[dst].m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) { groups.push_back(gmap[dst]); gmap.erase(dst); }