mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2024-11-20 02:25:40 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#27227: wallet: 25806 follow-up

Browse Source 
475c20aa56 wallet: remove coin control arg from AutomaticCoinSelection (furszy)
8a5583131c wallet: remove unused methods (furszy)
8471967d7b wallet: GroupOutput, remove unneeded "spendable" check (furszy)
a9aa04183c wallet: OutputGroup, remove unused effective_feerate member (furszy)
99034b2b72 wallet: APS, don't create empty groups (furszy)
805f399b17 wallet: do not make two COutputs, use shared_ptr (furszy)

Pull request description:

  Few small findings post-#25806 and extra cleanups, nothing biggie.

ACKs for top commit:
  S3RK:
    Code review ACK 475c20aa56
  Xekyo:
    utACK 475c20aa56
  achow101:
    ACK 475c20aa56

Tree-SHA512: df45efebd6e2e4ecac619d6ecef794979c328a2d6ef532e25124d0dc1c72b55ccf13498f61fb65958b907bfba6a72ed569bf34eb5fbe35419632fe0406e78798
This commit is contained in:
Andrew Chow 2023-03-15 18:59:37 -04:00
commit 609c95d4a8
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
5 changed files with 27 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/wallet/coinselection.cpp
View File

@ -387,13 +387,6 @@ void OutputGroupTypeMap::Push(const OutputGroup& group, OutputType type, bool in
}
}
std::optional<Groups> OutputGroupTypeMap::Find(OutputType type)
{
auto it_by_type = groups_by_type.find(type);
if (it_by_type == groups_by_type.end()) return std::nullopt;
return it_by_type->second;
}
CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmount change_cost, CAmount target, bool use_effective_value)
{
// This function should not be called with empty inputs as that would mean the selection failed

10
src/wallet/coinselection.h
View File

@ -153,6 +153,11 @@ struct CoinSelectionParams {
* associated with the same address. This helps reduce privacy leaks resulting from address
* reuse. Dust outputs are not eligible to be added to output groups and thus not considered. */
bool m_avoid_partial_spends = false;
/**
* When true, allow unsafe coins to be selected during Coin Selection. This may spend unconfirmed outputs:
* 1) Received from other wallets, 2) replacing other txs, 3) that have been replaced.
*/
bool m_include_unsafe_inputs = false;
CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size,
CAmount min_change_target, CFeeRate effective_feerate,
@ -222,8 +227,6 @@ struct OutputGroup
CAmount effective_value{0};
/** The fee to spend these UTXOs at the effective feerate. */
CAmount fee{0};
/** The target feerate of the transaction we're trying to build. */
CFeeRate m_effective_feerate{0};
/** The fee to spend these UTXOs at the long term feerate. */
CAmount long_term_fee{0};
/** The feerate for spending a created change output eventually (i.e. not urgently, and thus at
@ -238,7 +241,6 @@ struct OutputGroup
OutputGroup() {}
OutputGroup(const CoinSelectionParams& params) :
m_effective_feerate(params.m_effective_feerate),
m_long_term_feerate(params.m_long_term_feerate),
m_subtract_fee_outputs(params.m_subtract_fee_outputs)
{}
@ -266,8 +268,6 @@ struct OutputGroupTypeMap
// Based on the insert flag; appends group to the 'mixed_group' and, if value > 0, to the 'positive_group'.
// This affects both; the groups filtered by type and the overall groups container.
void Push(const OutputGroup& group, OutputType type, bool insert_positive, bool insert_mixed);
// Retrieves 'Groups' filtered by type
std::optional<Groups> Find(OutputType type);
// Different output types count
size_t TypesCount() { return groups_by_type.size(); }
};

48
src/wallet/spend.cpp
View File

@ -415,9 +415,6 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
// Allowing partial spends means no grouping. Each COutput gets its own OutputGroup
for (const auto& [type, outputs] : coins.coins) {
for (const COutput& output : outputs) {
// Skip outputs we cannot spend
if (!output.spendable) continue;
// Get mempool info
size_t ancestors, descendants;
wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants);
@ -444,11 +441,10 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
// to the last OutputGroup in the vector for the scriptPubKey. When the last OutputGroup has
// OUTPUT_GROUP_MAX_ENTRIES COutputs, a new OutputGroup is added to the end of the vector.
typedef std::map<std::pair<CScript, OutputType>, std::vector<OutputGroup>> ScriptPubKeyToOutgroup;
const auto& group_outputs = [](
const COutput& output, OutputType type, size_t ancestors, size_t descendants,
ScriptPubKeyToOutgroup& groups_map, const CoinSelectionParams& coin_sel_params,
bool positive_only) {
std::vector<OutputGroup>& groups = groups_map[std::make_pair(output.txout.scriptPubKey,type)];
const auto& insert_output = [&](
const std::shared_ptr<COutput>& output, OutputType type, size_t ancestors, size_t descendants,
ScriptPubKeyToOutgroup& groups_map) {
std::vector<OutputGroup>& groups = groups_map[std::make_pair(output->txout.scriptPubKey,type)];
if (groups.size() == 0) {
// No OutputGroups for this scriptPubKey yet, add one
@ -467,25 +463,24 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
group = &groups.back();
}
// Filter for positive only before adding the output to group
if (!positive_only || output.GetEffectiveValue() > 0) {
group->Insert(std::make_shared<COutput>(output), ancestors, descendants);
}
group->Insert(output, ancestors, descendants);
};
ScriptPubKeyToOutgroup spk_to_groups_map;
ScriptPubKeyToOutgroup spk_to_positive_groups_map;
for (const auto& [type, outs] : coins.coins) {
for (const COutput& output : outs) {
// Skip outputs we cannot spend
if (!output.spendable) continue;
size_t ancestors, descendants;
wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants);
group_outputs(output, type, ancestors, descendants, spk_to_groups_map, coin_sel_params, /*positive_only=*/ false);
group_outputs(output, type, ancestors, descendants, spk_to_positive_groups_map,
coin_sel_params, /*positive_only=*/ true);
const auto& shared_output = std::make_shared<COutput>(output);
// Filter for positive only before adding the output
if (output.GetEffectiveValue() > 0) {
insert_output(shared_output, type, ancestors, descendants, spk_to_positive_groups_map);
}
// 'All' groups
insert_output(shared_output, type, ancestors, descendants, spk_to_groups_map);
}
}
@ -622,7 +617,7 @@ util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& av
}
// Start wallet Coin Selection procedure
auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_control, coin_selection_params);
auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_selection_params);
if (!op_selection_result) return op_selection_result;
// If needed, add preset inputs to the automatic coin selection result
@ -637,7 +632,7 @@ util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& av
return op_selection_result;
}
util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params)
util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CoinSelectionParams& coin_selection_params)
{
unsigned int limit_ancestor_count = 0;
unsigned int limit_descendant_count = 0;
@ -646,12 +641,10 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin
const size_t max_descendants = (size_t)std::max<int64_t>(1, limit_descendant_count);
const bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS);
// 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 && available_coins.Size() > OUTPUT_GROUP_MAX_ENTRIES) {
// Cases where we have 101+ 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
// Cases where we have 101+ 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
if (coin_selection_params.m_avoid_partial_spends && available_coins.Size() > OUTPUT_GROUP_MAX_ENTRIES) {
available_coins.Shuffle(coin_selection_params.rng_fast);
}
@ -678,7 +671,7 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin
ordered_filters.push_back({CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, /*include_partial=*/true)});
// Try with unsafe inputs if they are allowed. This may spend unconfirmed outputs
// received from other wallets.
if (coin_control.m_include_unsafe_inputs) {
if (coin_selection_params.m_include_unsafe_inputs) {
ordered_filters.push_back({CoinEligibilityFilter(/*conf_mine=*/0, /*conf_theirs*/0, max_ancestors-1, max_descendants-1, /*include_partial=*/true)});
}
// Try with unlimited ancestors/descendants. The transaction will still need to meet
@ -809,6 +802,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
CoinSelectionParams coin_selection_params{rng_fast}; // Parameters for coin selection, init with dummy
coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends;
coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs;
// Set the long term feerate estimate to the wallet's consolidate feerate
coin_selection_params.m_long_term_feerate = wallet.m_consolidate_feerate;

2
src/wallet/spend.h
View File

@ -191,7 +191,7 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
* or (2) an specific error message if there was something particularly wrong (e.g. a selection
* result that surpassed the tx max weight size).
*/
util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control,
util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue,
const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
/**

6
src/wallet/wallet.h
View File

@ -592,12 +592,6 @@ public:
bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const
{
std::vector<CTxOut> v_txouts(txouts.size());
std::copy(txouts.begin(), txouts.end(), v_txouts.begin());
return DummySignTx(txNew, v_txouts, coin_control);
}
bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const;
bool ImportScripts(const std::set<CScript> scripts, int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);