From 82f22b2e76ca07011b82bde0bad2d85fd60fade3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 2 Feb 2021 23:15:01 +1030 Subject: [PATCH 1/7] pytest: fix false bad gossip issue in test_forward We construct the route manually so we may not have the channel_announcement yet. But we can get an update from the error packet, which can lead to: ``` 2021-01-29T01:38:23.4767334Z ValueError: 2021-01-29T01:38:23.4767987Z Node errors: 2021-01-29T01:38:23.4768767Z - lightningd-1: had bad gossip messages 2021-01-29T01:38:23.4769512Z Global errors: 2021-01-29T01:38:23.4770300Z 2021-01-29T01:38:23.4771109Z contrib/pyln-testing/pyln/testing/fixtures.py:197: ValueError ... 2021-01-29T01:38:23.7820197Z lightningd-1: 2021-01-29T01:26:57.460Z DEBUG gossipd: Extracted channel_update 01027217b3086ad9f3dee1fa55b94c5fd2a4b0637bec70ba727ba4151a8de5173ddc749db3502d41ab0ae164addc8fd013d2088b6a12a2f478ae0affa94d76d8845c06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f000067000001000160136459010000060000000000000000000000010000000a000000003b023380 from onionreply 100d0000007500887217b3086ad9f3dee1fa55b94c5fd2a4b0637bec70ba727ba4151a8de5173ddc749db3502d41ab0ae164addc8fd013d2088b6a12a2f478ae0affa94d76d8845c06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f000067000001000160136459010000060000000000000000000000010000000a000000003b023380 2021-01-29T01:38:23.7837450Z lightningd-1: 2021-01-29T01:26:57.461Z DEBUG gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 103x1x1/0 ``` Signed-off-by: Rusty Russell --- tests/test_pay.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/test_pay.py b/tests/test_pay.py index 6a81cbb7ed9d..f6a5a11bbfdd 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -1011,10 +1011,7 @@ def test_decodepay(node_factory): @unittest.skipIf(not DEVELOPER, "Too slow without --dev-fast-gossip") def test_forward(node_factory, bitcoind): # Connect 1 -> 2 -> 3. - l1, l2, l3 = node_factory.line_graph(3, fundchannel=True) - - # Allow announce messages. - l1.bitcoin.generate_block(5) + l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True) # If they're at different block heights we can get spurious errors. sync_blockheight(bitcoind, [l1, l2, l3]) From 5d011d34696c5e4ce993754dbc75720a6cb01ffa Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 2 Feb 2021 23:16:01 +1030 Subject: [PATCH 2/7] lightningd: implement receiving warnings. This takes from the draft spec at https://github.com/lightningnetwork/lightning-rfc/pull/834 Note that if this draft does not get included, the peer will simply ignore the warning message (we always close the connection afterwards anyway). Signed-off-by: Rusty Russell Changelog-Added: Protocol: we now report the new (draft) warning message. --- channeld/channeld.c | 1 + common/peer_failed.c | 4 ++-- common/read_peer_msg.c | 16 +++++++++++----- common/read_peer_msg.h | 3 ++- common/wire_error.c | 16 +++++++++++----- common/wire_error.h | 4 ++-- gossipd/gossipd.c | 1 + openingd/dualopend.c | 5 +++-- openingd/openingd.c | 6 +++--- wire/extracted_peer_warning.patch | 13 +++++++++++++ wire/peer_wire.c | 2 ++ wire/peer_wire.csv | 4 ++++ 12 files changed, 55 insertions(+), 20 deletions(-) create mode 100644 wire/extracted_peer_warning.patch diff --git a/channeld/channeld.c b/channeld/channeld.c index 554919fa254e..add6ca68745b 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1928,6 +1928,7 @@ static void peer_in(struct peer *peer, const u8 *msg) case WIRE_REPLY_SHORT_CHANNEL_IDS_END: case WIRE_PING: case WIRE_PONG: + case WIRE_WARNING: case WIRE_ERROR: case WIRE_ONION_MESSAGE: abort(); diff --git a/common/peer_failed.c b/common/peer_failed.c index 7206f325dcb0..05c717b69b1a 100644 --- a/common/peer_failed.c +++ b/common/peer_failed.c @@ -49,7 +49,7 @@ void peer_failed(struct per_peer_state *pps, peer_fatal_continue(take(msg), pps); } -/* We're failing because peer sent us an error message */ +/* We're failing because peer sent us an error/warning message */ void peer_failed_received_errmsg(struct per_peer_state *pps, const char *desc, const struct channel_id *channel_id, @@ -62,7 +62,7 @@ void peer_failed_received_errmsg(struct per_peer_state *pps, channel_id = &all_channels; msg = towire_status_peer_error(NULL, channel_id, desc, soft_error, pps, NULL); - peer_billboard(true, "Received error from peer: %s", desc); + peer_billboard(true, "Received %s", desc); peer_fatal_continue(take(msg), pps); } diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index 44794dd20666..fb561f9067ca 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -68,11 +68,16 @@ 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) + char **desc, bool *all_channels, + bool *warning) { struct channel_id err_chanid; - if (fromwire_peektype(msg) != WIRE_ERROR) + if (fromwire_peektype(msg) == WIRE_ERROR) + *warning = false; + else if (fromwire_peektype(msg) == WIRE_WARNING) + *warning = true; + else return false; *desc = sanitize_error(ctx, msg, &err_chanid); @@ -153,7 +158,7 @@ bool handle_peer_gossip_or_error(struct per_peer_state *pps, const u8 *msg TAKES) { char *err; - bool all_channels; + bool all_channels, warning; struct channel_id actual; #if DEVELOPER @@ -180,12 +185,13 @@ bool handle_peer_gossip_or_error(struct per_peer_state *pps, return true; } - if (is_peer_error(tmpctx, msg, channel_id, &err, &all_channels)) { + 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, - soft_error); + warning || soft_error); /* Ignore unknown channel errors. */ goto handled; diff --git a/common/read_peer_msg.h b/common/read_peer_msg.h index ae6a575f3374..51919526a20c 100644 --- a/common/read_peer_msg.h +++ b/common/read_peer_msg.h @@ -34,13 +34,14 @@ u8 *peer_or_gossip_sync_read(const tal_t *ctx, * @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 * to peer_failed_received_errmsg(). */ bool is_peer_error(const tal_t *ctx, const u8 *msg, const struct channel_id *channel_id, - char **desc, bool *all_channels); + char **desc, bool *all_channels, bool *warning); /** * is_wrong_channel - if it's a message about a different channel, return true diff --git a/common/wire_error.c b/common/wire_error.c index 67f6b773f295..7bd1c5d4f7d8 100644 --- a/common/wire_error.c +++ b/common/wire_error.c @@ -53,11 +53,16 @@ char *sanitize_error(const tal_t *ctx, const u8 *errmsg, struct channel_id dummy; u8 *data; size_t i; + bool warning; if (!channel_id) channel_id = &dummy; - if (!fromwire_error(ctx, errmsg, channel_id, &data)) + if (fromwire_error(ctx, errmsg, channel_id, &data)) + warning = false; + else if (fromwire_warning(ctx, errmsg, channel_id, &data)) + warning = true; + else return tal_fmt(ctx, "Invalid ERROR message '%s'", tal_hex(ctx, errmsg)); @@ -79,9 +84,10 @@ char *sanitize_error(const tal_t *ctx, const u8 *errmsg, } } - return tal_fmt(ctx, "channel %s: %.*s", - channel_id_is_all(channel_id) - ? "ALL" - : type_to_string(ctx, struct channel_id, channel_id), + return tal_fmt(ctx, "%s%s%s: %.*s", + warning ? "warning" : "error", + channel_id_is_all(channel_id) ? "": " channel ", + channel_id_is_all(channel_id) ? "" + : type_to_string(tmpctx, struct channel_id, channel_id), (int)tal_count(data), (char *)data); } diff --git a/common/wire_error.h b/common/wire_error.h index bf9ffa09bc09..fdccff3f11cb 100644 --- a/common/wire_error.h +++ b/common/wire_error.h @@ -43,10 +43,10 @@ u8 *towire_errorfmtv(const tal_t *ctx, bool channel_id_is_all(const struct channel_id *channel_id); /** - * sanitize_error - extract and sanitize contents of WIRE_ERROR. + * sanitize_error - extract and sanitize contents of WIRE_ERROR/WIRE_WARNING. * * @ctx: context to allocate from - * @errmsg: the wire_error + * @errmsg: the wire_error or wire_warning * @channel: (out) channel it's referring to, or NULL if don't care. */ char *sanitize_error(const tal_t *ctx, const u8 *errmsg, diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 5f2e499dcaa7..6bfa337808a5 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -737,6 +737,7 @@ static struct io_plan *peer_msg_in(struct io_conn *conn, goto handled_relay; /* These are non-gossip messages (!is_msg_for_gossipd()) */ + case WIRE_WARNING: case WIRE_INIT: case WIRE_ERROR: case WIRE_OPEN_CHANNEL: diff --git a/openingd/dualopend.c b/openingd/dualopend.c index c5cc84971cf0..9bb7a655113b 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; + bool all_channels, 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)) { + &err, &all_channels, &warning)) { /* BOLT #1: * * - if no existing channel is referred to by the @@ -1355,6 +1355,7 @@ static bool run_tx_interactive(struct state *state, break; case WIRE_INIT: case WIRE_ERROR: + case WIRE_WARNING: case WIRE_OPEN_CHANNEL: case WIRE_ACCEPT_CHANNEL: case WIRE_FUNDING_CREATED: diff --git a/openingd/openingd.c b/openingd/openingd.c index fc1066089382..9ad57a86d86c 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; + bool all_channels, 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)) { + &err, &all_channels, &warning)) { /* BOLT #1: * * - if no existing channel is referred to by the @@ -262,7 +262,7 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state, NULL, false); } negotiation_aborted(state, am_opener, - 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/wire/extracted_peer_warning.patch b/wire/extracted_peer_warning.patch new file mode 100644 index 000000000000..6dc9e7c5bdac --- /dev/null +++ b/wire/extracted_peer_warning.patch @@ -0,0 +1,13 @@ +--- wire/peer_exp_wire.csv 2021-01-14 11:00:27.526336550 +1030 ++++ - 2021-01-21 15:31:37.071118999 +1030 +@@ -10,6 +10,10 @@ + msgdata,error,channel_id,channel_id, + msgdata,error,len,u16, + msgdata,error,data,byte,len ++msgtype,warning,1 ++msgdata,warning,channel_id,channel_id, ++msgdata,warning,len,u16, ++msgdata,warning,data,byte,len + msgtype,ping,18 + msgdata,ping,num_pong_bytes,u16, + msgdata,ping,byteslen,u16, diff --git a/wire/peer_wire.c b/wire/peer_wire.c index 2581d3ead241..82bd3009b82c 100644 --- a/wire/peer_wire.c +++ b/wire/peer_wire.c @@ -4,6 +4,7 @@ static bool unknown_type(enum peer_wire t) { switch (t) { + case WIRE_WARNING: case WIRE_INIT: case WIRE_ERROR: case WIRE_OPEN_CHANNEL: @@ -64,6 +65,7 @@ bool is_msg_for_gossipd(const u8 *cursor) case WIRE_PONG: case WIRE_ONION_MESSAGE: return true; + case WIRE_WARNING: case WIRE_INIT: case WIRE_ERROR: case WIRE_OPEN_CHANNEL: diff --git a/wire/peer_wire.csv b/wire/peer_wire.csv index 01061d9fc555..4ced58c8015b 100644 --- a/wire/peer_wire.csv +++ b/wire/peer_wire.csv @@ -10,6 +10,10 @@ msgtype,error,17 msgdata,error,channel_id,channel_id, msgdata,error,len,u16, msgdata,error,data,byte,len +msgtype,warning,1 +msgdata,warning,channel_id,channel_id, +msgdata,warning,len,u16, +msgdata,warning,data,byte,len msgtype,ping,18 msgdata,ping,num_pong_bytes,u16, msgdata,ping,byteslen,u16, From 0f6eed20c1ea0b6f728fc806903d79afa9614096 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 2 Feb 2021 23:17:01 +1030 Subject: [PATCH 3/7] 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 05c717b69b1a..336fb094bbef 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 27b5792f5d6c..8162443e2ffb 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 52055bd72839..08982e737f80 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 2dce5dc953f1..a95a6f27524e 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 fb561f9067ca..aaf5773445c2 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -68,8 +68,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; @@ -94,8 +93,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; @@ -158,7 +160,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 @@ -185,15 +187,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 51919526a20c..824c2ba3d518 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 80d83cf2b5db..bd90e726f55a 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 5648e608fec9..66ec0baefb1d 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 313c6b6c3b53..2bc7424a4f1e 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 a819c6b54741..70b5e7d648a5 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 ffdeeb5993b8..cf8843274663 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 02a8cd830b68..48095ef34779 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 f0cd09c0c841..efb84fe3c6ec 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 13a64342c604..245b5cb18fe9 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 9bb7a655113b..1a7a660231c9 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 9ad57a86d86c..b672c9ea6878 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)); From 24336e0601112c215bfaaf3f4e192d3d25975d0a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 2 Feb 2021 23:18:01 +1030 Subject: [PATCH 4/7] common: infrastructure to construct warning messages. Signed-off-by: Rusty Russell --- common/wire_error.c | 37 +++++++++++++++++++++++++++++++++++++ common/wire_error.h | 24 ++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/common/wire_error.c b/common/wire_error.c index 7bd1c5d4f7d8..bff0c0cb76f8 100644 --- a/common/wire_error.c +++ b/common/wire_error.c @@ -42,6 +42,43 @@ u8 *towire_errorfmt(const tal_t *ctx, return msg; } +u8 *towire_warningfmtv(const tal_t *ctx, + const struct channel_id *channel, + const char *fmt, + va_list ap) +{ + /* BOLT #1: + * + * The channel is referred to by `channel_id`, unless `channel_id` is + * 0 (i.e. all bytes are 0), in which case it refers to all + * channels. */ + static const struct channel_id all_channels; + char *estr; + u8 *msg; + + estr = tal_vfmt(ctx, fmt, ap); + /* We need tal_len to work, so we use copy. */ + msg = towire_warning(ctx, channel ? channel : &all_channels, + (u8 *)tal_dup_arr(estr, char, estr, strlen(estr), 0)); + tal_free(estr); + + return msg; +} + +u8 *towire_warningfmt(const tal_t *ctx, + const struct channel_id *channel, + const char *fmt, ...) +{ + va_list ap; + u8 *msg; + + va_start(ap, fmt); + msg = towire_warningfmtv(ctx, channel, fmt, ap); + va_end(ap); + + return msg; +} + bool channel_id_is_all(const struct channel_id *channel_id) { return memeqzero(channel_id, sizeof(*channel_id)); diff --git a/common/wire_error.h b/common/wire_error.h index fdccff3f11cb..6334956ed093 100644 --- a/common/wire_error.h +++ b/common/wire_error.h @@ -32,6 +32,30 @@ u8 *towire_errorfmtv(const tal_t *ctx, const char *fmt, va_list ap); +/** + * towire_warningfmt - helper to turn string into WIRE_WARNING. + * + * @ctx: context to allocate from + * @channel: specific channel to complain about, or NULL for all. + * @fmt: format for warning. + */ +u8 *towire_warningfmt(const tal_t *ctx, + const struct channel_id *channel, + const char *fmt, ...) PRINTF_FMT(3,4); + +/** + * towire_warningfmtv - helper to turn string into WIRE_WARNING. + * + * @ctx: context to allocate from + * @channel: specific channel to complain about, or NULL for all. + * @fmt: format for warning. + * @ap: accumulated varargs. + */ +u8 *towire_warningfmtv(const tal_t *ctx, + const struct channel_id *channel, + const char *fmt, + va_list ap); + /* BOLT #1: * * The channel is referred to by `channel_id`, unless `channel_id` is 0 From 467eb295ce35e5e347c4256dbbc2c0f9815cb5a3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 2 Feb 2021 23:19:01 +1030 Subject: [PATCH 5/7] common: remove peer_failed in favor of peer_failed_warn/peer_failed_err And make all the callers choose which one. In general, I prefer warn, which lets them reconnect and try again, however some places are either stated that they must be errors in the spec itself, or in openingd where we abandon the channel when we close the connection anyway. Signed-off-by: Rusty Russell Changelog-Changed: Protocol: we now send warning messages and close the connection, except on unrecoverable errors. --- channeld/channeld.c | 433 ++++++++++++++++++--------------------- closingd/closingd.c | 100 ++++----- common/peer_failed.c | 60 ++++-- common/peer_failed.h | 21 +- openingd/dualopend.c | 407 ++++++++++++++++++------------------ openingd/openingd.c | 107 +++++----- tests/test_connection.py | 14 +- tests/test_misc.py | 9 +- tests/test_pay.py | 3 +- 9 files changed, 581 insertions(+), 573 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index add6ca68745b..c8a29c0ce372 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -445,14 +445,13 @@ static void check_short_ids_match(struct peer *peer) if (!short_channel_id_eq(&peer->short_channel_ids[LOCAL], &peer->short_channel_ids[REMOTE])) - peer_failed(peer->pps, - &peer->channel_id, - "We disagree on short_channel_ids:" - " I have %s, you say %s", - type_to_string(peer, struct short_channel_id, - &peer->short_channel_ids[LOCAL]), - type_to_string(peer, struct short_channel_id, - &peer->short_channel_ids[REMOTE])); + peer_failed_warn(peer->pps, &peer->channel_id, + "We disagree on short_channel_ids:" + " I have %s, you say %s", + type_to_string(peer, struct short_channel_id, + &peer->short_channel_ids[LOCAL]), + type_to_string(peer, struct short_channel_id, + &peer->short_channel_ids[REMOTE])); } static void announce_channel(struct peer *peer) @@ -550,17 +549,15 @@ static void handle_peer_funding_locked(struct peer *peer, const u8 *msg) peer->old_remote_per_commit = peer->remote_per_commit; if (!fromwire_funding_locked(msg, &chanid, &peer->remote_per_commit)) - peer_failed(peer->pps, - &peer->channel_id, - "Bad funding_locked %s", tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad funding_locked %s", tal_hex(msg, msg)); if (!channel_id_eq(&chanid, &peer->channel_id)) - peer_failed(peer->pps, - &peer->channel_id, - "Wrong channel id in %s (expected %s)", - tal_hex(tmpctx, msg), - type_to_string(msg, struct channel_id, - &peer->channel_id)); + peer_failed_err(peer->pps, &chanid, + "Wrong channel id in %s (expected %s)", + tal_hex(tmpctx, msg), + type_to_string(msg, struct channel_id, + &peer->channel_id)); peer->tx_sigs_allowed = false; peer->funding_locked[REMOTE] = true; @@ -581,19 +578,17 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg &peer->short_channel_ids[REMOTE], &peer->announcement_node_sigs[REMOTE], &peer->announcement_bitcoin_sigs[REMOTE])) - peer_failed(peer->pps, - &peer->channel_id, - "Bad announcement_signatures %s", - tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad announcement_signatures %s", + tal_hex(msg, msg)); /* Make sure we agree on the channel ids */ if (!channel_id_eq(&chanid, &peer->channel_id)) { - peer_failed(peer->pps, - &peer->channel_id, - "Wrong channel_id: expected %s, got %s", - type_to_string(tmpctx, struct channel_id, - &peer->channel_id), - type_to_string(tmpctx, struct channel_id, &chanid)); + peer_failed_err(peer->pps, &chanid, + "Wrong channel_id: expected %s, got %s", + type_to_string(tmpctx, struct channel_id, + &peer->channel_id), + type_to_string(tmpctx, struct channel_id, &chanid)); } peer->have_sigs[REMOTE] = true; @@ -624,9 +619,8 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg) , tlvs #endif )) - peer_failed(peer->pps, - &peer->channel_id, - "Bad peer_add_htlc %s", tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad peer_add_htlc %s", tal_hex(msg, msg)); #if EXPERIMENTAL_FEATURES blinding = tlvs->blinding; @@ -635,10 +629,9 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg) cltv_expiry, &payment_hash, onion_routing_packet, blinding, &htlc, NULL); if (add_err != CHANNEL_ERR_ADD_OK) - peer_failed(peer->pps, - &peer->channel_id, - "Bad peer_add_htlc: %s", - channel_add_err_name(add_err)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad peer_add_htlc: %s", + channel_add_err_name(add_err)); } static void handle_peer_feechange(struct peer *peer, const u8 *msg) @@ -647,9 +640,8 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) u32 feerate; if (!fromwire_update_fee(msg, &channel_id, &feerate)) { - peer_failed(peer->pps, - &peer->channel_id, - "Bad update_fee %s", tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad update_fee %s", tal_hex(msg, msg)); } /* BOLT #2: @@ -660,9 +652,8 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) * - MUST fail the channel. */ if (peer->channel->opener != REMOTE) - peer_failed(peer->pps, - &peer->channel_id, - "update_fee from non-opener?"); + peer_failed_warn(peer->pps, &peer->channel_id, + "update_fee from non-opener?"); status_debug("update_fee %u, range %u-%u", feerate, peer->feerate_min, peer->feerate_max); @@ -675,10 +666,9 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) * - SHOULD fail the channel. */ if (feerate < peer->feerate_min || feerate > peer->feerate_max) - peer_failed(peer->pps, - &peer->channel_id, - "update_fee %u outside range %u-%u", - feerate, peer->feerate_min, peer->feerate_max); + peer_failed_warn(peer->pps, &peer->channel_id, + "update_fee %u outside range %u-%u", + feerate, peer->feerate_min, peer->feerate_max); /* BOLT #2: * @@ -688,10 +678,9 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) * - but MAY delay this check until the `update_fee` is committed. */ if (!channel_update_feerate(peer->channel, feerate)) - peer_failed(peer->pps, - &peer->channel_id, - "update_fee %u unaffordable", - feerate); + peer_failed_warn(peer->pps, &peer->channel_id, + "update_fee %u unaffordable", + feerate); status_debug("peer updated fee to %u", feerate); } @@ -1275,9 +1264,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) status_debug("Oh hi LND! Empty commitment at #%"PRIu64, peer->next_index[LOCAL]); if (peer->last_empty_commitment == peer->next_index[LOCAL] - 1) - peer_failed(peer->pps, - &peer->channel_id, - "commit_sig with no changes (again!)"); + peer_failed_warn(peer->pps, &peer->channel_id, + "commit_sig with no changes (again!)"); peer->last_empty_commitment = peer->next_index[LOCAL]; } @@ -1293,9 +1281,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) if (!fromwire_commitment_signed(tmpctx, msg, &channel_id, &commit_sig.s, &raw_sigs)) - peer_failed(peer->pps, - &peer->channel_id, - "Bad commit_sig %s", tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad commit_sig %s", tal_hex(msg, msg)); /* SIGHASH_ALL is implied. */ commit_sig.sighash_type = SIGHASH_ALL; htlc_sigs = unraw_sigs(tmpctx, raw_sigs, @@ -1333,18 +1320,17 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) if (!check_tx_sig(txs[0], 0, NULL, funding_wscript, &peer->channel->funding_pubkey[REMOTE], &commit_sig)) { dump_htlcs(peer->channel, "receiving commit_sig"); - peer_failed(peer->pps, - &peer->channel_id, - "Bad commit_sig signature %"PRIu64" %s for tx %s wscript %s key %s feerate %u", - peer->next_index[LOCAL], - type_to_string(msg, struct bitcoin_signature, - &commit_sig), - type_to_string(msg, struct bitcoin_tx, txs[0]), - tal_hex(msg, funding_wscript), - type_to_string(msg, struct pubkey, - &peer->channel->funding_pubkey - [REMOTE]), - channel_feerate(peer->channel, LOCAL)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad commit_sig signature %"PRIu64" %s for tx %s wscript %s key %s feerate %u", + peer->next_index[LOCAL], + type_to_string(msg, struct bitcoin_signature, + &commit_sig), + type_to_string(msg, struct bitcoin_tx, txs[0]), + tal_hex(msg, funding_wscript), + type_to_string(msg, struct pubkey, + &peer->channel->funding_pubkey + [REMOTE]), + channel_feerate(peer->channel, LOCAL)); } /* BOLT #2: @@ -1356,10 +1342,9 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) * - MUST fail the channel. */ if (tal_count(htlc_sigs) != tal_count(txs) - 1) - peer_failed(peer->pps, - &peer->channel_id, - "Expected %zu htlc sigs, not %zu", - tal_count(txs) - 1, tal_count(htlc_sigs)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Expected %zu htlc sigs, not %zu", + tal_count(txs) - 1, tal_count(htlc_sigs)); /* BOLT #2: * @@ -1375,14 +1360,13 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) if (!check_tx_sig(txs[1+i], 0, NULL, wscript, &remote_htlckey, &htlc_sigs[i])) - peer_failed(peer->pps, - &peer->channel_id, - "Bad commit_sig signature %s for htlc %s wscript %s key %s", - type_to_string(msg, struct bitcoin_signature, &htlc_sigs[i]), - type_to_string(msg, struct bitcoin_tx, txs[1+i]), - tal_hex(msg, wscript), - type_to_string(msg, struct pubkey, - &remote_htlckey)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad commit_sig signature %s for htlc %s wscript %s key %s", + type_to_string(msg, struct bitcoin_signature, &htlc_sigs[i]), + type_to_string(msg, struct bitcoin_tx, txs[1+i]), + tal_hex(msg, wscript), + type_to_string(msg, struct pubkey, + &remote_htlckey)); } status_debug("Received commit_sig with %zu htlc sigs", @@ -1460,15 +1444,13 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) if (!fromwire_revoke_and_ack(msg, &channel_id, &old_commit_secret, &next_per_commit)) { - peer_failed(peer->pps, - &peer->channel_id, - "Bad revoke_and_ack %s", tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad revoke_and_ack %s", tal_hex(msg, msg)); } if (peer->revocations_received != peer->next_index[REMOTE] - 2) { - peer_failed(peer->pps, - &peer->channel_id, - "Unexpected revoke_and_ack"); + peer_failed_warn(peer->pps, &peer->channel_id, + "Unexpected revoke_and_ack"); } /* BOLT #2: @@ -1480,19 +1462,17 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) */ memcpy(&privkey, &old_commit_secret, sizeof(privkey)); if (!pubkey_from_privkey(&privkey, &per_commit_point)) { - peer_failed(peer->pps, - &peer->channel_id, - "Bad privkey %s", - type_to_string(msg, struct privkey, &privkey)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad privkey %s", + type_to_string(msg, struct privkey, &privkey)); } if (!pubkey_eq(&per_commit_point, &peer->old_remote_per_commit)) { - peer_failed(peer->pps, - &peer->channel_id, - "Wrong privkey %s for %"PRIu64" %s", - type_to_string(msg, struct privkey, &privkey), - peer->next_index[LOCAL]-2, - type_to_string(msg, struct pubkey, - &peer->old_remote_per_commit)); + peer_failed_err(peer->pps, &peer->channel_id, + "Wrong privkey %s for %"PRIu64" %s", + type_to_string(msg, struct privkey, &privkey), + peer->next_index[LOCAL]-2, + type_to_string(msg, struct pubkey, + &peer->old_remote_per_commit)); } /* We start timer even if this returns false: we might have delayed @@ -1532,9 +1512,8 @@ static void handle_peer_fulfill_htlc(struct peer *peer, const u8 *msg) if (!fromwire_update_fulfill_htlc(msg, &channel_id, &id, &preimage)) { - peer_failed(peer->pps, - &peer->channel_id, - "Bad update_fulfill_htlc %s", tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad update_fulfill_htlc %s", tal_hex(msg, msg)); } e = channel_fulfill_htlc(peer->channel, LOCAL, id, &preimage, &h); @@ -1551,10 +1530,9 @@ static void handle_peer_fulfill_htlc(struct peer *peer, const u8 *msg) case CHANNEL_ERR_HTLC_UNCOMMITTED: case CHANNEL_ERR_HTLC_NOT_IRREVOCABLE: case CHANNEL_ERR_BAD_PREIMAGE: - peer_failed(peer->pps, - &peer->channel_id, - "Bad update_fulfill_htlc: failed to fulfill %" - PRIu64 " error %s", id, channel_remove_err_name(e)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad update_fulfill_htlc: failed to fulfill %" + PRIu64 " error %s", id, channel_remove_err_name(e)); } abort(); } @@ -1571,9 +1549,8 @@ static void handle_peer_fail_htlc(struct peer *peer, const u8 *msg) /* reason is not an onionreply because spec doesn't know about that */ if (!fromwire_update_fail_htlc(msg, msg, &channel_id, &id, &reason)) { - peer_failed(peer->pps, - &peer->channel_id, - "Bad update_fail_htlc %s", tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad update_fail_htlc %s", tal_hex(msg, msg)); } e = channel_fail_htlc(peer->channel, LOCAL, id, &htlc); @@ -1591,11 +1568,10 @@ static void handle_peer_fail_htlc(struct peer *peer, const u8 *msg) case CHANNEL_ERR_HTLC_UNCOMMITTED: case CHANNEL_ERR_HTLC_NOT_IRREVOCABLE: case CHANNEL_ERR_BAD_PREIMAGE: - peer_failed(peer->pps, - &peer->channel_id, - "Bad update_fail_htlc: failed to remove %" - PRIu64 " error %s", id, - channel_remove_err_name(e)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad update_fail_htlc: failed to remove %" + PRIu64 " error %s", id, + channel_remove_err_name(e)); } abort(); } @@ -1613,10 +1589,9 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) if (!fromwire_update_fail_malformed_htlc(msg, &channel_id, &id, &sha256_of_onion, &failure_code)) { - peer_failed(peer->pps, - &peer->channel_id, - "Bad update_fail_malformed_htlc %s", - tal_hex(msg, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad update_fail_malformed_htlc %s", + tal_hex(msg, msg)); } /* BOLT #2: @@ -1626,10 +1601,9 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) * - MUST fail the channel. */ if (!(failure_code & BADONION)) { - peer_failed(peer->pps, - &peer->channel_id, - "Bad update_fail_malformed_htlc failure code %u", - failure_code); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad update_fail_malformed_htlc failure code %u", + failure_code); } e = channel_fail_htlc(peer->channel, LOCAL, id, &htlc); @@ -1647,10 +1621,9 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) case CHANNEL_ERR_HTLC_UNCOMMITTED: case CHANNEL_ERR_HTLC_NOT_IRREVOCABLE: case CHANNEL_ERR_BAD_PREIMAGE: - peer_failed(peer->pps, - &peer->channel_id, - "Bad update_fail_malformed_htlc: failed to remove %" - PRIu64 " error %s", id, channel_remove_err_name(e)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad update_fail_malformed_htlc: failed to remove %" + PRIu64 " error %s", id, channel_remove_err_name(e)); } abort(); } @@ -1664,9 +1637,8 @@ static void handle_peer_shutdown(struct peer *peer, const u8 *shutdown) send_channel_update(peer, ROUTING_FLAGS_DISABLED); if (!fromwire_shutdown(tmpctx, shutdown, &channel_id, &scriptpubkey)) - peer_failed(peer->pps, - &peer->channel_id, - "Bad shutdown %s", tal_hex(peer, shutdown)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad shutdown %s", tal_hex(peer, shutdown)); /* BOLT #2: * @@ -1681,9 +1653,8 @@ static void handle_peer_shutdown(struct peer *peer, const u8 *shutdown) && !memeq(scriptpubkey, tal_count(scriptpubkey), peer->remote_upfront_shutdown_script, tal_count(peer->remote_upfront_shutdown_script))) - peer_failed(peer->pps, - &peer->channel_id, - "scriptpubkey %s is not as agreed upfront (%s)", + peer_failed_err(peer->pps, &peer->channel_id, + "scriptpubkey %s is not as agreed upfront (%s)", tal_hex(peer, scriptpubkey), tal_hex(peer, peer->remote_upfront_shutdown_script)); @@ -1741,8 +1712,7 @@ static void handle_unexpected_tx_sigs(struct peer *peer, const u8 *msg) * but they did not receive our funding_locked. */ if (!fromwire_tx_signatures(tmpctx, msg, &cid, &txid, cast_const3(struct witness_stack ***, &ws))) - peer_failed(peer->pps, - &peer->channel_id, + peer_failed_warn(peer->pps, &peer->channel_id, "Bad tx_signatures %s", tal_hex(msg, msg)); @@ -1750,8 +1720,8 @@ static void handle_unexpected_tx_sigs(struct peer *peer, const u8 *msg) peer->tx_sigs_allowed ? "Allowing." : "Failing."); if (!peer->tx_sigs_allowed) - peer_failed(peer->pps, &peer->channel_id, - "Unexpected `tx_signatures`"); + peer_failed_warn(peer->pps, &peer->channel_id, + "Unexpected `tx_signatures`"); peer->tx_sigs_allowed = false; } @@ -1770,9 +1740,8 @@ static void handle_unexpected_reestablish(struct peer *peer, const u8 *msg) &next_revocation_number, &your_last_per_commitment_secret, &my_current_per_commitment_point)) - peer_failed(peer->pps, - &peer->channel_id, - "Bad channel_reestablish %s", tal_hex(peer, msg)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad channel_reestablish %s", tal_hex(peer, msg)); /* Is it the same as the peer channel ID? */ if (channel_id_eq(&channel_id, &peer->channel_id)) { @@ -1803,12 +1772,12 @@ static void handle_unexpected_reestablish(struct peer *peer, const u8 *msg) * peer getting its wires crossed somewhere. * Fail the channel they sent, not the channel we are actively * handling. */ - peer_failed(peer->pps, &channel_id, - "Peer sent unexpected message %u, (%s) " - "for nonexistent channel %s", - WIRE_CHANNEL_REESTABLISH, "WIRE_CHANNEL_REESTABLISH", - type_to_string(tmpctx, struct channel_id, - &channel_id)); + peer_failed_err(peer->pps, &channel_id, + "Peer sent unexpected message %u, (%s) " + "for nonexistent channel %s", + WIRE_CHANNEL_REESTABLISH, "WIRE_CHANNEL_REESTABLISH", + type_to_string(tmpctx, struct channel_id, + &channel_id)); } static void peer_in(struct peer *peer, const u8 *msg) @@ -1848,10 +1817,9 @@ static void peer_in(struct peer *peer, const u8 *msg) /* lnd sends these early; it's harmless. */ && type != WIRE_UPDATE_FEE && type != WIRE_ANNOUNCEMENT_SIGNATURES) { - peer_failed(peer->pps, - &peer->channel_id, - "%s (%u) before funding locked", - peer_wire_name(type), type); + peer_failed_warn(peer->pps, &peer->channel_id, + "%s (%u) before funding locked", + peer_wire_name(type), type); } } @@ -1934,10 +1902,9 @@ static void peer_in(struct peer *peer, const u8 *msg) abort(); } - peer_failed(peer->pps, - &peer->channel_id, - "Peer sent unknown message %u (%s)", - type, peer_wire_name(type)); + peer_failed_warn(peer->pps, &peer->channel_id, + "Peer sent unknown message %u (%s)", + type, peer_wire_name(type)); } static void resend_revoke(struct peer *peer) @@ -1968,10 +1935,9 @@ static void send_fail_or_fulfill(struct peer *peer, const struct htlc *h) msg = towire_update_fulfill_htlc(NULL, &peer->channel_id, h->id, h->r); } else - peer_failed(peer->pps, - &peer->channel_id, - "HTLC %"PRIu64" state %s not failed/fulfilled", - h->id, htlc_state_name(h->state)); + peer_failed_warn(peer->pps, &peer->channel_id, + "HTLC %"PRIu64" state %s not failed/fulfilled", + h->id, htlc_state_name(h->state)); sync_crypto_write(peer->pps, take(msg)); } @@ -2029,10 +1995,9 @@ static void resend_commitment(struct peer *peer, struct changed_htlc *last) /* I think this can happen if we actually received revoke_and_ack * then they asked for a retransmit */ if (!h) - peer_failed(peer->pps, - &peer->channel_id, - "Can't find HTLC %"PRIu64" to resend", - last[i].id); + peer_failed_warn(peer->pps, &peer->channel_id, + "Can't find HTLC %"PRIu64" to resend", + last[i].id); if (h->state == SENT_REMOVE_COMMIT) send_fail_or_fulfill(peer, h); @@ -2049,10 +2014,9 @@ static void resend_commitment(struct peer *peer, struct changed_htlc *last) /* I think this can happen if we actually received revoke_and_ack * then they asked for a retransmit */ if (!h) - peer_failed(peer->pps, - &peer->channel_id, - "Can't find HTLC %"PRIu64" to resend", - last[i].id); + peer_failed_warn(peer->pps, &peer->channel_id, + "Can't find HTLC %"PRIu64" to resend", + last[i].id); if (h->state == SENT_ADD_COMMIT) { #if EXPERIMENTAL_FEATURES @@ -2140,12 +2104,12 @@ static void check_future_dataloss_fields(struct peer *peer, tal_hex(tmpctx, msg)); if (!correct) - peer_failed(peer->pps, - &peer->channel_id, - "bad future last_local_per_commit_secret: %"PRIu64 - " vs %"PRIu64, - next_revocation_number, - peer->next_index[LOCAL] - 1); + peer_failed_err(peer->pps, + &peer->channel_id, + "bad future last_local_per_commit_secret: %"PRIu64 + " vs %"PRIu64, + next_revocation_number, + peer->next_index[LOCAL] - 1); /* Oh shit, they really are from the future! */ peer_billboard(true, "They have future commitment number %"PRIu64 @@ -2165,7 +2129,8 @@ static void check_future_dataloss_fields(struct peer *peer, remote_current_per_commitment_point))); /* We have to send them an error to trigger dropping to chain. */ - peer_failed(peer->pps, &peer->channel_id, "Awaiting unilateral close"); + peer_failed_err(peer->pps, &peer->channel_id, + "Awaiting unilateral close"); } /* BOLT #2: @@ -2222,15 +2187,15 @@ static void check_current_dataloss_fields(struct peer *peer, if (!secret_eq_consttime(&old_commit_secret, last_local_per_commit_secret)) - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish: your_last_per_commitment_secret %"PRIu64 - ": %s should be %s", - next_revocation_number, - type_to_string(tmpctx, struct secret, - last_local_per_commit_secret), - type_to_string(tmpctx, struct secret, - &old_commit_secret)); + peer_failed_err(peer->pps, + &peer->channel_id, + "bad reestablish: your_last_per_commitment_secret %"PRIu64 + ": %s should be %s", + next_revocation_number, + type_to_string(tmpctx, struct secret, + last_local_per_commit_secret), + type_to_string(tmpctx, struct secret, + &old_commit_secret)); if (!remote_current_per_commitment_point) { status_debug("option_static_remotekey: fields are correct"); @@ -2248,35 +2213,35 @@ static void check_current_dataloss_fields(struct peer *peer, if (next_commitment_number == peer->revocations_received + 1) { if (!pubkey_eq(remote_current_per_commitment_point, &peer->old_remote_per_commit)) { - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish: remote's " - "my_current_per_commitment_point %"PRIu64 - "is %s; expected %s (new is %s).", - next_commitment_number - 1, - type_to_string(tmpctx, struct pubkey, - remote_current_per_commitment_point), - type_to_string(tmpctx, struct pubkey, - &peer->old_remote_per_commit), - type_to_string(tmpctx, struct pubkey, - &peer->remote_per_commit)); + peer_failed_warn(peer->pps, + &peer->channel_id, + "bad reestablish: remote's " + "my_current_per_commitment_point %"PRIu64 + "is %s; expected %s (new is %s).", + next_commitment_number - 1, + type_to_string(tmpctx, struct pubkey, + remote_current_per_commitment_point), + type_to_string(tmpctx, struct pubkey, + &peer->old_remote_per_commit), + type_to_string(tmpctx, struct pubkey, + &peer->remote_per_commit)); } } else { /* We've sent a commit sig but haven't gotten a revoke+ack back */ if (!pubkey_eq(remote_current_per_commitment_point, &peer->remote_per_commit)) { - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish: remote's " - "my_current_per_commitment_point %"PRIu64 - "is %s; expected %s (old is %s).", - next_commitment_number - 1, - type_to_string(tmpctx, struct pubkey, - remote_current_per_commitment_point), - type_to_string(tmpctx, struct pubkey, - &peer->remote_per_commit), - type_to_string(tmpctx, struct pubkey, - &peer->old_remote_per_commit)); + peer_failed_warn(peer->pps, + &peer->channel_id, + "bad reestablish: remote's " + "my_current_per_commitment_point %"PRIu64 + "is %s; expected %s (old is %s).", + next_commitment_number - 1, + type_to_string(tmpctx, struct pubkey, + remote_current_per_commitment_point), + type_to_string(tmpctx, struct pubkey, + &peer->remote_per_commit), + type_to_string(tmpctx, struct pubkey, + &peer->old_remote_per_commit)); } } @@ -2400,11 +2365,11 @@ static void peer_reconnect(struct peer *peer, &next_revocation_number, &last_local_per_commitment_secret, &remote_current_per_commitment_point)) { - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish msg: %s %s", - peer_wire_name(fromwire_peektype(msg)), - tal_hex(msg, msg)); + peer_failed_warn(peer->pps, + &peer->channel_id, + "bad reestablish msg: %s %s", + peer_wire_name(fromwire_peektype(msg)), + tal_hex(msg, msg)); } status_debug("Got reestablish commit=%"PRIu64" revoke=%"PRIu64, @@ -2455,31 +2420,31 @@ static void peer_reconnect(struct peer *peer, if (next_revocation_number == peer->next_index[LOCAL] - 2) { /* Don't try to retransmit revocation index -1! */ if (peer->next_index[LOCAL] < 2) { - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish revocation_number: %" - PRIu64, - next_revocation_number); + peer_failed_err(peer->pps, + &peer->channel_id, + "bad reestablish revocation_number: %" + PRIu64, + next_revocation_number); } retransmit_revoke_and_ack = true; } else if (next_revocation_number < peer->next_index[LOCAL] - 1) { - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish revocation_number: %"PRIu64 - " vs %"PRIu64, - next_revocation_number, - peer->next_index[LOCAL]); + peer_failed_err(peer->pps, + &peer->channel_id, + "bad reestablish revocation_number: %"PRIu64 + " vs %"PRIu64, + next_revocation_number, + peer->next_index[LOCAL]); } else if (next_revocation_number > peer->next_index[LOCAL] - 1) { if (!check_extra_fields) /* They don't support option_data_loss_protect or * option_static_remotekey, we fail it due to * unexpected number */ - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish revocation_number: %"PRIu64 - " vs %"PRIu64, - next_revocation_number, - peer->next_index[LOCAL] - 1); + peer_failed_err(peer->pps, + &peer->channel_id, + "bad reestablish revocation_number: %"PRIu64 + " vs %"PRIu64, + next_revocation_number, + peer->next_index[LOCAL] - 1); /* Remote claims it's ahead of us: can it prove it? * Does not return. */ @@ -2502,11 +2467,11 @@ static void peer_reconnect(struct peer *peer, if (next_commitment_number == peer->next_index[REMOTE] - 1) { /* We completed opening, we don't re-transmit that one! */ if (next_commitment_number == 0) - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish commitment_number: %" - PRIu64, - next_commitment_number); + peer_failed_err(peer->pps, + &peer->channel_id, + "bad reestablish commitment_number: %" + PRIu64, + next_commitment_number); retransmit_commitment_signed = true; @@ -2519,12 +2484,12 @@ static void peer_reconnect(struct peer *peer, * - SHOULD fail the channel. */ } else if (next_commitment_number != peer->next_index[REMOTE]) - peer_failed(peer->pps, - &peer->channel_id, - "bad reestablish commitment_number: %"PRIu64 - " vs %"PRIu64, - next_commitment_number, - peer->next_index[REMOTE]); + peer_failed_err(peer->pps, + &peer->channel_id, + "bad reestablish commitment_number: %"PRIu64 + " vs %"PRIu64, + next_commitment_number, + peer->next_index[REMOTE]); else retransmit_commitment_signed = false; diff --git a/closingd/closingd.c b/closingd/closingd.c index 517481cb2080..a090e574e758 100644 --- a/closingd/closingd.c +++ b/closingd/closingd.c @@ -51,13 +51,13 @@ static struct bitcoin_tx *close_tx(const tal_t *ctx, out_minus_fee[LOCAL] = out[LOCAL]; out_minus_fee[REMOTE] = out[REMOTE]; if (!amount_sat_sub(&out_minus_fee[opener], out[opener], fee)) - peer_failed(pps, channel_id, - "Funder cannot afford fee %s (%s and %s)", - type_to_string(tmpctx, struct amount_sat, &fee), - type_to_string(tmpctx, struct amount_sat, - &out[LOCAL]), - type_to_string(tmpctx, struct amount_sat, - &out[REMOTE])); + peer_failed_warn(pps, channel_id, + "Funder cannot afford fee %s (%s and %s)", + type_to_string(tmpctx, struct amount_sat, &fee), + type_to_string(tmpctx, struct amount_sat, + &out[LOCAL]), + type_to_string(tmpctx, struct amount_sat, + &out[REMOTE])); status_debug("Making close tx at = %s/%s fee %s", type_to_string(tmpctx, struct amount_sat, &out[LOCAL]), @@ -76,18 +76,18 @@ static struct bitcoin_tx *close_tx(const tal_t *ctx, out_minus_fee[REMOTE], dust_limit); if (!tx) - peer_failed(pps, channel_id, - "Both outputs below dust limit:" - " funding = %s" - " fee = %s" - " dust_limit = %s" - " LOCAL = %s" - " REMOTE = %s", - type_to_string(tmpctx, struct amount_sat, &funding), - type_to_string(tmpctx, struct amount_sat, &fee), - type_to_string(tmpctx, struct amount_sat, &dust_limit), - type_to_string(tmpctx, struct amount_sat, &out[LOCAL]), - type_to_string(tmpctx, struct amount_sat, &out[REMOTE])); + peer_failed_err(pps, channel_id, + "Both outputs below dust limit:" + " funding = %s" + " fee = %s" + " dust_limit = %s" + " LOCAL = %s" + " REMOTE = %s", + type_to_string(tmpctx, struct amount_sat, &funding), + type_to_string(tmpctx, struct amount_sat, &fee), + type_to_string(tmpctx, struct amount_sat, &dust_limit), + type_to_string(tmpctx, struct amount_sat, &out[LOCAL]), + type_to_string(tmpctx, struct amount_sat, &out[REMOTE])); return tx; } @@ -201,10 +201,10 @@ static void do_reconnect(struct per_peer_state *pps, &next_remote_revocation_number, &their_secret, &next_commitment_point)) { - peer_failed(pps, channel_id, - "bad reestablish msg: %s %s", - peer_wire_name(fromwire_peektype(channel_reestablish)), - tal_hex(tmpctx, channel_reestablish)); + peer_failed_warn(pps, channel_id, + "bad reestablish msg: %s %s", + peer_wire_name(fromwire_peektype(channel_reestablish)), + tal_hex(tmpctx, channel_reestablish)); } status_debug("Got reestablish commit=%"PRIu64" revoke=%"PRIu64, next_local_commitment_number, @@ -360,9 +360,9 @@ receive_offer(struct per_peer_state *pps, their_sig.sighash_type = SIGHASH_ALL; if (!fromwire_closing_signed(msg, &their_channel_id, &received_fee, &their_sig.s)) - peer_failed(pps, channel_id, - "Expected closing_signed: %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(pps, channel_id, + "Expected closing_signed: %s", + tal_hex(tmpctx, msg)); /* BOLT #2: * @@ -412,17 +412,17 @@ receive_offer(struct per_peer_state *pps, if (!trimmed || !check_tx_sig(trimmed, 0, NULL, funding_wscript, &funding_pubkey[REMOTE], &their_sig)) { - peer_failed(pps, channel_id, - "Bad closing_signed signature for" - " %s (and trimmed version %s)", - type_to_string(tmpctx, - struct bitcoin_tx, - tx), - trimmed ? - type_to_string(tmpctx, - struct bitcoin_tx, - trimmed) - : "NONE"); + peer_failed_warn(pps, channel_id, + "Bad closing_signed signature for" + " %s (and trimmed version %s)", + type_to_string(tmpctx, + struct bitcoin_tx, + tx), + trimmed ? + type_to_string(tmpctx, + struct bitcoin_tx, + trimmed) + : "NONE"); } tx = trimmed; } @@ -507,10 +507,10 @@ adjust_offer(struct per_peer_state *pps, const struct channel_id *channel_id, /* Within 1 satoshi? Agree. */ if (!amount_sat_add(&min_plus_one, feerange->min, AMOUNT_SAT(1))) - peer_failed(pps, channel_id, - "Fee offer %s min too large", - type_to_string(tmpctx, struct amount_sat, - &feerange->min)); + peer_failed_warn(pps, channel_id, + "Fee offer %s min too large", + type_to_string(tmpctx, struct amount_sat, + &feerange->min)); if (amount_sat_greater_eq(min_plus_one, feerange->max)) return remote_offer; @@ -524,15 +524,15 @@ adjust_offer(struct per_peer_state *pps, const struct channel_id *channel_id, /* Max is below our minimum acceptable? */ if (!amount_sat_sub(&range_len, feerange->max, min_fee_to_accept)) - peer_failed(pps, channel_id, - "Feerange %s-%s" - " below minimum acceptable %s", - type_to_string(tmpctx, struct amount_sat, - &feerange->min), - type_to_string(tmpctx, struct amount_sat, - &feerange->max), - type_to_string(tmpctx, struct amount_sat, - &min_fee_to_accept)); + peer_failed_warn(pps, channel_id, + "Feerange %s-%s" + " below minimum acceptable %s", + type_to_string(tmpctx, struct amount_sat, + &feerange->min), + type_to_string(tmpctx, struct amount_sat, + &feerange->max), + type_to_string(tmpctx, struct amount_sat, + &min_fee_to_accept)); if (fee_negotiation_step_unit == CLOSING_FEE_NEGOTIATION_STEP_UNIT_SATOSHI) { diff --git a/common/peer_failed.c b/common/peer_failed.c index 336fb094bbef..e0b4ffde1d6a 100644 --- a/common/peer_failed.c +++ b/common/peer_failed.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -24,34 +25,59 @@ peer_fatal_continue(const u8 *msg TAKES, const struct per_peer_state *pps) } /* We only support one channel per peer anyway */ -void peer_failed(struct per_peer_state *pps, - const struct channel_id *channel_id, - const char *fmt, ...) +static void NORETURN +peer_failed(struct per_peer_state *pps, + bool warn, + const struct channel_id *channel_id, + const char *desc) { - va_list ap; - const char *desc; - u8 *msg, *err; - - va_start(ap, fmt); - desc = tal_vfmt(NULL, fmt, ap); - va_end(ap); + u8 *msg; - /* Tell peer the error. */ - err = towire_errorfmt(desc, channel_id, "%s", desc); - sync_crypto_write(pps, err); + if (warn) { + msg = towire_warningfmt(desc, channel_id, "%s", desc); + } else { + msg = towire_errorfmt(desc, channel_id, "%s", desc); + } + sync_crypto_write(pps, msg); /* Tell master the error so it can re-xmit. */ msg = towire_status_peer_error(NULL, channel_id, desc, - /* all-channels errors should not close channels */ - channel_id_is_all(channel_id), + warn, pps, - err); + msg); peer_billboard(true, desc); - tal_free(desc); peer_fatal_continue(take(msg), pps); } +void peer_failed_warn(struct per_peer_state *pps, + const struct channel_id *channel_id, + const char *fmt, ...) +{ + va_list ap; + const char *desc; + + va_start(ap, fmt); + desc = tal_vfmt(tmpctx, fmt, ap); + va_end(ap); + + peer_failed(pps, true, channel_id, desc); +} + +void peer_failed_err(struct per_peer_state *pps, + const struct channel_id *channel_id, + const char *fmt, ...) +{ + va_list ap; + const char *desc; + + va_start(ap, fmt); + desc = tal_vfmt(tmpctx, fmt, ap); + va_end(ap); + + peer_failed(pps, false, channel_id, desc); +} + /* We're failing because peer sent us an error/warning message */ void peer_failed_received_errmsg(struct per_peer_state *pps, const char *desc, diff --git a/common/peer_failed.h b/common/peer_failed.h index cf5da6cf90df..51fc8a7fae81 100644 --- a/common/peer_failed.h +++ b/common/peer_failed.h @@ -8,14 +8,25 @@ struct channel_id; struct per_peer_state; /** - * peer_failed - Exit with error for peer. + * peer_failed_warn - Send a warning msg and close the connection. * @pps: the per-peer state. - * @channel_id: channel with error, or NULL for all. + * @channel_id: channel with error, or NULL for no particular channel. * @fmt...: format as per status_failed(STATUS_FAIL_PEER_BAD) */ -void peer_failed(struct per_peer_state *pps, - const struct channel_id *channel_id, - const char *fmt, ...) +void peer_failed_warn(struct per_peer_state *pps, + const struct channel_id *channel_id, + const char *fmt, ...) + PRINTF_FMT(3,4) NORETURN; + +/** + * peer_failed_err - Send a warning msg and close the channel. + * @pps: the per-peer state. + * @channel_id: channel with error. + * @fmt...: format as per status_failed(STATUS_FAIL_PEER_BAD) + */ +void peer_failed_err(struct per_peer_state *pps, + const struct channel_id *channel_id, + const char *fmt, ...) PRINTF_FMT(3,4) NORETURN; /* We're failing because peer sent us an error message: NULL diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 1a7a660231c9..550a618d7ed0 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -341,18 +341,18 @@ static void handle_peer_shutdown(struct state *state, u8 *msg) struct channel_id cid; if (!fromwire_shutdown(tmpctx, msg, &cid, &scriptpubkey)) - peer_failed(state->pps, &state->channel_id, + peer_failed_warn(state->pps, &state->channel_id, "Bad shutdown %s", tal_hex(msg, msg)); if (tal_count(state->upfront_shutdown_script[REMOTE]) && !memeq(scriptpubkey, tal_count(scriptpubkey), state->upfront_shutdown_script[REMOTE], tal_count(state->upfront_shutdown_script[REMOTE]))) - peer_failed(state->pps, &state->channel_id, - "scriptpubkey %s is not as agreed upfront (%s)", - tal_hex(state, scriptpubkey), - tal_hex(state, - state->upfront_shutdown_script[REMOTE])); + peer_failed_warn(state->pps, &state->channel_id, + "scriptpubkey %s is not as agreed upfront (%s)", + tal_hex(state, scriptpubkey), + tal_hex(state, + state->upfront_shutdown_script[REMOTE])); wire_sync_write(REQ_FD, take(towire_dualopend_got_shutdown(NULL, @@ -391,10 +391,10 @@ static void check_channel_id(struct state *state, * the `temporary_channel_id` in the `open_channel` message. */ if (!channel_id_eq(id_in, orig_id)) - peer_failed(state->pps, id_in, - "channel ids don't match. expected %s, got %s", - type_to_string(tmpctx, struct channel_id, orig_id), - type_to_string(tmpctx, struct channel_id, id_in)); + peer_failed_err(state->pps, id_in, + "channel ids don't match. expected %s, got %s", + type_to_string(tmpctx, struct channel_id, orig_id), + type_to_string(tmpctx, struct channel_id, id_in)); } static void set_reserve(struct state *state, struct amount_sat funding_total) @@ -778,10 +778,9 @@ static void handle_tx_sigs(struct state *state, const u8 *msg) cast_const3( struct witness_stack ***, &ws))) - peer_failed(state->pps, - &state->channel_id, - "Bad tx_signatures %s", - tal_hex(msg, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Bad tx_signatures %s", + tal_hex(msg, msg)); /* Maybe they didn't get our funding_locked message ? */ if (state->funding_locked[LOCAL] && !state->reconnected) { @@ -796,10 +795,9 @@ static void handle_tx_sigs(struct state *state, const u8 *msg) /* On reconnect, we expect them to resend tx_sigs if they haven't * gotten our funding_locked yet */ if (state->funding_locked[REMOTE] && !state->reconnected) - peer_failed(state->pps, - &state->channel_id, - "tx_signatures sent after funding_locked %s", - tal_hex(msg, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "tx_signatures sent after funding_locked %s", + tal_hex(msg, msg)); if (state->remote_funding_sigs_rcvd) { status_info("Got duplicate WIRE_TX_SIGNATURES, " @@ -825,9 +823,10 @@ static void handle_tx_sigs(struct state *state, const u8 *msg) continue; if (j == tal_count(ws)) - peer_failed(state->pps, &state->channel_id, - "Mismatch witness stack count %s", - tal_hex(msg, msg)); + peer_failed_warn(state->pps, + &state->channel_id, + "Mismatch witness stack count %s", + tal_hex(msg, msg)); elem = cast_const2(const struct witness_element **, ws[j++]->witness_element); @@ -929,8 +928,8 @@ static bool send_next(struct state *state, struct wally_psbt **psbt) /* We should always get a updated psbt back */ if (!updated_psbt) - peer_failed(state->pps, &state->channel_id, - "Unable to determine next tx update"); + peer_failed_err(state->pps, &state->channel_id, + "Unable to determine next tx update"); state->changeset = tal_free(state->changeset); state->changeset = psbt_get_changeset(state, *psbt, updated_psbt); @@ -1096,9 +1095,9 @@ static bool run_tx_interactive(struct state *state, cast_const2(u8 **, &redeemscript), add_tlvs)) - peer_failed(state->pps, &state->channel_id, - "Parsing tx_add_input %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Parsing tx_add_input %s", + tal_hex(tmpctx, msg)); check_channel_id(state, &cid, &state->channel_id); @@ -1109,9 +1108,9 @@ static bool run_tx_interactive(struct state *state, * - it receives more than 2^12 `tx_add_input` * messages */ if (++state->tx_msg_count[TX_ADD_INPUT] > MAX_TX_MSG_RCVD) - peer_failed(state->pps, &state->channel_id, - "Too many `tx_add_input`s" - " received"); + peer_failed_warn(state->pps, &state->channel_id, + "Too many `tx_add_input`s" + " received"); /* * BOLT-fe0351ca2cea3105c4f2eb18c571afca9d21c85b #2: * - if is the `initiator`: @@ -1122,9 +1121,9 @@ static bool run_tx_interactive(struct state *state, * with the incorrect parity */ if (serial_id % 2 == our_role) - peer_failed(state->pps, &state->channel_id, - "Invalid serial_id rcvd. %"PRIu64, - serial_id); + peer_failed_warn(state->pps, &state->channel_id, + "Invalid serial_id rcvd. %"PRIu64, + serial_id); /* * BOLT-fe0351ca2cea3105c4f2eb18c571afca9d21c85b #2: * - MUST fail the transaction collaboration if: @@ -1132,20 +1131,20 @@ static bool run_tx_interactive(struct state *state, * - it recieves a duplicate `serial_id` */ if (psbt_find_serial_input(psbt, serial_id) != -1) - peer_failed(state->pps, &state->channel_id, - "Duplicate serial_id rcvd." - " %"PRIu64, serial_id); + peer_failed_warn(state->pps, &state->channel_id, + "Duplicate serial_id rcvd." + " %"PRIu64, serial_id); /* Convert tx_bytes to a tx! */ len = tal_bytelen(tx_bytes); tx = pull_bitcoin_tx(state, &tx_bytes, &len); if (!tx || len != 0) - peer_failed(state->pps, &state->channel_id, - "Invalid tx sent."); + peer_failed_warn(state->pps, &state->channel_id, + "Invalid tx sent."); if (outnum >= tx->wtx->num_outputs) - peer_failed(state->pps, &state->channel_id, - "Invalid tx outnum sent. %u", outnum); + peer_failed_warn(state->pps, &state->channel_id, + "Invalid tx outnum sent. %u", outnum); /* * BOLT-fe0351ca2cea3105c4f2eb18c571afca9d21c85b #2: * - MUST fail the transaction collaboration if: @@ -1155,11 +1154,11 @@ static bool run_tx_interactive(struct state *state, */ if (!is_segwit_output(&tx->wtx->outputs[outnum], redeemscript)) - peer_failed(state->pps, &state->channel_id, - "Invalid tx sent. Not SegWit %s", - type_to_string(tmpctx, - struct bitcoin_tx, - tx)); + peer_failed_warn(state->pps, &state->channel_id, + "Invalid tx sent. Not SegWit %s", + type_to_string(tmpctx, + struct bitcoin_tx, + tx)); /* * BOLT-fe0351ca2cea3105c4f2eb18c571afca9d21c85b #2: @@ -1173,9 +1172,10 @@ static bool run_tx_interactive(struct state *state, */ bitcoin_txid(tx, &txid); if (psbt_has_input(psbt, &txid, outnum)) - peer_failed(state->pps, &state->channel_id, - "Unable to add input - " - "already present"); + peer_failed_warn(state->pps, + &state->channel_id, + "Unable to add input - " + "already present"); /* * BOLT-fe0351ca2cea3105c4f2eb18c571afca9d21c85b #2: @@ -1189,8 +1189,8 @@ static bool run_tx_interactive(struct state *state, NULL, redeemscript); if (!in) - peer_failed(state->pps, &state->channel_id, - "Unable to add input"); + peer_failed_warn(state->pps, &state->channel_id, + "Unable to add input"); tal_wally_start(); wally_psbt_input_set_utxo(in, tx->wtx); @@ -1220,9 +1220,9 @@ static bool run_tx_interactive(struct state *state, int input_index; if (!fromwire_tx_remove_input(msg, &cid, &serial_id)) - peer_failed(state->pps, &state->channel_id, - "Parsing tx_remove_input %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Parsing tx_remove_input %s", + tal_hex(tmpctx, msg)); check_channel_id(state, &cid, &state->channel_id); @@ -1233,24 +1233,24 @@ static bool run_tx_interactive(struct state *state, * - it receives more than 2^12 `tx_rm_input` * messages */ if (++state->tx_msg_count[TX_RM_INPUT] > MAX_TX_MSG_RCVD) - peer_failed(state->pps, &state->channel_id, - "Too many `tx_rm_input`s" - " received"); + peer_failed_warn(state->pps, &state->channel_id, + "Too many `tx_rm_input`s" + " received"); /* BOLT-fe0351ca2cea3105c4f2eb18c571afca9d21c85b #2 * The sending node: * - MUST NOT send a `tx_remove_input` for an * input which is not theirs */ if (serial_id % 2 == our_role) - peer_failed(state->pps, &state->channel_id, - "Invalid serial_id rcvd. %"PRIu64, - serial_id); + peer_failed_warn(state->pps, &state->channel_id, + "Invalid serial_id rcvd. %"PRIu64, + serial_id); input_index = psbt_find_serial_input(psbt, serial_id); if (input_index == -1) - peer_failed(state->pps, &state->channel_id, - "No input added with serial_id" - " %"PRIu64, serial_id); + peer_failed_err(state->pps, &state->channel_id, + "No input added with serial_id" + " %"PRIu64, serial_id); psbt_rm_input(psbt, input_index); break; @@ -1263,9 +1263,10 @@ static bool run_tx_interactive(struct state *state, if (!fromwire_tx_add_output(tmpctx, msg, &cid, &serial_id, &value, &scriptpubkey)) - peer_failed(state->pps, &state->channel_id, - "Parsing tx_add_output %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(state->pps, + &state->channel_id, + "Parsing tx_add_output %s", + tal_hex(tmpctx, msg)); check_channel_id(state, &cid, &state->channel_id); /* @@ -1275,9 +1276,9 @@ static bool run_tx_interactive(struct state *state, * - it receives more than 2^12 `tx_add_output` * messages */ if (++state->tx_msg_count[TX_ADD_OUTPUT] > MAX_TX_MSG_RCVD) - peer_failed(state->pps, &state->channel_id, - "Too many `tx_add_output`s" - " received"); + peer_failed_warn(state->pps, &state->channel_id, + "Too many `tx_add_output`s" + " received"); /* BOLT-fe0351ca2cea3105c4f2eb18c571afca9d21c85b #2 * The receiving node: @@ -1287,14 +1288,14 @@ static bool run_tx_interactive(struct state *state, * - it receives a `serial_id` from the peer with the * incorrect parity */ if (serial_id % 2 == our_role) - peer_failed(state->pps, &state->channel_id, - "Invalid serial_id rcvd. %"PRIu64, - serial_id); + peer_failed_warn(state->pps, &state->channel_id, + "Invalid serial_id rcvd. %"PRIu64, + serial_id); if (psbt_find_serial_output(psbt, serial_id) != -1) - peer_failed(state->pps, &state->channel_id, - "Duplicate serial_id rcvd." - " %"PRIu64, serial_id); + peer_failed_warn(state->pps, &state->channel_id, + "Duplicate serial_id rcvd." + " %"PRIu64, serial_id); amt = amount_sat(value); out = psbt_append_output(psbt, scriptpubkey, amt); psbt_output_set_serial_id(psbt, out, serial_id); @@ -1304,9 +1305,9 @@ static bool run_tx_interactive(struct state *state, int output_index; if (!fromwire_tx_remove_output(msg, &cid, &serial_id)) - peer_failed(state->pps, &state->channel_id, - "Parsing tx_remove_output %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Parsing tx_remove_output %s", + tal_hex(tmpctx, msg)); check_channel_id(state, &cid, &state->channel_id); @@ -1317,32 +1318,35 @@ static bool run_tx_interactive(struct state *state, * - it receives more than 2^12 `tx_rm_output` * messages */ if (++state->tx_msg_count[TX_RM_OUTPUT] > MAX_TX_MSG_RCVD) - peer_failed(state->pps, &state->channel_id, - "Too many `tx_rm_output`s" - " received"); + peer_failed_warn(state->pps, &state->channel_id, + "Too many `tx_rm_output`s" + " received"); /* BOLT-fe0351ca2cea3105c4f2eb18c571afca9d21c85b #2 * The sending node: * - MUST NOT send a `tx_remove_ouput` for an * input which is not theirs */ if (serial_id % 2 == our_role) - peer_failed(state->pps, &state->channel_id, - "Invalid serial_id rcvd." - " %"PRIu64, serial_id); + peer_failed_warn(state->pps, + &state->channel_id, + "Invalid serial_id rcvd." + " %"PRIu64, serial_id); output_index = psbt_find_serial_output(psbt, serial_id); if (output_index == -1) - peer_failed(state->pps, &state->channel_id, - "No output added with serial_id" - " %"PRIu64, serial_id); + peer_failed_warn(state->pps, + &state->channel_id, + "No output added with serial_id" + " %"PRIu64, serial_id); psbt_rm_output(psbt, output_index); break; } case WIRE_TX_COMPLETE: if (!fromwire_tx_complete(msg, &cid)) - peer_failed(state->pps, &state->channel_id, - "Parsing tx_complete %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(state->pps, + &state->channel_id, + "Parsing tx_complete %s", + tal_hex(tmpctx, msg)); check_channel_id(state, &cid, &state->channel_id); they_complete = true; break; @@ -1381,9 +1385,9 @@ static bool run_tx_interactive(struct state *state, case WIRE_REPLY_SHORT_CHANNEL_IDS_END: case WIRE_PING: case WIRE_PONG: - peer_failed(state->pps, &state->channel_id, - "Unexpected wire message %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Unexpected wire message %s", + tal_hex(tmpctx, msg)); return false; } @@ -1443,9 +1447,9 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) &state->first_per_commitment_point[REMOTE], &channel_flags, open_tlv)) - peer_failed(state->pps, &state->channel_id, - "Parsing open_channel2 %s", - tal_hex(tmpctx, oc2_msg)); + peer_failed_err(state->pps, &state->channel_id, + "Parsing open_channel2 %s", + tal_hex(tmpctx, oc2_msg)); if (open_tlv->option_upfront_shutdown_script) { state->upfront_shutdown_script[REMOTE] = tal_steal(state, @@ -1533,13 +1537,13 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) /* Check that total funding doesn't overflow */ if (!amount_sat_add(&total, state->opener_funding, state->accepter_funding)) - peer_failed(state->pps, &state->channel_id, - "Amount overflow. Local sats %s. " - "Remote sats %s", - type_to_string(tmpctx, struct amount_sat, - &state->accepter_funding), - type_to_string(tmpctx, struct amount_sat, - &state->opener_funding)); + peer_failed_err(state->pps, &state->channel_id, + "Amount overflow. Local sats %s. " + "Remote sats %s", + type_to_string(tmpctx, struct amount_sat, + &state->accepter_funding), + type_to_string(tmpctx, struct amount_sat, + &state->opener_funding)); /* Check that total funding doesn't exceed allowed channel capacity */ /* BOLT #2: @@ -1634,11 +1638,11 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) if (!find_txout(state->psbt, scriptpubkey_p2wsh(tmpctx, wscript), &state->funding_txout)) - peer_failed(state->pps, &state->channel_id, - "Expected output %s not found on funding tx %s", - tal_hex(tmpctx, scriptpubkey_p2wsh(tmpctx, wscript)), - type_to_string(tmpctx, struct wally_psbt, - state->psbt)); + peer_failed_err(state->pps, &state->channel_id, + "Expected output %s not found on funding tx %s", + tal_hex(tmpctx, scriptpubkey_p2wsh(tmpctx, wscript)), + type_to_string(tmpctx, struct wally_psbt, + state->psbt)); /* Check tx funds are sane */ err_reason = check_balances(tmpctx, state, @@ -1661,17 +1665,17 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) if (!fromwire_commitment_signed(tmpctx, msg, &cid, &remote_sig.s, &htlc_sigs)) - peer_failed(state->pps, &state->channel_id, - "Parsing commitment signed %s", - tal_hex(tmpctx, msg)); + peer_failed_err(state->pps, &state->channel_id, + "Parsing commitment signed %s", + tal_hex(tmpctx, msg)); check_channel_id(state, &cid, &state->channel_id); if (htlc_sigs != NULL) - peer_failed(state->pps, &state->channel_id, - "Must not send HTLCs with first" - " commitment. %s", - tal_hex(tmpctx, msg)); + peer_failed_err(state->pps, &state->channel_id, + "Must not send HTLCs with first" + " commitment. %s", + tal_hex(tmpctx, msg)); if (!amount_sat_to_msat(&our_msats, state->accepter_funding)) status_failed(STATUS_FAIL_INTERNAL_ERROR, @@ -1730,22 +1734,21 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) * a courtesy to other implementaters whose brains may be so * twisted by coding in Go, Scala and Rust that they can no * longer read C code. */ - peer_failed(state->pps, - &state->channel_id, - "Bad signature %s on tx %s using key %s" - " (funding txid %s, psbt %s)", - type_to_string(tmpctx, struct bitcoin_signature, - &remote_sig), - type_to_string(tmpctx, struct bitcoin_tx, - local_commit), - type_to_string(tmpctx, struct pubkey, - &state->their_funding_pubkey), - /* This is the first place we'd discover - * the funding tx doesn't match up */ - type_to_string(tmpctx, struct bitcoin_txid, - &state->funding_txid), - type_to_string(tmpctx, struct wally_psbt, - state->psbt)); + peer_failed_err(state->pps, &state->channel_id, + "Bad signature %s on tx %s using key %s" + " (funding txid %s, psbt %s)", + type_to_string(tmpctx, struct bitcoin_signature, + &remote_sig), + type_to_string(tmpctx, struct bitcoin_tx, + local_commit), + type_to_string(tmpctx, struct pubkey, + &state->their_funding_pubkey), + /* This is the first place we'd discover + * the funding tx doesn't match up */ + type_to_string(tmpctx, struct bitcoin_txid, + &state->funding_txid), + type_to_string(tmpctx, struct wally_psbt, + state->psbt)); } /* Create commitment tx signatures for remote */ @@ -1950,8 +1953,8 @@ static void opener_start(struct state *state, u8 *msg) &state->their_points.htlc, &state->first_per_commitment_point[REMOTE], a_tlv)) - peer_failed(state->pps, &state->channel_id, - "Parsing accept_channel2 %s", tal_hex(msg, msg)); + peer_failed_err(state->pps, &state->channel_id, + "Parsing accept_channel2 %s", tal_hex(msg, msg)); if (a_tlv->option_upfront_shutdown_script) { state->upfront_shutdown_script[REMOTE] @@ -1967,12 +1970,12 @@ static void opener_start(struct state *state, u8 *msg) &state->their_points.revocation); if (!channel_id_eq(&cid, &state->channel_id)) - peer_failed(state->pps, &state->channel_id, - "accept_channel2 ids don't match: " - "expected %s, got %s", - type_to_string(msg, struct channel_id, - &state->channel_id), - type_to_string(msg, struct channel_id, &cid)); + peer_failed_err(state->pps, &cid, + "accept_channel2 ids don't match: " + "expected %s, got %s", + type_to_string(msg, struct channel_id, + &state->channel_id), + type_to_string(msg, struct channel_id, &cid)); /* BOLT-5fcbda56901af9e3b1d057cc41d0c5cfa60a2b94 #2: * The receiving node: @@ -1982,22 +1985,22 @@ static void opener_start(struct state *state, u8 *msg) */ if (feerate_min > state->feerate_per_kw_funding || feerate_max < state->feerate_per_kw_funding) - peer_failed(state->pps, &state->channel_id, - "Invalid feerate %d chosen. Valid min %d," - " valid max %d", state->feerate_per_kw_funding, - feerate_min, feerate_max); + peer_failed_warn(state->pps, &state->channel_id, + "Invalid feerate %d chosen. Valid min %d," + " valid max %d", state->feerate_per_kw_funding, + feerate_min, feerate_max); /* Check that total funding doesn't overflow */ if (!amount_sat_add(&total, state->opener_funding, state->accepter_funding)) - peer_failed(state->pps, &state->channel_id, - "Amount overflow. Local sats %s. " - "Remote sats %s", - type_to_string(tmpctx, struct amount_sat, - &state->opener_funding), - type_to_string(tmpctx, struct amount_sat, - &state->accepter_funding)); + peer_failed_warn(state->pps, &state->channel_id, + "Amount overflow. Local sats %s. " + "Remote sats %s", + type_to_string(tmpctx, struct amount_sat, + &state->opener_funding), + type_to_string(tmpctx, struct amount_sat, + &state->accepter_funding)); /* Check that total funding doesn't exceed allowed channel capacity */ /* BOLT #2: @@ -2071,11 +2074,11 @@ static void opener_start(struct state *state, u8 *msg) /* Figure out the txout */ if (!find_txout(state->psbt, scriptpubkey_p2wsh(tmpctx, wscript), &state->funding_txout)) - peer_failed(state->pps, &state->channel_id, - "Expected output %s not found on funding tx %s", - tal_hex(tmpctx, scriptpubkey_p2wsh(tmpctx, wscript)), - type_to_string(tmpctx, struct wally_psbt, - state->psbt)); + peer_failed_warn(state->pps, &state->channel_id, + "Expected output %s not found on funding tx %s", + tal_hex(tmpctx, scriptpubkey_p2wsh(tmpctx, wscript)), + type_to_string(tmpctx, struct wally_psbt, + state->psbt)); /* Check tx funds are sane */ err_reason = check_balances(tmpctx, state, state->psbt, @@ -2168,15 +2171,15 @@ static void opener_start(struct state *state, u8 *msg) if (!fromwire_commitment_signed(tmpctx, msg, &cid, &remote_sig.s, &htlc_sigs)) - peer_failed(state->pps, &state->channel_id, - "Parsing commitment signed %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Parsing commitment signed %s", + tal_hex(tmpctx, msg)); if (htlc_sigs != NULL) - peer_failed(state->pps, &state->channel_id, - "Must not send HTLCs with first" - " commitment. %s", - tal_hex(tmpctx, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Must not send HTLCs with first" + " commitment. %s", + tal_hex(tmpctx, msg)); local_commit = initial_channel_tx(state, &wscript, state->channel, &state->first_per_commitment_point[LOCAL], @@ -2212,22 +2215,21 @@ static void opener_start(struct state *state, u8 *msg) * a courtesy to other implementaters whose brains may be so * twisted by coding in Go, Scala and Rust that they can no * longer read C code. */ - peer_failed(state->pps, - &state->channel_id, - "Bad signature %s on tx %s using key %s " - "(funding txid %s, psbt %s)", - type_to_string(tmpctx, struct bitcoin_signature, - &remote_sig), - type_to_string(tmpctx, struct bitcoin_tx, - local_commit), - type_to_string(tmpctx, struct pubkey, - &state->their_funding_pubkey), - /* This is the first place we'd discover the - * funding tx doesn't match up */ - type_to_string(tmpctx, struct bitcoin_txid, - &state->funding_txid), - type_to_string(tmpctx, struct wally_psbt, - state->psbt)); + peer_failed_err(state->pps, &state->channel_id, + "Bad signature %s on tx %s using key %s " + "(funding txid %s, psbt %s)", + type_to_string(tmpctx, struct bitcoin_signature, + &remote_sig), + type_to_string(tmpctx, struct bitcoin_tx, + local_commit), + type_to_string(tmpctx, struct pubkey, + &state->their_funding_pubkey), + /* This is the first place we'd discover the + * funding tx doesn't match up */ + type_to_string(tmpctx, struct bitcoin_txid, + &state->funding_txid), + type_to_string(tmpctx, struct wally_psbt, + state->psbt)); } if (direct_outputs[LOCAL]) @@ -2274,23 +2276,22 @@ static u8 *handle_funding_locked(struct state *state, u8 *msg) struct pubkey remote_per_commit; if (!fromwire_funding_locked(msg, &cid, &remote_per_commit)) - peer_failed(state->pps, &state->channel_id, - "Bad funding_locked %s", tal_hex(msg, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Bad funding_locked %s", tal_hex(msg, msg)); if (!channel_id_eq(&cid, &state->channel_id)) - peer_failed(state->pps, &state->channel_id, - "funding_locked ids don't match: " - "expected %s, got %s", - type_to_string(msg, struct channel_id, - &state->channel_id), - type_to_string(msg, struct channel_id, &cid)); + peer_failed_err(state->pps, &cid, + "funding_locked ids don't match: " + "expected %s, got %s", + type_to_string(msg, struct channel_id, + &state->channel_id), + type_to_string(msg, struct channel_id, &cid)); /* If we haven't gotten their tx_sigs yet, this is a protocol error */ if (!state->remote_funding_sigs_rcvd) { - peer_failed(state->pps, - &state->channel_id, - "funding_locked sent before tx_signatures %s", - tal_hex(msg, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "funding_locked sent before tx_signatures %s", + tal_hex(msg, msg)); } state->funding_locked[REMOTE] = true; @@ -2440,11 +2441,11 @@ check_future_dataloss_fields(struct state *state, tal_hex(tmpctx, msg)); if (!correct) - peer_failed(state->pps, - &state->channel_id, - "bad future last_local_per_commit_secret: %"PRIu64 - " vs %d", - next_revocation_number, 0); + peer_failed_err(state->pps, + &state->channel_id, + "bad future last_local_per_commit_secret: %"PRIu64 + " vs %d", + next_revocation_number, 0); /* Oh shit, they really are from the future! */ peer_billboard(true, "They have future commitment number %"PRIu64 @@ -2460,7 +2461,7 @@ check_future_dataloss_fields(struct state *state, take(towire_dualopend_fail_fallen_behind(NULL))); /* We have to send them an error to trigger dropping to chain. */ - peer_failed(state->pps, &state->channel_id, + peer_failed_err(state->pps, &state->channel_id, "Awaiting unilateral close"); } @@ -2511,11 +2512,10 @@ static void do_reconnect_dance(struct state *state) &next_revocation_number, &last_local_per_commit_secret, &remote_current_per_commit_point)) - peer_failed(state->pps, - &state->channel_id, - "Bad reestablish msg: %s %s", - peer_wire_name(fromwire_peektype(msg)), - tal_hex(msg, msg)); + peer_failed_warn(state->pps, &state->channel_id, + "Bad reestablish msg: %s %s", + peer_wire_name(fromwire_peektype(msg)), + tal_hex(msg, msg)); check_channel_id(state, &cid, &state->channel_id); @@ -2541,11 +2541,10 @@ static void do_reconnect_dance(struct state *state) } if (next_commitment_number != 1) - peer_failed(state->pps, - &state->channel_id, - "bad reestablish commitment_number: %"PRIu64 - " vs %d", - next_commitment_number, 1); + peer_failed_err(state->pps, &state->channel_id, + "bad reestablish commitment_number: %"PRIu64 + " vs %d", + next_commitment_number, 1); /* It's possible we sent our sigs, but they didn't get them. * Resend our signatures, just in case */ diff --git a/openingd/openingd.c b/openingd/openingd.c index b672c9ea6878..5b5377b8eb7b 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -400,9 +400,9 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags) &state->their_points.htlc, &state->first_per_commitment_point[REMOTE], accept_tlvs)) { - peer_failed(state->pps, - &state->channel_id, - "Parsing accept_channel %s", tal_hex(msg, msg)); + peer_failed_err(state->pps, + &state->channel_id, + "Parsing accept_channel %s", tal_hex(msg, msg)); } state->upfront_shutdown_script[REMOTE] = tal_steal(state, accept_tlvs->upfront_shutdown_script); @@ -413,12 +413,11 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags) * `temporary_channel_id` in the `open_channel` message. */ if (!channel_id_eq(&id_in, &state->channel_id)) /* In this case we exit, since we don't know what's going on. */ - peer_failed(state->pps, - &state->channel_id, - "accept_channel ids don't match: sent %s got %s", - type_to_string(msg, struct channel_id, &id_in), - type_to_string(msg, struct channel_id, - &state->channel_id)); + peer_failed_err(state->pps, &id_in, + "accept_channel ids don't match: sent %s got %s", + type_to_string(msg, struct channel_id, &id_in), + type_to_string(msg, struct channel_id, + &state->channel_id)); if (amount_sat_greater(state->remoteconf.dust_limit, state->localconf.channel_reserve)) { @@ -510,9 +509,9 @@ static bool funder_finalize_channel_setup(struct state *state, /* We were supposed to do enough checks above, but just in case, * new_initial_channel will fail to create absurd channels */ if (!state->channel) - peer_failed(state->pps, - &state->channel_id, - "could not create channel with given config"); + peer_failed_err(state->pps, + &state->channel_id, + "could not create channel with given config"); /* BOLT #2: * @@ -592,9 +591,8 @@ static bool funder_finalize_channel_setup(struct state *state, sig->sighash_type = SIGHASH_ALL; if (!fromwire_funding_signed(msg, &id_in, &sig->s)) - peer_failed(state->pps, - &state->channel_id, - "Parsing funding_signed: %s", tal_hex(msg, msg)); + peer_failed_err(state->pps, &state->channel_id, + "Parsing funding_signed: %s", tal_hex(msg, msg)); /* BOLT #2: * * This message introduces the `channel_id` to identify the channel. @@ -621,11 +619,11 @@ static bool funder_finalize_channel_setup(struct state *state, state->channel_id = cid; if (!channel_id_eq(&id_in, &state->channel_id)) - peer_failed(state->pps, &id_in, - "funding_signed ids don't match: expected %s got %s", - type_to_string(msg, struct channel_id, - &state->channel_id), - type_to_string(msg, struct channel_id, &id_in)); + peer_failed_err(state->pps, &id_in, + "funding_signed ids don't match: expected %s got %s", + type_to_string(msg, struct channel_id, + &state->channel_id), + type_to_string(msg, struct channel_id, &id_in)); /* BOLT #2: * @@ -645,14 +643,13 @@ static bool funder_finalize_channel_setup(struct state *state, } if (!check_tx_sig(*tx, 0, NULL, wscript, &state->their_funding_pubkey, sig)) { - peer_failed(state->pps, - &state->channel_id, - "Bad signature %s on tx %s using key %s", - type_to_string(tmpctx, struct bitcoin_signature, - sig), - type_to_string(tmpctx, struct bitcoin_tx, *tx), - type_to_string(tmpctx, struct pubkey, - &state->their_funding_pubkey)); + peer_failed_err(state->pps, &state->channel_id, + "Bad signature %s on tx %s using key %s", + type_to_string(tmpctx, struct bitcoin_signature, + sig), + type_to_string(tmpctx, struct bitcoin_tx, *tx), + type_to_string(tmpctx, struct pubkey, + &state->their_funding_pubkey)); } /* We save their sig to our first commitment tx */ @@ -764,9 +761,9 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) &state->first_per_commitment_point[REMOTE], &channel_flags, open_tlvs)) - peer_failed(state->pps, - &state->channel_id, - "Parsing open_channel %s", tal_hex(tmpctx, open_channel_msg)); + peer_failed_err(state->pps, + &state->channel_id, + "Parsing open_channel %s", tal_hex(tmpctx, open_channel_msg)); state->upfront_shutdown_script[REMOTE] = tal_steal(state, open_tlvs->upfront_shutdown_script); @@ -809,14 +806,13 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) * - `push_msat` is greater than `funding_satoshis` * 1000. */ if (amount_msat_greater_sat(state->push_msat, state->funding)) { - peer_failed(state->pps, - &state->channel_id, - "Their push_msat %s" - " would be too large for funding_satoshis %s", - type_to_string(tmpctx, struct amount_msat, - &state->push_msat), - type_to_string(tmpctx, struct amount_sat, - &state->funding)); + peer_failed_err(state->pps, &state->channel_id, + "Their push_msat %s" + " would be too large for funding_satoshis %s", + type_to_string(tmpctx, struct amount_msat, + &state->push_msat), + type_to_string(tmpctx, struct amount_sat, + &state->funding)); return NULL; } @@ -966,8 +962,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) &state->funding_txid, &state->funding_txout, &theirsig.s)) - peer_failed(state->pps, - &state->channel_id, + peer_failed_err(state->pps, &state->channel_id, "Parsing funding_created"); /* BOLT #2: @@ -976,11 +971,11 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) * `temporary_channel_id` in the `open_channel` message. */ if (!channel_id_eq(&id_in, &state->channel_id)) - peer_failed(state->pps, &id_in, - "funding_created ids don't match: sent %s got %s", - type_to_string(msg, struct channel_id, - &state->channel_id), - type_to_string(msg, struct channel_id, &id_in)); + peer_failed_err(state->pps, &id_in, + "funding_created ids don't match: sent %s got %s", + type_to_string(msg, struct channel_id, + &state->channel_id), + type_to_string(msg, struct channel_id, &id_in)); /* Now we can create the channel structure. */ state->channel = new_initial_channel(state, @@ -1003,9 +998,8 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) /* We don't expect this to fail, but it does do some additional * internal sanity checks. */ if (!state->channel) - peer_failed(state->pps, - &state->channel_id, - "We could not create channel with given config"); + peer_failed_err(state->pps, &state->channel_id, + "We could not create channel with given config"); /* BOLT #2: * @@ -1038,14 +1032,13 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) * a courtesy to other implementaters whose brains may be so * twisted by coding in Go, Scala and Rust that they can no * longer read C code. */ - peer_failed(state->pps, - &state->channel_id, - "Bad signature %s on tx %s using key %s", - type_to_string(tmpctx, struct bitcoin_signature, - &theirsig), - type_to_string(tmpctx, struct bitcoin_tx, local_commit), - type_to_string(tmpctx, struct pubkey, - &their_funding_pubkey)); + peer_failed_err(state->pps, &state->channel_id, + "Bad signature %s on tx %s using key %s", + type_to_string(tmpctx, struct bitcoin_signature, + &theirsig), + type_to_string(tmpctx, struct bitcoin_tx, local_commit), + type_to_string(tmpctx, struct pubkey, + &their_funding_pubkey)); } /* BOLT #2: diff --git a/tests/test_connection.py b/tests/test_connection.py index 46d99a00604c..d04fddde1911 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1971,7 +1971,19 @@ def test_fee_limits(node_factory, bitcoind): l1.set_feerates((15, 15, 15, 15), False) l1.start() - l1.daemon.wait_for_log('Peer transient failure in CHANNELD_NORMAL: channeld: .*: update_fee 253 outside range 1875-75000') + l1.daemon.wait_for_log('Peer transient failure in CHANNELD_NORMAL: channeld WARNING: .*: update_fee 253 outside range 1875-75000') + + # Closes, but does not error. Make sure it's noted in their status though. + assert 'update_fee 253 outside range 1875-75000' in only_one(only_one(l1.rpc.listpeers(l2.info['id'])['peers'])['channels'])['status'][0] + assert 'update_fee 253 outside range 1875-75000' in only_one(only_one(l2.rpc.listpeers(l1.info['id'])['peers'])['channels'])['status'][0] + + # Make l2 accept those fees, and it should recover. + l2.stop() + l2.set_feerates((15, 15, 15, 15), False) + l2.start() + + l1.rpc.close(l2.info['id']) + # Make sure the resolution of this one doesn't interfere with the next! # Note: may succeed, may fail with insufficient fee, depending on how # bitcoind feels! diff --git a/tests/test_misc.py b/tests/test_misc.py index 57dc78b870b5..f9e634f29829 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1352,13 +1352,14 @@ def test_reserve_enforcement(node_factory, executor): l2.start() wait_for(lambda: only_one(l2.rpc.listpeers(l1.info['id'])['peers'])['connected']) - # This should be impossible to pay entire thing back: l1 should - # kill us for trying to violate reserve. + # This should be impossible to pay entire thing back: l1 should warn and + # close connection for trying to violate reserve. executor.submit(l2.pay, l1, 1000000) l1.daemon.wait_for_log( - 'Peer permanent failure in CHANNELD_NORMAL: channeld: sent ' - 'ERROR Bad peer_add_htlc: CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED' + 'Peer transient failure in CHANNELD_NORMAL: channeld.*' + ' CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED' ) + assert only_one(l1.rpc.listpeers()['peers'])['connected'] is False @unittest.skipIf(not DEVELOPER, "needs dev_disconnect") diff --git a/tests/test_pay.py b/tests/test_pay.py index f6a5a11bbfdd..9f15a0db4bc4 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -263,7 +263,8 @@ def test_pay_disconnect(node_factory, bitcoind): # Wait for l1 notice l1.daemon.wait_for_log(r'Peer transient failure in CHANNELD_NORMAL: channeld: .*: update_fee \d+ outside range 1875-75000') - # l2 fails hard. + # Make l2 fail hard. + l2.rpc.close(l1.info['id'], unilateraltimeout=1) l2.daemon.wait_for_log('sendrawtx exit') bitcoind.generate_block(1, wait_for_mempool=1) sync_blockheight(bitcoind, [l1, l2]) From 24ba8661c0c351a452b30e2ecb4ea3f57510a715 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 3 Feb 2021 13:21:41 +1030 Subject: [PATCH 6/7] common: disallow NULL channel_id to peer_failed_err. No more sending "all-channel" errors; in particular, gossipd now only sends warnings (which make us hang up), not errors, and peer_connected rejections are warnings (and disconnect), not errors. Signed-off-by: Rusty Russell Changelog-Changed: Plugins: `peer_connected` rejections now send a warning, not an error, to the peer. --- common/peer_failed.c | 1 + common/read_peer_msg.c | 4 +- common/wire_error.c | 8 +- common/wire_error.h | 8 +- connectd/connectd.c | 6 +- connectd/peer_exchange_initmsg.c | 2 +- gossipd/gossipd.c | 6 +- gossipd/queries.c | 114 +++++++-------- gossipd/routing.c | 146 ++++++++++---------- gossipd/test/run-bench-find_route.c | 10 +- gossipd/test/run-crc32_of_update.c | 10 +- gossipd/test/run-extended-info.c | 10 +- gossipd/test/run-find_route-specific.c | 10 +- gossipd/test/run-find_route.c | 10 +- gossipd/test/run-overlong.c | 10 +- gossipd/test/run-txout_failure.c | 10 +- lightningd/peer_control.c | 9 +- lightningd/test/run-invoice-select-inchan.c | 5 + openingd/dualopend.c | 14 +- openingd/openingd.c | 28 ++-- tests/test_plugin.py | 2 +- wallet/db_postgres_sqlgen.c | 2 +- wallet/db_sqlite3_sqlgen.c | 2 +- wallet/statements_gettextgen.po | 4 +- wallet/test/run-wallet.c | 5 + 25 files changed, 220 insertions(+), 216 deletions(-) diff --git a/common/peer_failed.c b/common/peer_failed.c index e0b4ffde1d6a..109323608d67 100644 --- a/common/peer_failed.c +++ b/common/peer_failed.c @@ -71,6 +71,7 @@ void peer_failed_err(struct per_peer_state *pps, va_list ap; const char *desc; + assert(channel_id); va_start(ap, fmt); desc = tal_vfmt(tmpctx, fmt, ap); va_end(ap); diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index aaf5773445c2..b202661092b6 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -125,8 +125,8 @@ void handle_gossip_msg(struct per_peer_state *pps, const u8 *msg TAKES) /* It's a raw gossip msg: this copies or takes() */ gossip = tal_dup_talarr(tmpctx, u8, msg); - /* Gossipd can send us gossip messages, OR errors */ - if (fromwire_peektype(gossip) == WIRE_ERROR) { + /* Gossipd can send us gossip messages, OR warnings */ + if (fromwire_peektype(gossip) == WIRE_WARNING) { sync_crypto_write(pps, gossip); peer_failed_connection_lost(); } else { diff --git a/common/wire_error.c b/common/wire_error.c index bff0c0cb76f8..4fd7007ebd13 100644 --- a/common/wire_error.c +++ b/common/wire_error.c @@ -10,18 +10,12 @@ u8 *towire_errorfmtv(const tal_t *ctx, const char *fmt, va_list ap) { - /* BOLT #1: - * - * The channel is referred to by `channel_id`, unless `channel_id` is - * 0 (i.e. all bytes are 0), in which case it refers to all - * channels. */ - static const struct channel_id all_channels; char *estr; u8 *msg; estr = tal_vfmt(ctx, fmt, ap); /* We need tal_len to work, so we use copy. */ - msg = towire_error(ctx, channel ? channel : &all_channels, + msg = towire_error(ctx, channel, (u8 *)tal_dup_arr(estr, char, estr, strlen(estr), 0)); tal_free(estr); diff --git a/common/wire_error.h b/common/wire_error.h index 6334956ed093..2afd741e9a0b 100644 --- a/common/wire_error.h +++ b/common/wire_error.h @@ -12,25 +12,25 @@ struct channel_id; * towire_errorfmt - helper to turn string into WIRE_ERROR. * * @ctx: context to allocate from - * @channel: specific channel to complain about, or NULL for all. + * @channel: specific channel to complain about * @fmt: format for error. */ u8 *towire_errorfmt(const tal_t *ctx, const struct channel_id *channel, - const char *fmt, ...) PRINTF_FMT(3,4); + const char *fmt, ...) PRINTF_FMT(3,4) NON_NULL_ARGS(2); /** * towire_errorfmtv - helper to turn string into WIRE_ERROR. * * @ctx: context to allocate from - * @channel: specific channel to complain about, or NULL for all. + * @channel: specific channel to complain about * @fmt: format for error. * @ap: accumulated varargs. */ u8 *towire_errorfmtv(const tal_t *ctx, const struct channel_id *channel, const char *fmt, - va_list ap); + va_list ap) NON_NULL_ARGS(2); /** * towire_warningfmt - helper to turn string into WIRE_WARNING. diff --git a/connectd/connectd.c b/connectd/connectd.c index 06cc62af8253..54766732e892 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -463,14 +463,14 @@ struct io_plan *peer_connected(struct io_conn *conn, unsup = features_unsupported(daemon->our_features, their_features, INIT_FEATURE); if (unsup != -1) { - msg = towire_errorfmt(NULL, NULL, "Unsupported feature %u", - unsup); + msg = towire_warningfmt(NULL, NULL, "Unsupported feature %u", + unsup); msg = cryptomsg_encrypt_msg(tmpctx, cs, take(msg)); return io_write(conn, msg, tal_count(msg), io_close_cb, NULL); } if (!feature_check_depends(their_features, &depender, &missing)) { - msg = towire_errorfmt(NULL, NULL, + msg = towire_warningfmt(NULL, NULL, "Feature %zu requires feature %zu", depender, missing); msg = cryptomsg_encrypt_msg(tmpctx, cs, take(msg)); diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c index 5283777fbf77..72e962b42557 100644 --- a/connectd/peer_exchange_initmsg.c +++ b/connectd/peer_exchange_initmsg.c @@ -79,7 +79,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn, status_peer_debug(&peer->id, "No common chain with this peer '%s', closing", tal_hex(tmpctx, msg)); - msg = towire_errorfmt(NULL, NULL, "No common network"); + msg = towire_warningfmt(NULL, NULL, "No common network"); msg = cryptomsg_encrypt_msg(NULL, &peer->cs, take(msg)); return io_write(conn, msg, tal_count(msg), io_close_cb, NULL); } diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 6bfa337808a5..12cfb8c7c050 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -319,7 +319,7 @@ static u8 *handle_ping(struct peer *peer, const u8 *ping) /* This checks the ping packet and makes a pong reply if needed; peer * can specify it doesn't want a response, to simulate traffic. */ if (!check_ping_make_pong(NULL, ping, &pong)) - return towire_errorfmt(peer, NULL, "Bad ping"); + return towire_warningfmt(peer, NULL, "Bad ping"); if (pong) queue_peer_msg(peer, take(pong)); @@ -333,7 +333,7 @@ static const u8 *handle_pong(struct peer *peer, const u8 *pong) const char *err = got_pong(pong, &peer->num_pings_outstanding); if (err) - return towire_errorfmt(peer, NULL, "%s", err); + return towire_warningfmt(peer, NULL, "%s", err); daemon_conn_send(peer->daemon->master, take(towire_gossipd_ping_reply(NULL, &peer->id, true, @@ -454,7 +454,7 @@ static u8 *handle_onion_message(struct peer *peer, const u8 *msg) /* FIXME: ratelimit! */ if (!fromwire_onion_message(msg, msg, &onion, tlvs)) - return towire_errorfmt(peer, NULL, "Bad onion_message"); + return towire_warningfmt(peer, NULL, "Bad onion_message"); /* We unwrap the onion now. */ op = parse_onionpacket(tmpctx, onion, tal_bytelen(onion), &badreason); diff --git a/gossipd/queries.c b/gossipd/queries.c index 78b1869b6225..ed10aabdd332 100644 --- a/gossipd/queries.c +++ b/gossipd/queries.c @@ -250,9 +250,9 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg) if (!fromwire_query_short_channel_ids(tmpctx, msg, &chain, &encoded, tlvs)) { - return towire_errorfmt(peer, NULL, - "Bad query_short_channel_ids w/tlvs %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "Bad query_short_channel_ids w/tlvs %s", + tal_hex(tmpctx, msg)); } if (tlvs->query_flags) { /* BOLT #7: @@ -266,9 +266,9 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg) */ flags = decode_scid_query_flags(tmpctx, tlvs->query_flags); if (!flags) { - return towire_errorfmt(peer, NULL, - "Bad query_short_channel_ids query_flags %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "Bad query_short_channel_ids query_flags %s", + tal_hex(tmpctx, msg)); } } else flags = NULL; @@ -295,15 +295,15 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg) * - MAY fail the connection. */ if (peer->scid_queries || peer->scid_query_nodes) { - return towire_errorfmt(peer, NULL, - "Bad concurrent query_short_channel_ids"); + return towire_warningfmt(peer, NULL, + "Bad concurrent query_short_channel_ids"); } scids = decode_short_ids(tmpctx, encoded); if (!scids) { - return towire_errorfmt(peer, NULL, - "Bad query_short_channel_ids encoding %s", - tal_hex(tmpctx, encoded)); + return towire_warningfmt(peer, NULL, + "Bad query_short_channel_ids encoding %s", + tal_hex(tmpctx, encoded)); } /* BOLT #7: @@ -320,9 +320,9 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg) memset(flags, 0xFF, tal_bytelen(flags)); } else { if (tal_count(flags) != tal_count(scids)) { - return towire_errorfmt(peer, NULL, - "Bad query_short_channel_ids flags count %zu scids %zu", - tal_count(flags), tal_count(scids)); + return towire_warningfmt(peer, NULL, + "Bad query_short_channel_ids flags count %zu scids %zu", + tal_count(flags), tal_count(scids)); } } @@ -652,9 +652,9 @@ const u8 *handle_query_channel_range(struct peer *peer, const u8 *msg) if (!fromwire_query_channel_range(msg, &chain_hash, &first_blocknum, &number_of_blocks, tlvs)) { - return towire_errorfmt(peer, NULL, - "Bad query_channel_range w/tlvs %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "Bad query_channel_range w/tlvs %s", + tal_hex(tmpctx, msg)); } if (tlvs->query_option) query_option_flags = *tlvs->query_option; @@ -703,13 +703,13 @@ static u8 *append_range_reply(struct peer *peer, ts = decode_channel_update_timestamps(tmpctx, timestamps_tlv); if (!ts) - return towire_errorfmt(peer, NULL, - "reply_channel_range can't decode timestamps."); + return towire_warningfmt(peer, NULL, + "reply_channel_range can't decode timestamps."); if (tal_count(ts) != tal_count(scids)) { - return towire_errorfmt(peer, NULL, - "reply_channel_range %zu timestamps when %zu scids?", - tal_count(ts), - tal_count(scids)); + return towire_warningfmt(peer, NULL, + "reply_channel_range %zu timestamps when %zu scids?", + tal_count(ts), + tal_count(scids)); } } else ts = NULL; @@ -749,35 +749,35 @@ const u8 *handle_reply_channel_range(struct peer *peer, const u8 *msg) if (!fromwire_reply_channel_range(tmpctx, msg, &chain, &first_blocknum, &number_of_blocks, &complete, &encoded, tlvs)) { - return towire_errorfmt(peer, NULL, - "Bad reply_channel_range w/tlvs %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "Bad reply_channel_range w/tlvs %s", + tal_hex(tmpctx, msg)); } if (!bitcoin_blkid_eq(&chainparams->genesis_blockhash, &chain)) { - return towire_errorfmt(peer, NULL, - "reply_channel_range for bad chain: %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "reply_channel_range for bad chain: %s", + tal_hex(tmpctx, msg)); } if (!peer->range_replies) { - return towire_errorfmt(peer, NULL, - "reply_channel_range without query: %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "reply_channel_range without query: %s", + tal_hex(tmpctx, msg)); } /* Beware overflow! */ if (first_blocknum + number_of_blocks < first_blocknum) { - return towire_errorfmt(peer, NULL, - "reply_channel_range invalid %u+%u", - first_blocknum, number_of_blocks); + return towire_warningfmt(peer, NULL, + "reply_channel_range invalid %u+%u", + first_blocknum, number_of_blocks); } scids = decode_short_ids(tmpctx, encoded); if (!scids) { - return towire_errorfmt(peer, NULL, - "Bad reply_channel_range encoding %s", - tal_hex(tmpctx, encoded)); + return towire_warningfmt(peer, NULL, + "Bad reply_channel_range encoding %s", + tal_hex(tmpctx, encoded)); } status_peer_debug(&peer->id, @@ -807,12 +807,12 @@ const u8 *handle_reply_channel_range(struct peer *peer, const u8 *msg) /* ie. They can be outside range we asked, but they must overlap! */ if (first_blocknum + number_of_blocks <= peer->range_first_blocknum || first_blocknum >= peer->range_end_blocknum) { - return towire_errorfmt(peer, NULL, - "reply_channel_range invalid %u+%u for query %u+%u", - first_blocknum, number_of_blocks, - peer->range_first_blocknum, - peer->range_end_blocknum - - peer->range_first_blocknum); + return towire_warningfmt(peer, NULL, + "reply_channel_range invalid %u+%u for query %u+%u", + first_blocknum, number_of_blocks, + peer->range_first_blocknum, + peer->range_end_blocknum + - peer->range_first_blocknum); } start = first_blocknum; @@ -838,10 +838,10 @@ const u8 *handle_reply_channel_range(struct peer *peer, const u8 *msg) * can overlap. */ if (first_blocknum != peer->range_prev_end_blocknum + 1 && first_blocknum != peer->range_prev_end_blocknum) { - return towire_errorfmt(peer, NULL, - "reply_channel_range %u+%u previous end was block %u", - first_blocknum, number_of_blocks, - peer->range_prev_end_blocknum); + return towire_warningfmt(peer, NULL, + "reply_channel_range %u+%u previous end was block %u", + first_blocknum, number_of_blocks, + peer->range_prev_end_blocknum); } peer->range_prev_end_blocknum = end; @@ -878,21 +878,21 @@ const u8 *handle_reply_short_channel_ids_end(struct peer *peer, const u8 *msg) u8 complete; if (!fromwire_reply_short_channel_ids_end(msg, &chain, &complete)) { - return towire_errorfmt(peer, NULL, - "Bad reply_short_channel_ids_end %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "Bad reply_short_channel_ids_end %s", + tal_hex(tmpctx, msg)); } if (!bitcoin_blkid_eq(&chainparams->genesis_blockhash, &chain)) { - return towire_errorfmt(peer, NULL, - "reply_short_channel_ids_end for bad chain: %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "reply_short_channel_ids_end for bad chain: %s", + tal_hex(tmpctx, msg)); } if (!peer->scid_query_outstanding) { - return towire_errorfmt(peer, NULL, - "unexpected reply_short_channel_ids_end: %s", - tal_hex(tmpctx, msg)); + return towire_warningfmt(peer, NULL, + "unexpected reply_short_channel_ids_end: %s", + tal_hex(tmpctx, msg)); } peer->scid_query_outstanding = false; diff --git a/gossipd/routing.c b/gossipd/routing.c index fa9d3fce14f6..9d8b4ec0678a 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1391,16 +1391,16 @@ static u8 *check_channel_update(const tal_t *ctx, sha256_double(&hash, update + offset, tal_count(update) - offset); if (!check_signed_hash_nodeid(&hash, node_sig, node_id)) - return towire_errorfmt(ctx, NULL, - "Bad signature for %s hash %s" - " on channel_update %s", - type_to_string(tmpctx, - secp256k1_ecdsa_signature, - node_sig), - type_to_string(tmpctx, - struct sha256_double, - &hash), - tal_hex(tmpctx, update)); + return towire_warningfmt(ctx, NULL, + "Bad signature for %s hash %s" + " on channel_update %s", + type_to_string(tmpctx, + secp256k1_ecdsa_signature, + node_sig), + type_to_string(tmpctx, + struct sha256_double, + &hash), + tal_hex(tmpctx, update)); return NULL; } @@ -1419,52 +1419,52 @@ static u8 *check_channel_announcement(const tal_t *ctx, tal_count(announcement) - offset); if (!check_signed_hash_nodeid(&hash, node1_sig, node1_id)) { - return towire_errorfmt(ctx, NULL, - "Bad node_signature_1 %s hash %s" - " on channel_announcement %s", - type_to_string(tmpctx, - secp256k1_ecdsa_signature, - node1_sig), - type_to_string(tmpctx, - struct sha256_double, - &hash), - tal_hex(tmpctx, announcement)); + return towire_warningfmt(ctx, NULL, + "Bad node_signature_1 %s hash %s" + " on channel_announcement %s", + type_to_string(tmpctx, + secp256k1_ecdsa_signature, + node1_sig), + type_to_string(tmpctx, + struct sha256_double, + &hash), + tal_hex(tmpctx, announcement)); } if (!check_signed_hash_nodeid(&hash, node2_sig, node2_id)) { - return towire_errorfmt(ctx, NULL, - "Bad node_signature_2 %s hash %s" - " on channel_announcement %s", - type_to_string(tmpctx, - secp256k1_ecdsa_signature, - node2_sig), - type_to_string(tmpctx, - struct sha256_double, - &hash), - tal_hex(tmpctx, announcement)); + return towire_warningfmt(ctx, NULL, + "Bad node_signature_2 %s hash %s" + " on channel_announcement %s", + type_to_string(tmpctx, + secp256k1_ecdsa_signature, + node2_sig), + type_to_string(tmpctx, + struct sha256_double, + &hash), + tal_hex(tmpctx, announcement)); } if (!check_signed_hash(&hash, bitcoin1_sig, bitcoin1_key)) { - return towire_errorfmt(ctx, NULL, - "Bad bitcoin_signature_1 %s hash %s" - " on channel_announcement %s", - type_to_string(tmpctx, - secp256k1_ecdsa_signature, - bitcoin1_sig), - type_to_string(tmpctx, - struct sha256_double, - &hash), - tal_hex(tmpctx, announcement)); + return towire_warningfmt(ctx, NULL, + "Bad bitcoin_signature_1 %s hash %s" + " on channel_announcement %s", + type_to_string(tmpctx, + secp256k1_ecdsa_signature, + bitcoin1_sig), + type_to_string(tmpctx, + struct sha256_double, + &hash), + tal_hex(tmpctx, announcement)); } if (!check_signed_hash(&hash, bitcoin2_sig, bitcoin2_key)) { - return towire_errorfmt(ctx, NULL, - "Bad bitcoin_signature_2 %s hash %s" - " on channel_announcement %s", - type_to_string(tmpctx, - secp256k1_ecdsa_signature, - bitcoin2_sig), - type_to_string(tmpctx, - struct sha256_double, - &hash), - tal_hex(tmpctx, announcement)); + return towire_warningfmt(ctx, NULL, + "Bad bitcoin_signature_2 %s hash %s" + " on channel_announcement %s", + type_to_string(tmpctx, + secp256k1_ecdsa_signature, + bitcoin2_sig), + type_to_string(tmpctx, + struct sha256_double, + &hash), + tal_hex(tmpctx, announcement)); } return NULL; } @@ -1715,9 +1715,9 @@ u8 *handle_channel_announcement(struct routing_state *rstate, &pending->node_id_2, &pending->bitcoin_key_1, &pending->bitcoin_key_2)) { - err = towire_errorfmt(rstate, NULL, - "Malformed channel_announcement %s", - tal_hex(pending, pending->announce)); + err = towire_warningfmt(rstate, NULL, + "Malformed channel_announcement %s", + tal_hex(pending, pending->announce)); goto malformed; } @@ -2309,9 +2309,9 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, &channel_flags, &expiry, &htlc_minimum, &fee_base_msat, &fee_proportional_millionths)) { - err = towire_errorfmt(rstate, NULL, - "Malformed channel_update %s", - tal_hex(tmpctx, serialized)); + err = towire_warningfmt(rstate, NULL, + "Malformed channel_update %s", + tal_hex(tmpctx, serialized)); return err; } direction = channel_flags & 0x1; @@ -2587,9 +2587,9 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, * - SHOULD fail the connection. * - MUST NOT process the message further. */ - u8 *err = towire_errorfmt(rstate, NULL, - "Malformed node_announcement %s", - tal_hex(tmpctx, node_ann)); + u8 *err = towire_warningfmt(rstate, NULL, + "Malformed node_announcement %s", + tal_hex(tmpctx, node_ann)); return err; } @@ -2606,16 +2606,16 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, * - MUST NOT process the message further. * - SHOULD fail the connection. */ - u8 *err = towire_errorfmt(rstate, NULL, - "Bad signature for %s hash %s" - " on node_announcement %s", - type_to_string(tmpctx, - secp256k1_ecdsa_signature, - &signature), - type_to_string(tmpctx, - struct sha256_double, - &hash), - tal_hex(tmpctx, node_ann)); + u8 *err = towire_warningfmt(rstate, NULL, + "Bad signature for %s hash %s" + " on node_announcement %s", + type_to_string(tmpctx, + secp256k1_ecdsa_signature, + &signature), + type_to_string(tmpctx, + struct sha256_double, + &hash), + tal_hex(tmpctx, node_ann)); return err; } @@ -2627,10 +2627,10 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, * descriptors of the known types: * - SHOULD fail the connection. */ - u8 *err = towire_errorfmt(rstate, NULL, - "Malformed wireaddrs %s in %s.", - tal_hex(tmpctx, wireaddrs), - tal_hex(tmpctx, node_ann)); + u8 *err = towire_warningfmt(rstate, NULL, + "Malformed wireaddrs %s in %s.", + tal_hex(tmpctx, wireaddrs), + tal_hex(tmpctx, node_ann)); return err; } diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index 1d1f213fb3db..15b674a7cf19 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/gossipd/test/run-bench-find_route.c @@ -98,11 +98,6 @@ char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED, void status_failed(enum status_failreason code UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "status_failed called!\n"); abort(); } -/* Generated stub for towire_errorfmt */ -u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, - const struct channel_id *channel UNNEEDED, - const char *fmt UNNEEDED, ...) -{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); } /* Generated stub for towire_gossip_store_channel_amount */ u8 *towire_gossip_store_channel_amount(const tal_t *ctx UNNEEDED, struct amount_sat satoshis UNNEEDED) { fprintf(stderr, "towire_gossip_store_channel_amount called!\n"); abort(); } @@ -115,6 +110,11 @@ u8 *towire_gossip_store_private_channel(const tal_t *ctx UNNEEDED, struct amount /* Generated stub for towire_gossip_store_private_update */ u8 *towire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const u8 *update UNNEEDED) { fprintf(stderr, "towire_gossip_store_private_update called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* Generated stub for update_peers_broadcast_index */ void update_peers_broadcast_index(struct list_head *peers UNNEEDED, u32 offset UNNEEDED) { fprintf(stderr, "update_peers_broadcast_index called!\n"); abort(); } diff --git a/gossipd/test/run-crc32_of_update.c b/gossipd/test/run-crc32_of_update.c index f9a9cd26ef6e..239354883a6e 100644 --- a/gossipd/test/run-crc32_of_update.c +++ b/gossipd/test/run-crc32_of_update.c @@ -113,17 +113,17 @@ void status_fmt(enum log_level level UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "status_fmt called!\n"); abort(); } -/* Generated stub for towire_errorfmt */ -u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, - const struct channel_id *channel UNNEEDED, - const char *fmt UNNEEDED, ...) -{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); } /* Generated stub for towire_hsmd_cupdate_sig_req */ u8 *towire_hsmd_cupdate_sig_req(const tal_t *ctx UNNEEDED, const u8 *cu UNNEEDED) { fprintf(stderr, "towire_hsmd_cupdate_sig_req called!\n"); abort(); } /* Generated stub for towire_hsmd_node_announcement_sig_req */ u8 *towire_hsmd_node_announcement_sig_req(const tal_t *ctx UNNEEDED, const u8 *announcement UNNEEDED) { fprintf(stderr, "towire_hsmd_node_announcement_sig_req called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* Generated stub for towire_wireaddr */ void towire_wireaddr(u8 **pptr UNNEEDED, const struct wireaddr *addr UNNEEDED) { fprintf(stderr, "towire_wireaddr called!\n"); abort(); } diff --git a/gossipd/test/run-extended-info.c b/gossipd/test/run-extended-info.c index 309d5b4abdf2..4400e2b44c80 100644 --- a/gossipd/test/run-extended-info.c +++ b/gossipd/test/run-extended-info.c @@ -82,11 +82,11 @@ void queue_peer_from_store(struct peer *peer UNNEEDED, /* Generated stub for queue_peer_msg */ void queue_peer_msg(struct peer *peer UNNEEDED, const u8 *msg TAKES UNNEEDED) { fprintf(stderr, "queue_peer_msg called!\n"); abort(); } -/* Generated stub for towire_errorfmt */ -u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, - const struct channel_id *channel UNNEEDED, - const char *fmt UNNEEDED, ...) -{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ void status_fmt(enum log_level level UNNEEDED, diff --git a/gossipd/test/run-find_route-specific.c b/gossipd/test/run-find_route-specific.c index e64f60af3b10..e009cdfe0967 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/gossipd/test/run-find_route-specific.c @@ -85,11 +85,6 @@ char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED, void status_failed(enum status_failreason code UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "status_failed called!\n"); abort(); } -/* Generated stub for towire_errorfmt */ -u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, - const struct channel_id *channel UNNEEDED, - const char *fmt UNNEEDED, ...) -{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); } /* Generated stub for towire_gossip_store_channel_amount */ u8 *towire_gossip_store_channel_amount(const tal_t *ctx UNNEEDED, struct amount_sat satoshis UNNEEDED) { fprintf(stderr, "towire_gossip_store_channel_amount called!\n"); abort(); } @@ -102,6 +97,11 @@ u8 *towire_gossip_store_private_channel(const tal_t *ctx UNNEEDED, struct amount /* Generated stub for towire_gossip_store_private_update */ u8 *towire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const u8 *update UNNEEDED) { fprintf(stderr, "towire_gossip_store_private_update called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* Generated stub for update_peers_broadcast_index */ void update_peers_broadcast_index(struct list_head *peers UNNEEDED, u32 offset UNNEEDED) { fprintf(stderr, "update_peers_broadcast_index called!\n"); abort(); } diff --git a/gossipd/test/run-find_route.c b/gossipd/test/run-find_route.c index 4b60544fb8fb..797ad2a35cb8 100644 --- a/gossipd/test/run-find_route.c +++ b/gossipd/test/run-find_route.c @@ -85,11 +85,6 @@ char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED, void status_failed(enum status_failreason code UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "status_failed called!\n"); abort(); } -/* Generated stub for towire_errorfmt */ -u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, - const struct channel_id *channel UNNEEDED, - const char *fmt UNNEEDED, ...) -{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); } /* Generated stub for towire_gossip_store_channel_amount */ u8 *towire_gossip_store_channel_amount(const tal_t *ctx UNNEEDED, struct amount_sat satoshis UNNEEDED) { fprintf(stderr, "towire_gossip_store_channel_amount called!\n"); abort(); } @@ -102,6 +97,11 @@ u8 *towire_gossip_store_private_channel(const tal_t *ctx UNNEEDED, struct amount /* Generated stub for towire_gossip_store_private_update */ u8 *towire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const u8 *update UNNEEDED) { fprintf(stderr, "towire_gossip_store_private_update called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* Generated stub for update_peers_broadcast_index */ void update_peers_broadcast_index(struct list_head *peers UNNEEDED, u32 offset UNNEEDED) { fprintf(stderr, "update_peers_broadcast_index called!\n"); abort(); } diff --git a/gossipd/test/run-overlong.c b/gossipd/test/run-overlong.c index 1f1ea365f488..90b24797ebd3 100644 --- a/gossipd/test/run-overlong.c +++ b/gossipd/test/run-overlong.c @@ -85,11 +85,6 @@ char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED, void status_failed(enum status_failreason code UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "status_failed called!\n"); abort(); } -/* Generated stub for towire_errorfmt */ -u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, - const struct channel_id *channel UNNEEDED, - const char *fmt UNNEEDED, ...) -{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); } /* Generated stub for towire_gossip_store_channel_amount */ u8 *towire_gossip_store_channel_amount(const tal_t *ctx UNNEEDED, struct amount_sat satoshis UNNEEDED) { fprintf(stderr, "towire_gossip_store_channel_amount called!\n"); abort(); } @@ -102,6 +97,11 @@ u8 *towire_gossip_store_private_channel(const tal_t *ctx UNNEEDED, struct amount /* Generated stub for towire_gossip_store_private_update */ u8 *towire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const u8 *update UNNEEDED) { fprintf(stderr, "towire_gossip_store_private_update called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* Generated stub for update_peers_broadcast_index */ void update_peers_broadcast_index(struct list_head *peers UNNEEDED, u32 offset UNNEEDED) { fprintf(stderr, "update_peers_broadcast_index called!\n"); abort(); } diff --git a/gossipd/test/run-txout_failure.c b/gossipd/test/run-txout_failure.c index 793adc7ae8e1..7f642967d835 100644 --- a/gossipd/test/run-txout_failure.c +++ b/gossipd/test/run-txout_failure.c @@ -91,14 +91,14 @@ void status_fmt(enum log_level level UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "status_fmt called!\n"); abort(); } -/* Generated stub for towire_errorfmt */ -u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, - const struct channel_id *channel UNNEEDED, - const char *fmt UNNEEDED, ...) -{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); } /* Generated stub for towire_gossip_store_channel_amount */ u8 *towire_gossip_store_channel_amount(const tal_t *ctx UNNEEDED, struct amount_sat satoshis UNNEEDED) { fprintf(stderr, "towire_gossip_store_channel_amount called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ /* NOOP stub for gossip_store_new */ diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 70b5e7d648a5..439f3e32fee4 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1083,9 +1083,9 @@ peer_connected_hook_deserialize(struct peer_connected_hook_payload *payload, if (json_tok_streq(buffer, t_res, "disconnect")) { payload->error = (u8*)""; if (t_err) { - payload->error = towire_errorfmt(tmpctx, NULL, "%.*s", - t_err->end - t_err->start, - buffer + t_err->start); + payload->error = towire_warningfmt(tmpctx, NULL, "%.*s", + t_err->end - t_err->start, + buffer + t_err->start); } log_debug(ld->log, "peer_connected hook rejects and says '%s'", payload->error); @@ -2238,7 +2238,8 @@ static void process_dev_forget_channel(struct bitcoind *bitcoind UNUSED, json_add_txid(response, "funding_txid", &forget->channel->funding_txid); /* Set error so we don't try to reconnect. */ - forget->channel->error = towire_errorfmt(forget->channel, NULL, + forget->channel->error = towire_errorfmt(forget->channel, + &forget->channel->cid, "dev_forget_channel"); delete_channel(forget->channel); diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 50df74b7e387..fa859916877c 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -615,6 +615,11 @@ void towire_node_id(u8 **pptr UNNEEDED, const struct node_id *id UNNEEDED) /* Generated stub for towire_onchaind_dev_memleak */ u8 *towire_onchaind_dev_memleak(const tal_t *ctx UNNEEDED) { fprintf(stderr, "towire_onchaind_dev_memleak called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* Generated stub for version */ const char *version(void) { fprintf(stderr, "version called!\n"); abort(); } diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 550a618d7ed0..4e9f10dc43c4 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -2694,13 +2694,13 @@ static u8 *handle_peer_in(struct state *state) return NULL; sync_crypto_write(state->pps, - take(towire_errorfmt(NULL, - extract_channel_id(msg, - &channel_id) ? - &channel_id : NULL, - "Unexpected message %s: %s", - peer_wire_name(t), - tal_hex(tmpctx, msg)))); + take(towire_warningfmt(NULL, + extract_channel_id(msg, + &channel_id) ? + &channel_id : NULL, + "Unexpected message %s: %s", + peer_wire_name(t), + tal_hex(tmpctx, msg)))); /* FIXME: We don't actually want master to try to send an * error, since peer is transient. This is a hack. diff --git a/openingd/openingd.c b/openingd/openingd.c index 5b5377b8eb7b..96ed7318615a 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -1153,11 +1153,11 @@ static u8 *handle_peer_in(struct state *state) return NULL; sync_crypto_write(state->pps, - take(towire_errorfmt(NULL, - extract_channel_id(msg, &channel_id) ? &channel_id : NULL, - "Unexpected message %s: %s", - peer_wire_name(t), - tal_hex(tmpctx, msg)))); + take(towire_warningfmt(NULL, + extract_channel_id(msg, &channel_id) ? &channel_id : NULL, + "Unexpected message %s: %s", + peer_wire_name(t), + tal_hex(tmpctx, msg)))); /* FIXME: We don't actually want master to try to send an * error, since peer is transient. This is a hack. @@ -1179,20 +1179,18 @@ static void handle_gossip_in(struct state *state) handle_gossip_msg(state->pps, take(msg)); } -/*~ Is this message of type `error` with the special zero-id - * "fail-everything"? If lightningd asked us to send such a thing, we're - * done. */ -static void fail_if_all_error(const u8 *inner) +/*~ Is this message of a `warning` or `error`? If lightningd asked us to send + * such a thing, it wants to close the connection. */ +static void fail_if_warning_or_error(const u8 *inner) { struct channel_id channel_id; u8 *data; - if (!fromwire_error(tmpctx, inner, &channel_id, &data) - || !channel_id_is_all(&channel_id)) { + if (!fromwire_warning(tmpctx, inner, &channel_id, &data) + && !fromwire_error(tmpctx, inner, &channel_id, &data)) return; - } - status_info("Master said send err: %s", + status_info("Master said send %s", sanitize_error(tmpctx, inner, NULL)); exit(0); } @@ -1358,10 +1356,10 @@ int main(int argc, char *argv[]) per_peer_state_set_fds(state->pps, 3, 4, 5); /*~ If lightningd wanted us to send a msg, do so before we waste time - * doing work. If it's a global error, we'll close immediately. */ + * doing work. If it's a warning, we'll close immediately. */ if (inner != NULL) { sync_crypto_write(state->pps, inner); - fail_if_all_error(inner); + fail_if_warning_or_error(inner); tal_free(inner); } diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 64f10b3785f6..7483fa638352 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -424,7 +424,7 @@ def test_plugin_connected_hook_chaining(node_factory): ]) # FIXME: this error occurs *after* connection, so we connect then drop. - l3.daemon.wait_for_log(r"chan#1: peer_in WIRE_ERROR") + l3.daemon.wait_for_log(r"chan#1: peer_in WIRE_WARNING") l3.daemon.wait_for_log(r"You are in reject list") def check_disconnect(): diff --git a/wallet/db_postgres_sqlgen.c b/wallet/db_postgres_sqlgen.c index b381335c38a3..de5f0251909e 100644 --- a/wallet/db_postgres_sqlgen.c +++ b/wallet/db_postgres_sqlgen.c @@ -1786,4 +1786,4 @@ struct db_query db_postgres_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */ -// SHA256STAMP:e7f5fdbc96b1e28c14a395767b8c9a29ccf9a9773c69e2bf24b77b31f11ee4b6 +// SHA256STAMP:d99c7d86df2c57de4dad0ae207bb71ac7245e1beb3bee1066ab5d947db9b1e5c diff --git a/wallet/db_sqlite3_sqlgen.c b/wallet/db_sqlite3_sqlgen.c index 5ee3b99ba6d2..7e70f34019c3 100644 --- a/wallet/db_sqlite3_sqlgen.c +++ b/wallet/db_sqlite3_sqlgen.c @@ -1786,4 +1786,4 @@ struct db_query db_sqlite3_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */ -// SHA256STAMP:e7f5fdbc96b1e28c14a395767b8c9a29ccf9a9773c69e2bf24b77b31f11ee4b6 +// SHA256STAMP:d99c7d86df2c57de4dad0ae207bb71ac7245e1beb3bee1066ab5d947db9b1e5c diff --git a/wallet/statements_gettextgen.po b/wallet/statements_gettextgen.po index 8b44dff0d9ab..664430b4906b 100644 --- a/wallet/statements_gettextgen.po +++ b/wallet/statements_gettextgen.po @@ -1174,7 +1174,7 @@ msgstr "" msgid "not a valid SQL statement" msgstr "" -#: wallet/test/run-wallet.c:1390 +#: wallet/test/run-wallet.c:1395 msgid "INSERT INTO channels (id) VALUES (1);" msgstr "" -# SHA256STAMP:95ea4c16d30a6f6bc7c6f9651cc2312a0e3b299ecd58f3a4ed48d89f2bebacce +# SHA256STAMP:878611b064230015bd80f43f4d7acde6bbf1c468da09bc8621898ff6718308e6 diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 147eb5ef0b90..04d23ebf92a1 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -792,6 +792,11 @@ u8 *towire_temporary_node_failure(const tal_t *ctx UNNEEDED) /* Generated stub for towire_unknown_next_peer */ u8 *towire_unknown_next_peer(const tal_t *ctx UNNEEDED) { fprintf(stderr, "towire_unknown_next_peer called!\n"); abort(); } +/* Generated stub for towire_warningfmt */ +u8 *towire_warningfmt(const tal_t *ctx UNNEEDED, + const struct channel_id *channel UNNEEDED, + const char *fmt UNNEEDED, ...) +{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } /* Generated stub for watch_txid */ struct txwatch *watch_txid(const tal_t *ctx UNNEEDED, struct chain_topology *topo UNNEEDED, From f8092abcec9456dc747178ca4ebee44fab816ab2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 3 Feb 2021 14:41:09 +1030 Subject: [PATCH 7/7] pytest: detect warnings, too. Since we turned many errors into warnings, we want our tests to fail when they happen unexpectedly. We make WARNING clear in the strings we print, too, to help out. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/fixtures.py | 1 + contrib/pyln-testing/pyln/testing/utils.py | 9 +++++++-- lightningd/peer_control.c | 2 +- tests/test_closing.py | 11 ++++++++-- tests/test_connection.py | 6 ++++-- tests/test_misc.py | 2 +- tests/test_pay.py | 5 +++-- tests/test_plugin.py | 20 +++++++++++++------ 8 files changed, 40 insertions(+), 16 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/fixtures.py b/contrib/pyln-testing/pyln/testing/fixtures.py index c02844890957..6f1d96b0e482 100644 --- a/contrib/pyln-testing/pyln/testing/fixtures.py +++ b/contrib/pyln-testing/pyln/testing/fixtures.py @@ -229,6 +229,7 @@ def map_node_error(nodes, f, msg): map_node_error(nf.nodes, printValgrindErrors, "reported valgrind errors") map_node_error(nf.nodes, printCrashLog, "had crash.log files") map_node_error(nf.nodes, lambda n: not n.allow_broken_log and n.daemon.is_in_log(r'\*\*BROKEN\*\*'), "had BROKEN messages") + map_node_error(nf.nodes, lambda n: not n.allow_warning and n.daemon.is_in_log(r' WARNING:'), "had warning messages") map_node_error(nf.nodes, checkReconnect, "had unexpected reconnections") map_node_error(nf.nodes, checkBadGossip, "had bad gossip messages") map_node_error(nf.nodes, lambda n: n.daemon.is_in_log('Bad reestablish'), "had bad reestablish") diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index 905cd12622af..2f5d40f5cda9 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -618,8 +618,11 @@ def call(self, method, payload=None): class LightningNode(object): def __init__(self, node_id, lightning_dir, bitcoind, executor, valgrind, may_fail=False, - may_reconnect=False, allow_broken_log=False, - allow_bad_gossip=False, db=None, port=None, disconnect=None, random_hsm=None, options=None, + may_reconnect=False, + allow_broken_log=False, + allow_warning=False, + allow_bad_gossip=False, + db=None, port=None, disconnect=None, random_hsm=None, options=None, **kwargs): self.bitcoin = bitcoind self.executor = executor @@ -627,6 +630,7 @@ def __init__(self, node_id, lightning_dir, bitcoind, executor, valgrind, may_fai self.may_reconnect = may_reconnect self.allow_broken_log = allow_broken_log self.allow_bad_gossip = allow_bad_gossip + self.allow_warning = allow_warning self.db = db # Assume successful exit @@ -1203,6 +1207,7 @@ def split_options(self, opts): 'disconnect', 'may_fail', 'allow_broken_log', + 'allow_warning', 'may_reconnect', 'random_hsm', 'feerates', diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 439f3e32fee4..7f87bf549256 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -396,7 +396,7 @@ void channel_errmsg(struct channel *channel, * 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_fail_reconnect_later(channel, "%s WARNING: %s", channel->owner->name, desc); return; } diff --git a/tests/test_closing.py b/tests/test_closing.py index 42d37b04b835..b5376211fbcf 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -1784,7 +1784,11 @@ def test_listfunds_after_their_unilateral(node_factory, bitcoind): Make sure we show the address. """ coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') - l1, l2 = node_factory.line_graph(2, opts={'plugin': coin_mvt_plugin}) + # FIXME: We can get warnings from unilteral changes, since we treat + # such errors a soft because LND. + l1, l2 = node_factory.line_graph(2, opts=[{'plugin': coin_mvt_plugin, + "allow_warning": True}, + {'plugin': coin_mvt_plugin}]) channel_id = first_channel_id(l1, l2) # listfunds will show 1 output change, and channels. @@ -2523,7 +2527,10 @@ def test_shutdown(node_factory): @flaky @unittest.skipIf(not DEVELOPER, "needs to set upfront_shutdown_script") def test_option_upfront_shutdown_script(node_factory, bitcoind, executor): - l1 = node_factory.get_node(start=False) + # There's a workaround in channeld, that it treats incoming errors + # before both sides are locked in as warnings; this happens in + # this test, so l1 reports the error as a warning! + l1 = node_factory.get_node(start=False, allow_warning=True) # Insist on upfront script we're not going to match. l1.daemon.env["DEV_OPENINGD_UPFRONT_SHUTDOWN_SCRIPT"] = "76a91404b61f7dc1ea0dc99424464cc4064dc564d91e8988ac" l1.start() diff --git a/tests/test_connection.py b/tests/test_connection.py index d04fddde1911..88a57fa351bd 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1956,8 +1956,10 @@ def test_update_fee(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_fee_limits(node_factory, bitcoind): - l1, l2, l3, l4 = node_factory.get_nodes(4, opts=[{'dev-max-fee-multiplier': 5, 'may_reconnect': True}, - {'dev-max-fee-multiplier': 5, 'may_reconnect': True}, + l1, l2, l3, l4 = node_factory.get_nodes(4, opts=[{'dev-max-fee-multiplier': 5, 'may_reconnect': True, + 'allow_warning': True}, + {'dev-max-fee-multiplier': 5, 'may_reconnect': True, + 'allow_warning': True}, {'ignore-fee-limits': True, 'may_reconnect': True}, {}]) diff --git a/tests/test_misc.py b/tests/test_misc.py index f9e634f29829..1c35ff829758 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1336,7 +1336,7 @@ def test_bitcoind_goes_backwards(node_factory, bitcoind): @flaky def test_reserve_enforcement(node_factory, executor): """Channeld should disallow you spending into your reserve""" - l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True}) + l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True, 'allow_warning': True}) # Pay 1000 satoshi to l2. l1.pay(l2, 1000000) diff --git a/tests/test_pay.py b/tests/test_pay.py index 9f15a0db4bc4..0a2c36da4a92 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -234,7 +234,8 @@ def test_pay0(node_factory): def test_pay_disconnect(node_factory, bitcoind): """If the remote node has disconnected, we fail payment, but can try again when it reconnects""" l1, l2 = node_factory.line_graph(2, opts={'dev-max-fee-multiplier': 5, - 'may_reconnect': True}) + 'may_reconnect': True, + 'allow_warning': True}) # Dummy payment to kick off update_fee messages l1.pay(l2, 1000) @@ -261,7 +262,7 @@ def test_pay_disconnect(node_factory, bitcoind): l1.set_feerates((10**6, 1000**6, 1000**6, 1000**6), False) # Wait for l1 notice - l1.daemon.wait_for_log(r'Peer transient failure in CHANNELD_NORMAL: channeld: .*: update_fee \d+ outside range 1875-75000') + l1.daemon.wait_for_log(r'Peer transient failure in CHANNELD_NORMAL: channeld WARNING: .*: update_fee \d+ outside range 1875-75000') # Make l2 fail hard. l2.rpc.close(l1.info['id'], unilateraltimeout=1) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 7483fa638352..e0e9d86d844d 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -398,11 +398,16 @@ def test_plugin_connected_hook_chaining(node_factory): l1 is configured to accept connections from l2, but not from l3. we check that logger_a is always called and logger_b only for l2. """ - opts = [{'plugin': [ - os.path.join(os.getcwd(), 'tests/plugins/peer_connected_logger_a.py'), - os.path.join(os.getcwd(), 'tests/plugins/reject.py'), - os.path.join(os.getcwd(), 'tests/plugins/peer_connected_logger_b.py'), - ]}, {}, {}] + opts = [{'plugin': + [os.path.join(os.getcwd(), + 'tests/plugins/peer_connected_logger_a.py'), + os.path.join(os.getcwd(), + 'tests/plugins/reject.py'), + os.path.join(os.getcwd(), + 'tests/plugins/peer_connected_logger_b.py')], + 'allow_warning': True}, + {}, + {'allow_warning': True}] l1, l2, l3 = node_factory.get_nodes(3, opts=opts) l2id = l2.info['id'] @@ -822,7 +827,10 @@ def test_channel_state_changed_unilateral(node_factory, bitcoind): The misc_notifications.py plugin logs `channel_state_changed` events. """ - opts = {"plugin": os.path.join(os.getcwd(), "tests/plugins/misc_notifications.py")} + # FIXME: We can get warnings from unilteral changes, since we treat + # such errors a soft because LND. + opts = {"plugin": os.path.join(os.getcwd(), "tests/plugins/misc_notifications.py"), + "allow_warning": True} l1, l2 = node_factory.line_graph(2, opts=opts) l1_id = l1.rpc.getinfo()["id"]