Merge #20220: wallet, rpc: explicit fee rate follow-ups/fixes for 0.21

0be29000c011dec0722481dbebb159873da6fa54 rpc: update conf_target helps for correctness/consistency (Jon Atack)
778b9be40667876c705e586849ea9c9e44cf451c wallet, rpc: fix send subtract_fee_from_outputs help (Jon Atack)
603c0050837ec65765208dd54dde354145fbe098 wallet: add rpc send explicit fee rate coverage (Jon Atack)
dd341e602d5160fc621c0299179b91403756b61d wallet: add sendtoaddress/sendmany explicit fee rate coverage (Jon Atack)
44e7bfa60313e4ae67da49e5ba4535038b71b453 wallet: add walletcreatefundedpsbt explicit fee rate coverage (Jon Atack)
6e1ea4273e52fdcd86c87628aa595c03a071ca8c test: refactor for walletcreatefundedpsbt fee rate coverage (Jon Atack)
3ac7b0c6f1c68e74a84d868a454f508bada6b09d wallet: fundrawtx fee rate coverage, fixup ParseConfirmTarget() (Jon Atack)
2d8eba8f8425a2515022d51f1f5b4911329fbf55 wallet: combine redundant bumpfee invalid params and args tests (Jon Atack)
1697a40b6f841a54ee0d9744ed7fd09034b0ddad wallet: improve bumpfee error/help, add explicit fee rate coverage (Jon Atack)
fc5721723d34f76f9e1ffd2e31f274ea6b22f894 wallet: fix SetFeeEstimateMode() error message (Jon Atack)
052427eef1c9da84c474c5161b1910d3328ef0da wallet, bugfix: fix bumpfee with explicit fee rate modes (Jon Atack)

Pull request description:

  Follow-up to #11413 providing a base to build on for #19543:

  - bugfix for `bumpfee` raising a JSON error with explicit feerates, fixes issue #20219
  - adds explicit feerate test coverage for `bumpfee`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendtoaddress`, and `sendmany`
  - improves a few related RPC error messages and `ParseConfirmTarget()` / error message
  - fixes/improves the explicit fee rate information in the 6 RPC helps, of which 2 were also missing `conf_target` sat/B units

  This provides a spec and regression coverage for the potential next step of a universal `sat/vB` feerate argument (see #19543), as well as immediate coverage and minimum fixes for 0.21.

ACKs for top commit:
  kallewoof:
    Concept/Tested ACK 0be29000c011dec0722481dbebb159873da6fa54
  meshcollider:
    Code review + functional test run ACK 0be29000c011dec0722481dbebb159873da6fa54

Tree-SHA512: efd965003e991cba51d4504e2940f06ab3d742e34022e96a673606b44fad85596aa03a8c1809f06df7ebcf21a38e18a891e54392fe3d6fb4d120bbe4ea0cf5e0

src/rpc/util.cpp

@@ -319,11 +319,12 @@ UniValue DescribeAddress(const CTxDestination& dest)
 
 unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target)
 {
-    int target = value.get_int();
+    const int target{value.get_int()};
-    if (target < 1 || (unsigned int)target > max_target) {
+    const unsigned int unsigned_target{static_cast<unsigned int>(target)};
-        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid conf_target, must be between %u - %u", 1, max_target));
+    if (target < 1 || unsigned_target > max_target) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid conf_target, must be between %u and %u", 1, max_target));
     }
-    return (unsigned int)target;
+    return unsigned_target;
 }
 
 /**

src/wallet/rpcwallet.cpp

@@ -221,7 +221,7 @@ static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const Un
 
     if (cc.m_fee_mode == FeeEstimateMode::DASH_KB || cc.m_fee_mode == FeeEstimateMode::DUFF_B) {
         if (estimate_param.isNull()) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Selected estimate_mode requires a fee rate");
+            throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Selected estimate_mode %s requires a fee rate to be specified in conf_target", estimate_mode.get_str()));
         }
 
         CAmount fee_rate = AmountFromValue(estimate_param);
@@ -426,7 +426,8 @@ static RPCHelpMan sendtoaddress()
                 "The recipient will receive less amount of Dash than you enter in the amount field."},
             {"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"},
             {"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"},
-            {"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)"},
+            {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n"
+                                         "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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\"") + "\""},
             {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Avoid spending from dirty addresses; addresses are considered\n"
@@ -921,7 +922,8 @@ static RPCHelpMan sendmany()
                     },
                     {"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"},
                     {"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"},
-                    {"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)"},
+                    {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n"
+                                       "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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\"") + "\""},
                     {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra infomration about the transaction."},
@@ -3378,7 +3380,7 @@ static RPCHelpMan listunspent()
     };
 }
 
-void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options, CCoinControl& coinControl)
+void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, const 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
@@ -3533,7 +3535,8 @@ static RPCHelpMan fundrawtransaction()
                                     {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."},
                                 },
                             },
-                            {"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)"},
+                            {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n"
+                                                          "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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\"") + "\""},
                         },
@@ -4177,7 +4180,8 @@ static RPCHelpMan send()
                     },
                 },
             },
-            {"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)"},
+            {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n"
+                    "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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, "",
@@ -4189,7 +4193,8 @@ static RPCHelpMan send()
                     {"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)"},
+                    {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n"
+                            "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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"
@@ -4205,7 +4210,7 @@ static RPCHelpMan send()
                     {"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"
+                    {"subtract_fee_from_outputs", RPCArg::Type::ARR, /* default */ "empty array", "Outputs to subtract the fee from, specified as integer indices.\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.",
@@ -4279,7 +4284,7 @@ static RPCHelpMan send()
     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.
+    // be overridden by options.add_inputs.
     coin_control.m_add_inputs = rawTx.vin.size() == 0;
     FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control);
 
@@ -4534,7 +4539,8 @@ static RPCHelpMan walletcreatefundedpsbt()
                             {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."},
                         },
                     },
-                    {"conf_target", RPCArg::Type::NUM, /* default */ "Fallback to wallet's confirmation target", "Confirmation target (in blocks)"},
+                    {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n"
+                                                          "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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\"") + "\""},
                 },

test/functional/rpc_fundrawtransaction.py

@@ -8,6 +8,7 @@ from decimal import Decimal
 from test_framework.descriptors import descsum_create
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
+    assert_approx,
     assert_equal,
     assert_fee_amount,
     assert_greater_than,
@@ -93,6 +94,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.test_op_return()
         self.test_watchonly()
         self.test_all_watched_funds()
+        self.test_feerate_with_conf_target_and_estimate_mode()
         self.test_option_feerate()
         self.test_address_reuse()
         self.test_option_subtract_fee_from_outputs()
@@ -718,6 +720,59 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate)
         assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate)
 
+    def test_feerate_with_conf_target_and_estimate_mode(self):
+        self.log.info("Test fundrawtxn passing an explicit fee rate using conf_target and estimate_mode")
+        node = self.nodes[3]
+        # Make sure there is exactly one input so coin selection can't skew the result.
+        assert_equal(len(node.listunspent(1)), 1)
+        inputs = []
+        outputs = {node.getnewaddress() : 1}
+        rawtx = node.createrawtransaction(inputs, outputs)
+
+        for unit, fee_rate in {"dash/kb": 0.1, "duff/b": 10000}.items():
+            self.log.info("Test fundrawtxn with conf_target {} estimate_mode {} produces expected fee".format(fee_rate, unit))
+            # With no arguments passed, expect fee of 225 sats/b.
+            assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000225, vspan=0.00000001)
+            # Expect fee to be 10,000x higher when explicit fee 10,000x greater is specified.
+            result = node.fundrawtransaction(rawtx, {"conf_target": fee_rate, "estimate_mode": unit})
+            assert_approx(result["fee"], vexp=0.0225, vspan=0.0001)
+
+        for field, fee_rate in {"conf_target": 0.1, "estimate_mode": "duff/b"}.items():
+            self.log.info("Test fundrawtxn raises RPC error if both feeRate and {} are passed".format(field))
+            assert_raises_rpc_error(
+                -8, "Cannot specify both {} and feeRate".format(field),
+                lambda: node.fundrawtransaction(rawtx, {"feeRate": 0.1, field: fee_rate}))
+
+        self.log.info("Test fundrawtxn with invalid estimate_mode settings")
+        for k, v in {"number": 42, "object": {"foo": "bar"}}.items():
+            assert_raises_rpc_error(-3, "Expected type string for estimate_mode, got {}".format(k),
+                lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": v, "conf_target": 0.1}))
+        for mode in ["foo", Decimal("3.141592")]:
+            assert_raises_rpc_error(-8, "Invalid estimate_mode parameter",
+                lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": 0.1}))
+
+        self.log.info("Test fundrawtxn with invalid conf_target settings")
+        for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]:
+            self.log.debug("{}".format(mode))
+            for k, v in {"string": "", "object": {"foo": "bar"}}.items():
+                assert_raises_rpc_error(-3, "Expected type number for conf_target, got {}".format(k),
+                    lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": v}))
+            if mode in ["dash/kb", "duff/b"]:
+                assert_raises_rpc_error(-3, "Amount out of range",
+                    lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": -1}))
+                assert_raises_rpc_error(-4, "Fee rate (0.00000000 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)",
+                    lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": 0}))
+            else:
+                for n in [-1, 0, 1009]:
+                    assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008",
+                        lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": n}))
+
+        for unit, fee_rate in {"duff/B": 0.99999999, "DASH/kB": 0.00000999}.items():
+            self.log.info("- raises RPC error 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit))
+            assert_raises_rpc_error(-4, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)",
+                lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": unit, "conf_target": fee_rate, "add_inputs": True}))
+
+
     def test_address_reuse(self):
         """Test no address reuse occurs."""
         self.log.info("Test fundrawtxn does not reuse addresses")

test/functional/rpc_psbt.py

@@ -94,8 +94,11 @@ class PSBTTest(BitcoinTestFramework):
             elif out['scriptPubKey']['address'] == p2pkh:
                 p2pkh_pos = out['n']
 
+        inputs = [{"txid":txid,"vout":p2pkh_pos}]
+        outputs = [{self.nodes[1].getnewaddress(): 9.99}]
+
         # spend single key from node 1
-        created_psbt = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99})
+        created_psbt = self.nodes[1].walletcreatefundedpsbt(inputs, outputs)
         walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(created_psbt['psbt'])
         # Make sure it has both types of UTXOs
         decoded = self.nodes[1].decodepsbt(walletprocesspsbt_out['psbt'])
@@ -105,18 +108,62 @@ class PSBTTest(BitcoinTestFramework):
         assert_equal(walletprocesspsbt_out['complete'], True)
         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):
+        self.log.info("Test walletcreatefundedpsbt feeRate of 0.1 DASH/kB produces a total fee at or 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, "add_inputs": True}, False)
+        res = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, "add_inputs": True})
         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
+        self.log.info("Test walletcreatefundedpsbt explicit fee rate with conf_target and estimate_mode")
+        for unit, fee_rate in {"dash/kb": 0.1, "duff/b": 10000}.items():
+            fee = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"conf_target": fee_rate, "estimate_mode": unit, "add_inputs": True})["fee"]
+            self.log.info("- conf_target {}, estimate_mode {} produces fee {} at or slightly below -maxtxfee (~0.05290000)".format(fee_rate, unit, fee))
+            assert_approx(fee, vexp=0.04, vspan=0.03)
+
+        for field, fee_rate in {"conf_target": 0.1, "estimate_mode": "duff/b"}.items():
+            self.log.info("- raises RPC error if both feeRate and {} are passed".format(field))
+            assert_raises_rpc_error(-8, "Cannot specify both {} and feeRate".format(field),
+                lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, field: fee_rate, "add_inputs": True}))
+
+        self.log.info("- raises RPC error with invalid estimate_mode settings")
+        for k, v in {"number": 42, "object": {"foo": "bar"}}.items():
+            assert_raises_rpc_error(-3, "Expected type string for estimate_mode, got {}".format(k),
+                lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": v, "conf_target": 0.1, "add_inputs": True}))
+        for mode in ["foo", Decimal("3.141592")]:
+            assert_raises_rpc_error(-8, "Invalid estimate_mode parameter",
+                lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": 0.1, "add_inputs": True}))
+
+        self.log.info("- raises RPC error if estimate_mode is passed without a conf_target")
+        for unit in ["DUFF/B", "DASH/KB"]:
+            assert_raises_rpc_error(-8, "Selected estimate_mode {} requires a fee rate to be specified in conf_target".format(unit),
+                lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": unit}))
+
+        self.log.info("- raises RPC error with invalid conf_target settings")
+        for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]:
+            self.log.debug("{}".format(mode))
+            for k, v in {"string": "", "object": {"foo": "bar"}}.items():
+                assert_raises_rpc_error(-3, "Expected type number for conf_target, got {}".format(k),
+                    lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": v, "add_inputs": True}))
+            if mode in ["dash/kb", "duff/b"]:
+                assert_raises_rpc_error(-3, "Amount out of range",
+                    lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": -1, "add_inputs": True}))
+                assert_raises_rpc_error(-4, "Fee rate (0.00000000 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)",
+                    lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": 0, "add_inputs": True}))
+            else:
+                for n in [-1, 0, 1009]:
+                    assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008",
+                        lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": n, "add_inputs": True}))
+
+        for unit, fee_rate in {"DUFF/B": 0.99999999, "DASH/KB": 0.00000999}.items():
+            self.log.info("- raises RPC error 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit))
+            assert_raises_rpc_error(-4, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)",
+                lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": unit, "conf_target": fee_rate, "add_inputs": True}))
+
+        self.log.info("Test walletcreatefundedpsbt feeRate of 10 DASH/kB produces total fee well above -maxtxfee and raises RPC error")
         # 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, "add_inputs": True})
+        for bool_add, outputs_array in {True: outputs, False: [{self.nodes[1].getnewaddress(): 1}]}.items():
-        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})
+            assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)",
+                self.nodes[1].walletcreatefundedpsbt, inputs, outputs_array, 0, {"feeRate": 10, "add_inputs": bool_add})
 
+        self.log.info("Test various PSBT operations")
         # 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']
         walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(psbtx)

test/functional/wallet_basic.py

@@ -221,6 +221,8 @@ class WalletTest(BitcoinTestFramework):
         assert_equal(self.nodes[2].getbalance(), node_2_bal)
         node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal('200'), fee_per_byte, count_bytes(self.nodes[2].gettransaction(txid)['hex']))
 
+        self.log.info("Test sendmany")
+
         # Sendmany 100 DASH
         txid = self.nodes[2].sendmany('', {address: 100}, 0, False, "", [])
         self.nodes[2].generate(1)
@@ -239,9 +241,9 @@ class WalletTest(BitcoinTestFramework):
 
         self.start_node(3, self.nodes[3].extra_args)
         self.connect_nodes(0, 3)
-        # Sendmany with explicit fee (DASH/kB)
+        self.log.info("Test case-insensitive explicit fee rate (sendmany as DASH/kB)")
         # Throw if no conf_target provided
-        assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate",
+        assert_raises_rpc_error(-8, "Selected estimate_mode dash/kB requires a fee rate to be specified in conf_target",
             self.nodes[2].sendmany,
             amounts={ address: 10 },
             estimate_mode='dash/kB')
@@ -265,9 +267,9 @@ class WalletTest(BitcoinTestFramework):
         node_0_bal += Decimal('10')
         assert_equal(self.nodes[0].getbalance(), node_0_bal)
 
-        # Sendmany with explicit fee (DUFF/B)
+        self.log.info("Test case-insensitive explicit fee rate (sendmany as duff/B)")
         # Throw if no conf_target provided
-        assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate",
+        assert_raises_rpc_error(-8, "Selected estimate_mode duff/b requires a fee rate to be specified in conf_target",
             self.nodes[2].sendmany,
             amounts={ address: 10 },
             estimate_mode='duff/b')
@@ -293,6 +295,11 @@ class WalletTest(BitcoinTestFramework):
         node_0_bal += Decimal('10')
         assert_equal(self.nodes[0].getbalance(), node_0_bal)
 
+        # Test setting explicit fee rate just below the minimum.
+        for unit, fee_rate in {"DASH/kB": 0.00000999, "duff/B": 0.99999999}.items():
+            self.log.info("Test sendmany raises 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit))
+            assert_raises_rpc_error(-6, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)",
+                self.nodes[2].sendmany, amounts={address: 10}, estimate_mode=unit, conf_target=fee_rate)
 
         # check if we can list zero value tx as available coins
         # 1. create raw_tx
@@ -423,15 +430,14 @@ class WalletTest(BitcoinTestFramework):
             self.nodes[0].generate(1)
             self.sync_all(self.nodes[0:3])
 
-            # send with explicit dash/kb fee
+            self.log.info("Test case-insensitive explicit fee rate (sendtoaddress as DASH/kB)")
-            self.log.info("test explicit fee (sendtoaddress as dash/kb)")
             self.nodes[0].generate(1)
             self.sync_all(self.nodes[0:3])
             prebalance = self.nodes[2].getbalance()
             assert prebalance > 2
             address = self.nodes[1].getnewaddress()
             # Throw if no conf_target provided
-            assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate",
+            assert_raises_rpc_error(-8, "Selected estimate_mode dash/Kb requires a fee rate to be specified in conf_target",
                 self.nodes[2].sendtoaddress,
                 address=address,
                 amount=1.0,
@@ -457,15 +463,15 @@ class WalletTest(BitcoinTestFramework):
             fee = prebalance - postbalance - Decimal('1')
             assert_fee_amount(fee, tx_size, Decimal('0.00002500'))
 
-            # send with explicit duff/b fee
             self.sync_all(self.nodes[0:3])
-            self.log.info("test explicit fee (sendtoaddress as duff/b)")
+
+            self.log.info("Test case-insensitive explicit fee rate (sendtoaddress as duff/B)")
             self.nodes[0].generate(1)
             prebalance = self.nodes[2].getbalance()
             assert prebalance > 2
             address = self.nodes[1].getnewaddress()
             # Throw if no conf_target provided
-            assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate",
+            assert_raises_rpc_error(-8, "Selected estimate_mode duff/b requires a fee rate to be specified in conf_target",
                 self.nodes[2].sendtoaddress,
                 address=address,
                 amount=1.0,
@@ -491,6 +497,12 @@ class WalletTest(BitcoinTestFramework):
             fee = prebalance - postbalance - Decimal('1')
             assert_fee_amount(fee, tx_size, Decimal('0.00002000'))
 
+            # Test setting explicit fee rate just below the minimum.
+            for unit, fee_rate in {"DASH/kB": 0.00000999, "sat/B": 0.99999999}.items():
+                self.log.info("Test sendtoaddress raises 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit))
+                assert_raises_rpc_error(-6, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)",
+                    self.nodes[2].sendtoaddress, address=address, amount=1, estimate_mode=unit, conf_target=fee_rate)
+
             # 2. Import address from node2 to node1
             self.nodes[1].importaddress(address_to_import)
 

test/functional/wallet_send.py

@@ -97,6 +97,8 @@ class WalletSendTest(BitcoinTestFramework):
             except AssertionError:
                 # Provide debug info if the test fails
                 self.log.error("Unexpected successful result:")
+                self.log.error(arg_conf_target)
+                self.log.error(arg_estimate_mode)
                 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)
@@ -271,8 +273,10 @@ class WalletSendTest(BitcoinTestFramework):
         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",
+        for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]:
-                       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.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical",
+                conf_target=1, estimate_mode=mode, 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)
@@ -297,12 +301,61 @@ class WalletSendTest(BitcoinTestFramework):
         res = w2.walletprocesspsbt(res["psbt"])
         assert res["complete"]
 
-        self.log.info("Set fee rate...")
+        self.log.info("Test setting explicit fee rate")
+        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"])
+
+        res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.00007, estimate_mode="dash/kb", add_to_wallet=False)
+        fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
+        assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00007"))
+
         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"))
+        res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=0.00004531, arg_estimate_mode="dash/kb", add_to_wallet=False)
+        fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
+        assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00004531"))
+
+        res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=3, arg_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.00003"))
+
+        # TODO: This test should pass with all modes, e.g. with the next line uncommented, for consistency with the other explicit feerate RPCs.
+        # for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]:
+        for mode in ["dash/kb", "duff/b"]:
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=-1, estimate_mode=mode,
+                expect_error=(-3, "Amount out of range"))
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0, estimate_mode=mode,
+                expect_error=(-4, "Fee rate (0.00000000 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)"))
+
+        for mode in ["foo", Decimal("3.141592")]:
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode=mode,
+                expect_error=(-8, "Invalid estimate_mode parameter"))
+            # TODO: these 2 equivalent sends with an invalid estimate_mode arg should both fail, but they do not...why?
+            # self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=0.1, arg_estimate_mode=mode,
+            #     expect_error=(-8, "Invalid estimate_mode parameter"))
+            # assert_raises_rpc_error(-8, "Invalid estimate_mode parameter", lambda: w0.send({w1.getnewaddress(): 1}, 0.1, mode))
+
+        # TODO: These tests should pass for consistency with the other explicit feerate RPCs, but they do not.
+        # for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]:
+        #     self.log.debug("{}".format(mode))
+        #     for k, v in {"string": "", "object": {"foo": "bar"}}.items():
+        #         self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=v, estimate_mode=mode,
+        #             expect_error=(-3, "Expected type number for conf_target, got {}".format(k)))
+
+        # TODO: error should use duff/B instead of DASH/kB if duff/B is selected.
+        # Test setting explicit fee rate just below the minimum.
+        for unit, fee_rate in {"duff/B": 0.99999999, "DASH/kB": 0.00000999}.items():
+            self.log.info("Explicit fee rate raises RPC error 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit))
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=fee_rate, estimate_mode=unit,
+                expect_error=(-4, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)"))
+
+        self.log.info("Explicit fee rate raises RPC error if estimate_mode is passed without a conf_target")
+        for unit, fee_rate in {"duff/B": 100, "DASH/kB": 0.001}.items():
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, estimate_mode=unit,
+                expect_error=(-8, "Selected estimate_mode {} requires a fee rate to be specified in conf_target".format(unit)))
 
         # 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)