Skip to content

Commit

Permalink
lightningd: make option_static_remotekey compulsory.
Browse files Browse the repository at this point in the history 
As suggested in lightning/bolts#1092.

We still support channels opened without it, but you can no longer open new ones without it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: `option_gossip_queries` is now required (advertized by all but 16 nodes)
  • Loading branch information
@rustyrussell @cdecker
rustyrussell authored and cdecker committed Nov 13, 2023
1 parent c5e9bcf commit 920b736
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 47 deletions.
2 changes: 1 addition & 1 deletion lightningd/lightningd.c
View file
Expand Up @@ -887,7 +887,7 @@ static struct feature_set *default_features(const tal_t *ctx)
OPTIONAL_FEATURE(OPT_BASIC_MPP),
OPTIONAL_FEATURE(OPT_LARGE_CHANNELS),
OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES_EX),
OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY),
COMPULSORY_FEATURE(OPT_STATIC_REMOTEKEY),
OPTIONAL_FEATURE(OPT_SHUTDOWN_ANYSEGWIT),
OPTIONAL_FEATURE(OPT_PAYMENT_METADATA),
OPTIONAL_FEATURE(OPT_SCID_ALIAS),
Expand Down
112 changes: 76 additions & 36 deletions tests/test_connection.py
View file
Expand Up @@ -3575,15 +3575,19 @@ def test_channel_features(node_factory, bitcoind, anchors):

def test_nonstatic_channel(node_factory, bitcoind):
"""Smoke test for a channel without option_static_remotekey"""
l1, l2 = node_factory.line_graph(2,
opts=[{},
# needs at least 1, 7 and 15 to connect
# (and 9 is a dependent)
{'dev-force-features': '1,7,9,15////////'}])
l1, l2 = node_factory.get_nodes(2,
# This forces us to allow send/recv of non-static-remotekey!
opts={'dev-any-channel-type': None})
l1.fundwallet(2000000)
l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
bitcoind.generate_block(1, wait_for_mempool=1)

chan = only_one(l1.rpc.listpeerchannels()['channels'])
assert 'option_static_remotekey' not in chan['features']
assert 'option_anchor' not in chan['features']
assert 'option_anchors_zero_fee_htlc_tx' not in chan['features']
wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')

l1.pay(l2, 1000)
l1.rpc.close(l2.info['id'])
Expand Down Expand Up @@ -3690,12 +3694,20 @@ def test_openchannel_init_alternate(node_factory, executor):

def test_upgrade_statickey(node_factory, executor):
"""l1 doesn't have option_static_remotekey, l2 offers it."""
l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True,
'dev-force-features': ["-13"],
'experimental-upgrade-protocol': None},
{'may_reconnect': True,
'experimental-upgrade-protocol': None}])
l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True,
'experimental-upgrade-protocol': None,
# This forces us to allow sending non-static-remotekey!
'dev-any-channel-type': None},
{'may_reconnect': True,
# This forces us to accept non-static-remotekey!
'dev-any-channel-type': None,
'experimental-upgrade-protocol': None}])

l1.fundwallet(2000000)
l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])

# Now reconnect.
l1.rpc.disconnect(l2.info['id'], force=True)
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)

Expand All @@ -3720,15 +3732,22 @@ def test_upgrade_statickey(node_factory, executor):

def test_upgrade_statickey_onchaind(node_factory, executor, bitcoind):
"""We test penalty before/after, and unilateral before/after"""
l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True,
'dev-no-reconnect': None,
'dev-force-features': ["-13"],
'experimental-upgrade-protocol': None,
# We try to cheat!
'allow_broken_log': True},
{'may_reconnect': True,
'dev-no-reconnect': None,
'experimental-upgrade-protocol': None}])
l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True,
'experimental-upgrade-protocol': None,
# This forces us to allow sending non-static-remotekey!
'dev-any-channel-type': None,
# We try to cheat!
'allow_broken_log': True},
{'may_reconnect': True,
# This forces us to allow non-static-remotekey!
'dev-any-channel-type': None,
'experimental-upgrade-protocol': None}])

l1.fundwallet(FUNDAMOUNT + 1000)
l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
bitcoind.generate_block(1, wait_for_mempool=1)
wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')

# TEST 1: Cheat from pre-upgrade.
tx = l1.rpc.dev_sign_last_tx(l2.info['id'])['tx']
Expand Down Expand Up @@ -3766,12 +3785,17 @@ def test_upgrade_statickey_onchaind(node_factory, executor, bitcoind):
wait_for(lambda: l2.rpc.listpeerchannels()['channels'] == [])

# TEST 2: Cheat from post-upgrade.
node_factory.join_nodes([l1, l2])
l1.fundwallet(FUNDAMOUNT + 1000)
l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])

l1.rpc.disconnect(l2.info['id'], force=True)
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)

l1.daemon.wait_for_log('option_static_remotekey enabled at 1/1')
l2.daemon.wait_for_log('option_static_remotekey enabled at 1/1')
bitcoind.generate_block(1, wait_for_mempool=1)
wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')

l1.pay(l2, 1000000)

Expand All @@ -3793,7 +3817,11 @@ def test_upgrade_statickey_onchaind(node_factory, executor, bitcoind):
wait_for(lambda: len(l2.rpc.listpeerchannels()['channels']) == 0)

# TEST 3: Unilateral close from pre-upgrade
node_factory.join_nodes([l1, l2])
l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
l1.fundwallet(FUNDAMOUNT + 1000)
l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
bitcoind.generate_block(1, wait_for_mempool=1)
wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')

# Give them both something for onchain close.
l1.pay(l2, 1000000)
Expand Down Expand Up @@ -3824,12 +3852,16 @@ def test_upgrade_statickey_onchaind(node_factory, executor, bitcoind):
wait_for(lambda: len(l2.rpc.listpeerchannels()['channels']) == 0)

# TEST 4: Unilateral close from post-upgrade
node_factory.join_nodes([l1, l2])
l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])

l1.rpc.disconnect(l2.info['id'], force=True)
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
l1.daemon.wait_for_log('option_static_remotekey enabled at 1/1')

bitcoind.generate_block(1, wait_for_mempool=1)
wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')

# Move to static_remotekey.
l1.pay(l2, 1000000)

Expand Down Expand Up @@ -3859,20 +3891,28 @@ def test_upgrade_statickey_fail(node_factory, executor, bitcoind):
l2_disconnects = ['-WIRE_REVOKE_AND_ACK',
'-WIRE_COMMITMENT_SIGNED']

l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True,
'dev-no-reconnect': None,
'disconnect': l1_disconnects,
'experimental-upgrade-protocol': None,
'dev-force-features': ["-13"],
# Don't have feerate changes!
'feerates': (7500, 7500, 7500, 7500)},
{'may_reconnect': True,
'dev-no-reconnect': None,
'experimental-upgrade-protocol': None,
'disconnect': l2_disconnects,
'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_htlcs.py'),
'hold-time': 10000,
'hold-result': 'fail'}])
l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True,
'dev-no-reconnect': None,
'disconnect': l1_disconnects,
# This allows us to send non-static-remotekey!
'dev-any-channel-type': None,
'experimental-upgrade-protocol': None,
# Don't have feerate changes!
'feerates': (7500, 7500, 7500, 7500)},
{'may_reconnect': True,
'dev-no-reconnect': None,
'experimental-upgrade-protocol': None,
# This forces us to accept non-static-remotekey!
'dev-any-channel-type': None,
'disconnect': l2_disconnects,
'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_htlcs.py'),
'hold-time': 10000,
'hold-result': 'fail'}])
l1.fundwallet(FUNDAMOUNT + 1000)
l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
bitcoind.generate_block(1, wait_for_mempool=1)
wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')

# This HTLC will fail
l1.rpc.sendpay([{'amount_msat': 1000, 'id': l2.info['id'], 'delay': 5, 'channel': first_scid(l1, l2)}], '00' * 32, payment_secret='00' * 32)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_misc.py
View file
Expand Up @@ -2248,7 +2248,7 @@ def test_list_features_only(node_factory):
'option_gossip_queries/even',
'option_var_onion_optin/even',
'option_gossip_queries_ex/odd',
'option_static_remotekey/odd',
'option_static_remotekey/even',
'option_payment_secret/even',
'option_basic_mpp/odd',
'option_support_large_channel/odd',
Expand Down
17 changes: 11 additions & 6 deletions tests/test_opening.py
View file
Expand Up @@ -2599,24 +2599,22 @@ def test_opening_explicit_channel_type(node_factory, bitcoind):
ZEROCONF = 50

for zeroconf in ([], [ZEROCONF]):
for ctype in ([],
[STATIC_REMOTEKEY],
for ctype in ([STATIC_REMOTEKEY],
[ANCHORS_ZERO_FEE_HTLC_TX, STATIC_REMOTEKEY]):
l1.rpc.fundchannel_start(l2.info['id'], FUNDAMOUNT,
channel_type=ctype + zeroconf)
l1.rpc.fundchannel_cancel(l2.info['id'])
# FIXME: Check type is actually correct!

# Zeroconf is refused to l4.
for ctype in ([],
[STATIC_REMOTEKEY],
for ctype in ([STATIC_REMOTEKEY],
[ANCHORS_ZERO_FEE_HTLC_TX, STATIC_REMOTEKEY]):
with pytest.raises(RpcError, match=r'not on our allowlist'):
l1.rpc.fundchannel_start(l4.info['id'], FUNDAMOUNT,
channel_type=ctype + [ZEROCONF])

psbt = l1.rpc.fundpsbt(FUNDAMOUNT - 1000, '253perkw', 250, reserve=0)['psbt']
for ctype in ([], [12], [22, 12]):
for ctype in ([12], [22, 12]):
cid = l1.rpc.openchannel_init(l3.info['id'], FUNDAMOUNT - 1000, psbt, channel_type=ctype)['channel_id']
l1.rpc.openchannel_abort(cid)

Expand All @@ -2627,6 +2625,13 @@ def test_opening_explicit_channel_type(node_factory, bitcoind):
with pytest.raises(RpcError, match=r'channel_type not supported'):
l1.rpc.openchannel_init(l3.info['id'], FUNDAMOUNT - 1000, psbt, channel_type=[STATIC_REMOTEKEY, ANCHORS_OLD])

# We need static_remotekey now, too
with pytest.raises(RpcError, match=r'channel_type not supported'):
l1.rpc.fundchannel_start(l2.info['id'], FUNDAMOUNT, channel_type=[])

with pytest.raises(RpcError, match=r'channel_type not supported'):
l1.rpc.openchannel_init(l3.info['id'], FUNDAMOUNT - 1000, psbt, channel_type=[])

# l1 will try, with dev-any-channel-type, l2 will reject.
l1.stop()
l1.daemon.opts['dev-any-channel-type'] = None
Expand All @@ -2635,7 +2640,7 @@ def test_opening_explicit_channel_type(node_factory, bitcoind):

with pytest.raises(RpcError, match=r'They sent ERROR .*: You gave bad parameters: Did not support channel_type 12,20'):
l1.rpc.fundchannel_start(l2.info['id'], FUNDAMOUNT, channel_type=[STATIC_REMOTEKEY, ANCHORS_OLD])

# Now make l2 accept it!
l2.stop()
l2.daemon.opts['dev-any-channel-type'] = None
Expand Down
2 changes: 1 addition & 1 deletion tests/test_plugin.py
View file
Expand Up @@ -1660,7 +1660,7 @@ def test_plugin_feature_announce(node_factory):

# Check the featurebits we've set in the `init` message from
# feature-test.py.
assert l1.daemon.is_in_log(r'\[OUT\] 001000022100....{}'
assert l1.daemon.is_in_log(r'\[OUT\] 001000021100....{}'
.format(expected_peer_features(extra=[201])))

# Check the invoice featurebit we set in feature-test.py
Expand Down
4 changes: 2 additions & 2 deletions tests/utils.py
View file
Expand Up @@ -36,7 +36,7 @@ def hex_bits(features):

def expected_peer_features(extra=[]):
"""Return the expected peer features hexstring for this configuration"""
features = [0, 5, 6, 8, 11, 13, 14, 17, 19, 25, 27, 45, 47, 51]
features = [0, 5, 6, 8, 11, 12, 14, 17, 19, 25, 27, 45, 47, 51]
if EXPERIMENTAL_DUAL_FUND:
# option_dual_fund
features += [29]
Expand All @@ -50,7 +50,7 @@ def expected_peer_features(extra=[]):
# features for the 'node' and the 'peer' feature sets
def expected_node_features(extra=[]):
"""Return the expected node features hexstring for this configuration"""
features = [0, 5, 6, 8, 11, 13, 14, 17, 19, 25, 27, 45, 47, 51, 55]
features = [0, 5, 6, 8, 11, 12, 14, 17, 19, 25, 27, 45, 47, 51, 55]
if EXPERIMENTAL_DUAL_FUND:
# option_dual_fund
features += [29]
Expand Down

0 comments on commit 920b736

Please sign in to comment.