Skip to content

Commit 94c0766

Browse files
committed
wallet: skip available coins fetch if "other inputs" are disallowed
no need to waste resources calculating the wallet available coins if they are not going to be used. The 'm_allow_other_inputs=true` default value change is to correct an ugly misleading behavior: The tx creation process was having a workaround patch to automatically fall back to select coins from the wallet if `m_allow_other_inputs=false` (previous default value) and no manual inputs were selected. This could be seen in master in flows like `sendtoaddress`, `sendmany` and even the GUI, where the `m_allow_other_inputs` value isn't customized and the wallet still selects and adds coins to the tx internally.
1 parent fabc031 commit 94c0766

File tree

4 files changed

+27
-11
lines changed

4 files changed

+27
-11
lines changed

src/qt/sendcoinsdialog.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,9 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa
289289

290290
updateCoinControlState();
291291

292-
prepareStatus = model->prepareTransaction(*m_current_transaction, *m_coin_control);
292+
CCoinControl coin_control = *m_coin_control;
293+
coin_control.m_allow_other_inputs = !coin_control.HasSelected(); // future, could introduce a checkbox to customize this value.
294+
prepareStatus = model->prepareTransaction(*m_current_transaction, coin_control);
293295

294296
// process prepareStatus and on error generate message shown to user
295297
processSendCoinsReturn(prepareStatus,

src/wallet/coincontrol.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class CCoinControl
3737
bool m_include_unsafe_inputs = false;
3838
//! If true, the selection process can add extra unselected inputs from the wallet
3939
//! while requires all selected inputs be used
40-
bool m_allow_other_inputs = false;
40+
bool m_allow_other_inputs = true;
4141
//! Includes watch only addresses which are solvable
4242
bool fAllowWatchOnly = false;
4343
//! Override automatic min/max checks on fee, m_feerate must be set if true

src/wallet/spend.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
579579
}
580580

581581
// coin control -> return all selected outputs (we want all selected to go into the transaction for sure)
582-
if (coin_control.HasSelected() && !coin_control.m_allow_other_inputs) {
582+
if (!coin_control.m_allow_other_inputs) {
583583
SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL);
584584
result.AddInput(preset_inputs);
585585

@@ -893,14 +893,18 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
893893
const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size);
894894
CAmount selection_target = recipients_sum + not_input_fees;
895895

896-
// Get available coins
897-
auto available_coins = AvailableCoins(wallet,
898-
&coin_control,
899-
coin_selection_params.m_effective_feerate,
900-
1, /*nMinimumAmount*/
901-
MAX_MONEY, /*nMaximumAmount*/
902-
MAX_MONEY, /*nMinimumSumAmount*/
903-
0); /*nMaximumCount*/
896+
// Fetch wallet available coins if "other inputs" are
897+
// allowed (coins automatically selected by the wallet)
898+
CoinsResult available_coins;
899+
if (coin_control.m_allow_other_inputs) {
900+
available_coins = AvailableCoins(wallet,
901+
&coin_control,
902+
coin_selection_params.m_effective_feerate,
903+
1, /*nMinimumAmount*/
904+
MAX_MONEY, /*nMaximumAmount*/
905+
MAX_MONEY, /*nMinimumSumAmount*/
906+
0); /*nMaximumCount*/
907+
}
904908

905909
// Choose coins to use
906910
std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);

test/functional/rpc_fundrawtransaction.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,6 +1095,8 @@ def test_add_inputs_default_value(self):
10951095
# Expect: only preset inputs are used.
10961096
# 5. Explicit add_inputs=true, no preset inputs (same as (1) but with an explicit set):
10971097
# Expect: include inputs from the wallet.
1098+
# 6. Explicit add_inputs=false, no preset inputs:
1099+
# Expect: failure as we did not provide inputs and the process cannot automatically select coins.
10981100

10991101
# Case (1), 'send' command
11001102
# 'add_inputs' value is true unless "inputs" are specified, in such case, add_inputs=false.
@@ -1146,6 +1148,10 @@ def test_add_inputs_default_value(self):
11461148
tx = wallet.send(outputs=[{addr1: 8}], options=options)
11471149
assert tx["complete"]
11481150

1151+
# 6. Explicit add_inputs=false, no preset inputs:
1152+
options = {"add_inputs": False}
1153+
assert_raises_rpc_error(-4, "Insufficient funds", wallet.send, outputs=[{addr1: 3}], options=options)
1154+
11491155
################################################
11501156

11511157
# Case (1), 'walletcreatefundedpsbt' command
@@ -1187,6 +1193,10 @@ def test_add_inputs_default_value(self):
11871193
}
11881194
assert "psbt" in wallet.walletcreatefundedpsbt(inputs=[], outputs=outputs, options=options)
11891195

1196+
# Case (6). Explicit add_inputs=false, no preset inputs:
1197+
options = {"add_inputs": False}
1198+
assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, inputs=[], outputs=outputs, options=options)
1199+
11901200
self.nodes[2].unloadwallet("test_preset_inputs")
11911201

11921202
def test_weight_calculation(self):

0 commit comments

Comments
 (0)