channeld: don't get details of our own failed htlcs at init.

For incoming htlcs, we need failure details in case we need to
re-xmit them.  But for outgoing htlcs, lightningd is telling us it
already knows they've failed, so we just need to flag them failed
and don't need the details.

Internally, we set the ->fail to a dummy non-NULL value; this is
cleaned up next.

This matters for the next patch, which moves onion handling into
lightningd.

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

channeld/channel_wire.csv

@@ -45,9 +45,10 @@ msgdata,channel_init,htlc_states,enum htlc_state,num_htlcs
 msgdata,channel_init,num_fulfilled,u16,
 msgdata,channel_init,fulfilled,fulfilled_htlc,num_fulfilled
 msgdata,channel_init,fulfilled_sides,enum side,num_fulfilled
-msgdata,channel_init,num_failed,u16,
-msgdata,channel_init,failed,failed_htlc,num_failed
-msgdata,channel_init,failed_sides,enum side,num_failed
+msgdata,channel_init,num_failed_in,u16,
+msgdata,channel_init,failed_in,failed_htlc,num_failed_in
+msgdata,channel_init,num_failed_out,u16,
+msgdata,channel_init,failed_out,u64,num_failed_out
 msgdata,channel_init,failheight,u32,
 msgdata,channel_init,local_funding_locked,bool,
 msgdata,channel_init,remote_funding_locked,bool,

channeld/channeld.c

@@ -3005,8 +3005,8 @@ static void init_channel(struct peer *peer)
 	enum htlc_state *hstates;
 	struct fulfilled_htlc *fulfilled;
 	enum side *fulfilled_sides;
-	struct failed_htlc **failed;
-	enum side *failed_sides;
+	struct failed_htlc **failed_in;
+	u64 *failed_out;
 	struct added_htlc *htlcs;
 	bool reconnected;
 	u8 *funding_signed;
@@ -3060,8 +3060,8 @@ static void init_channel(struct peer *peer)
 				   &hstates,
 				   &fulfilled,
 				   &fulfilled_sides,
-				   &failed,
-				   &failed_sides,
+				   &failed_in,
+				   &failed_out,
 				   &failheight,
 				   &peer->funding_locked[LOCAL],
 				   &peer->funding_locked[REMOTE],
@@ -3143,8 +3143,9 @@ static void init_channel(struct peer *peer)
 	if (!channel_force_htlcs(peer->channel, htlcs, hstates,
 				 fulfilled, fulfilled_sides,
 				 cast_const2(const struct failed_htlc **,
-					     failed),
-				 failed_sides, failheight))
+					     failed_in),
+				 failed_out,
+				 failheight))
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "Could not restore HTLCs");
 
@@ -3157,8 +3158,8 @@ static void init_channel(struct peer *peer)
 	tal_free(hstates);
 	tal_free(fulfilled);
 	tal_free(fulfilled_sides);
-	tal_free(failed);
-	tal_free(failed_sides);
+	tal_free(failed_in);
+	tal_free(failed_out);
 	tal_free(remote_ann_node_sig);
 	tal_free(remote_ann_bitcoin_sig);
 

channeld/full_channel.c

@@ -1188,8 +1188,8 @@ bool channel_force_htlcs(struct channel *channel,
 			 const enum htlc_state *hstates,
 			 const struct fulfilled_htlc *fulfilled,
 			 const enum side *fulfilled_sides,
-			 const struct failed_htlc **failed,
-			 const enum side *failed_sides,
+			 const struct failed_htlc **failed_in,
+			 const u64 *failed_out,
 			 u32 failheight)
 {
 	size_t i;
@@ -1209,12 +1209,6 @@ bool channel_force_htlcs(struct channel *channel,
 		return false;
 	}
 
-	if (tal_count(failed) != tal_count(failed_sides)) {
-		status_broken("#failed sides %zu != #failed %zu",
-			     tal_count(failed_sides), tal_count(failed));
-		return false;
-	}
-
 	for (i = 0; i < tal_count(htlcs); i++) {
 		enum channel_add_err e;
 		struct htlc *htlc;
@@ -1282,58 +1276,88 @@ bool channel_force_htlcs(struct channel *channel,
 				  &fulfilled[i].payment_preimage);
 	}
 
-	for (i = 0; i < tal_count(failed); i++) {
+	for (i = 0; i < tal_count(failed_in); i++) {
 		struct htlc *htlc;
-		htlc = channel_get_htlc(channel, failed_sides[i],
-					failed[i]->id);
+		htlc = channel_get_htlc(channel, REMOTE, failed_in[i]->id);
 		if (!htlc) {
-			status_broken("Fail %s HTLC %"PRIu64" not found",
-				     failed_sides[i] == LOCAL ? "out" : "in",
-				     failed[i]->id);
+			status_broken("Fail in HTLC %"PRIu64" not found",
+				     failed_in[i]->id);
 			return false;
 		}
 		if (htlc->r) {
-			status_broken("Fail %s HTLC %"PRIu64" already fulfilled",
-				     failed_sides[i] == LOCAL ? "out" : "in",
-				     failed[i]->id);
+			status_broken("Fail in HTLC %"PRIu64" already fulfilled",
+				     failed_in[i]->id);
 			return false;
 		}
 		if (htlc->fail) {
-			status_broken("Fail %s HTLC %"PRIu64" already failed",
-				     failed_sides[i] == LOCAL ? "out" : "in",
-				     failed[i]->id);
+			status_broken("Fail in HTLC %"PRIu64" already failed_in",
+				     failed_in[i]->id);
 			return false;
 		}
 		if (htlc->failcode) {
-			status_broken("Fail %s HTLC %"PRIu64" already fail %u",
-				     failed_sides[i] == LOCAL ? "out" : "in",
-				     failed[i]->id, htlc->failcode);
+			status_broken("Fail in HTLC %"PRIu64" already fail %u",
+				     failed_in[i]->id, htlc->failcode);
 			return false;
 		}
 		if (!htlc_has(htlc, HTLC_REMOVING)) {
-			status_broken("Fail %s HTLC %"PRIu64" state %s",
-				     failed_sides[i] == LOCAL ? "out" : "in",
-				     failed[i]->id,
+			status_broken("Fail in HTLC %"PRIu64" state %s",
+				     failed_in[i]->id,
 				     htlc_state_name(htlc->state));
 			return false;
 		}
-		htlc->failcode = failed[i]->failcode;
-		/* We assume they all failed at the same height, which is
+		htlc->failcode = failed_in[i]->failcode;
+		/* We assume they all failed_in at the same height, which is
 		 * not necessarily true in case of restart.  But it's only
 		 * a hint. */
 		htlc->failblock = failheight;
-		if (failed[i]->failreason)
-			htlc->fail = dup_onionreply(htlc, failed[i]->failreason);
+		if (failed_in[i]->failreason)
+			htlc->fail = dup_onionreply(htlc, failed_in[i]->failreason);
 		else
 			htlc->fail = NULL;
-		if (failed[i]->scid)
+		if (failed_in[i]->scid)
 			htlc->failed_scid = tal_dup(htlc,
 						    struct short_channel_id,
-						    failed[i]->scid);
+						    failed_in[i]->scid);
 		else
 			htlc->failed_scid = NULL;
 	}
 
+	for (i = 0; i < tal_count(failed_out); i++) {
+		struct htlc *htlc;
+
+		htlc = channel_get_htlc(channel, LOCAL, failed_out[i]);
+		if (!htlc) {
+			status_broken("Fail out HTLC %"PRIu64" not found",
+				      failed_out[i]);
+			return false;
+		}
+		if (htlc->r) {
+			status_broken("Fail out HTLC %"PRIu64" already fulfilled",
+				      failed_out[i]);
+			return false;
+		}
+		if (htlc->fail) {
+			status_broken("Fail out HTLC %"PRIu64" already failed",
+				      failed_out[i]);
+			return false;
+		}
+		if (htlc->failcode) {
+			status_broken("Fail out HTLC %"PRIu64" already fail %u",
+				      failed_out[i], htlc->failcode);
+			return false;
+		}
+		if (!htlc_has(htlc, HTLC_REMOVING)) {
+			status_broken("Fail out HTLC %"PRIu64" state %s",
+				      failed_out[i],
+				      htlc_state_name(htlc->state));
+			return false;
+		}
+
+		/* Now, we don't really care why our htlcs failed: lightningd
+		 * already knows.  Just mark it failed using anything. */
+		htlc->fail = tal(htlc, struct onionreply);
+	}
+
 	/* You'd think, since we traverse HTLCs in ID order, this would never
 	 * go negative.  But this ignores the fact that HTLCs ids from each
 	 * side have no correlation with each other.  Copy into struct balance,

channeld/full_channel.h

@@ -231,8 +231,8 @@ size_t num_channel_htlcs(const struct channel *channel);
  * @hstates: the states for the htlcs (tal_arr of same size)
  * @fulfilled: htlcs of those which are fulfilled
  * @fulfilled_sides: sides for ids in @fulfilled
- * @failed: htlcs of those which are failed
- * @failed_sides: sides for ids in @failed
+ * @failed_in: incoming htlcs which are failed
+ * @failed_out: outgoing htlc ids which are failed
  * @failheight: block number which htlcs failed at.
  *
  * This is used for restoring a channel state.
@@ -242,8 +242,8 @@ bool channel_force_htlcs(struct channel *channel,
 			 const enum htlc_state *hstates,
 			 const struct fulfilled_htlc *fulfilled,
 			 const enum side *fulfilled_sides,
-			 const struct failed_htlc **failed,
-			 const enum side *failed_sides,
+			 const struct failed_htlc **failed_in,
+			 const u64 *failed_out,
 			 u32 failheight);
 
 /**

lightningd/channel_control.c

@@ -369,8 +369,8 @@ void peer_start_channeld(struct channel *channel,
 	enum htlc_state *htlc_states;
 	struct fulfilled_htlc *fulfilled_htlcs;
 	enum side *fulfilled_sides;
-	const struct failed_htlc **failed_htlcs;
-	enum side *failed_sides;
+	const struct failed_htlc **failed_in;
+	u64 *failed_out;
 	struct short_channel_id scid;
 	u64 num_revocations;
 	struct lightningd *ld = channel->peer->ld;
@@ -409,7 +409,7 @@ void peer_start_channeld(struct channel *channel,
 	}
 
 	peer_htlcs(tmpctx, channel, &htlcs, &htlc_states, &fulfilled_htlcs,
-		   &fulfilled_sides, &failed_htlcs, &failed_sides);
+		   &fulfilled_sides, &failed_in, &failed_out);
 
 	if (channel->scid) {
 		scid = *channel->scid;
@@ -492,7 +492,7 @@ void peer_start_channeld(struct channel *channel,
 				      channel->next_htlc_id,
 				      htlcs, htlc_states,
 				      fulfilled_htlcs, fulfilled_sides,
-				      failed_htlcs, failed_sides,
+				      failed_in, failed_out,
 				      /* This is an approximation, but failing
 				       * on restart is a corner case */
 				      get_block_height(ld->topology),

lightningd/peer_htlcs.c

@@ -1799,12 +1799,11 @@ static void add_fulfill(u64 id, enum side side,
 	tal_arr_expand(fulfilled_sides, side);
 }
 
-static void add_fail(u64 id, enum side side,
+static void add_fail(u64 id,
 		     enum onion_type failcode,
 		     const struct short_channel_id *failing_channel,
 		     const struct onionreply *failonion,
-		     const struct failed_htlc ***failed_htlcs,
-		     enum side **failed_sides)
+		     const struct failed_htlc ***failed_htlcs)
 {
 	struct failed_htlc *newf;
 
@@ -1824,7 +1823,6 @@ static void add_fail(u64 id, enum side side,
 		newf->failreason = NULL;
 
 	tal_arr_expand(failed_htlcs, newf);
-	tal_arr_expand(failed_sides, side);
 }
 
 /* FIXME: Load direct from db. */
@@ -1834,8 +1832,8 @@ void peer_htlcs(const tal_t *ctx,
 		enum htlc_state **htlc_states,
 		struct fulfilled_htlc **fulfilled_htlcs,
 		enum side **fulfilled_sides,
-		const struct failed_htlc ***failed_htlcs,
-		enum side **failed_sides)
+		const struct failed_htlc ***failed_in,
+		u64 **failed_out)
 {
 	struct htlc_in_map_iter ini;
 	struct htlc_out_map_iter outi;
@@ -1847,8 +1845,8 @@ void peer_htlcs(const tal_t *ctx,
 	*htlc_states = tal_arr(ctx, enum htlc_state, 0);
 	*fulfilled_htlcs = tal_arr(ctx, struct fulfilled_htlc, 0);
 	*fulfilled_sides = tal_arr(ctx, enum side, 0);
-	*failed_htlcs = tal_arr(ctx, const struct failed_htlc *, 0);
-	*failed_sides = tal_arr(ctx, enum side, 0);
+	*failed_in = tal_arr(ctx, const struct failed_htlc *, 0);
+	*failed_out = tal_arr(ctx, u64, 0);
 
 	for (hin = htlc_in_map_first(&ld->htlcs_in, &ini);
 	     hin;
@@ -1862,9 +1860,9 @@ void peer_htlcs(const tal_t *ctx,
 			 hin->hstate);
 
 		if (hin->failonion || hin->failcode)
-			add_fail(hin->key.id, REMOTE, hin->failcode,
+			add_fail(hin->key.id, hin->failcode,
 				 &hin->failoutchannel,
-				 hin->failonion, failed_htlcs, failed_sides);
+				 hin->failonion, failed_in);
 		if (hin->preimage)
 			add_fulfill(hin->key.id, REMOTE, hin->preimage,
 				    fulfilled_htlcs, fulfilled_sides);
@@ -1882,9 +1880,8 @@ void peer_htlcs(const tal_t *ctx,
 			 hout->hstate);
 
 		if (hout->failonion || hout->failcode)
-			add_fail(hout->key.id, LOCAL, hout->failcode,
-				 hout->key.channel->scid,
-				 hout->failonion, failed_htlcs, failed_sides);
+			tal_arr_expand(failed_out, hout->key.id);
+
 		if (hout->preimage)
 			add_fulfill(hout->key.id, LOCAL, hout->preimage,
 				    fulfilled_htlcs, fulfilled_sides);

lightningd/peer_htlcs.h

@@ -35,8 +35,8 @@ void peer_htlcs(const tal_t *ctx,
 		enum htlc_state **htlc_states,
 		struct fulfilled_htlc **fulfilled_htlcs,
 		enum side **fulfilled_sides,
-		const struct failed_htlc ***failed_htlcs,
-		enum side **failed_sides);
+		const struct failed_htlc ***failed_in,
+		u64 **failed_out);
 
 void free_htlcs(struct lightningd *ld, const struct channel *channel);