diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index b1784b4d9b1..433759e086f 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -398,6 +398,17 @@ CAmount GetSelectionWaste(const std::set& inputs, CAmount change_cost, return waste; } +CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng) +{ + if (payment_value <= CHANGE_LOWER / 2) { + return CHANGE_LOWER; + } else { + // random value between 50ksat and min (payment_value * 2, 1milsat) + const auto upper_bound = std::min(payment_value * 2, CHANGE_UPPER); + return rng.randrange(upper_bound - CHANGE_LOWER) + CHANGE_LOWER; + } +} + void SelectionResult::ComputeAndSetWaste(CAmount change_cost) { m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective); diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 480c9664ca1..1da9f86cb5b 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -17,6 +17,14 @@ namespace wallet { static constexpr CAmount MIN_CHANGE{COIN / 100}; //! final minimum change amount after paying for fees static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2; +//! lower bound for randomly-chosen target change amount +static constexpr CAmount CHANGE_LOWER{50000}; +//! upper bound for randomly-chosen target change amount +static constexpr CAmount CHANGE_UPPER{1000000}; +// Ensure that any randomly generated change targets are less than or equal to before. +// Otherwise, tests may fail if funds are not enough to cover change. +static_assert(CHANGE_UPPER <= MIN_CHANGE); +static_assert(CHANGE_LOWER <= MIN_FINAL_CHANGE); /** A UTXO under consideration for use in funding a new transaction. */ class COutput @@ -99,6 +107,8 @@ struct CoinSelectionParams { CAmount m_min_change_target{MIN_CHANGE}; /** Cost of creating the change output. */ CAmount m_change_fee{0}; + /** The pre-determined minimum value to target when funding a change output. */ + CAmount m_change_target{0}; /** Cost of creating the change output + cost of spending the change output in the future. */ CAmount m_cost_of_change{0}; /** The targeted feerate of the transaction being built. */ @@ -223,6 +233,21 @@ struct OutputGroup */ [[nodiscard]] CAmount GetSelectionWaste(const std::set& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true); + +/** Chooose a random change target for each transaction to make it harder to fingerprint the Core + * wallet based on the change output values of transactions it creates. + * The random value is between 50ksat and min(2 * payment_value, 1milsat) + * When payment_value <= 25ksat, the value is just 50ksat. + * + * Making change amounts similar to the payment value may help disguise which output(s) are payments + * are which ones are change. Using double the payment value may increase the number of inputs + * needed (and thus be more expensive in fees), but breaks analysis techniques which assume the + * coins selected are just sufficient to cover the payment amount ("unnecessary input" heuristic). + * + * @param[in] payment_value Average payment value of the transaction output(s). + */ +[[nodiscard]] CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng); + struct SelectionResult { private: diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 228725646c6..b2839f2afed 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -19,6 +19,8 @@ #include #include +#include + using interfaces::FoundBlock; namespace wallet { @@ -395,9 +397,11 @@ std::optional AttemptSelection(const CWallet& wallet, const CAm results.push_back(*knapsack_result); } - // We include the minimum final change for SRD as we do want to avoid making really small change. - // KnapsackSolver does not need this because it includes MIN_CHANGE internally. - const CAmount srd_target = nTargetValue + coin_selection_params.m_change_fee + MIN_FINAL_CHANGE; + // Include change for SRD as we want to avoid making really small change if the selection just + // barely meets the target. Just use the lower bound change target instead of the randomly + // generated one, since SRD will result in a random change amount anyway; avoid making the + // target needlessly large. + const CAmount srd_target = nTargetValue + coin_selection_params.m_change_fee + CHANGE_LOWER; if (auto srd_result{SelectCoinsSRD(positive_groups, srd_target, coin_selection_params.rng_fast)}) { srd_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change); results.push_back(*srd_result); @@ -681,6 +685,7 @@ static bool CreateTransactionInternal( coin_selection_params.m_subtract_fee_outputs = true; } } + coin_selection_params.m_change_target = GenerateChangeTarget(std::floor(recipients_sum / vecSend.size()), rng_fast); // Create change script that will be used if we need change CScript scriptChange; diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index f6843d597de..3b23ee8e94a 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -442,7 +442,9 @@ def test_watchonly_psbt(self, peer_node, rbf_node, dest_address): self.generate(peer_node, 1) # Create single-input PSBT for transaction to be bumped - psbt = watcher.walletcreatefundedpsbt([], {dest_address: 0.0005}, 0, {"fee_rate": 1}, True)['psbt'] + # Ensure the payment amount + change can be fully funded using one of the 0.001BTC inputs. + psbt = watcher.walletcreatefundedpsbt([watcher.listunspent()[0]], {dest_address: 0.0005}, 0, + {"fee_rate": 1, "add_inputs": False}, True)['psbt'] psbt_signed = signer.walletprocesspsbt(psbt=psbt, sign=True, sighashtype="ALL", bip32derivs=True) psbt_final = watcher.finalizepsbt(psbt_signed["psbt"]) original_txid = watcher.sendrawtransaction(psbt_final["hex"])