lightningd: accept OP_RETURN closer_scriptpubkey in simple-close check
What changed, and why it matters
This commit fixes a bug in Core Lightning's experimental simple-close feature. When two Lightning nodes tried to close a channel cooperatively using the new 'simple close' protocol, one peer was allowed by the specification to burn its own share of the funds to an unusable 'OP_RETURN' output instead of sending it to a normal address. Core Lightning's safety check did not recognize this valid case, so it rejected the mutually signed closing transaction and force-closed the channel on-chain instead. The fix teaches Core Lightning to accept a zero-value OP_RETURN output from the peer, but only when simple close was negotiated and only if the output carries no funds. A regression test is included.
Treat as a bug-fix commit with limited security impact. Reviewers should verify that is_valid_op_return correctly implements BOLT 2's length rules and that amount_sat_eq cannot be bypassed. Because the feature is experimental, no urgent release action is required, but the regression test should be run in CI.
Security signals we found
Denial of service / unwanted force-close: a spec-compliant peer could cause the local node to force-close a channel, incurring on-chain fees and delays.
Funds not stolen: the OP_RETURN output is unspendable, so accepting it cannot redirect funds to an attacker.
Value-zero check prevents burning real funds: the patch enforces AMOUNT_SAT(0) on the OP_RETURN output.
Feature-gated: the exception applies only when OPT_SIMPLE_CLOSE was negotiated.
Experimental feature: the changelog notes option_simple_close is experimental and unreleased, limiting real-world exposure.
Evidence from the diff
In lightningd/simple_close_control.c, close_tx_check previously required every transaction output to match one of the two stored shutdown_scriptpubkey values. Under option_simple_close, BOLT 2 permits the closer to set closer_scriptpubkey to a spec-valid OP_RETURN, which is unspendable and must have value zero. The old check therefore rejected a valid closing_complete transaction, causing lightningd to treat its own signed transaction as invalid and force-close. The patch adds an exception: if option_simple_close is negotiated, the script is a valid OP_RETURN, and the output amount is exactly zero, the output is accepted. The change also adds a unit test file, run-close_tx_check.c, covering acceptance, feature-gated rejection, non-zero-value rejection, unknown-script rejection, and normal shutdown-script acceptance.
Changed components
lightningd/simple_close_control.clightningd/test/run-close_tx_check.cclosing transaction validation for option_simple_closeInspect captured patch +356 / −5
diff --git a/lightningd/simple_close_control.c b/lightningd/simple_close_control.c
index cd52fef6..07e08473 100644
--- a/lightningd/simple_close_control.c
+++ b/lightningd/simple_close_control.c
@@ -4,6 +4,7 @@
#include <bitcoin/signature.h>
#include <ccan/tal/str/str.h>
#include <closingd/simpleclosed_wiregen.h>
+#include <common/features.h>
#include <common/fee_states.h>
#include <common/memleak.h>
#include <common/shutdown_scriptpubkey.h>
@@ -55,11 +56,36 @@ static const char *close_tx_check(const tal_t *ctx,
}
const u8 *script = tal_dup_arr(ctx, u8,
out->script, out->script_len, 0);
- if (!scripteq(script, channel->shutdown_scriptpubkey[LOCAL])
- && !scripteq(script, channel->shutdown_scriptpubkey[REMOTE]))
- return tal_fmt(ctx,
- "output %zu goes to unknown script %s",
- i, tal_hex(ctx, script));
+ if (scripteq(script, channel->shutdown_scriptpubkey[LOCAL]))
+ continue;
+ if (scripteq(script, channel->shutdown_scriptpubkey[REMOTE]))
+ continue;
+ /* Our own output is always paid to shutdown_scriptpubkey[LOCAL]
+ * (master passes it to closingd verbatim); we never substitute
+ * an OP_RETURN for it. So an OP_RETURN output that matches
+ * neither stored script can only be the peer's closer_scriptpubkey.
+ * It is unspendable, so accepting it cannot redirect funds to a
+ * third party; but we still require a zero value so it burns
+ * nothing. */
+ /* BOLT #2:
+ * 4. if (and only if) `option_simple_close` is negotiated:
+ * * `OP_RETURN` followed by one of:
+ * * `6` to `75` inclusive followed by exactly that many bytes
+ * * `76` followed by `76` to `80` followed by exactly that many bytes
+ */
+ /* BOLT #2:
+ * - If it does, the output value MUST be set to zero so that all funds go to fees, as specified in [BOLT #3](03-transactions.md#closing-transaction).
+ */
+ if (feature_negotiated(channel->peer->ld->our_features,
+ channel->peer->their_features,
+ OPT_SIMPLE_CLOSE)
+ && is_valid_op_return(script, tal_bytelen(script))
+ && amount_sat_eq(bitcoin_tx_output_get_amount_sat(tx, i),
+ AMOUNT_SAT(0)))
+ continue;
+ return tal_fmt(ctx,
+ "output %zu goes to unknown script %s",
+ i, tal_hex(ctx, script));
}
return NULL;
}
diff --git a/lightningd/test/run-close_tx_check.c b/lightningd/test/run-close_tx_check.c
new file mode 100644
index 00000000..9b132647
--- /dev/null
+++ b/lightningd/test/run-close_tx_check.c
@@ -0,0 +1,325 @@
+#include "config.h"
+#include "../simple_close_control.c"
+#include <common/close_tx.h>
+#include <common/setup.h>
+#include <external/libwally-core/include/wally_script.h>
+#include <stdio.h>
+
+/* AUTOGENERATED MOCKS START */
+/* Generated stub for channel_errmsg */
+void channel_errmsg(struct channel *channel UNNEEDED,
+ struct peer_fd *peer_fd UNNEEDED,
+ const char *desc UNNEEDED,
+ const u8 *err_for_them UNNEEDED,
+ bool disconnect UNNEEDED,
+ bool warning UNNEEDED)
+{ fprintf(stderr, "channel_errmsg called!\n"); abort(); }
+/* Generated stub for channel_fail_permanent */
+void channel_fail_permanent(struct channel *channel UNNEEDED,
+ enum state_change reason UNNEEDED,
+ const char *fmt UNNEEDED,
+ ...)
+{ fprintf(stderr, "channel_fail_permanent called!\n"); abort(); }
+/* Generated stub for channel_internal_error */
+void channel_internal_error(struct channel *channel UNNEEDED, const char *fmt UNNEEDED, ...)
+{ fprintf(stderr, "channel_internal_error called!\n"); abort(); }
+/* Generated stub for channel_set_billboard */
+void channel_set_billboard(struct channel *channel UNNEEDED, bool perm UNNEEDED,
+ const char *str TAKES UNNEEDED)
+{ fprintf(stderr, "channel_set_billboard called!\n"); abort(); }
+/* Generated stub for channel_set_last_tx */
+void channel_set_last_tx(struct channel *channel UNNEEDED,
+ struct bitcoin_tx *tx UNNEEDED,
+ const struct bitcoin_signature *sig UNNEEDED)
+{ fprintf(stderr, "channel_set_last_tx called!\n"); abort(); }
+/* Generated stub for channel_set_owner */
+void channel_set_owner(struct channel *channel UNNEEDED, struct subd *owner UNNEEDED)
+{ fprintf(stderr, "channel_set_owner called!\n"); abort(); }
+/* Generated stub for channel_set_state */
+void channel_set_state(struct channel *channel UNNEEDED,
+ enum channel_state old_state UNNEEDED,
+ enum channel_state state UNNEEDED,
+ enum state_change reason UNNEEDED,
+ const char *why UNNEEDED)
+{ fprintf(stderr, "channel_set_state called!\n"); abort(); }
+/* Generated stub for channel_watch_funding_out */
+void channel_watch_funding_out(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED)
+{ fprintf(stderr, "channel_watch_funding_out called!\n"); abort(); }
+/* Generated stub for drop_to_chain */
+void drop_to_chain(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED,
+ bool cooperative UNNEEDED,
+ const struct bitcoin_tx *unilateral_tx UNNEEDED)
+{ fprintf(stderr, "drop_to_chain called!\n"); abort(); }
+/* Generated stub for force_peer_disconnect */
+void force_peer_disconnect(struct lightningd *ld UNNEEDED,
+ const struct peer *peer UNNEEDED,
+ const char *why UNNEEDED)
+{ fprintf(stderr, "force_peer_disconnect called!\n"); abort(); }
+/* Generated stub for fromwire_simpleclosed_closee_broadcast */
+bool fromwire_simpleclosed_closee_broadcast(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct bitcoin_tx **tx UNNEEDED, struct bitcoin_signature *sig UNNEEDED)
+{ fprintf(stderr, "fromwire_simpleclosed_closee_broadcast called!\n"); abort(); }
+/* Generated stub for fromwire_simpleclosed_complete */
+bool fromwire_simpleclosed_complete(const void *p UNNEEDED, bool *delay_broadcast UNNEEDED)
+{ fprintf(stderr, "fromwire_simpleclosed_complete called!\n"); abort(); }
+/* Generated stub for fromwire_simpleclosed_got_sig */
+bool fromwire_simpleclosed_got_sig(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct bitcoin_tx **tx UNNEEDED, struct bitcoin_signature *sig UNNEEDED)
+{ fprintf(stderr, "fromwire_simpleclosed_got_sig called!\n"); abort(); }
+/* Generated stub for get_feerate_floor */
+u32 get_feerate_floor(const struct chain_topology *topo UNNEEDED)
+{ fprintf(stderr, "get_feerate_floor called!\n"); abort(); }
+/* Generated stub for hsm_get_client_fd */
+int hsm_get_client_fd(struct lightningd *ld UNNEEDED,
+ const struct node_id *id UNNEEDED,
+ u64 dbid UNNEEDED,
+ u64 permissions UNNEEDED)
+{ fprintf(stderr, "hsm_get_client_fd called!\n"); abort(); }
+/* Generated stub for log_ */
+void log_(struct logger *logger UNNEEDED, enum log_level level UNNEEDED,
+ const struct node_id *node_id UNNEEDED,
+ bool call_notifier UNNEEDED,
+ const char *fmt UNNEEDED, ...)
+
+{ fprintf(stderr, "log_ called!\n"); abort(); }
+/* Generated stub for mutual_close_feerate */
+u32 mutual_close_feerate(struct chain_topology *topo UNNEEDED)
+{ fprintf(stderr, "mutual_close_feerate called!\n"); abort(); }
+/* Generated stub for new_channel_subd_ */
+struct subd *new_channel_subd_(const tal_t *ctx UNNEEDED,
+ struct lightningd *ld UNNEEDED,
+ const char *name UNNEEDED,
+ void *channel UNNEEDED,
+ const struct node_id *node_id UNNEEDED,
+ struct logger *base_log UNNEEDED,
+ bool talks_to_peer UNNEEDED,
+ const char *(*msgname)(int msgtype) UNNEEDED,
+ unsigned int (*msgcb)(struct subd * UNNEEDED, const u8 * UNNEEDED,
+ const int *fds) UNNEEDED,
+ void (*errcb)(void *errcb_channel UNNEEDED,
+ struct peer_fd *peer_fd UNNEEDED,
+ const char *desc UNNEEDED,
+ const u8 *err_for_them UNNEEDED,
+ bool disconnect UNNEEDED,
+ bool warning) UNNEEDED,
+ void (*billboardcb)(void *billboardcb_channel UNNEEDED, bool perm UNNEEDED,
+ const char *happenings) UNNEEDED,
+ ...)
+{ fprintf(stderr, "new_channel_subd_ called!\n"); abort(); }
+/* Generated stub for resolve_close_command */
+void resolve_close_command(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED,
+ bool cooperative UNNEEDED, const struct bitcoin_tx **close_txs UNNEEDED)
+{ fprintf(stderr, "resolve_close_command called!\n"); abort(); }
+/* Generated stub for simpleclosed_wire_name */
+const char *simpleclosed_wire_name(int e UNNEEDED)
+{ fprintf(stderr, "simpleclosed_wire_name called!\n"); abort(); }
+/* Generated stub for subd_send_msg */
+void subd_send_msg(struct subd *sd UNNEEDED, const u8 *msg_out UNNEEDED)
+{ fprintf(stderr, "subd_send_msg called!\n"); abort(); }
+/* Generated stub for towire_simpleclosed_got_sig_reply */
+u8 *towire_simpleclosed_got_sig_reply(const tal_t *ctx UNNEEDED, const struct bitcoin_txid *closing_txid UNNEEDED)
+{ fprintf(stderr, "towire_simpleclosed_got_sig_reply called!\n"); abort(); }
+/* Generated stub for towire_simpleclosed_init */
+u8 *towire_simpleclosed_init(const tal_t *ctx UNNEEDED, const struct chainparams *chainparams UNNEEDED, const struct channel_id *channel_id UNNEEDED, const struct bitcoin_outpoint *funding UNNEEDED, struct amount_sat funding_satoshi UNNEEDED, const struct pubkey *local_fundingkey UNNEEDED, const struct pubkey *remote_fundingkey UNNEEDED, struct amount_sat local_sat UNNEEDED, struct amount_sat remote_sat UNNEEDED, struct amount_sat our_dust_limit UNNEEDED, u32 feerate_perkw UNNEEDED, u32 *local_wallet_index UNNEEDED, const struct ext_key *local_wallet_ext_key UNNEEDED, const u8 *local_scriptpubkey UNNEEDED, const u8 *remote_scriptpubkey UNNEEDED, enum side opener UNNEEDED)
+{ fprintf(stderr, "towire_simpleclosed_init called!\n"); abort(); }
+/* Generated stub for wallet_can_spend */
+bool wallet_can_spend(struct wallet *w UNNEEDED,
+ const u8 *script UNNEEDED,
+ size_t script_len UNNEEDED,
+ u32 *index UNNEEDED,
+ enum addrtype *addrtype UNNEEDED)
+{ fprintf(stderr, "wallet_can_spend called!\n"); abort(); }
+/* Generated stub for wallet_channel_save */
+void wallet_channel_save(struct wallet *w UNNEEDED, struct channel *chan UNNEEDED)
+{ fprintf(stderr, "wallet_channel_save called!\n"); abort(); }
+/* AUTOGENERATED MOCKS END */
+
+/* A valid P2WPKH (OP_0 <20 bytes>). */
+static const u8 p2wpkh_local[] = {
+ 0x00, 0x14, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88,
+ 0x99, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88,
+ 0x99, 0x00 };
+static const u8 p2wpkh_remote[] = {
+ 0x00, 0x14, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, 0xaa, 0xbb,
+ 0xcc, 0xdd, 0xee, 0xff, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+ 0xaa, 0xbb };
+/* Some unrelated script the channel never agreed to. */
+static const u8 p2wpkh_stranger[] = {
+ 0x00, 0x14, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+ 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,
+ 0xbe, 0xef };
+/* Valid OP_RETURN: OP_RETURN <6-byte push> (is_valid_op_return needs >= 6). */
+static const u8 op_return_burn[] = {
+ OP_RETURN, 0x06, 'b', 'u', 'r', 'n', 'e', 'd' };
+
+/* Build a channel that only populates the fields close_tx_check reads. */
+static struct channel *make_channel(const tal_t *ctx,
+ const struct bitcoin_outpoint *funding,
+ const u8 *local_script, size_t local_len,
+ const u8 *remote_script, size_t remote_len,
+ bool simple_close_negotiated)
+{
+ struct channel *channel = talz(ctx, struct channel);
+ struct peer *peer = talz(ctx, struct peer);
+ struct lightningd *ld = talz(ctx, struct lightningd);
+
+ u8 *their = tal_arr(peer, u8, 0);
+ if (simple_close_negotiated) {
+ ld->our_features = feature_set_for_feature(ld,
+ OPTIONAL_FEATURE(OPT_SIMPLE_CLOSE));
+ set_feature_bit(&their, OPTIONAL_FEATURE(OPT_SIMPLE_CLOSE));
+ } else {
+ /* A feature set that lacks option_simple_close. */
+ ld->our_features = feature_set_for_feature(ld,
+ OPTIONAL_FEATURE(OPT_DATA_LOSS_PROTECT));
+ }
+ peer->their_features = their;
+
+ peer->ld = ld;
+ channel->peer = peer;
+ channel->funding = *funding;
+ channel->shutdown_scriptpubkey[LOCAL]
+ = tal_dup_arr(channel, u8, local_script, local_len, 0);
+ channel->shutdown_scriptpubkey[REMOTE]
+ = tal_dup_arr(channel, u8, remote_script, remote_len, 0);
+ return channel;
+}
+
+static struct bitcoin_tx *close_tx_with_scripts(const tal_t *ctx,
+ const struct bitcoin_outpoint *funding,
+ const u8 *closer_script,
+ const u8 *closee_script)
+{
+ struct pubkey pk1, pk2;
+ const u8 *funding_wscript;
+ struct bitcoin_tx *tx;
+
+ assert(pubkey_from_hexstr("034fede2c619f647fe7c01d40ae22e4c285291ca2ffb47937bbfb7d6e8285a081f",
+ 2 * PUBKEY_CMPR_LEN, &pk1));
+ assert(pubkey_from_hexstr("028dfe31019dd61fa04c76ad065410e5d063ac2949c04c14b214c1b363e517452f",
+ 2 * PUBKEY_CMPR_LEN, &pk2));
+ funding_wscript = bitcoin_redeem_2of2(ctx, &pk1, &pk2);
+
+ tx = create_simple_close_tx(ctx, NULL, NULL, closer_script, closee_script,
+ funding_wscript, funding, AMOUNT_SAT(1000000),
+ AMOUNT_SAT(600000), AMOUNT_SAT(400000), 0);
+ assert(tx != NULL);
+ tx->chainparams = chainparams;
+ return tx;
+}
+
+/* The reported bug: peer (closer) burns its output to a spec-compliant
+ * OP_RETURN. The output matches neither stored shutdown script, but must be
+ * accepted rather than triggering a force close. */
+static void test_op_return_closer_accepted(void)
+{
+ struct bitcoin_outpoint funding;
+ struct channel *channel;
+ struct bitcoin_tx *tx;
+ const u8 *closer = tal_dup_arr(tmpctx, u8, op_return_burn, sizeof(op_return_burn), 0);
+ const u8 *closee = tal_dup_arr(tmpctx, u8, p2wpkh_local, sizeof(p2wpkh_local), 0);
+
+ memset(&funding, 0, sizeof(funding));
+ channel = make_channel(tmpctx, &funding,
+ p2wpkh_local, sizeof(p2wpkh_local),
+ p2wpkh_remote, sizeof(p2wpkh_remote), true);
+ tx = close_tx_with_scripts(tmpctx, &funding, closer, closee);
+
+ assert(close_tx_check(tmpctx, channel, tx) == NULL);
+}
+
+/* Same tx, but option_simple_close was NOT negotiated: the OP_RETURN is not
+ * permitted and must be rejected. */
+static void test_op_return_rejected_without_feature(void)
+{
+ struct bitcoin_outpoint funding;
+ struct channel *channel;
+ struct bitcoin_tx *tx;
+ const u8 *closer = tal_dup_arr(tmpctx, u8, op_return_burn, sizeof(op_return_burn), 0);
+ const u8 *closee = tal_dup_arr(tmpctx, u8, p2wpkh_local, sizeof(p2wpkh_local), 0);
+
+ memset(&funding, 0, sizeof(funding));
+ channel = make_channel(tmpctx, &funding,
+ p2wpkh_local, sizeof(p2wpkh_local),
+ p2wpkh_remote, sizeof(p2wpkh_remote), false);
+ tx = close_tx_with_scripts(tmpctx, &funding, closer, closee);
+
+ assert(close_tx_check(tmpctx, channel, tx) != NULL);
+}
+
+/* A valid OP_RETURN script but with a non-zero value burns real funds; BOLT #2
+ * requires the value to be zero, so it must be rejected. */
+static void test_op_return_nonzero_value_rejected(void)
+{
+ struct bitcoin_outpoint funding;
+ struct channel *channel;
+ struct bitcoin_tx *tx;
+ const u8 *closer = tal_dup_arr(tmpctx, u8, op_return_burn, sizeof(op_return_burn), 0);
+ const u8 *closee = tal_dup_arr(tmpctx, u8, p2wpkh_local, sizeof(p2wpkh_local), 0);
+
+ memset(&funding, 0, sizeof(funding));
+ channel = make_channel(tmpctx, &funding,
+ p2wpkh_local, sizeof(p2wpkh_local),
+ p2wpkh_remote, sizeof(p2wpkh_remote), true);
+ tx = close_tx_with_scripts(tmpctx, &funding, closer, closee);
+
+ /* create_simple_close_tx forces the OP_RETURN value to zero; override it
+ * to simulate a peer (or compromised subd) burning real funds. */
+ for (size_t i = 0; i < tx->wtx->num_outputs; i++) {
+ if (tx->wtx->outputs[i].script_len > 0
+ && tx->wtx->outputs[i].script[0] == OP_RETURN)
+ tx->wtx->outputs[i].satoshi = 12345;
+ }
+
+ assert(close_tx_check(tmpctx, channel, tx) != NULL);
+}
+
+/* A non-OP_RETURN output that matches neither stored script is still an
+ * exfiltration risk and must remain rejected. */
+static void test_unknown_script_rejected(void)
+{
+ struct bitcoin_outpoint funding;
+ struct channel *channel;
+ struct bitcoin_tx *tx;
+ const u8 *closer = tal_dup_arr(tmpctx, u8, p2wpkh_stranger, sizeof(p2wpkh_stranger), 0);
+ const u8 *closee = tal_dup_arr(tmpctx, u8, p2wpkh_local, sizeof(p2wpkh_local), 0);
+
+ memset(&funding, 0, sizeof(funding));
+ channel = make_channel(tmpctx, &funding,
+ p2wpkh_local, sizeof(p2wpkh_local),
+ p2wpkh_remote, sizeof(p2wpkh_remote), true);
+ tx = close_tx_with_scripts(tmpctx, &funding, closer, closee);
+
+ assert(close_tx_check(tmpctx, channel, tx) != NULL);
+}
+
+/* Ordinary close: both outputs go to the agreed shutdown scripts. */
+static void test_known_scripts_accepted(void)
+{
+ struct bitcoin_outpoint funding;
+ struct channel *channel;
+ struct bitcoin_tx *tx;
+ const u8 *closer = tal_dup_arr(tmpctx, u8, p2wpkh_remote, sizeof(p2wpkh_remote), 0);
+ const u8 *closee = tal_dup_arr(tmpctx, u8, p2wpkh_local, sizeof(p2wpkh_local), 0);
+
+ memset(&funding, 0, sizeof(funding));
+ channel = make_channel(tmpctx, &funding,
+ p2wpkh_local, sizeof(p2wpkh_local),
+ p2wpkh_remote, sizeof(p2wpkh_remote), true);
+ tx = close_tx_with_scripts(tmpctx, &funding, closer, closee);
+
+ assert(close_tx_check(tmpctx, channel, tx) == NULL);
+}
+
+int main(int argc, char *argv[])
+{
+ common_setup(argv[0]);
+ chainparams = chainparams_for_network("bitcoin");
+
+ test_op_return_closer_accepted();
+ test_op_return_rejected_without_feature();
+ test_op_return_nonzero_value_rejected();
+ test_unknown_script_rejected();
+ test_known_scripts_accepted();
+
+ common_shutdown();
+ return 0;
+}
Why this scored 43/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.