daemon: make funding directions local-centric.

Previous to this, we kept the remote side's 'struct channel_state'
backwards: peer->remote.commit->cstate.side[OURS] was their HTLCs,
and [THEIRS] was our HTLCs.  This made some things easier, but was
horrible for readability.

This inverts things so we keep track of the remote side's state from
our point of view: [OURS] is ours, [THEIRS] is theirs.  Which makes
much more sense.

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

commit_tx.c

@@ -38,18 +38,22 @@ static bool add_htlc(struct bitcoin_tx *tx, size_t n,
 struct bitcoin_tx *create_commit_tx(const tal_t *ctx,
 				    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,
 				    const struct sha256 *rhash,
 				    const struct channel_state *cstate,
+				    enum channel_side side,
 				    int **map)
 {
 	struct bitcoin_tx *tx;
 	const u8 *redeemscript;
 	size_t i, num;
 	uint64_t total;
+	const struct pubkey *self, *other;
+	const struct rel_locktime *locktime;
 
 	/* Now create commitment tx: one input, two outputs (plus htlcs) */
 	tx = bitcoin_tx(ctx, 1, 2 + tal_count(cstate->side[OURS].htlcs)
@@ -60,37 +64,49 @@ struct bitcoin_tx *create_commit_tx(const tal_t *ctx,
 	tx->input[0].index = anchor_index;
 	tx->input[0].amount = tal_dup(tx->input, u64, &anchor_satoshis);
 
-	/* First output is a P2WSH to a complex redeem script (usu. for me) */
-	redeemscript = bitcoin_redeem_secret_or_delay(tx, our_final,
-						      their_locktime,
-						      their_final,
+	/* For our commit tx, our payment is delayed by amount they said */
+	if (side == OURS) {
+		self = our_final;
+		other = their_final;
+		locktime = their_locktime;
+	} else {
+		self = their_final;
+		other = our_final;
+		locktime = our_locktime;
+	}
+	
+	/* First output is a P2WSH to a complex redeem script
+	 * (usu. for this side) */
+	redeemscript = bitcoin_redeem_secret_or_delay(tx, 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[OURS].pay_msat / 1000;
+	tx->output[0].amount = cstate->side[side].pay_msat / 1000;
 
-	/* Second output is a P2WPKH payment to them. */
-	tx->output[1].script = scriptpubkey_p2wpkh(tx, their_final);
+	/* Second output is a P2WPKH payment to other side. */
+	tx->output[1].script = scriptpubkey_p2wpkh(tx, other);
 	tx->output[1].script_length = tal_count(tx->output[1].script);
-	tx->output[1].amount = cstate->side[THEIRS].pay_msat / 1000;
+	tx->output[1].amount = cstate->side[!side].pay_msat / 1000;
 
 	/* First two outputs done, now for the HTLCs. */
 	total = tx->output[0].amount + tx->output[1].amount;
 	num = 2;
 
-	/* HTLCs we've sent. */
-	for (i = 0; i < tal_count(cstate->side[OURS].htlcs); i++) {
-		if (!add_htlc(tx, num, &cstate->side[OURS].htlcs[i],
-			      our_final, their_final,
-			      rhash, their_locktime, bitcoin_redeem_htlc_send))
+	/* HTLCs this side sent. */
+	for (i = 0; i < tal_count(cstate->side[side].htlcs); i++) {
+		if (!add_htlc(tx, num, &cstate->side[side].htlcs[i],
+			      self, other, rhash, locktime,
+			      bitcoin_redeem_htlc_send))
 			return tal_free(tx);
 		total += tx->output[num++].amount;
 	}
-	/* HTLCs we've received. */
-	for (i = 0; i < tal_count(cstate->side[THEIRS].htlcs); i++) {
-		if (!add_htlc(tx, num, &cstate->side[THEIRS].htlcs[i],
-			      our_final, their_final,
-			      rhash, their_locktime, bitcoin_redeem_htlc_recv))
+	/* HTLCs this side has received. */
+	for (i = 0; i < tal_count(cstate->side[!side].htlcs); i++) {
+		if (!add_htlc(tx, num, &cstate->side[!side].htlcs[i],
+			      self, other, rhash, locktime,
+			      bitcoin_redeem_htlc_recv))
 			return tal_free(tx);
 		total += tx->output[num++].amount;
 	}

commit_tx.h

@@ -1,8 +1,7 @@
 #ifndef LIGHTNING_COMMIT_TX_H
 #define LIGHTNING_COMMIT_TX_H
 #include "config.h"
-#include <ccan/short_types/short_types.h>
-#include <ccan/tal/tal.h>
+#include "funding.h"
 
 struct channel_state;
 struct sha256_double;
@@ -15,11 +14,13 @@ struct rel_locktime;
 struct bitcoin_tx *create_commit_tx(const tal_t *ctx,
 				    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,
 				    const struct sha256 *rhash,
 				    const struct channel_state *cstate,
+				    enum channel_side side,
 				    int **map);
 #endif

daemon/packets.c

@@ -191,7 +191,7 @@ void queue_pkt_htlc_add(struct peer *peer,
 			      htlc_prog->stage.add.htlc.msatoshis,
 			      &htlc_prog->stage.add.htlc.expiry,
 			      &htlc_prog->stage.add.htlc.rhash,
-			      htlc_prog->stage.add.htlc.id, THEIRS))
+			      htlc_prog->stage.add.htlc.id, OURS))
 		fatal("Could not add HTLC?");
 	add_unacked(&peer->remote, &htlc_prog->stage);
 
@@ -219,9 +219,9 @@ void queue_pkt_htlc_fulfill(struct peer *peer,
 	 * The sending node MUST add the HTLC fulfill/fail to the
 	 * unacked changeset for its remote commitment
 	 */
-	n = funding_htlc_by_id(peer->remote.staging_cstate, f->id, OURS);
+	n = funding_htlc_by_id(peer->remote.staging_cstate, f->id, THEIRS);
 	assert(n != -1);
-	funding_fulfill_htlc(peer->remote.staging_cstate, n, OURS);
+	funding_fulfill_htlc(peer->remote.staging_cstate, n, THEIRS);
 	add_unacked(&peer->remote, &htlc_prog->stage);
 
 	remote_changes_pending(peer);
@@ -248,9 +248,9 @@ void queue_pkt_htlc_fail(struct peer *peer,
 	 * The sending node MUST add the HTLC fulfill/fail to the
 	 * unacked changeset for its remote commitment
 	 */
-	n = funding_htlc_by_id(peer->remote.staging_cstate, f->id, OURS);
+	n = funding_htlc_by_id(peer->remote.staging_cstate, f->id, THEIRS);
 	assert(n != -1);
-	funding_fail_htlc(peer->remote.staging_cstate, n, OURS);
+	funding_fail_htlc(peer->remote.staging_cstate, n, THEIRS);
 	add_unacked(&peer->remote, &htlc_prog->stage);
 
 	remote_changes_pending(peer);
@@ -274,14 +274,16 @@ void queue_pkt_commit(struct peer *peer)
 	 * before generating `sig`. */
 	ci->cstate = copy_funding(ci, peer->remote.staging_cstate);
 	ci->tx = create_commit_tx(ci,
-				  &peer->remote.finalkey,
 				  &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);
 
 	log_debug(peer->log, "Signing tx for %u/%u msatoshis, %zu/%zu htlcs",
@@ -316,6 +318,7 @@ void queue_pkt_commit(struct peer *peer)
 /* At revocation time, we apply the changeset to the other side. */
 static void apply_changeset(struct peer *peer,
 			    struct peer_visible_state *which,
+			    enum channel_side side,
 			    const union htlc_staging *changes,
 			    size_t num_changes)
 {
@@ -326,7 +329,7 @@ static void apply_changeset(struct peer *peer,
 		switch (changes[i].type) {
 		case HTLC_ADD:
 			n = funding_htlc_by_id(which->staging_cstate,
-					       changes[i].add.htlc.id, OURS);
+					       changes[i].add.htlc.id, side);
 			if (n != -1)
 				fatal("Can't add duplicate HTLC id %"PRIu64,
 				      changes[i].add.htlc.id);
@@ -334,24 +337,25 @@ static void apply_changeset(struct peer *peer,
 					      changes[i].add.htlc.msatoshis,
 					      &changes[i].add.htlc.expiry,
 					      &changes[i].add.htlc.rhash,
-					      changes[i].add.htlc.id, OURS))
-				fatal("Adding HTLC failed");
+					      changes[i].add.htlc.id, side))
+				fatal("Adding HTLC to %s failed",
+				      side == OURS ? "ours" : "theirs");
 			continue;
 		case HTLC_FAIL:
 			n = funding_htlc_by_id(which->staging_cstate,
-					       changes[i].fail.id, THEIRS);
+					       changes[i].fail.id, !side);
 			if (n == -1)
 				fatal("Can't fail non-exisent HTLC id %"PRIu64,
 				      changes[i].fail.id);
-			funding_fail_htlc(which->staging_cstate, n, THEIRS);
+			funding_fail_htlc(which->staging_cstate, n, !side);
 			continue;
 		case HTLC_FULFILL:
 			n = funding_htlc_by_id(which->staging_cstate,
-					       changes[i].fulfill.id, THEIRS);
+					       changes[i].fulfill.id, !side);
 			if (n == -1)
 				fatal("Can't fulfill non-exisent HTLC id %"PRIu64,
 				      changes[i].fulfill.id);
-			funding_fulfill_htlc(which->staging_cstate, n, THEIRS);
+			funding_fulfill_htlc(which->staging_cstate, n, !side);
 			continue;
 		}
 		abort();
@@ -391,7 +395,7 @@ void queue_pkt_revocation(struct peer *peer)
 	 * The node sending `update_revocation` MUST add the local unacked
 	 * changes to the set of remote acked changes.
 	 */
-	apply_changeset(peer, &peer->remote,
+	apply_changeset(peer, &peer->remote, THEIRS,
 			peer->local.unacked_changes,
 			tal_count(peer->local.unacked_changes));
 
@@ -599,7 +603,7 @@ Pkt *accept_pkt_htlc_add(struct peer *peer, const Pkt *pkt)
 	 * A node MUST NOT add a HTLC if it would result in it
 	 * offering more than 300 HTLCs in either commitment transaction.
 	 */
-	if (tal_count(peer->remote.staging_cstate->side[OURS].htlcs) == 300
+	if (tal_count(peer->remote.staging_cstate->side[THEIRS].htlcs) == 300
 	    || tal_count(peer->local.staging_cstate->side[THEIRS].htlcs) == 300)
 		return pkt_err(peer, "Too many HTLCs");
 
@@ -608,7 +612,7 @@ Pkt *accept_pkt_htlc_add(struct peer *peer, const Pkt *pkt)
 	 * A node MUST NOT set `id` equal to another HTLC which is in
 	 * the current staged commitment transaction.
 	 */
-	if (funding_htlc_by_id(peer->remote.staging_cstate, u->id, OURS) != -1)
+	if (funding_htlc_by_id(peer->remote.staging_cstate, u->id, THEIRS) != -1)
 		return pkt_err(peer, "HTLC id %"PRIu64" clashes for you", u->id);
 
 	/* FIXME: Assert this... */
@@ -752,12 +756,14 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt)
 	ci->tx = create_commit_tx(ci,
 				  &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);
 
 	/* BOLT #2:
@@ -823,7 +829,7 @@ Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt)
 	 * The receiver of `update_revocation`... MUST add the remote
 	 * unacked changes to the set of local acked changes.
 	 */
-	apply_changeset(peer, &peer->local,
+	apply_changeset(peer, &peer->local, OURS,
 			peer->remote.unacked_changes,
 			tal_count(peer->remote.unacked_changes));
 

daemon/peer.c

@@ -837,7 +837,7 @@ static struct channel_htlc *htlc_by_index(const struct commit_info *ci,
 		+ index;
 }
 
-static bool htlc_this_side_offered(const struct commit_info *ci, size_t index)
+static bool htlc_is_ours(const struct commit_info *ci, size_t index)
 {
 	assert(index >= 2);
 	index -= 2;
@@ -1021,7 +1021,7 @@ static void resolve_cheating(struct peer *peer)
 		connect_input(ci, &steal_tx->input[n], ci->map[i]);
 
 		h = htlc_by_index(ci, i);
-		if (htlc_this_side_offered(ci, i)) {
+		if (!htlc_is_ours(ci, i)) {
 			wscripts[n]
 				= bitcoin_redeem_htlc_send(wscripts,
 							   &peer->remote.finalkey,
@@ -1399,9 +1399,8 @@ static void resolve_their_unilateral(struct peer *peer)
 	 */
 	peer->closing_onchain.resolved[0] = tx;
 
-	/* Note the reversal, since ci is theirs, we are B */
-	num_ours = tal_count(ci->cstate->side[THEIRS].htlcs);
-	num_theirs = tal_count(ci->cstate->side[OURS].htlcs);
+	num_ours = tal_count(ci->cstate->side[OURS].htlcs);
+	num_theirs = tal_count(ci->cstate->side[THEIRS].htlcs);
 
 	/* BOLT #onchain:
 	 *
@@ -1926,36 +1925,34 @@ bool setup_first_commit(struct peer *peer)
 	if (!peer->local.commit->cstate)
 		return false;
 
-	peer->remote.commit->cstate = initial_funding(peer,
-						      peer->anchor.satoshis,
-						      peer->remote.commit_fee_rate,
-						      peer->remote.offer_anchor
-						      == CMD_OPEN_WITH_ANCHOR ?
-						      OURS : THEIRS);
-	if (!peer->remote.commit->cstate)
-		return false;
+	peer->remote.commit->cstate = copy_funding(peer,
+						   peer->local.commit->cstate);
 
 	peer->local.commit->tx = create_commit_tx(peer->local.commit,
-					       &peer->local.finalkey,
-					       &peer->remote.finalkey,
-					       &peer->remote.locktime,
-					       &peer->anchor.txid,
-					       peer->anchor.index,
-					       peer->anchor.satoshis,
-					       &peer->local.commit->revocation_hash,
-					       peer->local.commit->cstate,
-					       &peer->local.commit->map);
+						  &peer->local.finalkey,
+						  &peer->remote.finalkey,
+						  &peer->local.locktime,
+						  &peer->remote.locktime,
+						  &peer->anchor.txid,
+						  peer->anchor.index,
+						  peer->anchor.satoshis,
+						  &peer->local.commit->revocation_hash,
+						  peer->local.commit->cstate,
+						  OURS,
+						  &peer->local.commit->map);
 
 	peer->remote.commit->tx = create_commit_tx(peer->remote.commit,
-						 &peer->remote.finalkey,
-						 &peer->local.finalkey,
-						 &peer->local.locktime,
-						 &peer->anchor.txid,
-						 peer->anchor.index,
-						 peer->anchor.satoshis,
-						 &peer->remote.commit->revocation_hash,
-						 peer->remote.commit->cstate,
-						 &peer->remote.commit->map);
+						   &peer->local.finalkey,
+						   &peer->remote.finalkey,
+						   &peer->local.locktime,
+						   &peer->remote.locktime,
+						   &peer->anchor.txid,
+						   peer->anchor.index,
+						   peer->anchor.satoshis,
+						   &peer->remote.commit->revocation_hash,
+						   peer->remote.commit->cstate,
+						   THEIRS,
+						   &peer->remote.commit->map);
 
 	peer->local.staging_cstate = copy_funding(peer, peer->local.commit->cstate);
 	peer->remote.staging_cstate = copy_funding(peer, peer->remote.commit->cstate);
@@ -2089,8 +2086,8 @@ static void check_htlc_expiry(struct peer *peer, void *unused)
 
 	/* Check their currently still-existing htlcs for expiry:
 	 * We eliminate them from staging as we go. */
-	for (i = 0; i < tal_count(peer->remote.staging_cstate->side[OURS].htlcs); i++) {
-		struct channel_htlc *htlc = &peer->remote.staging_cstate->side[OURS].htlcs[i];
+	for (i = 0; i < tal_count(peer->remote.staging_cstate->side[THEIRS].htlcs); i++) {
+		struct channel_htlc *htlc = &peer->remote.staging_cstate->side[THEIRS].htlcs[i];
 
 		/* Not a seconds-based expiry? */
 		if (!abs_locktime_is_seconds(&htlc->expiry))
@@ -2148,7 +2145,7 @@ static void do_newhtlc(struct peer *peer, struct newhtlc *newhtlc)
 	 * offering more than 300 HTLCs in either commitment transaction.
 	 */
 	if (tal_count(peer->local.staging_cstate->side[OURS].htlcs) == 300
-	    || tal_count(peer->remote.staging_cstate->side[THEIRS].htlcs) == 300) {
+	    || tal_count(peer->remote.staging_cstate->side[OURS].htlcs) == 300) {
 		command_fail(newhtlc->jsoncmd, "Too many HTLCs");
 	}
 
@@ -2161,7 +2158,7 @@ static void do_newhtlc(struct peer *peer, struct newhtlc *newhtlc)
 	cstate = copy_funding(newhtlc, peer->remote.staging_cstate);
 	if (!funding_add_htlc(cstate, newhtlc->htlc.msatoshis,
 			      &newhtlc->htlc.expiry, &newhtlc->htlc.rhash,
-			      newhtlc->htlc.id, THEIRS)) {
+			      newhtlc->htlc.id, OURS)) {
 		command_fail(newhtlc->jsoncmd,
 			     "Cannot afford %"PRIu64
 			     " milli-satoshis in their commit tx",
@@ -2279,10 +2276,10 @@ static size_t find_their_committed_htlc(struct peer *peer,
 					const struct sha256 *rhash)
 {
 	/* Must be in last committed cstate. */
-	if (funding_find_htlc(peer->remote.commit->cstate, rhash, OURS) == -1)
+	if (funding_find_htlc(peer->remote.commit->cstate, rhash, THEIRS) == -1)
 		return -1;
 
-	return funding_find_htlc(peer->remote.staging_cstate, rhash, OURS);
+	return funding_find_htlc(peer->remote.staging_cstate, rhash, THEIRS);
 }
 
 struct fulfillhtlc {
@@ -2307,7 +2304,7 @@ static void do_fullfill(struct peer *peer,
 		command_fail(fulfillhtlc->jsoncmd, "preimage htlc not found");
 		return;
 	}
-	stage.fulfill.id = peer->remote.staging_cstate->side[OURS].htlcs[i].id;
+	stage.fulfill.id = peer->remote.staging_cstate->side[THEIRS].htlcs[i].id;
 	set_htlc_command(peer, fulfillhtlc->jsoncmd,
 			 CMD_SEND_HTLC_FULFILL, &stage);
 }
@@ -2381,7 +2378,7 @@ static void do_failhtlc(struct peer *peer,
 		command_fail(failhtlc->jsoncmd, "htlc not found");
 		return;
 	}
-	stage.fail.id = peer->remote.staging_cstate->side[OURS].htlcs[i].id;
+	stage.fail.id = peer->remote.staging_cstate->side[THEIRS].htlcs[i].id;
 
 	set_htlc_command(peer, failhtlc->jsoncmd, CMD_SEND_HTLC_FAIL, &stage);
 }

funding.c

@@ -209,15 +209,6 @@ bool force_fee(struct channel_state *cstate, uint64_t fee)
 	cstate->changes++;
 	return cstate->side[OURS].fee_msat + cstate->side[THEIRS].fee_msat == fee * 1000;
 }
-	
-void invert_cstate(struct channel_state *cstate)
-{
-	struct channel_oneside tmp;
-
-	tmp = cstate->side[OURS];
-	cstate->side[OURS] = cstate->side[THEIRS];
-	cstate->side[THEIRS] = tmp;
-}
 
 /* Add a HTLC to @creator if it can afford it. */
 struct channel_htlc *funding_add_htlc(struct channel_state *cstate,

funding.h

@@ -121,12 +121,6 @@ void adjust_fee(struct channel_state *cstate, uint32_t fee_rate);
  */
 bool force_fee(struct channel_state *cstate, uint64_t fee);
 
-/**
- * invert_cstate: Get the other side's state.
- * @cstate: the state to invert.
- */
-void invert_cstate(struct channel_state *cstate);
-
 /**
  * funding_find_htlc: find an HTLC on this side of the channel.
  * @cstate: The channel state