diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 146ed5cefe8c..5d53ce8be5de 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -328,7 +328,9 @@ bool SendCoinsDialog::send(const QList& recipients, QString& 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 e1d59b3bc76e..7fe4f5693543 100644 --- a/src/test/util/wallet.cpp +++ b/src/test/util/wallet.cpp @@ -25,7 +25,13 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq #ifdef ENABLE_WALLET std::string getnewaddress(CWallet& w) { - return EncodeDestination(*Assert(w.GetNewDestination(""))); + return EncodeDestination(getNewDestination(w, OutputType::LEGACY)); +} + +CTxDestination getNewDestination(CWallet& w, OutputType /* output_type */) +{ + // Dash only supports LEGACY output type (no SegWit) + return *Assert(w.GetNewDestination("")); } // void importaddress(CWallet& wallet, const std::string& address) diff --git a/src/test/util/wallet.h b/src/test/util/wallet.h index d8a81c2d8224..0411125a9243 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 #include namespace wallet { @@ -20,8 +21,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 83b446d75367..32ce7c2abfdb 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -48,7 +48,7 @@ class CCoinControl 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; //! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with m_allow_other_inputs bool fRequireAllInputs = true; //! Includes watch only addresses which are solvable diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 8a5d6bf79d03..7fa010d74da2 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -500,6 +500,22 @@ void SelectionResult::AddInput(const OutputGroup& group) m_use_effective = !group.m_subtract_fee_outputs; } +void SelectionResult::AddInputs(const std::set& 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; + m_use_effective |= other.m_use_effective; + if (m_algo == SelectionAlgorithm::MANUAL) { + m_algo = other.m_algo; + } + util::insert(m_selected_inputs, other.m_selected_inputs); +} + const std::set& SelectionResult::GetInputSet() const { return m_selected_inputs; diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 9cae820a9eeb..0b03fc0101ff 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -284,9 +284,9 @@ struct SelectionResult public: /** The target the algorithm selected for. Note that this may not be equal to the recipient amount as it can include non-input fees */ - const CAmount m_target; + CAmount m_target; /** The algorithm used to produce this result */ - const SelectionAlgorithm m_algo; + SelectionAlgorithm m_algo; explicit SelectionResult(const CAmount target, SelectionAlgorithm algo) : m_target(target), m_algo(algo) {} @@ -299,6 +299,10 @@ struct SelectionResult void Clear(); void AddInput(const OutputGroup& group); + void AddInputs(const std::set& inputs, bool subtract_fee_outputs); + + /** Combine another SelectionResult into this one */ + void Merge(const SelectionResult& other); /** Calculates and stores the waste for this selection via GetSelectionWaste */ void ComputeAndSetWaste(CAmount change_cost); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 1de3acd1e16b..17d31c8c8825 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -97,6 +97,46 @@ void CoinsResult::clear() other.clear(); } +// Fetch and validate the coin control selected inputs. +// Coins could be internal (from the wallet) or external. +util::Result FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +{ + PreSelectedInputs result; + std::vector 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 (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 feerate, @@ -185,7 +225,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) && nCoinType != CoinType::ONLY_MASTERNODE_COLLATERAL) @@ -511,95 +552,42 @@ std::optional ChooseSelectionResult(const CWallet& wallet, cons return best_result; } -std::optional SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) +std::optional SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, + const CAmount& nTargetValue, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) { - // Note: this function should never be used for "always free" tx types like dstx - CoinType nCoinType = coin_control.nCoinType; - 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 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 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; - } - input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control); - } - if (nCoinType == CoinType::ONLY_FULLY_MIXED) { - // Make sure to include mixed preset inputs only, - // even if some non-mixed inputs were manually selected via CoinControl - if (!wallet.IsFullyMixed(outpoint)) continue; - } - 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); - bool all_inputs{coin_control.fRequireAllInputs}; - if (!all_inputs) { - // Calculate the smallest set of inputs required to meet nTargetValue from available_coins - bool success{false}; - OutputGroup preset_candidates(coin_selection_params); - for (const COutput& out : available_coins.all()) { - if (!out.spendable) continue; - if (preset_coins.count(out.outpoint)) { - preset_candidates.Insert(out, /*ancestors=*/0, /*descendants=*/0, /*positive_only=*/false); - } - if (preset_candidates.GetSelectionAmount() >= nTargetValue) { - result.AddInput(preset_candidates); - success = true; - break; - } - } - // Couldn't meet target, add all inputs - if (!success) all_inputs = true; - } - if (all_inputs) { - result.AddInput(preset_inputs); - } - 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.m_cost_of_change); return result; } - // 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.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()); + // 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.m_cost_of_change); } + return op_selection_result; +} + +std::optional AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) +{ + // Note: this function should never be used for "always free" tx types like dstx + CoinType nCoinType = coin_control.nCoinType; unsigned int limit_ancestor_count = 0; unsigned int limit_descendant_count = 0; @@ -617,13 +605,11 @@ std::optional SelectCoins(const CWallet& wallet, CoinsResult& a Shuffle(available_coins.legacy.begin(), available_coins.legacy.end(), coin_selection_params.rng_fast); Shuffle(available_coins.other.begin(), available_coins.other.end(), coin_selection_params.rng_fast); } + // 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 res = [&] { - // Pre-selected inputs already cover the target amount. - if (value_to_select <= 0) return std::make_optional(SelectionResult(nTargetValue, 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, nCoinType)}) return r1; @@ -673,14 +659,6 @@ std::optional SelectCoins(const CWallet& wallet, CoinsResult& a return std::optional(); }(); - if (!res) return std::nullopt; - - // Add preset inputs to result - res->AddInput(preset_inputs); - if (res->m_algo == SelectionAlgorithm::MANUAL) { - res->ComputeAndSetWaste(coin_selection_params.m_cost_of_change); - } - return res; } @@ -884,23 +862,29 @@ static util::Result 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; - // This can only happen if feerate is 0, and requested destinations are value of 0 (e.g. OP_RETURN) - // and no pre-selected inputs. This will result in 0-input transaction, which is consensus-invalid anyways - if (selection_target == 0 && !coin_control.HasSelected()) { - return util::Error{_("Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input")}; + // 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; } - // 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 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 result = SelectCoins(wallet, available_coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); + std::optional result = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); if (!result) { if (coin_control.nCoinType == CoinType::ONLY_NONDENOMINATED) { return util::Error{_("Unable to locate enough non-denominated funds for this transaction.")}; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 41f2e828910b..93e5c56a1c8d 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -122,9 +122,35 @@ std::optional AttemptSelection(const CWallet& wallet, const CAm std::optional ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector& available_coins, const CoinSelectionParams& coin_selection_params, CoinType nCoinType = CoinType::ALL_COINS); +// User manually selected inputs that must be part of the transaction +struct PreSelectedInputs +{ + std::set 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 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 @@ -133,9 +159,17 @@ std::optional ChooseSelectionResult(const CWallet& wallet, cons * returns If successful, a SelectionResult containing the selected coins * If failed, a nullopt. */ -std::optional SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, +std::optional 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 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 2d567548412f..97767229863f 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -332,9 +332,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.legacy.erase(available_coins.legacy.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); } { @@ -357,7 +361,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(); @@ -372,7 +376,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); // NOTE: Dash does not use BnB and therefore, this check will fail // BOOST_CHECK(EquivalentResult(expected_result, *result12)); available_coins.clear(); @@ -389,8 +393,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.legacy.erase(++available_coins.legacy.begin()); + const auto result13 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result13)); } } @@ -776,7 +784,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) /*avoid_partial=*/ false, }; 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); } @@ -916,7 +924,7 @@ BOOST_AUTO_TEST_CASE(effective_value_test) } /* --------------------------- Dash-specific tests start here --------------------------- */ -BOOST_AUTO_TEST_CASE(minimum_inputs_test) +BOOST_AUTO_TEST_CASE(preset_inputs_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateMockWalletDatabase()); wallet->LoadWallet(); @@ -924,18 +932,17 @@ BOOST_AUTO_TEST_CASE(minimum_inputs_test) wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet->SetupDescriptorScriptPubKeyMans("", ""); - // Create coins (denominations) for a target that can be met without consuming all the coins + // Create coins CoinsResult available_coins{}; CAmount target{25 * COIN}; add_coin(available_coins, *wallet, 9 * COIN, CFeeRate(0), /*nAge=*/6*24, /*fIsFromMe=*/false, /*nInput=*/0, /*spendable=*/true); add_coin(available_coins, *wallet, 16 * COIN, CFeeRate(0), /*nAge=*/6*24, /*fIsFromMe=*/false, /*nInput=*/0, /*spendable=*/true); add_coin(available_coins, *wallet, 24 * COIN, CFeeRate(0), /*nAge=*/6*24, /*fIsFromMe=*/false, /*nInput=*/0, /*spendable=*/true); - // Setup coin control to select from the given coins (!m_allow_other_inputs) *but* consume as little - // as possible (!fRequireAllInputs) and select our coins. + // Setup coin control to select only preset inputs (!m_allow_other_inputs) + // When m_allow_other_inputs=false, all preset inputs are used CCoinControl coin_control{}; coin_control.m_allow_other_inputs = false; - coin_control.fRequireAllInputs = false; for (const auto& coin : available_coins.all()) { coin_control.Select(coin.outpoint); } @@ -953,12 +960,19 @@ BOOST_AUTO_TEST_CASE(minimum_inputs_test) /*tx_noinputs_size=*/0, /*avoid_partial=*/false, }; - const auto result = SelectCoins(*wallet, available_coins, target, coin_control, coin_selection_params); + // First fetch the pre-selected inputs + const auto preset_inputs_result = FetchSelectedInputs(*wallet, coin_control, coin_selection_params); + BOOST_REQUIRE(preset_inputs_result); + const auto& preset_inputs = *preset_inputs_result; + + // Clear available coins since we only want to use preset inputs + CoinsResult empty_coins{}; + const auto result = SelectCoins(*wallet, empty_coins, preset_inputs, target, coin_control, coin_selection_params); BOOST_REQUIRE(result); - // Should consume only the first two coins (9 + 16) >= 25 and account correctly - BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2); - BOOST_CHECK_EQUAL(result->GetSelectedValue(), 25 * COIN); + // With the new architecture, all preset inputs are used when m_allow_other_inputs=false + BOOST_CHECK_EQUAL(result->GetInputSet().size(), 3); + BOOST_CHECK_EQUAL(result->GetSelectedValue(), 49 * COIN); } BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index de78585d0746..ee484ce4c9fa 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -387,10 +387,12 @@ def test_two_vin_two_vout(self): 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() : 10} rawtx = self.nodes[2].createrawtransaction(inputs, outputs) - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx) + assert_raises_rpc_error(-4, "Not found pre-selected input COutPoint(%s, %s)" % (txid[0:10], vout), self.nodes[2].fundrawtransaction, rawtx) def test_fee_p2pkh(self): """Compare fee of a standard pubkeyhash transaction.""" @@ -968,8 +970,8 @@ def test_external_inputs(self): ext_utxo = self.nodes[0].listunspent(addresses=[addr])[0] # An external input without solving data should result in an error - raw_tx = wallet.createrawtransaction([ext_utxo], {self.nodes[0].getnewaddress(): 15}) - assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, raw_tx) + raw_tx = wallet.createrawtransaction([ext_utxo], {self.nodes[0].getnewaddress(): ext_utxo["amount"] / 2}) + 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"]}}) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index d7b6678722c5..06042e269e78 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -484,7 +484,7 @@ def test_psbt_input_keys(psbt_input, keys): 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", self.nodes[1].walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 10 + ext_utxo['amount']}, 0, {'add_inputs': True}) + assert_raises_rpc_error(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"]), self.nodes[1].walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {'add_inputs': True}) # But funding should work when the solving data is provided psbt = self.nodes[1].walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {'add_inputs': True, "solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}}) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index df64548cc677..6460c29d78ee 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -488,7 +488,7 @@ def run_test(self): 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"]]})