diff --git a/doc/release-notes-5861.md b/doc/release-notes-5861.md new file mode 100644 index 000000000000..47d998132f86 --- /dev/null +++ b/doc/release-notes-5861.md @@ -0,0 +1,14 @@ +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. + +- The `fundrawtransaction` RPC now supports `add_inputs` option that when `false` + prevents adding more inputs if necessary and consequently the RPC fails. + +- A new `send` RPC with similar syntax to `walletcreatefundedpsbt`, including + support for coin selection and a custom fee rate. The `send` RPC is experimental + and may change in subsequent releases. Using it is encouraged once it's no + longer experimental: `sendmany` and `sendtoaddress` may be deprecated in a future release. diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index c59d4244fd6e..d787647bb64d 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -146,6 +146,9 @@ static const CRPCConvertParam vRPCConvertParams[] = { "gettxoutsetinfo", 2, "use_index"}, { "lockunspent", 0, "unlock" }, { "lockunspent", 1, "transactions" }, + { "send", 0, "outputs" }, + { "send", 1, "conf_target" }, + { "send", 3, "options" }, { "importprivkey", 2, "rescan" }, { "importelectrumwallet", 1, "index" }, { "importaddress", 2, "rescan" }, diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 67c0421926ae..11df0c274ef6 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -21,10 +21,17 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime) { - if (inputs_in.isNull() || outputs_in.isNull()) - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, arguments 1 and 2 must be non-null"); + if (outputs_in.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument must be non-null"); + } + + UniValue inputs; + if (inputs_in.isNull()) { + inputs = UniValue::VARR; + } else { + inputs = inputs_in.get_array(); + } - UniValue inputs = inputs_in.get_array(); const bool outputs_is_obj = outputs_in.isObject(); UniValue outputs = outputs_is_obj ? outputs_in.get_obj() : outputs_in.get_array(); diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp index 2778a566a324..f21ff8d3bf21 100644 --- a/src/wallet/coincontrol.cpp +++ b/src/wallet/coincontrol.cpp @@ -9,6 +9,7 @@ void CCoinControl::SetNull(bool fResetCoinType) { destChange = CNoDestination(); + m_add_inputs = true; fAllowOtherInputs = false; fAllowWatchOnly = false; m_avoid_partial_spends = gArgs.GetBoolArg("-avoidpartialspends", DEFAULT_AVOIDPARTIALSPENDS); @@ -26,4 +27,3 @@ void CCoinControl::SetNull(bool fResetCoinType) nCoinType = CoinType::ALL_COINS; } } - diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 221f5a8e394c..920e9d606b6d 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -37,6 +37,8 @@ class CCoinControl { public: CTxDestination destChange; + //! If false, only selected inputs are used + bool m_add_inputs; //! If false, allows unselected inputs, but requires all selected inputs be used if fAllowOtherInputs is true (default) bool fAllowOtherInputs; //! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with fAllowOtherInputs diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b438ddb27b87..727612554256 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -3234,13 +3235,12 @@ static UniValue listunspent(const JSONRPCRequest& request) return results; } -void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options) +void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& 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; @@ -3255,34 +3255,52 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f RPCTypeCheckArgument(options, UniValue::VOBJ); RPCTypeCheckObj(options, { + {"add_inputs", UniValueType(UniValue::VBOOL)}, + {"add_to_wallet", UniValueType(UniValue::VBOOL)}, {"changeAddress", UniValueType(UniValue::VSTR)}, + {"change_address", UniValueType(UniValue::VSTR)}, {"changePosition", UniValueType(UniValue::VNUM)}, + {"change_position", UniValueType(UniValue::VNUM)}, {"includeWatching", UniValueType(UniValue::VBOOL)}, + {"include_watching", UniValueType(UniValue::VBOOL)}, + {"inputs", UniValueType(UniValue::VARR)}, {"lockUnspents", UniValueType(UniValue::VBOOL)}, + {"lock_unspents", UniValueType(UniValue::VBOOL)}, + {"locktime", UniValueType(UniValue::VNUM)}, {"feeRate", UniValueType()}, // will be checked below + {"psbt", UniValueType(UniValue::VBOOL)}, {"subtractFeeFromOutputs", UniValueType(UniValue::VARR)}, + {"subtract_fee_from_outputs", UniValueType(UniValue::VARR)}, {"conf_target", UniValueType(UniValue::VNUM)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, }, true, true); - if (options.exists("changeAddress")) { - CTxDestination dest = DecodeDestination(options["changeAddress"].get_str()); + if (options.exists("add_inputs") ) { + coinControl.m_add_inputs = options["add_inputs"].get_bool(); + } + + if (options.exists("changeAddress") || options.exists("change_address")) { + const std::string change_address_str = (options.exists("change_address") ? options["change_address"] : options["changeAddress"]).get_str(); + CTxDestination dest = DecodeDestination(change_address_str); if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "changeAddress must be a valid Dash address"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Change address must be a valid Dash address"); } coinControl.destChange = dest; } - if (options.exists("changePosition")) - change_position = options["changePosition"].get_int(); + if (options.exists("changePosition") || options.exists("change_position")) { + change_position = (options.exists("change_position") ? options["change_position"] : options["changePosition"]).get_int(); + } - coinControl.fAllowWatchOnly = ParseIncludeWatchonly(options["includeWatching"], *pwallet); + const UniValue include_watching_option = options.exists("include_watching") ? options["include_watching"] : options["includeWatching"]; + coinControl.fAllowWatchOnly = ParseIncludeWatchonly(include_watching_option, *pwallet); - if (options.exists("lockUnspents")) - lockUnspents = options["lockUnspents"].get_bool(); + if (options.exists("lockUnspents") || options.exists("lock_unspents")) { + lockUnspents = (options.exists("lock_unspents") ? options["lock_unspents"] : options["lockUnspents"]).get_bool(); + } if (options.exists("feeRate")) { @@ -3296,8 +3314,8 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f coinControl.fOverrideFeeRate = true; } - if (options.exists("subtractFeeFromOutputs")) - subtractFeeFromOutputs = options["subtractFeeFromOutputs"].get_array(); + if (options.exists("subtractFeeFromOutputs") || options.exists("subtract_fee_from_outputs") ) + subtractFeeFromOutputs = (options.exists("subtract_fee_from_outputs") ? options["subtract_fee_from_outputs"] : options["subtractFeeFromOutputs"]).get_array(); SetFeeEstimateMode(pwallet, coinControl, options["estimate_mode"], options["conf_target"]); @@ -3334,8 +3352,8 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f static UniValue fundrawtransaction(const JSONRPCRequest& request) { RPCHelpMan{"fundrawtransaction", - "\nAdd inputs to a transaction until it has enough in value to meet its out value.\n" - "This will not modify existing inputs, and will add at most one change output to the outputs.\n" + "\nIf the transaction has no inputs, they will be automatically selected to meet its out value.\n" + "It will add at most one change output to the outputs.\n" "No existing outputs will be modified unless \"subtractFeeFromOutputs\" is specified.\n" "Note that inputs which were signed may need to be resigned after completion since in/outputs have been added.\n" "The inputs added will not be signed, use signrawtransactionwithkey\n" @@ -3349,6 +3367,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", { + {"add_inputs", RPCArg::Type::BOOL, /* default */ "true", "For a transaction with existing inputs, automatically include more if they are not enough."}, {"changeAddress", RPCArg::Type::STR, /* default */ "pool address", "The Dash address to receive the change"}, {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only.\n" @@ -3404,7 +3423,10 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) CAmount fee; int change_position; - FundTransaction(pwallet, tx, fee, change_position, request.params[1]); + CCoinControl coin_control; + // Automatically select (additional) coins. Can be overriden by options.add_inputs. + coin_control.m_add_inputs = true; + FundTransaction(pwallet, tx, fee, change_position, request.params[1], coin_control); UniValue result(UniValue::VOBJ); result.pushKV("hex", EncodeHexTx(CTransaction(tx))); @@ -3981,6 +4003,175 @@ static UniValue listlabels(const JSONRPCRequest& request) return ret; } +static UniValue send(const JSONRPCRequest& request) +{ + RPCHelpMan{"send", + "\nEXPERIMENTAL warning: this call may be changed in future releases.\n" + "\nSend a transaction.\n", + { + {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "A JSON array with outputs (key-value pairs), where none of the keys are duplicated.\n" + "That is, each address can only appear once and there can only be one 'data' object.\n" + "For convenience, a dictionary, which holds the key-value pairs directly, is also accepted.", + { + {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + { + {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the Dash address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""}, + }, + }, + {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + { + {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"}, + }, + }, + }, + }, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, + {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" + " \"" + FeeModes("\"\n\"") + "\""}, + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + { + {"add_inputs", RPCArg::Type::BOOL, /* default */ "false", "If inputs are specified, automatically include more if they are not enough."}, + {"add_to_wallet", RPCArg::Type::BOOL, /* default */ "true", "When false, returns a serialized transaction which will not be added to the wallet or broadcast"}, + {"change_address", RPCArg::Type::STR_HEX, /* default */ "pool address", "The Dash address to receive the change"}, + {"change_position", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, + {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" + " \"" + FeeModes("\"\n\"") + "\""}, + {"include_watching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only.\n" + "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" + "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, + {"inputs", RPCArg::Type::ARR, /* default */ "empty array", "Specify inputs instead of adding them automatically. A JSON array of JSON objects", + { + {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, + {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"}, + {"sequence", RPCArg::Type::NUM, RPCArg::Optional::NO, "The sequence number"}, + }, + }, + {"locktime", RPCArg::Type::NUM, /* default */ "0", "Raw locktime. Non-0 value also locktime-activates inputs"}, + {"lock_unspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, + {"psbt", RPCArg::Type::BOOL, /* default */ "automatic", "Always return a PSBT, implies add_to_wallet=false."}, + {"subtract_fee_from_outputs", RPCArg::Type::ARR, /* default */ "empty array", "A JSON array of integers.\n" + "The fee will be equally deducted from the amount of each specified output.\n" + "Those recipients will receive less funds than you enter in their corresponding amount field.\n" + "If no outputs are specified here, the sender pays the fee.", + { + {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, + }, + }, + }, + "options"}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, + {RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of the number of addresses."}, + {RPCResult::Type::STR_HEX, "hex", "If add_to_wallet is false, the hex-encoded raw transaction with signature(s)"}, + {RPCResult::Type::STR, "psbt", "If more signatures are needed, or if add_to_wallet is false, the base64-encoded (partially) signed transaction"} + } + }, + RPCExamples{"" + "\nSend with a fee rate of 1 " + CURRENCY_ATOM + "/B\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.1}' 1 " + CURRENCY_ATOM + "/B\n") + + "\nCreate a transaction that should confirm the next block, with a specific input, and return result without adding to wallet or broadcasting to the network\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.1}' 1 economical '{\"add_to_wallet\": false, \"inputs\": [{\"txid\":\"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0\", \"vout\":1}]}'") + } + }.Check(request); + + RPCTypeCheck(request.params, { + UniValueType(), // ARR or OBJ, checked later + UniValue::VNUM, + UniValue::VSTR, + UniValue::VOBJ + }, true + ); + + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + + UniValue options = request.params[3]; + if (options.exists("feeRate") || options.exists("fee_rate") || options.exists("estimate_mode") || options.exists("conf_target")) { + if (!request.params[1].isNull() || !request.params[2].isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use either conf_target and estimate_mode or the options dictionary to control fee rate"); + } + } else { + options.pushKV("conf_target", request.params[1]); + options.pushKV("estimate_mode", request.params[2]); + } + if (!options["conf_target"].isNull() && (options["estimate_mode"].isNull() || (options["estimate_mode"].get_str() == "unset"))) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Specify estimate_mode"); + } + if (options.exists("changeAddress")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_address"); + } + if (options.exists("changePosition")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_position"); + } + if (options.exists("includeWatching")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use include_watching"); + } + if (options.exists("lockUnspents")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use lock_unspents"); + } + if (options.exists("subtractFeeFromOutputs")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use subtract_fee_from_outputs"); + } + + const bool psbt_opt_in = options.exists("psbt") && options["psbt"].get_bool(); + + CAmount fee; + int change_position; + CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"]); + CCoinControl coin_control; + // Automatically select coins, unless at least one is manually selected. Can + // be overriden by options.add_inputs. + coin_control.m_add_inputs = rawTx.vin.size() == 0; + FundTransaction(pwallet, rawTx, fee, change_position, options, coin_control); + + bool add_to_wallet = true; + if (options.exists("add_to_wallet")) { + add_to_wallet = options["add_to_wallet"].get_bool(); + } + + // Make a blank psbt + PartiallySignedTransaction psbtx(rawTx); + + // Fill transaction with our data and sign + bool complete = true; + const TransactionError err = pwallet->FillPSBT(psbtx, complete, SIGHASH_ALL, true, false); + if (err != TransactionError::OK) { + throw JSONRPCTransactionError(err); + } + + CMutableTransaction mtx; + complete = FinalizeAndExtractPSBT(psbtx, mtx); + + UniValue result(UniValue::VOBJ); + + if (psbt_opt_in || !complete || !add_to_wallet) { + // Serialize the PSBT + CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); + ssTx << psbtx; + result.pushKV("psbt", EncodeBase64(ssTx.str())); + } + + if (complete) { + std::string err_string; + std::string hex = EncodeHexTx(CTransaction(mtx)); + CTransactionRef tx(MakeTransactionRef(std::move(mtx))); + result.pushKV("txid", tx->GetHash().GetHex()); + if (add_to_wallet && !psbt_opt_in) { + pwallet->CommitTransaction(tx, {}, {} /* orderForm */); + } else { + result.pushKV("hex", hex); + } + } + result.pushKV("complete", complete); + + return result; +} + UniValue abortrescan(const JSONRPCRequest& request); // in rpcdump.cpp UniValue dumpprivkey(const JSONRPCRequest& request); // in rpcdump.cpp UniValue importprivkey(const JSONRPCRequest& request); @@ -4066,10 +4257,10 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) { RPCHelpMan{"walletcreatefundedpsbt", - "\nCreates and funds a transaction in the Partially Signed Transaction format. Inputs will be added if supplied inputs are not enough\n" + "\nCreates and funds a transaction in the Partially Signed Transaction format.\n" "Implements the Creator and Updater roles.\n", { - {"inputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The inputs", + {"inputs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Leave empty to add inputs automatically. See add_inputs option.", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -4100,6 +4291,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) {"locktime", RPCArg::Type::NUM, /* default */ "0", "Raw locktime. Non-0 value also locktime-activates inputs"}, {"options", RPCArg::Type::OBJ, 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 Dash address to receive the change"}, {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only"}, @@ -4150,7 +4342,11 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) CAmount fee; int change_position; CMutableTransaction rawTx = ConstructTransaction(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 overriden 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}; @@ -4260,6 +4456,7 @@ static const CRPCCommand commands[] = { "wallet", "lockunspent", &lockunspent, {"unlock","transactions"} }, { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, + { "wallet", "send", &send, {"outputs","conf_target","estimate_mode","options"} }, { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","addlocked","comment","subtractfeefrom","use_is","use_cj","conf_target","estimate_mode"} }, { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","use_is","use_cj","conf_target","estimate_mode", "avoid_reuse"} }, { "wallet", "setcoinjoinrounds", &setcoinjoinrounds, {"rounds"} }, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6974dc3b4cb4..d56ac2adf742 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2625,6 +2625,11 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const } if(!found) continue; + // Only consider selected coins if add_inputs is false + if (coinControl && !coinControl->m_add_inputs && !coinControl->IsSelected(COutPoint(wtxid, i))) { + continue; + } + if (pcoin->tx->vout[i].nValue < nMinimumAmount || pcoin->tx->vout[i].nValue > nMaximumAmount) continue; diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 7e32d1cba943..c4977efab061 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -230,7 +230,7 @@ def test_invalid_change_address(self): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - assert_raises_rpc_error(-5, "changeAddress must be a valid Dash address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'}) + assert_raises_rpc_error(-5, "Change address must be a valid Dash address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'}) def test_valid_change_address(self): self.log.info("Test fundrawtxn with a provided change address") @@ -264,7 +264,11 @@ def test_coin_selection(self): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) 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}) + # add_inputs is enabled by default rawtxfund = self.nodes[2].fundrawtransaction(rawtx) + dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -292,7 +296,10 @@ def test_two_vin(self): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - rawtxfund = self.nodes[2].fundrawtransaction(rawtx) + # Should fail without add_inputs: + assert_raises_rpc_error(-4, "Insufficient funds", 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']) totalOut = 0 matchingOuts = 0 @@ -323,7 +330,10 @@ def test_two_vin_two_vout(self): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - rawtxfund = self.nodes[2].fundrawtransaction(rawtx) + # Should fail without add_inputs: + assert_raises_rpc_error(-4, "Insufficient funds", 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']) totalOut = 0 matchingOuts = 0 diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 188f9e7dd61f..2b378d99f5cc 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -8,8 +8,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 ) @@ -31,6 +31,16 @@ def run_test(self): # Create and fund a raw tx for sending 10 DASH 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():900}) + + psbtx1 = self.nodes[0].walletcreatefundedpsbt([{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():900}, 0, {"add_inputs": True})['psbt'] + assert_equal(len(self.nodes[0].decodepsbt(psbtx1)['tx']['vin']), 2) + + # Inputs argument can be null + self.nodes[0].walletcreatefundedpsbt(None, {self.nodes[2].getnewaddress():10}) + # 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'] assert_equal(psbtx1, psbtx) @@ -96,16 +106,16 @@ def run_test(self): self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex']) # feeRate of 0.1 DASH / KB produces a total fee slightly below -maxtxfee (~0.06650000): - res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 0.1}, False) - assert_greater_than(res["fee"], 0.03) - assert_greater_than(0.04, res["fee"]) + res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 0.1, "add_inputs": True}, False) + assert_approx(res["fee"], 0.04, 0.03) decoded_psbt = self.nodes[0].decodepsbt(res['psbt']) for psbt_in in decoded_psbt["inputs"]: assert "bip32_derivs" not in psbt_in # feeRate of 10 DASH / 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 user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 10}) + assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 10, "add_inputs": True}) + assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():1}, 0, {"feeRate": 10, "add_inputs": True}) # partially sign multisig things with node 1 psbtx = wmulti.walletcreatefundedpsbt(inputs=[{"txid":txid,"vout":p2sh_pos}], outputs={self.nodes[1].getnewaddress():9.99}, options={'changeAddress': self.nodes[1].getrawchangeaddress()})['psbt'] @@ -185,7 +195,7 @@ def run_test(self): # Regression test for 14473 (mishandling of already-signed witness transaction): unspent = self.nodes[0].listunspent()[0] - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}]) + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{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"]) assert_equal(complete_psbt, double_processed_psbt) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index f402075a4d8b..56f0faf9d6a2 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -272,6 +272,7 @@ 'rpc_verifyislock.py', 'rpc_verifychainlock.py', 'wallet_create_tx.py', + 'wallet_send.py', 'p2p_fingerprint.py', 'rpc_platform_filter.py', 'rpc_wipewallettxes.py', diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 7015a61dcdd3..5330077dec6c 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -168,6 +168,7 @@ def run_test(self): self.nodes[0].generate(1) # Generate one block for each send variant.confirmation_height = self.nodes[0].getblockcount() variant.timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"] + self.sync_all() # Conclude sync before calling setmocktime to avoid timeouts # Generate a block further in the future (past the rescan window). assert_equal(self.nodes[0].getrawmempool(), []) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py new file mode 100755 index 000000000000..d25ec48df8e7 --- /dev/null +++ b/test/functional/wallet_send.py @@ -0,0 +1,323 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the send RPC command.""" + +from decimal import Decimal, getcontext +from test_framework.authproxy import JSONRPCException +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_fee_amount, + assert_greater_than, + assert_raises_rpc_error +) + +class WalletSendTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + # whitelist all peers to speed up tx relay / mempool sync + self.extra_args = [ + ["-whitelist=127.0.0.1"], + ["-whitelist=127.0.0.1"], + ] + getcontext().prec = 8 # Satoshi precision for Decimal + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def test_send(self, from_wallet, to_wallet=None, amount=None, data=None, + arg_conf_target=None, arg_estimate_mode=None, + conf_target=None, estimate_mode=None, add_to_wallet=None, psbt=None, + inputs=None, add_inputs=None, change_address=None, change_position=None, + include_watching=None, locktime=None, lock_unspents=None, subtract_fee_from_outputs=None, + expect_error=None): + assert (amount is None) != (data is None) + + from_balance_before = from_wallet.getbalance() + if to_wallet is None: + assert amount is None + else: + to_untrusted_pending_before = to_wallet.getbalances()["mine"]["untrusted_pending"] + + if amount: + dest = to_wallet.getnewaddress() + outputs = {dest: amount} + else: + outputs = {"data": data} + + # Construct options dictionary + options = {} + if add_to_wallet is not None: + options["add_to_wallet"] = add_to_wallet + else: + if psbt: + add_to_wallet = False + else: + add_to_wallet = from_wallet.getwalletinfo()["private_keys_enabled"] # Default value + if psbt is not None: + options["psbt"] = psbt + if conf_target is not None: + options["conf_target"] = conf_target + if estimate_mode is not None: + options["estimate_mode"] = estimate_mode + if inputs is not None: + options["inputs"] = inputs + if add_inputs is not None: + options["add_inputs"] = add_inputs + if change_address is not None: + options["change_address"] = change_address + if change_position is not None: + options["change_position"] = change_position + if include_watching is not None: + options["include_watching"] = include_watching + if locktime is not None: + options["locktime"] = locktime + if lock_unspents is not None: + options["lock_unspents"] = lock_unspents + if subtract_fee_from_outputs is not None: + options["subtract_fee_from_outputs"] = subtract_fee_from_outputs + + if len(options.keys()) == 0: + options = None + + if expect_error is None: + res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, options=options) + else: + try: + assert_raises_rpc_error(expect_error[0], expect_error[1], from_wallet.send, + outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, options=options) + except AssertionError: + # Provide debug info if the test fails + self.log.error("Unexpected successful result:") + self.log.error(options) + res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, options=options) + self.log.error(res) + if "txid" in res and add_to_wallet: + self.log.error("Transaction details:") + try: + tx = from_wallet.gettransaction(res["txid"]) + self.log.error(tx) + self.log.error("testmempoolaccept (transaction may already be in mempool):") + self.log.error(from_wallet.testmempoolaccept([tx["hex"]])) + except JSONRPCException as exc: + self.log.error(exc) + + raise + + return + + if locktime: + return res + + if from_wallet.getwalletinfo()["private_keys_enabled"] and not include_watching: + assert_equal(res["complete"], True) + assert "txid" in res + else: + assert_equal(res["complete"], False) + assert not "txid" in res + assert "psbt" in res + + if add_to_wallet and not include_watching: + # Ensure transaction exists in the wallet: + tx = from_wallet.gettransaction(res["txid"]) + assert tx + # Ensure transaction exists in the mempool: + tx = from_wallet.getrawtransaction(res["txid"], True) + assert tx + if amount: + if subtract_fee_from_outputs: + assert_equal(from_balance_before - from_wallet.getbalance(), amount) + else: + assert_greater_than(from_balance_before - from_wallet.getbalance(), amount) + else: + assert next((out for out in tx["vout"] if out["scriptPubKey"]["asm"] == "OP_RETURN 35"), None) + else: + assert_equal(from_balance_before, from_wallet.getbalance()) + + if to_wallet: + self.sync_mempools() + if add_to_wallet: + if not subtract_fee_from_outputs: + assert_equal(to_wallet.getbalances()["mine"]["untrusted_pending"], to_untrusted_pending_before + Decimal(amount if amount else 0)) + else: + assert_equal(to_wallet.getbalances()["mine"]["untrusted_pending"], to_untrusted_pending_before) + + return res + + def run_test(self): + self.log.info("Setup wallets...") + # w0 is a wallet with coinbase rewards + w0 = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + # w1 is a regular wallet + self.nodes[1].createwallet(wallet_name="w1") + w1 = self.nodes[1].get_wallet_rpc("w1") + # w2 contains the private keys for w3 + self.nodes[1].createwallet(wallet_name="w2") + w2 = self.nodes[1].get_wallet_rpc("w2") + # w3 is a watch-only wallet, based on w2 + self.nodes[1].createwallet(wallet_name="w3", disable_private_keys=True) + w3 = self.nodes[1].get_wallet_rpc("w3") + for _ in range(3): + a2_receive = w2.getnewaddress() + a2_change = w2.getrawchangeaddress() # doesn't actually use change derivation + res = w3.importmulti([{ + "desc": w2.getaddressinfo(a2_receive)["desc"], + "timestamp": "now", + "keypool": True, + "watchonly": True + },{ + "desc": w2.getaddressinfo(a2_change)["desc"], + "timestamp": "now", + "keypool": True, + "internal": True, + "watchonly": True + }]) + assert_equal(res, [{"success": True}, {"success": True}]) + + w0.sendtoaddress(a2_receive, 10) # fund w3 + self.nodes[0].generate(1) + self.sync_blocks() + + # w4 has private keys enabled, but only contains watch-only keys (from w2) + self.nodes[1].createwallet(wallet_name="w4", disable_private_keys=False) + w4 = self.nodes[1].get_wallet_rpc("w4") + for _ in range(3): + a2_receive = w2.getnewaddress() + res = w4.importmulti([{ + "desc": w2.getaddressinfo(a2_receive)["desc"], + "timestamp": "now", + "keypool": False, + "watchonly": True + }]) + assert_equal(res, [{"success": True}]) + + w0.sendtoaddress(a2_receive, 10) # fund w4 + self.nodes[0].generate(1) + self.sync_blocks() + + self.log.info("Send to address...") + self.test_send(from_wallet=w0, to_wallet=w1, amount=1) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=True) + + self.log.info("Don't broadcast...") + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False) + assert(res["hex"]) + + self.log.info("Return PSBT...") + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, psbt=True) + assert(res["psbt"]) + + self.log.info("Create transaction that spends to address, but don't broadcast...") + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False) + # conf_target & estimate_mode can be set as argument or option + res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical", add_to_wallet=False) + res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=1, estimate_mode="economical", add_to_wallet=False) + assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], + self.nodes[1].decodepsbt(res2["psbt"])["fee"]) + # but not at the same time + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical", + conf_target=1, estimate_mode="economical", add_to_wallet=False, expect_error=(-8,"Use either conf_target and estimate_mode or the options dictionary to control fee rate")) + + self.log.info("Create PSBT from watch-only wallet w3, sign with w2...") + res = self.test_send(from_wallet=w3, to_wallet=w1, amount=1) + res = w2.walletprocesspsbt(res["psbt"]) + assert res["complete"] + + self.log.info("Create PSBT from wallet w4 with watch-only keys, sign with w2...") + self.test_send(from_wallet=w4, to_wallet=w1, amount=1, expect_error=(-4, "Insufficient funds")) + res = self.test_send(from_wallet=w4, to_wallet=w1, amount=1, include_watching=True, add_to_wallet=False) + res = w2.walletprocesspsbt(res["psbt"]) + assert res["complete"] + + self.log.info("Create OP_RETURN...") + self.test_send(from_wallet=w0, to_wallet=w1, amount=1) + self.test_send(from_wallet=w0, data="Hello World", expect_error=(-8, "Data must be hexadecimal string (not 'Hello World')")) + self.test_send(from_wallet=w0, data="23") + res = self.test_send(from_wallet=w3, data="23") + res = w2.walletprocesspsbt(res["psbt"]) + assert res["complete"] + + self.log.info("Set fee rate...") + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=2, estimate_mode="duff/b", add_to_wallet=False) + fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] + assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00002")) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=-1, estimate_mode="duff/b", + expect_error=(-3, "Amount out of range")) + + # TODO: Return hex if fee rate is below -maxmempool + # res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode="duff/b", add_to_wallet=False) + # assert res["hex"] + # hex = res["hex"] + # res = self.nodes[0].testmempoolaccept([hex]) + # assert not res[0]["allowed"] + # assert_equal(res[0]["reject-reason"], "...") # low fee + # assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.000001")) + + self.log.info("If inputs are specified, do not automatically add more...") + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=501, inputs=[], add_to_wallet=False) + assert res["complete"] + utxo1 = w0.listunspent()[0] + assert_equal(utxo1["amount"], 500) + self.test_send(from_wallet=w0, to_wallet=w1, amount=501, inputs=[utxo1], + expect_error=(-4, "Insufficient funds")) + self.test_send(from_wallet=w0, to_wallet=w1, amount=501, inputs=[utxo1], add_inputs=False, + expect_error=(-4, "Insufficient funds")) + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=501, inputs=[utxo1], add_inputs=True, add_to_wallet=False) + assert res["complete"] + + self.log.info("Manual change address and position...") + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, change_address="not an address", + expect_error=(-5, "Change address must be a valid Dash address")) + change_address = w0.getnewaddress() + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False, change_address=change_address) + assert res["complete"] + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False, change_address=change_address, change_position=0) + assert res["complete"] + assert_equal(self.nodes[0].decodepsbt(res["psbt"])["tx"]["vout"][0]["scriptPubKey"]["addresses"], [change_address]) + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False, change_position=0) + assert res["complete"] + change_address = self.nodes[0].decodepsbt(res["psbt"])["tx"]["vout"][0]["scriptPubKey"]["addresses"][0] + assert change_address[0] == "y" or change_address[0] == "8" or change_address[0] == "9" + + self.log.info("Set lock time...") + height = self.nodes[0].getblockchaininfo()["blocks"] + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, locktime=height + 1) + assert res["complete"] + assert res["txid"] + txid = res["txid"] + # Although the wallet finishes the transaction, it can't be added to the mempool yet: + hex = self.nodes[0].gettransaction(res["txid"])["hex"] + res = self.nodes[0].testmempoolaccept([hex]) + assert not res[0]["allowed"] + assert_equal(res[0]["reject-reason"], "non-final") + # It shouldn't be confirmed in the next block + self.nodes[0].generate(1) + assert_equal(self.nodes[0].gettransaction(txid)["confirmations"], 0) + # The mempool should allow it now: + res = self.nodes[0].testmempoolaccept([hex]) + assert res[0]["allowed"] + # Don't wait for wallet to add it to the mempool: + res = self.nodes[0].sendrawtransaction(hex) + self.nodes[0].generate(1) + assert_equal(self.nodes[0].gettransaction(txid)["confirmations"], 1) + self.sync_all() + + self.log.info("Lock unspents...") + utxo1 = w0.listunspent()[0] + assert_greater_than(utxo1["amount"], 1) + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, inputs=[utxo1], add_to_wallet=False, lock_unspents=True) + assert res["complete"] + locked_coins = w0.listlockunspent() + assert_equal(len(locked_coins), 1) + # Locked coins are automatically unlocked when manually selected + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, inputs=[utxo1], add_to_wallet=False) + assert res["complete"] + + self.log.info("Subtract fee from output") + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, subtract_fee_from_outputs=[0]) + + +if __name__ == '__main__': + WalletSendTest().main()