create_commit_tx: use HTLC map, not cstate HTLC array.

The HTLC state tells us whether a HTLC is in the commitment transaction.

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

daemon/commit_tx.c

@@ -6,21 +6,20 @@
 #include "channel.h"
 #include "commit_tx.h"
 #include "htlc.h"
+#include "lightningd.h"
 #include "overflows.h"
+#include "peer.h"
 #include "permute_tx.h"
 #include "remove_dust.h"
 #include <assert.h>
 
 u8 *wscript_for_htlc(const tal_t *ctx,
-		     secp256k1_context *secpctx,
+		     const struct peer *peer,
 		     const struct htlc *h,
-		     const struct pubkey *our_final,
-		     const struct pubkey *their_final,
-		     const struct rel_locktime *our_locktime,
-		     const struct rel_locktime *their_locktime,
 		     const struct sha256 *rhash,
 		     enum htlc_side side)
 {
+	const struct peer_visible_state *this_side, *other_side;
 	u8 *(*fn)(const tal_t *, secp256k1_context *,
 		  const struct pubkey *, const struct pubkey *,
 		  const struct abs_locktime *, const struct rel_locktime *,
@@ -32,107 +31,108 @@ u8 *wscript_for_htlc(const tal_t *ctx,
 	else
 		fn = bitcoin_redeem_htlc_recv;
 
-	if (side == LOCAL)
-		return fn(ctx, secpctx, our_final, their_final,
-			  &h->expiry, our_locktime, rhash, &h->rhash);
-	else
-		return fn(ctx, secpctx, their_final, our_final,
-			  &h->expiry, their_locktime, rhash, &h->rhash);
+	if (side == LOCAL) {
+		this_side = &peer->local;
+		other_side = &peer->remote;
+	} else {
+		this_side = &peer->remote;
+		other_side = &peer->local;
+	}
+
+	return fn(ctx, peer->dstate->secpctx,
+		  &this_side->finalkey, &other_side->finalkey,
+		  &h->expiry, &this_side->locktime, rhash, &h->rhash);
+}
+
+static size_t count_htlcs(const struct htlc_map *htlcs, int flag)
+{
+	struct htlc_map_iter it;
+	struct htlc *h;
+	size_t n = 0;
+
+	for (h = htlc_map_first(htlcs, &it); h; h = htlc_map_next(htlcs, &it)) {
+		if (htlc_has(h, flag))
+			n++;
+	}
+	return n;
 }
 
 struct bitcoin_tx *create_commit_tx(const tal_t *ctx,
-				    secp256k1_context *secpctx,
-				    const struct pubkey *our_final,
-				    const struct pubkey *their_final,
-				    const struct rel_locktime *our_locktime,
-				    const struct rel_locktime *their_locktime,
-				    const struct sha256_double *anchor_txid,
-				    unsigned int anchor_index,
-				    u64 anchor_satoshis,
+				    struct peer *peer,
 				    const struct sha256 *rhash,
 				    const struct channel_state *cstate,
-				    enum channel_side side,
+				    enum htlc_side side,
 				    int **map)
 {
 	struct bitcoin_tx *tx;
 	const u8 *redeemscript;
-	size_t i, num;
+	size_t num;
 	uint64_t total;
 	const struct pubkey *self, *other;
 	const struct rel_locktime *locktime;
-	enum htlc_side htlc_side;
+	struct htlc_map_iter it;
+	struct htlc *h;
+	enum channel_side channel_side;
+	int committed_flag = HTLC_FLAG(side,HTLC_F_COMMITTED);
 
 	/* Now create commitment tx: one input, two outputs (plus htlcs) */
-	tx = bitcoin_tx(ctx, 1, 2 + tal_count(cstate->side[OURS].htlcs)
-			+ tal_count(cstate->side[THEIRS].htlcs));
+	tx = bitcoin_tx(ctx, 1, 2 + count_htlcs(&peer->htlcs, committed_flag));
 
 	/* Our input spends the anchor tx output. */
-	tx->input[0].txid = *anchor_txid;
-	tx->input[0].index = anchor_index;
-	tx->input[0].amount = tal_dup(tx->input, u64, &anchor_satoshis);
+	tx->input[0].txid = peer->anchor.txid;
+	tx->input[0].index = peer->anchor.index;
+	tx->input[0].amount = tal_dup(tx->input, u64, &peer->anchor.satoshis);
 
 	/* For our commit tx, our payment is delayed by amount they said */
-	if (side == OURS) {
-		htlc_side = LOCAL;
-		self = our_final;
-		other = their_final;
-		locktime = their_locktime;
+	if (side == LOCAL) {
+		channel_side = OURS;
+		self = &peer->local.finalkey;
+		other = &peer->remote.finalkey;
+		locktime = &peer->remote.locktime;
 	} else {
-		htlc_side = REMOTE;
-		self = their_final;
-		other = our_final;
-		locktime = our_locktime;
+		channel_side = THEIRS;
+		self = &peer->remote.finalkey;
+		other = &peer->local.finalkey;
+		locktime = &peer->local.locktime;
 	}
-	
+
 	/* First output is a P2WSH to a complex redeem script
 	 * (usu. for this side) */
-	redeemscript = bitcoin_redeem_secret_or_delay(tx, secpctx, self,
+	redeemscript = bitcoin_redeem_secret_or_delay(tx, peer->dstate->secpctx,
+						      self,
 						      locktime,
 						      other,
 						      rhash);
 	tx->output[0].script = scriptpubkey_p2wsh(tx, redeemscript);
 	tx->output[0].script_length = tal_count(tx->output[0].script);
-	tx->output[0].amount = cstate->side[side].pay_msat / 1000;
+	tx->output[0].amount = cstate->side[channel_side].pay_msat / 1000;
 
 	/* Second output is a P2WPKH payment to other side. */
-	tx->output[1].script = scriptpubkey_p2wpkh(tx, secpctx, other);
+	tx->output[1].script = scriptpubkey_p2wpkh(tx, peer->dstate->secpctx,
+						   other);
 	tx->output[1].script_length = tal_count(tx->output[1].script);
-	tx->output[1].amount = cstate->side[!side].pay_msat / 1000;
+	tx->output[1].amount = cstate->side[!channel_side].pay_msat / 1000;
 
 	/* First two outputs done, now for the HTLCs. */
 	total = tx->output[0].amount + tx->output[1].amount;
 	num = 2;
 
-	for (i = 0; i < tal_count(cstate->side[side].htlcs); i++) {
+	for (h = htlc_map_first(&peer->htlcs, &it);
+	     h;
+	     h = htlc_map_next(&peer->htlcs, &it)) {
+		if (!htlc_has(h, committed_flag))
+			continue;
 		tx->output[num].script
 			= scriptpubkey_p2wsh(tx,
-				wscript_for_htlc(tx, secpctx,
-						 cstate->side[side].htlcs[i],
-						 our_final, their_final,
-						 our_locktime, their_locktime,
-						 rhash, htlc_side));
+					     wscript_for_htlc(tx, peer, h,
+							      rhash, side));
 		tx->output[num].script_length
 			= tal_count(tx->output[num].script);
-		tx->output[num].amount
-			= cstate->side[side].htlcs[i]->msatoshis / 1000;
-		total += tx->output[num++].amount;
-	}
-	for (i = 0; i < tal_count(cstate->side[!side].htlcs); i++) {
-		tx->output[num].script
-			= scriptpubkey_p2wsh(tx,
-				wscript_for_htlc(tx, secpctx,
-						 cstate->side[!side].htlcs[i],
-						 our_final, their_final,
-						 our_locktime, their_locktime,
-						 rhash, htlc_side));
-		tx->output[num].script_length
-			= tal_count(tx->output[num].script);
-		tx->output[num].amount
-			= cstate->side[!side].htlcs[i]->msatoshis / 1000;
+		tx->output[num].amount = h->msatoshis / 1000;
 		total += tx->output[num++].amount;
 	}
 	assert(num == tx->output_count);
-	assert(total <= anchor_satoshis);
+	assert(total <= peer->anchor.satoshis);
 
 	*map = tal_arr(ctx, int, tx->output_count);
 	permute_outputs(tx->output, tx->output_count, *map);

daemon/commit_tx.h

@@ -1,38 +1,25 @@
 #ifndef LIGHTNING_COMMIT_TX_H
 #define LIGHTNING_COMMIT_TX_H
 #include "config.h"
-#include "daemon/channel.h"
-#include "daemon/htlc.h"
+#include "htlc.h"
 
 struct channel_state;
-struct sha256_double;
 struct sha256;
 struct pubkey;
-struct rel_locktime;
+struct peer;
 
 u8 *wscript_for_htlc(const tal_t *ctx,
-		     secp256k1_context *secpctx,
+		     const struct peer *peer,
 		     const struct htlc *h,
-		     const struct pubkey *our_final,
-		     const struct pubkey *their_final,
-		     const struct rel_locktime *our_locktime,
-		     const struct rel_locktime *their_locktime,
 		     const struct sha256 *rhash,
 		     enum htlc_side side);
 
 /* Create commitment tx to spend the anchor tx output; doesn't fill in
  * input scriptsig. */
 struct bitcoin_tx *create_commit_tx(const tal_t *ctx,
-				    secp256k1_context *secpctx,
-				    const struct pubkey *our_final,
-				    const struct pubkey *their_final,
-				    const struct rel_locktime *our_locktime,
-				    const struct rel_locktime *their_locktime,
-				    const struct sha256_double *anchor_txid,
-				    unsigned int anchor_index,
-				    u64 anchor_satoshis,
+				    struct peer *peer,
 				    const struct sha256 *rhash,
 				    const struct channel_state *cstate,
-				    enum channel_side side,
+				    enum htlc_side side,
 				    int **map);
 #endif

daemon/packets.c

@@ -313,18 +313,8 @@ void queue_pkt_commit(struct peer *peer)
 	 * changes except unacked fee changes to the remote commitment
 	 * before generating `sig`. */
 	ci->cstate = copy_cstate(ci, peer->remote.staging_cstate);
-	ci->tx = create_commit_tx(ci, peer->dstate->secpctx,
-				  &peer->local.finalkey,
-				  &peer->remote.finalkey,
-				  &peer->local.locktime,
-				  &peer->remote.locktime,
-				  &peer->anchor.txid,
-				  peer->anchor.index,
-				  peer->anchor.satoshis,
-				  &ci->revocation_hash,
-				  ci->cstate,
-				  THEIRS,
-				  &ci->map);
+	ci->tx = create_commit_tx(ci, peer, &ci->revocation_hash,
+				  ci->cstate, REMOTE, &ci->map);
 
 	log_debug(peer->log, "Signing tx for %u/%u msatoshis, %zu/%zu htlcs",
 		  ci->cstate->side[OURS].pay_msat,
@@ -827,18 +817,8 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt)
 	 */
 	/* (We already applied them to staging_cstate as we went) */
 	ci->cstate = copy_cstate(ci, peer->local.staging_cstate);
-	ci->tx = create_commit_tx(ci, peer->dstate->secpctx,
-				  &peer->local.finalkey,
-				  &peer->remote.finalkey,
-				  &peer->local.locktime,
-				  &peer->remote.locktime,
-				  &peer->anchor.txid,
-				  peer->anchor.index,
-				  peer->anchor.satoshis,
-				  &ci->revocation_hash,
-				  ci->cstate,
-				  OURS,
-				  &ci->map);
+	ci->tx = create_commit_tx(ci, peer, &ci->revocation_hash,
+				  ci->cstate, LOCAL, &ci->map);
 
 	/* BOLT #2:
 	 *

daemon/peer.c

@@ -581,12 +581,7 @@ static const struct bitcoin_tx *htlc_fulfill_tx(const struct peer *peer,
 	htlc = htlc_by_index(ci, i);
 	assert(htlc->r);
 
-	wscript = wscript_for_htlc(peer, peer->dstate->secpctx, htlc,
-				   &peer->local.finalkey,
-				   &peer->remote.finalkey,
-				   &peer->local.locktime,
-				   &peer->remote.locktime,
-				   &ci->revocation_hash,
+	wscript = wscript_for_htlc(peer, peer, htlc, &ci->revocation_hash,
 				   REMOTE);
 
 	tx->input[0].index = ci->map[i];
@@ -2817,31 +2812,17 @@ bool setup_first_commit(struct peer *peer)
         return false;
 
 	peer->local.commit->tx = create_commit_tx(peer->local.commit,
-						  peer->dstate->secpctx,
-						  &peer->local.finalkey,
-						  &peer->remote.finalkey,
-						  &peer->local.locktime,
-						  &peer->remote.locktime,
-						  &peer->anchor.txid,
-						  peer->anchor.index,
-						  peer->anchor.satoshis,
+						  peer,
 						  &peer->local.commit->revocation_hash,
 						  peer->local.commit->cstate,
-						  OURS,
+						  LOCAL,
 						  &peer->local.commit->map);
 
 	peer->remote.commit->tx = create_commit_tx(peer->remote.commit,
-						   peer->dstate->secpctx,
-						   &peer->local.finalkey,
-						   &peer->remote.finalkey,
-						   &peer->local.locktime,
-						   &peer->remote.locktime,
-						   &peer->anchor.txid,
-						   peer->anchor.index,
-						   peer->anchor.satoshis,
+						   peer,
 						   &peer->remote.commit->revocation_hash,
 						   peer->remote.commit->cstate,
-						   THEIRS,
+						   REMOTE,
 						   &peer->remote.commit->map);
 
 	peer->local.staging_cstate = copy_cstate(peer, peer->local.commit->cstate);