bitcoin: add weight calculation helpers.

These are pulled from wallet/wallet.c, with the fix now that we grind sigs.

This reduces the fees we pay slightly, as you can see in the coinmoves changes.

I now print out all the coin moves in suitable format before we match:
you only see this if the test fails, but it's really helpful.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

bitcoin/tx.c

@@ -731,3 +731,75 @@ wally_tx_output_get_amount(const struct wally_tx_output *output)
 
 	return amount;
 }
+
+/* Various weights of transaction parts. */
+size_t bitcoin_tx_core_weight(size_t num_inputs, size_t num_outputs)
+{
+	size_t weight;
+
+	/* version, input count, output count, locktime */
+	weight = (4 + varint_size(num_inputs) + varint_size(num_outputs) + 4)
+		* 4;
+
+	/* Add segwit fields: marker + flag */
+	weight += 1 + 1;
+
+	/* A couple of things need to change for elements: */
+	if (chainparams->is_elements) {
+                /* Each transaction has surjection and rangeproof (both empty
+		 * for us as long as we use unblinded L-BTC transactions). */
+		weight += 2 * 4;
+
+		/* An elements transaction has 1 additional output for fees */
+		weight += bitcoin_tx_output_weight(0);
+	}
+	return weight;
+}
+
+size_t bitcoin_tx_output_weight(size_t outscript_len)
+{
+	size_t weight;
+
+	/* amount, len, scriptpubkey */
+	weight = (8 + varint_size(outscript_len) + outscript_len) * 4;
+
+	if (chainparams->is_elements) {
+		/* Each output additionally has an asset_tag (1 + 32), value
+		 * is prefixed by a version (1 byte), an empty nonce (1
+		 * byte), two empty proofs (2 bytes). */
+		weight += (32 + 1 + 1 + 1) * 4;
+	}
+
+	return weight;
+}
+
+/* We grind signatures to get them down to 71 bytes (+1 for sighash flags) */
+size_t bitcoin_tx_input_sig_weight(void)
+{
+	return 1 + 71 + 1;
+}
+
+/* We only do segwit inputs, and we assume witness is sig + key  */
+size_t bitcoin_tx_simple_input_weight(bool p2sh)
+{
+	size_t weight;
+
+	/* Input weight: txid + index + sequence */
+	weight = (32 + 4 + 4) * 4;
+
+	/* We always encode the length of the script, even if empty */
+	weight += 1 * 4;
+
+	/* P2SH variants include push of <0 <20-byte-key-hash>> */
+	if (p2sh)
+		weight += 23 * 4;
+
+	/* Account for witness (1 byte count + sig + key) */
+	weight += 1 + (bitcoin_tx_input_sig_weight() + 1 + 33);
+
+	/* Elements inputs have 6 bytes of blank proofs attached. */
+	if (chainparams->is_elements)
+		weight += 6;
+
+	return weight;
+}

bitcoin/tx.h

@@ -230,4 +230,15 @@ void towire_bitcoin_tx(u8 **pptr, const struct bitcoin_tx *tx);
 void towire_bitcoin_tx_output(u8 **pptr, const struct bitcoin_tx_output *output);
 
 int wally_tx_clone(struct wally_tx *tx, struct wally_tx **output);
+
+/* Various weights of transaction parts. */
+size_t bitcoin_tx_core_weight(size_t num_inputs, size_t num_outputs);
+size_t bitcoin_tx_output_weight(size_t outscript_len);
+
+/* Weight to push sig on stack. */
+size_t bitcoin_tx_input_sig_weight(void);
+
+/* We only do segwit inputs, and we assume witness is sig + key  */
+size_t bitcoin_tx_simple_input_weight(bool p2sh);
+
 #endif /* LIGHTNING_BITCOIN_TX_H */

common/utxo.c

@@ -113,3 +113,8 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx,
 
 	return tx;
 }
+
+size_t utxo_spend_weight(const struct utxo *utxo)
+{
+	return bitcoin_tx_simple_input_weight(utxo->is_p2sh);
+}

common/utxo.h

@@ -59,4 +59,6 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx,
 				     u32 nlocktime,
 				     u32 nsequence);
 
+/* Estimate of (signed) UTXO weight in transaction */
+size_t utxo_spend_weight(const struct utxo *utxo);
 #endif /* LIGHTNING_COMMON_UTXO_H */

lightningd/closing_control.c

@@ -69,7 +69,7 @@ static bool closing_fee_is_acceptable(struct lightningd *ld,
 		  type_to_string(tmpctx, struct amount_sat, &last_fee));
 
 	/* Weight once we add in sigs. */
-	weight = bitcoin_tx_weight(tx) + 74 * 2;
+	weight = bitcoin_tx_weight(tx) + bitcoin_tx_input_sig_weight() * 2;
 
 	/* If we don't have a feerate estimate, this gives feerate_floor */
 	min_feerate = feerate_min(ld, &feerate_unknown);

tests/test_misc.py

@@ -623,40 +623,40 @@ def test_withdraw_misc(node_factory, bitcoind, chainparams):
         {'type': 'chain_mvt', 'credit': 2000000000, 'debit': 0, 'tag': 'deposit'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 1993730000, 'tag': 'withdrawal'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 1993745000, 'tag': 'withdrawal'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 2000000000, 'tag': 'withdrawal'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 6270000, 'tag': 'chain_fees'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 6255000, 'tag': 'chain_fees'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 1993730000, 'tag': 'withdrawal'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 1993745000, 'tag': 'withdrawal'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 2000000000, 'tag': 'withdrawal'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 6270000, 'tag': 'chain_fees'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 6255000, 'tag': 'chain_fees'},
-        {'type': 'chain_mvt', 'credit': 1993730000, 'debit': 0, 'tag': 'deposit'},
+        {'type': 'chain_mvt', 'credit': 1993745000, 'debit': 0, 'tag': 'deposit'},
-        {'type': 'chain_mvt', 'credit': 1993730000, 'debit': 0, 'tag': 'deposit'},
+        {'type': 'chain_mvt', 'credit': 1993745000, 'debit': 0, 'tag': 'deposit'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 1993730000, 'tag': 'withdrawal'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 1993745000, 'tag': 'withdrawal'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 2000000000, 'tag': 'withdrawal'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 6270000, 'tag': 'chain_fees'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 6255000, 'tag': 'chain_fees'},
-        {'type': 'chain_mvt', 'credit': 1993730000, 'debit': 0, 'tag': 'deposit'},
+        {'type': 'chain_mvt', 'credit': 1993745000, 'debit': 0, 'tag': 'deposit'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 1993370000, 'tag': 'withdrawal'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 1993385000, 'tag': 'withdrawal'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 2000000000, 'tag': 'withdrawal'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 6630000, 'tag': 'chain_fees'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 6615000, 'tag': 'chain_fees'},
-        {'type': 'chain_mvt', 'credit': 1993370000, 'debit': 0, 'tag': 'deposit'},
+        {'type': 'chain_mvt', 'credit': 1993385000, 'debit': 0, 'tag': 'deposit'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 11961030000, 'tag': 'withdrawal'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 11961135000, 'tag': 'withdrawal'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 13530000, 'tag': 'chain_fees'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 13485000, 'tag': 'chain_fees'},
-        {'type': 'chain_mvt', 'credit': 11961030000, 'debit': 0, 'tag': 'deposit'},
+        {'type': 'chain_mvt', 'credit': 11961135000, 'debit': 0, 'tag': 'deposit'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 11957378000, 'tag': 'withdrawal'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 11957490000, 'tag': 'withdrawal'},
-        {'type': 'chain_mvt', 'credit': 0, 'debit': 3652000, 'tag': 'chain_fees'},
+        {'type': 'chain_mvt', 'credit': 0, 'debit': 3645000, 'tag': 'chain_fees'},
     ]
     check_coin_moves(l1, 'wallet', wallet_moves, chainparams)
 

tests/test_plugin.py

@@ -1408,10 +1408,10 @@ def test_coin_movement_notices(node_factory, bitcoind, chainparams):
         l2_wallet_mvts = [
             {'type': 'chain_mvt', 'credit': 2000000000, 'debit': 0, 'tag': 'deposit'},
             {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'},
-            {'type': 'chain_mvt', 'credit': 0, 'debit': 995418000, 'tag': 'withdrawal'},
+            {'type': 'chain_mvt', 'credit': 0, 'debit': 995425000, 'tag': 'withdrawal'},
             {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tag': 'withdrawal'},
-            {'type': 'chain_mvt', 'credit': 0, 'debit': 4582000, 'tag': 'chain_fees'},
+            {'type': 'chain_mvt', 'credit': 0, 'debit': 4575000, 'tag': 'chain_fees'},
-            {'type': 'chain_mvt', 'credit': 995418000, 'debit': 0, 'tag': 'deposit'},
+            {'type': 'chain_mvt', 'credit': 995425000, 'debit': 0, 'tag': 'deposit'},
             {'type': 'chain_mvt', 'credit': 100001000, 'debit': 0, 'tag': 'deposit'},
             {'type': 'chain_mvt', 'credit': 944570000, 'debit': 0, 'tag': 'deposit'},
         ]

tests/test_wallet.py

@@ -710,7 +710,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
         # do proper acccounting for it.
         {'type': 'chain_mvt', 'credit': 0, 'debit': 4000000000, 'tag': 'withdrawal'},
         {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'chain_fees'},
-        {'type': 'chain_mvt', 'credit': 988255000, 'debit': 0, 'tag': 'deposit'},
+        {'type': 'chain_mvt', 'credit': 988285000, 'debit': 0, 'tag': 'deposit'},
     ]
     check_coin_moves(l1, 'wallet', wallet_coin_mvts, chainparams)
 

tests/utils.py

@@ -1,6 +1,7 @@
 from pyln.testing.utils import TEST_NETWORK, SLOW_MACHINE, TIMEOUT, VALGRIND, DEVELOPER, DEPRECATED_APIS  # noqa: F401
 from pyln.testing.utils import env, only_one, wait_for, write_config, TailableProc, sync_blockheight, wait_channel_quiescent, get_tx_p2wsh_outnum  # noqa: F401
 import bitstring
+from pyln.client import Millisatoshi
 
 EXPERIMENTAL_FEATURES = env("EXPERIMENTAL_FEATURES", "0") == "1"
 COMPAT = env("COMPAT", "1") == "1"
@@ -53,6 +54,13 @@ def check_coin_moves(n, account_id, expected_moves, chainparams):
     moves = n.rpc.call('listcoinmoves_plugin')['coin_moves']
     node_id = n.info['id']
     acct_moves = [m for m in moves if m['account_id'] == account_id]
+    for mv in acct_moves:
+        print("{{'type': '{}', 'credit': {}, 'debit': {}, 'tag': '{}'}},"
+              .format(mv['type'],
+                      Millisatoshi(mv['credit']).millisatoshis,
+                      Millisatoshi(mv['debit']).millisatoshis,
+                      mv['tag']))
+
     assert len(acct_moves) == len(expected_moves)
     for mv, exp in list(zip(acct_moves, expected_moves)):
         assert mv['version'] == 1

wallet/wallet.c

@@ -418,34 +418,13 @@ static const struct utxo **wallet_select(const tal_t *ctx, struct wallet *w,
 	const struct utxo **utxos = tal_arr(ctx, const struct utxo *, 0);
 	tal_add_destructor2(utxos, destroy_utxos, w);
 
-	/* version, input count, output count, locktime */
+	/* We assume < 253 inputs, and margin is tiny if we're wrong */
-	weight = (4 + 1 + 1 + 4) * 4;
+	weight = bitcoin_tx_core_weight(1, num_outputs)
-
+		+ bitcoin_tx_output_weight(outscriptlen);
-	/* Add segwit fields: marker + flag */
-	weight += 1 + 1;
-
-	/* The main output: amount, len, scriptpubkey */
-	weight += (8 + 1 + outscriptlen) * 4;
 
 	/* Change output will be P2WPKH */
 	if (may_have_change)
-		weight += (8 + 1 + BITCOIN_SCRIPTPUBKEY_P2WPKH_LEN) * 4;
+		weight += bitcoin_tx_output_weight(BITCOIN_SCRIPTPUBKEY_P2WPKH_LEN);
-
-	/* A couple of things need to change for elements: */
-	if (chainparams->is_elements) {
-                /* Each transaction has surjection and rangeproof (both empty
-		 * for us as long as we use unblinded L-BTC transactions). */
-		weight += 2 * 4;
-
-		/* Each output additionally has an asset_tag (1 + 32), value
-		 * is prefixed by a version (1 byte), an empty nonce (1
-		 * byte), two empty proofs (2 bytes). */
-		weight += (32 + 1 + 1 + 1) * 4 * num_outputs;
-
-		/* An elements transaction has 1 additional output for fees */
-		weight += (8 + 1) * 4; /* Bitcoin style output */
-		weight += (32 + 1 + 1 + 1) * 4; /* Elements added fields */
-	}
 
 	*fee_estimate = AMOUNT_SAT(0);
 	*satoshi_in = AMOUNT_SAT(0);
@@ -453,7 +432,6 @@ static const struct utxo **wallet_select(const tal_t *ctx, struct wallet *w,
 	available = wallet_get_utxos(ctx, w, output_state_available);
 
 	for (i = 0; i < tal_count(available); i++) {
-		size_t input_weight;
 		struct amount_sat needed;
 		struct utxo *u = tal_steal(utxos, available[i]);
 
@@ -473,24 +451,7 @@ static const struct utxo **wallet_select(const tal_t *ctx, struct wallet *w,
 			output_state_available, output_state_reserved))
 			fatal("Unable to reserve output");
 
-		/* Input weight: txid + index + sequence */
+		weight += bitcoin_tx_simple_input_weight(u->is_p2sh);
-		input_weight = (32 + 4 + 4) * 4;
-
-		/* We always encode the length of the script, even if empty */
-		input_weight += 1 * 4;
-
-		/* P2SH variants include push of <0 <20-byte-key-hash>> */
-		if (u->is_p2sh)
-			input_weight += 23 * 4;
-
-		/* Account for witness (1 byte count + sig + key) */
-		input_weight += 1 + (1 + 73 + 1 + 33);
-
-		/* Elements inputs have 6 bytes of blank proofs attached. */
-		if (chainparams->is_elements)
-			input_weight += 6;
-
-		weight += input_weight;
 
 		if (!amount_sat_add(satoshi_in, *satoshi_in, u->amount))
 			fatal("Overflow in available satoshis %zu/%zu %s + %s",