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

refactor: make OutputGroup::m_outputs field a vector of shared_ptr

Browse Source 
Initial steps towards sharing COutput instances across all possible
OutputGroups (instead of copying them time after time).
This commit is contained in:
furszy 2022-08-01 16:15:36 -03:00
parent d8e749bb84
commit 461f0821a2
No known key found for this signature in database
GPG Key ID: 5DD23CCC686AA623
7 changed files with 38 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/bench/coin_selection.cpp
View File

@ -91,7 +91,7 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup>
tx.vout[nInput].nValue = nValue;
COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 0, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ true, /*fees=*/ 0);
set.emplace_back();
set.back().Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0);
set.back().Insert(std::make_shared<COutput>(output), /*ancestors=*/ 0, /*descendants=*/ 0);
}
// Copied from src/wallet/test/coinselector_tests.cpp
static CAmount make_hard_case(int utxos, std::vector<OutputGroup>& utxo_pool)

27
src/wallet/coinselection.cpp
View File

@ -333,9 +333,9 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups,
******************************************************************************/
void OutputGroup::Insert(const COutput& output, size_t ancestors, size_t descendants) {
void OutputGroup::Insert(const std::shared_ptr<COutput>& output, size_t ancestors, size_t descendants) {
m_outputs.push_back(output);
COutput& coin = m_outputs.back();
auto& coin = *m_outputs.back();
fee += coin.GetFee();
@ -355,8 +355,8 @@ void OutputGroup::Insert(const COutput& output, size_t ancestors, size_t descend
// coin itself; thus, this value is counted as the max, not the sum
m_descendants = std::max(m_descendants, descendants);
if (output.input_bytes > 0) {
m_weight += output.input_bytes * WITNESS_SCALE_FACTOR;
if (output->input_bytes > 0) {
m_weight += output->input_bytes * WITNESS_SCALE_FACTOR;
}
}
@ -372,7 +372,7 @@ CAmount OutputGroup::GetSelectionAmount() const
return m_subtract_fee_outputs ? m_value : effective_value;
}
CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, CAmount target, bool use_effective_value)
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
assert(!inputs.empty());
@ -380,7 +380,8 @@ CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost,
// Always consider the cost of spending an input now vs in the future.
CAmount waste = 0;
CAmount selected_effective_value = 0;
for (const COutput& coin : inputs) {
for (const auto& coin_ptr : inputs) {
const COutput& coin = *coin_ptr;
waste += coin.GetFee() - coin.long_term_fee;
selected_effective_value += use_effective_value ? coin.GetEffectiveValue() : coin.txout.nValue;
}
@ -428,12 +429,12 @@ CAmount SelectionResult::GetWaste() const
CAmount SelectionResult::GetSelectedValue() const
{
return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin.txout.nValue; });
return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin->txout.nValue; });
}
CAmount SelectionResult::GetSelectedEffectiveValue() const
{
return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin.GetEffectiveValue(); });
return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin->GetEffectiveValue(); });
}
void SelectionResult::Clear()
@ -452,14 +453,14 @@ void SelectionResult::AddInput(const OutputGroup& group)
m_weight += group.m_weight;
}
void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs)
void SelectionResult::AddInputs(const std::set<std::shared_ptr<COutput>>& inputs, bool subtract_fee_outputs)
{
// As it can fail, combine inputs first
InsertInputs(inputs);
m_use_effective = !subtract_fee_outputs;
m_weight += std::accumulate(inputs.cbegin(), inputs.cend(), 0, [](int sum, const auto& coin) {
return sum + std::max(coin.input_bytes, 0) * WITNESS_SCALE_FACTOR;
return sum + std::max(coin->input_bytes, 0) * WITNESS_SCALE_FACTOR;
});
}
@ -477,14 +478,14 @@ void SelectionResult::Merge(const SelectionResult& other)
m_weight += other.m_weight;
}
const std::set<COutput>& SelectionResult::GetInputSet() const
const std::set<std::shared_ptr<COutput>>& SelectionResult::GetInputSet() const
{
return m_selected_inputs;
}
std::vector<COutput> SelectionResult::GetShuffledInputVector() const
std::vector<std::shared_ptr<COutput>> SelectionResult::GetShuffledInputVector() const
{
std::vector<COutput> coins(m_selected_inputs.begin(), m_selected_inputs.end());
std::vector<std::shared_ptr<COutput>> coins(m_selected_inputs.begin(), m_selected_inputs.end());
Shuffle(coins.begin(), coins.end(), FastRandomContext());
return coins;
}

14
src/wallet/coinselection.h
View File

@ -198,7 +198,7 @@ struct CoinEligibilityFilter
struct OutputGroup
{
/** The list of UTXOs contained in this output group. */
std::vector<COutput> m_outputs;
std::vector<std::shared_ptr<COutput>> m_outputs;
/** Whether the UTXOs were sent by the wallet to itself. This is relevant because we may want at
* least a certain number of confirmations on UTXOs received from outside wallets while trusting
* our own UTXOs more. */
@ -237,7 +237,7 @@ struct OutputGroup
m_subtract_fee_outputs(params.m_subtract_fee_outputs)
{}
void Insert(const COutput& output, size_t ancestors, size_t descendants);
void Insert(const std::shared_ptr<COutput>& output, size_t ancestors, size_t descendants);
bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const;
CAmount GetSelectionAmount() const;
};
@ -259,7 +259,7 @@ struct OutputGroup
* @param[in] use_effective_value Whether to use the input's effective value (when true) or the real value (when false).
* @return The waste
*/
[[nodiscard]] CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true);
[[nodiscard]] CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true);
/** Choose a random change target for each transaction to make it harder to fingerprint the Core
@ -292,7 +292,7 @@ struct SelectionResult
{
private:
/** Set of inputs selected by the algorithm to use in the transaction */
std::set<COutput> m_selected_inputs;
std::set<std::shared_ptr<COutput>> m_selected_inputs;
/** The target the algorithm selected for. Equal to the recipient amount plus non-input fees */
CAmount m_target;
/** The algorithm used to produce this result */
@ -329,7 +329,7 @@ public:
void Clear();
void AddInput(const OutputGroup& group);
void AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs);
void AddInputs(const std::set<std::shared_ptr<COutput>>& inputs, bool subtract_fee_outputs);
/** Calculates and stores the waste for this selection via GetSelectionWaste */
void ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee);
@ -344,9 +344,9 @@ public:
void Merge(const SelectionResult& other);
/** Get m_selected_inputs */
const std::set<COutput>& GetInputSet() const;
const std::set<std::shared_ptr<COutput>>& GetInputSet() const;
/** Get the vector of COutputs that will be used to fill in a CTransaction's vin */
std::vector<COutput> GetShuffledInputVector() const;
std::vector<std::shared_ptr<COutput>> GetShuffledInputVector() const;
bool operator<(SelectionResult other) const;

8
src/wallet/spend.cpp
View File

@ -421,7 +421,7 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
if (!positive_only || output.GetEffectiveValue() > 0) {
// Make an OutputGroup containing just this output
OutputGroup group{coin_sel_params};
group.Insert(output, ancestors, descendants);
group.Insert(std::make_shared<COutput>(output), ancestors, descendants);
if (group.EligibleForSpending(filter)) groups_out.push_back(group);
}
@ -465,7 +465,7 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
// Filter for positive only before adding the output to group
if (!positive_only || output.GetEffectiveValue() > 0) {
group->Insert(output, ancestors, descendants);
group->Insert(std::make_shared<COutput>(output), ancestors, descendants);
}
}
@ -936,7 +936,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
}
// Shuffle selected coins and fill in final vin
std::vector<COutput> selected_coins = result.GetShuffledInputVector();
std::vector<std::shared_ptr<COutput>> selected_coins = result.GetShuffledInputVector();
// The sequence number is set to non-maxint so that DiscourageFeeSniping
// works.
@ -948,7 +948,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// behavior."
const uint32_t nSequence{coin_control.m_signal_bip125_rbf.value_or(wallet.m_signal_rbf) ? MAX_BIP125_RBF_SEQUENCE : CTxIn::MAX_SEQUENCE_NONFINAL};
for (const auto& coin : selected_coins) {
txNew.vin.push_back(CTxIn(coin.outpoint, CScript(), nSequence));
txNew.vin.push_back(CTxIn(coin->outpoint, CScript(), nSequence));
}
DiscourageFeeSniping(txNew, rng_fast, wallet.chain(), wallet.GetLastBlockHash(), wallet.GetLastBlockHeight());

4
src/wallet/spend.h
View File

@ -148,7 +148,7 @@ util::Result<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const
// User manually selected inputs that must be part of the transaction
struct PreSelectedInputs
{
std::set<COutput> coins;
std::set<std::shared_ptr<COutput>> coins;
// If subtract fee from outputs is disabled, the 'total_amount'
// will be the sum of each output effective value
// instead of the sum of the outputs amount
@ -161,7 +161,7 @@ struct PreSelectedInputs
} else {
total_amount += output.GetEffectiveValue();
}
coins.insert(output);
coins.insert(std::make_shared<COutput>(output));
}
};

18
src/wallet/test/coinselector_tests.cpp
View File

@ -29,7 +29,7 @@ BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup)
// we repeat those tests this many times and only complain if all iterations of the test fail
#define RANDOM_REPEATS 5
typedef std::set<COutput> CoinSet;
typedef std::set<std::shared_ptr<COutput>> CoinSet;
static const CoinEligibilityFilter filter_standard(1, 6, 0);
static const CoinEligibilityFilter filter_confirmed(1, 1, 0);
@ -53,7 +53,7 @@ static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result)
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, /*fees=*/ 0);
OutputGroup group;
group.Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0);
group.Insert(std::make_shared<COutput>(output), /*ancestors=*/ 0, /*descendants=*/ 0);
result.AddInput(group);
}
@ -65,7 +65,7 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fe
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
COutput coin(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ 148, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, fee);
coin.long_term_fee = long_term_fee;
set.insert(coin);
set.insert(std::make_shared<COutput>(coin));
}
static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmount& nValue, CFeeRate feerate = CFeeRate(0), int nAge = 6*24, bool fIsFromMe = false, int nInput =0, bool spendable = false)
@ -94,10 +94,10 @@ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b)
std::vector<CAmount> a_amts;
std::vector<CAmount> b_amts;
for (const auto& coin : a.GetInputSet()) {
a_amts.push_back(coin.txout.nValue);
a_amts.push_back(coin->txout.nValue);
}
for (const auto& coin : b.GetInputSet()) {
b_amts.push_back(coin.txout.nValue);
b_amts.push_back(coin->txout.nValue);
}
std::sort(a_amts.begin(), a_amts.end());
std::sort(b_amts.begin(), b_amts.end());
@ -110,8 +110,8 @@ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b)
static bool EqualResult(const SelectionResult& a, const SelectionResult& b)
{
std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin(),
[](const COutput& a, const COutput& b) {
return a.outpoint == b.outpoint;
[](const std::shared_ptr<COutput>& a, const std::shared_ptr<COutput>& b) {
return a->outpoint == b->outpoint;
});
return ret.first == a.GetInputSet().end() && ret.second == b.GetInputSet().end();
}
@ -134,7 +134,7 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& availabl
static_groups.clear();
for (auto& coin : available_coins) {
static_groups.emplace_back();
static_groups.back().Insert(coin, /*ancestors=*/ 0, /*descendants=*/ 0);
static_groups.back().Insert(std::make_shared<COutput>(coin), /*ancestors=*/ 0, /*descendants=*/ 0);
}
return static_groups;
}
@ -943,7 +943,7 @@ static util::Result<SelectionResult> select_coins(const CAmount& target, const C
static bool has_coin(const CoinSet& set, CAmount amount)
{
return std::any_of(set.begin(), set.end(), [&](const auto& coin) { return coin.GetEffectiveValue() == amount; });
return std::any_of(set.begin(), set.end(), [&](const auto& coin) { return coin->GetEffectiveValue() == amount; });
}
BOOST_AUTO_TEST_CASE(check_max_weight)

2
src/wallet/test/fuzz/coinselection.cpp
View File

@ -30,7 +30,7 @@ static void GroupCoins(FuzzedDataProvider& fuzzed_data_provider, const std::vect
bool valid_outputgroup{false};
for (auto& coin : coins) {
if (!positive_only || (positive_only && coin.GetEffectiveValue() > 0)) {
output_group.Insert(coin, /*ancestors=*/0, /*descendants=*/0);
output_group.Insert(std::make_shared<COutput>(coin), /*ancestors=*/0, /*descendants=*/0);
}
// If positive_only was specified, nothing was inserted, leading to an empty output group
// that would be invalid for the BnB algorithm