From 6d24c83c958dfdd8800955a5694d0ac44aedd73d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 25 Nov 2024 14:38:18 +1030 Subject: [PATCH] lightningd: wire channel closing tx through channel_fail_permanent. Cleans up the API: we have two functions now, one which is explicitly for "I'm failing this because I saw this tx onchain". Now we can correctly report the tx which closed the channel (previously we would always report our own tx(s)!). Signed-off-by: Rusty Russell Changelog-Fixed: JSON-RPC: `close` now correctly reports the txid of the remote onchain unilateral tx if it races with a peer close. Changelog-Fixed: Protocol: we no longer try to spend anchors if a commitment tx is already mined (reported by @niftynei). Fixes: #7526 --- lightningd/channel.c | 65 ++++++++++++++------- lightningd/channel.h | 8 +++ lightningd/closing_control.c | 2 +- lightningd/onchain_control.c | 5 +- lightningd/peer_control.c | 28 ++++++--- lightningd/peer_control.h | 14 ++--- lightningd/test/run-invoice-select-inchan.c | 7 +++ 7 files changed, 86 insertions(+), 43 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index cd26e229eaff..aacd29d72378 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -930,28 +930,18 @@ const char *channel_change_state_reason_str(enum state_change reason) abort(); } -void channel_fail_permanent(struct channel *channel, - enum state_change reason, - const char *fmt, - ...) +static void channel_fail_perm(struct channel *channel, + enum state_change reason, + const char *why, + const struct bitcoin_tx *spent_by) { + struct lightningd *ld = channel->peer->ld; + /* Don't do anything if it's an stub channel because * peer has already closed it unilatelrally. */ if (channel->scid && is_stub_scid(*channel->scid)) return; - struct lightningd *ld = channel->peer->ld; - va_list ap; - char *why; - /* Do we want to rebroadcast close transactions? If we're - * witnessing the close on-chain there is no point in doing - * this. */ - bool rebroadcast; - - va_start(ap, fmt); - why = tal_vfmt(tmpctx, fmt, ap); - va_end(ap); - log_unusual(channel->log, "Peer permanent failure in %s: %s (reason=%s)", channel_state_name(channel), why, @@ -965,13 +955,10 @@ void channel_fail_permanent(struct channel *channel, channel_set_owner(channel, NULL); /* Drop non-cooperatively (unilateral) to chain. If we detect - * the close from the blockchain (i.e., reason is - * REASON_ONCHAIN, or FUNDING_SPEND_SEEN) then we can observe + * the close from the blockchain, then we can observe * passively, and not broadcast our own unilateral close, as * it doesn't stand a chance anyway. */ - rebroadcast = !(channel->state == ONCHAIN || - channel->state == FUNDING_SPEND_SEEN); - drop_to_chain(ld, channel, false, rebroadcast); + drop_to_chain(ld, channel, false, spent_by); if (channel_state_wants_onchain_fail(channel->state)) channel_set_state(channel, @@ -982,8 +969,42 @@ void channel_fail_permanent(struct channel *channel, if (channel_state_open_uncommitted(channel->state)) delete_channel(channel); +} - tal_free(why); +void channel_fail_permanent(struct channel *channel, + enum state_change reason, + const char *fmt, + ...) +{ + va_list ap; + char *why; + + va_start(ap, fmt); + why = tal_vfmt(tmpctx, fmt, ap); + va_end(ap); + + channel_fail_perm(channel, reason, why, NULL); +} + +void channel_fail_saw_onchain(struct channel *channel, + enum state_change reason, + const struct bitcoin_tx *tx, + const char *fmt, + ...) +{ + va_list ap; + char *why; + struct bitcoin_txid txid; + + va_start(ap, fmt); + why = tal_vfmt(tmpctx, fmt, ap); + va_end(ap); + + bitcoin_txid(tx, &txid); + tal_append_fmt(&why, ": onchain txid %s", + fmt_bitcoin_txid(tmpctx, &txid)); + + channel_fail_perm(channel, reason, why, tx); } void channel_fail_forget(struct channel *channel, const char *fmt, ...) diff --git a/lightningd/channel.h b/lightningd/channel.h index ca2dbfa9b816..475b3933bfc6 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -752,6 +752,14 @@ void channel_fail_permanent(struct channel *channel, enum state_change reason, const char *fmt, ...); + +/* Channel has failed, give up on it, specifically because we saw this tx spend it. */ +void channel_fail_saw_onchain(struct channel *channel, + enum state_change reason, + const struct bitcoin_tx *tx, + const char *fmt, + ...); + /* Forget the channel. This is only used for the case when we "receive" error * during CHANNELD_AWAITING_LOCKIN if we are "fundee". */ void channel_fail_forget(struct channel *channel, const char *fmt, ...); diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index b6623334a668..6b260029cdc6 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -325,7 +325,7 @@ static void peer_closing_complete(struct channel *channel, const u8 *msg) return; /* Channel gets dropped to chain cooperatively. */ - drop_to_chain(channel->peer->ld, channel, true, true /* rebroadcast */); + drop_to_chain(channel->peer->ld, channel, true, NULL); channel_set_state(channel, CLOSINGD_SIGEXCHANGE, CLOSINGD_COMPLETE, diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index f89174429360..e96a96229612 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -1708,8 +1708,9 @@ enum watch_result onchaind_funding_spent(struct channel *channel, if (channel->closer != NUM_SIDES) reason = REASON_UNKNOWN; /* will use last cause as reason */ - channel_fail_permanent(channel, reason, - "Funding transaction spent"); + channel_fail_saw_onchain(channel, reason, + tx, + "Funding transaction spent"); /* If we haven't posted the open event yet, post an open */ if (!channel->scid || !channel->remote_channel_ready) { diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 16326eaab538..0b6db63e764e 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -340,15 +340,16 @@ static enum watch_result closed_inflight_depth_cb(struct lightningd *ld, /* This is now the main tx. */ update_channel_from_inflight(ld, inflight->channel, inflight); - channel_fail_permanent(inflight->channel, - REASON_UNKNOWN, - "Inflight tx %s confirmed after mutual close", - fmt_bitcoin_txid(tmpctx, txid)); + channel_fail_saw_onchain(inflight->channel, + REASON_UNKNOWN, + tx, + "Inflight tx confirmed after mutual close"); return DELETE_WATCH; } void drop_to_chain(struct lightningd *ld, struct channel *channel, - bool cooperative, bool rebroadcast) + bool cooperative, + const struct bitcoin_tx *unilateral_tx) { struct channel_inflight *inflight; const char *cmd_id; @@ -387,10 +388,19 @@ void drop_to_chain(struct lightningd *ld, struct channel *channel, log_broken(channel->log, "Cannot broadcast our commitment tx:" " it's invalid! (ancient channel?)"); - } else if (!rebroadcast && !cooperative) { + } else if (unilateral_tx) { + struct bitcoin_txid txid; + const struct bitcoin_tx **txs = tal_arr(tmpctx, const struct bitcoin_tx *, 1); + bitcoin_txid(unilateral_tx, &txid); + log_unusual(channel->log, "Not dropping our unilateral close onchain since " - "we already saw theirs confirm."); + "we already saw %s confirm.", + fmt_bitcoin_txid(tmpctx, &txid)); + + /* If we were waiting for a close, this is it (expects array of txs!) */ + txs[0] = unilateral_tx; + resolve_close_command(ld, channel, false, txs); } else { const struct bitcoin_tx **txs = tal_arr(tmpctx, const struct bitcoin_tx*, 0); @@ -457,10 +467,10 @@ void resend_closing_transactions(struct lightningd *ld) case CLOSED: continue; case CLOSINGD_COMPLETE: - drop_to_chain(ld, channel, true, true); + drop_to_chain(ld, channel, true, NULL); continue; case AWAITING_UNILATERAL: - drop_to_chain(ld, channel, false, true); + drop_to_chain(ld, channel, false, NULL); continue; } abort(); diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index face71198575..ddb2e228d98d 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -109,16 +109,12 @@ void peer_set_dbid(struct peer *peer, u64 dbid); /* At startup, re-send any transactions we want bitcoind to have */ void resend_closing_transactions(struct lightningd *ld); -/** - * Initiate the close of a channel. - * - * @param rebroadcast: Whether we should be broadcasting our - * commitment transaction in order to close the channel, or not. - */ -void drop_to_chain(struct lightningd *ld, - struct channel *channel, +/* Initiate the close of a channel, maybe broadcast. If we've seen a + * unilateral close, pass it here (means we don't need to broadcast + * our own, or any anchors). */ +void drop_to_chain(struct lightningd *ld, struct channel *channel, bool cooperative, - bool rebroadcast); + const struct bitcoin_tx *unilateral_tx); void update_channel_from_inflight(struct lightningd *ld, struct channel *channel, diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 144726f1f8f0..f8cae210aa69 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -91,6 +91,13 @@ void channel_fail_permanent(struct channel *channel UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "channel_fail_permanent called!\n"); abort(); } +/* Generated stub for channel_fail_saw_onchain */ +void channel_fail_saw_onchain(struct channel *channel UNNEEDED, + enum state_change reason UNNEEDED, + const struct bitcoin_tx *tx UNNEEDED, + const char *fmt UNNEEDED, + ...) +{ fprintf(stderr, "channel_fail_saw_onchain called!\n"); abort(); } /* Generated stub for channel_fail_transient */ void channel_fail_transient(struct channel *channel UNNEEDED, bool disconnect UNNEEDED,