splice: Fix weight calculations & use opening feerate

Here we update `psbt_input_get_weight` to allow the caller to specify what kind of input size estimation they would like. Before it was just “zero witness bytes” which we now move to the default behavior and add an assumption option for P2WSH -> 2of2 multisig which is what we typically see in splicing.

At the same time we move channeld over to using the opening feerate estimation instead of the less useful “max feerate.” We make splice script use this same feerate.

After auditing the feerates for these two places against each other and the actually created transaction, we implement some fixes making them all match.

While we’re here, we add relevant debug log messages.

Changelog-Changed: Audited and updated splice’s feerate calculations to be precise to the byte — a critical change to accomodate splice script.

Author: ddustin

bitcoin/psbt.c

@@ -7,6 +7,7 @@
 #include <ccan/ccan/mem/mem.h>
 #include <common/utils.h>
 #include <wally_psbt.h>
+#include <wally_psbt_members.h>
 #include <wire/wire.h>
 
 
@@ -480,6 +481,28 @@ void psbt_input_set_witscript(struct wally_psbt *psbt, size_t in, const u8 *wscr
 	tal_wally_end(psbt);
 }
 
+const u8 *psbt_input_get_witscript(const tal_t *ctx,
+				    const struct wally_psbt *psbt,
+				    size_t in)
+{
+	size_t witscript_len, written_len;
+	u8 *witscript;
+	if (wally_psbt_get_input_witness_script_len(psbt, in, &witscript_len) != WALLY_OK)
+		abort();
+	witscript = tal_arr(ctx, u8, witscript_len);
+	if (wally_psbt_get_input_witness_script(psbt, in, witscript, witscript_len, &written_len) != WALLY_OK)
+		abort();
+	if (witscript_len != written_len)
+		abort();
+	return witscript;
+}
+
+bool psbt_input_get_ecdsa_sig(const tal_t *ctx,
+						 const struct wally_psbt *psbt,
+						 size_t in,
+						 const struct pubkey *pubkey,
+						 struct bitcoin_signature **sig);
+
 void psbt_elements_input_set_asset(struct wally_psbt *psbt, size_t in,
 				   struct amount_asset *asset)
 {
@@ -592,10 +615,16 @@ struct amount_sat psbt_input_get_amount(const struct wally_psbt *psbt,
 }
 
 size_t psbt_input_get_weight(const struct wally_psbt *psbt,
-			     size_t in)
+			     size_t in,
+			     enum PSBT_GUESS guess)
 {
 	size_t weight;
 	const struct wally_map_item *redeem_script;
+	struct wally_psbt_input *input = &psbt->inputs[in];
+	struct wally_tx_output *utxo_out = NULL;
+
+	if (input->utxo)
+		utxo_out = &input->utxo->outputs[input->index];
 
 	redeem_script = wally_map_get_integer(&psbt->inputs[in].psbt_fields, /* PSBT_IN_REDEEM_SCRIPT */ 0x04);
 
@@ -605,8 +634,20 @@ size_t psbt_input_get_weight(const struct wally_psbt *psbt,
 		weight +=
 			(redeem_script->value_len +
 				varint_size(redeem_script->value_len)) * 4;
+	} else if ((guess & PSBT_GUESS_2OF2)
+		   && utxo_out
+		   && is_p2wsh(utxo_out->script, utxo_out->script_len, NULL)) {
+		weight = bitcoin_tx_input_weight(false,
+						 bitcoin_tx_2of2_input_witness_weight());
+	} else if (utxo_out
+		   && is_p2wpkh(utxo_out->script, utxo_out->script_len, NULL)) {
+		weight = bitcoin_tx_input_weight(false,
+						 bitcoin_tx_input_witness_weight(UTXO_P2SH_P2WPKH));
+	} else if (utxo_out
+		   && is_p2tr(utxo_out->script, utxo_out->script_len, NULL)) {
+		weight = bitcoin_tx_input_weight(false,
+						 bitcoin_tx_input_witness_weight(UTXO_P2TR));
 	} else {
-		/* zero scriptSig length */
 		weight += varint_size(0) * 4;
 	}
 

bitcoin/psbt.h

@@ -207,6 +207,10 @@ WARN_UNUSED_RESULT bool psbt_input_get_ecdsa_sig(const tal_t *ctx,
 
 void psbt_input_set_witscript(struct wally_psbt *psbt, size_t in, const u8 *wscript);
 
+const u8 *psbt_input_get_witscript(const tal_t *ctx,
+				   const struct wally_psbt *psbt,
+				   size_t in);
+
 /* psbt_input_set_unknown - Set the given Key-Value in the psbt's input keymap
  * @ctx - tal context for allocations
  * @in - psbt input to set key-value on
@@ -265,9 +269,20 @@ void psbt_output_set_unknown(const tal_t *ctx,
 struct amount_sat psbt_input_get_amount(const struct wally_psbt *psbt,
 					size_t in);
 
-/* psbt_input_get_weight - Calculate the tx weight for input index `in` */
+enum PSBT_GUESS {
+	PSBT_GUESS_ZERO = 0x0, /* Assume unknown is 0 bytes (fallback) */
+	PSBT_GUESS_2OF2 = 0x1, /* Assume P2WSH is 2of2 multisig (req prevtx) */
+};
+
+/* psbt_input_get_weight - Calculate the tx weight for input index `in`.
+ *
+ * @psbt - psbt
+ * @in - index of input who's weight you want
+ * @guess - How to guess if we have incomplete information
+ * */
 size_t psbt_input_get_weight(const struct wally_psbt *psbt,
-			     size_t in);
+			     size_t in,
+			     enum PSBT_GUESS guess);
 
 /* psbt_output_get_amount - Returns the value of this output
  *

channeld/channeld.c

@@ -3178,47 +3178,70 @@ static struct wally_psbt_output *find_channel_output(struct peer *peer,
 	return NULL;
 }
 
-static size_t calc_weight(enum tx_role role, const struct wally_psbt *psbt)
+static size_t calc_weight(enum tx_role role, const struct wally_psbt *psbt,
+			  bool log_math)
 {
-	size_t weight = 0;
+	size_t lweight = 0, weight = 0;
 
-	/* BOLT #2:
-	 * The *initiator* is responsible for paying the fees for the following fields,
-	 * to be referred to as the `common fields`.
-	 *
-	 *   - version
-	 *   - segwit marker + flag
-	 *   - input count
-	 *   - output count
-	 *   - locktime
-	 */
-	if (role == TX_INITIATOR)
-		weight += bitcoin_tx_core_weight(psbt->num_inputs,
-						 psbt->num_outputs);
+	if (log_math)
+		status_debug("Counting tx weight;");
 
 	/* BOLT #2:
 	 * The rest of the transaction bytes' fees are the responsibility of
 	 * the peer who contributed that input or output via `tx_add_input` or
 	 * `tx_add_output`, at the agreed upon `feerate`.
 	 */
-	for (size_t i = 0; i < psbt->num_inputs; i++)
+	for (size_t i = 0; i < psbt->num_inputs; i++) {
 		if (is_initiators_serial(&psbt->inputs[i].unknowns)) {
 			if (role == TX_INITIATOR)
-				weight += psbt_input_get_weight(psbt, i);
+				weight += psbt_input_get_weight(psbt, i, PSBT_GUESS_2OF2);
 		}
-		else
+		else {
 			if (role != TX_INITIATOR)
-				weight += psbt_input_get_weight(psbt, i);
+				weight += psbt_input_get_weight(psbt, i, PSBT_GUESS_2OF2);
+		}
+		if (log_math)
+			status_debug(" Adding input"
+				     " %lu; weight: %lu", i, weight - lweight);
+		lweight = weight;
+	}
 
-	for (size_t i = 0; i < psbt->num_outputs; i++)
+	for (size_t i = 0; i < psbt->num_outputs; i++) {
 		if (is_initiators_serial(&psbt->outputs[i].unknowns)) {
 			if (role == TX_INITIATOR)
 				weight += psbt_output_get_weight(psbt, i);
 		}
-		else
+		else {
 			if (role != TX_INITIATOR)
 				weight += psbt_output_get_weight(psbt, i);
+		}
+		if (log_math)
+			status_debug(" Adding output"
+				     " %lu; weight: %lu", i, weight - lweight);
+		lweight = weight;
+	}
 
+	/* BOLT #2:
+	 * The *initiator* is responsible for paying the fees for the following fields,
+	 * to be referred to as the `common fields`.
+	 *
+	 *   - version
+	 *   - segwit marker + flag
+	 *   - input count
+	 *   - output count
+	 *   - locktime
+	 */
+	if (role == TX_INITIATOR) {
+		weight += bitcoin_tx_core_weight(psbt->num_inputs,
+						 psbt->num_outputs);
+		if (log_math)
+			status_debug(" Adding bitcoin_tx_core_weight;"
+				     " weight: %lu", weight - lweight);
+		lweight = weight;
+	  }
+
+	if (log_math)
+		status_debug("Total weight: %lu", weight);
 	return weight;
 }
 
@@ -3319,8 +3342,7 @@ static struct amount_sat check_balances(struct peer *peer,
 {
 	struct amount_sat min_initiator_fee, min_accepter_fee,
 			  max_initiator_fee, max_accepter_fee,
-			  funding_amount_res, min_multiplied,
-			  initiator_penalty_fee, accepter_penalty_fee;
+			  funding_amount_res;
 	struct amount_msat funding_amount,
 			   initiator_fee, accepter_fee;
 	struct amount_msat in[NUM_TX_ROLES], out[NUM_TX_ROLES],
@@ -3469,33 +3491,26 @@ static struct amount_sat check_balances(struct peer *peer,
 			      "amount_sat_less / amount_sat_sub mismtach");
 
 	min_initiator_fee = amount_tx_fee(peer->splicing->feerate_per_kw,
-					  calc_weight(TX_INITIATOR, psbt));
+					  calc_weight(TX_INITIATOR, psbt, false));
 	min_accepter_fee = amount_tx_fee(peer->splicing->feerate_per_kw,
-					 calc_weight(TX_ACCEPTER, psbt));
+					 calc_weight(TX_ACCEPTER, psbt, false));
 
 	/* As a safeguard max feerate is checked (only) locally, if it's
 	 * particularly high we fail and tell the user but allow them to
 	 * override with `splice_force_feerate` */
-	max_accepter_fee = amount_tx_fee(peer->feerate_max,
-					 calc_weight(TX_ACCEPTER, psbt));
-	max_initiator_fee = amount_tx_fee(peer->feerate_max,
-					  calc_weight(TX_INITIATOR, psbt));
-	initiator_penalty_fee = amount_tx_fee(peer->feerate_penalty,
-					      calc_weight(TX_INITIATOR, psbt));
-	accepter_penalty_fee = amount_tx_fee(peer->feerate_penalty,
-					     calc_weight(TX_ACCEPTER, psbt));
-
-	/* Sometimes feerate_max is some absurdly high value, in that case we
-	 * give a fee warning based of a multiple of the min value. */
-	amount_sat_mul(&min_multiplied, min_accepter_fee, 5);
-	max_accepter_fee = SAT_MIN(min_multiplied, max_accepter_fee);
-	if (amount_sat_greater(accepter_penalty_fee, max_accepter_fee))
-		max_accepter_fee = accepter_penalty_fee;
-
-	amount_sat_mul(&min_multiplied, min_initiator_fee, 5);
-	max_initiator_fee = SAT_MIN(min_multiplied, max_initiator_fee);
-	if (amount_sat_greater(initiator_penalty_fee, max_initiator_fee))
-		max_initiator_fee = initiator_penalty_fee;
+	max_accepter_fee = amount_tx_fee(peer->feerate_opening,
+					 calc_weight(TX_ACCEPTER, psbt, false));
+	max_initiator_fee = amount_tx_fee(peer->feerate_opening,
+					  calc_weight(TX_INITIATOR, psbt, opener));
+
+	if (opener) {
+		status_debug("User specified fee of %s. Opening feerate %"PRIu32
+			     " * weight %lu / 1000 = %s",
+			     fmt_amount_m_as_sat(tmpctx, initiator_fee),
+			     peer->feerate_opening,
+			     calc_weight(TX_INITIATOR, psbt, false),
+			     fmt_amount_sat(tmpctx, max_initiator_fee));
+	}
 
 	/* Check initiator fee */
 	if (amount_msat_less_sat(initiator_fee, min_initiator_fee)) {
@@ -3512,23 +3527,38 @@ static struct amount_sat check_balances(struct peer *peer,
 		&& amount_msat_greater_sat(initiator_fee, max_initiator_fee)) {
 		msg = towire_channeld_splice_feerate_error(NULL, initiator_fee,
 							   true);
+		status_debug("Our own fee (%s) is too high to use without"
+			     " forcing. Opening feerate %"PRIu32
+			     " x weight %lu / 1000 = %s (max)",
+			     fmt_amount_m_as_sat(tmpctx, initiator_fee),
+			     peer->feerate_opening,
+			     calc_weight(TX_INITIATOR, psbt, false),
+			     fmt_amount_sat(tmpctx, max_initiator_fee));
+
 		wire_sync_write(MASTER_FD, take(msg));
+
 		splice_abort(peer,
-				 "Our own fee (%s) was too high, max without"
-				 " forcing is %s.",
-				 fmt_amount_msat(tmpctx, initiator_fee),
-				 fmt_amount_sat(tmpctx, max_initiator_fee));
+			     "Our own fee (%s) is too high to use without"
+			     " forcing. Opening feerate %"PRIu32
+			     " x weight %lu / 1000 = %s (max)",
+			     fmt_amount_m_as_sat(tmpctx, initiator_fee),
+			     peer->feerate_opening,
+			     calc_weight(TX_INITIATOR, psbt, false),
+			     fmt_amount_sat(tmpctx, max_initiator_fee));
 	}
 	/* Check accepter fee */
 	if (amount_msat_less_sat(accepter_fee, min_accepter_fee)) {
 		msg = towire_channeld_splice_feerate_error(NULL, accepter_fee,
 							   false);
 		wire_sync_write(MASTER_FD, take(msg));
 		splice_abort(peer,
-				 "%s fee (%s) was too low, must be at least %s",
-				 opener ? "Your" : "Our",
-				 fmt_amount_msat(tmpctx, accepter_fee),
-				 fmt_amount_sat(tmpctx, min_accepter_fee));
+			     "%s fee (%s) was too low, must be at least %s"
+			     " weight: %"PRIu64", feerate_max: %"PRIu32,
+			     opener ? "Your" : "Our",
+			     fmt_amount_msat(tmpctx, accepter_fee),
+			     fmt_amount_sat(tmpctx, min_accepter_fee),
+			     calc_weight(TX_INITIATOR, psbt, false),
+			     peer->feerate_opening);
 	}
 	if (!peer->splicing->force_feerate && !opener
 		&& amount_msat_greater_sat(accepter_fee, max_accepter_fee)) {
@@ -3981,6 +4011,9 @@ static void resume_splice_negotiation(struct peer *peer,
 
 		peer->splicing = tal_free(peer->splicing);
 
+		if (our_role == TX_INITIATOR)
+			calc_weight(TX_INITIATOR, current_psbt, true);
+
 		final_tx = bitcoin_tx_with_psbt(tmpctx, current_psbt);
 		msg = towire_channeld_splice_confirmed_signed(tmpctx, final_tx,
 							      new_output_index);

lightningd/channel_control.c

@@ -588,9 +588,10 @@ static void send_splice_tx(struct channel *channel,
 	u8* tx_bytes = linearize_tx(tmpctx, tx);
 
 	log_debug(channel->log,
-		  "Broadcasting splice tx %s for channel %s.",
+		  "Broadcasting splice tx %s for channel %s. Final weight %lu",
 		  tal_hex(tmpctx, tx_bytes),
-		  fmt_channel_id(tmpctx, &channel->cid));
+		  fmt_channel_id(tmpctx, &channel->cid),
+		  bitcoin_tx_weight(tx));
 
 	struct send_splice_info *info = tal(NULL, struct send_splice_info);
 

openingd/dualopend.c

@@ -818,14 +818,14 @@ static char *check_balances(const tal_t *ctx,
 			assert(ok);
 
 			initiator_weight +=
-				psbt_input_get_weight(psbt, i);
+				psbt_input_get_weight(psbt, i, PSBT_GUESS_ZERO);
 		} else {
 			ok = amount_sat_add(&accepter_inputs,
 					    accepter_inputs, amt);
 			assert(ok);
 
 			accepter_weight +=
-				psbt_input_get_weight(psbt, i);
+				psbt_input_get_weight(psbt, i, PSBT_GUESS_ZERO);
 		}
 	}
 	tot_output_amt = AMOUNT_SAT(0);