Merge #16378: The ultimate send RPC

92326d8976 [rpc] add send method (Sjors Provoost)
2c2a1445dc [rpc] add snake case aliases for transaction methods (Sjors Provoost)
1bc8d0fd59 [rpc] walletcreatefundedpsbt: allow inputs to be null (Sjors Provoost)

Pull request description:

  `walletcreatefundedpsbt` has some interesting features that `sendtoaddress` and `sendmany` don't have:
  * manual coin selection
  * outputting a PSBT (it was controversial to add this, see #18201)
  * create a transaction without adding to wallet (which leads to broadcasting, unless `-walletbroadcast=0`)

  At the same time `walletcreatefundedpsbt` can't broadcast a transaction, which is inconvenient for simple use cases.

  This PR introduces a new `send` RPC method which creates a PSBT, signs it if possible and adds it to the wallet by default. If it can't sign all inputs, it outputs a PSBT. If `add_to_wallet` is set to `false` it will return the transaction in both PSBT and hex format.

  Because it uses a PSBT internally, it will much easier to add hardware wallet support to this method (see #16546).

  For `bitcoin-cli` users, it tries to keep the simplest use case easy to use:

  ```sh
  bitcoin-cli -regtest send '{"ADDRESS": 0.1}' 1 sat/b
  ```

  This paves the way for deprecating `sendtoaddress` and `sendmany` though there's no rush. The only missing feature compared to these older methods is adding labels to a destination address.

  Depends on:
  - [x] #16377 (`[rpc] don't automatically append inputs in walletcreatefundedpsbt`)
  - [x] #11413 (`[wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option`)
  - [x] #18244 (`[rpc] have lockUnspents also lock manually selected coins`)

ACKs for top commit:
  meshcollider:
    Light re-utACK 92326d8976
  achow101:
    ACK 92326d8976 Reviewed code and test, ran tests.
  kallewoof:
    utACK 92326d8976

Tree-SHA512: 7552ef1b193d4c06e381c44932fdb0d54f64383e4c7d6b988f49d059c7d4bba45ce6aa7813e03df86360ad9dad6f3010eb76ee7da480551742d5fd98c2251c0f

doc/release-notes-16378.md

@@ -0,0 +1,5 @@
+RPC
+---
+- A new `send` RPC with similar syntax to `walletcreatefundedpsbt`, including
+  support for coin selection and a custom fee rate. Using the new `send` method
+  is encouraged: `sendmany` and `sendtoaddress` may be deprecated in a future release.

src/rpc/client.cpp

@@ -125,6 +125,9 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "gettxoutproof", 0, "txids" },
     { "lockunspent", 0, "unlock" },
     { "lockunspent", 1, "transactions" },
+    { "send", 0, "outputs" },
+    { "send", 1, "conf_target" },
+    { "send", 3, "options" },
     { "importprivkey", 2, "rescan" },
     { "importaddress", 2, "rescan" },
     { "importaddress", 3, "p2sh" },

src/rpc/rawtransaction_util.cpp

@@ -21,10 +21,15 @@
 
 CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, bool rbf)
 {
-    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();
 

src/wallet/rpcwallet.cpp

@@ -11,6 +11,7 @@
 #include <outputtype.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
+#include <policy/policy.h>
 #include <policy/rbf.h>
 #include <rpc/rawtransaction_util.h>
 #include <rpc/server.h>
@@ -2955,13 +2956,22 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
         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)},
                 {"change_type", UniValueType(UniValue::VSTR)},
                 {"includeWatching", UniValueType(UniValue::VBOOL)},
+                {"include_watching", UniValueType(UniValue::VBOOL)},
+                {"inputs", UniValueType(UniValue::VARR)},
                 {"lockUnspents", UniValueType(UniValue::VBOOL)},
-                {"feeRate", UniValueType()}, // will be checked below
+                {"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)},
                 {"replaceable", UniValueType(UniValue::VBOOL)},
                 {"conf_target", UniValueType(UniValue::VNUM)},
                 {"estimate_mode", UniValueType(UniValue::VSTR)},
@@ -2972,22 +2982,24 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
             coinControl.m_add_inputs = options["add_inputs"].get_bool();
         }
 
-        if (options.exists("changeAddress")) {
-            CTxDestination dest = DecodeDestination(options["changeAddress"].get_str());
+        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 bitcoin address");
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Change address must be a valid bitcoin 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();
+        }
 
         if (options.exists("change_type")) {
-            if (options.exists("changeAddress")) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both changeAddress and address_type options");
+            if (options.exists("changeAddress") || options.exists("change_address")) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both change address and address type options");
             }
             OutputType out_type;
             if (!ParseOutputType(options["change_type"].get_str(), out_type)) {
@@ -2996,10 +3008,12 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
             coinControl.m_change_type.emplace(out_type);
         }
 
-        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"))
         {
@@ -3013,8 +3027,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();
 
         if (options.exists("replaceable")) {
             coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool();
@@ -3857,6 +3871,185 @@ static UniValue listlabels(const JSONRPCRequest& request)
     return ret;
 }
 
+static RPCHelpMan send()
+{
+    return RPCHelpMan{"send",
+        "\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 bitcoin 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 bitcoin address to receive the change"},
+                    {"change_position", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"},
+                    {"change_type", RPCArg::Type::STR, /* default */ "set by -changetype", "The output type to use. Only valid if change_address is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."},
+                    {"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 bitcoins 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."},
+                        },
+                    },
+                    {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n"
+                    "                              Allows this transaction to be replaced by a transaction with higher fees"},
+                },
+                "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 satoshi per byte\n"
+        + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.1}' 1 sat/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}]}'")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+        {
+            RPCTypeCheck(request.params, {
+                UniValueType(), // ARR or OBJ, checked later
+                UniValue::VNUM,
+                UniValue::VSTR,
+                UniValue::VOBJ
+                }, true
+            );
+
+            std::shared_ptr<CWallet> 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;
+            bool rbf = pwallet->m_signal_rbf;
+            if (options.exists("replaceable")) {
+                rbf = options["add_to_wallet"].get_bool();
+            }
+            CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"], rbf);
+            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 out 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);
+
+            // Serialize the PSBT
+            CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
+            ssTx << psbtx;
+            const std::string result_str = EncodeBase64(ssTx.str());
+
+            if (psbt_opt_in || !complete || !add_to_wallet) {
+                result.pushKV("psbt", result_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 sethdseed(const JSONRPCRequest& request)
 {
             RPCHelpMan{"sethdseed",
@@ -3997,7 +4190,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
                 "\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. Leave empty to add inputs automatically. See add_inputs option.",
+                    {"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, "",
                                 {
@@ -4214,6 +4407,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","comment","subtractfeefrom","replaceable","conf_target","estimate_mode"} },
     { "wallet",             "sendtoaddress",                    &sendtoaddress,                 {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode","avoid_reuse"} },
     { "wallet",             "sethdseed",                        &sethdseed,                     {"newkeypool","seed"} },

test/functional/rpc_fundrawtransaction.py

@@ -224,7 +224,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         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 bitcoin address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'})
+        assert_raises_rpc_error(-5, "Change address must be a valid bitcoin address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'})
 
     def test_valid_change_address(self):
         self.log.info("Test fundrawtxn with a provided change address")

test/functional/rpc_psbt.py

@@ -94,6 +94,9 @@ class PSBTTest(BitcoinTestFramework):
         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)
 
+        # 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)

test/functional/test_runner.py

@@ -225,6 +225,7 @@ BASE_SCRIPTS = [
     'rpc_estimatefee.py',
     'rpc_getblockstats.py',
     'wallet_create_tx.py',
+    'wallet_send.py',
     'p2p_fingerprint.py',
     'feature_uacomment.py',
     'wallet_coinbase_category.py',

test/functional/wallet_send.py

@@ -0,0 +1,339 @@
+#!/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","-walletrbf=1"],
+            ["-whitelist=127.0.0.1","-walletrbf=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,change_type=None,
+                  include_watching=None,locktime=None,lock_unspents=None,replaceable=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 change_type is not None:
+            options["change_type"] = change_type
+        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 replaceable is None:
+            replaceable = True # default
+        else:
+            options["replaceable"] = replaceable
+        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
+            assert_equal(tx["bip125-replaceable"], "yes" if replaceable else "no")
+            # 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("")
+        # 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="sat/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="sat/b",
+                       expect_error=(-3, "Amount out of range"))
+        # Fee rate of 0.1 satoshi per byte should throw an error
+        # TODO: error should say 1.000 sat/b
+        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode="sat/b",
+                       expect_error=(-4, "Fee rate (0.00000100 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"))
+
+        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.000001, estimate_mode="BTC/KB",
+                       expect_error=(-4, "Fee rate (0.00000100 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"))
+
+        # 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="sat/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=51, inputs=[], add_to_wallet=False)
+        assert res["complete"]
+        utxo1 = w0.listunspent()[0]
+        assert_equal(utxo1["amount"], 50)
+        self.test_send(from_wallet=w0, to_wallet=w1, amount=51, inputs=[utxo1],
+                       expect_error=(-4, "Insufficient funds"))
+        self.test_send(from_wallet=w0, to_wallet=w1, amount=51, inputs=[utxo1], add_inputs=False,
+                       expect_error=(-4, "Insufficient funds"))
+        res = self.test_send(from_wallet=w0, to_wallet=w1, amount=51, 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 bitcoin 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_type="legacy", change_position=0)
+        assert res["complete"]
+        change_address = self.nodes[0].decodepsbt(res["psbt"])["tx"]["vout"][0]["scriptPubKey"]["addresses"][0]
+        assert change_address[0] == "m" or change_address[0] == "n"
+
+        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.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
+        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, inputs=[utxo1],add_to_wallet=False)
+
+        self.log.info("Replaceable...")
+        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False, replaceable=True)
+        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False, replaceable=False)
+
+        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()