Skip to content

Commit

Permalink
opening: Add dev-allowdustreserve option to opt into dust reserves
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cdecker committed Jun 24, 2022
1 parent 77a6afd commit 308e57c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 31 deletions.
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ DEFAULT_TARGETS :=
CPPFLAGS += -DBINTOPKGLIBEXECDIR="\"$(shell sh tools/rel.sh $(bindir) $(pkglibexecdir))\""
CFLAGS = $(CPPFLAGS) $(CWARNFLAGS) $(CDEBUGFLAGS) $(COPTFLAGS) -I $(CCANDIR) $(EXTERNAL_INCLUDE_FLAGS) -I . -I/usr/local/include $(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
Expand Down
47 changes: 24 additions & 23 deletions openingd/openingd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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"
Expand All @@ -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,
Expand Down Expand Up @@ -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"
Expand Down
17 changes: 10 additions & 7 deletions tests/test_opening.py
Original file line number Diff line number Diff line change
Expand Up @@ -1401,17 +1401,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)
Expand Down Expand Up @@ -1441,16 +1444,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['their_channel_reserve_satoshis'] == l2c1['our_channel_reserve_satoshis'] == 0
assert l1c1['our_channel_reserve_satoshis'] == l2c1['their_channel_reserve_satoshis'] == 10000

# 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['our_channel_reserve_satoshis'] == l3c2['their_channel_reserve_satoshis'] == 123

# 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_channel_reserve_satoshis'] == l1c3['our_channel_reserve_satoshis'] == 321
assert l3c3['our_channel_reserve_satoshis'] == l1c3['their_channel_reserve_satoshis'] == 0

# Now do some drain tests on c1, as that should be drainable
# completely by l2 being the fundee
Expand Down

0 comments on commit 308e57c

Please sign in to comment.