From 8ac3eb7c88fef94c4848f1e2ba0424bb150ff627 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 5f22c8aba1b1..f728a25bf28f 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 b2bad3ae436e..f680f495d847 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1723,7 +1723,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