diff --git a/lightningd/anchorspend.c b/lightningd/anchorspend.c index b54f4b59c3ce..9532b896ac56 100644 --- a/lightningd/anchorspend.c +++ b/lightningd/anchorspend.c @@ -270,7 +270,7 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx, struct amount_msat total_value; const u8 *msg; - /* Estimate weight of spend tx plus commitment_tx (not including any UTXO we add) */ + /* Estimate weight of anchorspend tx plus commitment_tx (not including any UTXO we add) */ base_weight = bitcoin_tx_core_weight(2, 1) + bitcoin_tx_input_weight(false, bitcoin_tx_input_sig_weight() diff --git a/lightningd/channel.c b/lightningd/channel.c index e9fe21c9686e..42008b15f69e 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -908,7 +908,7 @@ void channel_set_state(struct channel *channel, enum channel_state old_state, enum channel_state state, enum state_change reason, - char *why) + const char *why) { bool was_important; @@ -987,28 +987,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, @@ -1021,14 +1011,6 @@ 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 - * 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); - if (channel_state_wants_onchain_fail(channel->state)) channel_set_state(channel, channel->state, @@ -1036,13 +1018,50 @@ void channel_fail_permanent(struct channel *channel, reason, why); - /* Drop non-cooperatively (unilateral) to chain. */ - drop_to_chain(ld, channel, false, rebroadcast); + /* Drop non-cooperatively (unilateral) to chain. If we detect + * 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. */ + drop_to_chain(ld, channel, false, spent_by); 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 fd06570b27a1..d38d54ba1165 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, ...); @@ -765,7 +773,7 @@ void channel_set_state(struct channel *channel, enum channel_state old_state, enum channel_state state, enum state_change reason, - char *why); + const char *why); const char *channel_change_state_reason_str(enum state_change reason); diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 9407732bc415..a10a1f32808c 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -52,7 +52,7 @@ struct close_command { /* Resolve a single close command. */ static void resolve_one_close_command(struct close_command *cc, bool cooperative, - struct bitcoin_tx **close_txs) + const struct bitcoin_tx **close_txs) { assert(tal_count(close_txs)); struct json_stream *result = json_stream_success(cc->cmd); @@ -107,7 +107,7 @@ const char *cmd_id_from_close_command(const tal_t *ctx, /* Resolve a close command for a channel that will be closed soon. */ void resolve_close_command(struct lightningd *ld, struct channel *channel, - bool cooperative, struct bitcoin_tx **close_txs) + bool cooperative, const struct bitcoin_tx **close_txs) { struct close_command *cc; struct close_command *n; @@ -331,7 +331,7 @@ static void peer_closing_complete(struct channel *channel, const u8 *msg) "Closing complete"); /* 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); } static void peer_closing_notify(struct channel *channel, const u8 *msg) diff --git a/lightningd/closing_control.h b/lightningd/closing_control.h index 919d347cafed..ed5f5355d4a5 100644 --- a/lightningd/closing_control.h +++ b/lightningd/closing_control.h @@ -14,7 +14,7 @@ const char *cmd_id_from_close_command(const tal_t *ctx, /* Resolve a close command for a channel that will be closed soon. */ void resolve_close_command(struct lightningd *ld, struct channel *channel, - bool cooperative, struct bitcoin_tx **close_txs); + bool cooperative, const struct bitcoin_tx **close_txs); void peer_start_closingd(struct channel *channel, struct peer_fd *peer_fd); diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 1142e6eb891a..e96a96229612 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -342,7 +342,7 @@ static void handle_onchain_log_coin_move(struct channel *channel, const u8 *msg) return; } - /* Any 'ignored' payments get registed to the wallet */ + /* Any 'ignored' payments get registered to the wallet */ if (!mvt->account_name) mvt->account_name = fmt_channel_id(mvt, &channel->cid); @@ -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 d8528223d00a..37ccba4d2ec8 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -342,15 +342,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; @@ -389,12 +390,21 @@ 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, cooperative, txs); } else { - struct bitcoin_tx **txs = tal_arr(tmpctx, struct bitcoin_tx*, 0); + const struct bitcoin_tx **txs = tal_arr(tmpctx, const struct bitcoin_tx*, 0); /* We need to drop *every* commitment transaction to chain */ if (!cooperative && !list_empty(&channel->inflights)) { @@ -459,10 +469,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 c8015cbada40..77739a3d7c75 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -113,16 +113,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 a76d995d0b62..9d88dc0205db 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, @@ -927,7 +934,7 @@ void report_subd_memleak(struct leak_detect *leak_detect UNNEEDED, struct subd * { fprintf(stderr, "report_subd_memleak called!\n"); abort(); } /* Generated stub for resolve_close_command */ void resolve_close_command(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED, - bool cooperative UNNEEDED, struct bitcoin_tx **close_txs UNNEEDED) + bool cooperative UNNEEDED, const struct bitcoin_tx **close_txs UNNEEDED) { fprintf(stderr, "resolve_close_command called!\n"); abort(); } /* Generated stub for send_backtrace */ void send_backtrace(const char *why UNNEEDED) diff --git a/tests/test_closing.py b/tests/test_closing.py index d37edfbd5baf..cde5695a6fc5 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -2399,35 +2399,45 @@ def try_pay(): assert account_balance(l2, channel_id) == 0 assert account_balance(l1, channel_id) == 0 - # Graph of coin_move events we expect - expected_1 = { - '0': [('wallet', ['deposit'], ['withdrawal'], 'A')], - # This is ugly, but this wallet deposit is either unspent or used - # in the next channel open - 'A': [('wallet', ['deposit'], None, None), ('cid1', ['channel_open', 'opener'], ['channel_close'], 'B')], - 'B': [('wallet', ['deposit'], None, None), ('cid1', ['htlc_timeout'], ['to_wallet'], 'C')], - 'C': [('wallet', ['deposit'], None, None)], - } + # Graph of coin_move events we expect! + if anchors: + expected_1 = { + # Initial wallet deposit + '0': [('wallet', ['deposit'], ['withdrawal'], 'A')], + # Funding tx + 'A': [('wallet', ['deposit'], None, None), ('cid1', ['channel_open', 'opener'], ['channel_close'], 'B')], + # Commitment tx + 'B': [('wallet', ['deposit'], None, None), ('cid1', ['htlc_timeout'], ['to_wallet'], 'C'), ('external', ['anchor'], None, None), ('wallet', ['anchor', 'ignored'], None, None)], + # HTLC timeout tx + 'C': [('wallet', ['deposit'], None, None)], + } - expected_2 = { - 'A': [('cid1', ['channel_open'], ['channel_close'], 'B')], - 'B': [('external', ['to_them'], None, None), ('external', ['htlc_timeout'], None, None)], - } + expected_2 = { + # Funding tx + 'A': [('cid1', ['channel_open'], ['channel_close'], 'B')], + # Commitment tx + 'B': [('external', ['to_them'], None, None), ('external', ['htlc_timeout'], None, None), ('external', ['anchor'], None, None), ('wallet', ['anchor', 'ignored'], None, None)], + } + else: + expected_1 = { + '0': [('wallet', ['deposit'], ['withdrawal'], 'A')], + # This is ugly, but this wallet deposit is either unspent or used + # in the next channel open + 'A': [('wallet', ['deposit'], None, None), ('cid1', ['channel_open', 'opener'], ['channel_close'], 'B')], + 'B': [('wallet', ['deposit'], None, None), ('cid1', ['htlc_timeout'], ['to_wallet'], 'C')], + 'C': [('wallet', ['deposit'], None, None)], + } - if anchors: - expected_1['B'].append(('external', ['anchor'], None, None)) - expected_2['B'].append(('external', ['anchor'], None, None)) - expected_1['B'].append(('wallet', ['anchor', 'ignored'], None, None)) - expected_2['B'].append(('wallet', ['anchor', 'ignored'], None, None)) + expected_2 = { + 'A': [('cid1', ['channel_open'], ['channel_close'], 'B')], + 'B': [('external', ['to_them'], None, None), ('external', ['htlc_timeout'], None, None)], + } - # FIXME: Why does this fail? - if not anchors: - tags = check_utxos_channel(l1, [channel_id], expected_1) - check_utxos_channel(l2, [channel_id], expected_2, tags) + tags = check_utxos_channel(l1, [channel_id], expected_1) + check_utxos_channel(l2, [channel_id], expected_2, tags) # Check 'bkpr-inspect' and 'bkpr-listbalances' - # The wallet events aren't in the channel's events - del expected_1['0'] + del expected_1['0'] # Tx '0' was the initial deposit, its not in channel's events expected_1['A'] = expected_1['A'][1:] check_inspect_channel(l1, channel_id, expected_1) diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index b1780d49db2f..f21860bcde5a 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -944,7 +944,7 @@ void report_subd_memleak(struct leak_detect *leak_detect UNNEEDED, struct subd * { fprintf(stderr, "report_subd_memleak called!\n"); abort(); } /* Generated stub for resolve_close_command */ void resolve_close_command(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED, - bool cooperative UNNEEDED, struct bitcoin_tx **close_txs UNNEEDED) + bool cooperative UNNEEDED, const struct bitcoin_tx **close_txs UNNEEDED) { fprintf(stderr, "resolve_close_command called!\n"); abort(); } /* Generated stub for rune_is_ours */ const char *rune_is_ours(struct lightningd *ld UNNEEDED, const struct rune *rune UNNEEDED)