From 9f06a59e3c931e111e3d851f54a27400f91efbd7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 31 Mar 2022 19:40:50 +1030 Subject: [PATCH] shutdown: don't allow shutdown to p2pkh or p2sh addresses for anchor outputs. This doesn't have an effect now (except in experimental mode), but it will when we support anchors. So we deprecate the use of those in the close command too. For experimental mode we have to avoid using p2pkh; adapt that test. Signed-off-by: Rusty Russell Changelog-Deprecated: JSON-RPC: `shutdown` no longer allows p2pkh or p2sh addresses. --- common/features.c | 10 ++++++++++ common/features.h | 2 ++ common/shutdown_scriptpubkey.c | 15 ++++++++++----- common/shutdown_scriptpubkey.h | 14 ++++++-------- doc/lightning-close.7.md | 2 +- lightningd/channel_control.c | 8 +++++++- lightningd/closing_control.c | 5 +++-- lightningd/dual_open_control.c | 8 +++++++- openingd/openingd.c | 8 +++++++- tests/test_closing.py | 7 ++++--- 10 files changed, 57 insertions(+), 22 deletions(-) diff --git a/common/features.c b/common/features.c index 6647e7ff7..360946d74 100644 --- a/common/features.c +++ b/common/features.c @@ -80,6 +80,10 @@ static const struct feature_style feature_styles[] = { .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT, [CHANNEL_FEATURE] = FEATURE_DONT_REPRESENT } }, + { OPT_ANCHORS_ZERO_FEE_HTLC_TX, + .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, + [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT, + [CHANNEL_FEATURE] = FEATURE_DONT_REPRESENT } }, { OPT_DUAL_FUND, .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT, [NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT, @@ -119,6 +123,12 @@ static const struct dependency feature_deps[] = { * `option_anchor_outputs` | ... | ... | `option_static_remotekey` */ { OPT_ANCHOR_OUTPUTS, OPT_STATIC_REMOTEKEY }, + /* BOLT #9: + * Name | Description | Context | Dependencies | + *... + * `option_anchors_zero_fee_htlc_tx` | ... | ... | `option_static_remotekey` + */ + { OPT_ANCHORS_ZERO_FEE_HTLC_TX, OPT_STATIC_REMOTEKEY }, /* BOLT-f53ca2301232db780843e894f55d95d512f297f9 #9: * Name | Description | Context | Dependencies | * ... diff --git a/common/features.h b/common/features.h index 0bb9dfea9..1aa8ca829 100644 --- a/common/features.h +++ b/common/features.h @@ -117,12 +117,14 @@ const char *fmt_featurebits(const tal_t *ctx, const u8 *featurebits); * | 16/17 | `basic_mpp` |... IN9 ... * | 18/19 | `option_support_large_channel` |... IN ... * | 20/21 | `option_anchor_outputs` |... IN ... + * | 22/23 | `option_anchors_zero_fee_htlc_tx` |... 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 +#define OPT_ANCHORS_ZERO_FEE_HTLC_TX 22 #define OPT_SHUTDOWN_ANYSEGWIT 26 /* BOLT-f53ca2301232db780843e894f55d95d512f297f9 #9: diff --git a/common/shutdown_scriptpubkey.c b/common/shutdown_scriptpubkey.c index ad79ff20f..4a09ef30a 100644 --- a/common/shutdown_scriptpubkey.c +++ b/common/shutdown_scriptpubkey.c @@ -3,7 +3,7 @@ #include /* BOLT #2: - * 5. if (and only if) `option_shutdown_anysegwit` is negotiated: + * 3. 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) @@ -47,11 +47,16 @@ static bool is_valid_witnessprog(const u8 *scriptpubkey) } bool valid_shutdown_scriptpubkey(const u8 *scriptpubkey, - bool anysegwit) + bool anysegwit, + bool anchors) { - return is_p2pkh(scriptpubkey, NULL) - || is_p2sh(scriptpubkey, NULL) - || is_p2wpkh(scriptpubkey, NULL) + if (!anchors) { + if (is_p2pkh(scriptpubkey, NULL) + || is_p2sh(scriptpubkey, NULL)) + return true; + } + + return is_p2wpkh(scriptpubkey, NULL) || is_p2wsh(scriptpubkey, NULL) || (anysegwit && is_valid_witnessprog(scriptpubkey)); } diff --git a/common/shutdown_scriptpubkey.h b/common/shutdown_scriptpubkey.h index 90189f75b..e8c1fac1c 100644 --- a/common/shutdown_scriptpubkey.h +++ b/common/shutdown_scriptpubkey.h @@ -5,21 +5,19 @@ /* 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), OR - * 5. if (and only if) `option_shutdown_anysegwit` is negotiated: + * 1. `OP_0` `20` 20-bytes (version 0 pay to witness pubkey hash), OR + * 2. `OP_0` `32` 32-bytes (version 0 pay to witness script hash), OR + * 3. 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: *... * - if the `scriptpubkey` is not in one of the above forms: - * - SHOULD fail the connection. + * - SHOULD send a `warning` */ bool valid_shutdown_scriptpubkey(const u8 *scriptpubkey, - bool anysegwit); + bool anysegwit, + bool anchors); #endif /* LIGHTNING_COMMON_SHUTDOWN_SCRIPTPUBKEY_H */ diff --git a/doc/lightning-close.7.md b/doc/lightning-close.7.md index 5f05818d5..5d8fc6f66 100644 --- a/doc/lightning-close.7.md +++ b/doc/lightning-close.7.md @@ -25,7 +25,7 @@ If *unilateraltimeout* is zero, then the **close** command will wait indefinitely until the peer is online and can negotiate a mutual close. The default is 2 days (172800 seconds). -The *destination* can be of any Bitcoin accepted type, including bech32. +The *destination* can be of any Bitcoin bech32 type. If it isn't specified, the default is a c-lightning wallet address. If the peer hasn't offered the `option_shutdown_anysegwit` feature, then taproot addresses (or other v1+ segwit) are not allowed. Tell your diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index b6c1d4353..a46b47eac 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -275,6 +275,12 @@ static void peer_got_shutdown(struct channel *channel, const u8 *msg) bool anysegwit = feature_negotiated(ld->our_features, channel->peer->their_features, OPT_SHUTDOWN_ANYSEGWIT); + bool anchors = feature_negotiated(ld->our_features, + channel->peer->their_features, + OPT_ANCHOR_OUTPUTS) + || feature_negotiated(ld->our_features, + channel->peer->their_features, + OPT_ANCHORS_ZERO_FEE_HTLC_TX); if (!fromwire_channeld_got_shutdown(channel, msg, &scriptpubkey, &wrong_funding)) { @@ -287,7 +293,7 @@ static void peer_got_shutdown(struct channel *channel, const u8 *msg) tal_free(channel->shutdown_scriptpubkey[REMOTE]); channel->shutdown_scriptpubkey[REMOTE] = scriptpubkey; - if (!valid_shutdown_scriptpubkey(scriptpubkey, anysegwit)) { + if (!valid_shutdown_scriptpubkey(scriptpubkey, anysegwit, anchors)) { channel_fail_permanent(channel, REASON_PROTOCOL, "Bad shutdown scriptpubkey %s", diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index b11d05f3f..33589e28a 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -684,11 +685,11 @@ static struct command_result *json_close(struct command *cmd, channel->peer->their_features, OPT_SHUTDOWN_ANYSEGWIT); if (!valid_shutdown_scriptpubkey(channel->shutdown_scriptpubkey[LOCAL], - anysegwit)) { + anysegwit, !deprecated_apis)) { /* Explicit check for future segwits. */ if (!anysegwit && valid_shutdown_scriptpubkey(channel->shutdown_scriptpubkey - [LOCAL], true)) { + [LOCAL], true, !deprecated_apis)) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Peer does not allow v1+ shutdown addresses"); } diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index 6151984ad..6eaecd098 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -1289,6 +1289,12 @@ static void handle_peer_wants_to_close(struct subd *dualopend, bool anysegwit = feature_negotiated(ld->our_features, channel->peer->their_features, OPT_SHUTDOWN_ANYSEGWIT); + bool anchors = feature_negotiated(ld->our_features, + channel->peer->their_features, + OPT_ANCHOR_OUTPUTS) + || feature_negotiated(ld->our_features, + channel->peer->their_features, + OPT_ANCHORS_ZERO_FEE_HTLC_TX); /* We shouldn't get this message while we're waiting to finish */ if (channel_unsaved(channel)) { @@ -1320,7 +1326,7 @@ static void handle_peer_wants_to_close(struct subd *dualopend, * - if the `scriptpubkey` is not in one of the above forms: * - SHOULD fail the connection. */ - if (!valid_shutdown_scriptpubkey(scriptpubkey, anysegwit)) { + if (!valid_shutdown_scriptpubkey(scriptpubkey, anysegwit, anchors)) { channel_fail_permanent(channel, REASON_PROTOCOL, "Bad shutdown scriptpubkey %s", diff --git a/openingd/openingd.c b/openingd/openingd.c index 87bdbf25d..04807cd5d 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -276,6 +276,12 @@ static void set_remote_upfront_shutdown(struct state *state, bool anysegwit = feature_negotiated(state->our_features, state->their_features, OPT_SHUTDOWN_ANYSEGWIT); + bool anchors = feature_negotiated(state->our_features, + state->their_features, + OPT_ANCHOR_OUTPUTS) + || feature_negotiated(state->our_features, + state->their_features, + OPT_ANCHORS_ZERO_FEE_HTLC_TX); /* BOLT #2: * @@ -291,7 +297,7 @@ static void set_remote_upfront_shutdown(struct state *state, = tal_steal(state, shutdown_scriptpubkey); if (shutdown_scriptpubkey - && !valid_shutdown_scriptpubkey(shutdown_scriptpubkey, anysegwit)) + && !valid_shutdown_scriptpubkey(shutdown_scriptpubkey, anysegwit, anchors)) peer_failed_err(state->pps, &state->channel_id, "Unacceptable upfront_shutdown_script %s", diff --git a/tests/test_closing.py b/tests/test_closing.py index 62e3fdda6..0013cde15 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -3194,7 +3194,8 @@ def test_option_upfront_shutdown_script(node_factory, bitcoind, executor): # this test, so l1 reports the error as a warning! l1 = node_factory.get_node(start=False, allow_warning=True) # Insist on upfront script we're not going to match. - l1.daemon.env["DEV_OPENINGD_UPFRONT_SHUTDOWN_SCRIPT"] = "76a91404b61f7dc1ea0dc99424464cc4064dc564d91e8988ac" + # '0014' + l1.rpc.call('dev-listaddrs', [10])['addresses'][-1]['bech32_redeemscript'] + l1.daemon.env["DEV_OPENINGD_UPFRONT_SHUTDOWN_SCRIPT"] = "00143d43d226bcc27019ade52d7a3dc52a7ac1be28b8" l1.start() l2 = node_factory.get_node() @@ -3205,7 +3206,7 @@ def test_option_upfront_shutdown_script(node_factory, bitcoind, executor): fut = executor.submit(l1.rpc.close, l2.info['id']) # l2 will close unilaterally when it dislikes shutdown script. - l1.daemon.wait_for_log(r'scriptpubkey .* is not as agreed upfront \(76a91404b61f7dc1ea0dc99424464cc4064dc564d91e8988ac\)') + l1.daemon.wait_for_log(r'scriptpubkey .* is not as agreed upfront \(00143d43d226bcc27019ade52d7a3dc52a7ac1be28b8\)') # Clear channel. wait_for(lambda: len(bitcoind.rpc.getrawmempool()) != 0) @@ -3221,7 +3222,7 @@ def test_option_upfront_shutdown_script(node_factory, bitcoind, executor): l2.rpc.close(l1.info['id']) # l2 will close unilaterally when it dislikes shutdown script. - l1.daemon.wait_for_log(r'scriptpubkey .* is not as agreed upfront \(76a91404b61f7dc1ea0dc99424464cc4064dc564d91e8988ac\)') + l1.daemon.wait_for_log(r'scriptpubkey .* is not as agreed upfront \(00143d43d226bcc27019ade52d7a3dc52a7ac1be28b8\)') # Clear channel. wait_for(lambda: len(bitcoind.rpc.getrawmempool()) != 0)