Fix: don't send anchorspends for onchain commitment txs

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()

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,28 +1011,57 @@ 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,
 				  AWAITING_UNILATERAL,
 				  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, ...)

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);
 

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)

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);

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) {

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();

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,

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)

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)
 

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)