test: fix flaky router broadcast test (#6006)

Fixes #6004 by reordering test to avoid race condition. Will redesign router tests to be resistant to this later.

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)

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)
-	}
 }