wallet: introduce setfeerate, an improved settxfee in sat/vB

jonatack · jonatack · commit 8e863e3d3ce4 · 2021-01-28T21:21:21.000+01:00
diff --git a/doc/release-notes-20391.md b/doc/release-notes-20391.md
@@ -0,0 +1,9 @@
+New RPCs
+------------
+
+- A new `setfeerate` RPC is added to set the transaction fee for a wallet in
+  sat/vB, instead of in BTC/kvB like `settxfee`. This RPC is intended to replace
+  `settxfee`, which becomes a hidden RPC and is marked as deprecated. This
+  change is part of a larger migration from BTC/kvB to sat/vB units for fee
+  rates. (#20391)
+
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
@@ -43,6 +43,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "sendtoaddress", 8, "avoid_reuse" },
     { "sendtoaddress", 9, "fee_rate"},
     { "sendtoaddress", 10, "verbose"},
+    { "setfeerate", 0, "amount" },
     { "settxfee", 0, "amount" },
     { "sethdseed", 0, "newkeypool" },
     { "getreceivedbyaddress", 1, "minconf" },
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -2349,6 +2349,81 @@ static RPCHelpMan settxfee()
     };
 }
 
+static RPCHelpMan setfeerate()
+{
+    return RPCHelpMan{
+        "setfeerate",
+        "\nSet the transaction fee rate in " + CURRENCY_ATOM + "/vB for this wallet.\n"
+        "Overrides the global -paytxfee configuration option. Like -paytxfee, it is not persisted after bitcoind shutdown/restart.\n"
+        "Can be deactivated by passing 0 as the fee rate, in which case automatic fee selection will be used by default.\n",
+        {
+            {"amount", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The transaction fee rate in " + CURRENCY_ATOM + "/vB to set (0 to unset)"},
+        },
+        RPCResult{
+            RPCResult::Type::OBJ, "", "",
+            {
+                {RPCResult::Type::STR, "wallet_name", "Name of the wallet the fee rate setting applies to"},
+                {RPCResult::Type::NUM, "fee_rate", "Fee rate in " + CURRENCY_ATOM + "/vB for the wallet after this operation"},
+                {RPCResult::Type::STR, "result", /* optional */ true, "Description of result, if successful"},
+                {RPCResult::Type::STR, "error", /* optional */ true, "Description of error, if any"},
+            },
+        },
+        RPCExamples{
+            ""
+            "\nSet a fee rate of 1 " + CURRENCY_ATOM + "/vB\n"
+            + HelpExampleCli("setfeerate", "1") +
+            "\nSet a fee rate of 3.141 " + CURRENCY_ATOM + "/vB\n"
+            + HelpExampleCli("setfeerate", "3.141") +
+            "\nSet a fee rate of 7.75 " + CURRENCY_ATOM + "/vB with named arguments\n"
+            + HelpExampleCli("-named setfeerate", "amount=\"7.75\"") +
+            "\nSet a fee rate of 25 " + CURRENCY_ATOM + "/vB with the RPC\n"
+            + HelpExampleRpc("setfeerate", "25")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
+            std::shared_ptr<CWallet> const rpc_wallet{GetWalletForJSONRPCRequest(request)};
+            if (!rpc_wallet) return NullUniValue;
+            CWallet& wallet = *rpc_wallet;
+
+            LOCK(wallet.cs_wallet);
+
+            const CFeeRate amount{CFeeRate::FromSatB(AmountFromValue(request.params[0]))};
+            const CFeeRate relay_min_feerate{wallet.chain().relayMinFee().GetFeePerK()};
+            const CFeeRate wallet_min_feerate{wallet.m_min_fee.GetFeePerK()};
+            const CFeeRate wallet_max_feerate{CFeeRate::FromBtcKb(wallet.m_default_max_tx_fee)};
+            const CFeeRate zero{CFeeRate{0}};
+            const std::string amount_str{amount.ToString(FeeEstimateMode::SAT_VB)};
+            const std::string current_setting{strprintf("The current setting of %s for this wallet remains unchanged.", wallet.m_pay_tx_fee == zero ? "0 (unset)" : wallet.m_pay_tx_fee.ToString(FeeEstimateMode::SAT_VB))};
+            std::string result, error;
+
+            if (amount == zero) {
+                if (request.params[0].get_real() != 0) throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount");
+                wallet.m_pay_tx_fee = amount;
+                result = "Fee rate for transactions with this wallet successfully unset. By default, automatic fee selection will be used.";
+            } else if (amount < relay_min_feerate) {
+                error = strprintf("The requested fee rate of %s cannot be less than the minimum relay fee rate of %s. %s", amount_str, relay_min_feerate.ToString(FeeEstimateMode::SAT_VB), current_setting);
+            } else if (amount < wallet_min_feerate) {
+                error = strprintf("The requested fee rate of %s cannot be less than the wallet min fee rate of %s. %s", amount_str, wallet_min_feerate.ToString(FeeEstimateMode::SAT_VB), current_setting);
+            } else if (amount > wallet_max_feerate) {
+                error = strprintf("The requested fee rate of %s cannot be greater than the wallet max fee rate of %s. %s", amount_str, wallet_max_feerate.ToString(FeeEstimateMode::SAT_VB), current_setting);
+            } else {
+                wallet.m_pay_tx_fee = amount;
+                result = "Fee rate for transactions with this wallet successfully set to " + amount_str;
+            }
+            CHECK_NONFATAL(result.empty() != error.empty());
+
+            UniValue obj{UniValue::VOBJ};
+            obj.pushKV("wallet_name", wallet.GetName());
+            obj.pushKV("fee_rate", ValueFromFeeRate(wallet.m_pay_tx_fee));
+            if (error.empty()) {
+                obj.pushKV("result", result);
+            } else {
+                obj.pushKV("error", error);
+            }
+            return obj;
+        },
+    };
+}
+
 static RPCHelpMan getbalances()
 {
     return RPCHelpMan{
@@ -4607,6 +4682,7 @@ static const CRPCCommand commands[] =
     { "wallet",             &send,                           },
     { "wallet",             &sendmany,                       },
     { "wallet",             &sendtoaddress,                  },
+    { "wallet",             &setfeerate,                     },
     { "wallet",             &sethdseed,                      },
     { "wallet",             &setlabel,                       },
     { "wallet",             &settxfee,                       },
diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py
@@ -86,6 +86,7 @@ def test_client_conversion_table(self):
                 assert argname == 'dummy', ('WARNING: conversion mismatch for argument named %s (%s)' % (argname, list(zip(all_methods_by_argname[argname], converts_by_argname[argname]))))
 
     def test_categories(self):
+        self.log.info("Test RPC help categories")
         node = self.nodes[0]
 
         # wrong argument count
@@ -111,6 +112,7 @@ def test_categories(self):
         assert_equal(titles, components)
 
     def dump_help(self):
+        self.log.info("Test dump RPC help")
         dump_dir = os.path.join(self.options.tmpdir, 'rpc_help_dump')
         os.mkdir(dump_dir)
         calls = [line.split(' ', 1)[0] for line in self.nodes[0].help().splitlines() if line and not line.startswith('==')]
@@ -120,9 +122,11 @@ def dump_help(self):
                 f.write(self.nodes[0].help(call))
 
     def wallet_help(self):
+        self.log.info("Test wallet RPC help")
         assert 'getnewaddress ( "label" "address_type" )' in self.nodes[0].help('getnewaddress')
         self.restart_node(0, extra_args=['-nowallet=1'])
         assert 'getnewaddress ( "label" "address_type" )' in self.nodes[0].help('getnewaddress')
+        assert "setfeerate amount" in self.nodes[0].help("setfeerate")
 
 
 if __name__ == '__main__':
