From 5704653d4c0e9761194fd51f631cd692e3700091 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 21 Mar 2022 11:28:57 +1030 Subject: [PATCH] setchannel: don't let them advertize htlc_maximum_msat larger than capacity. And check for the obvious setting min > max. Signed-off-by: Rusty Russell --- doc/lightning-setchannel.7.md | 3 ++- doc/schemas/setchannel.schema.json | 4 ++++ lightningd/peer_control.c | 25 ++++++++++++++++++--- lightningd/test/run-invoice-select-inchan.c | 3 +++ tests/test_pay.py | 9 ++++++++ 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/doc/lightning-setchannel.7.md b/doc/lightning-setchannel.7.md index f915b7968..65755634c 100644 --- a/doc/lightning-setchannel.7.md +++ b/doc/lightning-setchannel.7.md @@ -71,6 +71,7 @@ On success, an object containing **channels** is returned. It is an array of ob - **short_channel_id** (short_channel_id, optional): the short_channel_id (if locked in) - the following warnings are possible: - **warning_htlcmin_too_low**: The requested htlcmin was too low for this peer, so we set it to the minimum they will allow + - **warning_htlcmax_too_high**: The requested htlcmax was greater than the channel capacity, so we set it to the channel capacity [comment]: # (GENERATE-FROM-SCHEMA-END) @@ -100,4 +101,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:0f153e7dddce61bc921b3743472f11316c5984b9b1459cac1b201d6f51ec1be1) +[comment]: # ( SHA256STAMP:a38b5ea12566d9e40eab07b95a90007bf66373ac1189f458d1678634522575b3) diff --git a/doc/schemas/setchannel.schema.json b/doc/schemas/setchannel.schema.json index 512e4dbdf..54a507cb8 100644 --- a/doc/schemas/setchannel.schema.json +++ b/doc/schemas/setchannel.schema.json @@ -54,6 +54,10 @@ "maximum_htlc_out_msat": { "type": "msat", "description": "The resulting htlcmax we will advertize (the BOLT #7 name is htlc_maximum_msat)" + }, + "warning_htlcmax_too_high": { + "type": "string", + "description": "The requested htlcmax was greater than the channel capacity, so we set it to the channel capacity" } } } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 39c6b4a31..8b90cd657 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -2019,7 +2019,7 @@ static void set_channel_config(struct command *cmd, struct channel *channel, struct json_stream *response, bool add_details) { - bool warn_cannot_set_min = false; + bool warn_cannot_set_min = false, warn_cannot_set_max = false; /* We only need to defer values if we *increase* fees (or drop * max, increase min); we always allow users to overpay fees. */ @@ -2053,8 +2053,17 @@ static void set_channel_config(struct command *cmd, struct channel *channel, } else channel->htlc_minimum_msat = *htlc_min; } - if (htlc_max) - channel->htlc_maximum_msat = *htlc_max; + if (htlc_max) { + struct amount_msat actual_max; + + /* Can't set it greater than actual capacity. */ + actual_max = htlc_max_possible_send(channel); + if (amount_msat_greater(*htlc_max, actual_max)) { + warn_cannot_set_max = true; + channel->htlc_maximum_msat = actual_max; + } else + channel->htlc_maximum_msat = *htlc_max; + } /* tell channeld to make a send_channel_update */ if (channel->owner && streq(channel->owner->name, "channeld")) @@ -2088,6 +2097,9 @@ static void set_channel_config(struct command *cmd, struct channel *channel, json_add_amount_msat_only(response, "maximum_htlc_out_msat", channel->htlc_maximum_msat); + if (warn_cannot_set_max) + json_add_string(response, "warning_htlcmax_too_high", + "Set maximum_htlc_out_msat to maximum possible in channel"); } json_object_end(response); } @@ -2188,6 +2200,13 @@ static struct command_result *json_setchannel(struct command *cmd, NULL)) return command_param_failed(); + /* Prevent obviously incorrect things! */ + if (htlc_min && htlc_max + && amount_msat_less(*htlc_max, *htlc_min)) { + return command_fail(cmd, LIGHTNINGD, + "htlcmax cannot be less than htlcmin"); + } + if (channel && channel->state != CHANNELD_NORMAL && channel->state != CHANNELD_AWAITING_LOCKIN diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 08bd6eef9..09da34f95 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -247,6 +247,9 @@ bool htlc_is_trimmed(enum side htlc_owner UNNEEDED, enum side side UNNEEDED, bool option_anchor_outputs UNNEEDED) { fprintf(stderr, "htlc_is_trimmed called!\n"); abort(); } +/* Generated stub for htlc_max_possible_send */ +struct amount_msat htlc_max_possible_send(const struct channel *channel UNNEEDED) +{ fprintf(stderr, "htlc_max_possible_send called!\n"); abort(); } /* Generated stub for htlc_set_fail */ void htlc_set_fail(struct htlc_set *set UNNEEDED, const u8 *failmsg TAKES UNNEEDED) { fprintf(stderr, "htlc_set_fail called!\n"); abort(); } diff --git a/tests/test_pay.py b/tests/test_pay.py index fee9e3f30..867fec63a 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2213,6 +2213,7 @@ def test_setchannel_zero(node_factory, bitcoind): # - payment can be done using zero fees DEF_BASE = 1 DEF_PPM = 10 + MAX_HTLC = Millisatoshi(int(FUNDAMOUNT * 1000 * 0.99)) l1, l2, l3 = node_factory.line_graph( 3, announce_channels=True, wait_for_announce=True, @@ -2242,6 +2243,14 @@ def test_setchannel_zero(node_factory, bitcoind): assert result['status'] == 'complete' assert result['msatoshi_sent'] == 4999999 + # FIXME: hack something up to advertize min_htlc > 0, then test mintoolow. + with pytest.raises(RpcError, match="htlcmax cannot be less than htlcmin"): + l2.rpc.setchannel(scid, htlcmin=100000, htlcmax=99999) + + ret = l2.rpc.setchannel(scid, htlcmax=FUNDAMOUNT * 1000) + assert 'warning_htlcmax_too_high' in only_one(ret['channels']) + assert only_one(ret['channels'])['maximum_htlc_out_msat'] == MAX_HTLC + @pytest.mark.developer("gossip without DEVELOPER=1 is slow") def test_setchannel_restart(node_factory, bitcoind):