Merge bc28ca3afb into merged_master (Bitcoin PR bitcoin/bitcoin#25118)

Please review this commit carefully. It touched wallet code.
This commit is contained in:
James Dorfman 2024-09-16 01:08:02 +00:00
commit 034a29075f
5 changed files with 28 additions and 54 deletions

View file

@ -36,12 +36,11 @@ public:
std::map<CAsset, CTxDestination> destChange{};
//! Override the default change type if set, ignored if destChange is set
std::optional<OutputType> m_change_type;
//! If false, only selected inputs are used
bool m_add_inputs = true;
//! If false, only safe inputs will be used
bool m_include_unsafe_inputs = false;
//! If false, allows unselected inputs, but requires all selected inputs be used
bool fAllowOtherInputs = 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;
//! Includes watch only addresses which are solvable
bool fAllowWatchOnly = false;
//! Override automatic min/max checks on fee, m_feerate must be set if true

View file

@ -230,7 +230,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
for (const auto& inputs : wtx.tx->vin) {
new_coin_control.Select(COutPoint(inputs.prevout));
}
new_coin_control.fAllowOtherInputs = true;
new_coin_control.m_allow_other_inputs = true;
// We cannot source new unconfirmed inputs(bip125 rule 2)
new_coin_control.m_min_depth = 1;

View file

@ -590,8 +590,8 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
},
true, true);
if (options.exists("add_inputs") ) {
coinControl.m_add_inputs = options["add_inputs"].get_bool();
if (options.exists("add_inputs")) {
coinControl.m_allow_other_inputs = options["add_inputs"].get_bool();
}
if (options.exists("changeAddress") || options.exists("change_address")) {
@ -925,7 +925,7 @@ RPCHelpMan fundrawtransaction()
int change_position;
CCoinControl coin_control;
// Automatically select (additional) coins. Can be overridden by options.add_inputs.
coin_control.m_add_inputs = true;
coin_control.m_allow_other_inputs = true;
FundTransaction(*pwallet, tx, fee, change_position, request.params[1], coin_control, request.params[3], /*override_min_fee=*/true);
UniValue result(UniValue::VOBJ);
@ -1330,7 +1330,7 @@ RPCHelpMan send()
CCoinControl coin_control;
// Automatically select coins, unless at least one is manually selected. Can
// be overridden by options.add_inputs.
coin_control.m_add_inputs = rawTx.vin.size() == 0;
coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
UniValue solving_data = NullUniValue;
if (options.exists("solving_data")) {
solving_data = options["solving_data"].get_obj();
@ -1871,7 +1871,7 @@ RPCHelpMan walletcreatefundedpsbt()
CCoinControl coin_control;
// Automatically select coins, unless at least one is manually selected. Can
// be overridden by options.add_inputs.
coin_control.m_add_inputs = rawTx.vin.size() == 0;
coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
// FundTransaction expects blinding keys, if present, to appear in the output nonces
for (CTxOut& txout : rawTx.vout) {
auto search_it = psbt_outs.find(txout);

View file

@ -265,11 +265,6 @@ CoinsResult AvailableCoins(const CWallet& wallet,
const CTxOut& output = wtx.tx->vout[i];
const COutPoint outpoint(wtxid, i);
// Only consider selected coins if add_inputs is false
if (coinControl && !coinControl->m_add_inputs && !coinControl->IsSelected(outpoint)) {
continue;
}
CAmount outValue = wtx.GetOutputValueOut(wallet, i);
CAsset asset = wtx.GetOutputAsset(wallet, i);
uint256 bfValue = wtx.GetOutputAmountBlindingFactor(wallet, i);
@ -280,7 +275,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
if (outValue < nMinimumAmount || outValue > nMaximumAmount)
continue;
if (coinControl && coinControl->HasSelected() && !coinControl->fAllowOtherInputs && !coinControl->IsSelected(outpoint))
if (coinControl && coinControl->HasSelected() && !coinControl->m_allow_other_inputs && !coinControl->IsSelected(outpoint))
continue;
if (wallet.IsLockedCoin(outpoint))
@ -584,31 +579,6 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
OutputGroup preset_inputs(coin_selection_params);
// coin control -> return all selected outputs (we want all selected to go into the transaction for sure)
if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs)
{
for (const COutput& out : vCoins)
{
if (!out.spendable) {
continue;
}
CAmount amt = out.value;
if (amt < 0) {
continue;
}
/* Set ancestors and descendants to 0 as these don't matter for preset inputs as 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(out, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false);
}
SelectionResult result(mapTargetValue, SelectionAlgorithm::MANUAL);
result.AddInput(preset_inputs);
if (result.GetSelectedValue() < mapTargetValue) return std::nullopt;
result.ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
return result;
}
// calculate value from preset inputs and store them
std::set<COutPoint> preset_coins;
@ -617,23 +587,21 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
for (const COutPoint& outpoint : vPresetInputs) {
int input_bytes = -1;
CTxOut txout;
std::map<uint256, CWalletTx>::const_iterator it = wallet.mapWallet.find(outpoint.hash);
/* 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 (it != wallet.mapWallet.end()) {
const CWalletTx& wtx = it->second;
auto ptr_wtx = wallet.GetWalletTx(outpoint.hash);
if (ptr_wtx) {
// Clearly invalid input, fail
if (wtx.tx->vout.size() <= outpoint.n) {
if (ptr_wtx->tx->vout.size() <= outpoint.n) {
return std::nullopt;
}
// Just to calculate the marginal byte size
if (GetTxSpendSize(wallet, wtx, outpoint.n, outpoint.n) < 0) {
if (GetTxSpendSize(wallet, *ptr_wtx, outpoint.n, outpoint.n) < 0) {
continue;
}
input_bytes = GetTxSpendSize(wallet, wtx, outpoint.n, false);
txout = wtx.tx->vout.at(outpoint.n);
output = COutput(wallet, wtx, outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate);
input_bytes = GetTxSpendSize(wallet, *ptr_wtx, outpoint.n, false);
txout = ptr_wtx->tx->vout.at(outpoint.n);
output = COutput(wallet, *ptr_wtx, outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate);
} else {
// The input is external. We did not find the tx in mapWallet.
if (!coin_control.GetExternalOutput(outpoint, txout)) {
@ -675,6 +643,15 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
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) {
SelectionResult result(mapTargetValue, SelectionAlgorithm::MANUAL);
result.AddInput(preset_inputs);
if (result.GetSelectedValue() < mapTargetValue) return std::nullopt;
result.ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
return result;
}
// remove preset inputs from vCoins so that Coin Selection doesn't pick them.
for (std::vector<COutput>::iterator it = vCoins.begin(); it != vCoins.end() && coin_control.HasSelected();)
{
@ -1877,8 +1854,6 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
vecSend.push_back(recipient);
}
coinControl.fAllowOtherInputs = true;
// Acquire the locks to prevent races to the new locked unspents between the
// CreateTransaction call and LockCoin calls (when lockUnspents is true).
LOCK(wallet.cs_wallet);

View file

@ -344,7 +344,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
add_coin(coins, *wallet, 3 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
add_coin(coins, *wallet, 2 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
CCoinControl coin_control;
coin_control.fAllowOtherInputs = true;
coin_control.m_allow_other_inputs = true;
coin_control.Select(coins.at(0).outpoint);
coin_selection_params_bnb.m_effective_feerate = CFeeRate(0);
CAmountMap mapTargetValue;
@ -404,7 +404,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);
coin_control.fAllowOtherInputs = true;
coin_control.m_allow_other_inputs = true;
coin_control.Select(coins.at(1).outpoint); // pre select 9 coin
const auto result13 = SelectCoins(*wallet, coins, mapTargetValue, coin_control, coin_selection_params_bnb);
BOOST_CHECK(EquivalentResult(expected_result, *result13));