Skip to content

Commit daf1560

Browse files
committed
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 <rusty@rustcorp.com.au>
1 parent f992eb1 commit daf1560

File tree

12 files changed

+160
-83
lines changed

12 files changed

+160
-83
lines changed

common/hsm_version.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
* v5 with preapprove_check: 0ed6dd4ea2c02b67c51b1420b3d07ab2227a4c06ce7e2942d946967687e9baf7
2828
* v6 no secret from get_per_commitment_point: 0cad1790beb3473d64355f4cb4f64daa80c28c8a241998b7ef0223385d7ffff9
2929
* v6 with sign_bolt12_2 (tweak using node id): 8fcb731279a10af3f95aeb8be1da6b2ced76a1984afa18c5f46a03515d70ea0e
30+
* v6 (internal rework only): fba120d3d926de00f0377c4cba91caa89a9eaacb666fd04a5a0e677b4d310d65
3031
*/
3132
#define HSM_MIN_VERSION 5
3233
#define HSM_MAX_VERSION 6

common/utxo.c

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,66 +2,6 @@
22
#include <common/utxo.h>
33
#include <wire/wire.h>
44

5-
void towire_utxo(u8 **pptr, const struct utxo *utxo)
6-
{
7-
/* Is this a unilateral close output and needs the
8-
* close_info? */
9-
bool is_unilateral_close = utxo->close_info != NULL;
10-
towire_bitcoin_outpoint(pptr, &utxo->outpoint);
11-
towire_amount_sat(pptr, utxo->amount);
12-
towire_u32(pptr, utxo->keyindex);
13-
towire_bool(pptr, utxo->is_p2sh);
14-
15-
towire_u16(pptr, tal_count(utxo->scriptPubkey));
16-
towire_u8_array(pptr, utxo->scriptPubkey, tal_count(utxo->scriptPubkey));
17-
18-
towire_bool(pptr, is_unilateral_close);
19-
if (is_unilateral_close) {
20-
towire_u64(pptr, utxo->close_info->channel_id);
21-
towire_node_id(pptr, &utxo->close_info->peer_id);
22-
towire_bool(pptr, utxo->close_info->commitment_point != NULL);
23-
if (utxo->close_info->commitment_point)
24-
towire_pubkey(pptr, utxo->close_info->commitment_point);
25-
towire_bool(pptr, utxo->close_info->option_anchors);
26-
towire_u32(pptr, utxo->close_info->csv);
27-
}
28-
29-
towire_bool(pptr, utxo->is_in_coinbase);
30-
}
31-
32-
struct utxo *fromwire_utxo(const tal_t *ctx, const u8 **ptr, size_t *max)
33-
{
34-
struct utxo *utxo = tal(ctx, struct utxo);
35-
36-
fromwire_bitcoin_outpoint(ptr, max, &utxo->outpoint);
37-
utxo->amount = fromwire_amount_sat(ptr, max);
38-
utxo->keyindex = fromwire_u32(ptr, max);
39-
utxo->is_p2sh = fromwire_bool(ptr, max);
40-
41-
utxo->scriptPubkey = fromwire_tal_arrn(utxo, ptr, max, fromwire_u16(ptr, max));
42-
43-
if (fromwire_bool(ptr, max)) {
44-
utxo->close_info = tal(utxo, struct unilateral_close_info);
45-
utxo->close_info->channel_id = fromwire_u64(ptr, max);
46-
fromwire_node_id(ptr, max, &utxo->close_info->peer_id);
47-
if (fromwire_bool(ptr, max)) {
48-
utxo->close_info->commitment_point = tal(utxo,
49-
struct pubkey);
50-
fromwire_pubkey(ptr, max,
51-
utxo->close_info->commitment_point);
52-
} else
53-
utxo->close_info->commitment_point = NULL;
54-
utxo->close_info->option_anchors
55-
= fromwire_bool(ptr, max);
56-
utxo->close_info->csv = fromwire_u32(ptr, max);
57-
} else {
58-
utxo->close_info = NULL;
59-
}
60-
61-
utxo->is_in_coinbase = fromwire_bool(ptr, max);
62-
return utxo;
63-
}
64-
655
size_t utxo_spend_weight(const struct utxo *utxo, size_t min_witness_weight)
666
{
677
size_t wit_weight = bitcoin_tx_simple_input_witness_weight();

common/utxo.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,6 @@ static inline bool utxo_is_csv_locked(const struct utxo *utxo, u32 current_heigh
8181
return *utxo->blockheight + utxo->close_info->csv > current_height;
8282
}
8383

84-
void towire_utxo(u8 **pptr, const struct utxo *utxo);
85-
struct utxo *fromwire_utxo(const tal_t *ctx, const u8 **ptr, size_t *max);
86-
8784
/* Estimate of (signed) UTXO weight in transaction */
8885
size_t utxo_spend_weight(const struct utxo *utxo, size_t min_witness_weight);
8986

hsmd/Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
HSMD_SRC := hsmd/hsmd.c \
44
hsmd/hsmd_wiregen.c \
5+
hsmd/hsm_utxo.c \
56
hsmd/libhsmd.c
67

78
HSMD_HEADERS := hsmd/hsmd_wiregen.h hsmd/permissions.h
@@ -12,6 +13,7 @@ $(HSMD_OBJS): $(HSMD_HEADERS)
1213
# Other programs which use the hsm need this.
1314
HSMD_CLIENT_OBJS := \
1415
hsmd/hsmd_wiregen.o \
16+
hsmd/hsm_utxo.o \
1517
common/htlc_wire.o
1618

1719
# Make sure these depend on everything.
@@ -50,7 +52,6 @@ HSMD_COMMON_OBJS := \
5052
common/status_wiregen.o \
5153
common/subdaemon.o \
5254
common/utils.o \
53-
common/utxo.o \
5455
common/version.o \
5556
common/wireaddr.o
5657

hsmd/hsm_utxo.c

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
#include "config.h"
2+
#include <hsmd/hsm_utxo.h>
3+
#include <wire/wire.h>
4+
5+
static const struct hsm_utxo *to_hsm_utxo(const tal_t *ctx,
6+
const struct utxo *utxo)
7+
{
8+
struct hsm_utxo *hutxo = tal(ctx, struct hsm_utxo);
9+
10+
hutxo->outpoint = utxo->outpoint;
11+
hutxo->amount = utxo->amount;
12+
hutxo->keyindex = utxo->keyindex;
13+
14+
if (utxo->close_info) {
15+
hutxo->close_info
16+
= tal_dup(hutxo, struct unilateral_close_info,
17+
utxo->close_info);
18+
if (hutxo->close_info->commitment_point)
19+
hutxo->close_info->commitment_point
20+
= tal_dup(hutxo->close_info,
21+
struct pubkey,
22+
hutxo->close_info->commitment_point);
23+
} else
24+
hutxo->close_info = NULL;
25+
26+
if (utxo->scriptPubkey)
27+
hutxo->scriptPubkey = tal_dup_talarr(hutxo, u8, utxo->scriptPubkey);
28+
else
29+
hutxo->scriptPubkey = NULL;
30+
31+
return hutxo;
32+
}
33+
34+
const struct hsm_utxo **utxos_to_hsm_utxos(const tal_t *ctx,
35+
struct utxo **utxos)
36+
{
37+
const struct hsm_utxo **hutxos
38+
= tal_arr(ctx, const struct hsm_utxo *, tal_count(utxos));
39+
40+
for (size_t i = 0; i < tal_count(hutxos); i++)
41+
hutxos[i] = to_hsm_utxo(hutxos, utxos[i]);
42+
return hutxos;
43+
}
44+
45+
void towire_hsm_utxo(u8 **pptr, const struct hsm_utxo *utxo)
46+
{
47+
/* Is this a unilateral close output and needs the
48+
* close_info? */
49+
bool is_unilateral_close = utxo->close_info != NULL;
50+
towire_bitcoin_outpoint(pptr, &utxo->outpoint);
51+
towire_amount_sat(pptr, utxo->amount);
52+
towire_u32(pptr, utxo->keyindex);
53+
/* Used to be ->is_p2sh, but HSM uses scriptpubkey to determine type */
54+
towire_bool(pptr, false);
55+
56+
towire_u16(pptr, tal_count(utxo->scriptPubkey));
57+
towire_u8_array(pptr, utxo->scriptPubkey, tal_count(utxo->scriptPubkey));
58+
59+
towire_bool(pptr, is_unilateral_close);
60+
if (is_unilateral_close) {
61+
towire_u64(pptr, utxo->close_info->channel_id);
62+
towire_node_id(pptr, &utxo->close_info->peer_id);
63+
towire_bool(pptr, utxo->close_info->commitment_point != NULL);
64+
if (utxo->close_info->commitment_point)
65+
towire_pubkey(pptr, utxo->close_info->commitment_point);
66+
towire_bool(pptr, utxo->close_info->option_anchors);
67+
towire_u32(pptr, utxo->close_info->csv);
68+
}
69+
70+
/* Used to be ->is_in_coinbase, but HSM doesn't care */
71+
towire_bool(pptr, false);
72+
}
73+
74+
struct hsm_utxo *fromwire_hsm_utxo(const tal_t *ctx, const u8 **ptr, size_t *max)
75+
{
76+
struct hsm_utxo *utxo = tal(ctx, struct hsm_utxo);
77+
78+
fromwire_bitcoin_outpoint(ptr, max, &utxo->outpoint);
79+
utxo->amount = fromwire_amount_sat(ptr, max);
80+
utxo->keyindex = fromwire_u32(ptr, max);
81+
fromwire_bool(ptr, max);
82+
83+
utxo->scriptPubkey = fromwire_tal_arrn(utxo, ptr, max, fromwire_u16(ptr, max));
84+
85+
if (fromwire_bool(ptr, max)) {
86+
utxo->close_info = tal(utxo, struct unilateral_close_info);
87+
utxo->close_info->channel_id = fromwire_u64(ptr, max);
88+
fromwire_node_id(ptr, max, &utxo->close_info->peer_id);
89+
if (fromwire_bool(ptr, max)) {
90+
utxo->close_info->commitment_point = tal(utxo,
91+
struct pubkey);
92+
fromwire_pubkey(ptr, max,
93+
utxo->close_info->commitment_point);
94+
} else
95+
utxo->close_info->commitment_point = NULL;
96+
utxo->close_info->option_anchors
97+
= fromwire_bool(ptr, max);
98+
utxo->close_info->csv = fromwire_u32(ptr, max);
99+
} else {
100+
utxo->close_info = NULL;
101+
}
102+
103+
fromwire_bool(ptr, max);
104+
return utxo;
105+
}

hsmd/hsm_utxo.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#ifndef LIGHTNING_HSMD_HSM_UTXO_H
2+
#define LIGHTNING_HSMD_HSM_UTXO_H
3+
#include "config.h"
4+
#include <bitcoin/tx.h>
5+
#include <common/amount.h>
6+
#include <common/utxo.h>
7+
8+
/* FIXME: If we make our static_remotekey a normal keypath key, we can
9+
* simply put that close information inside the PSBT, and we don't
10+
* need to hand the utxo to hsmd at all. */
11+
12+
/* /!\ This is part of the HSM ABI: do not change! /!\ */
13+
struct hsm_utxo {
14+
struct bitcoin_outpoint outpoint;
15+
struct amount_sat amount;
16+
u32 keyindex;
17+
18+
/* Optional unilateral close information, NULL if this is just
19+
* a HD key */
20+
struct unilateral_close_info *close_info;
21+
22+
/* The scriptPubkey if it is known */
23+
u8 *scriptPubkey;
24+
};
25+
26+
void towire_hsm_utxo(u8 **pptr, const struct hsm_utxo *utxo);
27+
struct hsm_utxo *fromwire_hsm_utxo(const tal_t *ctx, const u8 **ptr, size_t *max);
28+
29+
const struct hsm_utxo **utxos_to_hsm_utxos(const tal_t *ctx,
30+
struct utxo **utxos);
31+
#endif /* LIGHTNING_HSMD_HSM_UTXO_H */

hsmd/hsmd_wire.csv

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ msgdata,hsmd_forget_channel,dbid,u64,
118118
msgtype,hsmd_forget_channel_reply,134
119119

120120
# Return signature for a funding tx.
121-
#include <common/utxo.h>
121+
#include <hsmd/hsm_utxo.h>
122122

123123
# Master asks the HSM to sign a node_announcement
124124
msgtype,hsmd_node_announcement_sig_req,6
@@ -132,7 +132,7 @@ msgdata,hsmd_node_announcement_sig_reply,signature,secp256k1_ecdsa_signature,
132132
#include <bitcoin/psbt.h>
133133
msgtype,hsmd_sign_withdrawal,7
134134
msgdata,hsmd_sign_withdrawal,num_inputs,u16,
135-
msgdata,hsmd_sign_withdrawal,inputs,utxo,num_inputs
135+
msgdata,hsmd_sign_withdrawal,inputs,hsm_utxo,num_inputs
136136
msgdata,hsmd_sign_withdrawal,psbt,wally_psbt,
137137

138138
msgtype,hsmd_sign_withdrawal_reply,107
@@ -425,7 +425,7 @@ msgtype,hsmd_sign_anchorspend,147
425425
msgdata,hsmd_sign_anchorspend,peerid,node_id,
426426
msgdata,hsmd_sign_anchorspend,channel_dbid,u64,
427427
msgdata,hsmd_sign_anchorspend,num_inputs,u16,
428-
msgdata,hsmd_sign_anchorspend,inputs,utxo,num_inputs
428+
msgdata,hsmd_sign_anchorspend,inputs,hsm_utxo,num_inputs
429429
msgdata,hsmd_sign_anchorspend,psbt,wally_psbt,
430430

431431
msgtype,hsmd_sign_anchorspend_reply,148
@@ -474,7 +474,7 @@ msgtype,hsmd_sign_htlc_tx_mingle,149
474474
msgdata,hsmd_sign_htlc_tx_mingle,peerid,node_id,
475475
msgdata,hsmd_sign_htlc_tx_mingle,channel_dbid,u64,
476476
msgdata,hsmd_sign_htlc_tx_mingle,num_inputs,u16,
477-
msgdata,hsmd_sign_htlc_tx_mingle,inputs,utxo,num_inputs
477+
msgdata,hsmd_sign_htlc_tx_mingle,inputs,hsm_utxo,num_inputs
478478
msgdata,hsmd_sign_htlc_tx_mingle,psbt,wally_psbt,
479479

480480
msgtype,hsmd_sign_htlc_tx_mingle_reply,150

hsmd/libhsmd.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ static void bitcoin_key(struct privkey *privkey, struct pubkey *pubkey,
527527

528528
/* This gets the bitcoin private key needed to spend from our wallet */
529529
static void hsm_key_for_utxo(struct privkey *privkey, struct pubkey *pubkey,
530-
const struct utxo *utxo)
530+
const struct hsm_utxo *utxo)
531531
{
532532
if (utxo->close_info != NULL) {
533533
/* 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,
545545

546546
/* Find our inputs by the pubkey associated with the inputs, and
547547
* add a partial sig for each */
548-
static void sign_our_inputs(struct utxo **utxos, struct wally_psbt *psbt)
548+
static void sign_our_inputs(struct hsm_utxo **utxos, struct wally_psbt *psbt)
549549
{
550550
bool is_cache_enabled = false;
551551
for (size_t i = 0; i < tal_count(utxos); i++) {
552-
struct utxo *utxo = utxos[i];
552+
struct hsm_utxo *utxo = utxos[i];
553553
for (size_t j = 0; j < psbt->num_inputs; j++) {
554554
struct privkey privkey;
555555
struct pubkey pubkey;
@@ -1315,11 +1315,11 @@ static u8 *handle_get_per_commitment_point(struct hsmd_client *c, const u8 *msg_
13151315
* we can do more to check the previous case is valid. */
13161316
static u8 *handle_sign_withdrawal_tx(struct hsmd_client *c, const u8 *msg_in)
13171317
{
1318-
struct utxo **utxos;
1318+
struct hsm_utxo **utxos;
13191319
struct wally_psbt *psbt;
13201320

13211321
if (!fromwire_hsmd_sign_withdrawal(tmpctx, msg_in,
1322-
&utxos, &psbt))
1322+
&utxos, &psbt))
13231323
return hsmd_status_malformed_request(c, msg_in);
13241324

13251325
sign_our_inputs(utxos, psbt);
@@ -1705,7 +1705,7 @@ static u8 *handle_sign_anchorspend(struct hsmd_client *c, const u8 *msg_in)
17051705
{
17061706
struct node_id peer_id;
17071707
u64 dbid;
1708-
struct utxo **utxos;
1708+
struct hsm_utxo **utxos;
17091709
struct wally_psbt *psbt;
17101710
struct secret seed;
17111711
struct pubkey local_funding_pubkey;
@@ -1744,7 +1744,7 @@ static u8 *handle_sign_htlc_tx_mingle(struct hsmd_client *c, const u8 *msg_in)
17441744
{
17451745
struct node_id peer_id;
17461746
u64 dbid;
1747-
struct utxo **utxos;
1747+
struct hsm_utxo **utxos;
17481748
struct wally_psbt *psbt;
17491749

17501750
/* FIXME: Check output goes to us. */

lightningd/anchorspend.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx,
327327
struct amount_sat psbt_fee, diff;
328328
struct bitcoin_tx *tx;
329329
struct utxo **psbt_utxos;
330+
const struct hsm_utxo **hsm_utxos;
330331
struct wally_psbt *psbt, *signed_psbt;
331332
struct amount_msat total_value;
332333
const struct deadline_value *unimportant_deadline;
@@ -491,11 +492,11 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx,
491492
* 1000 / psbt_weight);
492493

493494
/* OK, HSM, sign it! */
495+
hsm_utxos = utxos_to_hsm_utxos(tmpctx, psbt_utxos);
494496
msg = towire_hsmd_sign_anchorspend(NULL,
495497
&channel->peer->id,
496498
channel->dbid,
497-
cast_const2(const struct utxo **,
498-
psbt_utxos),
499+
hsm_utxos,
499500
psbt);
500501
msg = hsm_sync_req(tmpctx, ld, take(msg));
501502
if (!fromwire_hsmd_sign_anchorspend_reply(tmpctx, msg, &signed_psbt))

lightningd/onchain_control.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,7 @@ static bool consider_onchain_htlc_tx_rebroadcast(struct channel *channel,
10561056
{
10571057
struct amount_sat change, excess;
10581058
struct utxo **utxos;
1059+
const struct hsm_utxo **hsm_utxos;
10591060
u32 feerate;
10601061
size_t weight;
10611062
struct bitcoin_tx *newtx;
@@ -1140,11 +1141,11 @@ static bool consider_onchain_htlc_tx_rebroadcast(struct channel *channel,
11401141
}
11411142

11421143
/* Now, get HSM to sign off. */
1144+
hsm_utxos = utxos_to_hsm_utxos(tmpctx, utxos);
11431145
msg = towire_hsmd_sign_htlc_tx_mingle(NULL,
11441146
&channel->peer->id,
11451147
channel->dbid,
1146-
cast_const2(const struct utxo **,
1147-
utxos),
1148+
hsm_utxos,
11481149
psbt);
11491150
msg = hsm_sync_req(tmpctx, ld, take(msg));
11501151
if (!fromwire_hsmd_sign_htlc_tx_mingle_reply(tmpctx, msg, &psbt))

0 commit comments

Comments
 (0)