From 6ee8c40b29a57a66a34bd08fe0150d605fe4612d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 8 Sep 2021 14:11:46 +0930 Subject: [PATCH] closing: add option to set closing range. This affects the range we offer even without quick-close, but it's more critical for quick-close. Signed-off-by: Rusty Russell Changelog-Added: JSONRPC: `close` now takes a `feerange` parameter to set min/max fee rates for mutual close. --- closingd/closingd.c | 1 + contrib/pyln-client/pyln/client/lightning.py | 3 +- doc/lightning-close.7 | 31 ++++++++++++++++++-- doc/lightning-close.7.md | 25 ++++++++++++++-- lightningd/channel.c | 2 ++ lightningd/channel.h | 3 ++ lightningd/closing_control.c | 13 ++++++-- lightningd/peer_control.c | 30 +++++++++++++++++++ lightningd/test/run-invoice-select-inchan.c | 5 ++++ tests/test_closing.py | 22 ++++++++++++++ wallet/db_postgres_sqlgen.c | 2 +- wallet/db_sqlite3_sqlgen.c | 2 +- wallet/statements_gettextgen.po | 6 ++-- wallet/test/run-wallet.c | 5 ++++ 14 files changed, 136 insertions(+), 14 deletions(-) diff --git a/closingd/closingd.c b/closingd/closingd.c index badeb74d8..b71b2c60a 100644 --- a/closingd/closingd.c +++ b/closingd/closingd.c @@ -622,6 +622,7 @@ static void calc_fee_bounds(size_t expected_weight, /* option_anchor_outputs sets commitment_fee to max, so this * doesn't do anything */ if (amount_sat_greater(*maxfee, commitment_fee)) { + /* FIXME: would be nice to notify close cmd here! */ status_unusual("Maximum feerate %u would give fee %s:" " we must limit it to the final commitment fee %s", *max_feerate, diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index 951cca349..e3f780e1f 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -516,7 +516,7 @@ class LightningRpc(UnixDomainSocketRpc): return self.call("check", payload) def close(self, peer_id, unilateraltimeout=None, destination=None, - fee_negotiation_step=None, force_lease_closed=None): + fee_negotiation_step=None, force_lease_closed=None, feerange=None): """ Close the channel with peer {id}, forcing a unilateral close after {unilateraltimeout} seconds if non-zero, and @@ -533,6 +533,7 @@ class LightningRpc(UnixDomainSocketRpc): "destination": destination, "fee_negotiation_step": fee_negotiation_step, "force_lease_closed": force_lease_closed, + "feerange": feerange, } return self.call("close", payload) diff --git a/doc/lightning-close.7 b/doc/lightning-close.7 index 61b03701c..1148fc571 100644 --- a/doc/lightning-close.7 +++ b/doc/lightning-close.7 @@ -3,7 +3,7 @@ lightning-close - Command for closing channels with direct peers .SH SYNOPSIS -\fBclose\fR \fIid\fR [\fIunilateraltimeout\fR] [\fIdestination\fR] [\fIfee_negotiation_step\fR] [\fIwrong_funding\fR] [\fIforce_lease_closed\fR] +\fBclose\fR \fIid\fR [\fIunilateraltimeout\fR] [\fIdestination\fR] [\fIfee_negotiation_step\fR] [\fIwrong_funding\fR] [\fIforce_lease_closed\fR] [*feerange*] .SH DESCRIPTION @@ -35,7 +35,12 @@ friends to upgrade! The \fIfee_negotiation_step\fR parameter controls how closing fee negotiation is performed assuming the peer proposes a fee that is -different than our estimate\. On every negotiation step we must give up +different than our estimate\. (Note that using this option +prevents \fBexperimental-quick-close\fR, as the quick-close protocol +does not allow negotiation)\. + + +On every negotiation step we must give up some amount from our proposal towards the peer's proposal\. This parameter can be an integer in which case it is interpreted as number of satoshis to step at a time\. Or it can be an integer followed by "%" to designate @@ -74,6 +79,26 @@ can rescue openings which have been manually miscreated\. unless this flag is passed in\. Defaults to false\. +\fIfeerange\fR is an optional array [ \fImin\fR, \fImax\fR ], indicating the +minimum and maximum feerates to offer\. \fIslow\fR and \fIunilateral_close\fR +are the defaults\. + + +Rates are one of the strings \fIurgent\fR (aim for next block), \fInormal\fR +(next 4 blocks or so) or \fIslow\fR (next 100 blocks or so) to use +lightningd’s internal estimates, or one of the names from +\fBlightning-feerates\fR(7)\. Otherwise, they can be numbers with +an optional suffix: \fIperkw\fR means the number is interpreted as +satoshi-per-kilosipa (weight), and \fIperkb\fR means it is interpreted +bitcoind-style as satoshi-per-kilobyte\. Omitting the suffix is +equivalent to \fIperkb\fR\. + + +Note that the maximum fee will be capped at the final commitment +transaction fee (unless the experimental anchor-outputs option is +negotiated)\. + + The peer needs to be live and connected in order to negotiate a mutual close\. The default of unilaterally closing after 48 hours is usually a reasonable indication that you can no longer contact the peer\. @@ -130,4 +155,4 @@ ZmnSCPxj \fI is mainly responsible\. Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:0af2affd80bd44dcd733b24c17429058c20b0c3ca41d23e06d70487ea759320c +\" SHA256STAMP:94359c9d63107e5558c7f9cec137899a057f2d172df08f4a78de8fe09defb440 diff --git a/doc/lightning-close.7.md b/doc/lightning-close.7.md index 978dfa7d4..1a25b071f 100644 --- a/doc/lightning-close.7.md +++ b/doc/lightning-close.7.md @@ -4,7 +4,7 @@ lightning-close -- Command for closing channels with direct peers SYNOPSIS -------- -**close** *id* \[*unilateraltimeout*\] \[*destination*\] \[*fee_negotiation_step*\] \[*wrong_funding*\] \[*force_lease_closed*\] +**close** *id* \[*unilateraltimeout*\] \[*destination*\] \[*fee_negotiation_step*\] \[*wrong_funding*\] \[*force_lease_closed*\] [\*feerange\*] DESCRIPTION ----------- @@ -33,7 +33,11 @@ friends to upgrade! The *fee_negotiation_step* parameter controls how closing fee negotiation is performed assuming the peer proposes a fee that is -different than our estimate. On every negotiation step we must give up +different than our estimate. (Note that using this option +prevents **experimental-quick-close**, as the quick-close protocol +does not allow negotiation). + +On every negotiation step we must give up some amount from our proposal towards the peer's proposal. This parameter can be an integer in which case it is interpreted as number of satoshis to step at a time. Or it can be an integer followed by "%" to designate @@ -62,6 +66,23 @@ can rescue openings which have been manually miscreated. unless this flag is passed in. Defaults to false. +*feerange* is an optional array [ *min*, *max* ], indicating the +minimum and maximum feerates to offer. *slow* and *unilateral_close* +are the defaults. + +Rates are one of the strings *urgent* (aim for next block), *normal* +(next 4 blocks or so) or *slow* (next 100 blocks or so) to use +lightningd’s internal estimates, or one of the names from +lightning-feerates(7). Otherwise, they can be numbers with +an optional suffix: *perkw* means the number is interpreted as +satoshi-per-kilosipa (weight), and *perkb* means it is interpreted +bitcoind-style as satoshi-per-kilobyte. Omitting the suffix is +equivalent to *perkb*. + +Note that the maximum fee will be capped at the final commitment +transaction fee (unless the experimental anchor-outputs option is +negotiated). + The peer needs to be live and connected in order to negotiate a mutual close. The default of unilaterally closing after 48 hours is usually a reasonable indication that you can no longer contact the peer. diff --git a/lightningd/channel.c b/lightningd/channel.c index a4ead163d..ead10e2f2 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -263,6 +263,7 @@ struct channel *new_unsaved_channel(struct peer *peer, channel->closing_fee_negotiation_step_unit = CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE; channel->shutdown_wrong_funding = NULL; + channel->closing_feerate_range = NULL; /* Channel is connected! */ channel->connected = true; @@ -429,6 +430,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, = CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE; channel->shutdown_wrong_funding = tal_steal(channel, shutdown_wrong_funding); + channel->closing_feerate_range = NULL; if (local_shutdown_scriptpubkey) channel->shutdown_scriptpubkey[LOCAL] = tal_steal(channel, local_shutdown_scriptpubkey); diff --git a/lightningd/channel.h b/lightningd/channel.h index 196119445..8c54a7db5 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -174,6 +174,9 @@ struct channel { /* optional wrong_funding for mutual close */ const struct bitcoin_outpoint *shutdown_wrong_funding; + /* optional feerate min/max for mutual close */ + u32 *closing_feerate_range; + /* Reestablishment stuff: last sent commit and revocation details. */ bool last_was_revoke; struct changed_htlc *last_sent_commit; diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 1edced8fd..ceb04dbb3 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -193,7 +193,7 @@ void peer_start_closingd(struct channel *channel, struct per_peer_state *pps) { u8 *initmsg; - u32 feerate, *max_feerate; + u32 min_feerate, feerate, *max_feerate; struct amount_msat their_msat; struct amount_sat feelimit; int hsmfd; @@ -266,6 +266,14 @@ void peer_start_closingd(struct channel *channel, } else max_feerate = NULL; + min_feerate = feerate_min(ld, NULL); + + /* If they specified feerates in `close`, they apply now! */ + if (channel->closing_feerate_range) { + min_feerate = channel->closing_feerate_range[0]; + max_feerate = &channel->closing_feerate_range[1]; + } + /* BOLT #3: * * Each node offering a signature: @@ -298,8 +306,7 @@ void peer_start_closingd(struct channel *channel, amount_msat_to_sat_round_down(channel->our_msat), amount_msat_to_sat_round_down(their_msat), channel->our_config.dust_limit, - feerate_min(ld, NULL), feerate, - max_feerate, + min_feerate, feerate, max_feerate, feelimit, channel->shutdown_scriptpubkey[LOCAL], channel->shutdown_scriptpubkey[REMOTE], diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index d8a6a8152..98b79493f 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1661,6 +1661,31 @@ static struct command_result *param_outpoint(struct command *cmd, "should be a txid:outnum"); } +static struct command_result *param_feerate_range(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + u32 **feerate_range) +{ + struct command_result *ret; + u32 *rate; + + *feerate_range = tal_arr(cmd, u32, 2); + if (tok->type != JSMN_ARRAY || tok->size != 2) + return command_fail_badparam(cmd, name, buffer, tok, + "should be an array of 2 entries"); + + ret = param_feerate(cmd, name, buffer, tok+1, &rate); + if (ret) + return ret; + (*feerate_range)[0] = *rate; + ret = param_feerate(cmd, name, buffer, tok+2, &rate); + if (ret) + return ret; + (*feerate_range)[1] = *rate; + return NULL; +} + static struct command_result *json_close(struct command *cmd, const char *buffer, const jsmntok_t *obj UNNEEDED, @@ -1674,6 +1699,7 @@ static struct command_result *json_close(struct command *cmd, bool close_script_set, wrong_funding_changed, *force_lease_close; const char *fee_negotiation_step_str; struct bitcoin_outpoint *wrong_funding; + u32 *feerate_range; char* end; bool anysegwit; @@ -1687,6 +1713,7 @@ static struct command_result *json_close(struct command *cmd, p_opt("wrong_funding", param_outpoint, &wrong_funding), p_opt_def("force_lease_closed", param_bool, &force_lease_close, false), + p_opt("feerange", param_feerate_range, &feerate_range), NULL)) return command_param_failed(); @@ -1845,6 +1872,9 @@ static struct command_result *json_close(struct command *cmd, wrong_funding_changed = false; } + /* Works fine if feerate_range is NULL */ + channel->closing_feerate_range = tal_steal(channel, feerate_range); + /* Normal case. * We allow states shutting down and sigexchange; a previous * close command may have timed out, and this current command diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index d16263f54..5a44a0db4 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -534,6 +534,11 @@ struct command_result *param_escaped_string(struct command *cmd UNNEEDED, const jsmntok_t *tok UNNEEDED, const char **str UNNEEDED) { fprintf(stderr, "param_escaped_string called!\n"); abort(); } +/* Generated stub for param_feerate */ +struct command_result *param_feerate(struct command *cmd UNNEEDED, const char *name UNNEEDED, + const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, + u32 **feerate UNNEEDED) +{ fprintf(stderr, "param_feerate called!\n"); abort(); } /* Generated stub for param_label */ struct command_result *param_label(struct command *cmd UNNEEDED, const char *name UNNEEDED, const char * buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, diff --git a/tests/test_closing.py b/tests/test_closing.py index 7b9a51171..a3b8e0ff7 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -3327,3 +3327,25 @@ def test_anysegwit_close_needs_feature(node_factory, bitcoind): l1.rpc.close(l2.info['id'], destination='bcrt1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7k0ylj56') wait_for(lambda: only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['state'] == 'CLOSINGD_COMPLETE') bitcoind.generate_block(1, wait_for_mempool=1) + + +def test_close_feerate_range(node_factory, bitcoind, chainparams): + """Test the quick-close fee range negotiation""" + l1, l2 = node_factory.line_graph(2, opts={'experimental-quick-close': None}) + + # Lowball the range here. + l1.rpc.close(l2.info['id'], feerange=['253perkw', 'normal']) + + if not chainparams['elements']: + l1_range = [138, 4110] + l2_range = [1027, 1000000] + else: + # That fee output is a little chunky. + l1_range = [175, 5212] + l2_range = [1303, 1000000] + + l1.daemon.wait_for_log('Negotiating closing fee between {}sat and {}sat satoshi'.format(l1_range[0], l1_range[1])) + l2.daemon.wait_for_log('Negotiating closing fee between {}sat and {}sat satoshi'.format(l2_range[0], l2_range[1])) + + overlap = [max(l1_range[0], l2_range[0]), min(l1_range[1], l2_range[1])] + l1.daemon.wait_for_log('performing quickclose in range {}sat-{}sat'.format(overlap[0], overlap[1])) diff --git a/wallet/db_postgres_sqlgen.c b/wallet/db_postgres_sqlgen.c index 622e38e61..6ca03f26a 100644 --- a/wallet/db_postgres_sqlgen.c +++ b/wallet/db_postgres_sqlgen.c @@ -2074,4 +2074,4 @@ struct db_query db_postgres_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */ -// SHA256STAMP:56ce3adfaa2de5ef901e32e7f698b9500859a0015d869470ea1be779847b32e3 +// SHA256STAMP:f929ee6db13bdf55b5e0cdf54840091948b664a61c63a4aaaef403dc7e6f23ad diff --git a/wallet/db_sqlite3_sqlgen.c b/wallet/db_sqlite3_sqlgen.c index 973efecab..fa71450da 100644 --- a/wallet/db_sqlite3_sqlgen.c +++ b/wallet/db_sqlite3_sqlgen.c @@ -2074,4 +2074,4 @@ struct db_query db_sqlite3_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */ -// SHA256STAMP:56ce3adfaa2de5ef901e32e7f698b9500859a0015d869470ea1be779847b32e3 +// SHA256STAMP:f929ee6db13bdf55b5e0cdf54840091948b664a61c63a4aaaef403dc7e6f23ad diff --git a/wallet/statements_gettextgen.po b/wallet/statements_gettextgen.po index 49ca227a0..a0bff95df 100644 --- a/wallet/statements_gettextgen.po +++ b/wallet/statements_gettextgen.po @@ -1362,11 +1362,11 @@ msgstr "" msgid "not a valid SQL statement" msgstr "" -#: wallet/test/run-wallet.c:1540 +#: wallet/test/run-wallet.c:1545 msgid "SELECT COUNT(1) FROM channel_funding_inflights WHERE channel_id = ?;" msgstr "" -#: wallet/test/run-wallet.c:1753 +#: wallet/test/run-wallet.c:1758 msgid "INSERT INTO channels (id) VALUES (1);" msgstr "" -# SHA256STAMP:a433fb1866ae16a28b87177ef67fc99669c03a6cd47a0433e1460884367bbf34 +# SHA256STAMP:35e10cb3ec34af54b6d78d9eea1aefa0861ef9acbfe0e78745d24b8e49e50b05 diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 1709c42e0..666068099 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -575,6 +575,11 @@ struct command_result *param_channel_id(struct command *cmd UNNEEDED, const jsmntok_t *tok UNNEEDED, struct channel_id **cid UNNEEDED) { fprintf(stderr, "param_channel_id called!\n"); abort(); } +/* Generated stub for param_feerate */ +struct command_result *param_feerate(struct command *cmd UNNEEDED, const char *name UNNEEDED, + const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, + u32 **feerate UNNEEDED) +{ fprintf(stderr, "param_feerate called!\n"); abort(); } /* Generated stub for param_loglevel */ struct command_result *param_loglevel(struct command *cmd UNNEEDED, const char *name UNNEEDED,