From 6799cd5d0b2bc270183660c8944d56daa8661626 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Apr 2023 14:09:05 +0930 Subject: [PATCH] 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 --- doc/lightning-listconfigs.7.md | 3 ++- doc/lightningd-config.5.md | 2 +- doc/schemas/listconfigs.schema.json | 5 +++++ lightningd/bitcoind.c | 9 ++++++++- lightningd/lightningd.h | 7 +++++++ lightningd/options.c | 18 ++++++++++++++++++ plugins/bcli.c | 26 ++------------------------ tests/test_misc.py | 19 ++++++++++++++----- 8 files changed, 57 insertions(+), 32 deletions(-) diff --git a/doc/lightning-listconfigs.7.md b/doc/lightning-listconfigs.7.md index fe9965746..f59e05719 100644 --- a/doc/lightning-listconfigs.7.md +++ b/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: -[comment]: # ( SHA256STAMP:1088401b9aeae1e079dab550d3b035ef82195f0466ad471bc7373386182f37dc) +[comment]: # ( SHA256STAMP:b24158a61bb79aaf3f0f6d1c20a4b10d474613b371e80aede4aeb59ab471a989) diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 0621ccb72..4ba84d860 100644 --- a/doc/lightningd-config.5.md +++ b/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. diff --git a/doc/schemas/listconfigs.schema.json b/doc/schemas/listconfigs.schema.json index b2ebdd003..32e7b096d 100644 --- a/doc/schemas/listconfigs.schema.json +++ b/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" } } } diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 1baec3c58..fb7b09649 100644 --- a/lightningd/bitcoind.c +++ b/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); diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index 0a57dbb97..e5aac115a 100644 --- a/lightningd/lightningd.h +++ b/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; diff --git a/lightningd/options.c b/lightningd/options.c index ee3d56001..101077a9f 100644 --- a/lightningd/options.c +++ b/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. " diff --git a/plugins/bcli.c b/plugins/bcli.c index a79b67b19..57ab9edf4 100644 --- a/plugins/bcli.c +++ b/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] - * bitcoind->max_fee_multiplier); + stash->perkb[FEERATE_HIGHEST] * 10); 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", diff --git a/tests/test_misc.py b/tests/test_misc.py index de40b3f4c..50bb48333 100644 --- a/tests/test_misc.py +++ b/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) - l1_commit_fees = l1.rpc.call("estimatefees")["unilateral_close"] - l2_commit_fees = l2.rpc.call("estimatefees")["unilateral_close"] + l.set_feerates((0, mock_wu, 0, 0), False) + l.start() - 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):