From b223a6acbb47f32ca509a30a05afb0e5141a11cf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 15 Jun 2021 06:39:51 +0930 Subject: [PATCH] common/read_peer_msg: don't try to handle reestablish/reopen. Let the callers do that (only channeld needs to do this). We temporarily send an error on unknown reestablish in openingd, as this mimic previous behavior and avoids breaking tests (it does leave a BROKEN message in the logs though, so test_funding_external_wallet_corners needs to ignore that for now. Signed-off-by: Rusty Russell --- channeld/channeld.c | 9 +++++++++ common/read_peer_msg.c | 13 ------------- common/read_peer_msg.h | 4 ++-- openingd/openingd.c | 21 +++++++++++++++------ tests/test_connection.py | 4 ++-- 5 files changed, 28 insertions(+), 23 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 305de7e9e..c655aae34 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2668,6 +2668,15 @@ static void peer_reconnect(struct peer *peer, tal_hex(msg, msg)); } + if (!channel_id_eq(&channel_id, &peer->channel_id)) { + peer_failed_err(peer->pps, + &channel_id, + "bad reestablish msg for unknown channel %s: %s", + type_to_string(tmpctx, struct channel_id, + &channel_id), + tal_hex(msg, msg)); + } + status_debug("Got reestablish commit=%"PRIu64" revoke=%"PRIu64, next_commitment_number, next_revocation_number); diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index 2215c1064..6859be7cd 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -162,7 +162,6 @@ bool handle_peer_gossip_or_error(struct per_peer_state *pps, { char *err; bool warning; - struct channel_id actual; #if DEVELOPER /* Any odd-typed unknown message is handled by the caller, so if we @@ -200,18 +199,6 @@ bool handle_peer_gossip_or_error(struct per_peer_state *pps, goto handled; } - /* They're talking about a different channel? */ - if (is_wrong_channel(msg, channel_id, &actual)) { - status_debug("Rejecting %s for unknown channel_id %s", - peer_wire_name(fromwire_peektype(msg)), - type_to_string(tmpctx, struct channel_id, &actual)); - sync_crypto_write(pps, - take(towire_errorfmt(NULL, &actual, - "Multiple channels" - " unsupported"))); - goto handled; - } - return false; handled: diff --git a/common/read_peer_msg.h b/common/read_peer_msg.h index 824c2ba3d..698a11976 100644 --- a/common/read_peer_msg.h +++ b/common/read_peer_msg.h @@ -62,8 +62,8 @@ bool is_wrong_channel(const u8 *msg, const struct channel_id *expected, * @msg: the peer message (only taken if returns true). * * This returns true if it handled the packet: a gossip packet (forwarded - * to gossipd), an error packet (causes peer_failed_received_errmsg or - * ignored), or a message about the wrong channel (sends sync error reply). + * to gossipd), or an error packet (causes peer_failed_received_errmsg or + * ignored). */ bool handle_peer_gossip_or_error(struct per_peer_state *pps, const struct channel_id *channel_id, diff --git a/openingd/openingd.c b/openingd/openingd.c index 7a0d1bdee..03f34059f 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -1170,12 +1170,21 @@ static u8 *handle_peer_in(struct state *state) &state->channel_id, false, msg)) return NULL; - sync_crypto_write(state->pps, - 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)))); + if (extract_channel_id(msg, &channel_id)) { + sync_crypto_write(state->pps, + take(towire_errorfmt(NULL, + &channel_id, + "Unexpected message %s: %s", + peer_wire_name(t), + tal_hex(tmpctx, msg)))); + } else { + sync_crypto_write(state->pps, + take(towire_warningfmt(NULL, + 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/tests/test_connection.py b/tests/test_connection.py index 4f808af91..09b6c429f 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1149,7 +1149,7 @@ def test_funding_by_utxos(node_factory, bitcoind): @pytest.mark.developer("needs dev_forget_channel") @pytest.mark.openchannel('v1') def test_funding_external_wallet_corners(node_factory, bitcoind): - l1 = node_factory.get_node(may_reconnect=True) + l1 = node_factory.get_node(may_reconnect=True, allow_broken_log=True) l2 = node_factory.get_node(may_reconnect=True) amount = 2**24 @@ -1242,7 +1242,7 @@ def test_funding_external_wallet_corners(node_factory, bitcoind): # on reconnect, channel should get destroyed l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.daemon.wait_for_log('Rejecting WIRE_CHANNEL_REESTABLISH for unknown channel_id') + l1.daemon.wait_for_log('Unexpected message WIRE_CHANNEL_REESTABLISH') wait_for(lambda: len(l1.rpc.listpeers()['peers']) == 0) wait_for(lambda: len(l2.rpc.listpeers()['peers']) == 0)