From daf1560eb4e99dfdfcfd1acf0212e1020ec0f933 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 May 2025 05:18:46 +0930 Subject: [PATCH] hsmd: make our private utxo type, to ensure binary compatibility. I'm about to update our utxo type, but Christian spotted that this is part of the ABI for the hsm. So make that a private "hsm_utxo" type, to insulate it from changes. In particular, the HSM versions only contain the fields that the hsm cares about, and the wire format is consistent (even though that *did* include some of those fields, they are now dummies). In the long term, this should be removed from the ABI: once we no longer have "close_info" utxos, this information should already be in the PSBT. I tested this hadn't accidentally changed the wire format by disabling version checks and using an old hsmd with the altered daemons and running the test suite. Signed-off-by: Rusty Russell --- common/hsm_version.h | 1 + common/utxo.c | 60 -------------------- common/utxo.h | 3 - hsmd/Makefile | 3 +- hsmd/hsm_utxo.c | 105 +++++++++++++++++++++++++++++++++++ hsmd/hsm_utxo.h | 31 +++++++++++ hsmd/hsmd_wire.csv | 8 +-- hsmd/libhsmd.c | 14 ++--- lightningd/anchorspend.c | 5 +- lightningd/onchain_control.c | 5 +- tools/generate-wire.py | 2 +- wallet/walletrpc.c | 6 +- 12 files changed, 160 insertions(+), 83 deletions(-) create mode 100644 hsmd/hsm_utxo.c create mode 100644 hsmd/hsm_utxo.h diff --git a/common/hsm_version.h b/common/hsm_version.h index e4c4cecf2..8ee80cf84 100644 --- a/common/hsm_version.h +++ b/common/hsm_version.h @@ -27,6 +27,7 @@ * v5 with preapprove_check: 0ed6dd4ea2c02b67c51b1420b3d07ab2227a4c06ce7e2942d946967687e9baf7 * v6 no secret from get_per_commitment_point: 0cad1790beb3473d64355f4cb4f64daa80c28c8a241998b7ef0223385d7ffff9 * v6 with sign_bolt12_2 (tweak using node id): 8fcb731279a10af3f95aeb8be1da6b2ced76a1984afa18c5f46a03515d70ea0e + * v6 (internal rework only): fba120d3d926de00f0377c4cba91caa89a9eaacb666fd04a5a0e677b4d310d65 */ #define HSM_MIN_VERSION 5 #define HSM_MAX_VERSION 6 diff --git a/common/utxo.c b/common/utxo.c index a28e99cd3..0b5bef357 100644 --- a/common/utxo.c +++ b/common/utxo.c @@ -2,66 +2,6 @@ #include #include -void towire_utxo(u8 **pptr, const struct utxo *utxo) -{ - /* Is this a unilateral close output and needs the - * close_info? */ - bool is_unilateral_close = utxo->close_info != NULL; - towire_bitcoin_outpoint(pptr, &utxo->outpoint); - towire_amount_sat(pptr, utxo->amount); - towire_u32(pptr, utxo->keyindex); - towire_bool(pptr, utxo->is_p2sh); - - towire_u16(pptr, tal_count(utxo->scriptPubkey)); - towire_u8_array(pptr, utxo->scriptPubkey, tal_count(utxo->scriptPubkey)); - - towire_bool(pptr, is_unilateral_close); - if (is_unilateral_close) { - towire_u64(pptr, utxo->close_info->channel_id); - towire_node_id(pptr, &utxo->close_info->peer_id); - towire_bool(pptr, utxo->close_info->commitment_point != NULL); - if (utxo->close_info->commitment_point) - towire_pubkey(pptr, utxo->close_info->commitment_point); - towire_bool(pptr, utxo->close_info->option_anchors); - towire_u32(pptr, utxo->close_info->csv); - } - - towire_bool(pptr, utxo->is_in_coinbase); -} - -struct utxo *fromwire_utxo(const tal_t *ctx, const u8 **ptr, size_t *max) -{ - struct utxo *utxo = tal(ctx, struct utxo); - - fromwire_bitcoin_outpoint(ptr, max, &utxo->outpoint); - utxo->amount = fromwire_amount_sat(ptr, max); - utxo->keyindex = fromwire_u32(ptr, max); - utxo->is_p2sh = fromwire_bool(ptr, max); - - utxo->scriptPubkey = fromwire_tal_arrn(utxo, ptr, max, fromwire_u16(ptr, max)); - - if (fromwire_bool(ptr, max)) { - utxo->close_info = tal(utxo, struct unilateral_close_info); - utxo->close_info->channel_id = fromwire_u64(ptr, max); - fromwire_node_id(ptr, max, &utxo->close_info->peer_id); - if (fromwire_bool(ptr, max)) { - utxo->close_info->commitment_point = tal(utxo, - struct pubkey); - fromwire_pubkey(ptr, max, - utxo->close_info->commitment_point); - } else - utxo->close_info->commitment_point = NULL; - utxo->close_info->option_anchors - = fromwire_bool(ptr, max); - utxo->close_info->csv = fromwire_u32(ptr, max); - } else { - utxo->close_info = NULL; - } - - utxo->is_in_coinbase = fromwire_bool(ptr, max); - return utxo; -} - size_t utxo_spend_weight(const struct utxo *utxo, size_t min_witness_weight) { size_t wit_weight = bitcoin_tx_simple_input_witness_weight(); diff --git a/common/utxo.h b/common/utxo.h index 830b86576..f4dcf50df 100644 --- a/common/utxo.h +++ b/common/utxo.h @@ -81,9 +81,6 @@ static inline bool utxo_is_csv_locked(const struct utxo *utxo, u32 current_heigh return *utxo->blockheight + utxo->close_info->csv > current_height; } -void towire_utxo(u8 **pptr, const struct utxo *utxo); -struct utxo *fromwire_utxo(const tal_t *ctx, const u8 **ptr, size_t *max); - /* Estimate of (signed) UTXO weight in transaction */ size_t utxo_spend_weight(const struct utxo *utxo, size_t min_witness_weight); diff --git a/hsmd/Makefile b/hsmd/Makefile index 8a788cef5..b4f51bef2 100644 --- a/hsmd/Makefile +++ b/hsmd/Makefile @@ -2,6 +2,7 @@ HSMD_SRC := hsmd/hsmd.c \ hsmd/hsmd_wiregen.c \ + hsmd/hsm_utxo.c \ hsmd/libhsmd.c HSMD_HEADERS := hsmd/hsmd_wiregen.h hsmd/permissions.h @@ -12,6 +13,7 @@ $(HSMD_OBJS): $(HSMD_HEADERS) # Other programs which use the hsm need this. HSMD_CLIENT_OBJS := \ hsmd/hsmd_wiregen.o \ + hsmd/hsm_utxo.o \ common/htlc_wire.o # Make sure these depend on everything. @@ -50,7 +52,6 @@ HSMD_COMMON_OBJS := \ common/status_wiregen.o \ common/subdaemon.o \ common/utils.o \ - common/utxo.o \ common/version.o \ common/wireaddr.o diff --git a/hsmd/hsm_utxo.c b/hsmd/hsm_utxo.c new file mode 100644 index 000000000..1a55aa575 --- /dev/null +++ b/hsmd/hsm_utxo.c @@ -0,0 +1,105 @@ +#include "config.h" +#include +#include + +static const struct hsm_utxo *to_hsm_utxo(const tal_t *ctx, + const struct utxo *utxo) +{ + struct hsm_utxo *hutxo = tal(ctx, struct hsm_utxo); + + hutxo->outpoint = utxo->outpoint; + hutxo->amount = utxo->amount; + hutxo->keyindex = utxo->keyindex; + + if (utxo->close_info) { + hutxo->close_info + = tal_dup(hutxo, struct unilateral_close_info, + utxo->close_info); + if (hutxo->close_info->commitment_point) + hutxo->close_info->commitment_point + = tal_dup(hutxo->close_info, + struct pubkey, + hutxo->close_info->commitment_point); + } else + hutxo->close_info = NULL; + + if (utxo->scriptPubkey) + hutxo->scriptPubkey = tal_dup_talarr(hutxo, u8, utxo->scriptPubkey); + else + hutxo->scriptPubkey = NULL; + + return hutxo; +} + +const struct hsm_utxo **utxos_to_hsm_utxos(const tal_t *ctx, + struct utxo **utxos) +{ + const struct hsm_utxo **hutxos + = tal_arr(ctx, const struct hsm_utxo *, tal_count(utxos)); + + for (size_t i = 0; i < tal_count(hutxos); i++) + hutxos[i] = to_hsm_utxo(hutxos, utxos[i]); + return hutxos; +} + +void towire_hsm_utxo(u8 **pptr, const struct hsm_utxo *utxo) +{ + /* Is this a unilateral close output and needs the + * close_info? */ + bool is_unilateral_close = utxo->close_info != NULL; + towire_bitcoin_outpoint(pptr, &utxo->outpoint); + towire_amount_sat(pptr, utxo->amount); + towire_u32(pptr, utxo->keyindex); + /* Used to be ->is_p2sh, but HSM uses scriptpubkey to determine type */ + towire_bool(pptr, false); + + towire_u16(pptr, tal_count(utxo->scriptPubkey)); + towire_u8_array(pptr, utxo->scriptPubkey, tal_count(utxo->scriptPubkey)); + + towire_bool(pptr, is_unilateral_close); + if (is_unilateral_close) { + towire_u64(pptr, utxo->close_info->channel_id); + towire_node_id(pptr, &utxo->close_info->peer_id); + towire_bool(pptr, utxo->close_info->commitment_point != NULL); + if (utxo->close_info->commitment_point) + towire_pubkey(pptr, utxo->close_info->commitment_point); + towire_bool(pptr, utxo->close_info->option_anchors); + towire_u32(pptr, utxo->close_info->csv); + } + + /* Used to be ->is_in_coinbase, but HSM doesn't care */ + towire_bool(pptr, false); +} + +struct hsm_utxo *fromwire_hsm_utxo(const tal_t *ctx, const u8 **ptr, size_t *max) +{ + struct hsm_utxo *utxo = tal(ctx, struct hsm_utxo); + + fromwire_bitcoin_outpoint(ptr, max, &utxo->outpoint); + utxo->amount = fromwire_amount_sat(ptr, max); + utxo->keyindex = fromwire_u32(ptr, max); + fromwire_bool(ptr, max); + + utxo->scriptPubkey = fromwire_tal_arrn(utxo, ptr, max, fromwire_u16(ptr, max)); + + if (fromwire_bool(ptr, max)) { + utxo->close_info = tal(utxo, struct unilateral_close_info); + utxo->close_info->channel_id = fromwire_u64(ptr, max); + fromwire_node_id(ptr, max, &utxo->close_info->peer_id); + if (fromwire_bool(ptr, max)) { + utxo->close_info->commitment_point = tal(utxo, + struct pubkey); + fromwire_pubkey(ptr, max, + utxo->close_info->commitment_point); + } else + utxo->close_info->commitment_point = NULL; + utxo->close_info->option_anchors + = fromwire_bool(ptr, max); + utxo->close_info->csv = fromwire_u32(ptr, max); + } else { + utxo->close_info = NULL; + } + + fromwire_bool(ptr, max); + return utxo; +} diff --git a/hsmd/hsm_utxo.h b/hsmd/hsm_utxo.h new file mode 100644 index 000000000..62bea0026 --- /dev/null +++ b/hsmd/hsm_utxo.h @@ -0,0 +1,31 @@ +#ifndef LIGHTNING_HSMD_HSM_UTXO_H +#define LIGHTNING_HSMD_HSM_UTXO_H +#include "config.h" +#include +#include +#include + +/* FIXME: If we make our static_remotekey a normal keypath key, we can + * simply put that close information inside the PSBT, and we don't + * need to hand the utxo to hsmd at all. */ + +/* /!\ This is part of the HSM ABI: do not change! /!\ */ +struct hsm_utxo { + struct bitcoin_outpoint outpoint; + struct amount_sat amount; + u32 keyindex; + + /* Optional unilateral close information, NULL if this is just + * a HD key */ + struct unilateral_close_info *close_info; + + /* The scriptPubkey if it is known */ + u8 *scriptPubkey; +}; + +void towire_hsm_utxo(u8 **pptr, const struct hsm_utxo *utxo); +struct hsm_utxo *fromwire_hsm_utxo(const tal_t *ctx, const u8 **ptr, size_t *max); + +const struct hsm_utxo **utxos_to_hsm_utxos(const tal_t *ctx, + struct utxo **utxos); +#endif /* LIGHTNING_HSMD_HSM_UTXO_H */ diff --git a/hsmd/hsmd_wire.csv b/hsmd/hsmd_wire.csv index 98faee7d4..a0b1823f6 100644 --- a/hsmd/hsmd_wire.csv +++ b/hsmd/hsmd_wire.csv @@ -118,7 +118,7 @@ msgdata,hsmd_forget_channel,dbid,u64, msgtype,hsmd_forget_channel_reply,134 # Return signature for a funding tx. -#include +#include # Master asks the HSM to sign a node_announcement msgtype,hsmd_node_announcement_sig_req,6 @@ -132,7 +132,7 @@ msgdata,hsmd_node_announcement_sig_reply,signature,secp256k1_ecdsa_signature, #include msgtype,hsmd_sign_withdrawal,7 msgdata,hsmd_sign_withdrawal,num_inputs,u16, -msgdata,hsmd_sign_withdrawal,inputs,utxo,num_inputs +msgdata,hsmd_sign_withdrawal,inputs,hsm_utxo,num_inputs msgdata,hsmd_sign_withdrawal,psbt,wally_psbt, msgtype,hsmd_sign_withdrawal_reply,107 @@ -425,7 +425,7 @@ msgtype,hsmd_sign_anchorspend,147 msgdata,hsmd_sign_anchorspend,peerid,node_id, msgdata,hsmd_sign_anchorspend,channel_dbid,u64, msgdata,hsmd_sign_anchorspend,num_inputs,u16, -msgdata,hsmd_sign_anchorspend,inputs,utxo,num_inputs +msgdata,hsmd_sign_anchorspend,inputs,hsm_utxo,num_inputs msgdata,hsmd_sign_anchorspend,psbt,wally_psbt, msgtype,hsmd_sign_anchorspend_reply,148 @@ -474,7 +474,7 @@ msgtype,hsmd_sign_htlc_tx_mingle,149 msgdata,hsmd_sign_htlc_tx_mingle,peerid,node_id, msgdata,hsmd_sign_htlc_tx_mingle,channel_dbid,u64, msgdata,hsmd_sign_htlc_tx_mingle,num_inputs,u16, -msgdata,hsmd_sign_htlc_tx_mingle,inputs,utxo,num_inputs +msgdata,hsmd_sign_htlc_tx_mingle,inputs,hsm_utxo,num_inputs msgdata,hsmd_sign_htlc_tx_mingle,psbt,wally_psbt, msgtype,hsmd_sign_htlc_tx_mingle_reply,150 diff --git a/hsmd/libhsmd.c b/hsmd/libhsmd.c index d5abcc750..7eda7f1f5 100644 --- a/hsmd/libhsmd.c +++ b/hsmd/libhsmd.c @@ -527,7 +527,7 @@ static void bitcoin_key(struct privkey *privkey, struct pubkey *pubkey, /* This gets the bitcoin private key needed to spend from our wallet */ static void hsm_key_for_utxo(struct privkey *privkey, struct pubkey *pubkey, - const struct utxo *utxo) + const struct hsm_utxo *utxo) { if (utxo->close_info != NULL) { /* This is a their_unilateral_close/to-us output, so @@ -545,11 +545,11 @@ static void hsm_key_for_utxo(struct privkey *privkey, struct pubkey *pubkey, /* Find our inputs by the pubkey associated with the inputs, and * add a partial sig for each */ -static void sign_our_inputs(struct utxo **utxos, struct wally_psbt *psbt) +static void sign_our_inputs(struct hsm_utxo **utxos, struct wally_psbt *psbt) { bool is_cache_enabled = false; for (size_t i = 0; i < tal_count(utxos); i++) { - struct utxo *utxo = utxos[i]; + struct hsm_utxo *utxo = utxos[i]; for (size_t j = 0; j < psbt->num_inputs; j++) { struct privkey privkey; struct pubkey pubkey; @@ -1315,11 +1315,11 @@ static u8 *handle_get_per_commitment_point(struct hsmd_client *c, const u8 *msg_ * we can do more to check the previous case is valid. */ static u8 *handle_sign_withdrawal_tx(struct hsmd_client *c, const u8 *msg_in) { - struct utxo **utxos; + struct hsm_utxo **utxos; struct wally_psbt *psbt; if (!fromwire_hsmd_sign_withdrawal(tmpctx, msg_in, - &utxos, &psbt)) + &utxos, &psbt)) return hsmd_status_malformed_request(c, msg_in); sign_our_inputs(utxos, psbt); @@ -1705,7 +1705,7 @@ static u8 *handle_sign_anchorspend(struct hsmd_client *c, const u8 *msg_in) { struct node_id peer_id; u64 dbid; - struct utxo **utxos; + struct hsm_utxo **utxos; struct wally_psbt *psbt; struct secret seed; struct pubkey local_funding_pubkey; @@ -1744,7 +1744,7 @@ static u8 *handle_sign_htlc_tx_mingle(struct hsmd_client *c, const u8 *msg_in) { struct node_id peer_id; u64 dbid; - struct utxo **utxos; + struct hsm_utxo **utxos; struct wally_psbt *psbt; /* FIXME: Check output goes to us. */ diff --git a/lightningd/anchorspend.c b/lightningd/anchorspend.c index d7b488e34..bf7417189 100644 --- a/lightningd/anchorspend.c +++ b/lightningd/anchorspend.c @@ -327,6 +327,7 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx, struct amount_sat psbt_fee, diff; struct bitcoin_tx *tx; struct utxo **psbt_utxos; + const struct hsm_utxo **hsm_utxos; struct wally_psbt *psbt, *signed_psbt; struct amount_msat total_value; const struct deadline_value *unimportant_deadline; @@ -491,11 +492,11 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx, * 1000 / psbt_weight); /* OK, HSM, sign it! */ + hsm_utxos = utxos_to_hsm_utxos(tmpctx, psbt_utxos); msg = towire_hsmd_sign_anchorspend(NULL, &channel->peer->id, channel->dbid, - cast_const2(const struct utxo **, - psbt_utxos), + hsm_utxos, psbt); msg = hsm_sync_req(tmpctx, ld, take(msg)); if (!fromwire_hsmd_sign_anchorspend_reply(tmpctx, msg, &signed_psbt)) diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 3ea834ee0..f4444bebb 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -1056,6 +1056,7 @@ static bool consider_onchain_htlc_tx_rebroadcast(struct channel *channel, { struct amount_sat change, excess; struct utxo **utxos; + const struct hsm_utxo **hsm_utxos; u32 feerate; size_t weight; struct bitcoin_tx *newtx; @@ -1140,11 +1141,11 @@ static bool consider_onchain_htlc_tx_rebroadcast(struct channel *channel, } /* Now, get HSM to sign off. */ + hsm_utxos = utxos_to_hsm_utxos(tmpctx, utxos); msg = towire_hsmd_sign_htlc_tx_mingle(NULL, &channel->peer->id, channel->dbid, - cast_const2(const struct utxo **, - utxos), + hsm_utxos, psbt); msg = hsm_sync_req(tmpctx, ld, take(msg)); if (!fromwire_hsmd_sign_htlc_tx_mingle_reply(tmpctx, msg, &psbt)) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index f408cca15..284a26cb8 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -233,7 +233,7 @@ class Type(FieldSet): 'existing_htlc', 'simple_htlc', 'inflight', - 'utxo', + 'hsm_utxo', 'bitcoin_tx', 'wirestring', 'per_peer_state', diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 8680d3a7e..e1d255382 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -779,6 +779,7 @@ static struct command_result *json_signpsbt(struct command *cmd, struct json_stream *response; struct wally_psbt *psbt, *signed_psbt; struct utxo **utxos; + const struct hsm_utxo **hsm_utxos; u32 *input_nums; u32 psbt_version; @@ -826,9 +827,8 @@ static struct command_result *json_signpsbt(struct command *cmd, /* FIXME: hsm will sign almost anything, but it should really * fail cleanly (not abort!) and let us report the error here. */ - u8 *msg = towire_hsmd_sign_withdrawal(cmd, - cast_const2(const struct utxo **, utxos), - psbt); + hsm_utxos = utxos_to_hsm_utxos(tmpctx, utxos); + u8 *msg = towire_hsmd_sign_withdrawal(cmd, hsm_utxos, psbt); if (!wire_sync_write(cmd->ld->hsm_fd, take(msg))) fatal("Could not write sign_withdrawal to HSM: %s",