Add CLN support for explicit RevokeCommitmentTx

channeld/Makefile

@@ -60,6 +60,7 @@ CHANNELD_COMMON_OBJS :=				\
 	common/status_wiregen.o			\
 	common/gossip_store.o			\
 	common/hmac.o				\
+	common/hsm_capable.o				\
 	common/interactivetx.o			\
 	common/htlc_state.o			\
 	common/htlc_trim.o			\

channeld/channeld.c

@@ -25,6 +25,8 @@
 #include <common/billboard.h>
 #include <common/ecdh_hsmd.h>
 #include <common/gossip_store.h>
+#include <common/hsm_capable.h>
+#include <common/hsm_version.h>
 #include <common/interactivetx.h>
 #include <common/key_derive.h>
 #include <common/memleak.h>
@@ -77,6 +79,9 @@ struct peer {
 	/* Features we support. */
 	struct feature_set *our_features;
 
+	/* What (additional) messages the HSM accepts */
+	u32 *hsm_capabilities;
+
 	/* Tolerable amounts for feerate (only relevant for fundee). */
 	u32 feerate_min, feerate_max;
 
@@ -1920,6 +1925,7 @@ static void send_revocation(struct peer *peer,
 	const struct failed_htlc **failed;
 	struct added_htlc *added;
 	const u8 *msg;
+	const u8 *msg2;
 	const u8 *msg_for_master;
 
 	/* Marshall it now before channel_sending_revoke_and_ack changes htlcs */
@@ -1931,11 +1937,6 @@ static void send_revocation(struct peer *peer,
 			   &failed,
 			   &added);
 
-	/* Revoke previous commit, get new point. */
-	msg = make_revocation_msg_from_secret(peer, peer->next_index[LOCAL]-1,
-					      &peer->next_local_per_commit,
-					      old_secret, next_point);
-
 	/* From now on we apply changes to the next commitment */
 	peer->next_index[LOCAL]++;
 
@@ -1966,6 +1967,29 @@ static void send_revocation(struct peer *peer,
 
 	peer->splice_state->await_commitment_succcess = false;
 
+	/* Now that the master has persisted the new commitment advance the HSMD
+	 * and fetch the revocation secret for the old one. */
+	struct secret old_secret2;
+	struct pubkey next_point2;
+	if (!hsm_is_capable(peer->hsm_capabilities, WIRE_HSMD_REVOKE_COMMITMENT_TX)) {
+		/* Prior to HSM_VERSION 5 we use the old_secret
+		 * received earlier from validate_commitment_tx. */
+		memcpy(&old_secret2, old_secret, sizeof(old_secret2));
+		memcpy(&next_point2, next_point, sizeof(next_point2));
+	} else {
+		msg2 = towire_hsmd_revoke_commitment_tx(tmpctx, peer->next_index[LOCAL] - 2);
+		msg2 = hsm_req(tmpctx, take(msg2));
+		if (!fromwire_hsmd_revoke_commitment_tx_reply(msg2, &old_secret2, &next_point2))
+			status_failed(STATUS_FAIL_HSM_IO,
+				      "Reading revoke_commitment_tx reply: %s",
+				      tal_hex(tmpctx, msg2));
+	}
+
+	/* Revoke previous commit, get new point. */
+	msg = make_revocation_msg_from_secret(peer, peer->next_index[LOCAL]-2,
+					      &peer->next_local_per_commit,
+					      &old_secret2, &next_point2);
+
 	/* Now we can finally send revoke_and_ack to peer */
 	peer_write(peer->pps, take(msg));
 }
@@ -2270,7 +2294,10 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer,
 		tal_steal(commitsigs, result);
 	}
 
-	assert(old_secret);
+	// If the HSM doesn't support WIRE_HSMD_REVOKE_COMMITMENT_TX we'd better
+	// have the old_secret at this point.
+	if (!hsm_is_capable(peer->hsm_capabilities, WIRE_HSMD_REVOKE_COMMITMENT_TX))
+		assert(old_secret);
 
 	send_revocation(peer, &commit_sig, htlc_sigs, changed_htlcs, txs[0],
 			old_secret, &next_point, commitsigs);
@@ -6085,6 +6112,7 @@ static void init_channel(struct peer *peer)
 	if (!fromwire_channeld_init(peer, msg,
 				    &chainparams,
 				    &peer->our_features,
+				    &peer->hsm_capabilities,
 				    &peer->channel_id,
 				    &funding,
 				    &funding_sats,

channeld/channeld_wire.csv

@@ -15,6 +15,8 @@
 msgtype,channeld_init,1000
 msgdata,channeld_init,chainparams,chainparams,
 msgdata,channeld_init,our_features,feature_set,
+msgdata,channeld_init,num_hsm_capabilities,u16,
+msgdata,channeld_init,hsm_capabilities,u32,num_hsm_capabilities
 msgdata,channeld_init,channel_id,channel_id,
 msgdata,channeld_init,funding,bitcoin_outpoint,
 msgdata,channeld_init,funding_satoshi,amount_sat,

common/Makefile

@@ -41,6 +41,7 @@ COMMON_SRC_NOGEN :=				\
 	common/gossmap.c			\
 	common/hash_u5.c			\
 	common/hmac.c				\
+	common/hsm_capable.c			\
 	common/hsm_encryption.c			\
 	common/htlc_state.c			\
 	common/htlc_trim.c			\

common/hsm_capable.c

@@ -0,0 +1,13 @@
+#include "config.h"
+#include <common/hsm_capable.h>
+
+/* Is this capability supported by the HSM? (So far, always a message
+ * number) */
+bool hsm_is_capable(u32 *capabilities, u32 msgtype)
+{
+	for (size_t i = 0; i < tal_count(capabilities); i++) {
+		if (capabilities[i] == msgtype)
+			return true;
+	}
+	return false;
+}

common/hsm_capable.h

@@ -0,0 +1,11 @@
+#ifndef LIGHTNING_COMMON_HSM_CAPABLE_H
+#define LIGHTNING_COMMON_HSM_CAPABLE_H
+#include "config.h"
+#include <ccan/short_types/short_types.h>
+#include <ccan/tal/tal.h>
+#include <stdbool.h>
+
+/* Is this capability supported by the HSM? (So far, always a message
+ * number) */
+bool hsm_is_capable(u32 *capabilities, u32 msgtype);
+#endif /* LIGHTNING_COMMON_HSM_CAPABLE_H */

common/hsm_version.h

@@ -19,7 +19,9 @@
  * v4 with capabilities called permissions: 7c5bf8ec7cf30302740db85260a9d1ac2c5b0323a2376c28df6b611831f91655
  * v4 with renaming of channel_ready to setup_channel: 60b92a0930b631cc77df564cb9235e6cb220f4337a2bb00e5153145e0bf8c80e
  * v4 with buried outpoint check: f44fae666895cab0347b3de7c245267c71cc7de834827b83e286e86318c08aec
+ * v4 with forget_channel: d87c6934ea188f92785d38d7cd0b13ed7f76aa7417f3200baf0c7b5aa832fe29
+ * v5 with hsmd_revoke_commitment_tx: 5742538f87ef5d5bf55b66dc19e52c8683cfeb1b887d3e64ba530ba9a4d8e638
  */
 #define HSM_MIN_VERSION 3
-#define HSM_MAX_VERSION 4
+#define HSM_MAX_VERSION 5
 #endif /* LIGHTNING_COMMON_HSM_VERSION_H */

hsmd/hsmd.c

@@ -650,6 +650,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
  	case WIRE_HSMD_FORGET_CHANNEL:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX:
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX:
 	case WIRE_HSMD_VALIDATE_REVOCATION:
 	case WIRE_HSMD_SIGN_PENALTY_TO_US:
 	case WIRE_HSMD_SIGN_REMOTE_COMMITMENT_TX:
@@ -705,6 +706,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
 	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY:
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_REVOCATION_REPLY:
 	case WIRE_HSMD_SIGN_TX_REPLY:
 	case WIRE_HSMD_SIGN_OPTION_WILL_FUND_OFFER_REPLY:

hsmd/hsmd_wire.csv

@@ -210,6 +210,14 @@ msgtype,hsmd_validate_commitment_tx_reply,135
 msgdata,hsmd_validate_commitment_tx_reply,old_commitment_secret,?secret,
 msgdata,hsmd_validate_commitment_tx_reply,next_per_commitment_point,pubkey,
 
+# Revoke our local commitment, returns the revocation secret and next point
+msgtype,hsmd_revoke_commitment_tx,40
+msgdata,hsmd_revoke_commitment_tx,commit_num,u64,
+
+msgtype,hsmd_revoke_commitment_tx_reply,140
+msgdata,hsmd_revoke_commitment_tx_reply,old_commitment_secret,secret,
+msgdata,hsmd_revoke_commitment_tx_reply,next_per_commitment_point,pubkey,
+
 # Vaidate the counterparty's revocation secret
 msgtype,hsmd_validate_revocation,36
 msgdata,hsmd_validate_revocation,revoke_num,u64,

hsmd/libhsmd.c

@@ -100,6 +100,7 @@ bool hsmd_check_client_capabilities(struct hsmd_client *client,
 	case WIRE_HSMD_SIGN_REMOTE_COMMITMENT_TX:
 	case WIRE_HSMD_SIGN_REMOTE_HTLC_TX:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX:
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX:
 	case WIRE_HSMD_VALIDATE_REVOCATION:
 		return (client->capabilities & HSM_PERM_SIGN_REMOTE_TX) != 0;
 
@@ -160,6 +161,7 @@ bool hsmd_check_client_capabilities(struct hsmd_client *client,
 	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY:
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_REVOCATION_REPLY:
 	case WIRE_HSMD_SIGN_TX_REPLY:
 	case WIRE_HSMD_SIGN_OPTION_WILL_FUND_OFFER_REPLY:
@@ -1709,20 +1711,58 @@ static u8 *handle_validate_commitment_tx(struct hsmd_client *c, const u8 *msg_in
 		return hsmd_status_bad_request_fmt(
 		    c, msg_in, "bad per_commit_point %" PRIu64, commit_num + 1);
 
-	if (commit_num >= 1) {
-		old_secret = tal(tmpctx, struct secret);
-		if (!per_commit_secret(&shaseed, old_secret, commit_num - 1)) {
-			return hsmd_status_bad_request_fmt(
-			    c, msg_in, "Cannot derive secret %" PRIu64, commit_num - 1);
-		}
-	} else {
-		old_secret = NULL;
-	}
+	/* Don't ever return the old_secret here anymore. The node should
+	 * call hsmd_revoke_commitment_tx to transactionally revoke the commitment
+	 * and return the secret ...
+	 */
+	old_secret = NULL;
 
 	return towire_hsmd_validate_commitment_tx_reply(
 		NULL, old_secret, &next_per_commitment_point);
 }
 
+/* ~This stub implementation is overriden by fully validating signers
+ * that need to independently revoke the old local commitment tx and
+ * release it's secret.
+ * Revoke the old commitment tx by disclosing its secret and also return
+ * the next commitiment's per-commitment-point.
+ */
+static u8 *handle_revoke_commitment_tx(struct hsmd_client *c, const u8 *msg_in)
+{
+	u64 commit_num;
+	struct secret channel_seed;
+	struct sha256 shaseed;
+	struct secret *old_secret;
+	struct pubkey next_per_commitment_point;
+
+	if (!fromwire_hsmd_revoke_commitment_tx(msg_in, &commit_num))
+		return hsmd_status_malformed_request(c, msg_in);
+
+	/* Stub implementation */
+
+	/* The signatures are not checked in this stub because they
+	 * are already checked by the caller.  However, the returned
+	 * old_secret and next_per_commitment_point are used.
+	 */
+
+	get_channel_seed(&c->id, c->dbid, &channel_seed);
+	if (!derive_shaseed(&channel_seed, &shaseed))
+		return hsmd_status_bad_request(c, msg_in, "bad derive_shaseed");
+
+	if (!per_commit_point(&shaseed, &next_per_commitment_point, commit_num + 2))
+		return hsmd_status_bad_request_fmt(
+		    c, msg_in, "bad per_commit_point %" PRIu64, commit_num + 2);
+
+	old_secret = tal(tmpctx, struct secret);
+	if (!per_commit_secret(&shaseed, old_secret, commit_num)) {
+		return hsmd_status_bad_request_fmt(
+		    c, msg_in, "Cannot derive secret %" PRIu64, commit_num);
+	}
+
+	return towire_hsmd_revoke_commitment_tx_reply(
+		NULL, old_secret, &next_per_commitment_point);
+}
+
 /* This stub implementation is overriden by fully validating signers
  * that need to independently verify that the latest state is
  * commited. */
@@ -2010,6 +2050,8 @@ u8 *hsmd_handle_client_message(const tal_t *ctx, struct hsmd_client *client,
 		return handle_sign_commitment_tx(client, msg);
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX:
 		return handle_validate_commitment_tx(client, msg);
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX:
+		return handle_revoke_commitment_tx(client, msg);
 	case WIRE_HSMD_VALIDATE_REVOCATION:
 		return handle_validate_revocation(client, msg);
 	case WIRE_HSMD_SIGN_REMOTE_HTLC_TO_US:
@@ -2052,6 +2094,7 @@ u8 *hsmd_handle_client_message(const tal_t *ctx, struct hsmd_client *client,
 	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY:
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_REVOCATION_REPLY:
 	case WIRE_HSMD_SIGN_TX_REPLY:
 	case WIRE_HSMD_SIGN_OPTION_WILL_FUND_OFFER_REPLY:
@@ -2088,6 +2131,7 @@ u8 *hsmd_init(struct secret hsm_secret,
 		WIRE_HSMD_SIGN_SPLICE_TX,
 		WIRE_HSMD_CHECK_OUTPOINT,
 		WIRE_HSMD_FORGET_CHANNEL,
+		WIRE_HSMD_REVOKE_COMMITMENT_TX,
 	};
 
 	/*~ Don't swap this. */

lightningd/Makefile

@@ -103,6 +103,7 @@ LIGHTNINGD_COMMON_OBJS :=			\
 	common/status_wiregen.o			\
 	common/hash_u5.o			\
 	common/hmac.o				\
+	common/hsm_capable.o			\
 	common/hsm_encryption.o			\
 	common/htlc_state.o			\
 	common/htlc_trim.o			\

lightningd/channel_control.c

@@ -1626,6 +1626,9 @@ bool peer_start_channeld(struct channel *channel,
 	initmsg = towire_channeld_init(tmpctx,
 				       chainparams,
 				       ld->our_features,
+				       /* Capabilities arg needs to be a tal array */
+				       tal_dup_arr(tmpctx, u32, ld->hsm_capabilities,
+						   tal_count(ld->hsm_capabilities), 0),
 				       &channel->cid,
 				       &channel->funding,
 				       channel->funding_sats,

lightningd/hsm_control.c

@@ -3,6 +3,7 @@
 #include <ccan/fdpass/fdpass.h>
 #include <common/ecdh.h>
 #include <common/errcode.h>
+#include <common/hsm_capable.h>
 #include <common/hsm_encryption.h>
 #include <common/hsm_version.h>
 #include <common/invoice_path_id.h>
@@ -78,11 +79,7 @@ static unsigned int hsm_msg(struct subd *hsmd,
  * number) */
 bool hsm_capable(struct lightningd *ld, u32 msgtype)
 {
-	for (size_t i = 0; i < tal_count(ld->hsm_capabilities); i++) {
-		if (ld->hsm_capabilities[i] == msgtype)
-			return true;
-	}
-	return false;
+	return hsm_is_capable(ld->hsm_capabilities, msgtype);
 }
 
 struct ext_key *hsm_init(struct lightningd *ld)

tests/test_closing.py

@@ -2438,6 +2438,8 @@ def try_pay():
                 assert acc['account_resolved']
                 assert acc['resolved_at_block'] > 0
 
+    # Have l1 send all funds to check that the unilateral close info is correct
+    l1.rpc.withdraw(l1.rpc.newaddr('bech32')['bech32'], 'all', minconf=0)
 
 def test_listfunds_after_their_unilateral(node_factory, bitcoind):
     """We keep spending info around for their unilateral closes.

wallet/test/Makefile

@@ -14,6 +14,7 @@ WALLET_TEST_COMMON_OBJS :=			\
 	common/derive_basepoints.o		\
 	common/features.o			\
 	common/htlc_state.o			\
+	common/hsm_capable.o			\
 	common/htlc_wire.o			\
 	common/fee_states.o			\
 	common/type_to_string.o			\