diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -11,3 +11,10 @@ The `-salvagewallet` startup option has been removed. A new `salvage` command has been added to the `bitcoin-wallet` tool which performs the salvage operations that `-salvagewallet` did. + +# RPC changes + +The `walletcreatefundedpsbt` RPC call will now fail with `Insufficient funds` +when inputs are manually selected but are not enough to cover the outputs and +fee. Additional inputs can automatically be added through the new `add_inputs` +option. \ No newline at end of file diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -21,6 +21,8 @@ CTxDestination destChange; //! Override the default change type if set, ignored if destChange is set std::optional m_change_type; + //! If false, only selected inputs are used + bool m_add_inputs; //! If false, allows unselected inputs, but requires all selected inputs be //! used bool fAllowOtherInputs; diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp --- a/src/wallet/coincontrol.cpp +++ b/src/wallet/coincontrol.cpp @@ -9,6 +9,7 @@ void CCoinControl::SetNull() { destChange = CNoDestination(); m_change_type.reset(); + m_add_inputs = true; fAllowOtherInputs = false; fAllowWatchOnly = false; m_avoid_partial_spends = diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3700,12 +3700,12 @@ } void FundTransaction(CWallet *const pwallet, CMutableTransaction &tx, - Amount &fee_out, int &change_position, UniValue options) { + Amount &fee_out, int &change_position, UniValue options, + CCoinControl &coinControl) { // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - CCoinControl coinControl; change_position = -1; bool lockUnspents = false; UniValue subtractFeeFromOutputs; @@ -3720,6 +3720,7 @@ RPCTypeCheckObj( options, { + {"add_inputs", UniValueType(UniValue::VBOOL)}, {"changeAddress", UniValueType(UniValue::VSTR)}, {"changePosition", UniValueType(UniValue::VNUM)}, {"includeWatching", UniValueType(UniValue::VBOOL)}, @@ -3730,6 +3731,10 @@ }, true, true); + if (options.exists("add_inputs")) { + coinControl.m_add_inputs = options["add_inputs"].get_bool(); + } + if (options.exists("changeAddress")) { CTxDestination dest = DecodeDestination(options["changeAddress"].get_str(), @@ -3929,7 +3934,9 @@ Amount fee; int change_position; - FundTransaction(pwallet, tx, fee, change_position, request.params[1]); + CCoinControl coin_control; + FundTransaction(pwallet, tx, fee, change_position, request.params[1], + coin_control); UniValue result(UniValue::VOBJ); result.pushKV("hex", EncodeHexTx(CTransaction(tx))); @@ -4798,14 +4805,15 @@ RPCHelpMan{ "walletcreatefundedpsbt", "Creates and funds a transaction in the Partially Signed Transaction " - "format. Inputs will be added if supplied inputs are not enough\n" + "format.\n" "Implements the Creator and Updater roles.\n", { { "inputs", RPCArg::Type::ARR, RPCArg::Optional::NO, - "A json array of json objects", + "The inputs. Leave empty to add inputs automatically. See " + "add_inputs option.", { { "", @@ -4873,6 +4881,9 @@ RPCArg::Optional::OMITTED_NAMED_ARG, "", { + {"add_inputs", RPCArg::Type::BOOL, /* default */ "false", + "If inputs are specified, automatically include more if they " + "are not enough."}, {"changeAddress", RPCArg::Type::STR_HEX, /* default */ "pool address", "The bitcoin address to receive the change"}, @@ -4942,7 +4953,12 @@ CMutableTransaction rawTx = ConstructTransaction(wallet->GetChainParams(), request.params[0], request.params[1], request.params[2]); - FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]); + 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; + FundTransaction(pwallet, rawTx, fee, change_position, request.params[3], + coin_control); // Make a blank psbt PartiallySignedTransaction psbtx(rawTx); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2377,6 +2377,12 @@ } for (uint32_t i = 0; i < wtx.tx->vout.size(); i++) { + // Only consider selected coins if add_inputs is false + if (coinControl && !coinControl->m_add_inputs && + !coinControl->IsSelected(COutPoint(entry.first, i))) { + continue; + } + if (wtx.tx->vout[i].nValue < nMinimumAmount || wtx.tx->vout[i].nValue > nMaximumAmount) { continue; diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -11,8 +11,8 @@ from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( + assert_approx, assert_equal, - assert_greater_than, assert_raises_rpc_error, find_output, ) @@ -35,6 +35,19 @@ psbtx1 = self.nodes[0].walletcreatefundedpsbt( [], {self.nodes[2].getnewaddress(): 10})['psbt'] + # 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}) + + 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) + # Node 1 should not be able to add anything to it but still return the # psbtx same as before psbtx = self.nodes[1].walletprocesspsbt(psbtx1)['psbt'] @@ -84,26 +97,39 @@ self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex']) # feeRate of 0.1 BCH / KB produces a total fee slightly below -maxtxfee - res = self.nodes[1].walletcreatefundedpsbt([ - {"txid": txid, "vout": p2sh_pos}, - {"txid": txid, "vout": p2pkh_pos} - ], {self.nodes[1].getnewaddress(): 29.99}, 0, {"feeRate": 0.1}) - assert_greater_than(res["fee"], 0.06) - assert_greater_than(0.07, res["fee"]) + res = self.nodes[1].walletcreatefundedpsbt( + [ + { + "txid": txid, "vout": p2sh_pos}, { + "txid": txid, "vout": p2pkh_pos}], { + self.nodes[1].getnewaddress(): 29.99}, 0, { + "feeRate": 0.1, "add_inputs": True}) + assert_approx(res["fee"], 0.065, 0.005) # feeRate of 10 BCH / KB produces a total fee well above -maxtxfee # previously this was silently capped at -maxtxfee - assert_raises_rpc_error( - -4, - "Fee exceeds maximum configured by -maxtxfee", - self.nodes[1].walletcreatefundedpsbt, - [ - {"txid": txid, "vout": p2sh_pos}, - {"txid": txid, "vout": p2pkh_pos}, - ], - {self.nodes[1].getnewaddress(): 29.99}, - 0, - {"feeRate": 10}) + assert_raises_rpc_error(-4, + "Fee exceeds maximum configured by -maxtxfee", + self.nodes[1].walletcreatefundedpsbt, + [{"txid": txid, + "vout": p2sh_pos}, + {"txid": txid, + "vout": p2pkh_pos}], + {self.nodes[1].getnewaddress(): 29.99}, + 0, + {"feeRate": 10, + "add_inputs": True}) + assert_raises_rpc_error(-4, + "Fee exceeds maximum configured by -maxtxfee", + self.nodes[1].walletcreatefundedpsbt, + [{"txid": txid, + "vout": p2sh_pos}, + {"txid": txid, + "vout": p2pkh_pos}], + {self.nodes[1].getnewaddress(): 1}, + 0, + {"feeRate": 10, + "add_inputs": False}) # partially sign multisig things with node 1 psbtx = self.nodes[1].walletcreatefundedpsbt([{"txid": txid, "vout": p2sh_pos}], { @@ -204,7 +230,7 @@ # Regression test for 14473 (mishandling of already-signed # transaction): psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid": unspent["txid"], "vout":unspent["vout"]}], [ - {self.nodes[2].getnewaddress():unspent["amount"] + 1}]) + {self.nodes[2].getnewaddress():unspent["amount"] + 1}], 0, {"add_inputs": True}) complete_psbt = self.nodes[0].walletprocesspsbt(psbtx_info["psbt"]) double_processed_psbt = self.nodes[0].walletprocesspsbt( complete_psbt["psbt"])