From 1e1a74facdbccb487746de5cc23a5c35fa6ea5f4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 25 May 2021 10:19:39 +0930 Subject: [PATCH 1/4] lightningd: respect anysegwit on dual-funding opens too. Instead of open-coding, use the helper. Signed-off-by: Rusty Russell --- lightningd/dual_open_control.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index 292531e1173f..5155129eaa9e 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -1212,6 +1213,9 @@ static void handle_peer_wants_to_close(struct subd *dualopend, struct lightningd *ld = dualopend->ld; struct channel *channel = dualopend->channel; char *errmsg; + bool anysegwit = feature_negotiated(ld->our_features, + channel->peer->their_features, + OPT_SHUTDOWN_ANYSEGWIT); /* We shouldn't get this message while we're waiting to finish */ if (channel_unsaved(channel)) { @@ -1237,20 +1241,13 @@ static void handle_peer_wants_to_close(struct subd *dualopend, channel->shutdown_scriptpubkey[REMOTE] = scriptpubkey; /* BOLT #2: - * - * 1. `OP_DUP` `OP_HASH160` `20` 20-bytes `OP_EQUALVERIFY` `OP_CHECKSIG` - * (pay to pubkey hash), OR - * 2. `OP_HASH160` `20` 20-bytes `OP_EQUAL` (pay to script hash), OR - * 3. `OP_0` `20` 20-bytes (version 0 pay to witness pubkey hash), OR - * 4. `OP_0` `32` 32-bytes (version 0 pay to witness script hash) * * A receiving node: *... * - if the `scriptpubkey` is not in one of the above forms: * - SHOULD fail the connection. */ - if (!is_p2pkh(scriptpubkey, NULL) && !is_p2sh(scriptpubkey, NULL) - && !is_p2wpkh(scriptpubkey, NULL) && !is_p2wsh(scriptpubkey, NULL)) { + if (!valid_shutdown_scriptpubkey(scriptpubkey, anysegwit)) { channel_fail_permanent(channel, REASON_PROTOCOL, "Bad shutdown scriptpubkey %s", From 0a7be967d43d14c41befb3cbe58d00250b91b749 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 26 May 2021 13:39:01 +0930 Subject: [PATCH 2/4] lightningd: option_shutdown_anysegwit is no longer experimental. https://github.com/lightningnetwork/lightning-rfc/pull/672 was merged. Signed-off-by: Rusty Russell Changelog-Added: Protocol: `option_shutdown_anysegwit` allows future segwit versions on shutdown transactions. --- common/features.c | 2 -- lightningd/lightningd.c | 2 +- tests/test_closing.py | 15 ++++----------- tests/test_misc.py | 2 ++ tests/utils.py | 8 ++------ 5 files changed, 9 insertions(+), 20 deletions(-) diff --git a/common/features.c b/common/features.c index 2ea1c0403e9a..681fdc334e99 100644 --- a/common/features.c +++ b/common/features.c @@ -86,12 +86,10 @@ static const struct feature_style feature_styles[] = { [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT, [BOLT11_FEATURE] = FEATURE_REPRESENT, [CHANNEL_FEATURE] = FEATURE_DONT_REPRESENT} }, -#if EXPERIMENTAL_FEATURES { OPT_SHUTDOWN_ANYSEGWIT, .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT, [CHANNEL_FEATURE] = FEATURE_DONT_REPRESENT } }, -#endif }; struct dependency { diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 4ae92196386d..ca9a1ae97e7a 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -802,10 +802,10 @@ static struct feature_set *default_features(const tal_t *ctx) OPTIONAL_FEATURE(OPT_BASIC_MPP), OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES_EX), OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY), + OPTIONAL_FEATURE(OPT_SHUTDOWN_ANYSEGWIT), #if EXPERIMENTAL_FEATURES OPTIONAL_FEATURE(OPT_ANCHOR_OUTPUTS), OPTIONAL_FEATURE(OPT_ONION_MESSAGES), - OPTIONAL_FEATURE(OPT_SHUTDOWN_ANYSEGWIT), #endif }; diff --git a/tests/test_closing.py b/tests/test_closing.py index cc34894bc59a..9638ce8591a3 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -6,7 +6,7 @@ from utils import ( only_one, sync_blockheight, wait_for, TIMEOUT, account_balance, first_channel_id, basic_fee, TEST_NETWORK, - EXPERIMENTAL_FEATURES, scriptpubkey_addr + scriptpubkey_addr ) import os @@ -2659,15 +2659,8 @@ def test_segwit_shutdown_script(node_factory, bitcoind, executor): else: valid = edge_valid + other_valid - if EXPERIMENTAL_FEATURES: - xsuccess = valid - xfail = invalid - else: - xsuccess = [] - xfail = valid + invalid - # More efficient to create them all up-front. - nodes = node_factory.get_nodes(len(xfail) + len(xsuccess)) + nodes = node_factory.get_nodes(len(valid) + len(invalid)) # Give it one UTXO to spend for each node. addresses = {} @@ -2680,7 +2673,7 @@ def test_segwit_shutdown_script(node_factory, bitcoind, executor): # FIXME: Since we don't support other non-v0 encodings, we need a protocol # test for this (we're actually testing our upfront check, not the real # shutdown one!), - for script in xsuccess: + for script in valid: # Insist on upfront script we're not going to match. l1.stop() l1.daemon.env["DEV_OPENINGD_UPFRONT_SHUTDOWN_SCRIPT"] = script @@ -2690,7 +2683,7 @@ def test_segwit_shutdown_script(node_factory, bitcoind, executor): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.rpc.fundchannel(l2.info['id'], 10**6) - for script in xfail: + for script in invalid: # Insist on upfront script we're not going to match. l1.stop() l1.daemon.env["DEV_OPENINGD_UPFRONT_SHUTDOWN_SCRIPT"] = script diff --git a/tests/test_misc.py b/tests/test_misc.py index b0f7b2a4a424..22e7931e1abd 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1897,6 +1897,8 @@ def test_list_features_only(node_factory): expected += ['option_anchor_outputs/odd'] expected += ['option_shutdown_anysegwit/odd'] expected += ['option_onion_messages/odd'] + else: + expected += ['option_shutdown_anysegwit/odd'] assert features == expected diff --git a/tests/utils.py b/tests/utils.py index 2c0e08840aa3..de1a7060cf95 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -20,14 +20,12 @@ def hex_bits(features): def expected_peer_features(wumbo_channels=False, extra=[]): """Return the expected peer features hexstring for this configuration""" - features = [1, 5, 7, 9, 11, 13, 15, 17] + features = [1, 5, 7, 9, 11, 13, 15, 17, 27] if EXPERIMENTAL_FEATURES: # OPT_ONION_MESSAGES features += [103] # option_anchor_outputs features += [21] - # option_shutdown_anysegwit - features += [27] if wumbo_channels: features += [19] if EXPERIMENTAL_DUAL_FUND: @@ -42,14 +40,12 @@ def expected_peer_features(wumbo_channels=False, extra=[]): # features for the 'node' and the 'peer' feature sets def expected_node_features(wumbo_channels=False, extra=[]): """Return the expected node features hexstring for this configuration""" - features = [1, 5, 7, 9, 11, 13, 15, 17, 55] + features = [1, 5, 7, 9, 11, 13, 15, 17, 27, 55] if EXPERIMENTAL_FEATURES: # OPT_ONION_MESSAGES features += [103] # option_anchor_outputs features += [21] - # option_shutdown_anysegwit - features += [27] if wumbo_channels: features += [19] if EXPERIMENTAL_DUAL_FUND: From ec04f54034210db1c275a67be4d4d8798ee3f331 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 26 May 2021 13:39:06 +0930 Subject: [PATCH 3/4] channeld: update test vectors for msat differentiation test. As per https://github.com/lightningnetwork/lightning-rfc/pull/872 Signed-off-by: Rusty Russell --- channeld/test/run-commit_tx.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/channeld/test/run-commit_tx.c b/channeld/test/run-commit_tx.c index 8fc6d2d1fae7..c8b1920489bf 100644 --- a/channeld/test/run-commit_tx.c +++ b/channeld/test/run-commit_tx.c @@ -175,14 +175,14 @@ static const struct htlc **setup_htlcs_0_to_4(const tal_t *ctx) return htlcs; } -/* BOLT #3: +/* BOLT-3508e4e85d26240ae7492c3d2e02770cdc360fe9 #3: * htlc 5 direction: local->remote * htlc 5 amount_msat: 5000000 - * htlc 5 expiry: 505 + * htlc 5 expiry: 506 * htlc 5 payment_preimage: 0505050505050505050505050505050505050505050505050505050505050505 * htlc 6 direction: local->remote - * htlc 6 amount_msat: 5000000 - * htlc 6 expiry: 506 + * htlc 6 amount_msat: 5000001 + * htlc 6 expiry: 505 * htlc 6 payment_preimage: 0505050505050505050505050505050505050505050505050505050505050505 */ static const struct htlc **setup_htlcs_1_5_and_6(const tal_t *ctx) @@ -211,7 +211,7 @@ static const struct htlc **setup_htlcs_1_5_and_6(const tal_t *ctx) break; case 6: htlc->state = SENT_ADD_ACK_REVOCATION; - htlc->amount = AMOUNT_MSAT(5000000); + htlc->amount = AMOUNT_MSAT(5000001); htlc->expiry.locktime = 506; memset(htlc->r, 5, sizeof(*htlc->r)); break; From dd5326182275c1ef667825460972b8889f640350 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 26 May 2021 13:39:06 +0930 Subject: [PATCH 4/4] Makefile: update to latest spec. This includes anysegwit and the updated HTLC tiebreak test vector. It also adds explicit wording for invalid per_commitment_secret (which nicely matches our code already!). Signed-off-by: Rusty Russell --- Makefile | 2 +- channeld/channeld.c | 4 ++-- channeld/test/run-commit_tx.c | 2 +- common/features.h | 6 +----- common/shutdown_scriptpubkey.c | 2 +- common/shutdown_scriptpubkey.h | 5 ++++- tests/test_closing.py | 2 +- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 3628bc7347ea..646d90e5ce95 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../lightning-rfc/ -DEFAULT_BOLTVERSION := b201efe0546120c14bf154ce5f4e18da7243fe7a +DEFAULT_BOLTVERSION := 3508e4e85d26240ae7492c3d2e02770cdc360fe9 # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/channeld/channeld.c b/channeld/channeld.c index 885086941b0c..862f5e5a6e8f 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1494,8 +1494,8 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) /* BOLT #2: * * A receiving node: - * - if `per_commitment_secret` does not generate the previous - * `per_commitment_point`: + * - if `per_commitment_secret` is not a valid secret key or does not + * generate the previous `per_commitment_point`: * - MUST fail the channel. */ memcpy(&privkey, &old_commit_secret, sizeof(privkey)); diff --git a/channeld/test/run-commit_tx.c b/channeld/test/run-commit_tx.c index c8b1920489bf..66590b49f336 100644 --- a/channeld/test/run-commit_tx.c +++ b/channeld/test/run-commit_tx.c @@ -175,7 +175,7 @@ static const struct htlc **setup_htlcs_0_to_4(const tal_t *ctx) return htlcs; } -/* BOLT-3508e4e85d26240ae7492c3d2e02770cdc360fe9 #3: +/* BOLT #3: * htlc 5 direction: local->remote * htlc 5 amount_msat: 5000000 * htlc 5 expiry: 506 diff --git a/common/features.h b/common/features.h index e4236b847f58..a03d6b5ced8e 100644 --- a/common/features.h +++ b/common/features.h @@ -107,16 +107,12 @@ u8 *featurebits_or(const tal_t *ctx, const u8 *f1 TAKES, const u8 *f2 TAKES); * | 16/17 | `basic_mpp` |... IN9 ... * | 18/19 | `option_support_large_channel` |... IN ... * | 20/21 | `option_anchor_outputs` |... IN ... + * | 26/27 | `option_shutdown_anysegwit` |... IN ... */ #define OPT_PAYMENT_SECRET 14 #define OPT_BASIC_MPP 16 #define OPT_LARGE_CHANNELS 18 #define OPT_ANCHOR_OUTPUTS 20 - -/* BOLT-4e329271a358ee52bf43ddbd96776943c5d74508 #9: - * - * | 26/27 | `option_shutdown_anysegwit` |... IN ... - */ #define OPT_SHUTDOWN_ANYSEGWIT 26 /* BOLT-f53ca2301232db780843e894f55d95d512f297f9 #9: diff --git a/common/shutdown_scriptpubkey.c b/common/shutdown_scriptpubkey.c index e875e60a9934..ceaf5dde39f9 100644 --- a/common/shutdown_scriptpubkey.c +++ b/common/shutdown_scriptpubkey.c @@ -3,7 +3,7 @@ #include -/* BOLT-4e329271a358ee52bf43ddbd96776943c5d74508 #2: +/* BOLT #2: * 5. if (and only if) `option_shutdown_anysegwit` is negotiated: * * `OP_1` through `OP_16` inclusive, followed by a single * push of 2 to 40 bytes diff --git a/common/shutdown_scriptpubkey.h b/common/shutdown_scriptpubkey.h index 65544f42d4c2..90189f75b9d0 100644 --- a/common/shutdown_scriptpubkey.h +++ b/common/shutdown_scriptpubkey.h @@ -9,7 +9,10 @@ * (pay to pubkey hash), OR * 2. `OP_HASH160` `20` 20-bytes `OP_EQUAL` (pay to script hash), OR * 3. `OP_0` `20` 20-bytes (version 0 pay to witness pubkey hash), OR - * 4. `OP_0` `32` 32-bytes (version 0 pay to witness script hash) + * 4. `OP_0` `32` 32-bytes (version 0 pay to witness script hash), OR + * 5. if (and only if) `option_shutdown_anysegwit` is negotiated: + * * `OP_1` through `OP_16` inclusive, followed by a single push of 2 to 40 bytes + * (witness program versions 1 through 16) * * A receiving node: *... diff --git a/tests/test_closing.py b/tests/test_closing.py index 9638ce8591a3..cd95224c2036 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -2626,7 +2626,7 @@ def test_segwit_shutdown_script(node_factory, bitcoind, executor): """ l1 = node_factory.get_node(allow_warning=True) - # BOLT-4e329271a358ee52bf43ddbd96776943c5d74508 #2: + # BOLT #2: # 5. if (and only if) `option_shutdown_anysegwit` is negotiated: # * `OP_1` through `OP_16` inclusive, followed by a single push of 2 to 40 bytes # (witness program versions 1 through 16)