From 7d666e9bfde3e9444639e8b686b2c82dcf1f9443 Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Fri, 4 Jun 2021 11:23:43 +0930
Subject: [PATCH] onchaind: don't rely on knowing option_static_remotekey for
 unknown commitments.

Just always handle both cases.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 onchaind/onchaind.c | 91 ++++++++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 42 deletions(-)

diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c
index 1e1857fa0..76a060ae3 100644
--- a/onchaind/onchaind.c
+++ b/onchaind/onchaind.c
@@ -3594,79 +3594,86 @@ static void handle_unknown_commitment(const struct tx_parts *tx,
 				      bool is_replay)
 {
 	int to_us_output = -1;
-	u8 *local_script;
+	/* We have two possible local scripts, depending on options */
+	u8 *local_scripts[2];
 	struct amount_sat amt_salvaged = AMOUNT_SAT(0);
 
 	onchain_annotate_txin(&tx->txid, 0, TX_CHANNEL_UNILATERAL | TX_THEIRS);
 
 	resolved_by_other(outs[0], &tx->txid, UNKNOWN_UNILATERAL);
 
-	/* If they don't give us a per-commitment point and we rotate keys,
-	 * we're out of luck. */
-	if (!possible_remote_per_commitment_point
-	    && !option_static_remotekey) {
-		goto search_done;
-	}
-
-	if (!option_static_remotekey) {
+	/* This is the not-option_static_remotekey case, if we got a hint
+	 * from them about the per-commitment point */
+	if (possible_remote_per_commitment_point) {
 		struct keyset *ks = tal(tmpctx, struct keyset);
 		if (!derive_keyset(possible_remote_per_commitment_point,
 				   &basepoints[REMOTE],
 				   &basepoints[LOCAL],
-				   option_static_remotekey,
+				   false,
 				   ks))
 			status_failed(STATUS_FAIL_INTERNAL_ERROR,
 				      "Deriving keyset for possible_remote_per_commitment_point %s",
 				      type_to_string(tmpctx, struct pubkey,
 						     possible_remote_per_commitment_point));
 
-		local_script = scriptpubkey_p2wpkh(tmpctx,
-						   &ks->other_payment_key);
+		local_scripts[0] = scriptpubkey_p2wpkh(tmpctx,
+						       &ks->other_payment_key);
 	} else {
-		local_script = scriptpubkey_to_remote(tmpctx,
-						      &basepoints[LOCAL].payment);
+		local_scripts[0] = NULL;
 	}
 
+	/* Other possible local script is for option_static_remotekey */
+	local_scripts[1] = scriptpubkey_to_remote(tmpctx,
+						  &basepoints[LOCAL].payment);
+
 	for (size_t i = 0; i < tal_count(tx->outputs); i++) {
 		struct tracked_output *out;
 		struct amount_asset asset = wally_tx_output_get_amount(tx->outputs[i]);
 		struct amount_sat amt;
+		int which_script;
+
 		assert(amount_asset_is_main(&asset));
 		amt = amount_asset_to_sat(&asset);
 
-		if (local_script
-		    && wally_tx_output_scripteq(tx->outputs[i],
-						local_script)) {
-			/* BOLT #5:
-			 *
-			 * - MAY take no action in regard to the associated
-			 *   `to_remote`, which is simply a P2WPKH output to
-			 *   the *local node*.
-			 *   - Note: `to_remote` is considered *resolved* by the
-			 *     commitment transaction itself.
-			 */
-			out = new_tracked_output(&outs, &tx->txid, tx_blockheight,
-						 UNKNOWN_UNILATERAL,
-						 i, amt,
-						 OUTPUT_TO_US, NULL, NULL, NULL);
-			ignore_output(out);
+		/* Elements can have empty output scripts (fee output) */
+		if (local_scripts[0]
+		    && wally_tx_output_scripteq(tx->outputs[i], local_scripts[0]))
+			which_script = 0;
+		else if (local_scripts[1]
+			 && wally_tx_output_scripteq(tx->outputs[i],
+						     local_scripts[1]))
+			which_script = 1;
+		else
+			continue;
 
-			if (!is_replay)
-				record_channel_withdrawal(&tx->txid, tx_blockheight, out);
+		/* BOLT #5:
+		 *
+		 * - MAY take no action in regard to the associated
+		 *   `to_remote`, which is simply a P2WPKH output to
+		 *   the *local node*.
+		 *   - Note: `to_remote` is considered *resolved* by the
+		 *     commitment transaction itself.
+		 */
+		out = new_tracked_output(&outs, &tx->txid, tx_blockheight,
+					 UNKNOWN_UNILATERAL,
+					 i, amt,
+					 OUTPUT_TO_US, NULL, NULL, NULL);
+		ignore_output(out);
 
-			add_amt(&amt_salvaged, amt);
+		if (!is_replay)
+			record_channel_withdrawal(&tx->txid, tx_blockheight, out);
 
-			tell_wallet_to_remote(tx, i,
-					      tx_blockheight,
-					      local_script,
-					      possible_remote_per_commitment_point,
-					      option_static_remotekey);
-			local_script = NULL;
-			to_us_output = i;
-		}
+		add_amt(&amt_salvaged, amt);
+
+		tell_wallet_to_remote(tx, i,
+				      tx_blockheight,
+				      local_scripts[which_script],
+				      possible_remote_per_commitment_point,
+				      which_script == 1);
+		local_scripts[0] = local_scripts[1] = NULL;
+		to_us_output = i;
 	}
 
-search_done:
 	if (to_us_output == -1) {
 		status_broken("FUNDS LOST.  Unknown commitment #%"PRIu64"!",
 			      commit_num);