From d14e273b04f05d1f3827f31fa62939a62d9c69bf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 2 Feb 2021 23:17:01 +1030 Subject: [PATCH] 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 Changelog-Added: Protocol: we treat error messages from peer which refer to "all channels" as warnings, not errors. --- common/peer_failed.c | 12 ++++++------ common/peer_status_wire.csv | 2 +- common/peer_status_wiregen.c | 10 +++++----- common/peer_status_wiregen.h | 6 +++--- common/read_peer_msg.c | 28 ++++++++++++++------------- common/read_peer_msg.h | 3 +-- lightningd/onchain_control.c | 2 +- lightningd/opening_common.c | 2 +- lightningd/opening_common.h | 2 +- lightningd/peer_control.c | 9 +++++---- lightningd/peer_control.h | 2 +- lightningd/subd.c | 12 ++++++------ lightningd/subd.h | 6 +++--- lightningd/test/run-find_my_abspath.c | 2 +- openingd/dualopend.c | 13 +++---------- openingd/openingd.c | 14 ++------------ 16 files changed, 55 insertions(+), 70 deletions(-) diff --git a/common/peer_failed.c b/common/peer_failed.c index 05c717b69..336fb094b 100644 --- a/common/peer_failed.c +++ b/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); diff --git a/common/peer_status_wire.csv b/common/peer_status_wire.csv index 27b5792f5..8162443e2 100644 --- a/common/peer_status_wire.csv +++ b/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 diff --git a/common/peer_status_wiregen.c b/common/peer_status_wiregen.c index 52055bd72..08982e737 100644 --- a/common/peer_status_wiregen.c +++ b/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 diff --git a/common/peer_status_wiregen.h b/common/peer_status_wiregen.h index 2dce5dc95..a95a6f275 100644 --- a/common/peer_status_wiregen.h +++ b/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 diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index 05eebe8d4..1f6ab83d3 100644 --- a/common/read_peer_msg.c +++ b/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; } diff --git a/common/read_peer_msg.h b/common/read_peer_msg.h index 51919526a..824c2ba3d 100644 --- a/common/read_peer_msg.h +++ b/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 diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 80d83cf2b..bd90e726f 100644 --- a/lightningd/onchain_control.c +++ b/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? */ diff --git a/lightningd/opening_common.c b/lightningd/opening_common.c index 5648e608f..66ec0baef 100644 --- a/lightningd/opening_common.c +++ b/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. */ diff --git a/lightningd/opening_common.h b/lightningd/opening_common.h index 313c6b6c3..2bc7424a4 100644 --- a/lightningd/opening_common.h +++ b/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, diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index a819c6b54..70b5e7d64 100644 --- a/lightningd/peer_control.c +++ b/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; diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index ffdeeb599..cf8843274 100644 --- a/lightningd/peer_control.h +++ b/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); diff --git a/lightningd/subd.c b/lightningd/subd.c index 02a8cd830..48095ef34 100644 --- a/lightningd/subd.c +++ b/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)) diff --git a/lightningd/subd.h b/lightningd/subd.h index f0cd09c0c..efb84fe3c 100644 --- a/lightningd/subd.h +++ b/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)); diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index 13a64342c..245b5cb18 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/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) diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 9bb7a6551..1a7a66023 100644 --- a/openingd/dualopend.c +++ b/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; diff --git a/openingd/openingd.c b/openingd/openingd.c index 9ad57a86d..b672c9ea6 100644 --- a/openingd/openingd.c +++ b/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));