diff --git a/eth/handler.go b/eth/handler.go index 2c6b980ff1..aec5c4039d 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -343,7 +343,7 @@ func (h *handler) runEthPeer(peer *eth.Peer, handler eth.Handler) error { return err } - defer h.removePeer(peer.ID()) + defer h.unregisterPeer(peer.ID()) // Quorum: changed by https://github.com/bnb-chain/bsc/pull/856 p := h.peers.peer(peer.ID()) if p == nil { @@ -415,9 +415,19 @@ func (h *handler) runSnapExtension(peer *snap.Peer, handler snap.Handler) error return handler(peer) } -// removePeer unregisters a peer from the downloader and fetchers, removes it from -// the set of tracked peers and closes the network connection to it. +// removePeer requests disconnection of a peer. +// Quorum: added by https://github.com/bnb-chain/bsc/pull/856 func (h *handler) removePeer(id string) { + peer := h.peers.peer(id) + if peer != nil { + // Hard disconnect at the networking layer. Handler will get an EOF and terminate the peer. defer unregisterPeer will do the cleanup task after then. + peer.Peer.Disconnect(p2p.DiscUselessPeer) + } +} + +// unregisterPeer removes a peer from the downloader, fetchers and main peer set. +// Quorum: changed by https://github.com/bnb-chain/bsc/pull/856 +func (h *handler) unregisterPeer(id string) { // Create a custom logger to avoid printing the entire id var logger log.Logger if len(id) < 16 { @@ -446,7 +456,7 @@ func (h *handler) removePeer(id string) { logger.Error("Ethereum peer removal failed", "err", err) } // Hard disconnect at the networking layer - peer.Peer.Disconnect(p2p.DiscUselessPeer) + // peer.Peer.Disconnect(p2p.DiscUselessPeer) // Quorum: removed by https://github.com/bnb-chain/bsc/pull/856 } func (h *handler) Start(maxPeers int) { diff --git a/eth/handler_eth_test.go b/eth/handler_eth_test.go index 517eb3bdee..370a68d7c9 100644 --- a/eth/handler_eth_test.go +++ b/eth/handler_eth_test.go @@ -614,14 +614,19 @@ func testCheckpointChallenge(t *testing.T, syncmode downloader.SyncMode, checkpo defer p2pLocal.Close() defer p2pRemote.Close() - local := eth.NewPeer(eth.ETH64, p2p.NewPeer(enode.ID{1}, "", nil), p2pLocal, handler.txpool) - remote := eth.NewPeer(eth.ETH64, p2p.NewPeer(enode.ID{2}, "", nil), p2pRemote, handler.txpool) + local := eth.NewPeer(eth.ETH64, p2p.NewPeerPipe(enode.ID{1}, "", nil, p2pLocal), p2pLocal, handler.txpool) + remote := eth.NewPeer(eth.ETH64, p2p.NewPeerPipe(enode.ID{2}, "", nil, p2pRemote), p2pRemote, handler.txpool) defer local.Close() defer remote.Close() - go handler.handler.runEthPeer(local, func(peer *eth.Peer) error { - return eth.Handle((*ethHandler)(handler.handler), peer) - }) + handlerDone := make(chan struct{}) + go func() { + defer close(handlerDone) + handler.handler.runEthPeer(local, func(peer *eth.Peer) error { + err := eth.Handle((*ethHandler)(handler.handler), peer) + return err + }) + }() // Run the handshake locally to avoid spinning up a remote handler var ( genesis = handler.chain.Genesis() @@ -658,6 +663,7 @@ func testCheckpointChallenge(t *testing.T, syncmode downloader.SyncMode, checkpo // Verify that the remote peer is maintained or dropped if drop { + <-handlerDone if peers := handler.handler.peers.len(); peers != 0 { t.Fatalf("peer count mismatch: have %d, want %d", peers, 0) } diff --git a/p2p/peer.go b/p2p/peer.go index 83de566c26..fb39fc2847 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -118,6 +118,7 @@ type Peer struct { events *event.Feed // Quorum + testPipe *MsgPipeRW // for testing EthPeerRegistered chan struct{} EthPeerDisconnected chan struct{} } @@ -189,6 +190,10 @@ func (p *Peer) LocalAddr() net.Addr { // Disconnect terminates the peer connection with the given reason. // It returns immediately and does not wait until the connection is closed. func (p *Peer) Disconnect(reason DiscReason) { + if p.testPipe != nil { + p.testPipe.Close() + } + select { case p.disc <- reason: case <-p.closed: @@ -528,3 +533,14 @@ func (p *Peer) Info() *PeerInfo { } return info } + +// Quorum + +// NewPeerPipe creates a peer for testing purposes. +// The message pipe given as the last parameter is closed when +// Disconnect is called on the peer. +func NewPeerPipe(id enode.ID, name string, caps []Cap, pipe *MsgPipeRW) *Peer { + p := NewPeer(id, name, caps) + p.testPipe = pipe + return p +}