plugins/bcli: move commit-fee (dev-max-fee-multiplier) and into core.

Turns out the two bcli replacements I checked (`sauron` and
`trustedcoin`) don't even implement this, and the multiplier makes
more sense in lightningd, especially as we move to bcli just providing
raw feerate estimates.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

doc/lightning-listconfigs.7.md

@@ -106,6 +106,7 @@ On success, an object is returned, containing:
 - **dev-allowdustreserve** (boolean, optional): Whether we allow setting dust reserves
 - **announce-addr-dns** (boolean, optional): Whether we put DNS entries into node\_announcement *(added v22.11.1)*
 - **require-confirmed-inputs** (boolean, optional): Request peers to only send confirmed inputs (dual-fund only)
+- **commit-fee** (u64, optional): The percentage of the 6-block fee estimate to use for commitment transactions *(added v23.05)*
 
 [comment]: # (GENERATE-FROM-SCHEMA-END)
 
@@ -224,4 +225,4 @@ RESOURCES
 
 Main web site: <https://github.com/ElementsProject/lightning>
 
-[comment]: # ( SHA256STAMP:1088401b9aeae1e079dab550d3b035ef82195f0466ad471bc7373386182f37dc)
+[comment]: # ( SHA256STAMP:b24158a61bb79aaf3f0f6d1c20a4b10d474613b371e80aede4aeb59ab471a989)

doc/lightningd-config.5.md

@@ -401,7 +401,7 @@ create a channel, and if an HTLC asks for longer, we'll refuse it.
   Confirmations required for the funding transaction when the other side
 opens a channel before the channel is usable.
 
-* **commit-fee**=*PERCENT* [plugin `bcli`]
+* **commit-fee**=*PERCENT*
 
   The percentage of *estimatesmartfee 2/CONSERVATIVE* to use for the commitment
 transactions: default is 100.

doc/schemas/listconfigs.schema.json

@@ -319,6 +319,11 @@
     "require-confirmed-inputs": {
       "type": "boolean",
       "description": "Request peers to only send confirmed inputs (dual-fund only)"
+    },
+    "commit-fee": {
+      "type": "u64",
+      "added": "v23.05",
+      "description": "The percentage of the 6-block fee estimate to use for commitment transactions"
     }
   }
 }

lightningd/bitcoind.c

@@ -215,10 +215,17 @@ static void estimatefees_callback(const char *buf, const jsmntok_t *toks,
 			else
 				feerates[f] = 0;
 #endif
-		} else
+		} else {
+			if (f == FEERATE_UNILATERAL_CLOSE) {
+				feerates[f] = feerates[f] * call->bitcoind->ld->config.commit_fee_percent / 100;
+			} else if (f == FEERATE_MAX) {
+				/* Plugins always use 10 as multiplier. */
+				feerates[f] = feerates[f] * call->bitcoind->ld->config.max_fee_multiplier / 10;
+			}
 			/* Rate in satoshi per kw. */
 			feerates[f] = feerate_from_style(feerates[f],
 							 FEERATE_PER_KBYTE);
+		}
 	}
 
 	call->cb(call->bitcoind, feerates, call->arg);

lightningd/lightningd.h

@@ -85,6 +85,13 @@ struct config {
 
 	/* Require peer to send confirmed inputs */
 	bool require_confirmed_inputs;
+
+	/* The factor to time the urgent feerate by to get the maximum
+	 * acceptable feerate.  (10, but can be overridden by dev-max-fee-multiplier) */
+	u32 max_fee_multiplier;
+
+	/* Percent of CONSERVATIVE/2 feerate we'll use for commitment txs. */
+	u64 commit_fee_percent;
 };
 
 typedef STRMAP(const char *) alt_subdaemon_map;

lightningd/options.c

@@ -799,6 +799,15 @@ static void dev_register_opts(struct lightningd *ld)
 			 opt_show_uintval,
 			 &dev_onion_reply_length,
 			 "Send onion errors of custom length");
+	opt_register_arg("--dev-max-fee-multiplier",
+			 opt_set_uintval,
+			 opt_show_uintval,
+			 &ld->config.max_fee_multiplier,
+			 "Allow the fee proposed by the remote end to"
+			 " be up to multiplier times higher than our "
+			 "own. Small values will cause channels to be"
+			 " closed more often due to fee fluctuations,"
+			 " large values may result in large fees.");
 }
 #endif /* DEVELOPER */
 
@@ -860,6 +869,9 @@ static const struct config testnet_config = {
 	.allowdustreserve = false,
 
 	.require_confirmed_inputs = false,
+
+	.max_fee_multiplier = 10,
+	.commit_fee_percent = 100,
 };
 
 /* aka. "Dude, where's my coins?" */
@@ -931,6 +943,9 @@ static const struct config mainnet_config = {
 	.allowdustreserve = false,
 
 	.require_confirmed_inputs = false,
+
+	.max_fee_multiplier = 10,
+	.commit_fee_percent = 100,
 };
 
 static void check_config(struct lightningd *ld)
@@ -1287,6 +1302,9 @@ static void register_opts(struct lightningd *ld)
 			 opt_force_feerates, NULL, ld,
 			 "Set testnet/regtest feerates in sats perkw, opening/mutual_close/unlateral_close/delayed_to_us/htlc_resolution/penalty: if fewer specified, last number applies to remainder");
 
+	opt_register_arg("--commit-fee",
+			 opt_set_u64, opt_show_u64, &ld->config.commit_fee_percent,
+			 "Percentage of fee to request for their commitment");
 	opt_register_arg("--subdaemon", opt_subdaemon, NULL,
 			 ld, "Arg specified as SUBDAEMON:PATH. "
 			 "Specifies an alternate subdaemon binary. "

plugins/bcli.c

@@ -60,13 +60,6 @@ struct bitcoind {
 	/* Passthrough parameters for bitcoin-cli */
 	char *rpcuser, *rpcpass, *rpcconnect, *rpcport;
 
-	/* The factor to time the urgent feerate by to get the maximum
-	 * acceptable feerate. */
-	u32 max_fee_multiplier;
-
-	/* Percent of CONSERVATIVE/2 feerate we'll use for commitment txs. */
-	u64 commit_fee_percent;
-
 	/* Whether we fake fees (regtest) */
 	bool fake_fees;
 
@@ -718,7 +711,7 @@ static struct command_result *estimatefees_next(struct command *cmd,
 	json_add_feerate(response, "mutual_close", cmd, stash,
 			 stash->perkb[FEERATE_SLOW]);
 	json_add_feerate(response, "unilateral_close", cmd, stash,
-			 stash->perkb[FEERATE_URGENT] * bitcoind->commit_fee_percent / 100);
+			 stash->perkb[FEERATE_URGENT]);
 	json_add_feerate(response, "delayed_to_us", cmd, stash,
 			 stash->perkb[FEERATE_NORMAL]);
 	json_add_feerate(response, "htlc_resolution", cmd, stash,
@@ -736,8 +729,7 @@ static struct command_result *estimatefees_next(struct command *cmd,
 	 * margin (say 5x the expected fee requirement)
 	 */
 	json_add_feerate(response, "max_acceptable", cmd, stash,
-			 stash->perkb[FEERATE_HIGHEST]
+			 stash->perkb[FEERATE_HIGHEST] * 10);
-			 * bitcoind->max_fee_multiplier);
 	return command_finished(cmd, response);
 }
 
@@ -1063,8 +1055,6 @@ static struct bitcoind *new_bitcoind(const tal_t *ctx)
 	bitcoind->rpcpass = NULL;
 	bitcoind->rpcconnect = NULL;
 	bitcoind->rpcport = NULL;
-	bitcoind->max_fee_multiplier = 10;
-	bitcoind->commit_fee_percent = 100;
 #if DEVELOPER
 	bitcoind->no_fake_fees = false;
 #endif
@@ -1111,19 +1101,7 @@ int main(int argc, char *argv[])
 				  "how long to keep retrying to contact bitcoind"
 				  " before fatally exiting",
 				  u64_option, &bitcoind->retry_timeout),
-		    plugin_option("commit-fee",
-				  "string",
-				  "Percentage of fee to request for their commitment",
-				  u64_option, &bitcoind->commit_fee_percent),
 #if DEVELOPER
-		    plugin_option("dev-max-fee-multiplier",
-				  "string",
-				  "Allow the fee proposed by the remote end to"
-				  " be up to multiplier times higher than our "
-				  "own. Small values will cause channels to be"
-				  " closed more often due to fee fluctuations,"
-				  " large values may result in large fees.",
-				  u32_option, &bitcoind->max_fee_multiplier),
 		    plugin_option("dev-no-fake-fees",
 				  "bool",
 				  "Suppress fee faking for regtest",

tests/test_misc.py

@@ -2653,15 +2653,24 @@ def test_restorefrompeer(node_factory, bitcoind):
 
 def test_commitfee_option(node_factory):
     """Sanity check for the --commit-fee startup option."""
-    l1, l2 = node_factory.get_nodes(2, opts=[{"commit-fee": "200"}, {}])
+    l1, l2 = node_factory.get_nodes(2, opts=[{"commit-fee": "200",
+                                              "start": False},
+                                             {"start": False}])
 
+    # set_feerates multiplies this by 4 to get perkb; but we divide.
     mock_wu = 5000
     for l in [l1, l2]:
-        l.set_feerates((0, mock_wu, 0, 0), True)
+        l.set_feerates((0, mock_wu, 0, 0), False)
-    l1_commit_fees = l1.rpc.call("estimatefees")["unilateral_close"]
+        l.start()
-    l2_commit_fees = l2.rpc.call("estimatefees")["unilateral_close"]
 
-    assert l1_commit_fees == 2 * l2_commit_fees == 2 * 4 * mock_wu  # WU->VB
+    # plugin gives same results:
+    assert l1.rpc.call("estimatefees") == l2.rpc.call("estimatefees")
+
+    # But feerates differ.
+    l1_commit_fees = l1.rpc.feerates("perkw")['perkw']['unilateral_close']
+    l2_commit_fees = l2.rpc.feerates("perkw")['perkw']['unilateral_close']
+
+    assert l1_commit_fees == 2 * l2_commit_fees == 2 * mock_wu
 
 
 def test_listtransactions(node_factory):