daemon: save acked changes, so we can process them when confirmed on both sides.

We need to know when changes are fully committed by both sides:
1) For their HTLC_ADDs, this is when we can fulfill/fail/route.
2) For their HTLC_FAILs, this is when we can fail incoming.

For HTLC_FULFULL we don't need to wait: as soon as we know the preimage
we can propogate it.

For the moment, we simply log and assert; acting on it comes later.

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

daemon/packets.c

@@ -85,6 +85,7 @@ static struct commit_info *new_commit_info(const tal_t *ctx)
 {
 	struct commit_info *ci = talz(ctx, struct commit_info);
 	ci->unacked_changes = tal_arr(ci, union htlc_staging, 0);
+	ci->acked_changes = tal_arr(ci, union htlc_staging, 0);
 	return ci;
 }
 
@@ -402,6 +403,7 @@ void queue_pkt_revocation(struct peer *peer)
 	 */
 	/* Note: this means the unacked changes as of the commit we're
 	 * revoking */
+	add_acked_changes(&peer->remote.commit->acked_changes, ci->unacked_changes);
 	apply_changeset(peer, &peer->remote, THEIRS,
 			ci->unacked_changes, tal_count(ci->unacked_changes));
 
@@ -410,6 +412,12 @@ void queue_pkt_revocation(struct peer *peer)
 
 	/* We should never look at this again. */
 	ci->unacked_changes = tal_free(ci->unacked_changes);
+
+	/* That revocation has committed us to changes in the current commitment.
+	 * Any acked changes come from their commitment, so those are now committed
+	 * by both of us.
+	 */
+	peer_both_committed_to(peer, ci->acked_changes, OURS);
 }
 
 Pkt *pkt_err(struct peer *peer, const char *msg, ...)
@@ -848,6 +856,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.
 	 */
+	add_acked_changes(&peer->local.commit->acked_changes, ci->unacked_changes);
 	apply_changeset(peer, &peer->local, OURS,
 			ci->unacked_changes,
 			tal_count(ci->unacked_changes));
@@ -855,6 +864,12 @@ Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt)
 	/* Should never examine these again. */
 	ci->unacked_changes = tal_free(ci->unacked_changes);
 
+	/* That revocation has committed them to changes in the current commitment.
+	 * Any acked changes come from our commitment, so those are now committed
+	 * by both of us.
+	 */
+	peer_both_committed_to(peer, ci->acked_changes, THEIRS);
+	
 	return NULL;
 }
 	

daemon/peer.c

@@ -2144,6 +2144,77 @@ void add_unacked(struct peer_visible_state *which,
 	which->commit->unacked_changes[n] = *stage;
 }
 
+void add_acked_changes(union htlc_staging **acked,
+		       const union htlc_staging *changes)
+{
+	size_t n_acked, n_changes;
+
+	n_acked = tal_count(*acked);
+	n_changes = tal_count(changes);
+	tal_resize(acked, n_acked + n_changes);
+	memcpy(*acked + n_acked, changes, n_changes * sizeof(*changes));
+}
+
+static const char *owner_name(enum channel_side side)
+{
+	return side == OURS ? "our" : "their";
+}
+
+/* When changes are committed to. */
+void peer_both_committed_to(struct peer *peer,
+			    const union htlc_staging *changes,
+			    enum channel_side side)
+{
+	size_t i, n = tal_count(changes);
+
+	for (i = 0; i < n; i++) {
+		u64 htlc_id;
+		const char *type, *owner;
+
+		switch (changes[i].type) {
+		case HTLC_ADD:
+			type = "ADD";
+			htlc_id = changes[i].add.htlc.id;
+			owner = owner_name(side);
+			assert(funding_htlc_by_id(peer->remote.commit->cstate, htlc_id,
+						  side));
+			assert(funding_htlc_by_id(peer->local.commit->cstate, htlc_id,
+						  side));
+			goto print;
+		case HTLC_FAIL:
+			type = "FAIL";
+			htlc_id = changes[i].fail.id;
+			owner = owner_name(!side);
+			assert(!funding_htlc_by_id(peer->remote.commit->cstate, htlc_id,
+						   !side));
+			assert(!funding_htlc_by_id(peer->local.commit->cstate, htlc_id,
+						   !side));
+			assert(funding_htlc_by_id(peer->remote.commit->prev->cstate,
+						  htlc_id, !side)
+			       || funding_htlc_by_id(peer->local.commit->prev->cstate,
+						     htlc_id, !side));
+			goto print;
+		case HTLC_FULFILL:
+			type = "FULFILL";
+			htlc_id = changes[i].fulfill.id;
+			owner = owner_name(!side);
+			assert(!funding_htlc_by_id(peer->remote.commit->cstate, htlc_id,
+						   !side));
+			assert(!funding_htlc_by_id(peer->local.commit->cstate, htlc_id,
+						   !side));
+			assert(funding_htlc_by_id(peer->remote.commit->prev->cstate,
+						  htlc_id, !side)
+			       || funding_htlc_by_id(peer->local.commit->prev->cstate,
+						     htlc_id, !side));
+			goto print;
+		}
+		abort();
+	print:
+		log_debug(peer->log, "Both committed to %s of %s HTLC %"PRIu64,
+			  type, owner, htlc_id);
+	}
+}
+
 /* Sets up the initial cstate and commit tx for both nodes: false if
  * insufficient funds. */
 bool setup_first_commit(struct peer *peer)

daemon/peer.h

@@ -73,6 +73,8 @@ struct commit_info {
 	struct sha256 *revocation_preimage;
 	/* unacked changes (already applied to staging_cstate) */
 	union htlc_staging *unacked_changes;
+	/* acked changes (already applied to staging_cstate) */
+	union htlc_staging *acked_changes;
 };
 
 struct peer_visible_state {
@@ -228,6 +230,14 @@ void remote_changes_pending(struct peer *peer);
 void add_unacked(struct peer_visible_state *which,
 		 const union htlc_staging *stage);
 
+/* These unacked changes are now acked; add them to acked set. */
+void add_acked_changes(union htlc_staging **acked,
+		       const union htlc_staging *changes);
+
+/* Both sides are committed to these changes they proposed. */
+void peer_both_committed_to(struct peer *peer,
+			    const union htlc_staging *changes, enum channel_side side);
+
 /* Peer has recieved revocation. */
 void peer_update_complete(struct peer *peer);
 

daemon/test/test.sh

@@ -439,8 +439,22 @@ if [ -n "$MANUALCOMMIT" ]; then
     # Node 2 has it committed.
     check_status_single lcli2 $B_AMOUNT $B_FEE "" $A_AMOUNT $A_FEE '{ "msatoshis" : '$HTLC_AMOUNT', "expiry" : { "second" : '$EXPIRY' }, "rhash" : "'$RHASH'" } '
 
+    # There should be no "both committed" here yet
+    if lcli1 getlog debug | $FGREP "Both committed"; then
+	echo "Node1 thinks they are both committed";
+	exit 1
+    fi
+    if lcli2 getlog debug | $FGREP "Both committed"; then
+	echo "Node2 thinks they are both committed";
+	exit 1
+    fi
+
     # Now node2 gives commitment to node1.
     lcli2 commit $ID1
+
+    # After revocation, they should know they're both committed.
+    check lcli1 "getlog debug | $FGREP 'Both committed to ADD of our HTLC'"
+    check lcli2 "getlog debug | $FGREP 'Both committed to ADD of their HTLC'"
 else
     A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
     A_FEE=$(($A_FEE + $EXTRA_FEE))
@@ -498,8 +512,23 @@ fi
     
 lcli2 fulfillhtlc $ID1 $SECRET
 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
+
+# Without manual commit, this check is racy.
+if [ -n "$MANUALCOMMIT" ]; then
+    if lcli1 getlog debug | $FGREP 'Both committed to FULFILL'; then
+	echo "Node1 thinks they are both committed";
+	exit 1
+    fi
+    if lcli2 getlog debug | $FGREP 'Both committed to FULFILL'; then
+	echo "Node2 thinks they are both committed";
+	exit 1
+    fi
+fi
 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
 
+check lcli1 "getlog debug | $FGREP 'Both committed to FULFILL of our HTLC'"
+check lcli2 "getlog debug | $FGREP 'Both committed to FULFILL of their HTLC'"
+
 # We've transferred the HTLC amount to 2, who now has to pay fees,
 # so no net change for A who saves on fees.
 B_FEE=$HTLC_AMOUNT