funding_tx: permute inputs, don't re-calculate fees

built_utxos needs to calculate fees for figuring out how many utxos to
use, so fix that logic and rely on it.

We make build_utxos return a pointer array, so funding_tx can simply hand
that to permute_inputs.

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

lightningd/build_utxos.c

@@ -175,39 +175,35 @@ static void unreserve_utxo(struct lightningd *ld, const struct utxo *unres)
 	struct tracked_utxo *utxo;
 
 	list_for_each(&ld->utxos, utxo, list) {
-		if (unres->outnum != utxo->utxo.outnum
-		    || !structeq(&unres->txid, &utxo->utxo.txid))
+		if (unres != &utxo->utxo)
 			continue;
 		assert(utxo->reserved);
-		assert(unres->amount == utxo->utxo.amount);
-		assert(unres->keyindex == utxo->utxo.keyindex);
-		assert(unres->is_p2sh == utxo->utxo.is_p2sh);
 		utxo->reserved = false;
 		return;
 	}
 	abort();
 }
 
-static void destroy_utxos(struct utxo *utxos, struct lightningd *ld)
+static void destroy_utxos(const struct utxo **utxos, struct lightningd *ld)
 {
 	size_t i;
 
 	for (i = 0; i < tal_count(utxos); i++)
-		unreserve_utxo(ld, &utxos[i]);
+		unreserve_utxo(ld, utxos[i]);
 }
 
-void confirm_utxos(struct lightningd *ld, struct utxo *utxos)
+void confirm_utxos(struct lightningd *ld, const struct utxo **utxos)
 {
 	tal_del_destructor2(utxos, destroy_utxos, ld);
 }
 
-struct utxo *build_utxos(const tal_t *ctx,
-			 struct lightningd *ld, u64 satoshi_out,
-			 u32 feerate_per_kw, u64 dust_limit,
-			 u64 *change_amount, u32 *change_keyindex)
+const struct utxo **build_utxos(const tal_t *ctx,
+				struct lightningd *ld, u64 satoshi_out,
+				u32 feerate_per_kw, u64 dust_limit,
+				u64 *change_satoshis, u32 *change_keyindex)
 {
 	size_t i = 0;
-	struct utxo *utxos = tal_arr(ctx, struct utxo, 0);
+	const struct utxo **utxos = tal_arr(ctx, const struct utxo *, 0);
 	struct tracked_utxo *utxo;
 	/* We assume two outputs for the weight. */
 	u64 satoshi_in = 0, weight = (4 + (8 + 22) * 2 + 4) * 4;
@@ -221,23 +217,26 @@ struct utxo *build_utxos(const tal_t *ctx,
 			continue;
 
 		tal_resize(&utxos, i+1);
-		utxos[i] = utxo->utxo;
+		utxos[i] = &utxo->utxo;
 		utxo->reserved = true;
 
 		/* Add this input's weight. */
 		weight += (32 + 4 + 4) * 4;
-		if (utxos[i].is_p2sh)
+		if (utxos[i]->is_p2sh)
 			weight += 22 * 4;
-
-		satoshi_in += utxos[i].amount;
+		/* Account for witness (1 byte count + sig + key */
+		weight += 1 + (1 + 73 + 1 + 33);
 
 		fee = weight * feerate_per_kw / 1000;
+		satoshi_in += utxos[i]->amount;
+
 		if (satoshi_in >= fee + satoshi_out) {
 			/* We simply eliminate change if it's dust. */
-			*change_amount = satoshi_in - (fee + satoshi_out);
-			if (*change_amount < dust_limit)
-				*change_amount = 0;
-			else
+			*change_satoshis = satoshi_in - (fee + satoshi_out);
+			if (*change_satoshis < dust_limit) {
+				*change_satoshis = 0;
+				*change_keyindex = 0;
+			} else
 				*change_keyindex = ld->bip32_max_index++;
 
 			return utxos;

lightningd/build_utxos.h

@@ -5,12 +5,12 @@
 #include <lightningd/utxo.h>
 
 /* Reserves UTXOs to build tx which pays this amount; returns NULL if
- * impossible.  *change_amount 0 if no change needed. */
-struct utxo *build_utxos(const tal_t *ctx,
-			 struct lightningd *ld, u64 satoshi_out,
-			 u32 feerate_per_kw, u64 dust_limit,
-			 u64 *change_amount, u32 *change_keyindex);
+ * impossible.  *change_satoshis 0 if no change needed. */
+const struct utxo **build_utxos(const tal_t *ctx,
+				struct lightningd *ld, u64 satoshi_out,
+				u32 feerate_per_kw, u64 dust_limit,
+				u64 *change_satoshis, u32 *change_keyindex);
 
 /* Once we've spent them, mark them confirmed. */
-void confirm_utxos(struct lightningd *ld, struct utxo *utxos);
+void confirm_utxos(struct lightningd *ld, const struct utxo **utxos);
 #endif /* LIGHTNING_LIGHTNINGD_BUILD_UTXOS_H */

lightningd/funding_tx.c

@@ -12,24 +12,22 @@
 
 struct bitcoin_tx *funding_tx(const tal_t *ctx,
 			      u32 *outnum,
-			      const struct utxo *utxos,
+			      const struct utxo **utxomap,
 			      u64 funding_satoshis,
 			      const struct pubkey *local_fundingkey,
 			      const struct pubkey *remote_fundingkey,
-			      const struct pubkey *changekey,
-			      u64 feerate_per_kw,
-			      u64 dust_limit_satoshis)
+			      u64 change_satoshis,
+			      const struct pubkey *changekey)
 {
-	struct bitcoin_tx *tx = bitcoin_tx(ctx, tal_count(utxos), 2);
+	struct bitcoin_tx *tx = bitcoin_tx(ctx, tal_count(utxomap),
+					   change_satoshis ? 2 : 1);
 	u8 *wscript;
-	u64 fee, weight, input_satoshis = 0;
 	size_t i;
 
-	for (i = 0; i < tal_count(utxos); i++) {
-		tx->input[i].txid = utxos[i].txid;
-		tx->input[i].index = utxos[i].outnum;
-		tx->input[i].amount = tal_dup(tx, u64, &utxos[i].amount);
-		input_satoshis += utxos[i].amount;
+	for (i = 0; i < tal_count(utxomap); i++) {
+		tx->input[i].txid = utxomap[i]->txid;
+		tx->input[i].index = utxomap[i]->outnum;
+		tx->input[i].amount = tal_dup(tx, u64, &utxomap[i]->amount);
 	}
 
 	tx->output[0].amount = funding_satoshis;
@@ -39,25 +37,19 @@ struct bitcoin_tx *funding_tx(const tal_t *ctx,
 	tx->output[0].script = scriptpubkey_p2wsh(tx, wscript);
 	tal_free(wscript);
 
-	assert(input_satoshis >= funding_satoshis);
 	tx->output[1].script = scriptpubkey_p2wpkh(tx, changekey);
 
-	/* Calculate what weight will be once we've signed. */
-	weight = measure_tx_cost(tx) + 4 * (73 + 34);
-	fee = weight * feerate_per_kw / 1000;
-
-	/* Too small an output after fee?  Drop it. */
-	if (input_satoshis - funding_satoshis < dust_limit_satoshis + fee) {
-		tal_resize(&tx->output, 1);
-		*outnum = 0;
-	} else {
+	if (change_satoshis != 0) {
 		const void *map[2];
 		map[0] = int2ptr(0);
 		map[1] = int2ptr(1);
-		tx->output[1].amount = input_satoshis - funding_satoshis - fee;
+		tx->output[1].amount = change_satoshis;
 		permute_outputs(tx->output, tal_count(tx->output), map);
 		*outnum = (map[0] == int2ptr(0) ? 0 : 1);
+	} else {
+		*outnum = 0;
 	}
 
+	permute_inputs(tx->input, tal_count(tx->input), (const void **)utxomap);
 	return tx;
 }

lightningd/funding_tx.h

@@ -13,22 +13,20 @@ struct utxo;
 /**
  * funding_tx: create a P2WSH funding transaction for a channel.
  * @ctx: context to tal from.
- * @outnum: txout (0 or 1) which is the funding output.
- * @utxo: tal_arr of UTXO to spend as inputs.
- * @funding_satoshis: satoshis to output.
- * @local_fundingkey: local key for 2of2 funding output.
- * @remote_fundingkey: remote key for 2of2 funding output.
- * @changekey: key to send change to (if any).
- * @feerate_per_kw: feerate for transaction.
- * @dust_limit_satoshis: dust limit to trim change output by.
+ * @outnum: (out) txout (0 or 1) which is the funding output.
+ * @utxomap: (in/out) tal_arr of UTXO pointers to spend (permuted to match)
+ * @funding_satoshis: (in) satoshis to output.
+ * @local_fundingkey: (in) local key for 2of2 funding output.
+ * @remote_fundingkey: (in) remote key for 2of2 funding output.
+ * @change_satoshis: (in) amount to send as change.
+ * @changekey: (in) key to send change to (only used if change_satoshis != 0).
  */
 struct bitcoin_tx *funding_tx(const tal_t *ctx,
 			      u32 *outnum,
-			      const struct utxo *utxos,
+			      const struct utxo **utxomap,
 			      u64 funding_satoshis,
 			      const struct pubkey *local_fundingkey,
 			      const struct pubkey *remote_fundingkey,
-			      const struct pubkey *changekey,
-			      u64 feerate_per_kw,
-			      u64 dust_limit_satoshis);
+			      u64 change_satoshis,
+			      const struct pubkey *changekey);
 #endif /* LIGHTNING_LIGHTNINGD_FUNDING_TX_H */

lightningd/peer_control.c

@@ -516,7 +516,7 @@ struct funding_channel {
 	struct peer *peer;
 	struct command *cmd;
 	u64 satoshi;
-	struct utxo *utxos;
+	const struct utxo **utxomap;
 	u64 change;
 	u32 change_keyindex;
 	struct crypto_state *cs;
@@ -553,15 +553,22 @@ static void opening_release_tx(struct subdaemon *opening, const u8 *resp,
 			       struct funding_channel *fc)
 {
 	u8 *msg;
+	size_t i;
+	/* FIXME: marshal code wants array, not array of pointers. */
+	struct utxo *utxos = tal_arr(fc, struct utxo, tal_count(fc->utxomap));
 
 	peer_set_condition(fc->peer, "Getting HSM to sign funding tx");
 
 	/* Get HSM to sign the funding tx. */
+	for (i = 0; i < tal_count(fc->utxomap); i++)
+		utxos[i] = *fc->utxomap[i];
+
 	msg = towire_hsmctl_sign_funding(fc, fc->satoshi, fc->change,
 					 fc->change_keyindex,
 					 &fc->local_fundingkey,
 					 &fc->remote_fundingkey,
-					 fc->utxos);
+					 utxos);
+	tal_free(utxos);
 	subdaemon_req(fc->peer->ld->hsm, take(msg), -1, NULL,
 		      opening_got_hsm_funding_sig, fc);
 }
@@ -584,14 +591,13 @@ static void opening_gen_funding(struct subdaemon *opening, const u8 *resp,
 		return;
 	}
 
-	bitcoin_pubkey(fc->peer->ld, &changekey, fc->change_keyindex);
+	if (fc->change)
+		bitcoin_pubkey(fc->peer->ld, &changekey, fc->change_keyindex);
 
-	/* FIXME: Real feerate! */
-	/* FIXME: Real dust limit! */
-	fc->funding_tx = funding_tx(fc, &outnum, fc->utxos, fc->satoshi,
+	fc->funding_tx = funding_tx(fc, &outnum, fc->utxomap, fc->satoshi,
 				    &fc->local_fundingkey,
 				    &fc->remote_fundingkey,
-				    &changekey, 15000, 546);
+				    fc->change, &changekey);
 	bitcoin_txid(fc->funding_tx, &txid);
 
 	msg = towire_opening_open_funding(fc, &txid, outnum);
@@ -809,9 +815,9 @@ static void json_fund_channel(struct command *cmd,
 
 	/* Try to do this now, so we know if insufficient funds. */
 	/* FIXME: Feerate & dustlimit */
-	fc->utxos = build_utxos(fc, ld, fc->satoshi, 15000, 600,
-				&fc->change, &fc->change_keyindex);
-	if (!fc->utxos) {
+	fc->utxomap = build_utxos(fc, ld, fc->satoshi, 15000, 600,
+				  &fc->change, &fc->change_keyindex);
+	if (!fc->utxomap) {
 		command_fail(cmd, "Cannot afford funding transaction");
 		return;
 	}

lightningd/test/run-funding_tx.c

@@ -39,12 +39,13 @@ int main(void)
 {
 	tal_t *tmpctx = tal_tmpctx(NULL);
 	struct bitcoin_tx *input, *funding;
-	u64 feerate_per_kw;
+	u64 fee;
 	struct pubkey local_funding_pubkey, remote_funding_pubkey;
 	struct privkey input_privkey;
 	struct pubkey inputkey;
 	bool testnet;
-	struct utxo *utxo = tal_arr(tmpctx, struct utxo, 1);
+	struct utxo utxo;
+	const struct utxo **utxomap = tal_arr(tmpctx, const struct utxo *, 1);
 	u64 funding_satoshis;
 	u32 funding_outnum;
 	u8 *subscript;
@@ -89,26 +90,26 @@ int main(void)
 				&remote_funding_pubkey))
 		abort();
 
-	bitcoin_txid(input, &utxo->txid);
-	utxo->outnum = 0;
-	utxo->amount = 5000000000;
+	bitcoin_txid(input, &utxo.txid);
+	utxo.outnum = 0;
+	utxo.amount = 5000000000;
 	funding_satoshis = 10000000;
-	feerate_per_kw = 15000;
+	fee = 13920;
 
 	printf("input[0] txid: %s\n",
-	       tal_hexstr(tmpctx, &utxo->txid, sizeof(utxo->txid)));
-	printf("input[0] input: %u\n", utxo->outnum);
-	printf("input[0] satoshis: %"PRIu64"\n", utxo->amount);
+	       tal_hexstr(tmpctx, &utxo.txid, sizeof(utxo.txid)));
+	printf("input[0] input: %u\n", utxo.outnum);
+	printf("input[0] satoshis: %"PRIu64"\n", utxo.amount);
 	printf("funding satoshis: %"PRIu64"\n", funding_satoshis);
 
-	funding = funding_tx(tmpctx, &funding_outnum, utxo,
+	utxomap[0] = &utxo;
+	funding = funding_tx(tmpctx, &funding_outnum, utxomap,
 			     funding_satoshis,
 			     &local_funding_pubkey,
 			     &remote_funding_pubkey,
-			     &inputkey,
-			     feerate_per_kw,
-			     0);
-	printf("# feerate_per_kw: %"PRIu64"\n", feerate_per_kw);
+			     utxo.amount - fee - funding_satoshis,
+			     &inputkey);
+	printf("# fee: %"PRIu64"\n", fee);
 	printf("change satoshis: %"PRIu64"\n",
 	       funding->output[!funding_outnum].amount);