Merge #11413: [wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option

25dac9fa65 doc: add release notes for explicit fee estimators and bumpfee change (Karl-Johan Alm)
05227a3554 tests for bumpfee / estimate_modes (Karl-Johan Alm)
3404c1b753 policy: optional FeeEstimateMode param to CFeeRate::ToString (Karl-Johan Alm)
6fcf448430 rpc/wallet: add two explicit modes to estimate_mode (Karl-Johan Alm)
b188d80c2d MOVEONLY: Make FeeEstimateMode available to CFeeRate (Karl-Johan Alm)
5d1a411eb1 fees: add FeeModes doc helper function (Karl-Johan Alm)
91f6d2bc8f rpc/wallet: add conf_target as alias to confTarget in bumpfee (Karl-Johan Alm)
69158b41fc added CURRENCY_ATOM to express minimum indivisible unit (Karl-Johan Alm)

Pull request description:

  This lets users pick their own fees when using `sendtoaddress`/`sendmany` if they prefer this over the estimators.

ACKs for top commit:
  Sjors:
    re-utACK 25dac9fa65: rebased, more fancy C++,
  jonatack:
    ACK 25dac9fa65 I think this should be merged after all this time, even though it looks to me like there are needed follow-ups, fixes and test coverage to be added (see further down), which I don't mind helping out with, if wanted.
  fjahr:
    Code review ACK 25dac9fa65

Tree-SHA512: f31177e6cabf3187a43cdfe93477144f8e8385c7344613743cbbd16e8490d53ff5144aec7b9de6c9a65eb855b55e0f99d7f164dee4b6bf3cfea4dce51cf11d33

doc/release-notes-11413.md

@@ -0,0 +1,11 @@
+Updated or changed RPC
+----------------------
+
+The `bumpfee`, `fundrawtransaction`, `sendmany`, `sendtoaddress`, and `walletcreatefundedpsbt`
+RPC commands have been updated to include two new fee estimation methods "BTC/kB" and "sat/B".
+The target is the fee expressed explicitly in the given form. Note that use of this feature
+will trigger BIP 125 (replace-by-fee) opt-in.
+
+In addition, the `estimate_mode` parameter is now case insensitive for all of the above RPC commands.
+
+The `bumpfee` command now uses `conf_target` rather than `confTarget` in the options.

src/policy/feerate.cpp

@@ -7,8 +7,6 @@
 
 #include <tinyformat.h>
 
-const std::string CURRENCY_UNIT = "BTC";
-
 CFeeRate::CFeeRate(const CAmount& nFeePaid, size_t nBytes_)
 {
     assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
@@ -37,7 +35,10 @@ CAmount CFeeRate::GetFee(size_t nBytes_) const
     return nFee;
 }
 
-std::string CFeeRate::ToString() const
+std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const
 {
-    return strprintf("%d.%08d %s/kB", nSatoshisPerK / COIN, nSatoshisPerK % COIN, CURRENCY_UNIT);
+    switch (fee_estimate_mode) {
+    case FeeEstimateMode::SAT_B:  return strprintf("%d.%03d %s/B", nSatoshisPerK / 1000, nSatoshisPerK % 1000, CURRENCY_ATOM);
+    default:                      return strprintf("%d.%08d %s/kB", nSatoshisPerK / COIN, nSatoshisPerK % COIN, CURRENCY_UNIT);
+    }
 }

src/policy/feerate.h

@@ -11,7 +11,17 @@
 
 #include <string>
 
-extern const std::string CURRENCY_UNIT;
+const std::string CURRENCY_UNIT = "BTC"; // One formatted unit
+const std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
+
+/* Used to determine type of fee estimation requested */
+enum class FeeEstimateMode {
+    UNSET,        //!< Use default settings based on other criteria
+    ECONOMICAL,   //!< Force estimateSmartFee to use non-conservative estimates
+    CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates
+    BTC_KB,       //!< Use explicit BTC/kB fee given in coin control
+    SAT_B,        //!< Use explicit sat/B fee given in coin control
+};
 
 /**
  * Fee rate in satoshis per kilobyte: CAmount / kB
@@ -46,7 +56,7 @@ public:
     friend bool operator>=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK >= b.nSatoshisPerK; }
     friend bool operator!=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK != b.nSatoshisPerK; }
     CFeeRate& operator+=(const CFeeRate& a) { nSatoshisPerK += a.nSatoshisPerK; return *this; }
-    std::string ToString() const;
+    std::string ToString(const FeeEstimateMode& fee_estimate_mode = FeeEstimateMode::BTC_KB) const;
 
     SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.nSatoshisPerK); }
 };

src/policy/fees.h

@@ -45,13 +45,6 @@ enum class FeeReason {
     REQUIRED,
 };
 
-/* Used to determine type of fee estimation requested */
-enum class FeeEstimateMode {
-    UNSET,        //!< Use default settings based on other criteria
-    ECONOMICAL,   //!< Force estimateSmartFee to use non-conservative estimates
-    CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates
-};
-
 /* Used to return detailed information about a feerate bucket */
 struct EstimatorBucket
 {

src/util/fees.cpp

@@ -6,11 +6,16 @@
 #include <util/fees.h>
 
 #include <policy/fees.h>
+#include <util/strencodings.h>
+#include <util/string.h>
 
 #include <map>
 #include <string>
+#include <vector>
+#include <utility>
 
-std::string StringForFeeReason(FeeReason reason) {
+std::string StringForFeeReason(FeeReason reason)
+{
     static const std::map<FeeReason, std::string> fee_reason_strings = {
         {FeeReason::NONE, "None"},
         {FeeReason::HALF_ESTIMATE, "Half Target 60% Threshold"},
@@ -29,16 +34,31 @@ std::string StringForFeeReason(FeeReason reason) {
     return reason_string->second;
 }
 
-bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode) {
+const std::vector<std::pair<std::string, FeeEstimateMode>>& FeeModeMap()
-    static const std::map<std::string, FeeEstimateMode> fee_modes = {
+{
-        {"UNSET", FeeEstimateMode::UNSET},
+    static const std::vector<std::pair<std::string, FeeEstimateMode>> FEE_MODES = {
-        {"ECONOMICAL", FeeEstimateMode::ECONOMICAL},
+        {"unset", FeeEstimateMode::UNSET},
-        {"CONSERVATIVE", FeeEstimateMode::CONSERVATIVE},
+        {"economical", FeeEstimateMode::ECONOMICAL},
+        {"conservative", FeeEstimateMode::CONSERVATIVE},
+        {(CURRENCY_UNIT + "/kB"), FeeEstimateMode::BTC_KB},
+        {(CURRENCY_ATOM + "/B"), FeeEstimateMode::SAT_B},
     };
-    auto mode = fee_modes.find(mode_string);
+    return FEE_MODES;
-
+}
-    if (mode == fee_modes.end()) return false;
+
-
+std::string FeeModes(const std::string& delimiter)
-    fee_estimate_mode = mode->second;
+{
-    return true;
+    return Join(FeeModeMap(), delimiter, [&](const std::pair<std::string, FeeEstimateMode>& i) { return i.first; });
+}
+
+bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode)
+{
+    auto searchkey = ToUpper(mode_string);
+    for (const auto& pair : FeeModeMap()) {
+        if (ToUpper(pair.first) == searchkey) {
+            fee_estimate_mode = pair.second;
+            return true;
+        }
+    }
+    return false;
 }

src/util/fees.h

@@ -12,5 +12,6 @@ enum class FeeReason;
 
 bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode);
 std::string StringForFeeReason(FeeReason reason);
+std::string FeeModes(const std::string& delimiter);
 
 #endif // BITCOIN_UTIL_FEES_H

src/wallet/rpcwallet.cpp

@@ -45,6 +45,8 @@ using interfaces::FoundBlock;
 static const std::string WALLET_ENDPOINT_BASE = "/wallet/";
 static const std::string HELP_REQUIRING_PASSPHRASE{"\nRequires wallet passphrase to be set with walletpassphrase call if wallet is encrypted.\n"};
 
+static const uint32_t WALLET_BTC_KB_TO_SAT_B = COIN / 1000; // 1 sat / B = 0.00001 BTC / kB
+
 static inline bool GetAvoidReuseFlag(const CWallet* const pwallet, const UniValue& param) {
     bool can_avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE);
     bool avoid_reuse = param.isNull() ? can_avoid_reuse : param.get_bool();
@@ -191,6 +193,42 @@ static std::string LabelFromValue(const UniValue& value)
     return label;
 }
 
+/**
+ * Update coin control with fee estimation based on the given parameters
+ *
+ * @param[in]     pwallet        Wallet pointer
+ * @param[in,out] cc             Coin control which is to be updated
+ * @param[in]     estimate_mode  String value (e.g. "ECONOMICAL")
+ * @param[in]     estimate_param Parameter (blocks to confirm, explicit fee rate, etc)
+ * @throws a JSONRPCError if estimate_mode is unknown, or if estimate_param is missing when required
+ */
+static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const UniValue& estimate_mode, const UniValue& estimate_param)
+{
+    if (!estimate_mode.isNull()) {
+        if (!FeeModeFromString(estimate_mode.get_str(), cc.m_fee_mode)) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter");
+        }
+    }
+
+    if (cc.m_fee_mode == FeeEstimateMode::BTC_KB || cc.m_fee_mode == FeeEstimateMode::SAT_B) {
+        if (estimate_param.isNull()) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Selected estimate_mode requires a fee rate");
+        }
+
+        CAmount fee_rate = AmountFromValue(estimate_param);
+        if (cc.m_fee_mode == FeeEstimateMode::SAT_B) {
+            fee_rate /= WALLET_BTC_KB_TO_SAT_B;
+        }
+
+        cc.m_feerate = CFeeRate(fee_rate);
+
+        // default RBF to true for explicit fee rate modes
+        if (cc.m_signal_bip125_rbf == boost::none) cc.m_signal_bip125_rbf = true;
+    } else if (!estimate_param.isNull()) {
+        cc.m_confirm_target = ParseConfirmTarget(estimate_param, pwallet->chain().estimateMaxBlocks());
+    }
+}
+
 static UniValue getnewaddress(const JSONRPCRequest& request)
 {
             RPCHelpMan{"getnewaddress",
@@ -369,11 +407,9 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
                     {"subtractfeefromamount", RPCArg::Type::BOOL, /* default */ "false", "The fee will be deducted from the amount being sent.\n"
             "                             The recipient will receive less bitcoins than you enter in the amount field."},
                     {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"},
-                    {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
+                    {"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", "The fee estimate mode, must be one of:\n"
+                    {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n"
-            "       \"UNSET\"\n"
+            "       \"" + FeeModes("\"\n\"") + "\""},
-            "       \"ECONOMICAL\"\n"
-            "       \"CONSERVATIVE\""},
                     {"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"
             "                             dirty if they have previously been used in a transaction."},
                 },
@@ -384,6 +420,8 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
                     HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1")
             + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"donation\" \"seans outpost\"")
             + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"\" \"\" true")
+            + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"\" \"\" false true 0.00002 " + (CURRENCY_UNIT + "/kB"))
+            + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"\" \"\" false true 2 " + (CURRENCY_ATOM + "/B"))
             + HelpExampleRpc("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\", 0.1, \"donation\", \"seans outpost\"")
                 },
             }.Check(request);
@@ -425,20 +463,12 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
         coin_control.m_signal_bip125_rbf = request.params[5].get_bool();
     }
 
-    if (!request.params[6].isNull()) {
-        coin_control.m_confirm_target = ParseConfirmTarget(request.params[6], pwallet->chain().estimateMaxBlocks());
-    }
-
-    if (!request.params[7].isNull()) {
-        if (!FeeModeFromString(request.params[7].get_str(), coin_control.m_fee_mode)) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter");
-        }
-    }
-
     coin_control.m_avoid_address_reuse = GetAvoidReuseFlag(pwallet, request.params[8]);
     // We also enable partial spend avoidance if reuse avoidance is set.
     coin_control.m_avoid_partial_spends |= coin_control.m_avoid_address_reuse;
 
+    SetFeeEstimateMode(pwallet, coin_control, request.params[7], request.params[6]);
+
     EnsureWalletIsUnlocked(pwallet);
 
     CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue));
@@ -780,11 +810,9 @@ static UniValue sendmany(const JSONRPCRequest& request)
                         },
                     },
                     {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"},
-                    {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
+                    {"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", "The fee estimate mode, must be one of:\n"
+                    {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n"
-            "       \"UNSET\"\n"
+            "       \"" + FeeModes("\"\n\"") + "\""},
-            "       \"ECONOMICAL\"\n"
-            "       \"CONSERVATIVE\""},
                 },
                  RPCResult{
                      RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n"
@@ -830,15 +858,7 @@ static UniValue sendmany(const JSONRPCRequest& request)
         coin_control.m_signal_bip125_rbf = request.params[5].get_bool();
     }
 
-    if (!request.params[6].isNull()) {
+    SetFeeEstimateMode(pwallet, coin_control, request.params[7], request.params[6]);
-        coin_control.m_confirm_target = ParseConfirmTarget(request.params[6], pwallet->chain().estimateMaxBlocks());
-    }
-
-    if (!request.params[7].isNull()) {
-        if (!FeeModeFromString(request.params[7].get_str(), coin_control.m_fee_mode)) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter");
-        }
-    }
 
     std::set<CTxDestination> destinations;
     std::vector<CRecipient> vecSend;
@@ -2986,6 +3006,12 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
 
         if (options.exists("feeRate"))
         {
+            if (options.exists("conf_target")) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate");
+            }
+            if (options.exists("estimate_mode")) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and feeRate");
+            }
             coinControl.m_feerate = CFeeRate(AmountFromValue(options["feeRate"]));
             coinControl.fOverrideFeeRate = true;
         }
@@ -2996,20 +3022,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
         if (options.exists("replaceable")) {
             coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool();
         }
-        if (options.exists("conf_target")) {
+        SetFeeEstimateMode(pwallet, coinControl, options["estimate_mode"], options["conf_target"]);
-            if (options.exists("feeRate")) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate");
-            }
-            coinControl.m_confirm_target = ParseConfirmTarget(options["conf_target"], pwallet->chain().estimateMaxBlocks());
-        }
-        if (options.exists("estimate_mode")) {
-            if (options.exists("feeRate")) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and feeRate");
-            }
-            if (!FeeModeFromString(options["estimate_mode"].get_str(), coinControl.m_fee_mode)) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter");
-            }
-        }
       }
     } else {
         // if options is null and not a bool
@@ -3077,11 +3090,9 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request)
                             },
                             {"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"},
-                            {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
+                            {"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", "The fee estimate mode, must be one of:\n"
+                            {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n"
-                            "         \"UNSET\"\n"
+                            "       \"" + FeeModes("\"\n\"") + "\""},
-                            "         \"ECONOMICAL\"\n"
-                            "         \"CONSERVATIVE\""},
                         },
                         "options"},
                     {"iswitness", RPCArg::Type::BOOL, /* default */ "depends on heuristic tests", "Whether the transaction hex is a serialized witness transaction.\n"
@@ -3249,8 +3260,8 @@ static UniValue bumpfee(const JSONRPCRequest& request)
                     {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"},
                     {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
                         {
-                            {"confTarget", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
+                            {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
-                            {"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'confTarget'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n"
+                            {"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'conf_target'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n"
             "                         Specify a fee rate instead of relying on the built-in fee estimator.\n"
                                      "Must be at least 0.0001 " + CURRENCY_UNIT + " per kB higher than the current transaction fee rate.\n"},
                             {"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n"
@@ -3260,10 +3271,8 @@ static UniValue bumpfee(const JSONRPCRequest& request)
             "                         so the new transaction will not be explicitly bip-125 replaceable (though it may\n"
             "                         still be replaceable in practice, for example if it has unconfirmed ancestors which\n"
             "                         are replaceable)."},
-                            {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n"
+                            {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n"
-            "         \"UNSET\"\n"
+            "         \"" + FeeModes("\"\n\"") + "\""},
-            "         \"ECONOMICAL\"\n"
-            "         \"CONSERVATIVE\""},
                         },
                         "options"},
                 },
@@ -3302,15 +3311,24 @@ static UniValue bumpfee(const JSONRPCRequest& request)
         RPCTypeCheckObj(options,
             {
                 {"confTarget", UniValueType(UniValue::VNUM)},
+                {"conf_target", UniValueType(UniValue::VNUM)},
                 {"fee_rate", UniValueType(UniValue::VNUM)},
                 {"replaceable", UniValueType(UniValue::VBOOL)},
                 {"estimate_mode", UniValueType(UniValue::VSTR)},
             },
             true, true);
-        if (options.exists("confTarget") && options.exists("fee_rate")) {
+
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.");
+        if (options.exists("confTarget") && options.exists("conf_target")) {
-        } else if (options.exists("confTarget")) { // TODO: alias this to conf_target
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget and conf_target options should not both be set. Use conf_target (confTarget is deprecated).");
-            coin_control.m_confirm_target = ParseConfirmTarget(options["confTarget"], pwallet->chain().estimateMaxBlocks());
+        }
+
+        auto conf_target = options.exists("confTarget") ? options["confTarget"] : options["conf_target"];
+
+        if (!conf_target.isNull()) {
+            if (options.exists("fee_rate")) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "conf_target can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.");
+            }
+            coin_control.m_confirm_target = ParseConfirmTarget(conf_target, pwallet->chain().estimateMaxBlocks());
         } else if (options.exists("fee_rate")) {
             CFeeRate fee_rate(AmountFromValue(options["fee_rate"]));
             if (fee_rate <= CFeeRate(0)) {
@@ -3322,11 +3340,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
         if (options.exists("replaceable")) {
             coin_control.m_signal_bip125_rbf = options["replaceable"].get_bool();
         }
-        if (options.exists("estimate_mode")) {
+        SetFeeEstimateMode(pwallet, coin_control, options["estimate_mode"], conf_target);
-            if (!FeeModeFromString(options["estimate_mode"].get_str(), coin_control.m_fee_mode)) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter");
-            }
-        }
     }
 
     // Make sure the results are valid at least up to the most recent block
@@ -4016,10 +4030,8 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
                             {"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"},
                             {"conf_target", RPCArg::Type::NUM, /* default */ "fall back to wallet's confirmation target (txconfirmtarget)", "Confirmation target (in blocks)"},
-                            {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n"
+                            {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n"
-                            "         \"UNSET\"\n"
+                            "         \"" + FeeModes("\"\n\"") + "\""},
-                            "         \"ECONOMICAL\"\n"
-                            "         \"CONSERVATIVE\""},
                         },
                         "options"},
                     {"bip32derivs", RPCArg::Type::BOOL, /* default */ "true", "Include BIP 32 derivation paths for public keys if we know them"},

test/functional/wallet_basic.py

@@ -219,6 +219,60 @@ class WalletTest(BitcoinTestFramework):
         assert_equal(self.nodes[2].getbalance(), node_2_bal)
         node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex']))
 
+        # Sendmany with explicit fee (BTC/kB)
+        # Throw if no conf_target provided
+        assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate",
+            self.nodes[2].sendmany,
+            amounts={ address: 10 },
+            estimate_mode='bTc/kB')
+        # Throw if negative feerate
+        assert_raises_rpc_error(-3, "Amount out of range",
+            self.nodes[2].sendmany,
+            amounts={ address: 10 },
+            conf_target=-1,
+            estimate_mode='bTc/kB')
+        fee_per_kb = 0.0002500
+        explicit_fee_per_byte = Decimal(fee_per_kb) / 1000
+        txid = self.nodes[2].sendmany(
+            amounts={ address: 10 },
+            conf_target=fee_per_kb,
+            estimate_mode='bTc/kB',
+        )
+        self.nodes[2].generate(1)
+        self.sync_all(self.nodes[0:3])
+        node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), node_2_bal - Decimal('10'), explicit_fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex']))
+        assert_equal(self.nodes[2].getbalance(), node_2_bal)
+        node_0_bal += Decimal('10')
+        assert_equal(self.nodes[0].getbalance(), node_0_bal)
+
+        # Sendmany with explicit fee (SAT/B)
+        # Throw if no conf_target provided
+        assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate",
+            self.nodes[2].sendmany,
+            amounts={ address: 10 },
+            estimate_mode='sat/b')
+        # Throw if negative feerate
+        assert_raises_rpc_error(-3, "Amount out of range",
+            self.nodes[2].sendmany,
+            amounts={ address: 10 },
+            conf_target=-1,
+            estimate_mode='sat/b')
+        fee_sat_per_b = 2
+        fee_per_kb = fee_sat_per_b / 100000.0
+        explicit_fee_per_byte = Decimal(fee_per_kb) / 1000
+        txid = self.nodes[2].sendmany(
+            amounts={ address: 10 },
+            conf_target=fee_sat_per_b,
+            estimate_mode='sAT/b',
+        )
+        self.nodes[2].generate(1)
+        self.sync_all(self.nodes[0:3])
+        balance = self.nodes[2].getbalance()
+        node_2_bal = self.check_fee_amount(balance, node_2_bal - Decimal('10'), explicit_fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex']))
+        assert_equal(balance, node_2_bal)
+        node_0_bal += Decimal('10')
+        assert_equal(self.nodes[0].getbalance(), node_0_bal)
+
         self.start_node(3, self.nodes[3].extra_args)
         connect_nodes(self.nodes[0], 3)
         self.sync_all()
@@ -349,6 +403,74 @@ class WalletTest(BitcoinTestFramework):
             self.nodes[0].generate(1)
             self.sync_all(self.nodes[0:3])
 
+            # send with explicit btc/kb fee
+            self.log.info("test explicit fee (sendtoaddress as btc/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",
+                self.nodes[2].sendtoaddress,
+                address=address,
+                amount=1.0,
+                estimate_mode='BTc/Kb')
+            # Throw if negative feerate
+            assert_raises_rpc_error(-3, "Amount out of range",
+                self.nodes[2].sendtoaddress,
+                address=address,
+                amount=1.0,
+                conf_target=-1,
+                estimate_mode='btc/kb')
+            txid = self.nodes[2].sendtoaddress(
+                address=address,
+                amount=1.0,
+                conf_target=0.00002500,
+                estimate_mode='btc/kb',
+            )
+            tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])
+            self.sync_all(self.nodes[0:3])
+            self.nodes[0].generate(1)
+            self.sync_all(self.nodes[0:3])
+            postbalance = self.nodes[2].getbalance()
+            fee = prebalance - postbalance - Decimal('1')
+            assert_fee_amount(fee, tx_size, Decimal('0.00002500'))
+
+            # send with explicit sat/b fee
+            self.sync_all(self.nodes[0:3])
+            self.log.info("test explicit fee (sendtoaddress as sat/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",
+                self.nodes[2].sendtoaddress,
+                address=address,
+                amount=1.0,
+                estimate_mode='SAT/b')
+            # Throw if negative feerate
+            assert_raises_rpc_error(-3, "Amount out of range",
+                self.nodes[2].sendtoaddress,
+                address=address,
+                amount=1.0,
+                conf_target=-1,
+                estimate_mode='SAT/b')
+            txid = self.nodes[2].sendtoaddress(
+                address=address,
+                amount=1.0,
+                conf_target=2,
+                estimate_mode='SAT/B',
+            )
+            tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])
+            self.sync_all(self.nodes[0:3])
+            self.nodes[0].generate(1)
+            self.sync_all(self.nodes[0:3])
+            postbalance = self.nodes[2].getbalance()
+            fee = prebalance - postbalance - Decimal('1')
+            assert_fee_amount(fee, tx_size, Decimal('0.00002000'))
+
             # 2. Import address from node2 to node1
             self.nodes[1].importaddress(address_to_import)
 

test/functional/wallet_bumpfee.py

@@ -71,6 +71,7 @@ class BumpFeeTest(BitcoinTestFramework):
 
         self.log.info("Running tests")
         dest_address = peer_node.getnewaddress()
+        test_invalid_parameters(rbf_node, dest_address)
         test_simple_bumpfee_succeeds(self, "default", rbf_node, peer_node, dest_address)
         test_simple_bumpfee_succeeds(self, "fee_rate", rbf_node, peer_node, dest_address)
         test_feerate_args(self, rbf_node, peer_node, dest_address)
@@ -92,6 +93,28 @@ class BumpFeeTest(BitcoinTestFramework):
         test_small_output_with_feerate_succeeds(self, rbf_node, dest_address)
         test_no_more_inputs_fails(self, rbf_node, dest_address)
 
+def test_invalid_parameters(node, dest_address):
+    txid = spend_one_input(node, dest_address)
+    # invalid estimate mode
+    assert_raises_rpc_error(-8, "Invalid estimate_mode parameter", node.bumpfee, txid, {
+        "estimate_mode": "moo",
+    })
+    assert_raises_rpc_error(-3, "Expected type string", node.bumpfee, txid, {
+        "estimate_mode": 38,
+    })
+    assert_raises_rpc_error(-3, "Expected type string", node.bumpfee, txid, {
+        "estimate_mode": {
+            "foo": "bar",
+        },
+    })
+    assert_raises_rpc_error(-8, "Invalid estimate_mode parameter", node.bumpfee, txid, {
+        "estimate_mode": Decimal("3.141592"),
+    })
+    # confTarget and conf_target
+    assert_raises_rpc_error(-8, "confTarget and conf_target options should not both be set", node.bumpfee, txid, {
+        "confTarget": 123,
+        "conf_target": 456,
+    })
 
 def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
     self.log.info('Test simple bumpfee: {}'.format(mode))
@@ -127,9 +150,10 @@ def test_feerate_args(self, rbf_node, peer_node, dest_address):
     self.sync_mempools((rbf_node, peer_node))
     assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool()
 
-    assert_raises_rpc_error(-8, "confTarget can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"fee_rate": NORMAL, "confTarget": 1})
+    assert_raises_rpc_error(-8, "conf_target can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"fee_rate": NORMAL, "confTarget": 1})
 
     assert_raises_rpc_error(-3, "Unexpected key totalFee", rbf_node.bumpfee, rbfid, {"totalFee": NORMAL})
+    assert_raises_rpc_error(-8, "conf_target can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"fee_rate":0.00001, "confTarget": 1})
 
     # Bumping to just above minrelay should fail to increase total fee enough, at least
     assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, rbfid, {"fee_rate": INSUFFICIENT})