diff options
| author | Andrew Chow <achow101-github@achow101.com> | 2022-08-16 20:00:15 -0400 |
|---|---|---|
| committer | Andrew Chow <achow101-github@achow101.com> | 2022-08-16 20:00:19 -0400 |
| commit | 64f7a1940d738a9acd207883af1dfda3e00f4a21 (patch) | |
| tree | dd70f6f0d319a616cec9ed66065df44f7ccc085e /src | |
| parent | c336f813b3cd1d222d4b37cce3a6fbd855c5f4ad (diff) | |
| parent | 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b (diff) | |
Merge bitcoin/bitcoin#25734: wallet, refactor: #24584 follow-ups
8cd21bb2799d37ed00dc9d0490bb5f5f1375932b refactor: improve readability for AttemptSelection (josibake)
f47ff717611182da27461e29b3c23933eb22fbce test: only run test for descriptor wallets (josibake)
0760ce0b9e646b6c86f4cc890c6ab78103a242ab test: add missing BOOST_ASSERT (josibake)
db09aec9378c5e8cc49c866fa50bfcb6c567d66c wallet: switch to new shuffle, erase, push_back (josibake)
b6b50b0f2b055d81c5d4ff9e21dd88cdc9a88ccb scripted-diff: Uppercase function names (josibake)
3f27a2adce12c6b0e7b43ba7c024331657bcf335 refactor: add new helper methods (josibake)
f5649db9d5e984ba7f376ccfd5b0a627f5c42402 refactor: add UNKNOWN OutputType (josibake)
Pull request description:
This PR is to address follow-ups for #24584, specifically:
* Remove redundant, hard-to-read code by adding a new `OutputType` and adding shuffle, erase, and push_back methods for `CoinsResult`
* Add missing `BOOST_ASSERT` to unit test
* Ensure functional test only runs if using descriptor wallets
* Improve readability of `AttemptSelection` by removing triple-nested if statement
Note for reviewers: commit `refactor: add new helper methods` should throw an "unused function warning"; the function is used in the next commit. Also, commit `wallet: switch to new shuffle, erase, push_back` will fail to compile, but this is fixed in the next commit with a scripted-diff. the commits are separate like this (code change then scripted-diff) to improve legibility.
ACKs for top commit:
achow101:
ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b
aureleoules:
ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b.
LarryRuane:
Concept, code review ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b
furszy:
utACK 8cd21bb2. Left a small, non-blocking, comment.
Tree-SHA512: a1bbc5962833e3df4f01a4895d8bd748cc4c608c3f296fd94e8afd8797b8d2e94e7bd44d598bd76fa5c9f5536864f396fcd097348fa0bb190a49a86b0917d60e
Diffstat (limited to 'src')
| -rw-r--r-- | src/bench/coin_selection.cpp | 2 | ||||
| -rw-r--r-- | src/outputtype.cpp | 10 | ||||
| -rw-r--r-- | src/outputtype.h | 2 | ||||
| -rw-r--r-- | src/wallet/rpc/coins.cpp | 2 | ||||
| -rw-r--r-- | src/wallet/rpc/spend.cpp | 2 | ||||
| -rw-r--r-- | src/wallet/scriptpubkeyman.cpp | 5 | ||||
| -rw-r--r-- | src/wallet/spend.cpp | 167 | ||||
| -rw-r--r-- | src/wallet/spend.h | 22 | ||||
| -rw-r--r-- | src/wallet/test/availablecoins_tests.cpp | 13 | ||||
| -rw-r--r-- | src/wallet/test/coinselector_tests.cpp | 112 | ||||
| -rw-r--r-- | src/wallet/test/wallet_tests.cpp | 4 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 1 |
12 files changed, 179 insertions, 163 deletions
diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index a80ec3703c..6ada28115e 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -59,7 +59,7 @@ static void CoinSelection(benchmark::Bench& bench) wallet::CoinsResult available_coins; for (const auto& wtx : wtxs) { const auto txout = wtx->tx->vout.at(0); - available_coins.bech32.emplace_back(COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0); + available_coins.coins[OutputType::BECH32].emplace_back(COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0); } const CoinEligibilityFilter filter_standard(1, 6, 0); diff --git a/src/outputtype.cpp b/src/outputtype.cpp index 5be9942d7d..9ab2902256 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -20,6 +20,7 @@ static const std::string OUTPUT_TYPE_STRING_LEGACY = "legacy"; static const std::string OUTPUT_TYPE_STRING_P2SH_SEGWIT = "p2sh-segwit"; static const std::string OUTPUT_TYPE_STRING_BECH32 = "bech32"; static const std::string OUTPUT_TYPE_STRING_BECH32M = "bech32m"; +static const std::string OUTPUT_TYPE_STRING_UNKNOWN = "unknown"; std::optional<OutputType> ParseOutputType(const std::string& type) { @@ -31,6 +32,8 @@ std::optional<OutputType> ParseOutputType(const std::string& type) return OutputType::BECH32; } else if (type == OUTPUT_TYPE_STRING_BECH32M) { return OutputType::BECH32M; + } else if (type == OUTPUT_TYPE_STRING_UNKNOWN) { + return OutputType::UNKNOWN; } return std::nullopt; } @@ -42,6 +45,7 @@ const std::string& FormatOutputType(OutputType type) case OutputType::P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT; case OutputType::BECH32: return OUTPUT_TYPE_STRING_BECH32; case OutputType::BECH32M: return OUTPUT_TYPE_STRING_BECH32M; + case OutputType::UNKNOWN: return OUTPUT_TYPE_STRING_UNKNOWN; } // no default case, so the compiler can warn about missing cases assert(false); } @@ -61,7 +65,8 @@ CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type) return witdest; } } - case OutputType::BECH32M: {} // This function should never be used with BECH32M, so let it assert + case OutputType::BECH32M: + case OutputType::UNKNOWN: {} // This function should never be used with BECH32M or UNKNOWN, so let it assert } // no default case, so the compiler can warn about missing cases assert(false); } @@ -99,7 +104,8 @@ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, return ScriptHash(witprog); } } - case OutputType::BECH32M: {} // This function should not be used for BECH32M, so let it assert + case OutputType::BECH32M: + case OutputType::UNKNOWN: {} // This function should not be used for BECH32M or UNKNOWN, so let it assert } // no default case, so the compiler can warn about missing cases assert(false); } diff --git a/src/outputtype.h b/src/outputtype.h index 6b4e695760..be5fd62b80 100644 --- a/src/outputtype.h +++ b/src/outputtype.h @@ -19,6 +19,7 @@ enum class OutputType { P2SH_SEGWIT, BECH32, BECH32M, + UNKNOWN, }; static constexpr auto OUTPUT_TYPES = std::array{ @@ -26,6 +27,7 @@ static constexpr auto OUTPUT_TYPES = std::array{ OutputType::P2SH_SEGWIT, OutputType::BECH32, OutputType::BECH32M, + OutputType::UNKNOWN, }; std::optional<OutputType> ParseOutputType(const std::string& str); diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index c01ca28290..50766f20eb 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -641,7 +641,7 @@ RPCHelpMan listunspent() cctl.m_max_depth = nMaxDepth; cctl.m_include_unsafe_inputs = include_unsafe; LOCK(pwallet->cs_wallet); - vecOutputs = AvailableCoinsListUnspent(*pwallet, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount).all(); + vecOutputs = AvailableCoinsListUnspent(*pwallet, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount).All(); } LOCK(pwallet->cs_wallet); diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index da0615ed1c..699d9062ff 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1382,7 +1382,7 @@ RPCHelpMan sendall() total_input_value += tx->tx->vout[input.prevout.n].nValue; } } else { - for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, /*nMinimumAmount=*/0).all()) { + for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, /*nMinimumAmount=*/0).All()) { CHECK_NONFATAL(output.input_bytes > 0); if (send_max && fee_rate.GetFee(output.input_bytes) > output.txout.nValue) { continue; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 7ff017775e..7511312f70 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1965,6 +1965,11 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ desc_prefix = "tr(" + xpub + "/86'"; break; } + case OutputType::UNKNOWN: { + // We should never have a DescriptorScriptPubKeyMan for an UNKNOWN OutputType, + // so if we get to this point something is wrong + assert(false); + } } // no default case, so the compiler can warn about missing cases assert(!desc_prefix.empty()); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 61d86844df..d6362c1f14 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -79,30 +79,68 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *walle return CalculateMaximumSignedTxSize(tx, wallet, txouts, coin_control); } -uint64_t CoinsResult::size() const +size_t CoinsResult::Size() const { - return bech32m.size() + bech32.size() + P2SH_segwit.size() + legacy.size() + other.size(); + size_t size{0}; + for (const auto& it : coins) { + size += it.second.size(); + } + return size; } -std::vector<COutput> CoinsResult::all() const +std::vector<COutput> CoinsResult::All() const { std::vector<COutput> all; - all.reserve(this->size()); - all.insert(all.end(), bech32m.begin(), bech32m.end()); - all.insert(all.end(), bech32.begin(), bech32.end()); - all.insert(all.end(), P2SH_segwit.begin(), P2SH_segwit.end()); - all.insert(all.end(), legacy.begin(), legacy.end()); - all.insert(all.end(), other.begin(), other.end()); + all.reserve(coins.size()); + for (const auto& it : coins) { + all.insert(all.end(), it.second.begin(), it.second.end()); + } return all; } -void CoinsResult::clear() +void CoinsResult::Clear() { + coins.clear(); +} + +void CoinsResult::Erase(std::set<COutPoint>& preset_coins) { - bech32m.clear(); - bech32.clear(); - P2SH_segwit.clear(); - legacy.clear(); - other.clear(); + for (auto& it : coins) { + auto& vec = it.second; + auto i = std::find_if(vec.begin(), vec.end(), [&](const COutput &c) { return preset_coins.count(c.outpoint);}); + if (i != vec.end()) { + vec.erase(i); + break; + } + } +} + +void CoinsResult::Shuffle(FastRandomContext& rng_fast) +{ + for (auto& it : coins) { + ::Shuffle(it.second.begin(), it.second.end(), rng_fast); + } +} + +void CoinsResult::Add(OutputType type, const COutput& out) +{ + coins[type].emplace_back(out); +} + +static OutputType GetOutputType(TxoutType type, bool is_from_p2sh) +{ + switch (type) { + case TxoutType::WITNESS_V1_TAPROOT: + return OutputType::BECH32M; + case TxoutType::WITNESS_V0_KEYHASH: + case TxoutType::WITNESS_V0_SCRIPTHASH: + if (is_from_p2sh) return OutputType::P2SH_SEGWIT; + else return OutputType::BECH32; + case TxoutType::SCRIPTHASH: + case TxoutType::PUBKEYHASH: + return OutputType::LEGACY; + default: + return OutputType::UNKNOWN; + } } CoinsResult AvailableCoins(const CWallet& wallet, @@ -222,51 +260,32 @@ CoinsResult AvailableCoins(const CWallet& wallet, // Filter by spendable outputs only if (!spendable && only_spendable) continue; - // When parsing a scriptPubKey, Solver returns the parsed pubkeys or hashes (depending on the script) - // We don't need those here, so we are leaving them in return_values_unused - std::vector<std::vector<uint8_t>> return_values_unused; - TxoutType type; - bool is_from_p2sh{false}; - // If the Output is P2SH and spendable, we want to know if it is // a P2SH (legacy) or one of P2SH-P2WPKH, P2SH-P2WSH (P2SH-Segwit). We can determine // this from the redeemScript. If the Output is not spendable, it will be classified // as a P2SH (legacy), since we have no way of knowing otherwise without the redeemScript + CScript script; + bool is_from_p2sh{false}; if (output.scriptPubKey.IsPayToScriptHash() && solvable) { - CScript redeemScript; CTxDestination destination; if (!ExtractDestination(output.scriptPubKey, destination)) continue; const CScriptID& hash = CScriptID(std::get<ScriptHash>(destination)); - if (!provider->GetCScript(hash, redeemScript)) + if (!provider->GetCScript(hash, script)) continue; - type = Solver(redeemScript, return_values_unused); is_from_p2sh = true; } else { - type = Solver(output.scriptPubKey, return_values_unused); + script = output.scriptPubKey; } COutput coin(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate); - switch (type) { - case TxoutType::WITNESS_UNKNOWN: - case TxoutType::WITNESS_V1_TAPROOT: - result.bech32m.push_back(coin); - break; - case TxoutType::WITNESS_V0_KEYHASH: - case TxoutType::WITNESS_V0_SCRIPTHASH: - if (is_from_p2sh) { - result.P2SH_segwit.push_back(coin); - break; - } - result.bech32.push_back(coin); - break; - case TxoutType::SCRIPTHASH: - case TxoutType::PUBKEYHASH: - result.legacy.push_back(coin); - break; - default: - result.other.push_back(coin); - }; + + // When parsing a scriptPubKey, Solver returns the parsed pubkeys or hashes (depending on the script) + // We don't need those here, so we are leaving them in return_values_unused + std::vector<std::vector<uint8_t>> return_values_unused; + TxoutType type; + type = Solver(script, return_values_unused); + result.Add(GetOutputType(type, is_from_p2sh), coin); // Cache total amount as we go result.total_amount += output.nValue; @@ -278,7 +297,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, } // Checks the maximum number of UTXO's. - if (nMaximumCount > 0 && result.size() >= nMaximumCount) { + if (nMaximumCount > 0 && result.Size() >= nMaximumCount) { return result; } } @@ -334,7 +353,7 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) std::map<CTxDestination, std::vector<COutput>> result; - for (const COutput& coin : AvailableCoinsListUnspent(wallet).all()) { + for (const COutput& coin : AvailableCoinsListUnspent(wallet).All()) { CTxDestination address; if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) && ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) { @@ -457,31 +476,25 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm { // Run coin selection on each OutputType and compute the Waste Metric std::vector<SelectionResult> results; - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.legacy, coin_selection_params)}) { - results.push_back(*result); - } - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.P2SH_segwit, coin_selection_params)}) { - results.push_back(*result); - } - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.bech32, coin_selection_params)}) { - results.push_back(*result); - } - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.bech32m, coin_selection_params)}) { - results.push_back(*result); + for (const auto& it : available_coins.coins) { + if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}) { + results.push_back(*result); + } } - - // If we can't fund the transaction from any individual OutputType, run coin selection - // over all available coins, else pick the best solution from the results - if (results.size() == 0) { - if (allow_mixed_output_types) { - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.all(), coin_selection_params)}) { - return result; - } + // If we have at least one solution for funding the transaction without mixing, choose the minimum one according to waste metric + // and return the result + if (results.size() > 0) return *std::min_element(results.begin(), results.end()); + + // If we can't fund the transaction from any individual OutputType, run coin selection one last time + // over all available coins, which would allow mixing + if (allow_mixed_output_types) { + if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params)}) { + return result; } - return std::optional<SelectionResult>(); - }; - std::optional<SelectionResult> result{*std::min_element(results.begin(), results.end())}; - return result; + } + // Either mixing is not allowed and we couldn't find a solution from any single OutputType, or mixing was allowed and we still couldn't + // find a solution using all available coins + return std::nullopt; }; std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins, const CoinSelectionParams& coin_selection_params) @@ -592,11 +605,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a // remove preset inputs from coins so that Coin Selection doesn't pick them. if (coin_control.HasSelected()) { - available_coins.legacy.erase(remove_if(available_coins.legacy.begin(), available_coins.legacy.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.legacy.end()); - available_coins.P2SH_segwit.erase(remove_if(available_coins.P2SH_segwit.begin(), available_coins.P2SH_segwit.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.P2SH_segwit.end()); - available_coins.bech32.erase(remove_if(available_coins.bech32.begin(), available_coins.bech32.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.bech32.end()); - available_coins.bech32m.erase(remove_if(available_coins.bech32m.begin(), available_coins.bech32m.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.bech32m.end()); - available_coins.other.erase(remove_if(available_coins.other.begin(), available_coins.other.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.other.end()); + available_coins.Erase(preset_coins); } unsigned int limit_ancestor_count = 0; @@ -608,15 +617,11 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a // 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) { + 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 - Shuffle(available_coins.legacy.begin(), available_coins.legacy.end(), coin_selection_params.rng_fast); - Shuffle(available_coins.P2SH_segwit.begin(), available_coins.P2SH_segwit.end(), coin_selection_params.rng_fast); - Shuffle(available_coins.bech32.begin(), available_coins.bech32.end(), coin_selection_params.rng_fast); - Shuffle(available_coins.bech32m.begin(), available_coins.bech32m.end(), coin_selection_params.rng_fast); - Shuffle(available_coins.other.begin(), available_coins.other.end(), coin_selection_params.rng_fast); + available_coins.Shuffle(coin_selection_params.rng_fast); } // Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the diff --git a/src/wallet/spend.h b/src/wallet/spend.h index fdb5113ba4..c29e5be5c7 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -34,26 +34,22 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* walle * This struct is really just a wrapper around OutputType vectors with a convenient * method for concatenating and returning all COutputs as one vector. * - * clear(), size() methods are implemented so that one can interact with - * the CoinsResult struct as if it was a vector + * Size(), Clear(), Erase(), Shuffle(), and Add() methods are implemented to + * allow easy interaction with the struct. */ struct CoinsResult { - /** Vectors for each OutputType */ - std::vector<COutput> legacy; - std::vector<COutput> P2SH_segwit; - std::vector<COutput> bech32; - std::vector<COutput> bech32m; - - /** Other is a catch-all for anything that doesn't match the known OutputTypes */ - std::vector<COutput> other; + std::map<OutputType, std::vector<COutput>> coins; /** Concatenate and return all COutputs as one vector */ - std::vector<COutput> all() const; + std::vector<COutput> All() const; /** The following methods are provided so that CoinsResult can mimic a vector, * i.e., methods can work with individual OutputType vectors or on the entire object */ - uint64_t size() const; - void clear(); + size_t Size() const; + void Clear(); + void Erase(std::set<COutPoint>& preset_coins); + void Shuffle(FastRandomContext& rng_fast); + void Add(OutputType type, const COutput& out); /** Sum of all available coins */ CAmount total_amount{0}; diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp index 3356128112..c8adef7fc2 100644 --- a/src/wallet/test/availablecoins_tests.cpp +++ b/src/wallet/test/availablecoins_tests.cpp @@ -63,8 +63,8 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) // Verify our wallet has one usable coinbase UTXO before starting // This UTXO is a P2PK, so it should show up in the Other bucket available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.size(), 1U); - BOOST_CHECK_EQUAL(available_coins.other.size(), 1U); + BOOST_CHECK_EQUAL(available_coins.Size(), 1U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::UNKNOWN].size(), 1U); // We will create a self transfer for each of the OutputTypes and // verify it is put in the correct bucket after running GetAvailablecoins @@ -78,27 +78,28 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) BOOST_ASSERT(dest); AddTx(CRecipient{{GetScriptForDestination(*dest)}, 1 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.bech32m.size(), 2U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::BECH32M].size(), 2U); // Bech32 dest = wallet->GetNewDestination(OutputType::BECH32, ""); BOOST_ASSERT(dest); AddTx(CRecipient{{GetScriptForDestination(*dest)}, 2 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.bech32.size(), 2U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::BECH32].size(), 2U); // P2SH-SEGWIT dest = wallet->GetNewDestination(OutputType::P2SH_SEGWIT, ""); + BOOST_ASSERT(dest); AddTx(CRecipient{{GetScriptForDestination(*dest)}, 3 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.P2SH_segwit.size(), 2U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::P2SH_SEGWIT].size(), 2U); // Legacy (P2PKH) dest = wallet->GetNewDestination(OutputType::LEGACY, ""); BOOST_ASSERT(dest); AddTx(CRecipient{{GetScriptForDestination(*dest)}, 4 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::LEGACY].size(), 2U); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index a4a7216436..4dfdc87a38 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -83,7 +83,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun assert(ret.second); CWalletTx& wtx = (*ret.first).second; const auto& txout = wtx.tx->vout.at(nInput); - available_coins.bech32.emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate); + available_coins.coins[OutputType::BECH32].emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate); } /** Check if SelectionResult a is equivalent to SelectionResult b. @@ -308,15 +308,15 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) CoinsResult available_coins; add_coin(available_coins, *wallet, 1, coin_selection_params_bnb.m_effective_feerate); - available_coins.all().at(0).input_bytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.all()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change)); + available_coins.All().at(0).input_bytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change)); // Test fees subtracted from output: - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate); - available_coins.all().at(0).input_bytes = 40; + available_coins.All().at(0).input_bytes = 40; coin_selection_params_bnb.m_subtract_fee_outputs = true; - const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.all()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change); + const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change); BOOST_CHECK(result9); BOOST_CHECK_EQUAL(result9->GetSelectedValue(), 1 * CENT); } @@ -335,7 +335,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(available_coins, *wallet, 2 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); CCoinControl coin_control; coin_control.m_allow_other_inputs = true; - coin_control.Select(available_coins.all().at(0).outpoint); + coin_control.Select(available_coins.All().at(0).outpoint); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); const auto result10 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(result10); @@ -362,7 +362,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) CCoinControl coin_control; const auto result11 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result11)); - available_coins.clear(); + available_coins.Clear(); // more coins should be selected when effective fee < long term fee coin_selection_params_bnb.m_effective_feerate = CFeeRate(3000); @@ -377,7 +377,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(1 * CENT, 2, expected_result); const auto result12 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result12)); - available_coins.clear(); + available_coins.Clear(); // pre selected coin should be selected even if disadvantageous coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000); @@ -391,7 +391,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(9 * CENT, 2, expected_result); add_coin(1 * CENT, 2, expected_result); coin_control.m_allow_other_inputs = true; - coin_control.Select(available_coins.all().at(1).outpoint); // pre select 9 coin + coin_control.Select(available_coins.All().at(1).outpoint); // pre select 9 coin const auto result13 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result13)); } @@ -413,28 +413,28 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test multiple times to allow for differences in the shuffle order for (int i = 0; i < RUN_TESTS; i++) { - available_coins.clear(); + available_coins.Clear(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 1 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1 * CENT, CENT)); add_coin(available_coins, *wallet, 1*CENT, CFeeRate(0), 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 1 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1 * CENT, CENT)); // but we can find a new 1 cent - const auto result1 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 1 * CENT, CENT); + const auto result1 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result1); BOOST_CHECK_EQUAL(result1->GetSelectedValue(), 1 * CENT); add_coin(available_coins, *wallet, 2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 3 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 3 * CENT, CENT)); // we can make 3 cents of new coins - const auto result2 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 3 * CENT, CENT); + const auto result2 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 3 * CENT, CENT); BOOST_CHECK(result2); BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 3 * CENT); @@ -445,44 +445,44 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 38 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 38 * CENT, CENT)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard_extra), 38 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard_extra), 38 * CENT, CENT)); // but we can make 37 cents if we accept new coins from ourself - const auto result3 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 37 * CENT, CENT); + const auto result3 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 37 * CENT, CENT); BOOST_CHECK(result3); BOOST_CHECK_EQUAL(result3->GetSelectedValue(), 37 * CENT); // and we can make 38 cents if we accept all new coins - const auto result4 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 38 * CENT, CENT); + const auto result4 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 38 * CENT, CENT); BOOST_CHECK(result4); BOOST_CHECK_EQUAL(result4->GetSelectedValue(), 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - const auto result5 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 34 * CENT, CENT); + const auto result5 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 34 * CENT, CENT); BOOST_CHECK(result5); BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(result5->GetInputSet().size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - const auto result6 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 7 * CENT, CENT); + const auto result6 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 7 * CENT, CENT); BOOST_CHECK(result6); BOOST_CHECK_EQUAL(result6->GetSelectedValue(), 7 * CENT); BOOST_CHECK_EQUAL(result6->GetInputSet().size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - const auto result7 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 8 * CENT, CENT); + const auto result7 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 8 * CENT, CENT); BOOST_CHECK(result7); BOOST_CHECK(result7->GetSelectedValue() == 8 * CENT); BOOST_CHECK_EQUAL(result7->GetInputSet().size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - const auto result8 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 9 * CENT, CENT); + const auto result8 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 9 * CENT, CENT); BOOST_CHECK(result8); BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 10 * CENT); BOOST_CHECK_EQUAL(result8->GetInputSet().size(), 1U); // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, 6*CENT); add_coin(available_coins, *wallet, 7*CENT); @@ -491,12 +491,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - const auto result9 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 71 * CENT, CENT); + const auto result9 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 71 * CENT, CENT); BOOST_CHECK(result9); - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 72 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 72 * CENT, CENT)); // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - const auto result10 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 16 * CENT, CENT); + const auto result10 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result10); BOOST_CHECK_EQUAL(result10->GetSelectedValue(), 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(result10->GetInputSet().size(), 1U); @@ -504,7 +504,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - const auto result11 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 16 * CENT, CENT); + const auto result11 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result11); BOOST_CHECK_EQUAL(result11->GetSelectedValue(), 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(result11->GetInputSet().size(), 3U); @@ -512,13 +512,13 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - const auto result12 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 16 * CENT, CENT); + const auto result12 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result12); BOOST_CHECK_EQUAL(result12->GetSelectedValue(), 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(result12->GetInputSet().size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - const auto result13 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 11 * CENT, CENT); + const auto result13 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 11 * CENT, CENT); BOOST_CHECK(result13); BOOST_CHECK_EQUAL(result13->GetSelectedValue(), 11 * CENT); BOOST_CHECK_EQUAL(result13->GetInputSet().size(), 2U); @@ -528,19 +528,19 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 2*COIN); add_coin(available_coins, *wallet, 3*COIN); add_coin(available_coins, *wallet, 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - const auto result14 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 95 * CENT, CENT); + const auto result14 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 95 * CENT, CENT); BOOST_CHECK(result14); BOOST_CHECK_EQUAL(result14->GetSelectedValue(), 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(result14->GetInputSet().size(), 1U); - const auto result15 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 195 * CENT, CENT); + const auto result15 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 195 * CENT, CENT); BOOST_CHECK(result15); BOOST_CHECK_EQUAL(result15->GetSelectedValue(), 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(result15->GetInputSet().size(), 1U); // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, CENT * 1 / 10); add_coin(available_coins, *wallet, CENT * 2 / 10); add_coin(available_coins, *wallet, CENT * 3 / 10); @@ -549,7 +549,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // try making 1 * CENT from the 1.5 * CENT // we'll get change smaller than CENT whatever happens, so can expect CENT exactly - const auto result16 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), CENT, CENT); + const auto result16 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT, CENT); BOOST_CHECK(result16); BOOST_CHECK_EQUAL(result16->GetSelectedValue(), CENT); @@ -557,7 +557,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 1111*CENT); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - const auto result17 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 1 * CENT, CENT); + const auto result17 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result17); BOOST_CHECK_EQUAL(result17->GetSelectedValue(), 1 * CENT); // we should get the exact amount @@ -566,17 +566,17 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, CENT * 7 / 10); // and try again to make 1.0 * CENT - const auto result18 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 1 * CENT, CENT); + const auto result18 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result18); BOOST_CHECK_EQUAL(result18->GetSelectedValue(), 1 * CENT); // we should get the exact amount // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - available_coins.clear(); + available_coins.Clear(); for (int j = 0; j < 20; j++) add_coin(available_coins, *wallet, 50000 * COIN); - const auto result19 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 500000 * COIN, CENT); + const auto result19 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 500000 * COIN, CENT); BOOST_CHECK(result19); BOOST_CHECK_EQUAL(result19->GetSelectedValue(), 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(result19->GetInputSet().size(), 10U); // in ten coins @@ -585,41 +585,41 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // we need to try finding an exact subset anyway // sometimes it will fail, and so we use the next biggest coin: - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, CENT * 5 / 10); add_coin(available_coins, *wallet, CENT * 6 / 10); add_coin(available_coins, *wallet, CENT * 7 / 10); add_coin(available_coins, *wallet, 1111 * CENT); - const auto result20 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 1 * CENT, CENT); + const auto result20 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result20); BOOST_CHECK_EQUAL(result20->GetSelectedValue(), 1111 * CENT); // we get the bigger coin BOOST_CHECK_EQUAL(result20->GetInputSet().size(), 1U); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, CENT * 4 / 10); add_coin(available_coins, *wallet, CENT * 6 / 10); add_coin(available_coins, *wallet, CENT * 8 / 10); add_coin(available_coins, *wallet, 1111 * CENT); - const auto result21 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), CENT, CENT); + const auto result21 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT, CENT); BOOST_CHECK(result21); BOOST_CHECK_EQUAL(result21->GetSelectedValue(), CENT); // we should get the exact amount BOOST_CHECK_EQUAL(result21->GetInputSet().size(), 2U); // in two coins 0.4+0.6 // test avoiding small change - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, CENT * 5 / 100); add_coin(available_coins, *wallet, CENT * 1); add_coin(available_coins, *wallet, CENT * 100); // trying to make 100.01 from these three coins - const auto result22 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), CENT * 10001 / 100, CENT); + const auto result22 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT * 10001 / 100, CENT); BOOST_CHECK(result22); BOOST_CHECK_EQUAL(result22->GetSelectedValue(), CENT * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(result22->GetInputSet().size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - const auto result23 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), CENT * 9990 / 100, CENT); + const auto result23 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT * 9990 / 100, CENT); BOOST_CHECK(result23); BOOST_CHECK_EQUAL(result23->GetSelectedValue(), 101 * CENT); BOOST_CHECK_EQUAL(result23->GetInputSet().size(), 2U); @@ -627,14 +627,14 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test with many inputs for (CAmount amt=1500; amt < COIN; amt*=10) { - available_coins.clear(); + available_coins.Clear(); // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) for (uint16_t j = 0; j < 676; j++) add_coin(available_coins, *wallet, amt); // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. for (int i = 0; i < RUN_TESTS; i++) { - const auto result24 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 2000, CENT); + const auto result24 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 2000, CENT); BOOST_CHECK(result24); if (amt - 2000 < CENT) { @@ -653,7 +653,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test randomness { - available_coins.clear(); + available_coins.Clear(); for (int i2 = 0; i2 < 100; i2++) add_coin(available_coins, *wallet, COIN); @@ -661,9 +661,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int i = 0; i < RUN_TESTS; i++) { // picking 50 from 100 coins doesn't depend on the shuffle, // but does depend on randomness in the stochastic approximation code - const auto result25 = KnapsackSolver(GroupCoins(available_coins.all()), 50 * COIN, CENT); + const auto result25 = KnapsackSolver(GroupCoins(available_coins.All()), 50 * COIN, CENT); BOOST_CHECK(result25); - const auto result26 = KnapsackSolver(GroupCoins(available_coins.all()), 50 * COIN, CENT); + const auto result26 = KnapsackSolver(GroupCoins(available_coins.All()), 50 * COIN, CENT); BOOST_CHECK(result26); BOOST_CHECK(!EqualResult(*result25, *result26)); @@ -674,9 +674,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // When choosing 1 from 100 identical coins, 1% of the time, this test will choose the same coin twice // which will cause it to fail. // To avoid that issue, run the test RANDOM_REPEATS times and only complain if all of them fail - const auto result27 = KnapsackSolver(GroupCoins(available_coins.all()), COIN, CENT); + const auto result27 = KnapsackSolver(GroupCoins(available_coins.All()), COIN, CENT); BOOST_CHECK(result27); - const auto result28 = KnapsackSolver(GroupCoins(available_coins.all()), COIN, CENT); + const auto result28 = KnapsackSolver(GroupCoins(available_coins.All()), COIN, CENT); BOOST_CHECK(result28); if (EqualResult(*result27, *result28)) fails++; @@ -697,9 +697,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) int fails = 0; for (int j = 0; j < RANDOM_REPEATS; j++) { - const auto result29 = KnapsackSolver(GroupCoins(available_coins.all()), 90 * CENT, CENT); + const auto result29 = KnapsackSolver(GroupCoins(available_coins.All()), 90 * CENT, CENT); BOOST_CHECK(result29); - const auto result30 = KnapsackSolver(GroupCoins(available_coins.all()), 90 * CENT, CENT); + const auto result30 = KnapsackSolver(GroupCoins(available_coins.All()), 90 * CENT, CENT); BOOST_CHECK(result30); if (EqualResult(*result29, *result30)) fails++; @@ -725,7 +725,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(available_coins, *wallet, 1000 * COIN); add_coin(available_coins, *wallet, 3 * COIN); - const auto result = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 1003 * COIN, CENT, rand); + const auto result = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1003 * COIN, CENT, rand); BOOST_CHECK(result); BOOST_CHECK_EQUAL(result->GetSelectedValue(), 1003 * COIN); BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2U); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 093f510945..4afeb88ae9 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -591,7 +591,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) // Lock both coins. Confirm number of available coins drops to 0. { LOCK(wallet->cs_wallet); - BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).size(), 2U); + BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).Size(), 2U); } for (const auto& group : list) { for (const auto& coin : group.second) { @@ -601,7 +601,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) } { LOCK(wallet->cs_wallet); - BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).size(), 0U); + BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).Size(), 0U); } // Confirm ListCoins still returns same result as before, despite coins // being locked. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e0a794dbe6..45f49954a3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3426,6 +3426,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() for (bool internal : {false, true}) { for (OutputType t : OUTPUT_TYPES) { + if (t == OutputType::UNKNOWN) continue; auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this)); if (IsCrypted()) { if (IsLocked()) { |