Fix fee calculations

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.
+		 *    <remotepubkey> OP_CHECKSIGVERIFY 1 OP_CHECKSEQUENCEVERIFY
+		 *
+		 * The output is spent by an input with `nSequence` field set
+		 * to `1` and witness: <remote_sig>
+		 * 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)

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);

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,

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

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)
 {

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,

common/utxo.c

@@ -2,76 +2,20 @@
 #include <common/utxo.h>
 #include <wire/wire.h>
 
-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();
+}