From 0643b9ea30885904b122182c7122564df69cb004 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 27 Jun 2025 14:12:18 +0930 Subject: [PATCH 1/7] lightningd: make option_channel_type compulsory. As per BOLT recommendation https://github.com/lightning/bolts/pull/1232, this means we will insist on this being available. For CLN, we added this in 0.12.0 (2022-08-23), though there were fixes as late as 24.02. Either way that's well outside our support window. Signed-off-by: Rusty Russell Closes: https://github.com/ElementsProject/lightning/issues/8152 Changelog-Changed: Protocol: We now insist that peers support `option_channel_type` (in CLN since 0.12.0 in late 2022, similar for other implementations). --- lightningd/lightningd.c | 2 +- tests/test_misc.py | 2 +- tests/utils.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 68b2e2462ec5..b7e69ab3187d 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -929,7 +929,7 @@ static struct feature_set *default_features(const tal_t *ctx) OPTIONAL_FEATURE(OPT_ZEROCONF), OPTIONAL_FEATURE(OPT_QUIESCE), OPTIONAL_FEATURE(OPT_ONION_MESSAGES), - OPTIONAL_FEATURE(OPT_CHANNEL_TYPE), + COMPULSORY_FEATURE(OPT_CHANNEL_TYPE), OPTIONAL_FEATURE(OPT_ROUTE_BLINDING), OPTIONAL_FEATURE(OPT_PROVIDE_STORAGE), /* Removed later for elements */ diff --git a/tests/test_misc.py b/tests/test_misc.py index c0f551ceabb8..f8d02f7559db 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -2405,7 +2405,7 @@ def test_list_features_only(node_factory): 'option_quiesce/odd', 'option_onion_messages/odd', 'option_provide_storage/odd', - 'option_channel_type/odd', + 'option_channel_type/even', 'option_scid_alias/odd', 'option_zeroconf/odd'] expected += ['supports_open_accept_channel_type'] diff --git a/tests/utils.py b/tests/utils.py index 5b682c6c3e10..ed1e12a73bf4 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -41,7 +41,7 @@ def hex_bits(features): def expected_peer_features(extra=[]): """Return the expected peer features hexstring for this configuration""" - features = [0, 5, 7, 8, 11, 12, 14, 17, 19, 25, 27, 35, 39, 43, 45, 47, 51] + features = [0, 5, 7, 8, 11, 12, 14, 17, 19, 25, 27, 35, 39, 43, 44, 47, 51] if EXPERIMENTAL_DUAL_FUND: # option_dual_fund features += [29] @@ -57,7 +57,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, 7, 8, 11, 12, 14, 17, 19, 25, 27, 35, 39, 43, 45, 47, 51, 55] + features = [0, 5, 7, 8, 11, 12, 14, 17, 19, 25, 27, 35, 39, 43, 44, 47, 51, 55] if EXPERIMENTAL_DUAL_FUND: # option_dual_fund features += [29] From c623efd71c795aca7b7c1c6770c3735c0b98e07c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 27 Jun 2025 14:13:18 +0930 Subject: [PATCH 2/7] openingd/dualopend: don't allow peer not to send channel_type. Simplifies our logic somewhat. Signed-off-by: Rusty Russell --- openingd/dualopend.c | 56 +++++++++++++++++++----------------- openingd/openingd.c | 68 ++++++++++++++++++-------------------------- 2 files changed, 58 insertions(+), 66 deletions(-) diff --git a/openingd/dualopend.c b/openingd/dualopend.c index b3e687fd30df..616df5dd8593 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -2434,30 +2434,30 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) * - if `type` is not suitable. * - if `type` includes `option_zeroconf` and it does not trust the sender to open an unconfirmed channel. */ - if (open_tlv->channel_type) { - state->channel_type = - channel_type_accept(state, - open_tlv->channel_type, - state->our_features); - if (!state->channel_type) { - if (state->dev_accept_any_channel_type) { - status_unusual("dev-any-channel-type: accepting %s", - fmt_featurebits(tmpctx, - open_tlv->channel_type)); - state->channel_type = channel_type_from(state, open_tlv->channel_type); - } else { - negotiation_failed(state, - "Did not support channel_type [%s]", - fmt_featurebits(tmpctx, - open_tlv->channel_type)); - return; - } + if (!open_tlv->channel_type) { + negotiation_failed(state, + "open_channel2 missing channel_type"); + return; + } + + state->channel_type = + channel_type_accept(state, + open_tlv->channel_type, + state->our_features); + if (!state->channel_type) { + if (state->dev_accept_any_channel_type) { + status_unusual("dev-any-channel-type: accepting %s", + fmt_featurebits(tmpctx, + open_tlv->channel_type)); + state->channel_type = channel_type_from(state, open_tlv->channel_type); + } else { + negotiation_failed(state, + "Did not support channel_type [%s]", + fmt_featurebits(tmpctx, + open_tlv->channel_type)); + return; } - } else - state->channel_type - = default_channel_type(state, - state->our_features, - state->their_features); + } /* Since anchor outputs are optional, we * only support liquidity ads if those are enabled. */ @@ -3164,9 +3164,13 @@ static void opener_start(struct state *state, u8 *msg) * `open_channel`, and they are not equal types: * - MUST fail the channel. */ - if (a_tlv->channel_type - && !featurebits_eq(a_tlv->channel_type, - state->channel_type->features)) { + if (!a_tlv->channel_type) { + negotiation_failed(state, + "Missing channel_type in accept_channel2"); + return; + } + if (!featurebits_eq(a_tlv->channel_type, + state->channel_type->features)) { negotiation_failed(state, "Return unoffered channel_type: %s", fmt_featurebits(tmpctx, diff --git a/openingd/openingd.c b/openingd/openingd.c index 601fcab2a7bb..f436177035a5 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -270,17 +270,12 @@ static void set_remote_upfront_shutdown(struct state *state, /* Since we can't send OPT_SCID_ALIAS due to compat issues, intuit whether * we really actually want it anyway, we just can't say that. */ -static bool intuit_scid_alias_type(struct state *state, u8 channel_flags, - bool peer_sent_channel_type) +static bool intuit_scid_alias_type(struct state *state, u8 channel_flags) { /* Don't need to intuit if actually set */ if (channel_type_has(state->channel_type, OPT_SCID_ALIAS)) return false; - /* Old clients didn't send channel_type at all */ - if (!peer_sent_channel_type) - return false; - /* Modern peer: no intuit hacks necessary. */ if (channel_type_has(state->channel_type, OPT_ANCHORS_ZERO_FEE_HTLC_TX)) return false; @@ -446,14 +441,13 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, * `open_channel`, and they are not equal types: * - MUST fail the channel. */ + if (!accept_tlvs->channel_type) { + negotiation_failed(state, + "accept_channel without a channel_type"); + } /* Simple case: caller specified, don't allow any variants */ if (ctype) { - if (!accept_tlvs->channel_type) { - negotiation_failed(state, - "They replied without a channel_type"); - return NULL; - } if (!featurebits_eq(accept_tlvs->channel_type, state->channel_type->features)) { negotiation_failed(state, "Return unoffered channel_type: %s", @@ -461,7 +455,7 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, accept_tlvs->channel_type)); return NULL; } - } else if (accept_tlvs->channel_type) { + } else { /* Except that v23.05 could set OPT_SCID_ALIAS in reply! */ struct channel_type *atype; @@ -587,8 +581,7 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, tal_hex(tmpctx, funding_output_script)); /* Backwards/cross compat hack */ - if (!ctype && intuit_scid_alias_type(state, channel_flags, - accept_tlvs->channel_type != NULL)) { + if (!ctype && intuit_scid_alias_type(state, channel_flags)) { channel_type_set_scid_alias(state->channel_type); } @@ -902,7 +895,6 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) struct tlv_accept_channel_tlvs *accept_tlvs; struct tlv_open_channel_tlvs *open_tlvs; struct amount_sat *reserve; - bool open_channel_had_channel_type; /* BOLT #2: * @@ -943,30 +935,27 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) * - if `type` is not suitable. * - if `type` includes `option_zeroconf` and it does not trust the sender to open an unconfirmed channel. */ - if (open_tlvs->channel_type) { - open_channel_had_channel_type = true; - state->channel_type = channel_type_accept( - state, open_tlvs->channel_type, state->our_features); - if (!state->channel_type) { - if (state->dev_accept_any_channel_type) { - status_unusual("dev-any-channel-type: accepting %s", - fmt_featurebits(tmpctx, - open_tlvs->channel_type)); - state->channel_type = channel_type_from(state, open_tlvs->channel_type); - } else { - negotiation_failed(state, - "Did not support channel_type [%s]", - fmt_featurebits(tmpctx, - open_tlvs->channel_type)); - return NULL; - } + /* option_channel_type is compulsory. */ + if (!open_tlvs->channel_type) { + negotiation_failed(state, + "Did not set channel_type in open_channel message"); + } + + state->channel_type = channel_type_accept( + state, open_tlvs->channel_type, state->our_features); + if (!state->channel_type) { + if (state->dev_accept_any_channel_type) { + status_unusual("dev-any-channel-type: accepting %s", + fmt_featurebits(tmpctx, + open_tlvs->channel_type)); + state->channel_type = channel_type_from(state, open_tlvs->channel_type); + } else { + negotiation_failed(state, + "Did not support channel_type [%s]", + fmt_featurebits(tmpctx, + open_tlvs->channel_type)); + return NULL; } - } else { - open_channel_had_channel_type = false; - state->channel_type - = default_channel_type(state, - state->our_features, - state->their_features); } /* BOLT #2: @@ -1191,8 +1180,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) fmt_channel_id(msg, &id_in)); /* Backwards/cross compat hack */ - if (intuit_scid_alias_type(state, channel_flags, - open_channel_had_channel_type)) { + if (intuit_scid_alias_type(state, channel_flags)) { channel_type_set_scid_alias(state->channel_type); } From 2d647419ad1c97ef57bb71fd7c5f1a396f20e61a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 3 Jul 2025 14:19:04 +0930 Subject: [PATCH 3/7] pytest: use _ not - in plugin options to zeroconf-selective.py. This allows us to specify: l2.rpc.plugin_start(plugin_path, zeroconf_allow=l1.info['id']) Signed-off-by: Rusty Russell --- tests/plugins/zeroconf-selective.py | 8 ++++---- tests/test_misc.py | 4 ++-- tests/test_opening.py | 18 +++++++++--------- tests/test_pay.py | 2 +- tests/test_xpay.py | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/plugins/zeroconf-selective.py b/tests/plugins/zeroconf-selective.py index b359a206f568..527e12282ca9 100755 --- a/tests/plugins/zeroconf-selective.py +++ b/tests/plugins/zeroconf-selective.py @@ -10,9 +10,9 @@ @plugin.hook('openchannel') def on_openchannel(openchannel, plugin, **kwargs): plugin.log(repr(openchannel)) - mindepth = int(plugin.options['zeroconf-mindepth']['value']) + mindepth = int(plugin.options['zeroconf_mindepth']['value']) - if openchannel['id'] == plugin.options['zeroconf-allow']['value'] or plugin.options['zeroconf-allow']['value'] == 'any': + if openchannel['id'] == plugin.options['zeroconf_allow']['value'] or plugin.options['zeroconf_allow']['value'] == 'any': plugin.log(f"This peer is in the zeroconf allowlist, setting mindepth={mindepth}") return {'result': 'continue', 'mindepth': mindepth} else: @@ -20,13 +20,13 @@ def on_openchannel(openchannel, plugin, **kwargs): plugin.add_option( - 'zeroconf-allow', + 'zeroconf_allow', '03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f', 'A node_id to allow zeroconf channels from', ) plugin.add_option( - 'zeroconf-mindepth', + 'zeroconf_mindepth', 0, 'Number of confirmations to require from allowlisted peers', ) diff --git a/tests/test_misc.py b/tests/test_misc.py index f8d02f7559db..2b4b737db4a1 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1390,7 +1390,7 @@ def test_funding_reorg_private(node_factory, bitcoind): 'dev-fast-reconnect': None, # if it's not zeroconf, we'll terminate on reorg. 'plugin': os.path.join(os.getcwd(), 'tests/plugins/zeroconf-selective.py'), - 'zeroconf-allow': 'any'} + 'zeroconf_allow': 'any'} l1, l2 = node_factory.line_graph(2, fundchannel=False, opts=opts) l1.fundwallet(10000000) sync_blockheight(bitcoind, [l1]) # height 102 @@ -1433,7 +1433,7 @@ def test_funding_reorg_remote_lags(node_factory, bitcoind): 'allow_warning': True, 'dev-fast-reconnect': None, # if it's not zeroconf, l2 will terminate on reorg. 'plugin': os.path.join(os.getcwd(), 'tests/plugins/zeroconf-selective.py'), - 'zeroconf-allow': 'any'} + 'zeroconf_allow': 'any'} l1, l2 = node_factory.line_graph(2, fundchannel=False, opts=opts) l1.fundwallet(10000000) sync_blockheight(bitcoind, [l1]) # height 102 diff --git a/tests/test_opening.py b/tests/test_opening.py index 7c086dc7cca2..fd5bf2a94eb8 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1580,8 +1580,8 @@ def test_zeroconf_mindepth(bitcoind, node_factory): {}, { 'plugin': str(plugin_path), - 'zeroconf-allow': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', - 'zeroconf-mindepth': '2', + 'zeroconf_allow': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', + 'zeroconf_mindepth': '2', }, ]) @@ -1627,7 +1627,7 @@ def test_zeroconf_open(bitcoind, node_factory): {}, { 'plugin': str(plugin_path), - 'zeroconf-allow': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59' + 'zeroconf_allow': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59' }, ]) @@ -1702,7 +1702,7 @@ def test_zeroconf_public(bitcoind, node_factory, chainparams): {'plugin': str(coin_mvt_plugin)}, { 'plugin': str(plugin_path), - 'zeroconf-allow': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518' + 'zeroconf_allow': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518' }, {} ]) @@ -1804,7 +1804,7 @@ def test_zeroconf_forward(node_factory, bitcoind): {}, { 'plugin': str(plugin_path), - 'zeroconf-allow': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59' + 'zeroconf_allow': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59' } ] l1, l2, l3 = node_factory.get_nodes(3, opts=opts) @@ -2054,7 +2054,7 @@ def test_zeroconf_multichan_forward(node_factory): {}, { 'plugin': str(plugin_path), - 'zeroconf-allow': node_id, + 'zeroconf_allow': node_id, } ], fundamount=10**6, wait_for_announce=True) @@ -2574,7 +2574,7 @@ def test_opening_explicit_channel_type(node_factory, bitcoind): l1, l2, l3, l4 = node_factory.get_nodes(4, opts=[{'experimental-dual-fund': None}, {'plugin': str(plugin_path), - 'zeroconf-allow': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518'}, + 'zeroconf_allow': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518'}, {'experimental-dual-fund': None}, {}]) @@ -2707,8 +2707,8 @@ def test_zeroconf_forget(node_factory, bitcoind, dopay: bool): {}, { "plugin": str(plugin_path), - "zeroconf-allow": "0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518", - "zeroconf-mindepth": "0", + "zeroconf_allow": "0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518", + "zeroconf_mindepth": "0", "dev-max-funding-unconfirmed-blocks": blocks, }, {}, diff --git a/tests/test_pay.py b/tests/test_pay.py index f6531d85b439..d56628873cde 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -5278,7 +5278,7 @@ def test_pay_multichannel_use_zeroconf(bitcoind, node_factory): fundamount=200_000, opts=[{}, {'plugin': zeroconf_plugin, - 'zeroconf-allow': 'any'}]) + 'zeroconf_allow': 'any'}]) # 1. Open a zeoconf channel l1 -> l2 zeroconf_sats = 1_000_000 diff --git a/tests/test_xpay.py b/tests/test_xpay.py index ff7179c888dd..405833fdb977 100644 --- a/tests/test_xpay.py +++ b/tests/test_xpay.py @@ -604,7 +604,7 @@ def test_xpay_zeroconf(node_factory): l1, l2 = node_factory.get_nodes(2, opts=[{}, {'plugin': zeroconf_plugin, - 'zeroconf-allow': 'any'}]) + 'zeroconf_allow': 'any'}]) l1.fundwallet(FUNDAMOUNT * 2) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) From ec9a663030b8092b628aaae58014c32d93468090 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 3 Jul 2025 14:19:06 +0930 Subject: [PATCH 4/7] pytest: test what happens if we *explicitly* ask for a zeroconf channel. Signed-off-by: Rusty Russell --- tests/test_opening.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_opening.py b/tests/test_opening.py index fd5bf2a94eb8..f218d2eaf4cc 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1836,6 +1836,26 @@ def test_zeroconf_forward(node_factory, bitcoind): l3.rpc.pay(inv) +def test_zeroconf_refusal(bitcoind, node_factory, chainparams): + """If we're not going to give you zeroconf, we should tell you!""" + l1, l2 = node_factory.get_nodes(2) + l1.fundwallet(10**6) + l1.connect(l2) + + # option_static_remotekey, option_zeroconf + ctype = [12, 50] + # No anchors for elements + if not chainparams['elements']: + ctype += [22] + with pytest.raises(RpcError, match="You required zeroconf, but you're not on our allowlist"): + l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=ctype) + + # OK, let's add ourselves to allow list. + plugin_path = str(Path(__file__).parent / "plugins" / "zeroconf-selective.py") + l2.rpc.plugin_start(plugin_path, zeroconf_allow=l1.info['id']) + l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=ctype) + + @pytest.mark.openchannel('v1') def test_buy_liquidity_ad_no_v2(node_factory, bitcoind): """ Test that you can't actually request amt for a From 8b95d175b249742d241e6e4d50c190aad51ba24b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 3 Jul 2025 15:22:55 +0930 Subject: [PATCH 5/7] pytest: actually test channel_type when negotiating prviate channel. Makes sure we don't break it! Signed-off-by: Rusty Russell --- tests/test_opening.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_opening.py b/tests/test_opening.py index f218d2eaf4cc..eb8a4cedd27f 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1999,7 +1999,8 @@ def test_scid_alias_private(node_factory, bitcoind): {'log-level': 'io'}]) l2.fundwallet(5000000) - l2.rpc.fundchannel(l3.info['id'], 'all', announce=False) + fc = l2.rpc.fundchannel(l3.info['id'], 'all', announce=False) + assert 'scid_alias/even' in fc['channel_type']['names'] bitcoind.generate_block(1, wait_for_mempool=1) wait_for(lambda: only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['state'] == 'CHANNELD_NORMAL') From 6f10d58b9f5719a41b47f57978aadb9ec9cc0530 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 11 Jul 2025 11:49:04 +0930 Subject: [PATCH 6/7] lightningd: always tell openingd/dualopend what channel type we want. Prior to it being compulsory, these daemons would need a default value. Now it's always required, it's clearer if it's always told. There's no "default_channel_type" now everyone has to specify channel_type either, so rename it to "desired_channel_type" and put it in lightningd specifically. Note that the channel_type can have options added: either option_scid_alias or option_zeroconf. This results in a slight behavior change: we will get type zeroconf even if we didn't ask for it, if they gave it to us. Signed-off-by: Rusty Russell Changelog-Changed: JSON-RPC: fundchannel / fundchannel_start returned `channel_type` will include option_zeroconf if it was implied by a 0 minimum_depth, even if we didn't explicitly ask for a zero conf channel. --- common/channel_type.c | 20 -------------------- common/channel_type.h | 5 ----- lightningd/channel.c | 12 +++++++++++- lightningd/channel.h | 4 ++++ lightningd/dual_open_control.c | 11 ++++++++--- lightningd/opening_control.c | 16 ++++++++++++---- openingd/dualopend.c | 19 +------------------ openingd/dualopend_wire.csv | 2 +- openingd/openingd.c | 32 ++++++++------------------------ openingd/openingd_wire.csv | 2 +- tests/test_opening.py | 13 +++++++------ 11 files changed, 53 insertions(+), 83 deletions(-) diff --git a/common/channel_type.c b/common/channel_type.c index 49c7e32dd707..61302b45c277 100644 --- a/common/channel_type.c +++ b/common/channel_type.c @@ -72,26 +72,6 @@ struct channel_type *channel_type_anchors_zero_fee_htlc(const tal_t *ctx) return type; } -struct channel_type *default_channel_type(const tal_t *ctx, - const struct feature_set *our_features, - const u8 *their_features) -{ - /* BOLT #2: - * Both peers: - * - if `channel_type` was present in both `open_channel` and `accept_channel`: - * - This is the `channel_type` (they must be equal, required above) - * - otherwise: - * - if `option_anchors` was negotiated: - * - the `channel_type` is `option_anchors` and `option_static_remotekey` (bits 22 and 12) - * - otherwise: - * - the `channel_type` is `option_static_remotekey` (bit 12) - */ - if (feature_negotiated(our_features, their_features, - OPT_ANCHORS_ZERO_FEE_HTLC_TX)) - return channel_type_anchors_zero_fee_htlc(ctx); - return channel_type_static_remotekey(ctx); -} - bool channel_type_has(const struct channel_type *type, int feature) { return feature_offered(type->features, feature); diff --git a/common/channel_type.h b/common/channel_type.h index 115c394fdeae..d2673eb3071d 100644 --- a/common/channel_type.h +++ b/common/channel_type.h @@ -21,11 +21,6 @@ struct channel_type *channel_type_dup(const tal_t *ctx, struct channel_type *channel_type_from(const tal_t *ctx, const u8 *features TAKES); -/* Derive channel type from feature negotiation */ -struct channel_type *default_channel_type(const tal_t *ctx, - const struct feature_set *our_features, - const u8 *their_features); - /* Does this type include this feature? */ bool channel_type_has(const struct channel_type *type, int feature); diff --git a/lightningd/channel.c b/lightningd/channel.c index 94ff06c7b842..346cb1c4c9fe 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -242,6 +242,16 @@ struct open_attempt *new_channel_open_attempt(struct channel *channel) return oa; } +struct channel_type *desired_channel_type(const tal_t *ctx, + const struct feature_set *our_features, + const u8 *their_features) +{ + if (feature_negotiated(our_features, their_features, + OPT_ANCHORS_ZERO_FEE_HTLC_TX)) + return channel_type_anchors_zero_fee_htlc(ctx); + return channel_type_static_remotekey(ctx); +} + struct channel *new_unsaved_channel(struct peer *peer, u32 feerate_base, u32 feerate_ppm) @@ -305,7 +315,7 @@ struct channel *new_unsaved_channel(struct peer *peer, channel->close_blockheight = NULL; /* In case someone looks at channels before open negotiation, * initialize this with default */ - channel->type = default_channel_type(channel, + channel->type = desired_channel_type(channel, ld->our_features, peer->their_features); diff --git a/lightningd/channel.h b/lightningd/channel.h index f133dd2b4e36..509c93fcd1bc 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -899,5 +899,9 @@ const u8 *channel_update_for_error(const tal_t *ctx, struct amount_msat htlc_max_possible_send(const struct channel *channel); +/* Given features, what channel_type do we want? */ +struct channel_type *desired_channel_type(const tal_t *ctx, + const struct feature_set *our_features, + const u8 *their_features); #endif /* LIGHTNING_LIGHTNINGD_CHANNEL_H */ diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index b96ac946d35a..4560f9f8c3e5 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -3249,8 +3249,11 @@ static struct command_result *json_openchannel_init(struct command *cmd, "by peer"); } - if (info->ctype && - !cmd->ld->dev_any_channel_type && + if (!info->ctype) + info->ctype = desired_channel_type(info, cmd->ld->our_features, + peer->their_features); + + if (!cmd->ld->dev_any_channel_type && !channel_type_accept(tmpctx, info->ctype->features, cmd->ld->our_features)) { @@ -3882,7 +3885,9 @@ static struct command_result *json_queryrates(struct command *cmd, NULL : request_amt, get_block_height(cmd->ld->topology), true, - NULL, NULL); + desired_channel_type(tmpctx, cmd->ld->our_features, + peer->their_features), + NULL); if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) { return command_fail(cmd, FUND_MAX_EXCEEDED, diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index f3e14005123c..43163835a02c 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -47,8 +47,7 @@ void json_add_uncommitted_channel(struct command *cmd, json_object_start(response, NULL); json_add_node_id(response, "peer_id", &peer->id); json_add_bool(response, "peer_connected", peer->connected == PEER_CONNECTED); - if (uc->fc->channel_type) - json_add_channel_type(response, "channel_type", uc->fc->channel_type); + json_add_channel_type(response, "channel_type", uc->fc->channel_type); json_add_string(response, "state", "OPENINGD"); json_add_string(response, "owner", "lightning_openingd"); json_add_string(response, "opener", "local"); @@ -332,6 +331,10 @@ static void opening_funder_start_replied(struct subd *openingd, const u8 *resp, { bool supports_shutdown_script; + /* It will tell us the resulting channel type (which can vary + * by ZEROCONF and SCID_ALIAS), so free old one */ + tal_free(fc->channel_type); + if (!fromwire_openingd_funder_start_reply(fc, resp, &fc->funding_scriptpubkey, &supports_shutdown_script, @@ -1297,7 +1300,7 @@ static struct command_result *json_fundchannel_start(struct command *cmd, } } } else { - fc->channel_type = NULL; + fc->channel_type = NULL; /* set later */ } if (!mindepth) @@ -1389,6 +1392,11 @@ static struct command_result *json_fundchannel_start(struct command *cmd, if (command_check_only(cmd)) return command_check_done(cmd); + /* Now we know the peer, we can derive the channel type to ask for */ + if (!fc->channel_type) + fc->channel_type = desired_channel_type(fc, cmd->ld->our_features, + peer->their_features); + fc->push = push_msat ? *push_msat : AMOUNT_MSAT(0); fc->channel_flags = OUR_CHANNEL_FLAGS; if (!*announce_channel) { @@ -1426,7 +1434,7 @@ static struct command_result *json_fundchannel_start(struct command *cmd, &tmp_channel_id, fc->channel_flags, reserve, - ctype); + fc->channel_type); if (!topology_synced(cmd->ld->topology)) { struct fundchannel_start_info *info diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 616df5dd8593..f6c183a917d2 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -2961,7 +2961,6 @@ static void opener_start(struct state *state, u8 *msg) struct amount_sat *requested_lease; size_t locktime; u32 nonanchor_feerate, anchor_feerate; - struct channel_type *ctype; if (!fromwire_dualopend_opener_init(state, msg, &tx_state->psbt, @@ -2975,7 +2974,7 @@ static void opener_start(struct state *state, u8 *msg) &requested_lease, &tx_state->blockheight, &dry_run, - &ctype, + &state->channel_type, &expected_rates)) master_badmsg(WIRE_DUALOPEND_OPENER_INIT, msg); @@ -2983,22 +2982,6 @@ static void opener_start(struct state *state, u8 *msg) wally_psbt_get_locktime(tx_state->psbt, &locktime); tx_state->tx_locktime = locktime; open_tlv = tlv_opening_tlvs_new(tmpctx); - - /* BOLT #2: - * - if it includes `channel_type`: - * - MUST set it to a defined type representing the type it wants. - * - MUST use the smallest bitmap possible to represent the channel - * type. - * - SHOULD NOT set it to a type containing a feature which was not - * negotiated. - */ - if (ctype) { - state->channel_type = ctype; - } else { - state->channel_type = default_channel_type(state, - state->our_features, - state->their_features); - } open_tlv->channel_type = state->channel_type->features; /* Given channel type, which feerate do we use? */ diff --git a/openingd/dualopend_wire.csv b/openingd/dualopend_wire.csv index 8d8915026c96..2d0479f56094 100644 --- a/openingd/dualopend_wire.csv +++ b/openingd/dualopend_wire.csv @@ -211,7 +211,7 @@ msgdata,dualopend_opener_init,channel_flags,u8, msgdata,dualopend_opener_init,requested_sats,?amount_sat, msgdata,dualopend_opener_init,blockheight,u32, msgdata,dualopend_opener_init,dry_run,bool, -msgdata,dualopend_opener_init,channel_type,?channel_type, +msgdata,dualopend_opener_init,channel_type,channel_type, # must go last because embedded tu32 msgdata,dualopend_opener_init,expected_rates,?lease_rates, diff --git a/openingd/openingd.c b/openingd/openingd.c index f436177035a5..60faefdeb5c4 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -294,7 +294,7 @@ static bool intuit_scid_alias_type(struct state *state, u8 channel_flags) * stop when we get to the part where we need the funding txid */ static u8 *funder_channel_start(struct state *state, u8 channel_flags, u32 nonanchor_feerate, u32 anchor_feerate, - const struct channel_type *ctype) + const struct channel_type *ctype TAKES) { u8 *msg; u8 *funding_output_script; @@ -323,28 +323,12 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, state->our_features, state->their_features); - if (ctype) { - state->channel_type = channel_type_dup(state, ctype); - } else { - state->channel_type = default_channel_type(state, - state->our_features, - state->their_features); - - /* Spec says we should use the option_scid_alias variation if we - * want them to *only* use the scid_alias (which we do for unannounced - * channels!). - * - * But: - * 1. We didn't accept this in CLN prior to v23.05. - * 2. LND won't accept that without OPT_ANCHORS_ZERO_FEE_HTLC_TX. - * 3. LND <= 18 won't accept OPT_SCID_ALIAS unless it advertizes it, - * which it does not by default. - */ - if (channel_type_has(state->channel_type, OPT_ANCHORS_ZERO_FEE_HTLC_TX) - && feature_offered(state->their_features, OPT_SCID_ALIAS)) { - if (!(channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL)) - channel_type_set_scid_alias(state->channel_type); - } + state->channel_type = channel_type_dup(state, ctype); + /* We set scid alias if we're not announcing */ + if (feature_negotiated(state->our_features, state->their_features, + OPT_SCID_ALIAS) + && !(channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL)) { + channel_type_set_scid_alias(state->channel_type); } /* Which feerate do we use? (We can lowball fees if using anchors!) */ @@ -569,7 +553,7 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, "We negotiated option_zeroconf, using our minimum_depth=%d", state->minimum_depth); /* We set this now to show we're zeroconf */ - if (their_mindepth == 0 && !ctype) + if (their_mindepth == 0) channel_type_set_zeroconf(state->channel_type); } else { state->minimum_depth = their_mindepth; diff --git a/openingd/openingd_wire.csv b/openingd/openingd_wire.csv index 65aac949fda4..c48e62b1cd71 100644 --- a/openingd/openingd_wire.csv +++ b/openingd/openingd_wire.csv @@ -88,7 +88,7 @@ msgdata,openingd_funder_start,anchor_feerate_per_kw,u32, msgdata,openingd_funder_start,temporary_channel_id,channel_id, msgdata,openingd_funder_start,channel_flags,u8, msgdata,openingd_funder_start,reserve,?amount_sat, -msgdata,openingd_funder_start,channel_type,?channel_type, +msgdata,openingd_funder_start,channel_type,channel_type, # openingd->master: send back output script for 2-of-2 funding output msgtype,openingd_funder_start_reply,6102 diff --git a/tests/test_opening.py b/tests/test_opening.py index eb8a4cedd27f..f91f1745947a 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -2614,8 +2614,9 @@ def test_opening_explicit_channel_type(node_factory, bitcoind): [STATIC_REMOTEKEY, ANCHORS_ZERO_FEE_HTLC_TX]): ret = l1.rpc.fundchannel_start(l2.info['id'], FUNDAMOUNT, channel_type=ctype + zeroconf) - assert ret['channel_type']['bits'] == ctype + zeroconf - assert only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['bits'] == ctype + zeroconf + # We get zeroconf even without asking for it. + assert ret['channel_type']['bits'] == ctype + [ZEROCONF] + assert only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['bits'] == ctype + [ZEROCONF] # Note: l2 doesn't show it in listpeerchannels yet... l1.rpc.fundchannel_cancel(l2.info['id']) @@ -2664,8 +2665,8 @@ def test_opening_explicit_channel_type(node_factory, bitcoind): l1.connect(l2) ret = l1.rpc.fundchannel_start(l2.info['id'], FUNDAMOUNT, channel_type=[STATIC_REMOTEKEY, ANCHORS_OLD]) - assert ret['channel_type']['bits'] == [STATIC_REMOTEKEY, ANCHORS_OLD] - assert only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['bits'] == [STATIC_REMOTEKEY, ANCHORS_OLD] + assert ret['channel_type']['bits'] == [STATIC_REMOTEKEY, ANCHORS_OLD, ZEROCONF] + assert only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['bits'] == [STATIC_REMOTEKEY, ANCHORS_OLD, ZEROCONF] # Note: l3 doesn't show it in listpeerchannels yet... l1.rpc.fundchannel_cancel(l2.info['id']) @@ -2673,8 +2674,8 @@ def test_opening_explicit_channel_type(node_factory, bitcoind): # Works with fundchannel / multifundchannel ret = l1.rpc.fundchannel(l2.info['id'], FUNDAMOUNT // 3, channel_type=[STATIC_REMOTEKEY]) - assert ret['channel_type']['bits'] == [STATIC_REMOTEKEY] - assert only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['bits'] == [STATIC_REMOTEKEY] + assert ret['channel_type']['bits'] == [STATIC_REMOTEKEY, ZEROCONF] + assert only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['bits'] == [STATIC_REMOTEKEY, ZEROCONF] assert only_one(l2.rpc.listpeerchannels()['channels'])['channel_type']['bits'] == [STATIC_REMOTEKEY] # FIXME: Check type is actually correct! From 3dd750095e80b9c404f6953a1b4f632a5eaa4cba Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 11 Jul 2025 11:49:06 +0930 Subject: [PATCH 7/7] openingd: remove compat hacks to "intuit" opt_scid_alias. This was needed for v23.05 which would set opt_scid_alias even if we didn't. Now everyone handles it properly, we can simply set it unconditionally. Signed-off-by: Rusty Russell Changelog-Removed: Protocol: backwards compatibility allowances for CLN before 23.08 which didn't handle option_scid_alias properly. --- openingd/openingd.c | 66 +++++---------------------------------------- 1 file changed, 6 insertions(+), 60 deletions(-) diff --git a/openingd/openingd.c b/openingd/openingd.c index 60faefdeb5c4..e98609d09d9a 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -268,28 +268,6 @@ static void set_remote_upfront_shutdown(struct state *state, peer_failed_err(state->pps, &state->channel_id, "%s", err); } -/* Since we can't send OPT_SCID_ALIAS due to compat issues, intuit whether - * we really actually want it anyway, we just can't say that. */ -static bool intuit_scid_alias_type(struct state *state, u8 channel_flags) -{ - /* Don't need to intuit if actually set */ - if (channel_type_has(state->channel_type, OPT_SCID_ALIAS)) - return false; - - /* Modern peer: no intuit hacks necessary. */ - if (channel_type_has(state->channel_type, OPT_ANCHORS_ZERO_FEE_HTLC_TX)) - return false; - - /* Public channel: don't want OPT_SCID_ALIAS which means "only use - * alias". */ - if (channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL) - return false; - - /* If we both support it, presumably we want it? */ - return feature_negotiated(state->our_features, state->their_features, - OPT_SCID_ALIAS); -} - /* We start the 'open a channel' negotation with the supplied peer, but * stop when we get to the part where we need the funding txid */ static u8 *funder_channel_start(struct state *state, u8 channel_flags, @@ -431,34 +409,12 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, } /* Simple case: caller specified, don't allow any variants */ - if (ctype) { - if (!featurebits_eq(accept_tlvs->channel_type, state->channel_type->features)) { - negotiation_failed(state, - "Return unoffered channel_type: %s", - fmt_featurebits(tmpctx, - accept_tlvs->channel_type)); - return NULL; - } - } else { - /* Except that v23.05 could set OPT_SCID_ALIAS in reply! */ - struct channel_type *atype; - - atype = channel_type_from(tmpctx, accept_tlvs->channel_type); - if (!channel_type_has(atype, OPT_ANCHORS_ZERO_FEE_HTLC_TX)) - featurebits_unset(&atype->features, OPT_SCID_ALIAS); - - if (!channel_type_eq(atype, state->channel_type)) { - negotiation_failed(state, - "Return unoffered channel_type: %s", - fmt_featurebits(tmpctx, - accept_tlvs->channel_type)); - return NULL; - } - - /* If they "accepted" SCID_ALIAS, roll with it. */ - tal_free(state->channel_type); - state->channel_type = channel_type_from(state, - accept_tlvs->channel_type); + if (!featurebits_eq(accept_tlvs->channel_type, state->channel_type->features)) { + negotiation_failed(state, + "Return unoffered channel_type: %s", + fmt_featurebits(tmpctx, + accept_tlvs->channel_type)); + return NULL; } /* BOLT #2: @@ -564,11 +520,6 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, "Funding channel start: awaiting funding_txid with output to %s", tal_hex(tmpctx, funding_output_script)); - /* Backwards/cross compat hack */ - if (!ctype && intuit_scid_alias_type(state, channel_flags)) { - channel_type_set_scid_alias(state->channel_type); - } - return towire_openingd_funder_start_reply(state, funding_output_script, feature_negotiated( @@ -1163,11 +1114,6 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) &state->channel_id), fmt_channel_id(msg, &id_in)); - /* Backwards/cross compat hack */ - if (intuit_scid_alias_type(state, channel_flags)) { - channel_type_set_scid_alias(state->channel_type); - } - /*~ Channel is ready; Report the channel parameters to the signer. */ msg = towire_hsmd_setup_channel(NULL, false, /* is_outbound */