diff options
| -rw-r--r-- | src/Makefile.bench.include | 1 | ||||
| -rw-r--r-- | src/bench/wallet_create_tx.cpp | 142 | ||||
| -rw-r--r-- | src/qt/sendcoinsdialog.cpp | 4 | ||||
| -rw-r--r-- | src/test/util/wallet.cpp | 7 | ||||
| -rw-r--r-- | src/test/util/wallet.h | 5 | ||||
| -rw-r--r-- | src/wallet/coincontrol.h | 2 | ||||
| -rw-r--r-- | src/wallet/coinselection.cpp | 6 | ||||
| -rw-r--r-- | src/wallet/coinselection.h | 1 | ||||
| -rw-r--r-- | src/wallet/spend.cpp | 184 | ||||
| -rw-r--r-- | src/wallet/spend.h | 40 | ||||
| -rw-r--r-- | src/wallet/test/coinselector_tests.cpp | 27 | ||||
| -rwxr-xr-x | test/functional/rpc_fundrawtransaction.py | 16 | ||||
| -rwxr-xr-x | test/functional/rpc_psbt.py | 2 | ||||
| -rwxr-xr-x | test/functional/wallet_send.py | 2 |
14 files changed, 330 insertions, 109 deletions
diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index e1e2066877..0a3f9df463 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -79,6 +79,7 @@ if ENABLE_WALLET bench_bench_bitcoin_SOURCES += bench/coin_selection.cpp bench_bench_bitcoin_SOURCES += bench/wallet_balance.cpp bench_bench_bitcoin_SOURCES += bench/wallet_loading.cpp +bench_bench_bitcoin_SOURCES += bench/wallet_create_tx.cpp bench_bench_bitcoin_LDADD += $(BDB_LIBS) $(SQLITE_LIBS) endif diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp new file mode 100644 index 0000000000..207b22c584 --- /dev/null +++ b/src/bench/wallet_create_tx.cpp @@ -0,0 +1,142 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include <bench/bench.h> +#include <chainparams.h> +#include <wallet/coincontrol.h> +#include <consensus/merkle.h> +#include <kernel/chain.h> +#include <node/context.h> +#include <test/util/setup_common.h> +#include <test/util/wallet.h> +#include <validation.h> +#include <wallet/spend.h> +#include <wallet/wallet.h> + +using wallet::CWallet; +using wallet::CreateMockWalletDatabase; +using wallet::DBErrors; +using wallet::WALLET_FLAG_DESCRIPTORS; + +struct TipBlock +{ + uint256 prev_block_hash; + int64_t prev_block_time; + int tip_height; +}; + +TipBlock getTip(const CChainParams& params, const node::NodeContext& context) +{ + auto tip = WITH_LOCK(::cs_main, return context.chainman->ActiveTip()); + return (tip) ? TipBlock{tip->GetBlockHash(), tip->GetBlockTime(), tip->nHeight} : + TipBlock{params.GenesisBlock().GetHash(), params.GenesisBlock().GetBlockTime(), 0}; +} + +void generateFakeBlock(const CChainParams& params, + const node::NodeContext& context, + CWallet& wallet, + const CScript& coinbase_out_script) +{ + TipBlock tip{getTip(params, context)}; + + // Create block + CBlock block; + CMutableTransaction coinbase_tx; + coinbase_tx.vin.resize(1); + coinbase_tx.vin[0].prevout.SetNull(); + coinbase_tx.vout.resize(2); + coinbase_tx.vout[0].scriptPubKey = coinbase_out_script; + coinbase_tx.vout[0].nValue = 49 * COIN; + coinbase_tx.vin[0].scriptSig = CScript() << ++tip.tip_height << OP_0; + coinbase_tx.vout[1].scriptPubKey = coinbase_out_script; // extra output + coinbase_tx.vout[1].nValue = 1 * COIN; + block.vtx = {MakeTransactionRef(std::move(coinbase_tx))}; + + block.nVersion = VERSIONBITS_LAST_OLD_BLOCK_VERSION; + block.hashPrevBlock = tip.prev_block_hash; + block.hashMerkleRoot = BlockMerkleRoot(block); + block.nTime = ++tip.prev_block_time; + block.nBits = params.GenesisBlock().nBits; + block.nNonce = 0; + + { + LOCK(::cs_main); + // Add it to the index + CBlockIndex* pindex{context.chainman->m_blockman.AddToBlockIndex(block, context.chainman->m_best_header)}; + // add it to the chain + context.chainman->ActiveChain().SetTip(*pindex); + } + + // notify wallet + const auto& pindex = WITH_LOCK(::cs_main, return context.chainman->ActiveChain().Tip()); + wallet.blockConnected(kernel::MakeBlockInfo(pindex, &block)); +} + +struct PreSelectInputs { + // How many coins from the wallet the process should select + int num_of_internal_inputs; + // future: this could have external inputs as well. +}; + +static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type, bool allow_other_inputs, std::optional<PreSelectInputs> preset_inputs) +{ + const auto test_setup = MakeNoLogFileContext<const TestingSetup>(); + + CWallet wallet{test_setup->m_node.chain.get(), "", gArgs, CreateMockWalletDatabase()}; + { + LOCK(wallet.cs_wallet); + wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet.SetupDescriptorScriptPubKeyMans(); + if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false); + } + + // Generate destinations + CScript dest = GetScriptForDestination(getNewDestination(wallet, output_type)); + + // Generate chain; each coinbase will have two outputs to fill-up the wallet + const auto& params = Params(); + unsigned int chain_size = 5000; // 5k blocks means 10k UTXO for the wallet (minus 200 due COINBASE_MATURITY) + for (unsigned int i = 0; i < chain_size; ++i) { + generateFakeBlock(params, test_setup->m_node, wallet, dest); + } + + // Check available balance + auto bal = wallet::GetAvailableBalance(wallet); // Cache + assert(bal == 50 * COIN * (chain_size - COINBASE_MATURITY)); + + wallet::CCoinControl coin_control; + coin_control.m_allow_other_inputs = allow_other_inputs; + + CAmount target = 0; + if (preset_inputs) { + // Select inputs, each has 49 BTC + const auto& res = WITH_LOCK(wallet.cs_wallet, + return wallet::AvailableCoins(wallet, nullptr, std::nullopt, 1, MAX_MONEY, + MAX_MONEY, preset_inputs->num_of_internal_inputs)); + for (int i=0; i < preset_inputs->num_of_internal_inputs; i++) { + const auto& coin{res.coins.at(output_type)[i]}; + target += coin.txout.nValue; + coin_control.Select(coin.outpoint); + } + } + + // If automatic coin selection is enabled, add the value of another UTXO to the target + if (coin_control.m_allow_other_inputs) target += 50 * COIN; + std::vector<wallet::CRecipient> recipients = {{dest, target, true}}; + + bench.epochIterations(5).run([&] { + LOCK(wallet.cs_wallet); + const auto& tx_res = CreateTransaction(wallet, recipients, -1, coin_control); + assert(tx_res); + }); +} + +static void WalletCreateTxUseOnlyPresetInputs(benchmark::Bench& bench) { WalletCreateTx(bench, OutputType::BECH32, /*allow_other_inputs=*/false, + {{/*num_of_internal_inputs=*/4}}); } + +static void WalletCreateTxUsePresetInputsAndCoinSelection(benchmark::Bench& bench) { WalletCreateTx(bench, OutputType::BECH32, /*allow_other_inputs=*/true, + {{/*num_of_internal_inputs=*/4}}); } + +BENCHMARK(WalletCreateTxUseOnlyPresetInputs, benchmark::PriorityLevel::LOW) +BENCHMARK(WalletCreateTxUsePresetInputsAndCoinSelection, benchmark::PriorityLevel::LOW) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 53c352b393..57094fc857 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -289,7 +289,9 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa updateCoinControlState(); - prepareStatus = model->prepareTransaction(*m_current_transaction, *m_coin_control); + CCoinControl coin_control = *m_coin_control; + coin_control.m_allow_other_inputs = !coin_control.HasSelected(); // future, could introduce a checkbox to customize this value. + prepareStatus = model->prepareTransaction(*m_current_transaction, coin_control); // process prepareStatus and on error generate message shown to user processSendCoinsReturn(prepareStatus, diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp index b54774cbb9..2dadffafb4 100644 --- a/src/test/util/wallet.cpp +++ b/src/test/util/wallet.cpp @@ -21,7 +21,12 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq std::string getnewaddress(CWallet& w) { constexpr auto output_type = OutputType::BECH32; - return EncodeDestination(*Assert(w.GetNewDestination(output_type, ""))); + return EncodeDestination(getNewDestination(w, output_type)); +} + +CTxDestination getNewDestination(CWallet& w, OutputType output_type) +{ + return *Assert(w.GetNewDestination(output_type, "")); } #endif // ENABLE_WALLET diff --git a/src/test/util/wallet.h b/src/test/util/wallet.h index 31281bf70e..d8f1db3fd7 100644 --- a/src/test/util/wallet.h +++ b/src/test/util/wallet.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_TEST_UTIL_WALLET_H #define BITCOIN_TEST_UTIL_WALLET_H +#include <outputtype.h> #include <string> namespace wallet { @@ -19,8 +20,10 @@ extern const std::string ADDRESS_BCRT1_UNSPENDABLE; /** Import the address to the wallet */ void importaddress(wallet::CWallet& wallet, const std::string& address); -/** Returns a new address from the wallet */ +/** Returns a new encoded destination from the wallet (hardcoded to BECH32) */ std::string getnewaddress(wallet::CWallet& w); +/** Returns a new destination, of an specific type, from the wallet */ +CTxDestination getNewDestination(wallet::CWallet& w, OutputType output_type); #endif // BITCOIN_TEST_UTIL_WALLET_H diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index d08d3664c4..b56a6d3aee 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -37,7 +37,7 @@ public: bool m_include_unsafe_inputs = false; //! If true, the selection process can add extra unselected inputs from the wallet //! while requires all selected inputs be used - bool m_allow_other_inputs = false; + bool m_allow_other_inputs = true; //! Includes watch only addresses which are solvable bool fAllowWatchOnly = false; //! Override automatic min/max checks on fee, m_feerate must be set if true diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index b568e90998..a8be6cd83a 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -444,6 +444,12 @@ void SelectionResult::AddInput(const OutputGroup& group) m_use_effective = !group.m_subtract_fee_outputs; } +void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs) +{ + util::insert(m_selected_inputs, inputs); + m_use_effective = !subtract_fee_outputs; +} + void SelectionResult::Merge(const SelectionResult& other) { m_target += other.m_target; diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 761c2be0b3..b23dd10867 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -308,6 +308,7 @@ public: void Clear(); void AddInput(const OutputGroup& group); + void AddInputs(const std::set<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); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 6833f9a095..644b2b587c 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -143,6 +143,51 @@ static OutputType GetOutputType(TxoutType type, bool is_from_p2sh) } } +// Fetch and validate the coin control selected inputs. +// Coins could be internal (from the wallet) or external. +util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +{ + PreSelectedInputs result; + std::vector<COutPoint> vPresetInputs; + coin_control.ListSelected(vPresetInputs); + for (const COutPoint& outpoint : vPresetInputs) { + int input_bytes = -1; + CTxOut txout; + if (auto ptr_wtx = wallet.GetWalletTx(outpoint.hash)) { + // Clearly invalid input, fail + if (ptr_wtx->tx->vout.size() <= outpoint.n) { + return util::Error{strprintf(_("Invalid pre-selected input %s"), outpoint.ToString())}; + } + txout = ptr_wtx->tx->vout.at(outpoint.n); + input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control); + } else { + // The input is external. We did not find the tx in mapWallet. + if (!coin_control.GetExternalOutput(outpoint, txout)) { + return util::Error{strprintf(_("Not found pre-selected input %s"), outpoint.ToString())}; + } + } + + if (input_bytes == -1) { + input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control); + } + + // If available, override calculated size with coin control specified size + if (coin_control.HasInputWeight(outpoint)) { + input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0); + } + + if (input_bytes == -1) { + return util::Error{strprintf(_("Not solvable pre-selected input %s"), outpoint.ToString())}; // Not solvable, can't estimate size for fee + } + + /* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */ + COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate); + result.Insert(output, coin_selection_params.m_subtract_fee_outputs); + } + return result; +} + CoinsResult AvailableCoins(const CWallet& wallet, const CCoinControl* coinControl, std::optional<CFeeRate> feerate, @@ -230,7 +275,8 @@ CoinsResult AvailableCoins(const CWallet& wallet, if (output.nValue < nMinimumAmount || output.nValue > nMaximumAmount) continue; - if (coinControl && coinControl->HasSelected() && !coinControl->m_allow_other_inputs && !coinControl->IsSelected(outpoint)) + // Skip manually selected coins (the caller can fetch them directly) + if (coinControl && coinControl->HasSelected() && coinControl->IsSelected(outpoint)) continue; if (wallet.IsLockedCoin(outpoint)) @@ -522,82 +568,42 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons return best_result; } -std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) +std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, + const CAmount& nTargetValue, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) { - CAmount value_to_select = nTargetValue; - - OutputGroup preset_inputs(coin_selection_params); + // Deduct preset inputs amount from the search target + CAmount selection_target = nTargetValue - pre_set_inputs.total_amount; - // calculate value from preset inputs and store them - std::set<COutPoint> preset_coins; + // Return if automatic coin selection is disabled, and we don't cover the selection target + if (!coin_control.m_allow_other_inputs && selection_target > 0) return std::nullopt; - std::vector<COutPoint> vPresetInputs; - coin_control.ListSelected(vPresetInputs); - for (const COutPoint& outpoint : vPresetInputs) { - int input_bytes = -1; - CTxOut txout; - auto ptr_wtx = wallet.GetWalletTx(outpoint.hash); - if (ptr_wtx) { - // Clearly invalid input, fail - if (ptr_wtx->tx->vout.size() <= outpoint.n) { - return std::nullopt; - } - txout = ptr_wtx->tx->vout.at(outpoint.n); - input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control); - } else { - // The input is external. We did not find the tx in mapWallet. - if (!coin_control.GetExternalOutput(outpoint, txout)) { - return std::nullopt; - } - } - - if (input_bytes == -1) { - input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control); - } - - // If available, override calculated size with coin control specified size - if (coin_control.HasInputWeight(outpoint)) { - input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0); - } - - if (input_bytes == -1) { - return std::nullopt; // Not solvable, can't estimate size for fee - } - - /* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */ - COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate); - if (coin_selection_params.m_subtract_fee_outputs) { - value_to_select -= output.txout.nValue; - } else { - value_to_select -= output.GetEffectiveValue(); - } - preset_coins.insert(outpoint); - /* Set ancestors and descendants to 0 as they don't matter for preset inputs since no actual selection is being done. - * positive_only is set to false because we want to include all preset inputs, even if they are dust. - */ - preset_inputs.Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false); - } - - // coin control -> return all selected outputs (we want all selected to go into the transaction for sure) - if (coin_control.HasSelected() && !coin_control.m_allow_other_inputs) { + // Return if we can cover the target only with the preset inputs + if (selection_target <= 0) { SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL); - result.AddInput(preset_inputs); - - if (!coin_selection_params.m_subtract_fee_outputs && result.GetSelectedEffectiveValue() < nTargetValue) { - return std::nullopt; - } else if (result.GetSelectedValue() < nTargetValue) { - return std::nullopt; - } - + result.AddInputs(pre_set_inputs.coins, coin_selection_params.m_subtract_fee_outputs); result.ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); return result; } - // remove preset inputs from coins so that Coin Selection doesn't pick them. - if (coin_control.HasSelected()) { - available_coins.Erase(preset_coins); + // Start wallet Coin Selection procedure + auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_control, coin_selection_params); + if (!op_selection_result) return op_selection_result; + + // If needed, add preset inputs to the automatic coin selection result + if (!pre_set_inputs.coins.empty()) { + SelectionResult preselected(pre_set_inputs.total_amount, SelectionAlgorithm::MANUAL); + preselected.AddInputs(pre_set_inputs.coins, coin_selection_params.m_subtract_fee_outputs); + op_selection_result->Merge(preselected); + op_selection_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, + coin_selection_params.m_cost_of_change, + coin_selection_params.m_change_fee); } + return op_selection_result; +} +std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) +{ unsigned int limit_ancestor_count = 0; unsigned int limit_descendant_count = 0; wallet.chain().getPackageLimits(limit_ancestor_count, limit_descendant_count); @@ -614,16 +620,10 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a available_coins.Shuffle(coin_selection_params.rng_fast); } - SelectionResult preselected(preset_inputs.GetSelectionAmount(), SelectionAlgorithm::MANUAL); - preselected.AddInput(preset_inputs); - // Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the // transaction at a target feerate. If an attempt fails, more attempts may be made using a more // permissive CoinEligibilityFilter. std::optional<SelectionResult> res = [&] { - // Pre-selected inputs already cover the target amount. - if (value_to_select <= 0) return std::make_optional(SelectionResult(value_to_select, SelectionAlgorithm::MANUAL)); - // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six // confirmations on outputs received from other wallets and only spend confirmed change. if (auto r1{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 6, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/false)}) return r1; @@ -673,14 +673,6 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a return std::optional<SelectionResult>(); }(); - if (!res) return std::nullopt; - - // Add preset inputs to result - res->Merge(preselected); - if (res->GetAlgo() == SelectionAlgorithm::MANUAL) { - res->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); - } - return res; } @@ -893,17 +885,29 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); CAmount selection_target = recipients_sum + not_input_fees; - // Get available coins - auto available_coins = AvailableCoins(wallet, - &coin_control, - coin_selection_params.m_effective_feerate, - 1, /*nMinimumAmount*/ - MAX_MONEY, /*nMaximumAmount*/ - MAX_MONEY, /*nMinimumSumAmount*/ - 0); /*nMaximumCount*/ + // Fetch manually selected coins + PreSelectedInputs preset_inputs; + if (coin_control.HasSelected()) { + auto res_fetch_inputs = FetchSelectedInputs(wallet, coin_control, coin_selection_params); + if (!res_fetch_inputs) return util::Error{util::ErrorString(res_fetch_inputs)}; + preset_inputs = *res_fetch_inputs; + } + + // Fetch wallet available coins if "other inputs" are + // allowed (coins automatically selected by the wallet) + CoinsResult available_coins; + if (coin_control.m_allow_other_inputs) { + available_coins = AvailableCoins(wallet, + &coin_control, + coin_selection_params.m_effective_feerate, + 1, /*nMinimumAmount*/ + MAX_MONEY, /*nMaximumAmount*/ + MAX_MONEY, /*nMinimumSumAmount*/ + 0); /*nMaximumCount*/ + } // Choose coins to use - std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); + std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); if (!result) { return util::Error{_("Insufficient funds")}; } diff --git a/src/wallet/spend.h b/src/wallet/spend.h index c29e5be5c7..b66bb3797c 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -121,9 +121,35 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm 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); +// User manually selected inputs that must be part of the transaction +struct PreSelectedInputs +{ + std::set<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 + CAmount total_amount{0}; + + void Insert(const COutput& output, bool subtract_fee_outputs) + { + if (subtract_fee_outputs) { + total_amount += output.txout.nValue; + } else { + total_amount += output.GetEffectiveValue(); + } + coins.insert(output); + } +}; + /** - * Select a set of coins such that nTargetValue is met and at least - * all coins from coin_control are selected; never select unconfirmed coins if they are not ours + * Fetch and validate coin control selected inputs. + * Coins could be internal (from the wallet) or external. +*/ +util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); + +/** + * Select a set of coins such that nTargetValue is met; never select unconfirmed coins if they are not ours * param@[in] wallet The wallet which provides data necessary to spend the selected coins * param@[in] available_coins The struct of coins, organized by OutputType, available for selection prior to filtering * param@[in] nTargetValue The target value @@ -132,9 +158,17 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons * returns If successful, a SelectionResult containing the selected coins * If failed, a nullopt. */ -std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, +std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); +/** + * Select all coins from coin_control, and if coin_control 'm_allow_other_inputs=true', call 'AutomaticCoinSelection' to + * select a set of coins such that nTargetValue - pre_set_inputs.total_amount is met. + */ +std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, + const CAmount& nTargetValue, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); + struct CreatedTransactionResult { CTransactionRef tx; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 23f024247d..f9c8c8ee9d 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -338,9 +338,13 @@ 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); + COutput select_coin = available_coins.All().at(0); + coin_control.Select(select_coin.outpoint); + PreSelectedInputs selected_input; + selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs); + available_coins.coins[OutputType::BECH32].erase(available_coins.coins[OutputType::BECH32].begin()); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); - const auto result10 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); + const auto result10 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(result10); } { @@ -363,7 +367,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) expected_result.Clear(); add_coin(10 * CENT, 2, expected_result); CCoinControl coin_control; - const auto result11 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); + const auto result11 = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result11)); available_coins.Clear(); @@ -378,7 +382,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) expected_result.Clear(); add_coin(9 * CENT, 2, expected_result); add_coin(1 * CENT, 2, expected_result); - const auto result12 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); + const auto result12 = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result12)); available_coins.Clear(); @@ -394,8 +398,12 @@ 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 - const auto result13 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); + COutput select_coin = available_coins.All().at(1); // pre select 9 coin + coin_control.Select(select_coin.outpoint); + PreSelectedInputs selected_input; + selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs); + available_coins.coins[OutputType::BECH32].erase(++available_coins.coins[OutputType::BECH32].begin()); + const auto result13 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result13)); } } @@ -783,7 +791,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) cs_params.m_cost_of_change = 1; cs_params.min_viable_change = 1; CCoinControl cc; - const auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params); + const auto result = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, target, cc, cs_params); BOOST_CHECK(result); BOOST_CHECK_GE(result->GetSelectedValue(), target); } @@ -965,7 +973,10 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test) cc.SetInputWeight(output.outpoint, 148); cc.SelectExternal(output.outpoint, output.txout); - const auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params); + const auto preset_inputs = *Assert(FetchSelectedInputs(*wallet, cc, cs_params)); + available_coins.coins[OutputType::BECH32].erase(available_coins.coins[OutputType::BECH32].begin()); + + const auto result = SelectCoins(*wallet, available_coins, preset_inputs, target, cc, cs_params); BOOST_CHECK(!result); } diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 17c6fce9c2..54b42667bb 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -406,7 +406,9 @@ class RawTransactionsTest(BitcoinTestFramework): def test_invalid_input(self): self.log.info("Test fundrawtxn with an invalid vin") - inputs = [ {'txid' : "1c7f966dab21119bac53213a2bc7532bff1fa844c124fd750a7d0b1332440bd1", 'vout' : 0} ] #invalid vin! + txid = "1c7f966dab21119bac53213a2bc7532bff1fa844c124fd750a7d0b1332440bd1" + vout = 0 + inputs = [ {'txid' : txid, 'vout' : vout} ] #invalid vin! outputs = { self.nodes[0].getnewaddress() : 1.0} rawtx = self.nodes[2].createrawtransaction(inputs, outputs) assert_raises_rpc_error(-4, "Unable to find UTXO for external input", self.nodes[2].fundrawtransaction, rawtx) @@ -1011,7 +1013,7 @@ class RawTransactionsTest(BitcoinTestFramework): # An external input without solving data should result in an error raw_tx = wallet.createrawtransaction([ext_utxo], {self.nodes[0].getnewaddress(): ext_utxo["amount"] / 2}) - assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, raw_tx) + assert_raises_rpc_error(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"]), wallet.fundrawtransaction, raw_tx) # Error conditions assert_raises_rpc_error(-5, "'not a pubkey' is not hex", wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["not a pubkey"]}}) @@ -1095,6 +1097,8 @@ class RawTransactionsTest(BitcoinTestFramework): # Expect: only preset inputs are used. # 5. Explicit add_inputs=true, no preset inputs (same as (1) but with an explicit set): # Expect: include inputs from the wallet. + # 6. Explicit add_inputs=false, no preset inputs: + # Expect: failure as we did not provide inputs and the process cannot automatically select coins. # Case (1), 'send' command # 'add_inputs' value is true unless "inputs" are specified, in such case, add_inputs=false. @@ -1146,6 +1150,10 @@ class RawTransactionsTest(BitcoinTestFramework): tx = wallet.send(outputs=[{addr1: 8}], options=options) assert tx["complete"] + # 6. Explicit add_inputs=false, no preset inputs: + options = {"add_inputs": False} + assert_raises_rpc_error(-4, "Insufficient funds", wallet.send, outputs=[{addr1: 3}], options=options) + ################################################ # Case (1), 'walletcreatefundedpsbt' command @@ -1187,6 +1195,10 @@ class RawTransactionsTest(BitcoinTestFramework): } assert "psbt" in wallet.walletcreatefundedpsbt(inputs=[], outputs=outputs, options=options) + # Case (6). Explicit add_inputs=false, no preset inputs: + options = {"add_inputs": False} + assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, inputs=[], outputs=outputs, options=options) + self.nodes[2].unloadwallet("test_preset_inputs") def test_weight_calculation(self): diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 3b78a7d095..b79b8f5187 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -657,7 +657,7 @@ class PSBTTest(BitcoinTestFramework): ext_utxo = self.nodes[0].listunspent(addresses=[addr])[0] # An external input without solving data should result in an error - assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 15}) + assert_raises_rpc_error(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"]), wallet.walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 15}) # But funding should work when the solving data is provided psbt = wallet.walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {"add_inputs": True, "solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"], addr_info["embedded"]["embedded"]["scriptPubKey"]]}}) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 07baa0595e..fb759c153d 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -508,7 +508,7 @@ class WalletSendTest(BitcoinTestFramework): ext_utxo = ext_fund.listunspent(addresses=[addr])[0] # An external input without solving data should result in an error - self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, expect_error=(-4, "Insufficient funds")) + self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, expect_error=(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"]))) # But funding should work when the solving data is provided res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"], addr_info["embedded"]["embedded"]["scriptPubKey"]]}) |