From 54b4baabb00d046c0725aeabb2b1737143a896c1 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Fri, 24 Jun 2022 12:58:26 +0200 Subject: [PATCH] opening: Add `dev-allowdustreserve` option to opt into dust reserves Technically this is a non-conformance with the spec, hence the `dev` flag to opt-in, however I'm being told that it is also implemented in other implementations. I'll follow this up with a proposal to the spec to remove the checks we now bypass. --- Makefile | 2 -- doc/lightning-listconfigs.7.md | 4 ++- doc/schemas/listconfigs.schema.json | 4 +++ openingd/openingd.c | 47 +++++++++++++++-------------- tests/test_opening.py | 21 +++++++------ 5 files changed, 43 insertions(+), 35 deletions(-) diff --git a/Makefile b/Makefile index 71c3c8daa..5e1d113d1 100644 --- a/Makefile +++ b/Makefile @@ -258,8 +258,6 @@ endif CPPFLAGS += -DBINTOPKGLIBEXECDIR="\"$(shell sh tools/rel.sh $(bindir) $(pkglibexecdir))\"" CFLAGS = $(CPPFLAGS) $(CWARNFLAGS) $(CDEBUGFLAGS) $(COPTFLAGS) -I $(CCANDIR) $(EXTERNAL_INCLUDE_FLAGS) -I . -I$(CPATH) $(SQLITE3_CFLAGS) $(POSTGRES_INCLUDE) $(FEATURES) $(COVFLAGS) $(DEV_CFLAGS) -DSHACHAIN_BITS=48 -DJSMN_PARENT_LINKS $(PIE_CFLAGS) $(COMPAT_CFLAGS) -DBUILD_ELEMENTS=1 -CFLAGS += -DZERORESERVE=1 - # If CFLAGS is already set in the environment of make (to whatever value, it # does not matter) then it would export it to subprocesses with the above value # we set, including CWARNFLAGS which by default contains -Wall -Werror. This diff --git a/doc/lightning-listconfigs.7.md b/doc/lightning-listconfigs.7.md index 7a4ed25fa..184083134 100644 --- a/doc/lightning-listconfigs.7.md +++ b/doc/lightning-listconfigs.7.md @@ -99,6 +99,7 @@ On success, an object is returned, containing: - **subdaemon** (string, optional): `subdaemon` fields from config or cmdline if any (can be more than one) - **fetchinvoice-noconnect** (boolean, optional): `featchinvoice-noconnect` fileds from config or cmdline, or default - **tor-service-password** (string, optional): `tor-service-password` field from config or cmdline, if any +- **dev-allowdustreserve** (boolean, optional): Whether we allow setting dust reserves [comment]: # (GENERATE-FROM-SCHEMA-END) @@ -216,4 +217,5 @@ RESOURCES --------- Main web site: -[comment]: # ( SHA256STAMP:dcab86f29b946fed925de5e05cb79faa03cc4421cefeab3561a596ed5e64962d) + +[comment]: # ( SHA256STAMP:310cc00ef62e7075d5d2588b0492c2dd96f507cc739f67d56ccc6c4f3135bca5) diff --git a/doc/schemas/listconfigs.schema.json b/doc/schemas/listconfigs.schema.json index ec7eabc5b..db1d451e0 100644 --- a/doc/schemas/listconfigs.schema.json +++ b/doc/schemas/listconfigs.schema.json @@ -286,6 +286,10 @@ "tor-service-password": { "type": "string", "description": "`tor-service-password` field from config or cmdline, if any" + }, + "dev-allowdustreserve": { + "type": "boolean", + "description": "Whether we allow setting dust reserves" } } } diff --git a/openingd/openingd.c b/openingd/openingd.c index 40a16a49a..9bdc3a264 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -146,27 +146,28 @@ static void set_reserve_absolute(struct state * state, const struct amount_sat d { status_debug("Setting their reserve to %s", type_to_string(tmpctx, struct amount_sat, &reserve_sat)); -#ifdef ZERORESERVE - state->localconf.channel_reserve = reserve_sat; -#else - /* BOLT #2: - * - * The sending node: - *... - * - MUST set `channel_reserve_satoshis` greater than or equal to - * `dust_limit_satoshis` from the `open_channel` message. - */ - if (amount_sat_greater(dust_limit, reserve_sat)) { - status_debug( - "Their reserve is too small, bumping to dust_limit: %s < %s", - type_to_string(tmpctx, struct amount_sat, &reserve_sat), - type_to_string(tmpctx, struct amount_sat, &dust_limit)); - state->localconf.channel_reserve - = dust_limit; - } else { + if (state->allowdustreserve) { state->localconf.channel_reserve = reserve_sat; + } else { + /* BOLT #2: + * + * The sending node: + *... + * - MUST set `channel_reserve_satoshis` greater than or equal + *to `dust_limit_satoshis` from the `open_channel` message. + */ + if (amount_sat_greater(dust_limit, reserve_sat)) { + status_debug("Their reserve is too small, bumping to " + "dust_limit: %s < %s", + type_to_string(tmpctx, struct amount_sat, + &reserve_sat), + type_to_string(tmpctx, struct amount_sat, + &dust_limit)); + state->localconf.channel_reserve = dust_limit; + } else { + state->localconf.channel_reserve = reserve_sat; + } } -#endif } /* We always set channel_reserve_satoshis to 1%, rounded down. */ @@ -464,8 +465,8 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags) type_to_string(msg, struct channel_id, &state->channel_id)); -#ifndef ZERORESERVE - if (amount_sat_greater(state->remoteconf.dust_limit, + if (!state->allowdustreserve && + amount_sat_greater(state->remoteconf.dust_limit, state->localconf.channel_reserve)) { negotiation_failed(state, "dust limit %s" @@ -476,7 +477,6 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags) &state->localconf.channel_reserve)); return NULL; } -#endif if (!check_config_bounds(tmpctx, state->funding_sats, state->feerate_per_kw, @@ -972,7 +972,8 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) * - MUST set `dust_limit_satoshis` less than or equal to * `channel_reserve_satoshis` from the `open_channel` message. */ - if (amount_sat_greater(state->remoteconf.dust_limit, + if (!state->allowdustreserve && + amount_sat_greater(state->remoteconf.dust_limit, state->localconf.channel_reserve)) { negotiation_failed(state, "Our channel reserve %s" diff --git a/tests/test_opening.py b/tests/test_opening.py index c3e1a1b21..ebc41f4c2 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1740,17 +1740,20 @@ def test_zeroreserve(node_factory, bitcoind): - l2 enforces default reserve - l3 enforces sub-dust reserves """ - ZEROCONF = True plugin_path = Path(__file__).parent / "plugins" / "zeroreserve.py" opts = [ { 'plugin': str(plugin_path), 'reserve': '0sat', + 'dev-allowdustreserve': True, + }, + { + 'dev-allowdustreserve': True, }, - {}, { 'plugin': str(plugin_path), - 'reserve': '123sat' + 'reserve': '123sat', + 'dev-allowdustreserve': True, } ] l1, l2, l3 = node_factory.get_nodes(3, opts=opts) @@ -1780,16 +1783,16 @@ def test_zeroreserve(node_factory, bitcoind): l1c3 = l1.rpc.listpeers(l3.info['id'])['peers'][0]['channels'][0] # l1 imposed a 0sat reserve on l2, while l2 imposed the default 1% reserve on l1 - assert l1c1['their_channel_reserve_satoshis'] == l2c1['our_channel_reserve_satoshis'] == (0 if ZEROCONF else 546) - assert l1c1['our_channel_reserve_satoshis'] == l2c1['their_channel_reserve_satoshis'] == 10000 + assert l1c1['their_reserve_msat'] == l2c1['our_reserve_msat'] == Millisatoshi('0sat') + assert l1c1['our_reserve_msat'] == l2c1['their_reserve_msat'] == Millisatoshi('10000sat') # l2 imposed the default 1% on l3, while l3 imposed a custom 123sat fee on l2 - assert l2c2['their_channel_reserve_satoshis'] == l3c2['our_channel_reserve_satoshis'] == 10000 - assert l2c2['our_channel_reserve_satoshis'] == l3c2['their_channel_reserve_satoshis'] == (123 if ZEROCONF else 546) + assert l2c2['their_reserve_msat'] == l3c2['our_reserve_msat'] == Millisatoshi('10000sat') + assert l2c2['our_reserve_msat'] == l3c2['their_reserve_msat'] == Millisatoshi('123sat') # l3 imposed a custom 321sat fee on l1, while l1 imposed a custom 0sat fee on l3 - assert l3c3['their_channel_reserve_satoshis'] == l1c3['our_channel_reserve_satoshis'] == (321 if ZEROCONF else 546) - assert l3c3['our_channel_reserve_satoshis'] == l1c3['their_channel_reserve_satoshis'] == (0 if ZEROCONF else 546) + assert l3c3['their_reserve_msat'] == l1c3['our_reserve_msat'] == Millisatoshi('321sat') + assert l3c3['our_reserve_msat'] == l1c3['their_reserve_msat'] == Millisatoshi('0sat') # Now do some drain tests on c1, as that should be drainable # completely by l2 being the fundee