diff --git a/bitcoin/tx.c b/bitcoin/tx.c index cee06b032b40..bb9aa3bd8061 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -893,7 +893,8 @@ size_t bitcoin_tx_input_sig_weight(void) /* Input weight */ size_t bitcoin_tx_input_weight(bool p2sh, size_t witness_weight) { - size_t weight = witness_weight; + /* We assume < 253 witness elements */ + size_t weight = 1 + witness_weight; /* Input weight: txid + index + sequence */ weight += (32 + 4 + 4) * 4; @@ -912,17 +913,32 @@ size_t bitcoin_tx_input_weight(bool p2sh, size_t witness_weight) return weight; } -size_t bitcoin_tx_simple_input_witness_weight(void) -{ - /* Account for witness (1 byte count + sig + key) */ - return 1 + (bitcoin_tx_input_sig_weight() + 1 + 33); -} - -/* We only do segwit inputs, and we assume witness is sig + key */ -size_t bitcoin_tx_simple_input_weight(bool p2sh) -{ - return bitcoin_tx_input_weight(p2sh, - bitcoin_tx_simple_input_witness_weight()); +size_t bitcoin_tx_input_witness_weight(enum utxotype utxotype) +{ + switch (utxotype) { + case UTXO_P2SH_P2WPKH: + case UTXO_P2WPKH: + /* Account for witness (sig + key) */ + return bitcoin_tx_input_sig_weight() + 1 + 33; + case UTXO_P2WSH_FROM_CLOSE: + /* BOLT #3: + * #### `to_remote` Output + * + * If `option_anchors` applies to the commitment + * transaction, the `to_remote` output is encumbered by a one + * block csv lock. + * OP_CHECKSIGVERIFY 1 OP_CHECKSEQUENCEVERIFY + * + * The output is spent by an input with `nSequence` field set + * to `1` and witness: + * Otherwise, this output is a simple P2WPKH to `remotepubkey`. + */ + /* In practice, these predate anchors, so: */ + return 1 + 1 + bitcoin_tx_input_sig_weight(); + case UTXO_P2TR: + return 1 + 64; + } + abort(); } size_t bitcoin_tx_2of2_input_witness_weight(void) diff --git a/bitcoin/tx.h b/bitcoin/tx.h index 860b635ff741..fc6844035fc1 100644 --- a/bitcoin/tx.h +++ b/bitcoin/tx.h @@ -48,6 +48,17 @@ struct bitcoin_tx_output { u8 *script; }; +enum utxotype { + /* Obsolete: we used to have P2SH-wrapped outputs (removed in 24.02, though can still have old UTXOs) */ + UTXO_P2SH_P2WPKH = 1, + /* "bech32" addresses */ + UTXO_P2WPKH = 2, + /* Used for closing addresses: implies ->close_info is non-NULL */ + UTXO_P2WSH_FROM_CLOSE = 3, + /* "p2tr" addresses. */ + UTXO_P2TR = 4, +}; + struct bitcoin_tx_output *new_tx_output(const tal_t *ctx, struct amount_sat amount, const u8 *script); @@ -315,14 +326,13 @@ size_t bitcoin_tx_output_weight(size_t outscript_len); /* Weight to push sig on stack. */ size_t bitcoin_tx_input_sig_weight(void); -/* Segwit input, but with parameter for witness weight (size) */ +/* Segwit input, but with parameter for witness weight (size). + * witness_weight must include the varint_size() for each witness element, + * but not the varint_size() for the number of elements. */ size_t bitcoin_tx_input_weight(bool p2sh, size_t witness_weight); -/* The witness weight for a simple (sig + key) input */ -size_t bitcoin_tx_simple_input_witness_weight(void); - -/* We only do segwit inputs, and we assume witness is sig + key */ -size_t bitcoin_tx_simple_input_weight(bool p2sh); +/* The witness weight */ +size_t bitcoin_tx_input_witness_weight(enum utxotype utxotype); /* The witness for our 2of2 input (closing or commitment tx). */ size_t bitcoin_tx_2of2_input_witness_weight(void); diff --git a/channeld/channeld.c b/channeld/channeld.c index 5db9df60194f..ca20eb990e6e 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -912,23 +912,22 @@ static u8 *master_wait_sync_reply(const tal_t *ctx, return reply; } -/* Collect the htlcs for call to hsmd. */ -static struct simple_htlc **collect_htlcs(const tal_t *ctx, const struct htlc **htlc_map) +static struct hsm_htlc *collect_htlcs(const tal_t *ctx, + const struct htlc **htlc_map) { - struct simple_htlc **htlcs; + struct hsm_htlc *htlcs; - htlcs = tal_arr(ctx, struct simple_htlc *, 0); + htlcs = tal_arr(ctx, struct hsm_htlc, 0); 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) { - struct simple_htlc *simple = - new_simple_htlc(htlcs, - htlc_state_owner(hh->state), - hh->amount, - &hh->rhash, - hh->expiry.locktime); - tal_arr_expand(&htlcs, simple); + struct hsm_htlc htlc; + htlc.side = htlc_state_owner(hh->state); + htlc.amount = hh->amount; + htlc.payment_hash = hh->rhash; + htlc.cltv_expiry = hh->expiry.locktime; + tal_arr_expand(&htlcs, htlc); } } return htlcs; @@ -946,7 +945,7 @@ static struct bitcoin_signature *calc_commitsigs(const tal_t *ctx, struct bitcoin_signature *commit_sig, struct pubkey remote_funding_pubkey) { - struct simple_htlc **htlcs; + struct hsm_htlc *htlcs; size_t i; struct pubkey local_htlckey; const u8 *msg; @@ -959,7 +958,7 @@ static struct bitcoin_signature *calc_commitsigs(const tal_t *ctx, channel_has(peer->channel, OPT_STATIC_REMOTEKEY), commit_index, - (const struct simple_htlc **) htlcs, + htlcs, channel_feerate(peer->channel, REMOTE)); msg = hsm_req(tmpctx, take(msg)); @@ -1200,7 +1199,10 @@ static u8 *send_commit_part(const tal_t *ctx, *anchor = tal(ctx, struct local_anchor_info); bitcoin_txid(txs[0], &(*anchor)->anchor_point.txid); (*anchor)->anchor_point.n = local_anchor_outnum; - (*anchor)->commitment_weight = bitcoin_tx_weight(txs[0]); + /* Actual weight will include witnesses! Note: this assumes + * 73 byte sigs (worst case as per BOLT), whereas we grind to + * 71, and so does everyone else. */ + (*anchor)->commitment_weight = bitcoin_tx_weight(txs[0]) + bitcoin_tx_2of2_input_witness_weight() - 4; (*anchor)->commitment_fee = bitcoin_tx_compute_fee(txs[0]); } @@ -1883,7 +1885,7 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, const struct htlc **htlc_map; const u8 *funding_wscript; size_t i; - struct simple_htlc **htlcs; + const struct hsm_htlc *htlcs; const u8 * msg2; u8 *splice_msg; int type; @@ -2091,7 +2093,7 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, htlcs = collect_htlcs(NULL, htlc_map); msg2 = towire_hsmd_validate_commitment_tx(NULL, txs[0], - (const struct simple_htlc **) htlcs, + htlcs, local_index, channel_feerate(peer->channel, LOCAL), &commit_sig, diff --git a/common/hsm_version.h b/common/hsm_version.h index e4c4cecf2e7e..b03fb6496c7c 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 with dev_warn_on_overgrind: a273b68e19336073e551c01a78bcd1e1f8cc510da7d0dde3afc45e249f9830cc */ #define HSM_MIN_VERSION 5 #define HSM_MAX_VERSION 6 diff --git a/common/htlc_wire.c b/common/htlc_wire.c index 22735388c653..aa3ef92f1d73 100644 --- a/common/htlc_wire.c +++ b/common/htlc_wire.c @@ -24,20 +24,6 @@ static struct failed_htlc *failed_htlc_dup(const tal_t *ctx, return newf; } -struct simple_htlc *new_simple_htlc(const tal_t *ctx, - enum side side, - struct amount_msat amount, - const struct sha256 *payment_hash, - u32 cltv_expiry) -{ - struct simple_htlc *simple = tal(ctx, struct simple_htlc); - simple->side = side; - simple->amount = amount; - simple->payment_hash = *payment_hash; - simple->cltv_expiry = cltv_expiry; - return simple; -} - struct existing_htlc *new_existing_htlc(const tal_t *ctx, u64 id, enum htlc_state state, @@ -113,14 +99,6 @@ void towire_existing_htlc(u8 **pptr, const struct existing_htlc *existing) towire_bool(pptr, false); } -void towire_simple_htlc(u8 **pptr, const struct simple_htlc *simple) -{ - towire_side(pptr, simple->side); - towire_amount_msat(pptr, simple->amount); - towire_sha256(pptr, &simple->payment_hash); - towire_u32(pptr, simple->cltv_expiry); -} - void towire_fulfilled_htlc(u8 **pptr, const struct fulfilled_htlc *fulfilled) { towire_u64(pptr, fulfilled->id); @@ -217,18 +195,6 @@ struct existing_htlc *fromwire_existing_htlc(const tal_t *ctx, return existing; } -struct simple_htlc *fromwire_simple_htlc(const tal_t *ctx, - const u8 **cursor, size_t *max) -{ - struct simple_htlc *simple = tal(ctx, struct simple_htlc); - - simple->side = fromwire_side(cursor, max); - simple->amount = fromwire_amount_msat(cursor, max); - fromwire_sha256(cursor, max, &simple->payment_hash); - simple->cltv_expiry = fromwire_u32(cursor, max); - return simple; -} - void fromwire_fulfilled_htlc(const u8 **cursor, size_t *max, struct fulfilled_htlc *fulfilled) { diff --git a/common/htlc_wire.h b/common/htlc_wire.h index c2026a8c42c0..4d758a649028 100644 --- a/common/htlc_wire.h +++ b/common/htlc_wire.h @@ -60,14 +60,6 @@ struct changed_htlc { u64 id; }; -/* For signing interfaces */ -struct simple_htlc { - enum side side; - struct amount_msat amount; - struct sha256 payment_hash; - u32 cltv_expiry; -}; - struct existing_htlc *new_existing_htlc(const tal_t *ctx, u64 id, enum htlc_state state, @@ -79,15 +71,8 @@ struct existing_htlc *new_existing_htlc(const tal_t *ctx, const struct preimage *preimage TAKES, const struct failed_htlc *failed TAKES); -struct simple_htlc *new_simple_htlc(const tal_t *ctx, - enum side side, - struct amount_msat amount, - const struct sha256 *payment_hash, - u32 cltv_expiry); - void towire_added_htlc(u8 **pptr, const struct added_htlc *added); void towire_existing_htlc(u8 **pptr, const struct existing_htlc *existing); -void towire_simple_htlc(u8 **pptr, const struct simple_htlc *simple); void towire_fulfilled_htlc(u8 **pptr, const struct fulfilled_htlc *fulfilled); void towire_failed_htlc(u8 **pptr, const struct failed_htlc *failed); void towire_changed_htlc(u8 **pptr, const struct changed_htlc *changed); @@ -98,8 +83,6 @@ void fromwire_added_htlc(const u8 **cursor, size_t *max, struct added_htlc *added); struct existing_htlc *fromwire_existing_htlc(const tal_t *ctx, const u8 **cursor, size_t *max); -struct simple_htlc *fromwire_simple_htlc(const tal_t *ctx, - const u8 **cursor, size_t *max); void fromwire_fulfilled_htlc(const u8 **cursor, size_t *max, struct fulfilled_htlc *fulfilled); struct failed_htlc *fromwire_failed_htlc(const tal_t *ctx, const u8 **cursor, diff --git a/common/utxo.c b/common/utxo.c index a28e99cd3b79..93ec50d3e897 100644 --- a/common/utxo.c +++ b/common/utxo.c @@ -2,76 +2,20 @@ #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) +size_t utxo_spend_weight(const struct utxo *utxo, size_t min_witness_weight) { - 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)); + size_t witness_weight; + bool p2sh = (utxo->utxotype == UTXO_P2SH_P2WPKH); - 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; - } + witness_weight = bitcoin_tx_input_witness_weight(utxo->utxotype); - 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(); /* If the min is less than what we'd use for a 'normal' tx, * we return the value with the greater added/calculated */ - if (wit_weight < min_witness_weight) - return bitcoin_tx_input_weight(utxo->is_p2sh, + if (witness_weight < min_witness_weight) + return bitcoin_tx_input_weight(p2sh, min_witness_weight); - return bitcoin_tx_input_weight(utxo->is_p2sh, wit_weight); + return bitcoin_tx_input_weight(p2sh, witness_weight); } u32 utxo_is_immature(const struct utxo *utxo, u32 blockheight) @@ -91,3 +35,18 @@ u32 utxo_is_immature(const struct utxo *utxo, u32 blockheight) return 0; } } + +const char *utxotype_to_str(enum utxotype utxotype) +{ + switch (utxotype) { + case UTXO_P2SH_P2WPKH: + return "p2sh_p2wpkh"; + case UTXO_P2WPKH: + return "p2wpkh"; + case UTXO_P2WSH_FROM_CLOSE: + return "p2wsh_from_close"; + case UTXO_P2TR: + return "p2tr"; + } + abort(); +} diff --git a/common/utxo.h b/common/utxo.h index 830b865762d4..aa91ee5100dc 100644 --- a/common/utxo.h +++ b/common/utxo.h @@ -31,11 +31,13 @@ enum output_status { OUTPUT_STATE_ANY = 255 }; +const char *utxotype_to_str(enum utxotype utxotype); + struct utxo { struct bitcoin_outpoint outpoint; struct amount_sat amount; u32 keyindex; - bool is_p2sh; + enum utxotype utxotype; enum output_status status; /* Optional unilateral close information, NULL if this is just @@ -81,9 +83,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/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json index fb6040e3e1aa..5c66d1457953 100644 --- a/contrib/msggen/msggen/schema.json +++ b/contrib/msggen/msggen/schema.json @@ -14889,8 +14889,8 @@ "response": { "psbt": "cHNidP8BAgQCAAAAAQMEbwAAAAEEAQpsbt310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000", "feerate_per_kw": 253, - "estimated_final_weight": 693, - "excess_msat": 196962507000, + "estimated_final_weight": 652, + "excess_msat": 196962518000, "change_outnum": 0 } }, @@ -14910,7 +14910,7 @@ "response": { "psbt": "cHNidP8BAgQCAAAAAQMEbwAAAAEEAQpsbt410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000", "feerate_per_kw": 11000, - "estimated_final_weight": 612, + "estimated_final_weight": 613, "excess_msat": 0, "change_outnum": 0 } diff --git a/devtools/mkfunding.c b/devtools/mkfunding.c index 2bd7cb24f63f..3191721312ba 100644 --- a/devtools/mkfunding.c +++ b/devtools/mkfunding.c @@ -42,7 +42,7 @@ static struct bitcoin_tx *tx_spending_utxo(const tal_t *ctx, struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, 1, num_output, nlocktime); - assert(!utxo->is_p2sh); + assert(utxo->utxotype != UTXO_P2SH_P2WPKH); bitcoin_tx_add_input(tx, &utxo->outpoint, nsequence, NULL, utxo->amount, utxo->scriptPubkey, NULL); @@ -95,7 +95,7 @@ int main(int argc, char *argv[]) if (argc != 1 + 7) errx(1, "Usage: mkfunding "); - input.is_p2sh = false; + input.utxotype = UTXO_P2WPKH; input.close_info = NULL; argnum = 1; diff --git a/doc/schemas/fundpsbt.json b/doc/schemas/fundpsbt.json index 2771efb170f2..5416ba46fee3 100644 --- a/doc/schemas/fundpsbt.json +++ b/doc/schemas/fundpsbt.json @@ -222,8 +222,8 @@ "response": { "psbt": "cHNidP8BAgQCAAAAAQMEbwAAAAEEAQpsbt310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000310000", "feerate_per_kw": 253, - "estimated_final_weight": 693, - "excess_msat": 196962507000, + "estimated_final_weight": 652, + "excess_msat": 196962518000, "change_outnum": 0 } }, @@ -243,7 +243,7 @@ "response": { "psbt": "cHNidP8BAgQCAAAAAQMEbwAAAAEEAQpsbt410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000410000", "feerate_per_kw": 11000, - "estimated_final_weight": 612, + "estimated_final_weight": 613, "excess_msat": 0, "change_outnum": 0 } diff --git a/hsmd/Makefile b/hsmd/Makefile index 8a788cef5715..b4f51bef2ffd 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 000000000000..1a55aa575ffc --- /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 000000000000..62bea00262ec --- /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.c b/hsmd/hsmd.c index 16bf1e09ac37..6a5cc487b90d 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -442,8 +442,11 @@ static struct io_plan *preinit_hsm(struct io_conn *conn, if (tlv->no_preapprove_check) dev_no_preapprove_check = *tlv->no_preapprove_check; - status_debug("preinit: dev_fail_preapprove = %u, dev_no_preapprove_check = %u", - dev_fail_preapprove, dev_no_preapprove_check); + if (tlv->warn_on_overgrind) + dev_warn_on_overgrind = *tlv->warn_on_overgrind; + + status_debug("preinit: dev_fail_preapprove = %u, dev_no_preapprove_check = %u, dev_warn_on_overgrind = %u", + dev_fail_preapprove, dev_no_preapprove_check, dev_warn_on_overgrind); /* We don't send a reply, just read next */ return client_read_next(conn, c); } diff --git a/hsmd/hsmd_wire.csv b/hsmd/hsmd_wire.csv index 98faee7d4775..b758eea28e3a 100644 --- a/hsmd/hsmd_wire.csv +++ b/hsmd/hsmd_wire.csv @@ -14,6 +14,8 @@ tlvtype,hsmd_dev_preinit_tlvs,fail_preapprove,1 tlvdata,hsmd_dev_preinit_tlvs,fail_preapprove,fail,bool, tlvtype,hsmd_dev_preinit_tlvs,no_preapprove_check,3 tlvdata,hsmd_dev_preinit_tlvs,no_preapprove_check,disable,bool, +tlvtype,hsmd_dev_preinit_tlvs,warn_on_overgrind,5 +tlvdata,hsmd_dev_preinit_tlvs,warn_on_overgrind,enable,bool, #include # Start the HSM. @@ -118,7 +120,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 +134,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 @@ -229,11 +231,18 @@ msgdata,hsmd_sign_commitment_tx,commit_num,u64, msgtype,hsmd_sign_commitment_tx_reply,105 msgdata,hsmd_sign_commitment_tx_reply,sig,bitcoin_signature, +#include // For enum side and towire_side +subtype,hsm_htlc +subtypedata,hsm_htlc,side,enum side, +subtypedata,hsm_htlc,amount,amount_msat, +subtypedata,hsm_htlc,payment_hash,sha256, +subtypedata,hsm_htlc,cltv_expiry,u32, + # Validate the counterparty's commitment signatures. msgtype,hsmd_validate_commitment_tx,35 msgdata,hsmd_validate_commitment_tx,tx,bitcoin_tx, msgdata,hsmd_validate_commitment_tx,num_htlcs,u16, -msgdata,hsmd_validate_commitment_tx,htlcs,simple_htlc,num_htlcs +msgdata,hsmd_validate_commitment_tx,htlcs,hsm_htlc,num_htlcs msgdata,hsmd_validate_commitment_tx,commit_num,u64, msgdata,hsmd_validate_commitment_tx,feerate,u32, msgdata,hsmd_validate_commitment_tx,sig,bitcoin_signature, @@ -290,7 +299,6 @@ msgdata,hsmd_sign_local_htlc_tx,wscript,u8,wscript_len msgdata,hsmd_sign_local_htlc_tx,option_anchor_outputs,bool, # Openingd/channeld asks HSM to sign the other sides' commitment tx. -#include msgtype,hsmd_sign_remote_commitment_tx,19 msgdata,hsmd_sign_remote_commitment_tx,tx,bitcoin_tx, msgdata,hsmd_sign_remote_commitment_tx,remote_funding_key,pubkey, @@ -298,7 +306,7 @@ msgdata,hsmd_sign_remote_commitment_tx,remote_per_commit,pubkey, msgdata,hsmd_sign_remote_commitment_tx,option_static_remotekey,bool, msgdata,hsmd_sign_remote_commitment_tx,commit_num,u64, msgdata,hsmd_sign_remote_commitment_tx,num_htlcs,u16, -msgdata,hsmd_sign_remote_commitment_tx,htlcs,simple_htlc,num_htlcs +msgdata,hsmd_sign_remote_commitment_tx,htlcs,hsm_htlc,num_htlcs msgdata,hsmd_sign_remote_commitment_tx,feerate,u32, # channeld asks HSM to sign remote HTLC tx. @@ -425,7 +433,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 +482,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 d5abcc750e1e..eeaf9a6cff83 100644 --- a/hsmd/libhsmd.c +++ b/hsmd/libhsmd.c @@ -40,6 +40,7 @@ bool initialized = false; /* Do we fail all preapprove requests? */ bool dev_fail_preapprove = false; bool dev_no_preapprove_check = false; +bool dev_warn_on_overgrind = false; struct hsmd_client *hsmd_client_new_main(const tal_t *ctx, u64 capabilities, void *extra) @@ -527,7 +528,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,14 +546,15 @@ 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; + bool needed_sig; if (!wally_psbt_input_spends(&psbt->inputs[j], &utxo->outpoint)) @@ -590,6 +592,10 @@ static void sign_our_inputs(struct utxo **utxos, struct wally_psbt *psbt) wally_psbt_signing_cache_enable(psbt, 0); is_cache_enabled = true; } + + /* We watch for pre-taproot variable-length sigs */ + needed_sig = (psbt->inputs[j].signatures.num_items == 0); + if (wally_psbt_sign(psbt, privkey.secret.data, sizeof(privkey.secret.data), EC_FLAG_GRIND_R) != WALLY_OK) { @@ -602,6 +608,14 @@ static void sign_our_inputs(struct utxo **utxos, struct wally_psbt *psbt) j, fmt_pubkey(tmpctx, &pubkey), fmt_wally_psbt(tmpctx, psbt)); } + if (dev_warn_on_overgrind + && needed_sig + && psbt->inputs[j].signatures.num_items == 1 + && psbt->inputs[j].signatures.items[0].value_len < 71) { + hsmd_status_fmt(LOG_BROKEN, NULL, + "overgrind: short signature length %zu", + psbt->inputs[j].signatures.items[0].value_len); + } tal_wally_end(psbt); } } @@ -1315,11 +1329,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); @@ -1584,7 +1598,7 @@ static u8 *handle_sign_remote_commitment_tx(struct hsmd_client *c, const u8 *msg struct pubkey remote_per_commit; bool option_static_remotekey; u64 commit_num; - struct simple_htlc **htlc; + struct hsm_htlc *htlc; u32 feerate; if (!fromwire_hsmd_sign_remote_commitment_tx(tmpctx, msg_in, @@ -1705,7 +1719,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 +1758,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. */ @@ -1820,7 +1834,7 @@ static u8 *handle_sign_commitment_tx(struct hsmd_client *c, const u8 *msg_in) static u8 *handle_validate_commitment_tx(struct hsmd_client *c, const u8 *msg_in) { struct bitcoin_tx *tx; - struct simple_htlc **htlc; + struct hsm_htlc *htlc; u64 commit_num; u32 feerate; struct bitcoin_signature sig; diff --git a/hsmd/libhsmd.h b/hsmd/libhsmd.h index 33a6dfadc085..db927284bc3b 100644 --- a/hsmd/libhsmd.h +++ b/hsmd/libhsmd.h @@ -100,4 +100,6 @@ extern struct secret *dev_force_bip32_seed; extern bool dev_fail_preapprove; /* If they specify --dev-no-preapprove-check it ends up in here. */ extern bool dev_no_preapprove_check; +/* If they specify --dev-warn-on-overgrind it ends up in here. */ +extern bool dev_warn_on_overgrind; #endif /* LIGHTNING_HSMD_LIBHSMD_H */ diff --git a/lightningd/anchorspend.c b/lightningd/anchorspend.c index 9531c61d7d84..2dbe10bb09fe 100644 --- a/lightningd/anchorspend.c +++ b/lightningd/anchorspend.c @@ -232,13 +232,14 @@ struct anchor_details *create_anchor_details(const tal_t *ctx, return adet; } -/* total_weight includes the commitment tx we're trying to push! */ +/* total_weight includes the commitment tx we're trying to push, and the anchor fee output */ static struct wally_psbt *anchor_psbt(const tal_t *ctx, struct channel *channel, const struct one_anchor *anch, struct utxo **utxos, u32 feerate_target, - size_t total_weight) + size_t total_weight, + bool insufficient_funds) { struct lightningd *ld = channel->peer->ld; struct wally_psbt *psbt; @@ -263,12 +264,23 @@ static struct wally_psbt *anchor_psbt(const tal_t *ctx, AMOUNT_SAT(330)); psbt_input_add_pubkey(psbt, psbt->num_inputs - 1, &channel->local_funding_pubkey, false); - /* A zero-output tx is invalid: we must have change, even if not really economic */ + /* Calculate fee we need, given rate and weight */ + fee = amount_tx_fee(feerate_target, total_weight); + /* Some fee already paid by commitment tx */ + if (!amount_sat_sub(&fee, fee, anch->info.commitment_fee)) + fee = AMOUNT_SAT(0); + + /* How much do we have? */ change = psbt_compute_fee(psbt); - /* Assume we add a change output, what would the total fee be? */ - fee = amount_tx_fee(feerate_target, total_weight + change_weight()); + + /* We have to pay dust, at least! */ if (!amount_sat_sub(&change, change, fee) || amount_sat_less(change, chainparams->dust_limit)) { + /* If we didn't run out of UTXOs, this implies our estimation was wrong! */ + if (!insufficient_funds) + log_broken(channel->log, "anchor: could not afford fee %s from change %s, reducing fee", + fmt_amount_sat(tmpctx, fee), + fmt_amount_sat(tmpctx, psbt_compute_fee(psbt))); change = chainparams->dust_limit; } @@ -293,19 +305,22 @@ static struct wally_psbt *try_anchor_psbt(const tal_t *ctx, struct lightningd *ld = channel->peer->ld; struct wally_psbt *psbt; struct amount_sat fee; + bool insufficient_funds; - /* Ask for some UTXOs which could meet this feerate */ + /* Ask for some UTXOs which could meet this feerate, and since + * we need one output, meet the minumum output requirement */ *total_weight = base_weight; *utxos = wallet_utxo_boost(ctx, ld->wallet, get_block_height(ld->topology), anch->info.commitment_fee, + chainparams->dust_limit, feerate_target, - total_weight); + total_weight, &insufficient_funds); /* Create a new candidate PSBT */ psbt = anchor_psbt(ctx, channel, anch, *utxos, feerate_target, - *total_weight); + *total_weight, insufficient_funds); *fee_spent = psbt_compute_fee(psbt); /* Add in base commitment fee to calculate *overall* package feerate */ @@ -327,23 +342,28 @@ 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; const u8 *msg; /* Estimate weight of anchorspend tx plus commitment_tx (not including any UTXO we add) */ - base_weight = bitcoin_tx_core_weight(2, 1) + base_weight = bitcoin_tx_core_weight(1, 1) + bitcoin_tx_input_weight(false, bitcoin_tx_input_sig_weight() + 1 + tal_bytelen(anch->adet->anchor_wscript)) - + bitcoin_tx_output_weight(BITCOIN_SCRIPTPUBKEY_P2WPKH_LEN) + + change_weight() + anch->info.commitment_weight; total_value = AMOUNT_MSAT(0); psbt = NULL; unimportant_deadline = NULL; + /* We start with furthest feerate target, and keep going backwards + * until it's either not worth making an anchor at that price for all + * the HTLCs from that point on, or we have an anchor for the closest + * HTLC deadline */ for (int i = tal_count(anch->adet->vals) - 1; i >= 0; --i) { const struct deadline_value *val = &anch->adet->vals[i]; u32 feerate, feerate_target; @@ -363,6 +383,16 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx, abort(); feerate_target = feerate_for_target(ld->topology, val->block); + + /* If the feerate for the commitment tx is already + * sufficient, don't try for anchor. */ + if (amount_feerate(&feerate, + anch->info.commitment_fee, + anch->info.commitment_weight) + && feerate >= feerate_target) { + continue; + } + candidate_psbt = try_anchor_psbt(tmpctx, channel, anch, feerate_target, base_weight, @@ -370,15 +400,17 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx, &fee, &feerate, &utxos); + log_debug(channel->log, "candidate_psbt total weight = %zu (commitment weight %u, anchor %zu)", + weight, anch->info.commitment_weight, weight - anch->info.commitment_weight); /* Is it even worth spending this fee to meet the deadline? */ if (!amount_msat_greater_sat(total_value, fee)) { log_debug(channel->log, - "Not worth fee %s for %s commit tx to get %s in %u blocks at feerate %uperkw", + "Not worth fee %s for %s commit tx to get %s at block %u (%+i) at feerate %uperkw", fmt_amount_sat(tmpctx, fee), anch->commit_side == LOCAL ? "local" : "remote", fmt_amount_msat(tmpctx, val->msat), - val->block, feerate_target); + val->block, val->block - get_block_height(ld->topology), feerate_target); break; } @@ -403,11 +435,11 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx, break; } - log_debug(channel->log, "Worth fee %s for %s commit tx to get %s in %u blocks at feerate %uperkw", + log_debug(channel->log, "Worth fee %s for %s commit tx to get %s at block %u (%+i) at feerate %uperkw", fmt_amount_sat(tmpctx, fee), anch->commit_side == LOCAL ? "local" : "remote", fmt_amount_msat(tmpctx, val->msat), - val->block, feerate); + val->block, val->block - get_block_height(ld->topology), feerate); psbt = candidate_psbt; psbt_fee = fee; psbt_weight = weight; @@ -424,23 +456,34 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx, block_target = get_block_height(ld->topology) + 12; feerate_target = feerate_for_target(ld->topology, block_target); - log_debug(channel->log, - "Low-priority anchorspend aiming for block %u (feerate %u)", - block_target, feerate_target); - psbt = try_anchor_psbt(tmpctx, channel, anch, - feerate_target, - base_weight, - &psbt_weight, - &psbt_fee, - &feerate, - &psbt_utxos); - /* Don't bother with anchor if we don't add UTXOs */ - if (tal_count(psbt_utxos) == 0) { - if (!psbt) - log_unusual(channel->log, - "No utxos to bump commit_tx to feerate %uperkw!", - feerate_target); - psbt = tal_free(psbt); + /* If the feerate for the commitment tx is already + * sufficient, don't try for anchor. */ + if (!amount_feerate(&feerate, + anch->info.commitment_fee, + anch->info.commitment_weight) + || feerate < feerate_target) { + log_debug(channel->log, + "Low-priority anchorspend aiming for block %u (feerate %u)", + block_target, feerate_target); + psbt = try_anchor_psbt(tmpctx, channel, anch, + feerate_target, + base_weight, + &psbt_weight, + &psbt_fee, + &feerate, + &psbt_utxos); + /* Don't bother with anchor if we don't add UTXOs */ + if (tal_count(psbt_utxos) == 0) { + if (!psbt) + log_unusual(channel->log, + "No utxos to bump commit_tx to feerate %uperkw!", + feerate_target); + psbt = tal_free(psbt); + } + } else { + log_debug(channel->log, + "Avoiding anchorspend: feerate already %u for target %u", + feerate, feerate_target); } } @@ -466,11 +509,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)) @@ -511,6 +554,7 @@ static struct bitcoin_tx *spend_anchor(const tal_t *ctx, tx->wtx = psbt_final_tx(tx, signed_psbt); assert(tx->wtx); tx->psbt = tal_steal(tx, signed_psbt); + log_debug(channel->log, "anchor actual weight: %zu", bitcoin_tx_weight(tx)); return tx; } diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index e45c9e51a79c..66e703d90bd7 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -485,7 +485,7 @@ void peer_start_closingd(struct channel *channel, struct peer_fd *peer_fd) ld->wallet, channel->shutdown_scriptpubkey[LOCAL], tal_bytelen(channel->shutdown_scriptpubkey[LOCAL]), - &index_val)) { + &index_val, NULL)) { if (bip32_key_from_parent( ld->bip32_base, index_val, @@ -744,7 +744,7 @@ static struct command_result *json_close(struct command *cmd, /* If they give a local address, adjust final_key_idx. */ if (!wallet_can_spend(cmd->ld->wallet, close_to_script, tal_bytelen(close_to_script), - &final_key_idx)) { + &final_key_idx, NULL)) { final_key_idx = channel->final_key_idx; } } else { diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index aa0b83d33406..8f971c5ffd13 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -696,7 +696,7 @@ openchannel2_hook_cb(struct openchannel2_payload *payload STEALS) if (wallet_can_spend(dualopend->ld->wallet, payload->our_shutdown_scriptpubkey, tal_bytelen(payload->our_shutdown_scriptpubkey), - &found_wallet_index)) { + &found_wallet_index, NULL)) { our_shutdown_script_wallet_index = tal(tmpctx, u32); *our_shutdown_script_wallet_index = found_wallet_index; } else @@ -1825,7 +1825,7 @@ static void handle_peer_tx_sigs_sent(struct subd *dualopend, !inflight->tx_broadcast) { inflight->tx_broadcast = true; - wtx = psbt_final_tx(NULL, inflight->funding_psbt); + wtx = psbt_final_tx(tmpctx, inflight->funding_psbt); if (!wtx) { channel_internal_error(channel, "Unable to extract final tx" @@ -1835,29 +1835,6 @@ static void handle_peer_tx_sigs_sent(struct subd *dualopend, return; } - /* Saves the now finalized version of the psbt */ - wallet_inflight_save(dualopend->ld->wallet, inflight); - send_funding_tx(channel, take(wtx)); - - /* Must be in an "init" state */ - assert(channel->state == DUALOPEND_OPEN_COMMITTED - || channel->state == DUALOPEND_AWAITING_LOCKIN); - - channel_set_state(channel, channel->state, - DUALOPEND_AWAITING_LOCKIN, - REASON_UNKNOWN, - "Sigs exchanged, waiting for lock-in"); - - /* Mimic the old behavior, notify a channel has been opened, - * for the accepter side */ - if (channel->opener == REMOTE) - /* Tell plugins about the success */ - notify_channel_opened(dualopend->ld, - &channel->peer->id, - &channel->funding_sats, - &channel->funding.txid, - channel->remote_channel_ready); - /* BOLT #2 * The receiving node: ... * - MUST fail the channel if: @@ -1879,7 +1856,31 @@ static void handle_peer_tx_sigs_sent(struct subd *dualopend, /* Notify the peer we're failing */ subd_send_msg(dualopend, take(towire_dualopend_fail(NULL, errmsg))); + return; } + + /* Saves the now finalized version of the psbt */ + wallet_inflight_save(dualopend->ld->wallet, inflight); + send_funding_tx(channel, take(wtx)); + + /* Must be in an "init" state */ + assert(channel->state == DUALOPEND_OPEN_COMMITTED + || channel->state == DUALOPEND_AWAITING_LOCKIN); + + channel_set_state(channel, channel->state, + DUALOPEND_AWAITING_LOCKIN, + REASON_UNKNOWN, + "Sigs exchanged, waiting for lock-in"); + + /* Mimic the old behavior, notify a channel has been opened, + * for the accepter side */ + if (channel->opener == REMOTE) + /* Tell plugins about the success */ + notify_channel_opened(dualopend->ld, + &channel->peer->id, + &channel->funding_sats, + &channel->funding.txid, + channel->remote_channel_ready); } } @@ -2170,7 +2171,7 @@ static void handle_peer_tx_sigs_msg(struct subd *dualopend, /* Saves the now finalized version of the psbt */ wallet_inflight_save(ld->wallet, inflight); - wtx = psbt_final_tx(NULL, inflight->funding_psbt); + wtx = psbt_final_tx(tmpctx, inflight->funding_psbt); if (!wtx) { channel_internal_error(channel, "Unable to extract final tx" @@ -2180,26 +2181,6 @@ static void handle_peer_tx_sigs_msg(struct subd *dualopend, return; } - send_funding_tx(channel, take(wtx)); - - assert(channel->state == DUALOPEND_OPEN_COMMITTED - /* We might be reconnecting */ - || channel->state == DUALOPEND_AWAITING_LOCKIN); - channel_set_state(channel, channel->state, - DUALOPEND_AWAITING_LOCKIN, - REASON_UNKNOWN, - "Sigs exchanged, waiting for lock-in"); - - /* Mimic the old behavior, notify a channel has been opened, - * for the accepter side */ - if (channel->opener == REMOTE) - /* Tell plugins about the success */ - notify_channel_opened(dualopend->ld, - &channel->peer->id, - &channel->funding_sats, - &channel->funding.txid, - channel->remote_channel_ready); - /* BOLT #2 * The receiving node: ... * - MUST fail the channel if: @@ -2221,7 +2202,28 @@ static void handle_peer_tx_sigs_msg(struct subd *dualopend, /* Notify the peer we're failing */ subd_send_msg(dualopend, take(towire_dualopend_fail(NULL, errmsg))); + return; } + send_funding_tx(channel, take(wtx)); + + assert(channel->state == DUALOPEND_OPEN_COMMITTED + /* We might be reconnecting */ + || channel->state == DUALOPEND_AWAITING_LOCKIN); + channel_set_state(channel, channel->state, + DUALOPEND_AWAITING_LOCKIN, + REASON_UNKNOWN, + "Sigs exchanged, waiting for lock-in"); + + /* Mimic the old behavior, notify a channel has been opened, + * for the accepter side */ + if (channel->opener == REMOTE) + /* Tell plugins about the success */ + notify_channel_opened(dualopend->ld, + &channel->peer->id, + &channel->funding_sats, + &channel->funding.txid, + channel->remote_channel_ready); + } /* Send notification with peer's signed PSBT */ @@ -3092,7 +3094,7 @@ static struct command_result *openchannel_init(struct command *cmd, if (wallet_can_spend(cmd->ld->wallet, oa->our_upfront_shutdown_script, tal_bytelen(oa->our_upfront_shutdown_script), - &found_wallet_index)) { + &found_wallet_index, NULL)) { our_upfront_shutdown_script_wallet_index = &found_wallet_index; } else our_upfront_shutdown_script_wallet_index = NULL; @@ -3861,7 +3863,7 @@ static struct command_result *json_queryrates(struct command *cmd, if (wallet_can_spend(cmd->ld->wallet, oa->our_upfront_shutdown_script, tal_bytelen(oa->our_upfront_shutdown_script), - &found_wallet_index)) { + &found_wallet_index, NULL)) { our_upfront_shutdown_script_wallet_index = tal(tmpctx, u32); *our_upfront_shutdown_script_wallet_index = found_wallet_index; } else @@ -4209,7 +4211,7 @@ bool peer_restart_dualopend(struct peer *peer, if (wallet_can_spend(peer->ld->wallet, channel->shutdown_scriptpubkey[LOCAL], tal_bytelen(channel->shutdown_scriptpubkey[LOCAL]), - &found_wallet_index)) { + &found_wallet_index, NULL)) { local_shutdown_script_wallet_index = tal(tmpctx, u32); *local_shutdown_script_wallet_index = found_wallet_index; } else diff --git a/lightningd/hsm_control.c b/lightningd/hsm_control.c index d4c0ccfb2274..7c0c6587fdc0 100644 --- a/lightningd/hsm_control.c +++ b/lightningd/hsm_control.c @@ -119,6 +119,8 @@ struct ext_key *hsm_init(struct lightningd *ld) &ld->dev_hsmd_fail_preapprove); tlv->no_preapprove_check = tal_dup(tlv, bool, &ld->dev_hsmd_no_preapprove_check); + tlv->warn_on_overgrind = tal_dup(tlv, bool, + &ld->dev_hsmd_warn_on_overgrind); msg = towire_hsmd_dev_preinit(tmpctx, tlv); if (!wire_sync_write(ld->hsm_fd, msg)) diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 49473c1c72e8..9820f75561af 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -152,6 +152,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx) ld->dev_allow_shutdown_destination_change = false; ld->dev_hsmd_no_preapprove_check = false; ld->dev_hsmd_fail_preapprove = false; + ld->dev_hsmd_warn_on_overgrind = false; ld->dev_handshake_no_reply = false; ld->dev_strict_forwarding = false; ld->dev_limit_connections_inflight = false; diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index c1ba87bbfa35..ce756f859355 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -356,6 +356,7 @@ struct lightningd { /* hsmd characteristic tweaks */ bool dev_hsmd_no_preapprove_check; bool dev_hsmd_fail_preapprove; + bool dev_hsmd_warn_on_overgrind; /* Tell connectd not to talk after handshake */ bool dev_handshake_no_reply; diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 3ea834ee0d64..725e434a5551 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; @@ -1090,9 +1091,10 @@ static bool consider_onchain_htlc_tx_rebroadcast(struct channel *channel, utxos = wallet_utxo_boost(tmpctx, ld->wallet, get_block_height(ld->topology), + AMOUNT_SAT(0), bitcoin_tx_compute_fee(newtx), feerate, - &weight); + &weight, NULL); /* Add those to create a new PSBT */ psbt = psbt_using_utxos(tmpctx, ld->wallet, utxos, locktime, @@ -1140,11 +1142,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/lightningd/opening_control.c b/lightningd/opening_control.c index 65c00ae0cb6f..544400fc4938 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -717,7 +717,7 @@ openchannel_hook_final(struct openchannel_hook_payload *payload STEALS) if (wallet_can_spend(payload->openingd->ld->wallet, our_upfront_shutdown_script, tal_bytelen(our_upfront_shutdown_script), - &found_wallet_index)) { + &found_wallet_index, NULL)) { upfront_shutdown_script_wallet_index = tal(tmpctx, u32); *upfront_shutdown_script_wallet_index = found_wallet_index; } else @@ -1408,7 +1408,7 @@ static struct command_result *json_fundchannel_start(struct command *cmd, if (wallet_can_spend(fc->cmd->ld->wallet, fc->our_upfront_shutdown_script, tal_bytelen(fc->our_upfront_shutdown_script), - &found_wallet_index)) { + &found_wallet_index, NULL)) { upfront_shutdown_script_wallet_index = tal(tmpctx, u32); *upfront_shutdown_script_wallet_index = found_wallet_index; } else diff --git a/lightningd/options.c b/lightningd/options.c index 767d03a5d25b..8efe2884088b 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -936,6 +936,10 @@ static void dev_register_opts(struct lightningd *ld) opt_set_crash_timeout, NULL, ld, "Crash if we are still going after this long."); + clnopt_noarg("--dev-warn-on-overgrind", OPT_DEV, + opt_set_bool, + &ld->dev_hsmd_warn_on_overgrind, + "Warn if we create signatures that are not exactly 71 bytes."); /* This is handled directly in daemon_developer_mode(), so we ignore it here */ clnopt_noarg("--dev-debug-self", OPT_DEV, opt_ignore, diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c index 89c86a79fa65..3ded3c1be4dd 100644 --- a/onchaind/onchaind.c +++ b/onchaind/onchaind.c @@ -186,9 +186,9 @@ static void trim_maximum_feerate(struct amount_sat funding, * * `txin[0]` script bytes: 0 * * `txin[0]` witness: `0 ` */ - /* Account for witness (1 byte count + 1 empty + sig + sig) */ + /* Account for witness (1 empty + sig + sig) */ assert(tal_count(commitment->inputs) == 1); - weight += bitcoin_tx_input_weight(false, 1 + 1 + 2 * bitcoin_tx_input_sig_weight()); + weight += bitcoin_tx_input_weight(false, 1 + 2 * bitcoin_tx_input_sig_weight()); for (size_t i = 0; i < tal_count(commitment->outputs); i++) { struct amount_asset amt; diff --git a/openingd/common.c b/openingd/common.c index 00f33da45962..78c72f908ba5 100644 --- a/openingd/common.c +++ b/openingd/common.c @@ -241,8 +241,6 @@ void validate_initial_commitment_signature(int hsm_fd, struct bitcoin_tx *tx, struct bitcoin_signature *sig) { - struct existing_htlc **htlcs; - struct bitcoin_signature *htlc_sigs; u32 feerate; u64 commit_num; const u8 *msg; @@ -250,19 +248,15 @@ void validate_initial_commitment_signature(int hsm_fd, struct pubkey next_point; /* Validate the counterparty's signature. */ - htlcs = tal_arr(NULL, struct existing_htlc *, 0); - htlc_sigs = tal_arr(NULL, struct bitcoin_signature, 0); feerate = 0; /* unused since there are no htlcs */ commit_num = 0; msg = towire_hsmd_validate_commitment_tx(NULL, tx, - (const struct simple_htlc **) htlcs, + NULL, /* No htlcs */ commit_num, feerate, sig, - htlc_sigs); - tal_free(htlc_sigs); - tal_free(htlcs); + NULL /* No htlc_sigs */); wire_sync_write(hsm_fd, take(msg)); msg = wire_sync_read(tmpctx, hsm_fd); if (!fromwire_hsmd_validate_commitment_tx_reply(tmpctx, msg, &old_secret, &next_point)) diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 302104e517e9..b3e687fd30df 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -1107,7 +1107,6 @@ static u8 *msg_for_remote_commit(const tal_t *ctx, struct wally_tx_output *direct_outputs[NUM_SIDES]; struct bitcoin_tx *remote_commit; struct bitcoin_signature local_sig; - struct simple_htlc **htlcs = tal_arr(tmpctx, struct simple_htlc *, 0); u32 feerate = 0; // unused since there are no htlcs u64 commit_num = 0; u8 *msg; @@ -1132,7 +1131,7 @@ static u8 *msg_for_remote_commit(const tal_t *ctx, &state->first_per_commitment_point[REMOTE], true, commit_num, - (const struct simple_htlc **) htlcs, + NULL, /* no HTLCS */ feerate); wire_sync_write(HSM_FD, take(msg)); msg = wire_sync_read(tmpctx, HSM_FD); diff --git a/openingd/openingd.c b/openingd/openingd.c index 6a90ef58a4bc..601fcab2a7bb 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -703,7 +703,6 @@ static bool funder_finalize_channel_setup(struct state *state, * witness script. It also needs the amount of the funding output, * as segwit signatures commit to that as well, even though it doesn't * explicitly appear in the transaction itself. */ - struct simple_htlc **htlcs = tal_arr(tmpctx, struct simple_htlc *, 0); u32 feerate = 0; // unused since there are no htlcs u64 commit_num = 0; msg = towire_hsmd_sign_remote_commitment_tx(NULL, @@ -713,7 +712,7 @@ static bool funder_finalize_channel_setup(struct state *state, channel_has(state->channel, OPT_STATIC_REMOTEKEY), commit_num, - (const struct simple_htlc **) htlcs, + NULL, /* No htlcs */ feerate); wire_sync_write(HSM_FD, take(msg)); @@ -1321,7 +1320,6 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) } /* Make HSM sign it */ - struct simple_htlc **htlcs = tal_arr(tmpctx, struct simple_htlc *, 0); u32 feerate = 0; // unused since there are no htlcs u64 commit_num = 0; msg = towire_hsmd_sign_remote_commitment_tx(NULL, @@ -1331,7 +1329,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) channel_has(state->channel, OPT_STATIC_REMOTEKEY), commit_num, - (const struct simple_htlc **) htlcs, + NULL, /* No htlcs */ feerate); wire_sync_write(HSM_FD, take(msg)); diff --git a/plugins/funder_policy.c b/plugins/funder_policy.c index 4d94bf267da8..1ce2cefc8aa1 100644 --- a/plugins/funder_policy.c +++ b/plugins/funder_policy.c @@ -127,9 +127,9 @@ default_lease_rates(const tal_t *ctx) rates->channel_fee_max_base_msat = 5000000; /* Let's set our default max weight to two inputs + an output - * (use helpers b/c elements) */ + * (use helpers b/c elements). We're mainly taproot now. */ rates->funding_weight - = 2 * bitcoin_tx_simple_input_weight(false) + = 2 * bitcoin_tx_input_weight(false, bitcoin_tx_input_witness_weight(UTXO_P2TR)) + bitcoin_tx_output_weight(BITCOIN_SCRIPTPUBKEY_P2WPKH_LEN); return rates; diff --git a/plugins/txprepare.c b/plugins/txprepare.c index b71b6717716e..a0cbd1bf2eab 100644 --- a/plugins/txprepare.c +++ b/plugins/txprepare.c @@ -285,6 +285,20 @@ static struct command_result *psbt_created(struct command *cmd, /* If we have an "all" output, we now know its value ("excess_msat") */ if (txp->all_output_idx != -1) { + /* Subtract any other outputs they specified! */ + for (size_t i = 0; i < tal_count(txp->outputs); i++) { + if (i == txp->all_output_idx) + continue; + if (!amount_sat_sub(&excess, excess, txp->outputs[i].amount)) { + return command_fail(cmd, FUND_CANNOT_AFFORD, + "Could not afford output %zu", i); + } + } + if (amount_sat_less(excess, chainparams->dust_limit)) { + return command_fail(cmd, FUND_CANNOT_AFFORD, + "Could not afford 'all' output: only %s left", + fmt_amount_sat(tmpctx, excess)); + } txp->outputs[txp->all_output_idx].amount = excess; } diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py index 4adfc1a9ee42..82d0e2599fa0 100644 --- a/tests/test_bookkeeper.py +++ b/tests/test_bookkeeper.py @@ -361,7 +361,7 @@ def test_bookkeeping_missed_chans_leases(node_factory, bitcoind): open_amt = 500000 feerate = 2000 - lease_fee = 6432000 + lease_fee = 6268000 invoice_msat = 11000000 l1.fundwallet(open_amt * 1000) @@ -410,7 +410,7 @@ def _check_events(node, channel_id, exp_events): _check_events(l1, channel_id, exp_events) exp_events = [('channel_open', open_amt * 1000, 0), - ('onchain_fee', 892000, 0), + ('onchain_fee', 894000, 0), ('lease_fee', lease_fee, 0), ('journal_entry', invoice_msat, 0)] _check_events(l2, channel_id, exp_events) diff --git a/tests/test_closing.py b/tests/test_closing.py index c102726512c0..2327a1aa2175 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -18,6 +18,7 @@ import re import subprocess import threading +import time import unittest @@ -75,8 +76,9 @@ def test_closing_simple(node_factory, bitcoind, chainparams): ] bitcoind.generate_block(1) - l1.daemon.wait_for_log(r'Owning output.* \(SEGWIT\).* txid %s.* CONFIRMED' % closetxid) - l2.daemon.wait_for_log(r'Owning output.* \(SEGWIT\).* txid %s.* CONFIRMED' % closetxid) + outtype = 'p2tr' if not chainparams['elements'] else 'p2wpkh' + l1.daemon.wait_for_log(rf'Owning output.* \({outtype}\).* txid %s.* CONFIRMED' % closetxid) + l2.daemon.wait_for_log(rf'Owning output.* \({outtype}\).* txid %s.* CONFIRMED' % closetxid) # Make sure both nodes have grabbed their close tx funds assert closetxid in set([o['txid'] for o in l1.rpc.listfunds()['outputs']]) @@ -332,13 +334,15 @@ def test_closing_specified_destination(node_factory, bitcoind, chainparams): bitcoind.generate_block(1) sync_blockheight(bitcoind, [l1, l2, l3, l4]) + outtype = 'p2tr' if not chainparams['elements'] else 'p2wpkh' + # l1 can't spent the output to addr. for txid in closetxs.values(): - assert not l1.daemon.is_in_log(r'Owning output.* \(SEGWIT\).* txid {}.* CONFIRMED'.format(txid)) + assert not l1.daemon.is_in_log(rf'Owning output.* \({outtype}\).* txid {txid}.* CONFIRMED') # Check the txid has at least 1 confirmation for n, txid in closetxs.items(): - n.daemon.wait_for_log(r'Owning output.* \(SEGWIT\).* txid {}.* CONFIRMED'.format(txid)) + n.daemon.wait_for_log(rf'Owning output.* \({outtype}\).* txid {txid}.* CONFIRMED') for n in [l2, l3, l4]: # Make sure both nodes have grabbed their close tx funds @@ -885,17 +889,17 @@ def test_channel_lease_post_expiry(node_factory, bitcoind, chainparams): l2.daemon.wait_for_log('Resolved FUNDING_TRANSACTION/FUNDING_OUTPUT by MUTUAL_CLOSE') channel_mvts_1 = [ - {'type': 'chain_mvt', 'credit_msat': 506432000, 'debit_msat': 0, 'tags': ['channel_open', 'opener', 'leased']}, - {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 6432000, 'tags': ['lease_fee'], 'fees_msat': '0msat'}, + {'type': 'chain_mvt', 'credit_msat': 506268000, 'debit_msat': 0, 'tags': ['channel_open', 'opener', 'leased']}, + {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 6268000, 'tags': ['lease_fee'], 'fees_msat': '0msat'}, {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 10000, 'tags': ['invoice'], 'fees_msat': '0msat'}, {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 499990000, 'tags': ['channel_close']}, ] channel_mvts_2 = [ {'type': 'chain_mvt', 'credit_msat': 500000000, 'debit_msat': 0, 'tags': ['channel_open', 'leased']}, - {'type': 'channel_mvt', 'credit_msat': 6432000, 'debit_msat': 0, 'tags': ['lease_fee'], 'fees_msat': '0msat'}, + {'type': 'channel_mvt', 'credit_msat': 6268000, 'debit_msat': 0, 'tags': ['lease_fee'], 'fees_msat': '0msat'}, {'type': 'channel_mvt', 'credit_msat': 10000, 'debit_msat': 0, 'tags': ['invoice'], 'fees_msat': '0msat'}, - {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 506442000, 'tags': ['channel_close']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 506278000, 'tags': ['channel_close']}, ] check_coin_moves(l1, channel_id, channel_mvts_1, chainparams) @@ -3773,9 +3777,9 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind): fundsats = int(Millisatoshi(only_one(l1.rpc.listfunds()['outputs'])['amount_msat']).to_satoshi()) psbt = l1.rpc.fundpsbt("all", "1000perkw", 1000)['psbt'] # Pay 5k sats in fees, send most to l2 - psbt = l1.rpc.addpsbtoutput(fundsats - 20000 - 5000, psbt, destination=l2.rpc.newaddr()['bech32'])['psbt'] - # 10x2000 sat outputs for l1 to use. - for i in range(10): + psbt = l1.rpc.addpsbtoutput(fundsats - 24000 - 5000, psbt, destination=l2.rpc.newaddr()['bech32'])['psbt'] + # 12x2000 sat outputs for l1 to use. + for i in range(12): psbt = l1.rpc.addpsbtoutput(2000, psbt)['psbt'] l1.rpc.sendpsbt(l1.rpc.signpsbt(psbt)['signed_psbt']) bitcoind.generate_block(1, wait_for_mempool=1) @@ -3807,6 +3811,13 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind): wait_for(lambda: len(bitcoind.rpc.getrawmempool()) == 2) + # Expect package feerate of 2000 + details = bitcoind.rpc.getrawmempool(True).values() + total_weight = sum([d['weight'] for d in details]) + total_fees = sum([float(d['fees']['base']) * 100_000_000 for d in details]) + total_feerate_perkw = total_fees / total_weight * 1000 + assert 2000 - 1 < total_feerate_perkw < 2000 + 1 + # But we don't mine it! And fees go up again! l1.set_feerates((3000, 3000, 3000, 3000)) bitcoind.generate_block(1, needfeerate=5000) @@ -3815,6 +3826,13 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind): # We actually resubmit the commit tx, then the RBF: l1.daemon.wait_for_logs(['sendrawtx exit 0'] * 2) + # Expect package feerate of 3000 + details = bitcoind.rpc.getrawmempool(True).values() + total_weight = sum([d['weight'] for d in details]) + total_fees = sum([float(d['fees']['base']) * 100_000_000 for d in details]) + total_feerate_perkw = total_fees / total_weight * 1000 + assert 3000 - 1 < total_feerate_perkw < 3000 + 1 + # And now we'll get it in (there's some rounding, so feerate a bit lower!) bitcoind.generate_block(1, needfeerate=2990) @@ -3836,8 +3854,15 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind): # It will enter the mempool wait_for(lambda: txid in bitcoind.rpc.getrawmempool()) + tx = bitcoind.rpc.getrawmempool(True)[txid] + feerate_perkw = float(tx['fees']['base']) * 100_000_000 / tx['weight'] * 1000 + # It actually has no change output, so it exceeds the fee quite a bit. + assert len(bitcoind.rpc.decoderawtransaction(bitcoind.rpc.getrawtransaction(txid))['vout']) == 1 + assert feerate_perkw > 5000 + # And this will mine it! - bitcoind.generate_block(1, needfeerate=4990) + bitcoind.generate_block(1, needfeerate=5000) + assert bitcoind.rpc.getrawmempool() == [] @pytest.mark.parametrize("anchors", [False, True]) @@ -3996,26 +4021,42 @@ def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams): # We put l3's tx in the mempool, but won't mine it. bitcoind.rpc.sendrawtransaction(closetx) - # We aim for feerate ~3750, so this won't mine it. - # HTLC's going to time out at block 119 - for block in range(108, 119): + # We aim for feerate ~3750, so this won't mine l3's unilateral close. + # HTLC's going to time out at block 120 (we give one block grace) + for block in range(110, 120): bitcoind.generate_block(1, needfeerate=5000) + assert bitcoind.rpc.getblockcount() == block sync_blockheight(bitcoind, [l2]) + assert only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['state'] == 'CHANNELD_NORMAL' # Drops to chain + bitcoind.generate_block(1, needfeerate=5000) wait_for(lambda: only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['state'] == 'AWAITING_UNILATERAL') - # But, l3's tx already there, and identical feerate will not RBF + # But, l3's tx already there, and identical feerate will not RBF. l2.daemon.wait_for_log("rejecting replacement") - assert bitcoind.rpc.getrawmempool() != [] + wait_for(lambda: len(bitcoind.rpc.getrawmempool()) == 2) # As blocks pass, we will use anchor to boost l3's tx. - for block in range(119, 124): - bitcoind.generate_block(1, needfeerate=5000) + for block, feerate in zip(range(120, 124), (12000, 13000, 14000, 15000)): + l2.daemon.wait_for_log(fr"Worth fee [0-9]*sat for remote commit tx to get 100000000msat at block 125 \(\+{125 - block}\) at feerate {feerate}perkw") + # Check feerate for entire package (commitment tx + anchor) is ~ correct + details = bitcoind.rpc.getrawmempool(True).values() + total_weight = sum([d['weight'] for d in details]) + total_fees = sum([float(d['fees']['base']) * 100_000_000 for d in details]) + total_feerate_perkw = total_fees / total_weight * 1000 + assert feerate - 1 < total_feerate_perkw < feerate + 1 + bitcoind.generate_block(1, needfeerate=16000) sync_blockheight(bitcoind, [l2]) + assert len(bitcoind.rpc.getrawmempool()) == 2 + + # Feerate tops out at +1, so this is the same. This time we mine it! + l2.daemon.wait_for_log(fr"Worth fee [0-9]*sat for remote commit tx to get 100000000msat at block 125 \(\+1\) at feerate 15000perkw") + l2.daemon.wait_for_log("sendrawtx exit 0") - # mempool should be empty, but our 'needfeerate' logic is bogus and leaves - # the anchor spend tx! So just check that l2 did see the commitment tx + bitcoind.generate_block(1, needfeerate=15000) + + assert bitcoind.rpc.getrawmempool() == [] wait_for(lambda: only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['state'] == 'ONCHAIN') @@ -4233,7 +4274,7 @@ def no_new_blocks(req): @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') def test_onchain_slow_anchor(node_factory, bitcoind): """We still use anchors for non-critical closes""" - l1, l2 = node_factory.line_graph(2) + l1, l2 = node_factory.line_graph(2, opts={'feerates': (15000, 11000, 253, 253)}) # Don't let l1 succeed in sending commit tx def censoring_sendrawtx(r): @@ -4247,12 +4288,14 @@ def censoring_sendrawtx(r): l1.rpc.disconnect(l2.info['id'], force=True) l1.rpc.close(l2.info['id'], unilateraltimeout=1) - # We will have a super-low-prio anchor spend. - l1.daemon.wait_for_log(r"Low-priority anchorspend aiming for block {} \(feerate 253\)".format(close_start_depth + 2016)) + # We will *not* have a super-low-prio anchor spend, since we're there already. + time.sleep(5) + assert not l1.daemon.is_in_log(r"Low-priority anchorspend") - # Restart with reduced block time. + # Restart with reduced block time, and increase feerates. l1.stop() l1.daemon.opts['dev-low-prio-anchor-blocks'] = 20 + l1.set_feerates((15000, 11000, 7500, 3750), wait_for_effect=False) l1.start() l1.daemon.wait_for_log("Low-priority anchorspend aiming for block {}".format(close_start_depth + 20)) @@ -4268,7 +4311,7 @@ def censoring_sendrawtx(r): height = bitcoind.rpc.getblockchaininfo()['blocks'] l1.daemon.wait_for_log(r"Low-priority anchorspend aiming for block {} \(feerate 7458\)".format(height + 13)) # Can be out-by-one (short sig)! - l1.daemon.wait_for_log(r"Anchorspend for local commit tx fee (12335|12328)sat \(w=714\), commit_tx fee 4545sat \(w=76[78]\): package feerate 1139[02] perkw") + l1.daemon.wait_for_log(r"Anchorspend for local commit tx fee 9377sat \(w=722\), commit_tx fee 1735sat \(w=768\): package feerate 7457 perkw") assert not l1.daemon.is_in_log("Low-priority anchorspend aiming for block {}".format(height + 12)) bitcoind.generate_block(1) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 9cd2485df61c..8e412787ff64 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -1117,7 +1117,7 @@ def test_gossip_lease_rates(node_factory, bitcoind): rates = l1.rpc.call('funderupdate') assert rates['channel_fee_max_base_msat'] == Millisatoshi('500000msat') assert rates['channel_fee_max_proportional_thousandths'] == 200 - assert rates['funding_weight'] == 666 # Default on regtest + assert rates['funding_weight'] == 584 # Default on regtest assert rates['lease_fee_base_msat'] == Millisatoshi('2000msat') assert rates['lease_fee_basis'] == 50 @@ -1150,7 +1150,7 @@ def test_gossip_lease_rates(node_factory, bitcoind): rates = l1_nodeinfo['option_will_fund'] assert rates['channel_fee_max_base_msat'] == Millisatoshi('500000msat') assert rates['channel_fee_max_proportional_thousandths'] == 200 - assert rates['funding_weight'] == 666 # Default on regtest + assert rates['funding_weight'] == 584 # Default on regtest assert rates['lease_fee_base_msat'] == Millisatoshi('2000msat') assert rates['lease_fee_basis'] == 50 @@ -1176,7 +1176,7 @@ def test_gossip_lease_rates(node_factory, bitcoind): rates = l2_nodeinfo['option_will_fund'] assert rates['channel_fee_max_base_msat'] == Millisatoshi('30000msat') assert rates['channel_fee_max_proportional_thousandths'] == 100 - assert rates['funding_weight'] == 666 # Default on regtest + assert rates['funding_weight'] == 584 # Default on regtest assert rates['lease_fee_base_msat'] == Millisatoshi('400000msat') assert rates['lease_fee_basis'] == 20 diff --git a/tests/test_misc.py b/tests/test_misc.py index 0699c40961a6..f3a0970b273a 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -359,7 +359,7 @@ def ping_tests(l1, l2): ping_tests(l1, l2) -def test_htlc_sig_persistence(node_factory, bitcoind, executor): +def test_htlc_sig_persistence(node_factory, bitcoind, executor, chainparams): """Interrupt a payment between two peers, then fail and recover funds using the HTLC sig. """ # Feerates identical so we don't get gratuitous commit to update them @@ -399,8 +399,9 @@ def test_htlc_sig_persistence(node_factory, bitcoind, executor): bitcoind.generate_block(5) bitcoind.generate_block(1, wait_for_mempool=txid) + outtype = 'p2tr' if not chainparams['elements'] else 'p2wpkh' l1.daemon.wait_for_logs([ - r'Owning output . (\d+)sat .SEGWIT. txid', + rf'Owning output . (\d+)sat \({outtype}\) txid {txid} CONFIRMED', ]) # We should now have 1) the unilateral to us, and b) the HTLC respend to us @@ -745,7 +746,7 @@ def dont_spend_outputs(n, txid): {'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']}, {'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']}, {'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit_msat': 11956163000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 11957393000, 'debit_msat': 0, 'tags': ['deposit']}, ] check_coin_moves(l1, 'external', external_moves, chainparams) diff --git a/tests/test_opening.py b/tests/test_opening.py index 060e7267f6d7..64d075556406 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1526,7 +1526,7 @@ def test_funder_contribution_limits(node_factory, bitcoind): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.fundchannel(l2, 10**7) - assert l2.daemon.is_in_log('Policy .* returned funding amount of 107530sat') + assert l2.daemon.is_in_log('Policy .* returned funding amount of 107470sat') assert l2.daemon.is_in_log(r'calling `signpsbt` .* inputs') l1.rpc.connect(l3.info['id'], 'localhost', l3.port) @@ -2720,10 +2720,10 @@ def censoring_sendrawtx(tx): l1.fundwallet(10**7) l3.fundwallet(10**7) + sync_blockheight(bitcoind, [l2]) l1.connect(l2) l1.rpc.fundchannel(l2.info["id"], 10**6, mindepth=0) - sync_blockheight(bitcoind, [l1, l2]) wait_for(lambda: l2.rpc.listincoming()["incoming"] != []) # If we are told to pay while still not confirmed we perform one diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 2446421e0330..bcf6d4df7d03 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -287,9 +287,42 @@ def test_txprepare_multi(node_factory, bitcoind): l1.rpc.txdiscard(prep['txid']) +def feerate_from_psbt(chainparams, bitcoind, node, psbt): + # signpsbt insists they are reserved! + node.rpc.reserveinputs(psbt, exclusive=False) + final = node.rpc.dev_finalizepsbt(node.rpc.signpsbt(psbt)['signed_psbt']) + node.rpc.unreserveinputs(psbt) + if chainparams['elements']: + # Already v1 + psbt = final['psbt'] + else: + psbt = node.rpc.setpsbtversion(final['psbt'], 0)['psbt'] + # analyzepsbt gives a vsize, but not a weight! + # e.g. 'estimated_vsize': 356, 'estimated_feerate': Decimal('0.00030042'), 'fee': Decimal('0.00010695') + fee = int(bitcoind.rpc.analyzepsbt(psbt)['fee'] * 100_000_000) + weight = bitcoind.rpc.decoderawtransaction(final['tx'])['weight'] + return fee / weight * 1000 + + +# I wish we could force libwally to use different entropy and thus force it to +# create 71-byte sigs always! +def did_short_sig(node): + # This can take a moment to appear in the log! + time.sleep(1) + return node.daemon.is_in_log('overgrind: short signature length') + + +def check_feerate(node, actual_feerate, expected_feerate): + # Feerate can't be lower. + assert actual_feerate > expected_feerate - 2 + if not did_short_sig(node): + assert actual_feerate < expected_feerate + 2 + + def test_txprepare(node_factory, bitcoind, chainparams): amount = 1000000 - l1 = node_factory.get_node(random_hsm=True) + l1 = node_factory.get_node(random_hsm=True, options={'dev-warn-on-overgrind': None}, + broken_log='overgrind: short signature length') addr = chainparams['example_addr'] # Add some funds to withdraw later @@ -299,6 +332,9 @@ def test_txprepare(node_factory, bitcoind, chainparams): bitcoind.generate_block(1) wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 10) + for est in l1.rpc.feerates('perkw')['perkw']['estimates']: + if est['blockcount'] == 12: + normal_feerate_perkw = est['feerate'] prep = l1.rpc.txprepare(outputs=[{addr: Millisatoshi(amount * 3 * 1000)}]) decode = bitcoind.rpc.decoderawtransaction(prep['unsigned_tx']) @@ -306,6 +342,8 @@ def test_txprepare(node_factory, bitcoind, chainparams): # 4 inputs, 2 outputs (3 if we have a fee output). assert len(decode['vin']) == 4 assert len(decode['vout']) == 2 if not chainparams['feeoutput'] else 3 + if not chainparams['elements']: # FIXME + check_feerate(l1, feerate_from_psbt(chainparams, bitcoind, l1, prep['psbt']), normal_feerate_perkw) # One output will be correct. outnum = [i for i, o in enumerate(decode['vout']) if o['value'] == Decimal(amount * 3) / 10**8][0] @@ -333,6 +371,8 @@ def test_txprepare(node_factory, bitcoind, chainparams): assert decode['vout'][0]['value'] > Decimal(amount * 6) / 10**8 - Decimal(0.0002) assert decode['vout'][0]['scriptPubKey']['type'] == 'witness_v0_keyhash' assert scriptpubkey_addr(decode['vout'][0]['scriptPubKey']) == addr + if not chainparams['elements']: # FIXME + check_feerate(l1, feerate_from_psbt(chainparams, bitcoind, l1, prep2['psbt']), normal_feerate_perkw) # If I cancel the first one, I can get those first 4 outputs. discard = l1.rpc.txdiscard(prep['txid']) @@ -351,6 +391,8 @@ def test_txprepare(node_factory, bitcoind, chainparams): assert decode['vout'][0]['value'] > Decimal(amount * 4) / 10**8 - Decimal(0.0002) assert decode['vout'][0]['scriptPubKey']['type'] == 'witness_v0_keyhash' assert scriptpubkey_addr(decode['vout'][0]['scriptPubKey']) == addr + if not chainparams['elements']: # FIXME + check_feerate(l1, feerate_from_psbt(chainparams, bitcoind, l1, prep3['psbt']), normal_feerate_perkw) # Cannot discard twice. with pytest.raises(RpcError, match=r'not an unreleased txid'): @@ -371,6 +413,8 @@ def test_txprepare(node_factory, bitcoind, chainparams): assert decode['vout'][0]['value'] > Decimal(amount * 10) / 10**8 - Decimal(0.0003) assert decode['vout'][0]['scriptPubKey']['type'] == 'witness_v0_keyhash' assert scriptpubkey_addr(decode['vout'][0]['scriptPubKey']) == addr + if not chainparams['elements']: # FIXME + check_feerate(l1, feerate_from_psbt(chainparams, bitcoind, l1, prep4['psbt']), normal_feerate_perkw) l1.rpc.txdiscard(prep4['txid']) # Try passing in a utxo set @@ -378,6 +422,8 @@ def test_txprepare(node_factory, bitcoind, chainparams): for utxo in l1.rpc.listfunds()["outputs"]][:4] prep5 = l1.rpc.txprepare([{addr: Millisatoshi(amount * 3.5 * 1000)}], utxos=utxos) + if not chainparams['elements']: # FIXME + check_feerate(l1, feerate_from_psbt(chainparams, bitcoind, l1, prep3['psbt']), normal_feerate_perkw) # Try passing unconfirmed utxos unconfirmed_utxo = l1.rpc.withdraw(l1.rpc.newaddr()["bech32"], 10**5) @@ -385,6 +431,11 @@ def test_txprepare(node_factory, bitcoind, chainparams): with pytest.raises(RpcError, match=r"Could not afford"): l1.rpc.txprepare([{addr: Millisatoshi(amount * 3.5 * 1000)}], utxos=uutxos) + # Feerate should be ~ as we asked for + unconfirmed_tx = bitcoind.rpc.getrawmempool(True)[unconfirmed_utxo["txid"]] + feerate_perkw = int(unconfirmed_tx['fees']['base'] * 100_000_000) * 1000 / unconfirmed_tx['weight'] + if not chainparams['elements']: # FIXME + check_feerate(l1, feerate_perkw, normal_feerate_perkw) decode = bitcoind.rpc.decoderawtransaction(prep5['unsigned_tx']) assert decode['txid'] == prep5['txid'] @@ -413,12 +464,18 @@ def test_txprepare(node_factory, bitcoind, chainparams): # You can have one which is all, but not two. prep5 = l1.rpc.txprepare([{addr: Millisatoshi(amount * 3 * 1000)}, {addr: 'all'}]) + # Feerate should be ~ as we asked for + if not chainparams['elements']: # FIXME + check_feerate(l1, feerate_from_psbt(chainparams, bitcoind, l1, prep5['psbt']), normal_feerate_perkw) l1.rpc.txdiscard(prep5['txid']) with pytest.raises(RpcError, match=r"'all'"): prep5 = l1.rpc.txprepare([{addr: 'all'}, {addr: 'all'}]) prep5 = l1.rpc.txprepare([{addr: Millisatoshi(amount * 3 * 500 + 100000)}, {addr: Millisatoshi(amount * 3 * 500 - 100000)}]) + # Feerate should be ~ as we asked for + if not chainparams['elements']: # FIXME + check_feerate(l1, feerate_from_psbt(chainparams, bitcoind, l1, prep5['psbt']), normal_feerate_perkw) decode = bitcoind.rpc.decoderawtransaction(prep5['unsigned_tx']) assert decode['txid'] == prep5['txid'] # 4 inputs, 3 outputs(include change). @@ -445,6 +502,91 @@ def test_txprepare(node_factory, bitcoind, chainparams): else: assert decode['vout'][changenum]['scriptPubKey']['type'] == 'witness_v1_taproot' + l1.rpc.txdiscard(prep5['txid']) + + +def test_txprepare_feerate(node_factory, bitcoind, chainparams): + # Make sure it works at different feerates! + l1, l2 = node_factory.get_nodes(2, opts={'dev-warn-on-overgrind': None, + 'broken_log': 'overgrind: short signature length'}) + + # Add some funds to withdraw later + for i in range(20): + bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['bech32'], + 1000 / 10**8) + + bitcoind.generate_block(1) + out_addrs = l2.rpc.newaddr('all') + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 20) + + for addrtype in ('bech32', 'p2tr'): + for feerate in range(255, 1000, 250): + prep = l1.rpc.txprepare([{out_addrs[addrtype]: Millisatoshi(9000)}], f"{feerate}perkw") + actual_feerate = feerate_from_psbt(chainparams, bitcoind, l1, prep['psbt']) + assert feerate - 2 < actual_feerate + # Feerate can be larger, if it chose not to give change output. + if chainparams['elements']: + fee_output = 1 + else: + fee_output = 0 + if len(bitcoind.rpc.decoderawtransaction(prep['unsigned_tx'])['vout']) == 1 + 1 + fee_output and not did_short_sig(l1): + assert actual_feerate < feerate + 2 + l1.rpc.txdiscard(prep['txid']) + + +@pytest.mark.parametrize("addrtype", ["bech32", "p2tr"]) +@unittest.skipIf(TEST_NETWORK != 'regtest', "FIXME: Elements fees are not quite right") +def test_fundpsbt_feerates(node_factory, bitcoind, chainparams, addrtype): + l1 = node_factory.get_node() + + # Add some funds to withdraw later + for i in range(20): + bitcoind.rpc.sendtoaddress(l1.rpc.newaddr(addrtype)[addrtype], + 5000 / 10**8) + + # See utxo_spend_weight() + if addrtype == 'bech32': + witness_weight = 1 + 71 + 1 + 33 + elif addrtype == 'p2tr': + witness_weight = 1 + 64 + else: + assert False + + input_weight = 1 + witness_weight + (32 + 4 + 4 + 1) * 4 + if chainparams['elements']: + input_weight += 6 + + bitcoind.generate_block(1) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 20) + + # version, input count, output count, locktime, segwit marker, flag + base_weight = (4 + 1 + 1 + 4) * 4 + 1 + 1 + if chainparams['elements']: + # Elements has empty surjection and rangeproof + base_weight += 2 * 4 + # And fee output (bitcoin_tx_output_weight(0)): + base_weight += (8 + 1 + 0) * 4 + (32 + 1 + 1 + 1) * 4 + # Bech32 change output + change_weight = (8 + 1 + (1 + 1 + 20)) * 4 + else: + # P2TR output + change_weight = (8 + 1 + (1 + 1 + 32)) * 4 + + # Both minimal and higher feerate + for feerate in (253, 1000): + # Try with both 1 and 2 inputs + for amount, num_inputs in ((260, 1), (5000, 2)): + prep = l1.rpc.fundpsbt(amount, f"{feerate}perkw", base_weight, excess_as_change=True) + assert prep['estimated_final_weight'] == base_weight + change_weight + input_weight * num_inputs + signed = l1.rpc.signpsbt(prep['psbt'])['signed_psbt'] + sent = l1.rpc.sendpsbt(signed) + txinfo = bitcoind.rpc.getmempoolentry(sent['txid']) + assert txinfo['weight'] == prep['estimated_final_weight'] + # We never actually added that `amount` output to PSBT, so that appears as "fee" + fee = int(txinfo['fees']['base'] * 100_000_000) - amount + actual_feerate = fee / (txinfo['weight'] / 1000) + check_feerate(l1, actual_feerate, feerate) + def test_reserveinputs(node_factory, bitcoind, chainparams): amount = 1000000 diff --git a/tools/generate-wire.py b/tools/generate-wire.py index f408cca15dc1..dd6c742498e9 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -231,9 +231,8 @@ class Type(FieldSet): 'gossip_getchannels_entry', 'failed_htlc', 'existing_htlc', - 'simple_htlc', 'inflight', - 'utxo', + 'hsm_utxo', 'bitcoin_tx', 'wirestring', 'per_peer_state', diff --git a/wallet/db.c b/wallet/db.c index 44cfeb70e18b..2f67add763df 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -1234,7 +1234,7 @@ void fillin_missing_scriptpubkeys(struct lightningd *ld, struct db *db) db_col_ignore(stmt, "peer_id"); db_col_ignore(stmt, "commitment_point"); bip32_pubkey(ld, &key, keyindex); - if (type == p2sh_wpkh) { + if (type == WALLET_OUTPUT_P2SH_WPKH) { u8 *redeemscript = bitcoin_redeem_p2sh_p2wpkh(stmt, &key); scriptPubkey = scriptpubkey_p2sh(tmpctx, redeemscript); } else diff --git a/wallet/reservation.c b/wallet/reservation.c index b68155420c87..9c100a0cc4bb 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -277,7 +277,7 @@ struct wally_psbt *psbt_using_utxos(const tal_t *ctx, u32 this_nsequence; struct bitcoin_tx *tx; - if (utxos[i]->is_p2sh) { + if (utxos[i]->utxotype == UTXO_P2SH_P2WPKH) { bip32_pubkey(wallet->ld, &key, utxos[i]->keyindex); scriptSig = bitcoin_scriptsig_p2sh_p2wpkh(tmpctx, &key); redeemscript = bitcoin_redeem_p2sh_p2wpkh(tmpctx, &key); diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 7e03d279e1ff..d82a8c72c84e 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1387,12 +1387,12 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) db_begin_transaction(w->db); /* Should work, it's the first time we add it */ - CHECK_MSG(wallet_add_utxo(w, &u, p2sh_wpkh), + CHECK_MSG(wallet_add_utxo(w, &u, WALLET_OUTPUT_P2SH_WPKH), "wallet_add_utxo failed on first add"); CHECK_MSG(!wallet_err, wallet_err); /* Should fail, we already have that UTXO */ - CHECK_MSG(!wallet_add_utxo(w, &u, p2sh_wpkh), + CHECK_MSG(!wallet_add_utxo(w, &u, WALLET_OUTPUT_P2SH_WPKH), "wallet_add_utxo succeeded on second add"); CHECK_MSG(!wallet_err, wallet_err); @@ -1403,10 +1403,12 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) u.close_info->peer_id = id; u.close_info->commitment_point = &pk; u.close_info->option_anchors = false; - /* Arbitrarily set scriptpubkey len to 20 */ - u.scriptPubkey = tal_arr(w, u8, 20); - memset(u.scriptPubkey, 1, 20); - CHECK_MSG(wallet_add_utxo(w, &u, our_change), + /* P2WSH */ + u.scriptPubkey = tal_arr(w, u8, BITCOIN_SCRIPTPUBKEY_P2WSH_LEN); + u.scriptPubkey[0] = OP_0; + u.scriptPubkey[1] = sizeof(struct sha256); + memset(u.scriptPubkey + 2, 1, sizeof(struct sha256)); + CHECK_MSG(wallet_add_utxo(w, &u, WALLET_OUTPUT_OUR_CHANGE), "wallet_add_utxo with close_info"); /* Now select them */ @@ -1473,9 +1475,11 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, wallet_err); u.blockheight = blockheight; - u.scriptPubkey = tal_arr(w, u8, 20); - memset(u.scriptPubkey, 1, 20); - CHECK_MSG(wallet_add_utxo(w, &u, p2sh_wpkh), + u.scriptPubkey = tal_arr(w, u8, BITCOIN_SCRIPTPUBKEY_P2WPKH_LEN); + u.scriptPubkey[0] = OP_0; + u.scriptPubkey[1] = sizeof(struct ripemd160); + memset(u.scriptPubkey + 2, 1, sizeof(struct ripemd160)); + CHECK_MSG(wallet_add_utxo(w, &u, WALLET_OUTPUT_P2SH_WPKH), "wallet_add_utxo with close_info no commitment_point"); CHECK_MSG(!wallet_err, wallet_err); @@ -1557,7 +1561,7 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) memset(&u.outpoint, 4, sizeof(u.outpoint)); u.amount = AMOUNT_SAT(4); u.close_info = tal_free(u.close_info); - CHECK_MSG(wallet_add_utxo(w, &u, p2wpkh), + CHECK_MSG(wallet_add_utxo(w, &u, WALLET_OUTPUT_P2WPKH), "wallet_add_utxo failed, p2wpkh"); utxos = tal_arr(w, const struct utxo *, 0); diff --git a/wallet/wallet.c b/wallet/wallet.c index 67a67c6b72aa..43f7e290a8cc 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -338,7 +338,6 @@ static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt) db_col_txid(stmt, "prev_out_tx", &utxo->outpoint.txid); utxo->outpoint.n = db_col_int(stmt, "prev_out_index"); utxo->amount = db_col_amount_sat(stmt, "value"); - utxo->is_p2sh = db_col_int(stmt, "type") == p2sh_wpkh; utxo->status = db_col_int(stmt, "status"); utxo->keyindex = db_col_int(stmt, "keyindex"); @@ -364,6 +363,19 @@ static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt) } utxo->scriptPubkey = db_col_arr(utxo, stmt, "scriptpubkey", u8); + /* FIXME: add p2tr to type? */ + if (wallet_output_type_in_db(db_col_int(stmt, "type")) == WALLET_OUTPUT_P2SH_WPKH) + utxo->utxotype = UTXO_P2SH_P2WPKH; + else if (is_p2wpkh(utxo->scriptPubkey, tal_bytelen(utxo->scriptPubkey), NULL)) + utxo->utxotype = UTXO_P2WPKH; + else if (is_p2tr(utxo->scriptPubkey, tal_bytelen(utxo->scriptPubkey), NULL)) + utxo->utxotype = UTXO_P2TR; + else if (is_p2wsh(utxo->scriptPubkey, tal_bytelen(utxo->scriptPubkey), NULL)) { + if (!utxo->close_info) + fatal("Unspendable scriptPubkey without close_info %s", tal_hex(tmpctx, utxo->scriptPubkey)); + utxo->utxotype = UTXO_P2WSH_FROM_CLOSE; + } else + fatal("Unknown utxo type %s", tal_hex(tmpctx, utxo->scriptPubkey)); utxo->blockheight = NULL; utxo->spendheight = NULL; @@ -562,13 +574,29 @@ struct utxo *wallet_utxo_get(const tal_t *ctx, struct wallet *w, return utxo; } +static u32 calc_feerate(struct amount_sat excess_sats, + struct amount_sat output_sats_required, + size_t weight) +{ + struct amount_sat fee; + u32 feerate; + + if (!amount_sat_sub(&fee, excess_sats, output_sats_required)) + return 0; + if (!amount_feerate(&feerate, fee, weight)) + abort(); + return feerate; +} + /* Gather enough utxos to meet feerate, otherwise all we can. */ struct utxo **wallet_utxo_boost(const tal_t *ctx, struct wallet *w, u32 blockheight, - struct amount_sat fee_amount, + struct amount_sat excess_sats, + struct amount_sat output_sats_required, u32 feerate_target, - size_t *weight) + size_t *weight, + bool *insufficient) { struct utxo **all_utxos = wallet_get_unspent_utxos(tmpctx, w); struct utxo **utxos = tal_arr(ctx, struct utxo *, 0); @@ -577,20 +605,26 @@ struct utxo **wallet_utxo_boost(const tal_t *ctx, /* Select in random order */ tal_arr_randomize(all_utxos, struct utxo *); - /* Can't overflow, it's from our tx! */ - if (!amount_feerate(&feerate, fee_amount, *weight)) - abort(); + feerate = calc_feerate(excess_sats, output_sats_required, *weight); for (size_t i = 0; i < tal_count(all_utxos); i++) { u32 new_feerate; size_t new_weight; - struct amount_sat new_fee_amount; + struct amount_sat new_excess_sats; /* Convenience var */ struct utxo *utxo = all_utxos[i]; /* Are we already happy? */ - if (feerate >= feerate_target) - break; + if (feerate >= feerate_target) { + log_debug(w->log, "wallet_utxo_boost: got %zu UTXOs, excess %s (needed %s), weight %zu, feerate %u >= %u", + tal_count(utxos), + fmt_amount_sat(tmpctx, excess_sats), + fmt_amount_sat(tmpctx, output_sats_required), + *weight, feerate, feerate_target); + if (insufficient) + *insufficient = false; + return utxos; + } /* Don't add reserved ones */ if (utxo_is_reserved(utxo, blockheight)) @@ -601,13 +635,13 @@ struct utxo **wallet_utxo_boost(const tal_t *ctx, continue; /* UTXOs must be sane amounts */ - if (!amount_sat_add(&new_fee_amount, - fee_amount, utxo->amount)) + if (!amount_sat_add(&new_excess_sats, + excess_sats, utxo->amount)) abort(); new_weight = *weight + utxo_spend_weight(utxo, 0); - if (!amount_feerate(&new_feerate, new_fee_amount, new_weight)) - abort(); + new_feerate = calc_feerate(new_excess_sats, output_sats_required, + new_weight); /* Don't add uneconomic ones! */ if (new_feerate < feerate) @@ -615,10 +649,14 @@ struct utxo **wallet_utxo_boost(const tal_t *ctx, feerate = new_feerate; *weight = new_weight; - fee_amount = new_fee_amount; + excess_sats = new_excess_sats; tal_arr_expand(&utxos, tal_steal(utxos, utxo)); } + log_debug(w->log, "wallet_utxo_boost: fell short, returning %zu UTXOs", + tal_count(utxos)); + if (insufficient) + *insufficient = true; return utxos; } @@ -776,7 +814,7 @@ struct utxo *wallet_find_utxo(const tal_t *ctx, struct wallet *w, while (!utxo && db_step(stmt)) { utxo = wallet_stmt2output(ctx, stmt); if (excluded(excludes, utxo) - || (nonwrapped && utxo->is_p2sh) + || (nonwrapped && utxo->utxotype == UTXO_P2SH_P2WPKH) || !deep_enough(maxheight, utxo, current_blockheight)) utxo = tal_free(utxo); @@ -885,7 +923,7 @@ bool wallet_add_onchaind_utxo(struct wallet *w, db_bind_txid(stmt, &outpoint->txid); db_bind_int(stmt, outpoint->n); db_bind_amount_sat(stmt, &amount); - db_bind_int(stmt, wallet_output_type_in_db(p2wpkh)); + db_bind_int(stmt, wallet_output_type_in_db(WALLET_OUTPUT_P2WPKH)); db_bind_int(stmt, OUTPUT_STATE_AVAILABLE); db_bind_int(stmt, 0); db_bind_u64(stmt, channel->dbid); @@ -910,7 +948,7 @@ bool wallet_add_onchaind_utxo(struct wallet *w, } bool wallet_can_spend(struct wallet *w, const u8 *script, size_t script_len, - u32 *index) + u32 *index, enum addrtype *addrtype) { u64 bip32_max_index; const struct wallet_address *waddr; @@ -931,6 +969,8 @@ bool wallet_can_spend(struct wallet *w, const u8 *script, size_t script_len, db_set_intvar(w->db, "bip32_max_index", waddr->index); *index = waddr->index; + if (addrtype) + *addrtype = waddr->addrtype; return true; } @@ -3014,6 +3054,7 @@ void wallet_confirm_tx(struct wallet *w, static void got_utxo(struct wallet *w, u64 keyindex, + enum addrtype addrtype, const struct wally_tx *wtx, size_t outnum, bool is_coinbase, @@ -3025,7 +3066,24 @@ static void got_utxo(struct wallet *w, struct amount_asset asset = wally_tx_output_get_amount(txout); utxo->keyindex = keyindex; - utxo->is_p2sh = is_p2sh(txout->script, txout->script_len, NULL); + /* This switch() pattern catches anyone adding new cases, plus + * runtime errors */ + switch (addrtype) { + case ADDR_P2SH_SEGWIT: + utxo->utxotype = UTXO_P2SH_P2WPKH; + goto type_ok; + case ADDR_BECH32: + utxo->utxotype = UTXO_P2WPKH; + goto type_ok; + case ADDR_P2TR: + utxo->utxotype = UTXO_P2TR; + goto type_ok; + case ADDR_ALL: + break; + } + abort(); + +type_ok: utxo->amount = amount_asset_to_sat(&asset); utxo->status = OUTPUT_STATE_AVAILABLE; wally_txid(wtx, &utxo->outpoint.txid); @@ -3039,7 +3097,7 @@ static void got_utxo(struct wallet *w, log_debug(w->log, "Owning output %zu %s (%s) txid %s%s%s", outnum, fmt_amount_sat(tmpctx, utxo->amount), - utxo->is_p2sh ? "P2SH" : "SEGWIT", + utxotype_to_str(utxo->utxotype), fmt_bitcoin_txid(tmpctx, &utxo->outpoint.txid), blockheight ? " CONFIRMED" : "", is_coinbase ? " COINBASE" : ""); @@ -3055,7 +3113,7 @@ static void got_utxo(struct wallet *w, notify_chain_mvt(w->ld, mvt); } - if (!wallet_add_utxo(w, utxo, utxo->is_p2sh ? p2sh_wpkh : our_change)) { + if (!wallet_add_utxo(w, utxo, utxo->utxotype == UTXO_P2SH_P2WPKH ? WALLET_OUTPUT_P2SH_WPKH : WALLET_OUTPUT_OUR_CHANGE)) { /* In case we already know the output, make * sure we actually track its * blockheight. This can happen when we grab @@ -3067,7 +3125,7 @@ static void got_utxo(struct wallet *w, } /* This is an unconfirmed change output, we should track it */ - if (!utxo->is_p2sh && !blockheight) + if (utxo->utxotype != UTXO_P2SH_P2WPKH && !blockheight) txfilter_add_scriptpubkey(w->ld->owned_txfilter, txout->script); outpointfilter_add(w->owned_outpoints, &utxo->outpoint); @@ -3087,14 +3145,15 @@ int wallet_extract_owned_outputs(struct wallet *w, const struct wally_tx *wtx, const struct wally_tx_output *txout = &wtx->outputs[i]; u32 keyindex; struct amount_asset asset = wally_tx_output_get_amount(txout); + enum addrtype addrtype; if (!amount_asset_is_main(&asset)) continue; - if (!wallet_can_spend(w, txout->script, txout->script_len, &keyindex)) + if (!wallet_can_spend(w, txout->script, txout->script_len, &keyindex, &addrtype)) continue; - got_utxo(w, keyindex, wtx, i, is_coinbase, blockheight, NULL); + got_utxo(w, keyindex, addrtype, wtx, i, is_coinbase, blockheight, NULL); num_utxos++; } return num_utxos; @@ -6751,7 +6810,7 @@ static void mutual_close_p2pkh_catch(struct bitcoind *bitcoind, missing->addrs[n].scriptpubkey, tal_bytelen(missing->addrs[n].scriptpubkey))) continue; - got_utxo(w, missing->addrs[n].keyidx, + got_utxo(w, missing->addrs[n].keyidx, ADDR_BECH32, wtx, outnum, i == 0, &height, &outp); log_broken(bitcoind->ld->log, "Rescan found %s!", fmt_bitcoin_outpoint(tmpctx, &outp)); diff --git a/wallet/wallet.h b/wallet/wallet.h index 94d7be7cc672..8788a0140365 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -83,34 +83,34 @@ static inline enum output_status output_status_in_db(enum output_status s) /* /!\ This is a DB ENUM, please do not change the numbering of any * already defined elements (adding is ok) /!\ */ enum wallet_output_type { - p2sh_wpkh = 0, - to_local = 1, - htlc_offer = 3, - htlc_recv = 4, - our_change = 5, - p2wpkh = 6 + WALLET_OUTPUT_P2SH_WPKH = 0, + WALLET_OUTPUT_TO_LOCAL = 1, + WALLET_OUTPUT_HTLC_OFFER = 3, + WALLET_OUTPUT_HTLC_RECV = 4, + WALLET_OUTPUT_OUR_CHANGE = 5, + WALLET_OUTPUT_P2WPKH = 6 }; static inline enum wallet_output_type wallet_output_type_in_db(enum wallet_output_type w) { switch (w) { - case p2sh_wpkh: - BUILD_ASSERT(p2sh_wpkh == 0); + case WALLET_OUTPUT_P2SH_WPKH: + BUILD_ASSERT(WALLET_OUTPUT_P2SH_WPKH == 0); return w; - case to_local: - BUILD_ASSERT(to_local == 1); + case WALLET_OUTPUT_TO_LOCAL: + BUILD_ASSERT(WALLET_OUTPUT_TO_LOCAL == 1); return w; - case htlc_offer: - BUILD_ASSERT(htlc_offer == 3); + case WALLET_OUTPUT_HTLC_OFFER: + BUILD_ASSERT(WALLET_OUTPUT_HTLC_OFFER == 3); return w; - case htlc_recv: - BUILD_ASSERT(htlc_recv == 4); + case WALLET_OUTPUT_HTLC_RECV: + BUILD_ASSERT(WALLET_OUTPUT_HTLC_RECV == 4); return w; - case our_change: - BUILD_ASSERT(our_change == 5); + case WALLET_OUTPUT_OUR_CHANGE: + BUILD_ASSERT(WALLET_OUTPUT_OUR_CHANGE == 5); return w; - case p2wpkh: - BUILD_ASSERT(p2wpkh == 6); + case WALLET_OUTPUT_P2WPKH: + BUILD_ASSERT(WALLET_OUTPUT_P2WPKH == 6); return w; } fatal("%s: %u is invalid", __func__, w); @@ -571,9 +571,11 @@ struct utxo *wallet_utxo_get(const tal_t *ctx, struct wallet *w, * @ctx: context to tal return array from * @w: the wallet * @blockheight: current height (to determine reserved status) - * @fee_amount: amount already paying in fees + * @excess_sats: how much excess it's already got. + * @output_sats_required: minimum amount required to output * @feerate_target: feerate we want, in perkw. * @weight: (in)existing weight before any utxos added, (out)final weight with utxos added. + * @insufficient: (out) if non-NULL, set true if we run out of utxos, otherwise false. * * May not meet the feerate, but will spend all available utxos to try. * You may also need to create change, as it may exceed. @@ -581,24 +583,26 @@ struct utxo *wallet_utxo_get(const tal_t *ctx, struct wallet *w, struct utxo **wallet_utxo_boost(const tal_t *ctx, struct wallet *w, u32 blockheight, - struct amount_sat fee_amount, + struct amount_sat excess_sats, + struct amount_sat output_sats_required, u32 feerate_target, - size_t *weight); + size_t *weight, + bool *insufficient); /** * wallet_can_spend - Do we have the private key matching this scriptpubkey? * - * FIXME: This is very slow with lots of inputs! - * * @w: (in) wallet holding the pubkeys to check against (privkeys are on HSM) * @script: (in) the script to check * @script_len: (in) the length of @script * @index: (out) the bip32 derivation index that matched the script + * @addrtype: (out) if non-NULL, set to address type of script. */ bool wallet_can_spend(struct wallet *w, const u8 *script, size_t script_len, - u32 *index); + u32 *index, + enum addrtype *addrtype); /** * wallet_get_newindex - get a new index from the wallet. diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index e9195e37a59a..20c77c857915 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -349,7 +349,7 @@ static void json_add_utxo(struct json_stream *response, json_add_num(response, "output", utxo->outpoint.n); json_add_amount_sat_msat(response, "amount_msat", utxo->amount); - if (utxo->is_p2sh) { + if (utxo->utxotype == UTXO_P2SH_P2WPKH) { struct pubkey key; bip32_pubkey(wallet->ld, &key, utxo->keyindex); @@ -693,7 +693,7 @@ static struct command_result *match_psbt_inputs_to_utxos(struct command *cmd, if (!psbt->inputs[i].utxo && !psbt->inputs[i].witness_utxo) { u8 *scriptPubKey; - if (utxo->is_p2sh) { + if (utxo->utxotype == UTXO_P2SH_P2WPKH) { struct pubkey key; u8 *redeemscript; int wally_err; @@ -729,7 +729,7 @@ static void match_psbt_outputs_to_wallet(struct wally_psbt *psbt, const size_t script_len = psbt->outputs[outndx].script_len; u32 index; - if (!wallet_can_spend(w, script, script_len, &index)) + if (!wallet_can_spend(w, script, script_len, &index, NULL)) continue; if (bip32_key_from_parent( @@ -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", @@ -936,7 +936,7 @@ static void maybe_notify_new_external_send(struct lightningd *ld, /* If it's going to our wallet, ignore */ script = wally_psbt_output_get_script(tmpctx, &psbt->outputs[outnum]); - if (wallet_can_spend(ld->wallet, script, tal_bytelen(script), &index)) + if (wallet_can_spend(ld->wallet, script, tal_bytelen(script), &index, NULL)) return; outpoint.txid = *txid; @@ -998,6 +998,42 @@ static void sendpsbt_done(struct bitcoind *bitcoind UNUSED, was_pending(command_success(sending->cmd, response)); } +static struct command_result *json_dev_finalizepsbt(struct command *cmd, + const char *buffer, + const jsmntok_t *obj, + const jsmntok_t *params) +{ + struct wally_psbt *psbt; + struct wally_tx *wtx; + struct bitcoin_txid txid; + struct json_stream *response; + + if (!param_check(cmd, buffer, params, + p_req("psbt", param_psbt, &psbt), + NULL)) + return command_param_failed(); + + if (!psbt_finalize(psbt)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "PSBT not finalizeable"); + + wtx = psbt_final_tx(cmd, psbt); + wally_txid(wtx, &txid); + + response = json_stream_success(cmd); + json_add_psbt(response, "psbt", psbt); + json_add_hex_talarr(response, "tx", linearize_wtx(tmpctx, wtx)); + json_add_txid(response, "txid", &txid); + return command_success(cmd, response); +} + +static const struct json_command dev_finalizepsbt_command = { + "dev-finalizepsbt", + json_dev_finalizepsbt, + true, +}; +AUTODATA(json_command, &dev_finalizepsbt_command); + static struct command_result *json_sendpsbt(struct command *cmd, const char *buffer, const jsmntok_t *obj,