Skip to content

Commit

Permalink
Merge #13808: wallet: shuffle coins before grouping, where warranted
Browse files Browse the repository at this point in the history
18f690e wallet: shuffle coins before grouping, where warranted (Karl-Johan Alm)

Pull request description:

  Coins are randomly shuffled in coin selection to avoid unintentional privacy leaks regarding the user's coin set. For the case where a user has a lot of coins with the same destination, these will be grouped into groups of 10 *before* the shuffling.

  It is unclear whether this has any implications at all, but this PR plugs the potential issue, if there ever is one, by shuffling the coins before they are grouped.

  Issue brought up in bitcoin/bitcoin#12257 (comment)

Tree-SHA512: fb50ed4b5fc03ab4853d45b76e1c64476ad5bcd797497179bc37b9262885c974ed6811159fd8e581f1461b6cc6d0a66146f4b70a2777c0f5e818d1322e0edb89
  • Loading branch information
laanwj committed Aug 13, 2018
2 parents b0d3e9b 18f690e commit 13d51a2
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 35,8 @@

#include <boost/algorithm/string/replace.hpp>

static const size_t OUTPUT_GROUP_MAX_ENTRIES = 10;

static CCriticalSection cs_wallets;
static std::vector<std::shared_ptr<CWallet>> vpwallets GUARDED_BY(cs_wallets);

Expand Down Expand Up @@ -2525,6 2527,12 @@ bool CWallet::SelectCoins(const std::vector<COutput>& 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<OutputGroup> groups = GroupOutputs(vCoins, !coin_control.m_avoid_partial_spends);

size_t max_ancestors = (size_t)std::max<int64_t>(1, gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT));
Expand Down Expand Up @@ -4444,7 4452,7 @@ std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& 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);
}
Expand Down

0 comments on commit 13d51a2

Please sign in to comment.