script: consistently take the script length in identification functions

Standardizes the is_xxx script function all take a script length, and changes
their first-level callers to pass it. This has several knock on benefits:

- We remove the repeated tal_count/tal_bytelen calls on the script, in
  particular the redundant calls that result when we must check for multiple
  types of script - which is almost all cases.
- We remove the dependency on the memory being tal-allocated (It is, in
  all cases, but theres no reason we need to require that).
- We remove all cases where we create a copy of the script just to id it.
- We remove all allocations for non-interesting scripts while iterating block
  txs in process_getfilteredblock_step1().
- We remove all allocations *including for potentially interesting scripts* in
  topo_add_utxos().

Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>

bitcoin/script.c

@@ -476,10 +476,8 @@ u8 *p2wpkh_scriptcode(const tal_t *ctx, const struct pubkey *key)
 	return script;
 }
 
-bool is_p2pkh(const u8 *script, struct bitcoin_address *addr)
+bool is_p2pkh(const u8 *script, size_t script_len, struct bitcoin_address *addr)
 {
-	size_t script_len = tal_count(script);
-
 	if (script_len != BITCOIN_SCRIPTPUBKEY_P2PKH_LEN)
 		return false;
 	if (script[0] != OP_DUP)
@@ -497,10 +495,8 @@ bool is_p2pkh(const u8 *script, struct bitcoin_address *addr)
 	return true;
 }
 
-bool is_p2sh(const u8 *script, struct ripemd160 *addr)
+bool is_p2sh(const u8 *script, size_t script_len, struct ripemd160 *addr)
 {
-	size_t script_len = tal_count(script);
-
 	if (script_len != BITCOIN_SCRIPTPUBKEY_P2SH_LEN)
 		return false;
 	if (script[0] != OP_HASH160)
@@ -514,10 +510,8 @@ bool is_p2sh(const u8 *script, struct ripemd160 *addr)
 	return true;
 }
 
-bool is_p2wsh(const u8 *script, struct sha256 *addr)
+bool is_p2wsh(const u8 *script, size_t script_len, struct sha256 *addr)
 {
-	size_t script_len = tal_count(script);
-
 	if (script_len != BITCOIN_SCRIPTPUBKEY_P2WSH_LEN)
 		return false;
 	if (script[0] != OP_0)
@@ -529,10 +523,8 @@ bool is_p2wsh(const u8 *script, struct sha256 *addr)
 	return true;
 }
 
-bool is_p2wpkh(const u8 *script, struct bitcoin_address *addr)
+bool is_p2wpkh(const u8 *script, size_t script_len, struct bitcoin_address *addr)
 {
-	size_t script_len = tal_count(script);
-
 	if (script_len != BITCOIN_SCRIPTPUBKEY_P2WPKH_LEN)
 		return false;
 	if (script[0] != OP_0)
@@ -544,10 +536,8 @@ bool is_p2wpkh(const u8 *script, struct bitcoin_address *addr)
 	return true;
 }
 
-bool is_p2tr(const u8 *script, u8 xonly_pubkey[32])
+bool is_p2tr(const u8 *script, size_t script_len, u8 xonly_pubkey[32])
 {
-	size_t script_len = tal_count(script);
-
 	if (script_len != BITCOIN_SCRIPTPUBKEY_P2TR_LEN)
 		return false;
 	if (script[0] != OP_1)
@@ -560,17 +550,20 @@ bool is_p2tr(const u8 *script, u8 xonly_pubkey[32])
 	return true;
 }
 
-bool is_known_scripttype(const u8 *script)
+bool is_known_scripttype(const u8 *script, size_t script_len)
 {
-	return is_p2wpkh(script, NULL) || is_p2wsh(script, NULL)
-		|| is_p2sh(script, NULL) || is_p2pkh(script, NULL)
-		|| is_p2tr(script, NULL);
+	return is_p2wpkh(script, script_len, NULL)
+		|| is_p2wsh(script, script_len, NULL)
+		|| is_p2sh(script, script_len, NULL)
+		|| is_p2pkh(script, script_len, NULL)
+		|| is_p2tr(script, script_len, NULL);
 }
 
-bool is_known_segwit_scripttype(const u8 *script)
+bool is_known_segwit_scripttype(const u8 *script, size_t script_len)
 {
-	return is_p2wpkh(script, NULL) || is_p2wsh(script, NULL)
-		|| is_p2tr(script, NULL);
+	return is_p2wpkh(script, script_len, NULL)
+		|| is_p2wsh(script, script_len, NULL)
+		|| is_p2tr(script, script_len, NULL);
 }
 
 u8 **bitcoin_witness_sig_and_element(const tal_t *ctx,

bitcoin/script.h

@@ -159,25 +159,25 @@ u8 *bitcoin_wscript_anchor(const tal_t *ctx,
 			   const struct pubkey *funding_pubkey);
 
 /* Is this a pay to pubkey hash? (extract addr if not NULL) */
-bool is_p2pkh(const u8 *script, struct bitcoin_address *addr);
+bool is_p2pkh(const u8 *script, size_t script_len, struct bitcoin_address *addr);
 
 /* Is this a pay to script hash? (extract addr if not NULL) */
-bool is_p2sh(const u8 *script, struct ripemd160 *addr);
+bool is_p2sh(const u8 *script, size_t script_len, struct ripemd160 *addr);
 
 /* Is this (version 0) pay to witness script hash? (extract addr if not NULL) */
-bool is_p2wsh(const u8 *script, struct sha256 *addr);
+bool is_p2wsh(const u8 *script, size_t script_len, struct sha256 *addr);
 
 /* Is this (version 0) pay to witness pubkey hash? (extract addr if not NULL) */
-bool is_p2wpkh(const u8 *script, struct bitcoin_address *addr);
+bool is_p2wpkh(const u8 *script, size_t script_len, struct bitcoin_address *addr);
 
 /* Is this a taproot output? (extract xonly_pubkey bytes if not NULL) */
-bool is_p2tr(const u8 *script, u8 xonly_pubkey[32]);
+bool is_p2tr(const u8 *script, size_t script_len, u8 xonly_pubkey[32]);
 
 /* Is this one of the above script types? */
-bool is_known_scripttype(const u8 *script);
+bool is_known_scripttype(const u8 *script, size_t script_len);
 
 /* Is this a witness script type? */
-bool is_known_segwit_scripttype(const u8 *script);
+bool is_known_segwit_scripttype(const u8 *script, size_t script_len);
 
 /* Is this a to-remote witness script (used for option_anchor_outputs)? */
 bool is_to_remote_anchored_witness_script(const u8 *script, size_t script_len);

channeld/watchtower.c

@@ -80,8 +80,12 @@ penalty_tx_create(const tal_t *ctx,
 
 	bitcoin_tx_add_output(tx, final_scriptpubkey, NULL, to_them_sats);
 	assert((final_index == NULL) == (final_ext_key == NULL));
-	if (final_index)
-		psbt_add_keypath_to_last_output(tx, *final_index, final_ext_key, is_p2tr(final_scriptpubkey, NULL));
+	if (final_index) {
+		size_t script_len = tal_bytelen(final_scriptpubkey);
+		bool is_tr = is_p2tr(final_scriptpubkey, script_len, NULL);
+		psbt_add_keypath_to_last_output(tx, *final_index,
+						final_ext_key, is_tr);
+        }
 
 	/* Worst-case sig is 73 bytes */
 	weight = bitcoin_tx_weight(tx) + 1 + 3 + 73 + 0 + tal_count(wscript);

common/addr.c

@@ -7,30 +7,31 @@
 
 char *encode_scriptpubkey_to_addr(const tal_t *ctx,
 				  const struct chainparams *chainparams,
-				  const u8 *scriptPubkey)
+				  const u8 *scriptpubkey)
 {
 	char *out;
-	size_t scriptLen = tal_bytelen(scriptPubkey);
+	const size_t script_len = tal_bytelen(scriptpubkey);
 	struct bitcoin_address pkh;
 	struct ripemd160 sh;
 	int witver;
 
-	if (is_p2pkh(scriptPubkey, &pkh))
+	if (is_p2pkh(scriptpubkey, script_len, &pkh))
 		return bitcoin_to_base58(ctx, chainparams, &pkh);
 
-	if (is_p2sh(scriptPubkey, &sh))
+	if (is_p2sh(scriptpubkey, script_len, &sh))
 		return p2sh_to_base58(ctx, chainparams, &sh);
 
 	out = tal_arr(ctx, char, 73 + strlen(chainparams->onchain_hrp));
-	if (is_p2tr(scriptPubkey, NULL))
+	if (is_p2tr(scriptpubkey, script_len, NULL))
 		witver = 1;
-	else if (is_p2wpkh(scriptPubkey, NULL) || is_p2wsh(scriptPubkey, NULL))
+	else if (is_p2wpkh(scriptpubkey, script_len, NULL)
+		 || is_p2wsh(scriptpubkey, script_len, NULL))
 		witver = 0;
 	else {
 		return tal_free(out);
 	}
 	if (!segwit_addr_encode(out, chainparams->onchain_hrp, witver,
-				scriptPubkey + 2, scriptLen - 2))
+				scriptpubkey + 2, script_len - 2))
 		return tal_free(out);
 
 	return out;

common/addr.h

@@ -6,6 +6,6 @@
 /* Given a scriptPubkey, return an encoded address for p2pkh/p2w{pkh,sh}/p2tr */
 char *encode_scriptpubkey_to_addr(const tal_t *ctx,
 				  const struct chainparams *chainparams,
-				  const u8 *scriptPubkey);
+				  const u8 *scriptpubkey);
 
 #endif /* LIGHTNING_COMMON_ADDR_H */

common/bolt11.c

@@ -1133,22 +1133,23 @@ static void encode_f(u5 **data, const u8 *fallback)
 	struct bitcoin_address pkh;
 	struct ripemd160 sh;
 	struct sha256 wsh;
+	const size_t fallback_len = tal_bytelen(fallback);
 
 	/* BOLT #11:
 	 *
 	 * for Bitcoin payments... MUST set an `f` field to a valid
 	 * witness version and program, OR to `17` followed by a
 	 * public key hash, OR to `18` followed by a script hash.
 	 */
-	if (is_p2pkh(fallback, &pkh)) {
+	if (is_p2pkh(fallback, fallback_len, &pkh)) {
 		push_fallback_addr(data, 17, &pkh, sizeof(pkh));
-	} else if (is_p2sh(fallback, &sh)) {
+	} else if (is_p2sh(fallback, fallback_len, &sh)) {
 		push_fallback_addr(data, 18, &sh, sizeof(sh));
-	} else if (is_p2wpkh(fallback, &pkh)) {
+	} else if (is_p2wpkh(fallback, fallback_len, &pkh)) {
 		push_fallback_addr(data, 0, &pkh, sizeof(pkh));
-	} else if (is_p2wsh(fallback, &wsh)) {
+	} else if (is_p2wsh(fallback, fallback_len, &wsh)) {
 		push_fallback_addr(data, 0, &wsh, sizeof(wsh));
-	} else if (tal_count(fallback) > 1
+	} else if (fallback_len > 1
 		   && fallback[0] >= 0x50
 		   && fallback[0] < (0x50+16)) {
 		/* Other (future) witness versions: turn OP_N into N */
@@ -1157,7 +1158,7 @@ static void encode_f(u5 **data, const u8 *fallback)
 	} else {
 		/* Copy raw. */
 		push_field(data, 'f',
-			   fallback, tal_count(fallback) * CHAR_BIT);
+			   fallback, fallback_len * CHAR_BIT);
 	}
 }
 

common/bolt11_json.c

@@ -15,17 +15,18 @@ static void json_add_fallback(struct json_stream *response,
 			      const struct chainparams *chain)
 {
 	char *addr;
+	const size_t fallback_len = tal_bytelen(fallback);
 
 	json_object_start(response, fieldname);
-	if (is_p2pkh(fallback, NULL)) {
+	if (is_p2pkh(fallback, fallback_len, NULL)) {
 		json_add_string(response, "type", "P2PKH");
-	} else if (is_p2sh(fallback, NULL)) {
+	} else if (is_p2sh(fallback, fallback_len, NULL)) {
 		json_add_string(response, "type", "P2SH");
-	} else if (is_p2wpkh(fallback, NULL)) {
+	} else if (is_p2wpkh(fallback, fallback_len, NULL)) {
 		json_add_string(response, "type", "P2WPKH");
-	} else if (is_p2wsh(fallback, NULL)) {
+	} else if (is_p2wsh(fallback, fallback_len, NULL)) {
 		json_add_string(response, "type", "P2WSH");
-	} else if (is_p2tr(fallback, NULL)) {
+	} else if (is_p2tr(fallback, fallback_len, NULL)) {
 		json_add_string(response, "type", "P2TR");
 	}
 

common/close_tx.c

@@ -50,9 +50,12 @@ struct bitcoin_tx *create_close_tx(const tal_t *ctx,
 		/* One output is to us. */
 		bitcoin_tx_add_output(tx, script, NULL, to_us);
 		assert((local_wallet_index == NULL) == (local_wallet_ext_key == NULL));
-		if (local_wallet_index)
+		if (local_wallet_index) {
+			size_t script_len = tal_bytelen(script);
 			psbt_add_keypath_to_last_output(
-				tx, *local_wallet_index, local_wallet_ext_key, is_p2tr(script, NULL));
+				tx, *local_wallet_index, local_wallet_ext_key,
+				is_p2tr(script, script_len, NULL));
+                }
 		num_outputs++;
 	}
 

common/interactivetx.c

@@ -80,13 +80,15 @@ static bool is_segwit_output(const tal_t *ctx,
 			     struct wally_tx_output *output,
 			     const u8 *redeemscript)
 {
-	const u8 *maybe_witness;
-	if (tal_bytelen(redeemscript) > 0)
-		maybe_witness = redeemscript;
-	else
-		maybe_witness = cln_wally_tx_output_get_script(ctx, output);
+	const u8 *maybe_witness = redeemscript;
+	size_t script_len = tal_bytelen(maybe_witness);
 
-	return is_known_segwit_scripttype(maybe_witness);
+	if (!script_len) {
+		maybe_witness = output->script;
+		script_len = output->script_len;
+        }
+
+	return is_known_segwit_scripttype(maybe_witness, script_len);
 }
 
 /* Return first non-handled message or NULL if connection is aborted */
@@ -627,7 +629,7 @@ char *process_interactivetx_updates(const tal_t *ctx,
 			 * The receiving node: ...
 			 * - MAY fail the negotiation if `script`
 			 *   is non-standard */
-			if (!is_known_scripttype(scriptpubkey))
+			if (!is_known_scripttype(scriptpubkey, tal_bytelen(scriptpubkey)))
 				return tal_fmt(ctx, "Script is not standard");
 
 			/*

common/psbt_open.c

@@ -382,6 +382,7 @@ bool psbt_has_required_fields(struct wally_psbt *psbt)
 	u64 serial_id;
 	for (size_t i = 0; i < psbt->num_inputs; i++) {
 		const struct wally_map_item *redeem_script;
+		const struct wally_tx_output *txout;
 		struct wally_psbt_input *input = &psbt->inputs[i];
 
 		if (!psbt_get_serial_id(&input->unknowns, &serial_id))
@@ -391,13 +392,14 @@ bool psbt_has_required_fields(struct wally_psbt *psbt)
 		if (!input->utxo)
 			return false;
 
-		/* If is P2SH, redeemscript must be present */
-		assert(psbt->inputs[i].index < input->utxo->num_outputs);
-		const u8 *outscript =
-			cln_wally_tx_output_get_script(tmpctx,
-				&input->utxo->outputs[psbt->inputs[i].index]);
-		redeem_script = wally_map_get_integer(&psbt->inputs[i].psbt_fields, /* PSBT_IN_REDEEM_SCRIPT */ 0x04);
-		if (is_p2sh(outscript, NULL) && (!redeem_script || redeem_script->value_len == 0))
+		assert(input->index < input->utxo->num_outputs);
+		txout = &input->utxo->outputs[input->index];
+		if (!is_p2sh(txout->script, txout->script_len, NULL))
+			continue;
+		/* P2SH: redeemscript must be present */
+		const u32 key = 0x04; /* PSBT_IN_REDEEM_SCRIPT */
+		redeem_script = wally_map_get_integer(&input->psbt_fields, key);
+		if (!redeem_script || !redeem_script->value_len)
 			return false;
 	}
 

common/shutdown_scriptpubkey.c

@@ -8,11 +8,11 @@
  *         push of 2 to 40 bytes
  *         (witness program versions 1 through 16)
  */
-static bool is_valid_witnessprog(const u8 *scriptpubkey)
+static bool is_valid_witnessprog(const u8 *scriptpubkey, size_t scriptpubkey_len)
 {
 	size_t pushlen;
 
-	if (tal_bytelen(scriptpubkey) < 2)
+	if (scriptpubkey_len < 2)
 		return false;
 
 	switch (scriptpubkey[0]) {
@@ -39,7 +39,7 @@ static bool is_valid_witnessprog(const u8 *scriptpubkey)
 
 	pushlen = scriptpubkey[1];
 	/* Must be all of the rest of scriptpubkey */
-	if (2 + pushlen != tal_bytelen(scriptpubkey)) {
+	if (2 + pushlen != scriptpubkey_len) {
 		return false;
 	}
 
@@ -50,13 +50,14 @@ bool valid_shutdown_scriptpubkey(const u8 *scriptpubkey,
 				 bool anysegwit,
 				 bool allow_oldstyle)
 {
+	const size_t script_len = tal_bytelen(scriptpubkey);
 	if (allow_oldstyle) {
-		if (is_p2pkh(scriptpubkey, NULL)
-		    || is_p2sh(scriptpubkey, NULL))
+		if (is_p2pkh(scriptpubkey, script_len, NULL)
+		    || is_p2sh(scriptpubkey, script_len, NULL))
 			return true;
 	}
 
-	return is_p2wpkh(scriptpubkey, NULL)
-		|| is_p2wsh(scriptpubkey, NULL)
-		|| (anysegwit && is_valid_witnessprog(scriptpubkey));
+	return is_p2wpkh(scriptpubkey, script_len, NULL)
+		|| is_p2wsh(scriptpubkey, script_len, NULL)
+		|| (anysegwit && is_valid_witnessprog(scriptpubkey, script_len));
 }

devtools/bolt11-cli.c

@@ -123,23 +123,25 @@ int main(int argc, char *argv[])
                 struct bitcoin_address pkh;
                 struct ripemd160 sh;
                 struct sha256 wsh;
+                const u8 *fallback = b11->fallbacks[i];
+                const size_t fallback_len = tal_bytelen(fallback);
 
-		printf("fallback: %s\n", tal_hex(ctx, b11->fallbacks[i]));
-                if (is_p2pkh(b11->fallbacks[i], &pkh)) {
+		printf("fallback: %s\n", tal_hex(ctx, fallback));
+                if (is_p2pkh(fallback, fallback_len, &pkh)) {
 			printf("fallback-P2PKH: %s\n",
 			       bitcoin_to_base58(ctx, b11->chain,
 						 &pkh));
-                } else if (is_p2sh(b11->fallbacks[i], &sh)) {
+                } else if (is_p2sh(fallback, fallback_len, &sh)) {
 			printf("fallback-P2SH: %s\n",
 			       p2sh_to_base58(ctx,
 					      b11->chain,
 					      &sh));
-                } else if (is_p2wpkh(b11->fallbacks[i], &pkh)) {
+                } else if (is_p2wpkh(fallback, fallback_len, &pkh)) {
                         char out[73 + strlen(b11->chain->onchain_hrp)];
                         if (segwit_addr_encode(out, b11->chain->onchain_hrp, 0,
                                                (const u8 *)&pkh, sizeof(pkh)))
 				printf("fallback-P2WPKH: %s\n", out);
-                } else if (is_p2wsh(b11->fallbacks[i], &wsh)) {
+                } else if (is_p2wsh(fallback, fallback_len, &wsh)) {
                         char out[73 + strlen(b11->chain->onchain_hrp)];
                         if (segwit_addr_encode(out, b11->chain->onchain_hrp, 0,
                                                (const u8 *)&wsh, sizeof(wsh)))