diff options
| -rw-r--r-- | src/wallet/spend.cpp | 43 | ||||
| -rw-r--r-- | src/wallet/spend.h | 12 | ||||
| -rwxr-xr-x | test/functional/rpc_psbt.py | 4 | ||||
| -rwxr-xr-x | test/functional/wallet_fundrawtransaction.py | 20 | ||||
| -rwxr-xr-x | test/functional/wallet_send.py | 6 |
5 files changed, 59 insertions, 26 deletions
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 95fec6e96c..6bd9563eae 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -510,15 +510,20 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C return groups_out; } +// Returns true if the result contains an error and the message is not empty +static bool HasErrorMsg(const util::Result<SelectionResult>& res) { return !util::ErrorString(res).empty(); } + util::Result<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins, const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types) { // Run coin selection on each OutputType and compute the Waste Metric std::vector<SelectionResult> results; for (const auto& it : available_coins.coins) { - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}) { - results.push_back(*result); - } + auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}; + // If any specific error message appears here, then something particularly wrong happened. + if (HasErrorMsg(result)) return result; // So let's return the specific error. + // Append the favorable result. + if (result) results.push_back(*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 @@ -528,9 +533,7 @@ util::Result<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmo // over all available coins, which would allow mixing. // If TypesCount() <= 1, there is nothing to mix. if (allow_mixed_output_types && available_coins.TypesCount() > 1) { - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params)}) { - return result; - } + return ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params); } // 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 @@ -571,7 +574,8 @@ util::Result<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const }); if (eligible_results.empty()) { - return util::Error(); + return util::Error{_("The inputs size exceeds the maximum weight. " + "Please try sending a smaller amount or manually consolidating your wallet's UTXOs")}; } // Choose the result with the least waste @@ -588,7 +592,10 @@ util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& av CAmount selection_target = nTargetValue - pre_set_inputs.total_amount; // 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 util::Error(); + if (!coin_control.m_allow_other_inputs && selection_target > 0) { + return util::Error{_("The preselected coins total amount does not cover the transaction target. " + "Please allow other inputs to be automatically selected or include more coins manually")}; + } // Return if we can cover the target only with the preset inputs if (selection_target <= 0) { @@ -681,13 +688,23 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin } } - // Walk-through the filters until the solution gets found + // Walk-through the filters until the solution gets found. + // If no solution is found, return the first detailed error (if any). + // future: add "error level" so the worst one can be picked instead. + std::vector<util::Result<SelectionResult>> res_detailed_errors; for (const auto& select_filter : ordered_filters) { if (auto res{AttemptSelection(wallet, value_to_select, select_filter.filter, available_coins, - coin_selection_params, select_filter.allow_mixed_output_types)}) return res; + coin_selection_params, select_filter.allow_mixed_output_types)}) { + return res; // result found + } else { + // If any specific error message appears here, then something particularly wrong might have happened. + // Save the error and continue the selection process. So if no solutions gets found, we can return + // the detailed error to the upper layers. + if (HasErrorMsg(res)) res_detailed_errors.emplace_back(res); + } } // Coin Selection failed. - return util::Result<SelectionResult>(util::Error()); + return res_detailed_errors.empty() ? util::Result<SelectionResult>(util::Error()) : res_detailed_errors.front(); }(); return res; @@ -916,7 +933,9 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( // Choose coins to use auto select_coins_res = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); if (!select_coins_res) { - return util::Error{_("Insufficient funds")}; + // 'SelectCoins' either returns a specific error message or, if empty, means a general "Insufficient funds". + const bilingual_str& err = util::ErrorString(select_coins_res); + return util::Error{err.empty() ?_("Insufficient funds") : err}; } const SelectionResult& result = *select_coins_res; TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result.GetAlgo()).c_str(), result.GetTarget(), result.GetWaste(), result.GetSelectedValue()); diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 68a277f85c..7afab27acf 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -119,7 +119,9 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C * param@[in] coin_selection_params Parameters for the coin selection * param@[in] allow_mixed_output_types Relax restriction that SelectionResults must be of the same OutputType * returns If successful, a SelectionResult containing the input set - * If failed, a nullopt + * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds") + * or (2) an specific error message if there was something particularly wrong (e.g. a selection + * result that surpassed the tx max weight size). */ util::Result<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins, const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types); @@ -135,7 +137,9 @@ util::Result<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmo * param@[in] available_coins The struct of coins, organized by OutputType, available for selection prior to filtering * param@[in] coin_selection_params Parameters for the coin selection * returns If successful, a SelectionResult containing the input set - * If failed, a nullopt + * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds") + * or (2) an specific error message if there was something particularly wrong (e.g. a selection + * result that surpassed the tx max weight size). */ util::Result<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins, const CoinSelectionParams& coin_selection_params); @@ -175,7 +179,9 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const * param@[in] coin_selection_params Parameters for this coin selection such as feerates, whether to avoid partial spends, * and whether to subtract the fee from the outputs. * returns If successful, a SelectionResult containing the selected coins - * If failed, a nullopt. + * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds") + * or (2) an specific error message if there was something particularly wrong (e.g. a selection + * result that surpassed the tx max weight size). */ util::Result<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); diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index f6a697438a..40ae2fd122 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -120,7 +120,9 @@ class PSBTTest(BitcoinTestFramework): # If inputs are specified, do not automatically add more: utxo1 = self.nodes[0].listunspent()[0] - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[0].walletcreatefundedpsbt, [{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():90}) + assert_raises_rpc_error(-4, "The preselected coins total amount does not cover the transaction target. " + "Please allow other inputs to be automatically selected or include more coins manually", + self.nodes[0].walletcreatefundedpsbt, [{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():90}) psbtx1 = self.nodes[0].walletcreatefundedpsbt([{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():90}, 0, {"add_inputs": True})['psbt'] assert_equal(len(self.nodes[0].decodepsbt(psbtx1)['tx']['vin']), 2) diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index bf218bfee9..8451f45d13 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -27,6 +27,8 @@ from test_framework.util import ( ) from test_framework.wallet_util import bytes_to_wif +ERR_NOT_ENOUGH_PRESET_INPUTS = "The preselected coins total amount does not cover the transaction target. " \ + "Please allow other inputs to be automatically selected or include more coins manually" def get_unspent(listunspent, amount): for utx in listunspent: @@ -328,7 +330,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal("00", dec_tx['vin'][0]['scriptSig']['hex']) # Should fail without add_inputs: - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) # add_inputs is enabled by default rawtxfund = self.nodes[2].fundrawtransaction(rawtx) @@ -360,7 +362,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) # Should fail without add_inputs: - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True}) dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) @@ -394,7 +396,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) # Should fail without add_inputs: - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True}) dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) @@ -987,7 +989,9 @@ class RawTransactionsTest(BitcoinTestFramework): outputs[recipient.getnewaddress()] = 0.1 wallet.sendmany("", outputs) self.generate(self.nodes[0], 10) - assert_raises_rpc_error(-4, "Insufficient funds", recipient.fundrawtransaction, rawtx) + assert_raises_rpc_error(-4, "The inputs size exceeds the maximum weight. " + "Please try sending a smaller amount or manually consolidating your wallet's UTXOs", + recipient.fundrawtransaction, rawtx) self.nodes[0].unloadwallet("large") def test_external_inputs(self): @@ -1128,7 +1132,7 @@ class RawTransactionsTest(BitcoinTestFramework): } ] } - assert_raises_rpc_error(-4, "Insufficient funds", wallet.send, outputs=[{addr1: 8}], options=options) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.send, outputs=[{addr1: 8}], options=options) # Case (3), Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount) options["add_inputs"] = True @@ -1156,7 +1160,7 @@ class RawTransactionsTest(BitcoinTestFramework): # 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) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.send, outputs=[{addr1: 3}], options=options) ################################################ @@ -1173,7 +1177,7 @@ class RawTransactionsTest(BitcoinTestFramework): "vout": 1 # change position was hardcoded to index 0 }] outputs = {self.nodes[1].getnewaddress(): 8} - assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, inputs=inputs, outputs=outputs) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.walletcreatefundedpsbt, inputs=inputs, outputs=outputs) # Case (3), Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount) options["add_inputs"] = True @@ -1200,7 +1204,7 @@ class RawTransactionsTest(BitcoinTestFramework): # 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) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.walletcreatefundedpsbt, inputs=[], outputs=outputs, options=options) self.nodes[2].unloadwallet("test_preset_inputs") diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index eb7d9616a1..ce1de264b2 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -412,10 +412,12 @@ class WalletSendTest(BitcoinTestFramework): assert res["complete"] utxo1 = w0.listunspent()[0] assert_equal(utxo1["amount"], 50) + ERR_NOT_ENOUGH_PRESET_INPUTS = "The preselected coins total amount does not cover the transaction target. " \ + "Please allow other inputs to be automatically selected or include more coins manually" self.test_send(from_wallet=w0, to_wallet=w1, amount=51, inputs=[utxo1], - expect_error=(-4, "Insufficient funds")) + expect_error=(-4, ERR_NOT_ENOUGH_PRESET_INPUTS)) self.test_send(from_wallet=w0, to_wallet=w1, amount=51, inputs=[utxo1], add_inputs=False, - expect_error=(-4, "Insufficient funds")) + expect_error=(-4, ERR_NOT_ENOUGH_PRESET_INPUTS)) res = self.test_send(from_wallet=w0, to_wallet=w1, amount=51, inputs=[utxo1], add_inputs=True, add_to_wallet=False) assert res["complete"] |