diff --git a/p2p/router.go b/p2p/router.go index 418bc004c..7158464a0 100644 --- a/p2p/router.go +++ b/p2p/router.go @@ -252,6 +252,19 @@ func (r *Router) acceptPeers(transport Transport) { _ = conn.Close() }() + // FIXME: Because we do the handshake in each transport, rather than + // here in the Router, the remote peer will think they've + // successfully connected and start sending us messages, although we + // can end up rejecting the connection here. This can e.g. cause + // problems in tests, where because of race conditions a + // disconnection can cause the local node to immediately redial, + // while the remote node may not have completed the disconnection + // registration yet and reject the accept below. + // + // The Router should do the handshake, and we should check with the + // peer manager before completing the handshake -- this probably + // requires protocol changes to send an additional message when the + // handshake is accepted. peerID := conn.NodeInfo().NodeID if err := r.peerManager.Accepted(peerID); err != nil { r.logger.Error("failed to accept connection", "peer", peerID, "err", err) diff --git a/p2p/router_test.go b/p2p/router_test.go index dfe1e9689..c42ef72d3 100644 --- a/p2p/router_test.go +++ b/p2p/router_test.go @@ -112,6 +112,16 @@ func TestRouter(t *testing.T) { }, (<-channel.In()).Strip()) } + // We now send a broadcast, which we should return back from all peers. + channel.Out() <- p2p.Envelope{ + Broadcast: true, + Message: &TestMessage{Value: "broadcast"}, + } + for i := 0; i < len(peers); i++ { + envelope := <-channel.In() + require.Equal(t, &TestMessage{Value: "broadcast"}, envelope.Message) + } + // We then submit an error for a peer, and watch it get disconnected. channel.Error() <- p2p.PeerError{ PeerID: peers[0].ID, @@ -131,14 +141,4 @@ func TestRouter(t *testing.T) { PeerID: peers[0].ID, Status: p2p.PeerStatusUp, }, peerUpdate) - - // We now send a broadcast, which we should return back from all peers. - channel.Out() <- p2p.Envelope{ - Broadcast: true, - Message: &TestMessage{Value: "broadcast"}, - } - for i := 0; i < len(peers); i++ { - envelope := <-channel.In() - require.Equal(t, &TestMessage{Value: "broadcast"}, envelope.Message) - } }