diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/RequestManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/RequestManager.java index cefcdab1ff..17028726c4 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/RequestManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/RequestManager.java @@ -79,12 +79,12 @@ public class RequestManager { Optional.ofNullable(responseStreams.get(requestIdAndEthMessage.getKey())) .ifPresentOrElse( responseStream -> responseStream.processMessage(requestIdAndEthMessage.getValue()), - // disconnect on incorrect requestIds + // Consider incorrect requestIds to be a useless response; too + // many of these and we will disconnect. () -> { - LOG.debug( - "Request ID incorrect (BREACH_OF_PROTOCOL), disconnecting peer {}", peer); - peer.disconnect(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL); + peer.recordUselessResponse("Request ID incorrect"); }); + } else { // otherwise iterate through all of them streams.forEach(stream -> stream.processMessage(ethMessage.getData())); diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java index 44d783fa39..dee16a5b29 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java @@ -494,7 +494,8 @@ public class DefaultP2PNetwork implements P2PNetwork { // Set up permissions // Fold peer reputation into permissions final PeerPermissionsDenylist misbehavingPeers = PeerPermissionsDenylist.create(500); - final PeerDenylistManager reputationManager = new PeerDenylistManager(misbehavingPeers); + final PeerDenylistManager reputationManager = + new PeerDenylistManager(misbehavingPeers, maintainedPeers); peerPermissions = PeerPermissions.combine(peerPermissions, misbehavingPeers); final MutableLocalNode localNode = diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManager.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManager.java index 17d1d11fc6..bf98a0bb3f 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManager.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManager.java @@ -14,6 +14,7 @@ */ package org.hyperledger.besu.ethereum.p2p.network; +import org.hyperledger.besu.ethereum.p2p.peers.MaintainedPeers; import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissionsDenylist; import org.hyperledger.besu.ethereum.p2p.rlpx.DisconnectCallback; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; @@ -36,8 +37,12 @@ public class PeerDenylistManager implements DisconnectCallback { private final PeerPermissionsDenylist denylist; - public PeerDenylistManager(final PeerPermissionsDenylist denylist) { + private final MaintainedPeers maintainedPeers; + + public PeerDenylistManager( + final PeerPermissionsDenylist denylist, final MaintainedPeers maintainedPeers) { this.denylist = denylist; + this.maintainedPeers = maintainedPeers; } @Override @@ -46,8 +51,15 @@ public class PeerDenylistManager implements DisconnectCallback { final DisconnectReason reason, final boolean initiatedByPeer) { if (shouldBlock(reason, initiatedByPeer)) { - LOG.debug("Added peer {} to peer denylist for reason {}", connection, reason.name()); - denylist.add(connection.getPeer()); + if (maintainedPeers.contains(connection.getPeer())) { + LOG.debug( + "Skip adding maintained peer {} to peer denylist for reason {}", + connection, + reason.name()); + } else { + LOG.debug("Added peer {} to peer denylist for reason {}", connection, reason.name()); + denylist.add(connection.getPeer()); + } } } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManagerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManagerTest.java index 8ebd40e6a7..38435d9d53 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManagerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManagerTest.java @@ -20,6 +20,7 @@ import static org.mockito.Mockito.when; import org.hyperledger.besu.ethereum.p2p.peers.DefaultPeer; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl; +import org.hyperledger.besu.ethereum.p2p.peers.MaintainedPeers; import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissions; import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissionsDenylist; @@ -34,10 +35,11 @@ public class PeerDenylistManagerTest { private final Peer localNode = generatePeer(); private final PeerDenylistManager peerDenylistManager; private final PeerPermissionsDenylist denylist; + private final MaintainedPeers maintainedPeers = new MaintainedPeers(); public PeerDenylistManagerTest() { denylist = PeerPermissionsDenylist.create(); - peerDenylistManager = new PeerDenylistManager(denylist); + peerDenylistManager = new PeerDenylistManager(denylist, maintainedPeers); } @Test @@ -69,6 +71,16 @@ public class PeerDenylistManagerTest { checkPermissions(denylist, peer.getPeer(), true); } + @Test + public void doesNotDenylistMaintainedPeer() { + final PeerConnection peer = generatePeerConnection(); + maintainedPeers.add(peer.getPeer()); + + checkPermissions(denylist, peer.getPeer(), true); + peerDenylistManager.onDisconnect(peer, DisconnectReason.BREACH_OF_PROTOCOL, false); + checkPermissions(denylist, peer.getPeer(), true); + } + @Test public void denylistIncompatiblePeer() { final PeerConnection peer = generatePeerConnection();