derive_keyset: don't rotate key for remote iff option_static_remotekey.

The largest change is inside hsmd: it hands a null per-commitment key
to the wallet to tell it to spend the to_remote output.

It can also now resolve unknown commitments, even if it doesn't have a
possible_remote_per_commitment_point from the peer.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

channeld/full_channel.c

@@ -273,6 +273,7 @@ struct bitcoin_tx **channel_txs(const tal_t *ctx,
 	if (!derive_keyset(per_commitment_point,
 			   &channel->basepoints[side],
 			   &channel->basepoints[!side],
+			   channel->option_static_remotekey,
 			   &keyset))
 		return NULL;
 

common/initial_channel.c

@@ -86,7 +86,8 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx,
 	if (!derive_keyset(per_commitment_point,
 			   &channel->basepoints[side],
 			   &channel->basepoints[!side],
-			   &keyset)){
+			   channel->option_static_remotekey,
+			   &keyset)) {
 		*err_reason = "Cannot derive keyset";
 		return NULL;
 	}

common/keyset.c

@@ -5,18 +5,18 @@
 bool derive_keyset(const struct pubkey *per_commitment_point,
 		   const struct basepoints *self,
 		   const struct basepoints *other,
+		   bool option_static_remotekey,
 		   struct keyset *keyset)
 {
-        /* BOLT #3:
+        /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3:
 	 *
-	 * ### `localpubkey`, `remotepubkey`, `local_htlcpubkey`, `remote_htlcpubkey`, `local_delayedpubkey`, and `remote_delayedpubkey` Derivation
+	 * ### `localpubkey`, `local_htlcpubkey`, `remote_htlcpubkey`, `local_delayedpubkey`, and `remote_delayedpubkey` Derivation
 	 *
 	 * These pubkeys are simply generated by addition from their base points:
 	 *
 	 *	pubkey = basepoint + SHA256(per_commitment_point || basepoint) * G
 	 *
 	 * The `localpubkey` uses the local node's `payment_basepoint`;
-	 * the `remotepubkey` uses the remote node's `payment_basepoint`;
 	 * the `local_htlcpubkey` uses the local node's `htlc_basepoint`;
 	 * the `remote_htlcpubkey` uses the remote node's `htlc_basepoint`;
 	 * the `local_delayedpubkey` uses the local node's `delayed_payment_basepoint`;
@@ -27,9 +27,19 @@ bool derive_keyset(const struct pubkey *per_commitment_point,
 			       &keyset->self_payment_key))
 		return false;
 
-	if (!derive_simple_key(&other->payment,
+	/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3:
-			       per_commitment_point,
+	 *
-			       &keyset->other_payment_key))
+	 * ### `remotepubkey` Derivation
+	 *
+	 * If `option_static_remotekey` is negotiated the `remotepubkey`
+	 * is simply the remote node's `payment_basepoint`, otherwise it is
+	 * calculated as above using the remote node's `payment_basepoint`.
+	 */
+	if (option_static_remotekey)
+		keyset->other_payment_key = other->payment;
+	else if (!derive_simple_key(&other->payment,
+				    per_commitment_point,
+				    &keyset->other_payment_key))
 		return false;
 
 	if (!derive_simple_key(&self->htlc,

common/keyset.h

@@ -18,5 +18,6 @@ struct keyset {
 bool derive_keyset(const struct pubkey *per_commitment_point,
 		   const struct basepoints *self,
 		   const struct basepoints *other,
+		   bool option_static_remotekey,
 		   struct keyset *keyset);
 #endif /* LIGHTNING_COMMON_KEYSET_H */

hsmd/hsmd.c

@@ -1385,9 +1385,20 @@ static void hsm_unilateral_close_privkey(struct privkey *dst,
 	get_channel_seed(&info->peer_id, info->channel_id, &channel_seed);
 	derive_basepoints(&channel_seed, NULL, &basepoints, &secrets, NULL);
 
-	if (!derive_simple_privkey(&secrets.payment_basepoint_secret,
+	/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3:
-				   &basepoints.payment, info->commitment_point,
+	 *
-				   dst)) {
+	 * If `option_static_remotekey` is negotiated the `remotepubkey`
+	 * is simply the remote node's `payment_basepoint`, otherwise it is
+	 * calculated as above using the remote node's `payment_basepoint`.
+	 */
+	/* In our UTXO representation, this is indicated by a NULL
+	 * commitment_point. */
+	if (!info->commitment_point)
+		dst->secret = secrets.payment_basepoint_secret;
+	else if (!derive_simple_privkey(&secrets.payment_basepoint_secret,
+					&basepoints.payment,
+					info->commitment_point,
+					dst)) {
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "Deriving unilateral_close_privkey");
 	}

onchaind/onchaind.c

@@ -1726,6 +1726,7 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx,
 	if (!derive_keyset(&local_per_commitment_point,
 			   &basepoints[LOCAL],
 			   &basepoints[REMOTE],
+			   option_static_remotekey,
 			   ks))
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "Deriving keyset for %"PRIu64, commit_num);
@@ -1969,6 +1970,31 @@ static void steal_htlc(struct tracked_output *out)
 	propose_resolution(out, tx, 0, tx_type);
 }
 
+/* Tell wallet that we have discovered a UTXO from a to-remote output,
+ * which it can spend with a little additional info we give here. */
+static void tell_wallet_to_remote(const struct bitcoin_tx *tx,
+				  unsigned int outnum,
+				  const struct bitcoin_txid *txid,
+				  u32 tx_blockheight,
+				  const u8 *scriptpubkey,
+				  const struct pubkey *per_commit_point,
+				  bool option_static_remotekey)
+{
+	struct amount_sat amt = bitcoin_tx_output_get_amount(tx, outnum);
+
+	/* A NULL per_commit_point is how we indicate the pubkey doesn't need
+	 * changing. */
+	if (option_static_remotekey)
+		per_commit_point = NULL;
+
+	wire_sync_write(REQ_FD,
+			take(towire_onchain_add_utxo(NULL, txid, outnum,
+						     per_commit_point,
+						     amt,
+						     tx_blockheight,
+						     scriptpubkey)));
+}
+
 /* BOLT #5:
  *
  * If any node tries to cheat by broadcasting an outdated commitment
@@ -2045,6 +2071,7 @@ static void handle_their_cheat(const struct bitcoin_tx *tx,
 	if (!derive_keyset(remote_per_commitment_point,
 			   &basepoints[REMOTE],
 			   &basepoints[LOCAL],
+			   option_static_remotekey,
 			   ks))
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "Deriving keyset for %"PRIu64, commit_num);
@@ -2056,7 +2083,8 @@ static void handle_their_cheat(const struct bitcoin_tx *tx,
 		     " self_payment_key: %s"
 		     " other_payment_key: %s"
 		     " self_htlc_key: %s"
-		     " other_htlc_key: %s",
+		     " other_htlc_key: %s"
+		     " (option_static_remotekey = %i)",
 		     commit_num,
 		     type_to_string(tmpctx, struct pubkey,
 				    &keyset->self_revocation_key),
@@ -2069,7 +2097,8 @@ static void handle_their_cheat(const struct bitcoin_tx *tx,
 		     type_to_string(tmpctx, struct pubkey,
 				    &keyset->self_htlc_key),
 		     type_to_string(tmpctx, struct pubkey,
-				    &keyset->other_htlc_key));
+				    &keyset->other_htlc_key),
+		     option_static_remotekey);
 
 	remote_wscript = to_self_wscript(tmpctx, to_self_delay[REMOTE], keyset);
 
@@ -2119,14 +2148,11 @@ static void handle_their_cheat(const struct bitcoin_tx *tx,
 						 OUTPUT_TO_US, NULL, NULL, NULL);
 			ignore_output(out);
 
-			/* Tell the master that it will want to add
+			tell_wallet_to_remote(tx, i, txid,
-			 * this UTXO to its outputs */
+					      tx_blockheight,
-			wire_sync_write(REQ_FD, towire_onchain_add_utxo(
+					      script[LOCAL],
-						    tmpctx, txid, i,
+					      remote_per_commitment_point,
-						    remote_per_commitment_point,
+					      option_static_remotekey);
-						    amt,
-						    tx_blockheight,
-						    script[LOCAL]));
 			script[LOCAL] = NULL;
 			continue;
 		}
@@ -2266,6 +2292,7 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx,
 	if (!derive_keyset(remote_per_commitment_point,
 			   &basepoints[REMOTE],
 			   &basepoints[LOCAL],
+			   option_static_remotekey,
 			   ks))
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "Deriving keyset for %"PRIu64, commit_num);
@@ -2339,14 +2366,11 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx,
 						 OUTPUT_TO_US, NULL, NULL, NULL);
 			ignore_output(out);
 
-			/* Tell the master that it will want to add
+			tell_wallet_to_remote(tx, i, txid,
-			 * this UTXO to its outputs */
+					      tx_blockheight,
-			wire_sync_write(REQ_FD, towire_onchain_add_utxo(
+					      script[LOCAL],
-						    tmpctx, txid, i,
+					      remote_per_commitment_point,
-						    remote_per_commitment_point,
+					      option_static_remotekey);
-						    amt,
-						    tx_blockheight,
-						    script[LOCAL]));
 			script[LOCAL] = NULL;
 			continue;
 		}
@@ -2433,7 +2457,6 @@ static void handle_unknown_commitment(const struct bitcoin_tx *tx,
 				      const bool *tell_if_missing,
 				      struct tracked_output **outs)
 {
-	struct keyset *ks;
 	int to_us_output = -1;
 	u8 *local_script;
 
@@ -2441,20 +2464,40 @@ static void handle_unknown_commitment(const struct bitcoin_tx *tx,
 
 	resolved_by_other(outs[0], txid, UNKNOWN_UNILATERAL);
 
-	if (!possible_remote_per_commitment_point)
+	/* 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;
 
-	keyset = ks = tal(tx, struct keyset);
+	if (!option_static_remotekey) {
-	if (!derive_keyset(possible_remote_per_commitment_point,
+		struct keyset *ks = tal(tmpctx, struct keyset);
-			   &basepoints[REMOTE],
+		if (!derive_keyset(possible_remote_per_commitment_point,
-			   &basepoints[LOCAL],
+				   &basepoints[REMOTE],
-			   ks))
+				   &basepoints[LOCAL],
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
+				   option_static_remotekey,
-			      "Deriving keyset for possible_remote_per_commitment_point %s",
+				   ks))
-			      type_to_string(tmpctx, struct pubkey,
+			status_failed(STATUS_FAIL_INTERNAL_ERROR,
-					     possible_remote_per_commitment_point));
+				      "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);
+	} else {
+		/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3:
+		 *
+		 * ### `remotepubkey` Derivation
+		 *
+		 * If `option_static_remotekey` is negotiated the
+		 * `remotepubkey` is simply the remote node's
+		 * `payment_basepoint`, otherwise it is calculated as above
+		 * using the remote node's `payment_basepoint`.
+		 */
+		local_script = scriptpubkey_p2wpkh(tmpctx,
+						   &basepoints[LOCAL].payment);
+	}
 
-	local_script = scriptpubkey_p2wpkh(tmpctx, &keyset->other_payment_key);
 	for (size_t i = 0; i < tx->wtx->num_outputs; i++) {
 		struct tracked_output *out;
 		const u8 *oscript = bitcoin_tx_output_get_script(tmpctx, tx, i);
@@ -2477,14 +2520,11 @@ static void handle_unknown_commitment(const struct bitcoin_tx *tx,
 						 OUTPUT_TO_US, NULL, NULL, NULL);
 			ignore_output(out);
 
-			/* Tell the master that it will want to add
+			tell_wallet_to_remote(tx, i, txid,
-			 * this UTXO to its outputs */
+					      tx_blockheight,
-			wire_sync_write(REQ_FD, towire_onchain_add_utxo(
+					      local_script,
-						    tmpctx, txid, i,
+					      possible_remote_per_commitment_point,
-						    possible_remote_per_commitment_point,
+					      option_static_remotekey);
-						    amt,
-						    tx_blockheight,
-						    local_script));
 			local_script = NULL;
 			to_us_output = i;
 		}

onchaind/test/run-grind_feerate-bug.c

@@ -21,6 +21,7 @@ void daemon_shutdown(void)
 bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED,
 		   const struct basepoints *self UNNEEDED,
 		   const struct basepoints *other UNNEEDED,
+		   bool option_static_remotekey UNNEEDED,
 		   struct keyset *keyset UNNEEDED)
 { fprintf(stderr, "derive_keyset called!\n"); abort(); }
 /* Generated stub for dump_memleak */

onchaind/test/run-grind_feerate.c

@@ -22,6 +22,7 @@ void daemon_shutdown(void)
 bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED,
 		   const struct basepoints *self UNNEEDED,
 		   const struct basepoints *other UNNEEDED,
+		   bool option_static_remotekey UNNEEDED,
 		   struct keyset *keyset UNNEEDED)
 { fprintf(stderr, "derive_keyset called!\n"); abort(); }
 /* Generated stub for dump_memleak */