ElementsProject · cdecker · Jul 2, 2024 · Jul 2, 2024 · Jul 10, 2024 · Jul 23, 2024
diff --git a/lightningd/channel.c b/lightningd/channel.c
@@ -902,22 +902,35 @@ void channel_fail_permanent(struct channel *channel,
 	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",
-		    channel_state_name(channel), why);
+	log_unusual(channel->log,
+		    "Peer permanent failure in %s: %s (reason=%s)",
+		    channel_state_name(channel), why,
+		    channel_change_state_reason_str(reason));
 
 	/* We can have multiple errors, eg. onchaind failures. */
 	if (!channel->error)
 		channel->error = towire_errorfmt(channel,
 						 &channel->cid, "%s", why);
 
 	channel_set_owner(channel, NULL);
-	/* Drop non-cooperatively (unilateral) to chain. */
-	drop_to_chain(ld, channel, false);
+
+	/* 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);
+	drop_to_chain(ld, channel, false, rebroadcast);
 
 	if (channel_state_wants_onchain_fail(channel->state))
 		channel_set_state(channel,

diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c
@@ -303,7 +303,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);
+	drop_to_chain(channel->peer->ld, channel, true, true /* rebroadcast */);
 	channel_set_state(channel,
 			  CLOSINGD_SIGEXCHANGE,
 			  CLOSINGD_COMPLETE,

diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c
@@ -1504,7 +1504,6 @@ enum watch_result onchaind_funding_spent(struct channel *channel,
 	u8 *msg;
 	struct bitcoin_txid our_last_txid;
 	struct lightningd *ld = channel->peer->ld;
-	struct pubkey final_key;
 	int hsmfd;
 	enum state_change reason;
 
@@ -1564,8 +1563,6 @@ enum watch_result onchaind_funding_spent(struct channel *channel,
 		return KEEP_WATCHING;
 	}
 
-	bip32_pubkey(ld, &final_key, channel->final_key_idx);
-
 	struct ext_key final_wallet_ext_key;
 	if (bip32_key_from_parent(
 		    ld->bip32_base,
@@ -1611,8 +1608,6 @@ enum watch_result onchaind_funding_spent(struct channel *channel,
 				  channel->shutdown_scriptpubkey[LOCAL],
 				  channel->shutdown_scriptpubkey[REMOTE],
 				  channel->final_key_idx,
-				  &final_wallet_ext_key,
-				  &final_key,
 				  channel->opener,
 				  &channel->local_basepoints,
 				  &channel->channel_info.theirbase,

diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c
@@ -348,7 +348,7 @@ static enum watch_result closed_inflight_depth_cb(struct lightningd *ld,
 }
 
 void drop_to_chain(struct lightningd *ld, struct channel *channel,
-		   bool cooperative)
+		   bool cooperative, bool rebroadcast)
 {
 	struct channel_inflight *inflight;
 	const char *cmd_id;
@@ -387,6 +387,10 @@ 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) {
+		log_unusual(channel->log,
+			    "Not dropping our unilateral close onchain since "
+			    "we already saw theirs confirm.");
 	} else {
 		struct bitcoin_tx *tx COMPILER_WANTS_INIT("gcc 12.3.0");
 
@@ -448,10 +452,10 @@ void resend_closing_transactions(struct lightningd *ld)
 			case CLOSED:
 				continue;
 			case CLOSINGD_COMPLETE:
-				drop_to_chain(ld, channel, true);
+				drop_to_chain(ld, channel, true, true);
 				continue;
 			case AWAITING_UNILATERAL:
-				drop_to_chain(ld, channel, false);
+				drop_to_chain(ld, channel, false, true);
 				continue;
 			}
 			abort();

diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h
@@ -109,7 +109,16 @@ 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);
 
-void drop_to_chain(struct lightningd *ld, struct channel *channel, bool cooperative);
+/**
+ * Initiate the close of a channel.
+ *
+ * @param passive: Just start watching the close, don't try to publish
+ *        the commitment transaction.
+ */
+void drop_to_chain(struct lightningd *ld,
+		   struct channel *channel,
+		   bool cooperative,
+		   bool rebroadcast);
 
 void update_channel_from_inflight(struct lightningd *ld,
 				  struct channel *channel,

diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c
@@ -48,8 +48,6 @@ static u32 to_self_delay[NUM_SIDES];
 
 /* Where we send money to (our wallet) */
 static u32 our_wallet_index;
-static struct ext_key our_wallet_ext_key;
-static struct pubkey our_wallet_pubkey;
 
 /* Their revocation secret (only if they cheated). */
 static const struct secret *remote_per_commitment_secret;
@@ -3432,8 +3430,6 @@ int main(int argc, char *argv[])
 				   &scriptpubkey[LOCAL],
 				   &scriptpubkey[REMOTE],
 				   &our_wallet_index,
-				   &our_wallet_ext_key,
-				   &our_wallet_pubkey,
 				   &opener,
 				   &basepoints[LOCAL],
 				   &basepoints[REMOTE],

diff --git a/onchaind/onchaind_wire.csv b/onchaind/onchaind_wire.csv
@@ -28,8 +28,6 @@ msgdata,onchaind_init,local_scriptpubkey,u8,local_scriptpubkey_len
 msgdata,onchaind_init,remote_scriptpubkey_len,u16,
 msgdata,onchaind_init,remote_scriptpubkey,u8,remote_scriptpubkey_len
 msgdata,onchaind_init,ourwallet_index,u32,
-msgdata,onchaind_init,ourwallet_ext_key,ext_key,
-msgdata,onchaind_init,ourwallet_pubkey,pubkey,
 # We need these two for commit number obscurer
 msgdata,onchaind_init,opener,enum side,
 msgdata,onchaind_init,local_basepoints,basepoints,

diff --git a/onchaind/test/run-grind_feerate-bug.c b/onchaind/test/run-grind_feerate-bug.c
@@ -42,9 +42,6 @@ void fromwire_basepoints(const u8 **ptr UNNEEDED, size_t *max UNNEEDED,
 /* Generated stub for fromwire_chain_coin_mvt */
 void fromwire_chain_coin_mvt(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct chain_coin_mvt *mvt UNNEEDED)
 { fprintf(stderr, "fromwire_chain_coin_mvt called!\n"); abort(); }
-/* Generated stub for fromwire_ext_key */
-void fromwire_ext_key(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct ext_key *bip32 UNNEEDED)
-{ fprintf(stderr, "fromwire_ext_key called!\n"); abort(); }
 /* Generated stub for fromwire_hsmd_get_per_commitment_point_reply */
 bool fromwire_hsmd_get_per_commitment_point_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey *per_commitment_point UNNEEDED, struct secret **old_commitment_secret UNNEEDED)
 { fprintf(stderr, "fromwire_hsmd_get_per_commitment_point_reply called!\n"); abort(); }
@@ -225,9 +222,6 @@ void towire_basepoints(u8 **pptr UNNEEDED, const struct basepoints *b UNNEEDED)
 /* Generated stub for towire_chain_coin_mvt */
 void towire_chain_coin_mvt(u8 **pptr UNNEEDED, const struct chain_coin_mvt *mvt UNNEEDED)
 { fprintf(stderr, "towire_chain_coin_mvt called!\n"); abort(); }
-/* Generated stub for towire_ext_key */
-void towire_ext_key(u8 **pptr UNNEEDED, const struct ext_key *bip32 UNNEEDED)
-{ fprintf(stderr, "towire_ext_key called!\n"); abort(); }
 /* Generated stub for towire_hsmd_get_per_commitment_point */
 u8 *towire_hsmd_get_per_commitment_point(const tal_t *ctx UNNEEDED, u64 n UNNEEDED)
 { fprintf(stderr, "towire_hsmd_get_per_commitment_point called!\n"); abort(); }

diff --git a/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c
@@ -56,7 +56,7 @@ bool fromwire_onchaind_dev_memleak(const void *p UNNEEDED)
 bool fromwire_onchaind_htlcs(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct htlc_stub **htlc UNNEEDED, bool **tell_if_missing UNNEEDED, bool **tell_immediately UNNEEDED)
 { fprintf(stderr, "fromwire_onchaind_htlcs called!\n"); abort(); }
 /* Generated stub for fromwire_onchaind_init */
-bool fromwire_onchaind_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct amount_msat *our_msat UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, u32 *ourwallet_index UNNEEDED, struct ext_key *ourwallet_ext_key UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *opener UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct tx_parts **tx_parts UNNEEDED, u32 *locktime UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, struct bitcoin_signature **htlc_signature UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey *local_funding_pubkey UNNEEDED, struct pubkey *remote_funding_pubkey UNNEEDED, u64 *local_static_remotekey_start UNNEEDED, u64 *remote_static_remotekey_start UNNEEDED, bool *option_anchor_outputs UNNEEDED, bool *option_anchors_zero_fee_htlc_tx UNNEEDED, u32 *min_relay_feerate UNNEEDED)
+bool fromwire_onchaind_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct amount_msat *our_msat UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, u32 *ourwallet_index UNNEEDED, enum side *opener UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct tx_parts **tx_parts UNNEEDED, u32 *locktime UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, struct bitcoin_signature **htlc_signature UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey *local_funding_pubkey UNNEEDED, struct pubkey *remote_funding_pubkey UNNEEDED, u64 *local_static_remotekey_start UNNEEDED, u64 *remote_static_remotekey_start UNNEEDED, bool *option_anchor_outputs UNNEEDED, bool *option_anchors_zero_fee_htlc_tx UNNEEDED, u32 *min_relay_feerate UNNEEDED)
 { fprintf(stderr, "fromwire_onchaind_init called!\n"); abort(); }
 /* Generated stub for fromwire_onchaind_known_preimage */
 bool fromwire_onchaind_known_preimage(const void *p UNNEEDED, struct preimage *preimage UNNEEDED)

diff --git a/tests/test_plugin.py b/tests/test_plugin.py
@@ -2059,11 +2059,8 @@ def mock_sendrawtransaction(tx):
     # Restart l2, and it should continue where the watchtower left off:
     l2.start()
 
-    # l2 will still try to broadcast its latest commitment tx, but it'll fail
-    # since l1 has cheated. All commitments share the same prefix, so look for
-    # that.
-    penalty_prefix = tx[:(4 + 1 + 36) * 2]  # version, txin_count, first txin in hex
-    l2.daemon.wait_for_log(r'Expected error broadcasting tx {}'.format(penalty_prefix))
+    # l2 notices that there has been a unilateral close from l1.
+    l2.daemon.wait_for_log(r'Peer permanent failure in')
 
     # Now make sure the penalty output ends up in our wallet
     fund_txids = [o['txid'] for o in l2.rpc.listfunds()['outputs']]