common: treat all "all-channels" errors as if they were warnings.

This is in line with the warnings draft, where all-zeroes in a
channel_id is no longer special (i.e. it will be ignored).

But gossipd would send these if it got upset with us, so it's best
practice to ignore them for now anyway.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we treat error messages from peer which refer to "all channels" as warnings, not errors.

common/peer_failed.c

@@ -42,7 +42,10 @@ void peer_failed(struct per_peer_state *pps,
 
 	/* Tell master the error so it can re-xmit. */
 	msg = towire_status_peer_error(NULL, channel_id,
-				       desc, false, pps,
+				       desc,
+				       /* all-channels errors should not close channels */
+				       channel_id_is_all(channel_id),
+				       pps,
 				       err);
 	peer_billboard(true, desc);
 	tal_free(desc);
@@ -53,14 +56,11 @@ void peer_failed(struct per_peer_state *pps,
 void peer_failed_received_errmsg(struct per_peer_state *pps,
 				 const char *desc,
 				 const struct channel_id *channel_id,
-				 bool soft_error)
+				 bool warning)
 {
-	static const struct channel_id all_channels;
 	u8 *msg;
 
-	if (!channel_id)
-		channel_id = &all_channels;
-	msg = towire_status_peer_error(NULL, channel_id, desc, soft_error, pps,
+	msg = towire_status_peer_error(NULL, channel_id, desc, warning, pps,
 				       NULL);
 	peer_billboard(true, "Received %s", desc);
 	peer_fatal_continue(take(msg), pps);

common/peer_status_wire.csv

@@ -7,7 +7,7 @@ msgtype,status_peer_error,0xFFF4
 msgdata,status_peer_error,channel,channel_id,
 msgdata,status_peer_error,desc,wirestring,
 # Take a deep breath, then try reconnecting to the precious little snowflake.
-msgdata,status_peer_error,soft_error,bool,
+msgdata,status_peer_error,warning,bool,
 msgdata,status_peer_error,pps,per_peer_state,
 msgdata,status_peer_error,len,u16,
 msgdata,status_peer_error,error_for_them,u8,len

common/peer_status_wiregen.c

@@ -42,7 +42,7 @@ bool peer_status_wire_is_defined(u16 type)
 
 /* WIRE: STATUS_PEER_ERROR */
 /* An error occurred: if error_for_them */
-u8 *towire_status_peer_error(const tal_t *ctx, const struct channel_id *channel, const wirestring *desc, bool soft_error, const struct per_peer_state *pps, const u8 *error_for_them)
+u8 *towire_status_peer_error(const tal_t *ctx, const struct channel_id *channel, const wirestring *desc, bool warning, const struct per_peer_state *pps, const u8 *error_for_them)
 {
 	u16 len = tal_count(error_for_them);
 	u8 *p = tal_arr(ctx, u8, 0);
@@ -52,14 +52,14 @@ u8 *towire_status_peer_error(const tal_t *ctx, const struct channel_id *channel,
 	towire_channel_id(&p, channel);
 	towire_wirestring(&p, desc);
 	/* Take a deep breath */
-	towire_bool(&p, soft_error);
+	towire_bool(&p, warning);
 	towire_per_peer_state(&p, pps);
 	towire_u16(&p, len);
 	towire_u8_array(&p, error_for_them, len);
 
 	return memcheck(p, tal_count(p));
 }
-bool fromwire_status_peer_error(const tal_t *ctx, const void *p, struct channel_id *channel, wirestring **desc, bool *soft_error, struct per_peer_state **pps, u8 **error_for_them)
+bool fromwire_status_peer_error(const tal_t *ctx, const void *p, struct channel_id *channel, wirestring **desc, bool *warning, struct per_peer_state **pps, u8 **error_for_them)
 {
 	u16 len;
 
@@ -72,7 +72,7 @@ bool fromwire_status_peer_error(const tal_t *ctx, const void *p, struct channel_
 	fromwire_channel_id(&cursor, &plen, channel);
  	*desc = fromwire_wirestring(ctx, &cursor, &plen);
  	/* Take a deep breath */
-	*soft_error = fromwire_bool(&cursor, &plen);
+	*warning = fromwire_bool(&cursor, &plen);
  	*pps = fromwire_per_peer_state(ctx, &cursor, &plen);
  	len = fromwire_u16(&cursor, &plen);
  	// 2nd case error_for_them
@@ -80,4 +80,4 @@ bool fromwire_status_peer_error(const tal_t *ctx, const void *p, struct channel_
 	fromwire_u8_array(&cursor, &plen, *error_for_them, len);
 	return cursor != NULL;
 }
-// SHA256STAMP:de3fc242012abe21984a0590cc604e83e52af8809e3ff357acc5e6f4d7d1d41d
+// SHA256STAMP:c002247f54d5016e614dd6d757c7d06f65c713c3e19d17901f7f685a6bd4b9d9

common/peer_status_wiregen.h

@@ -29,9 +29,9 @@ bool peer_status_wire_is_defined(u16 type);
 
 /* WIRE: STATUS_PEER_ERROR */
 /*  An error occurred: if error_for_them */
-u8 *towire_status_peer_error(const tal_t *ctx, const struct channel_id *channel, const wirestring *desc, bool soft_error, const struct per_peer_state *pps, const u8 *error_for_them);
-bool fromwire_status_peer_error(const tal_t *ctx, const void *p, struct channel_id *channel, wirestring **desc, bool *soft_error, struct per_peer_state **pps, u8 **error_for_them);
+u8 *towire_status_peer_error(const tal_t *ctx, const struct channel_id *channel, const wirestring *desc, bool warning, const struct per_peer_state *pps, const u8 *error_for_them);
+bool fromwire_status_peer_error(const tal_t *ctx, const void *p, struct channel_id *channel, wirestring **desc, bool *warning, struct per_peer_state **pps, u8 **error_for_them);
 
 
 #endif /* LIGHTNING_COMMON_PEER_STATUS_WIREGEN_H */
-// SHA256STAMP:de3fc242012abe21984a0590cc604e83e52af8809e3ff357acc5e6f4d7d1d41d
+// SHA256STAMP:c002247f54d5016e614dd6d757c7d06f65c713c3e19d17901f7f685a6bd4b9d9

common/read_peer_msg.c

@@ -69,8 +69,7 @@ u8 *peer_or_gossip_sync_read(const tal_t *ctx,
 
 bool is_peer_error(const tal_t *ctx, const u8 *msg,
 		   const struct channel_id *channel_id,
-		   char **desc, bool *all_channels,
-		   bool *warning)
+		   char **desc, bool *warning)
 {
 	struct channel_id err_chanid;
 
@@ -95,8 +94,11 @@ bool is_peer_error(const tal_t *ctx, const u8 *msg,
 	 *  - if no existing channel is referred to by the message:
 	 *    - MUST ignore the message.
 	 */
-	*all_channels = channel_id_is_all(&err_chanid);
-	if (!*all_channels && !channel_id_eq(&err_chanid, channel_id))
+	/* FIXME: The spec changed, so for *errors* all 0 is not special.
+	 * But old gossipd would send these, so we turn them into warnings */
+	if (channel_id_is_all(&err_chanid))
+		*warning = true;
+	else if (!channel_id_eq(&err_chanid, channel_id))
 		*desc = tal_free(*desc);
 
 	return true;
@@ -159,7 +161,7 @@ bool handle_peer_gossip_or_error(struct per_peer_state *pps,
 				 const u8 *msg TAKES)
 {
 	char *err;
-	bool all_channels, warning;
+	bool warning;
 	struct channel_id actual;
 
 #if DEVELOPER
@@ -186,15 +188,15 @@ bool handle_peer_gossip_or_error(struct per_peer_state *pps,
 		return true;
 	}
 
-	if (is_peer_error(tmpctx, msg, channel_id, &err, &all_channels,
-			  &warning)) {
-		if (err)
-			peer_failed_received_errmsg(pps, err,
-						    all_channels
-						    ? NULL : channel_id,
-						    warning || soft_error);
-
+	if (is_peer_error(tmpctx, msg, channel_id, &err, &warning)) {
 		/* Ignore unknown channel errors. */
+		if (!err)
+			goto handled;
+
+		/* We hang up when a warning is received. */
+		peer_failed_received_errmsg(pps, err, channel_id,
+					    soft_error || warning);
+
 		goto handled;
 	}
 

common/read_peer_msg.h

@@ -33,7 +33,6 @@ u8 *peer_or_gossip_sync_read(const tal_t *ctx,
  * @msg: the peer message.
  * @channel_id: the channel id of the current channel.
  * @desc: set to non-NULL if this describes a channel we care about.
- * @all_channels: set to true if this applies to all channels.
  * @warning: set to true if this is a warning, not an error.
  *
  * If @desc is NULL, ignore this message.  Otherwise, that's usually passed
@@ -41,7 +40,7 @@ u8 *peer_or_gossip_sync_read(const tal_t *ctx,
  */
 bool is_peer_error(const tal_t *ctx, const u8 *msg,
 		   const struct channel_id *channel_id,
-		   char **desc, bool *all_channels, bool *warning);
+		   char **desc, bool *warning);
 
 /**
  * is_wrong_channel - if it's a message about a different channel, return true

lightningd/onchain_control.c

@@ -541,7 +541,7 @@ static void onchain_error(struct channel *channel,
 			  struct per_peer_state *pps UNUSED,
 			  const struct channel_id *channel_id UNUSED,
 			  const char *desc,
-			  bool soft_error UNUSED,
+			  bool warning UNUSED,
 			  const u8 *err_for_them UNUSED)
 {
 	/* FIXME: re-launch? */

lightningd/opening_common.c

@@ -64,7 +64,7 @@ void opend_channel_errmsg(struct uncommitted_channel *uc,
 			  struct per_peer_state *pps,
 			  const struct channel_id *channel_id UNUSED,
 			  const char *desc,
-			  bool soft_error UNUSED,
+			  bool warning UNUSED,
 			  const u8 *err_for_them UNUSED)
 {
 	/* Close fds, if any. */

lightningd/opening_common.h

@@ -96,7 +96,7 @@ void opend_channel_errmsg(struct uncommitted_channel *uc,
 			  struct per_peer_state *pps,
 			  const struct channel_id *channel_id UNUSED,
 			  const char *desc,
-			  bool soft_error UNUSED,
+			  bool warning UNUSED,
 			  const u8 *err_for_them UNUSED);
 
 void opend_channel_set_billboard(struct uncommitted_channel *uc,

lightningd/peer_control.c

@@ -375,7 +375,7 @@ void channel_errmsg(struct channel *channel,
 		    struct per_peer_state *pps,
 		    const struct channel_id *channel_id UNUSED,
 		    const char *desc,
-		    bool soft_error,
+		    bool warning,
 		    const u8 *err_for_them)
 {
 	notify_disconnect(channel->peer->ld, &channel->peer->id);
@@ -388,13 +388,14 @@ void channel_errmsg(struct channel *channel,
 	}
 
 	/* Do we have an error to send? */
-	if (err_for_them && !channel->error)
+	if (err_for_them && !channel->error && !warning)
 		channel->error = tal_dup_talarr(channel, u8, err_for_them);
 
 	/* Other implementations chose to ignore errors early on.  Not
 	 * surprisingly, they now spew out spurious errors frequently,
-	 * and we would close the channel on them. */
-	if (soft_error) {
+	 * and we would close the channel on them.  We now support warnings
+	 * for this case. */
+	if (warning) {
 		channel_fail_reconnect_later(channel, "%s: (ignoring) %s",
 					     channel->owner->name, desc);
 		return;

lightningd/peer_control.h

@@ -76,7 +76,7 @@ void channel_errmsg(struct channel *channel,
 		    struct per_peer_state *pps,
 		    const struct channel_id *channel_id,
 		    const char *desc,
-		    bool soft_error,
+		    bool warning,
 		    const u8 *err_for_them);
 
 u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx);

lightningd/subd.c

@@ -375,10 +375,10 @@ static bool handle_peer_error(struct subd *sd, const u8 *msg, int fds[3])
 	char *desc;
 	struct per_peer_state *pps;
 	u8 *err_for_them;
-	bool soft_error;
+	bool warning;
 
 	if (!fromwire_status_peer_error(msg, msg,
-					&channel_id, &desc, &soft_error,
+					&channel_id, &desc, &warning,
 					&pps, &err_for_them))
 		return false;
 
@@ -386,7 +386,7 @@ static bool handle_peer_error(struct subd *sd, const u8 *msg, int fds[3])
 
 	/* Don't free sd; we may be about to free channel. */
 	sd->channel = NULL;
-	sd->errcb(channel, pps, &channel_id, desc, soft_error, err_for_them);
+	sd->errcb(channel, pps, &channel_id, desc, warning, err_for_them);
 	return true;
 }
 
@@ -617,7 +617,7 @@ static struct subd *new_subd(struct lightningd *ld,
 					   struct per_peer_state *pps,
 					   const struct channel_id *channel_id,
 					   const char *desc,
-					   bool soft_error,
+					   bool warning,
 					   const u8 *err_for_them),
 			     void (*billboardcb)(void *channel,
 						 bool perm,
@@ -730,7 +730,7 @@ struct subd *new_channel_subd_(struct lightningd *ld,
 					     struct per_peer_state *pps,
 					     const struct channel_id *channel_id,
 					     const char *desc,
-					     bool soft_error,
+					     bool warning,
 					     const u8 *err_for_them),
 			       void (*billboardcb)(void *channel, bool perm,
 						   const char *happenings),
@@ -832,7 +832,7 @@ void subd_swap_channel_(struct subd *daemon, void *channel,
 				      struct per_peer_state *pps,
 				      const struct channel_id *channel_id,
 				      const char *desc,
-				      bool soft_error,
+				      bool warning,
 				      const u8 *err_for_them),
 			void (*billboardcb)(void *channel, bool perm,
 					    const char *happenings))

lightningd/subd.h

@@ -54,7 +54,7 @@ struct subd {
 		      struct per_peer_state *pps,
 		      const struct channel_id *channel_id,
 		      const char *desc,
-		      bool soft_error,
+		      bool warning,
 		      const u8 *err_for_them);
 
 	/* Callback to display information for listpeers RPC */
@@ -133,7 +133,7 @@ struct subd *new_channel_subd_(struct lightningd *ld,
 					     struct per_peer_state *pps,
 					     const struct channel_id *channel_id,
 					     const char *desc,
-					     bool soft_error,
+					     bool warning,
 					     const u8 *err_for_them),
 			       void (*billboardcb)(void *channel, bool perm,
 						   const char *happenings),
@@ -175,7 +175,7 @@ void subd_swap_channel_(struct subd *daemon, void *channel,
 				      struct per_peer_state *pps,
 				      const struct channel_id *channel_id,
 				      const char *desc,
-				      bool soft_error,
+				      bool warning,
 				      const u8 *err_for_them),
 			void (*billboardcb)(void *channel, bool perm,
 					    const char *happenings));

lightningd/test/run-find_my_abspath.c

@@ -88,7 +88,7 @@ bool fromwire_status_fail(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, enu
 bool fromwire_status_peer_billboard(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, bool *perm UNNEEDED, wirestring **happenings UNNEEDED)
 { fprintf(stderr, "fromwire_status_peer_billboard called!\n"); abort(); }
 /* Generated stub for fromwire_status_peer_error */
-bool fromwire_status_peer_error(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct channel_id *channel UNNEEDED, wirestring **desc UNNEEDED, bool *soft_error UNNEEDED, struct per_peer_state **pps UNNEEDED, u8 **error_for_them UNNEEDED)
+bool fromwire_status_peer_error(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct channel_id *channel UNNEEDED, wirestring **desc UNNEEDED, bool *warning UNNEEDED, struct per_peer_state **pps UNNEEDED, u8 **error_for_them UNNEEDED)
 { fprintf(stderr, "fromwire_status_peer_error called!\n"); abort(); }
 /* Generated stub for gossip_init */
 void gossip_init(struct lightningd *ld UNNEEDED, int connectd_fd UNNEEDED)

openingd/dualopend.c

@@ -972,7 +972,7 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state)
 		u8 *msg;
 		bool from_gossipd;
 		char *err;
-		bool all_channels, warning;
+		bool warning;
 		struct channel_id actual;
 
 		/* The event loop is responsible for freeing tmpctx, so our
@@ -1011,7 +1011,7 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state)
 
 		/* A helper which decodes an error. */
 		if (is_peer_error(tmpctx, msg, &state->channel_id,
-				  &err, &all_channels, &warning)) {
+				  &err, &warning)) {
 			/* BOLT #1:
 			 *
 			 *  - if no existing channel is referred to by the
@@ -1024,15 +1024,8 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state)
 				tal_free(msg);
 				continue;
 			}
-			/* Close connection on all_channels error. */
-			if (all_channels) {
-				msg = towire_dualopend_failed(NULL, err);
-				wire_sync_write(REQ_FD, take(msg));
-				peer_failed_received_errmsg(state->pps, err,
-							    NULL, false);
-			}
 			negotiation_aborted(state,
-					    tal_fmt(tmpctx, "They sent error %s",
+					    tal_fmt(tmpctx, "They sent %s",
 						    err));
 			/* Return NULL so caller knows to stop negotiating. */
 			return NULL;

openingd/openingd.c

@@ -199,7 +199,7 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state,
 		u8 *msg;
 		bool from_gossipd;
 		char *err;
-		bool all_channels, warning;
+		bool warning;
 		struct channel_id actual;
 
 		/* The event loop is responsible for freeing tmpctx, so our
@@ -238,7 +238,7 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state,
 
 		/* A helper which decodes an error. */
 		if (is_peer_error(tmpctx, msg, &state->channel_id,
-				  &err, &all_channels, &warning)) {
+				  &err, &warning)) {
 			/* BOLT #1:
 			 *
 			 *  - if no existing channel is referred to by the
@@ -251,16 +251,6 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state,
 				tal_free(msg);
 				continue;
 			}
-			/* Close connection on all_channels error. */
-			if (all_channels) {
-				if (am_opener) {
-					msg = towire_openingd_funder_failed(NULL,
-									   err);
-					wire_sync_write(REQ_FD, take(msg));
-				}
-				peer_failed_received_errmsg(state->pps, err,
-							    NULL, false);
-			}
 			negotiation_aborted(state, am_opener,
 					    tal_fmt(tmpctx, "They sent %s",
 						    err));