Adds validate holder commitment API (#28)

* added hsm_validate_commitment, channeld call populated, other 2 opening calls not done.

* Replaced payment_hashmap in validate_commitment_tx with htlcs and added feerate.

* return next_point and old_secret from handle_validate_commitment_tx and use.

* added hsmd_validate_commitment_tx calls to openingd initial commitment cases

* fixed botched syntax, skipped wallet test which violates policy

Author: ksedgwic

channeld/channeld.c

@@ -1489,6 +1489,17 @@ static u8 *make_revocation_msg(const struct peer *peer, u64 revoke_index,
 				     point);
 }
 
+static u8 *make_revocation_msg_from_secret(const struct peer *peer,
+					   u64 revoke_index,
+					   struct pubkey *point,
+					   const struct secret *old_commit_secret,
+					   const struct pubkey *next_point)
+{
+	*point = *next_point;
+	return towire_revoke_and_ack(peer, &peer->channel_id,
+				     old_commit_secret, next_point);
+}
+
 /* Convert changed htlcs into parts which lightningd expects. */
 static void marshall_htlc_info(const tal_t *ctx,
 			       const struct htlc **changed_htlcs,
@@ -1547,7 +1558,9 @@ static void send_revocation(struct peer *peer,
 			    const struct bitcoin_signature *commit_sig,
 			    const struct bitcoin_signature *htlc_sigs,
 			    const struct htlc **changed_htlcs,
-			    const struct bitcoin_tx *committx)
+			    const struct bitcoin_tx *committx,
+			    const struct secret *old_secret,
+			    const struct pubkey *next_point)
 {
 	struct changed_htlc *changed;
 	struct fulfilled_htlc *fulfilled;
@@ -1565,8 +1578,9 @@ static void send_revocation(struct peer *peer,
 			   &added);
 
 	/* Revoke previous commit, get new point. */
-	u8 *msg = make_revocation_msg(peer, peer->next_index[LOCAL]-1,
-				      &peer->next_local_per_commit);
+	u8 *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]++;
@@ -1731,8 +1745,55 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg)
 	status_debug("Received commit_sig with %zu htlc sigs",
 		     tal_count(htlc_sigs));
 
-	send_revocation(peer,
-			&commit_sig, htlc_sigs, changed_htlcs, txs[0]);
+	// Collect the htlcs for call to hsmd validate.
+	//
+	// We use the existing_htlc to_wire routines, it's unfortunate that
+	// we have to send a dummy onion_routing_packet ...
+	//
+	struct existing_htlc **htlcs = tal_arr(NULL, struct existing_htlc *, 0);
+	u8 dummy_onion_routing_packet[TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE)];
+	memset(dummy_onion_routing_packet, 0, sizeof(dummy_onion_routing_packet));
+	size_t num_entries = tal_count(htlc_map);
+	for (size_t ndx = 0; ndx < num_entries; ++ndx) {
+		struct htlc const *hh = htlc_map[ndx];
+		if (hh) {
+			status_debug("HTLC[%lu]=%" PRIu64 ", %s",
+				     ndx, hh->id, htlc_state_name(hh->state));
+			struct existing_htlc *existing =
+				new_existing_htlc(NULL,
+						  hh->id,
+						  hh->state,
+						  hh->amount,
+						  &hh->rhash,
+						  hh->expiry.locktime,
+						  dummy_onion_routing_packet,
+						  NULL,
+						  NULL,
+						  NULL);
+			tal_arr_expand(&htlcs, tal_steal(htlcs, existing));
+		}
+	}
+
+	// Validate the counterparty's signatures, returns old_secret.
+	const u8 * msg2 =
+		towire_hsmd_validate_commitment_tx(NULL,
+						   txs[0],
+						   (const struct existing_htlc **) htlcs,
+						   peer->next_index[LOCAL],
+						   channel_feerate(peer->channel, LOCAL),
+						   &commit_sig,
+						   htlc_sigs);
+	tal_free(htlcs);
+	msg2 = hsm_req(tmpctx, take(msg2));
+	struct secret *old_secret;
+	struct pubkey next_point;
+	if (!fromwire_hsmd_validate_commitment_tx_reply(tmpctx, msg2, &old_secret, &next_point))
+		status_failed(STATUS_FAIL_HSM_IO,
+			      "Reading validate_commitment_tx reply: %s",
+			      tal_hex(tmpctx, msg2));
+
+	send_revocation(peer, &commit_sig, htlc_sigs, changed_htlcs, txs[0],
+			old_secret, &next_point);
 
 	/* We may now be quiescent on our side. */
 	maybe_send_stfu(peer);

contrib/remote_hsmd/Makefile

@@ -40,17 +40,20 @@ RMTHSMD_COMMON_OBJS :=				\
 	common/daemon.o				\
 	common/daemon_conn.o		\
 	common/derive_basepoints.o	\
-	common/status_wiregen.o		\
 	common/hash_u5.o			\
+	common/htlc_state.o			\
+	common/htlc_wire.o			\
 	common/key_derive.o			\
 	common/memleak.o			\
 	common/msg_queue.o			\
 	common/node_id.o			\
+	common/onionreply.o			\
 	common/permute_tx.o			\
 	common/pseudorand.o			\
 	common/setup.o				\
 	common/status.o				\
 	common/status_wire.o		\
+	common/status_wiregen.o		\
 	common/subdaemon.o			\
 	common/type_to_string.o		\
 	common/utils.o				\

contrib/remote_hsmd/dump.cc

@@ -6,6 +6,7 @@ extern "C" {
 #include <bitcoin/signature.h>
 #include <bitcoin/tx.h>
 #include <common/derive_basepoints.h>
+#include <common/htlc_wire.h>
 #include <common/node_id.h>
 #include <common/status.h>
 #include <common/utils.h>
@@ -56,11 +57,24 @@ string dump_bitcoin_signature(const struct bitcoin_signature *sp)
 	ostrm << "{ "
 	      << "\"sighash_type\":" << int(sp->sighash_type)
 	      << ", \"s\":"
-	      << '"' << dump_secp256k1_ecdsa_signature(&sp->s) << '"'
+	      << dump_secp256k1_ecdsa_signature(&sp->s)
 	      << " }";
 	return ostrm.str();
 }
 
+string dump_htlc_signatures(const struct bitcoin_signature *sps)
+{
+	ostringstream ostrm;
+ 	ostrm << "[";
+	for (size_t input_ndx = 0; input_ndx < tal_count(sps); ++input_ndx) {
+		if (input_ndx != 0)
+			ostrm << ", ";
+		ostrm << dump_bitcoin_signature(&sps[input_ndx]);
+	}
+ 	ostrm << "]";
+	return ostrm.str();
+}
+
 string dump_secp256k1_ecdsa_signature(const secp256k1_ecdsa_signature *sp)
 {
 	return dump_hex(sp->data, sizeof(sp->data));
@@ -428,7 +442,6 @@ string dump_wally_psbt_output(const struct wally_psbt_output *out)
 	ostrm << ", \"unknowns\":" << dump_wally_unknowns_map(&out->unknowns);
 	ostrm << " }";
 	return ostrm.str();
-
 }
 
 string dump_wally_psbt_outputs(const struct wally_psbt_output *outputs,
@@ -484,6 +497,32 @@ string dump_rhashes(const struct sha256 *rhashes, size_t num_rhashes)
 	return ostrm.str();
 }
 
+string dump_htlc(const struct existing_htlc *htlc)
+{
+	ostringstream ostrm;
+	ostrm << "{ "
+	      << "\"id\":" << htlc->id
+	      << ", \"state\":" << htlc_state_name(htlc->state)
+	      << ", \"amount_msat\":" << htlc->amount.millisatoshis
+	      << ", \"payment_hash\":" << dump_hex(&htlc->payment_hash, sizeof(htlc->payment_hash))
+	      << ", \"cltv_expiry\":" << htlc->cltv_expiry
+	      << " }";
+	return ostrm.str();
+}
+
+string dump_htlcs(const struct existing_htlc **htlc, size_t num_htlc)
+{
+	ostringstream ostrm;
+	ostrm << "[";
+	for (size_t ii = 0; ii < num_htlc; ii++) {
+		if (ii != 0)
+			ostrm << ",";
+		ostrm << dump_htlc(htlc[ii]);
+	}
+	ostrm << "]";
+	return ostrm.str();
+}
+
 /* <sigh>.  Bitcoind represents hashes as little-endian for RPC. */
 void reverse_bytes(u8 *arr, size_t len)
 {

contrib/remote_hsmd/dump.hpp

@@ -11,6 +11,7 @@ std::string dump_hex(const void *vptr, size_t sz);
 std::string dump_basepoints(const struct basepoints *bp);
 std::string dump_bitcoin_txid(const struct bitcoin_txid *txid);
 std::string dump_bitcoin_signature(const struct bitcoin_signature *sp);
+std::string dump_htlc_signatures(const struct bitcoin_signature *sps);
 std::string dump_secp256k1_ecdsa_signature(const secp256k1_ecdsa_signature *sp);
 std::string dump_secp256k1_ecdsa_recoverable_signature(const secp256k1_ecdsa_recoverable_signature *sp);
 std::string dump_secret(const struct secret *sp);
@@ -37,6 +38,8 @@ std::string dump_wally_tx(const struct wally_tx *wtx);
 std::string dump_wally_psbt(const struct wally_psbt *psbt);
 std::string dump_tx(const struct bitcoin_tx *tx);
 std::string dump_rhashes(const struct sha256 *rhashes, size_t num_rhashes);
+std::string dump_htlc(const struct existing_htlc *htlc);
+std::string dump_htlcs(const struct existing_htlc **htlc, size_t num_htlc);
 
 // needed for formatting txid
 void reverse_bytes(u8 *arr, size_t len);

contrib/remote_hsmd/hsmd.c

@@ -808,6 +808,46 @@ static struct io_plan *handle_sign_commitment_tx(struct io_conn *conn,
 			 take(towire_hsmd_sign_commitment_tx_reply(NULL, &sig)));
 }
 
+/* Validate the peer's signatures for our commitment and htlc txs. */
+static struct io_plan *handle_validate_commitment_tx(struct io_conn *conn,
+						     struct client *c,
+						     const u8 *msg_in)
+{
+	struct bitcoin_tx *tx;
+	struct existing_htlc **htlc;
+	u64 commit_num;
+	u32 feerate;
+	struct bitcoin_signature commit_sig;
+	struct bitcoin_signature *htlc_sigs;
+	struct secret *old_secret;
+	struct pubkey next_per_commitment_point;
+
+	if (!fromwire_hsmd_validate_commitment_tx(tmpctx, msg_in,
+						  &tx, &htlc,
+						  &commit_num, &feerate,
+						  &commit_sig, &htlc_sigs))
+		bad_req(conn, c, msg_in);
+
+	proxy_stat rv = proxy_handle_validate_commitment_tx(
+		tx,
+		&c->id, c->dbid,
+		htlc, commit_num, feerate,
+		&commit_sig, htlc_sigs,
+		&old_secret, &next_per_commitment_point);
+	if (PROXY_PERMANENT(rv))
+		status_failed(STATUS_FAIL_INTERNAL_ERROR,
+		              "proxy_%s failed: %s", __FUNCTION__,
+			      proxy_last_message());
+	else if (!PROXY_SUCCESS(rv))
+		return bad_req_fmt(conn, c, msg_in,
+				   "proxy_%s error: %s", __FUNCTION__,
+				   proxy_last_message());
+
+	return req_reply(conn, c,
+			 take(towire_hsmd_validate_commitment_tx_reply(
+				      NULL, old_secret, &next_per_commitment_point)));
+}
+
 /*~ This is used by channeld to create signatures for the remote peer's
  * commitment transaction.  It's functionally identical to signing our own,
  * but we expect to do this repeatedly as commitment transactions are
@@ -1595,6 +1635,7 @@ static bool check_client_capabilities(struct client *client,
 
 	case WIRE_HSMD_SIGN_REMOTE_COMMITMENT_TX:
 	case WIRE_HSMD_SIGN_REMOTE_HTLC_TX:
+	case WIRE_HSMD_VALIDATE_COMMITMENT_TX:
 		return (client->capabilities & HSM_CAP_SIGN_REMOTE_TX) != 0;
 
 	case WIRE_HSMD_SIGN_MUTUAL_CLOSE_TX:
@@ -1628,6 +1669,7 @@ static bool check_client_capabilities(struct client *client,
 	case WIRE_HSMD_INIT_REPLY:
 	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY:
+	case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_SIGN_TX_REPLY:
 	case WIRE_HSMD_GET_PER_COMMITMENT_POINT_REPLY:
 	case WIRE_HSMD_CHECK_FUTURE_SECRET_REPLY:
@@ -1704,6 +1746,9 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
 	case WIRE_HSMD_SIGN_COMMITMENT_TX:
 		return handle_sign_commitment_tx(conn, c, c->msg_in);
 
+	case WIRE_HSMD_VALIDATE_COMMITMENT_TX:
+		return handle_validate_commitment_tx(conn, c, c->msg_in);
+
 	case WIRE_HSMD_SIGN_DELAYED_PAYMENT_TO_US:
 		return handle_sign_delayed_payment_to_us(conn, c, c->msg_in);
 
@@ -1754,6 +1799,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
 	case WIRE_HSMD_INIT_REPLY:
 	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY:
+	case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_SIGN_TX_REPLY:
 	case WIRE_HSMD_GET_PER_COMMITMENT_POINT_REPLY:
 	case WIRE_HSMD_CHECK_FUTURE_SECRET_REPLY: