daemon: use HTLC states.

Since we only care about the latest commits, we can simply associate a
state with each HTLC, rather than using queues of HTLCs associated
with each commitment transaction.

This works far better in the context of a database.

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

daemon/Makefile

@@ -20,6 +20,7 @@ DAEMON_SRC :=					\
 	daemon/controlled_time.c		\
 	daemon/cryptopkt.c			\
 	daemon/dns.c				\
+	daemon/htlc.c				\
 	daemon/jsonrpc.c			\
 	daemon/lightningd.c			\
 	daemon/netaddr.c			\
@@ -45,6 +46,8 @@ DAEMON_CLI_OBJS := $(DAEMON_CLI_SRC:.c=.o)
 DAEMON_JSMN_OBJS := daemon/jsmn.o
 DAEMON_JSMN_HEADERS := daemon/jsmn/jsmn.h
 
+DAEMON_GEN_HEADERS := daemon/gen_htlc_state_names.h
+
 DAEMON_HEADERS :=				\
 	daemon/bitcoind.h			\
 	daemon/chaintopology.h			\
@@ -54,6 +57,7 @@ DAEMON_HEADERS :=				\
 	daemon/cryptopkt.h			\
 	daemon/dns.h				\
 	daemon/htlc.h				\
+	daemon/htlc_state.h			\
 	daemon/json.h				\
 	daemon/jsonrpc.h			\
 	daemon/lightningd.h			\
@@ -71,7 +75,10 @@ DAEMON_HEADERS :=				\
 	daemon/wallet.h				\
 	daemon/watch.h
 
-$(DAEMON_OBJS) $(DAEMON_LIB_OBJS) $(DAEMON_CLI_OBJS): $(DAEMON_HEADERS) $(DAEMON_JSMN_HEADERS) $(BITCOIN_HEADERS) $(CORE_HEADERS) $(GEN_HEADERS) $(CCAN_HEADERS)
+daemon/gen_htlc_state_names.h: daemon/htlc_state.h ccan/ccan/cdump/tools/cdump-enumstr
+	ccan/ccan/cdump/tools/cdump-enumstr daemon/htlc_state.h > $@
+
+$(DAEMON_OBJS) $(DAEMON_LIB_OBJS) $(DAEMON_CLI_OBJS): $(DAEMON_HEADERS) $(DAEMON_JSMN_HEADERS) $(BITCOIN_HEADERS) $(CORE_HEADERS) $(GEN_HEADERS) $(DAEMON_GEN_HEADERS) $(CCAN_HEADERS)
 $(DAEMON_JSMN_OBJS): $(DAEMON_JSMN_HEADERS)
 
 check-source: $(DAEMON_SRC:%=check-src-include-order/%)
@@ -79,7 +86,7 @@ check-source: $(DAEMON_LIB_SRC:%=check-src-include-order/%)
 check-source: $(DAEMON_CLI_SRC:%=check-src-include-order/%)
 check-source: $(DAEMON_HEADERS:%=check-hdr-include-order/%)
 check-daemon-makefile:
-	@if [ "`echo daemon/*.h`" != "$(DAEMON_HEADERS)" ]; then echo DAEMON_HEADERS incorrect; exit 1; fi
+	@if [ "`ls daemon/*.h | grep -v daemon/gen | tr '\012' ' '`" != "`echo $(DAEMON_HEADERS) ''`" ]; then echo DAEMON_HEADERS incorrect; exit 1; fi
 
 # Git submodules are seriously broken.
 daemon/jsmn/jsmn.c daemon/jsmn/jsmn.h:
@@ -95,3 +102,6 @@ daemon/lightning-cli: $(DAEMON_CLI_OBJS) $(DAEMON_LIB_OBJS) $(DAEMON_JSMN_OBJS)
 
 daemon-clean:
 	$(RM) $(DAEMON_OBJS) $(DAEMON_LIB_OBJS) $(DAEMON_CLI_OBJS) $(DAEMON_JSMN_OBJS)
+
+daemon-maintainer-clean:
+	$(RM) $(DAEMON_GEN_HEADERS)

daemon/htlc.c

@@ -0,0 +1,159 @@
+#include "htlc.h"
+#include "log.h"
+#include "peer.h"
+  #include "gen_htlc_state_names.h"
+#include <ccan/array_size/array_size.h>
+#include <inttypes.h>
+
+const char *htlc_state_name(enum htlc_state s)
+{
+	size_t i;
+
+	for (i = 0; enum_htlc_state_names[i].name; i++)
+		if (enum_htlc_state_names[i].v == s)
+			return enum_htlc_state_names[i].name;
+	return "unknown";
+}
+
+/* This is the flags for each state. */
+static const int per_state_bits[] = {
+	[SENT_ADD_HTLC] = HTLC_ADDING + HTLC_LOCAL_F_OWNER
+	+ HTLC_REMOTE_F_PENDING,
+
+	[SENT_ADD_COMMIT] = HTLC_ADDING + HTLC_LOCAL_F_OWNER
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_ADD_REVOCATION] = HTLC_ADDING + HTLC_LOCAL_F_OWNER
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_REMOTE_F_REVOKED
+	+ HTLC_LOCAL_F_PENDING
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_ADD_ACK_COMMIT] = HTLC_ADDING + HTLC_LOCAL_F_OWNER
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_REMOTE_F_REVOKED
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[SENT_ADD_ACK_REVOCATION] = HTLC_LOCAL_F_OWNER
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_REMOTE_F_REVOKED
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_LOCAL_F_REVOKED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_REMOVE_HTLC] = HTLC_REMOVING + HTLC_LOCAL_F_OWNER
+	+ HTLC_LOCAL_F_PENDING + HTLC_LOCAL_F_COMMITTED
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_REMOVE_COMMIT] = HTLC_REMOVING + HTLC_LOCAL_F_OWNER
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[SENT_REMOVE_REVOCATION] = HTLC_REMOVING + HTLC_LOCAL_F_OWNER
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_LOCAL_F_REVOKED
+	+ HTLC_REMOTE_F_PENDING
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[SENT_REMOVE_ACK_COMMIT] = HTLC_REMOVING + HTLC_LOCAL_F_OWNER
+	+ HTLC_LOCAL_F_REVOKED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_REMOVE_ACK_REVOCATION] = HTLC_LOCAL_F_OWNER
+	+ HTLC_LOCAL_F_REVOKED
+	+ HTLC_REMOTE_F_REVOKED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_ADD_HTLC] = HTLC_ADDING + HTLC_REMOTE_F_OWNER
+	+ HTLC_LOCAL_F_PENDING,
+
+	[RCVD_ADD_COMMIT] = HTLC_ADDING + HTLC_REMOTE_F_OWNER
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_LOCAL_F_WAS_COMMITTED,
+
+	[SENT_ADD_REVOCATION] = HTLC_ADDING + HTLC_REMOTE_F_OWNER
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_LOCAL_F_REVOKED
+	+ HTLC_REMOTE_F_PENDING
+	+ HTLC_LOCAL_F_WAS_COMMITTED,
+
+	[SENT_ADD_ACK_COMMIT] = HTLC_ADDING + HTLC_REMOTE_F_OWNER
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_LOCAL_F_REVOKED
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_ADD_ACK_REVOCATION] = HTLC_REMOTE_F_OWNER
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_LOCAL_F_REVOKED
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_REMOTE_F_REVOKED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[SENT_REMOVE_HTLC] = HTLC_REMOVING + HTLC_REMOTE_F_OWNER
+	+ HTLC_REMOTE_F_PENDING
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_REMOTE_F_COMMITTED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[SENT_REMOVE_COMMIT] = HTLC_REMOVING + HTLC_REMOTE_F_OWNER
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_REMOVE_REVOCATION] = HTLC_REMOVING + HTLC_REMOTE_F_OWNER
+	+ HTLC_LOCAL_F_COMMITTED
+	+ HTLC_REMOTE_F_REVOKED
+	+ HTLC_LOCAL_F_PENDING
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[RCVD_REMOVE_ACK_COMMIT] = HTLC_REMOVING + HTLC_REMOTE_F_OWNER
+	+ HTLC_REMOTE_F_REVOKED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED,
+
+	[SENT_REMOVE_ACK_REVOCATION] = HTLC_REMOTE_F_OWNER
+	+ HTLC_REMOTE_F_REVOKED
+	+ HTLC_LOCAL_F_REVOKED
+	+ HTLC_LOCAL_F_WAS_COMMITTED
+	+ HTLC_REMOTE_F_WAS_COMMITTED
+};
+
+int htlc_state_flags(enum htlc_state state)
+{
+	assert(state < ARRAY_SIZE(per_state_bits));
+	assert(per_state_bits[state]);
+	return per_state_bits[state];
+}
+
+void htlc_changestate(struct htlc *h,
+		      enum htlc_state oldstate,
+		      enum htlc_state newstate)
+{
+	log_debug(h->peer->log, "htlc %"PRIu64": %s->%s", h->id,
+		  htlc_state_name(h->state), htlc_state_name(newstate));
+	assert(h->state == oldstate);
+
+	/* You can only go to consecutive states. */
+	assert(newstate == h->state + 1);
+
+	/* You can't change sides. */
+	assert((htlc_state_flags(h->state)&(HTLC_LOCAL_F_OWNER|HTLC_REMOTE_F_OWNER))
+	       == (htlc_state_flags(newstate)&(HTLC_LOCAL_F_OWNER|HTLC_REMOTE_F_OWNER)));
+
+	h->state = newstate;
+}

daemon/htlc.h

@@ -3,19 +3,56 @@
 #include "config.h"
 #include "bitcoin/locktime.h"
 #include "channel.h"
+#include "htlc_state.h"
 #include "pseudorand.h"
+#include <assert.h>
 #include <ccan/crypto/sha256/sha256.h>
 #include <ccan/crypto/siphash24/siphash24.h>
 #include <ccan/htable/htable_type.h>
 #include <ccan/short_types/short_types.h>
 
+/* What are we doing: adding or removing? */
+#define HTLC_ADDING			0x400
+#define HTLC_REMOVING			0x800
+
+/* Uncommitted change is pending */
+#define HTLC_F_PENDING			0x01
+/* HTLC is in commit_tx */
+#define HTLC_F_COMMITTED		0x02
+/* We have revoked the previous commit_tx */
+#define HTLC_F_REVOKED			0x04
+/* We offered it it. */
+#define HTLC_F_OWNER			0x08
+/* HTLC was ever in a commit_tx */
+#define HTLC_F_WAS_COMMITTED		0x10
+
+/* Each of the above flags applies to both sides */
+enum htlc_side {
+	LOCAL,
+	REMOTE
+};
+
+#define HTLC_FLAG(side,flag)		((flag) << ((side) * 5))
+
+#define HTLC_REMOTE_F_PENDING		HTLC_FLAG(REMOTE,HTLC_F_PENDING)
+#define HTLC_REMOTE_F_COMMITTED		HTLC_FLAG(REMOTE,HTLC_F_COMMITTED)
+#define HTLC_REMOTE_F_REVOKED		HTLC_FLAG(REMOTE,HTLC_F_REVOKED)
+#define HTLC_REMOTE_F_OWNER		HTLC_FLAG(REMOTE,HTLC_F_OWNER)
+#define HTLC_REMOTE_F_WAS_COMMITTED	HTLC_FLAG(REMOTE,HTLC_F_WAS_COMMITTED)
+
+#define HTLC_LOCAL_F_PENDING		HTLC_FLAG(LOCAL,HTLC_F_PENDING)
+#define HTLC_LOCAL_F_COMMITTED		HTLC_FLAG(LOCAL,HTLC_F_COMMITTED)
+#define HTLC_LOCAL_F_REVOKED		HTLC_FLAG(LOCAL,HTLC_F_REVOKED)
+#define HTLC_LOCAL_F_OWNER		HTLC_FLAG(LOCAL,HTLC_F_OWNER)
+#define HTLC_LOCAL_F_WAS_COMMITTED	HTLC_FLAG(LOCAL,HTLC_F_WAS_COMMITTED)
+
 struct htlc {
 	/* Useful for debugging, and decoding via ->src. */
 	struct peer *peer;
 	/* Block number where we abort if it's still live (OURS only) */
 	u32 deadline;
-	/* Did we create it, or did they? */
+	/* What's the status. */
-	enum channel_side side; 
+	enum htlc_state state; 
 	/* The unique ID for this peer and this direction (ours or theirs) */
 	u64 id;
 	/* The amount in millisatoshi. */
@@ -34,6 +71,40 @@ struct htlc {
 	struct htlc *src;
 };
 
+const char *htlc_state_name(enum htlc_state s);
+void htlc_changestate(struct htlc *h,
+		      enum htlc_state oldstate,
+		      enum htlc_state newstate);
+int htlc_state_flags(enum htlc_state state);
+
+static inline bool htlc_has(const struct htlc *h, int flag)
+{
+	return htlc_state_flags(h->state) & flag;
+}
+
+static inline enum htlc_side htlc_owner(const struct htlc *h)
+{
+	if (h->state < RCVD_ADD_HTLC) {
+		assert((htlc_state_flags(h->state)
+			& (HTLC_REMOTE_F_OWNER|HTLC_LOCAL_F_OWNER))
+		       == HTLC_LOCAL_F_OWNER);
+		return LOCAL;
+	} else {
+		assert((htlc_state_flags(h->state)
+			& (HTLC_REMOTE_F_OWNER|HTLC_LOCAL_F_OWNER))
+		       == HTLC_REMOTE_F_OWNER);
+		return REMOTE;
+	}
+}
+
+/* FIXME: Transitional function. */
+static inline enum channel_side htlc_channel_side(const struct htlc *h)
+{
+	if (htlc_owner(h) == LOCAL)
+		return OURS;
+	return THEIRS;
+}
+
 /* htlc_map: ID -> htlc mapping. */
 static inline u64 htlc_key(const struct htlc *h)
 {
@@ -49,6 +120,20 @@ static inline size_t htlc_hash(u64 id)
 }
 HTABLE_DEFINE_TYPE(struct htlc, htlc_key, htlc_hash, htlc_cmp, htlc_map);
 
+static inline struct htlc *htlc_get(struct htlc_map *htlcs, u64 id, enum htlc_side owner)
+{
+	struct htlc *h;
+	struct htlc_map_iter it;
+
+	for (h = htlc_map_getfirst(htlcs, id, &it);
+	     h;
+	     h = htlc_map_getnext(htlcs, id, &it)) {
+		if (h->id == id && htlc_has(h, HTLC_FLAG(owner,HTLC_F_OWNER)))
+			return h;
+	}
+	return NULL;
+}
+
 static inline size_t htlc_map_count(const struct htlc_map *htlcs)
 {
 	return htlcs->raw.elems;

daemon/htlc_state.h

@@ -0,0 +1,34 @@
+#ifndef LIGHTNING_DAEMON_HTLC_STATE_H
+#define LIGHTNING_DAEMON_HTLC_STATE_H
+#include "config.h"
+
+enum htlc_state {
+	/* When we add a new htlc, it goes in this order. */
+	SENT_ADD_HTLC,
+	SENT_ADD_COMMIT,
+	RCVD_ADD_REVOCATION,
+	RCVD_ADD_ACK_COMMIT,
+	SENT_ADD_ACK_REVOCATION,
+
+	/* When they remove an htlc, it goes from SENT_ADD_ACK_REVOCATION: */
+	RCVD_REMOVE_HTLC,
+	RCVD_REMOVE_COMMIT,
+	SENT_REMOVE_REVOCATION,
+	SENT_REMOVE_ACK_COMMIT,
+	RCVD_REMOVE_ACK_REVOCATION,
+
+	/* When they add a new htlc, it goes in this order. */
+	RCVD_ADD_HTLC,
+	RCVD_ADD_COMMIT,
+	SENT_ADD_REVOCATION,
+	SENT_ADD_ACK_COMMIT,
+	RCVD_ADD_ACK_REVOCATION,
+
+	/* When we remove an htlc, it goes from RCVD_ADD_ACK_REVOCATION: */
+	SENT_REMOVE_HTLC,
+	SENT_REMOVE_COMMIT,
+	RCVD_REMOVE_REVOCATION,
+	RCVD_REMOVE_ACK_COMMIT,
+	SENT_REMOVE_ACK_REVOCATION
+};
+#endif /* LIGHTNING_DAEMON_HTLC_STATE_H */

daemon/packets.c

@@ -635,7 +635,7 @@ Pkt *accept_pkt_htlc_add(struct peer *peer, const Pkt *pkt)
 	htlc = peer_new_htlc(peer, u->id, u->amount_msat, &rhash,
 			     abs_locktime_to_blocks(&expiry),
 			     u->route->info.data, u->route->info.len,
-			     NULL, THEIRS);
+			     NULL, RCVD_ADD_HTLC);
 
 	/* BOLT #2:
 	 *

daemon/peer.c

@@ -739,7 +739,7 @@ struct htlc *command_htlc_add(struct peer *peer, u64 msatoshis,
 
 	htlc = peer_new_htlc(peer, peer->htlc_id_counter,
 			     msatoshis, rhash, expiry, route, tal_count(route),
-			     src, OURS);
+			     src, SENT_ADD_HTLC);
 
 	/* FIXME: BOLT is not correct here: we should say IFF we cannot
 	 * afford it in remote at its own current proposed fee-rate. */
@@ -1009,10 +1009,11 @@ static void htlc_destroy(struct htlc *htlc)
 {
 	struct htlc_map *map;
 
-	if (htlc->side == OURS)
+	/* FIXME: make peer->local/remote an array*/
+	if (htlc_owner(htlc) == LOCAL)
 		map = &htlc->peer->local.htlcs;
 	else {
-		assert(htlc->side == THEIRS);
+		assert(htlc_owner(htlc) == REMOTE);
 		map = &htlc->peer->remote.htlcs;
 	}
 
@@ -1028,11 +1029,11 @@ struct htlc *peer_new_htlc(struct peer *peer,
 			   const u8 *route,
 			   size_t routelen,
 			   struct htlc *src,
-			   enum channel_side side)
+			   enum htlc_state state)
 {
 	struct htlc *h = tal(peer, struct htlc);
 	h->peer = peer;
-	h->side = side;
+	assert(state == SENT_ADD_HTLC || state == RCVD_ADD_HTLC);
 	h->id = id;
 	h->msatoshis = msatoshis;
 	h->rhash = *rhash;
@@ -1041,7 +1042,7 @@ struct htlc *peer_new_htlc(struct peer *peer,
 		fatal("Invalid HTLC expiry %u", expiry);
 	h->routing = tal_dup_arr(h, u8, route, routelen, 0);
 	h->src = src;
-	if (side == OURS) {
+	if (htlc_owner(h) == LOCAL) {
 		if (src) {
 			h->deadline = abs_locktime_to_blocks(&src->expiry)
 				- peer->dstate->config.deadline_blocks;
@@ -1051,7 +1052,7 @@ struct htlc *peer_new_htlc(struct peer *peer,
 				+ peer->dstate->config.min_htlc_expiry;
 		htlc_map_add(&peer->local.htlcs, h);
 	} else {
-		assert(side == THEIRS);
+		assert(htlc_owner(h) == REMOTE);
 		htlc_map_add(&peer->remote.htlcs, h);
 	}
 	tal_add_destructor(h, htlc_destroy);

daemon/peer.h

@@ -247,7 +247,7 @@ struct htlc *peer_new_htlc(struct peer *peer,
 			   const u8 *route,
 			   size_t route_len,
 			   struct htlc *src,
-			   enum channel_side side);
+			   enum htlc_state state);
 
 struct htlc *command_htlc_add(struct peer *peer, u64 msatoshis,
 			      unsigned int expiry,