lightningd: even more HTLC consistency checking: check states.

This means we need to check when we've altered the state, so the checks
are moved to the callers of htlc_in_update_state and htlc_out_update_state.

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

lightningd/htlc_end.c

@@ -87,6 +87,23 @@ struct htlc_in *htlc_in_check(const struct htlc_in *hin, const char *abortstr)
 	else if (hin->failuremsg && (hin->failcode & BADONION))
 		return corrupt(abortstr, "Both failed and malformed");
 
+	/* Can't have a resolution while still being added. */
+	if (hin->hstate >= RCVD_ADD_HTLC
+	    && hin->hstate <= RCVD_ADD_ACK_REVOCATION) {
+		if (hin->preimage)
+			return corrupt(abortstr, "Still adding, has preimage");
+		if (hin->failuremsg)
+			return corrupt(abortstr, "Still adding, has failmsg");
+		if (hin->failcode)
+			return corrupt(abortstr, "Still adding, has failcode");
+	} else if (hin->hstate >= SENT_REMOVE_HTLC
+		   && hin->hstate <= SENT_REMOVE_ACK_REVOCATION) {
+		if (!hin->preimage && !hin->failuremsg && !hin->failcode)
+			return corrupt(abortstr, "Removing, no resolution");
+	} else
+		return corrupt(abortstr, "Bad state %s",
+			       htlc_state_name(hin->hstate));
+
 	return cast_const(struct htlc_in *, hin);
 }
 
@@ -171,10 +188,25 @@ struct htlc_out *htlc_out_check(const struct htlc_out *hout,
 				return corrupt(abortstr,
 					       "Output unresolved, input failcode");
 		}
-
-		/* FIXME: Check hout->in->hstate. */
 	}
 
+	/* Can't have a resolution while still being added. */
+	if (hout->hstate >= SENT_ADD_HTLC
+	    && hout->hstate <= SENT_ADD_ACK_REVOCATION) {
+		if (hout->preimage)
+			return corrupt(abortstr, "Still adding, has preimage");
+		if (hout->failuremsg)
+			return corrupt(abortstr, "Still adding, has failmsg");
+		if (hout->failcode)
+			return corrupt(abortstr, "Still adding, has failcode");
+	} else if (hout->hstate >= RCVD_REMOVE_HTLC
+		   && hout->hstate <= RCVD_REMOVE_ACK_REVOCATION) {
+		if (!hout->preimage && !hout->failuremsg && !hout->failcode)
+			return corrupt(abortstr, "Removing, no resolution");
+	} else
+		return corrupt(abortstr, "Bad state %s",
+			       htlc_state_name(hout->hstate));
+
 	return cast_const(struct htlc_out *, hout);
 }
 

lightningd/peer_htlcs.c

@@ -65,7 +65,6 @@ static bool htlc_in_update_state(struct channel *channel,
 			   hin->dbid, newstate, hin->preimage);
 
 	hin->hstate = newstate;
-	htlc_in_check(hin, __func__);
 	return true;
 }
 
@@ -81,7 +80,6 @@ static bool htlc_out_update_state(struct channel *channel,
 			   NULL);
 
 	hout->hstate = newstate;
-	htlc_out_check(hout, __func__);
 	return true;
 }
 
@@ -106,6 +104,7 @@ static void fail_in_htlc(struct htlc_in *hin,
 
 	/* We update state now to signal it's in progress, for persistence. */
 	htlc_in_update_state(hin->key.channel, hin, SENT_REMOVE_HTLC);
+	htlc_in_check(hin, __func__);
 
 	/* Tell peer, if we can. */
 	if (!hin->key.channel->owner)
@@ -215,10 +214,12 @@ static void fulfill_htlc(struct htlc_in *hin, const struct preimage *preimage)
 	struct wallet *wallet = channel->peer->ld->wallet;
 
 	hin->preimage = tal_dup(hin, struct preimage, preimage);
-	htlc_in_check(hin, __func__);
 
 	/* We update state now to signal it's in progress, for persistence. */
 	htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC);
+
+	htlc_in_check(hin, __func__);
+
 	/* Update channel stats */
 	wallet_channel_stats_incr_in_fulfilled(wallet,
 					       channel->dbid,
@@ -351,6 +352,12 @@ static void destroy_hout_subd_died(struct htlc_out *hout)
 		  hout->key.id);
 
 	hout->failcode = WIRE_TEMPORARY_CHANNEL_FAILURE;
+
+	/* Assign a temporary state (we're about to free it!) so checks
+	 * are happy that it has a failure code */
+	assert(hout->hstate == SENT_ADD_HTLC);
+	hout->hstate = RCVD_REMOVE_HTLC;
+
 	fail_out_htlc(hout, "Outgoing subdaemon died");
 }
 
@@ -615,6 +622,7 @@ static bool peer_accepted_htlc(struct channel *channel,
 
 	if (!htlc_in_update_state(channel, hin, RCVD_ADD_ACK_REVOCATION))
 		return false;
+	htlc_in_check(hin, __func__);
 
 #if DEVELOPER
 	if (channel->peer->ignore_htlcs) {
@@ -779,8 +787,11 @@ void onchain_fulfilled_htlc(struct channel *channel,
 
 		/* We may have already fulfilled before going onchain, or
 		 * we can fulfill onchain multiple times. */
-		if (!hout->preimage)
+		if (!hout->preimage) {
+			/* Force state to something which allows a preimage */
+			hout->hstate = RCVD_REMOVE_HTLC;
 			fulfill_our_htlc_out(channel, hout, preimage);
+		}
 
 		/* We keep going: this is something of a leak, but onchain
 		 * we have no real way of distinguishing HTLCs anyway */
@@ -855,6 +866,12 @@ void onchain_failed_our_htlc(const struct channel *channel,
 
 	hout->failcode = WIRE_PERMANENT_CHANNEL_FAILURE;
 
+	/* Force state to something which expects a failure, and save to db */
+	hout->hstate = RCVD_REMOVE_HTLC;
+	wallet_htlc_update(channel->peer->ld->wallet, hout->dbid, hout->hstate,
+			   NULL);
+	htlc_out_check(hout, __func__);
+
 	if (!hout->in) {
 		assert(why != NULL);
 		char *localfail = tal_fmt(channel, "%s: %s",
@@ -932,6 +949,7 @@ static bool update_in_htlc(struct channel *channel,
 	if (!htlc_in_update_state(channel, hin, newstate))
 		return false;
 
+	htlc_in_check(hin, __func__);
 	if (newstate == SENT_REMOVE_ACK_REVOCATION)
 		remove_htlc_in(channel, hin);