From 561ccc9e67688a92bac85015f28e3016d3b121d5 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 16 Aug 2022 17:12:37 +0200 Subject: [PATCH] openingd: Fail if dust and max_htlcs result in 0output commitment tx Prior to this we might end up with a commitment transaction without any outputs, if combined with `--dev-allowdustreserve`. Otherwise the reserve being larger than dust means the funder could not drop its direct output to be below dust. Reported-by: Rusty Russell <@rustyrussell> --- openingd/openingd.c | 41 +++++++++++++++++++++++++++++++++++++++++ tests/test_opening.py | 1 - 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/openingd/openingd.c b/openingd/openingd.c index 9bdc3a264637..e9de31a3dc74 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -478,6 +478,47 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags) return NULL; } + /* If we allow dust reserves, we might end up in a situation + * in which all the channel funds are allocated to HTLCs, + * leaving just dust to_us and to_them outputs. If the HTLCs + * themselves are dust as well, our commitment transaction is + * now invalid since it has no outputs at all, putting us in a + * weird situation where the channel cannot be closed + * unilaterally at all. (Thanks Rusty for identifying this + * edge case). */ + struct amount_sat alldust, mindust = + amount_sat_greater(state->remoteconf.dust_limit, + state->localconf.dust_limit) + ? state->localconf.dust_limit + : state->remoteconf.dust_limit; + size_t maxhtlcs = state->remoteconf.max_accepted_htlcs + + state->localconf.max_accepted_htlcs; + if (!amount_sat_mul(&alldust, mindust, maxhtlcs + 2)) { + negotiation_failed( + state, + "Overflow while computing total possible dust amount"); + return NULL; + } + + if (state->allowdustreserve && + feature_negotiated(state->our_features, state->their_features, + OPT_ZEROCONF) && + amount_sat_greater_eq(alldust, state->funding_sats)) { + negotiation_failed( + state, + "channel funding %s too small for chosen " + "parameters: a total of %zu HTLCs with dust value %s would " + "result in a commitment_transaction without outputs. " + "Please increase the funding amount or reduce the " + "max_accepted_htlcs to ensure at least one non-dust " + "output.", + type_to_string(tmpctx, struct amount_sat, + &state->funding_sats), + maxhtlcs, + type_to_string(tmpctx, struct amount_sat, &mindust)); + return NULL; + } + if (!check_config_bounds(tmpctx, state->funding_sats, state->feerate_per_kw, state->max_to_self_delay, diff --git a/tests/test_opening.py b/tests/test_opening.py index 8aaa89454f5d..c182fefcf547 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1841,7 +1841,6 @@ def test_zeroreserve_mixed(node_factory, bitcoind): l3.rpc.fundchannel(l1.info['id'], 10**6) -@pytest.mark.xfail("Restriction not implemented yet", strict=True) def test_zeroreserve_alldust(node_factory): """If we allow dust reserves we need larger fundings