tx: Strengthen transaction construction checks

We roll the `elements_add_fee_output` function and the cropping of
overallocated arrays into the `bitcoin_tx_finalize` function. This is supposed
to be the final cleanup and compaction step before a tx can be sent to bitcoin
or passed off to other daemons.

This is the cleanup promised in ElementsProject#3491

Author: cdecker

bitcoin/tx.c

@@ -160,6 +160,12 @@ bool bitcoin_tx_check(const struct bitcoin_tx *tx)
 	size_t written;
 	int flags = WALLY_TX_FLAG_USE_WITNESS;
 
+	if (tal_count(tx->input_amounts) != tx->wtx->num_inputs)
+		return false;
+
+	if (tal_count(tx->output_witscripts) != tx->wtx->num_outputs)
+		return false;
+
 	if (wally_tx_get_length(tx->wtx, flags, &written) != WALLY_OK)
 		return false;
 
@@ -408,6 +414,18 @@ struct bitcoin_tx *bitcoin_tx(const tal_t *ctx,
 	return tx;
 }
 
+void bitcoin_tx_finalize(struct bitcoin_tx *tx)
+{
+	size_t num_outputs, num_inputs;
+	elements_tx_add_fee_output(tx);
+
+	num_outputs = tx->wtx->num_outputs;
+	tal_resize(&(tx->output_witscripts), num_outputs);
+
+	num_inputs = tx->wtx->num_inputs;
+	tal_resize(&tx->input_amounts, num_inputs);
+}
+
 struct bitcoin_tx *pull_bitcoin_tx(const tal_t *ctx, const u8 **cursor,
 				   size_t *max)
 {

bitcoin/tx.h

@@ -156,6 +156,15 @@ void bitcoin_tx_input_get_txid(const struct bitcoin_tx *tx, int innum,
  */
 bool bitcoin_tx_check(const struct bitcoin_tx *tx);
 
+
+/**
+ * Finalize a transaction by truncating overallocated and temporary
+ * fields. This includes adding a fee output for elements transactions or
+ * adjusting an existing fee output, and resizing metadata arrays for inputs
+ * and outputs.
+ */
+void bitcoin_tx_finalize(struct bitcoin_tx *tx);
+
 /**
  * Add an explicit fee output if necessary.
  *

channeld/commit_tx.c

@@ -305,8 +305,8 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
 	u32 sequence = (0x80000000 | ((obscured_commitment_number>>24) & 0xFFFFFF));
 	bitcoin_tx_add_input(tx, funding_txid, funding_txout, sequence, funding, NULL);
 
-	elements_tx_add_fee_output(tx);
-	tal_resize(&(tx->output_witscripts), tx->wtx->num_outputs);
+	bitcoin_tx_finalize(tx);
+	assert(bitcoin_tx_check(tx));
 
 	return tx;
 }

common/close_tx.c

@@ -61,8 +61,8 @@ struct bitcoin_tx *create_close_tx(const tal_t *ctx,
 		return tal_free(tx);
 
 	permute_outputs(tx, NULL, NULL);
-	elements_tx_add_fee_output(tx);
 
+	bitcoin_tx_finalize(tx);
 	assert(bitcoin_tx_check(tx));
 	return tx;
 }

common/htlc_tx.c

@@ -61,7 +61,9 @@ static struct bitcoin_tx *htlc_tx(const tal_t *ctx,
 	wscript = bitcoin_wscript_htlc_tx(tx, to_self_delay, revocation_pubkey,
 					  local_delayedkey);
 	bitcoin_tx_add_output(tx, scriptpubkey_p2wsh(tx, wscript), amount);
-	elements_tx_add_fee_output(tx);
+
+	bitcoin_tx_finalize(tx);
+	assert(bitcoin_tx_check(tx));
 
 	tal_free(wscript);
 

common/initial_commit_tx.c

@@ -241,9 +241,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx,
 	sequence = (0x80000000 | ((obscured_commitment_number>>24) & 0xFFFFFF));
 	bitcoin_tx_add_input(tx, funding_txid, funding_txout, sequence, funding, NULL);
 
-	elements_tx_add_fee_output(tx);
-	tal_resize(&(tx->output_witscripts), tx->wtx->num_outputs);
-
+	bitcoin_tx_finalize(tx);
 	assert(bitcoin_tx_check(tx));
 
 	return tx;

common/withdraw_tx.c

@@ -52,7 +52,8 @@ struct bitcoin_tx *withdraw_tx(const tal_t *ctx,
 		*change_outnum = -1;
 
 	permute_inputs(tx, (const void **)utxos);
-	elements_tx_add_fee_output(tx);
+
+	bitcoin_tx_finalize(tx);
 	assert(bitcoin_tx_check(tx));
 	return tx;
 }