Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Negotiate close even if we already consider the channel closed. #4559

Merged
Prev Previous commit
Next Next commit
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 <rusty@rustcorp.com.au>
  • Loading branch information
rustyrussell committed Jun 14, 2021
commit b09ac0eb7d8e0d18d425ead5518a26192f9cacfb
9 changes: 9 additions & 0 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 0 additions & 13 deletions common/read_peer_msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions common/read_peer_msg.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 15 additions & 6 deletions openingd/openingd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down