diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/config/RlpxConfiguration.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/config/RlpxConfiguration.java index 4b71f38011..fa7fd1358d 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/config/RlpxConfiguration.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/config/RlpxConfiguration.java @@ -85,20 +85,12 @@ public class RlpxConfiguration { return this; } - public boolean isLimitRemoteWireConnectionsEnabled() { - return limitRemoteWireConnectionsEnabled; - } - public RlpxConfiguration setLimitRemoteWireConnectionsEnabled( final boolean limitRemoteWireConnectionsEnabled) { this.limitRemoteWireConnectionsEnabled = limitRemoteWireConnectionsEnabled; return this; } - public double getFractionRemoteWireConnectionsAllowed() { - return fractionRemoteWireConnectionsAllowed; - } - public RlpxConfiguration setFractionRemoteWireConnectionsAllowed( final double fractionRemoteWireConnectionsAllowed) { checkState( @@ -108,6 +100,14 @@ public class RlpxConfiguration { return this; } + public int getMaxRemotelyInitiatedConnections() { + if (!limitRemoteWireConnectionsEnabled) { + return maxPeers; + } + + return (int) Math.floor(maxPeers * fractionRemoteWireConnectionsAllowed); + } + @Override public boolean equals(final Object o) { if (this == o) { diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java index 24036efc36..bc58d90cc5 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java @@ -23,13 +23,13 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryAgent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; import tech.pegasys.pantheon.ethereum.p2p.discovery.VertxPeerDiscoveryAgent; -import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeerProperties; +import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeerPrivileges; import tech.pegasys.pantheon.ethereum.p2p.peers.EnodeURL; import tech.pegasys.pantheon.ethereum.p2p.peers.LocalNode; import tech.pegasys.pantheon.ethereum.p2p.peers.MaintainedPeers; import tech.pegasys.pantheon.ethereum.p2p.peers.MutableLocalNode; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerProperties; +import tech.pegasys.pantheon.ethereum.p2p.peers.PeerPrivileges; import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissionsBlacklist; import tech.pegasys.pantheon.ethereum.p2p.rlpx.ConnectCallback; @@ -424,9 +424,9 @@ public class DefaultP2PNetwork implements P2PNetwork { final MutableLocalNode localNode = MutableLocalNode.create(config.getRlpx().getClientId(), 5, supportedCapabilities); - final PeerProperties peerProperties = new DefaultPeerProperties(maintainedPeers); + final PeerPrivileges peerPrivileges = new DefaultPeerPrivileges(maintainedPeers); peerDiscoveryAgent = peerDiscoveryAgent == null ? createDiscoveryAgent() : peerDiscoveryAgent; - rlpxAgent = rlpxAgent == null ? createRlpxAgent(localNode, peerProperties) : rlpxAgent; + rlpxAgent = rlpxAgent == null ? createRlpxAgent(localNode, peerPrivileges) : rlpxAgent; return new DefaultP2PNetwork( localNode, @@ -457,12 +457,12 @@ public class DefaultP2PNetwork implements P2PNetwork { } private RlpxAgent createRlpxAgent( - final LocalNode localNode, final PeerProperties peerProperties) { + final LocalNode localNode, final PeerPrivileges peerPrivileges) { return RlpxAgent.builder() .keyPair(keyPair) .config(config.getRlpx()) .peerPermissions(peerPermissions) - .peerProperties(peerProperties) + .peerPrivileges(peerPrivileges) .localNode(localNode) .metricsSystem(metricsSystem) .build(); diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/DefaultPeerProperties.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/DefaultPeerPrivileges.java similarity index 76% rename from ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/DefaultPeerProperties.java rename to ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/DefaultPeerPrivileges.java index 63b67f4524..d6b76cb396 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/DefaultPeerProperties.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/DefaultPeerPrivileges.java @@ -12,15 +12,15 @@ */ package tech.pegasys.pantheon.ethereum.p2p.peers; -public class DefaultPeerProperties implements PeerProperties { - final MaintainedPeers maintainedPeers; +public class DefaultPeerPrivileges implements PeerPrivileges { + private final MaintainedPeers maintainedPeers; - public DefaultPeerProperties(final MaintainedPeers maintainedPeers) { + public DefaultPeerPrivileges(final MaintainedPeers maintainedPeers) { this.maintainedPeers = maintainedPeers; } @Override - public boolean ignoreMaxPeerLimits(final Peer peer) { + public boolean canExceedConnectionLimits(final Peer peer) { return maintainedPeers.contains(peer); } } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerProperties.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerPrivileges.java similarity index 78% rename from ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerProperties.java rename to ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerPrivileges.java index d68d7a7959..bd2ebf1c9c 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerProperties.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerPrivileges.java @@ -12,14 +12,14 @@ */ package tech.pegasys.pantheon.ethereum.p2p.peers; -public interface PeerProperties { +public interface PeerPrivileges { /** - * If true, the given peer can connect or remain connected even if the max connection limit has - * been reached or exceeded. + * If true, the given peer can connect or remain connected even if the max connection limit or the + * maximum remote connection limit has been reached or exceeded. * * @param peer The peer to be checked. - * @return {@code true} if the peer should be allowed to connect regardless of max peer limits. + * @return {@code true} if the peer should be allowed to connect regardless of connection limits. */ - boolean ignoreMaxPeerLimits(final Peer peer); + boolean canExceedConnectionLimits(final Peer peer); } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/RlpxAgent.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/RlpxAgent.java index 62c1e9f9ec..118a644873 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/RlpxAgent.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/RlpxAgent.java @@ -22,7 +22,7 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.EnodeURL; import tech.pegasys.pantheon.ethereum.p2p.peers.LocalNode; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerProperties; +import tech.pegasys.pantheon.ethereum.p2p.peers.PeerPrivileges; import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; import tech.pegasys.pantheon.ethereum.p2p.rlpx.connections.ConnectionInitializer; import tech.pegasys.pantheon.ethereum.p2p.rlpx.connections.PeerConnection; @@ -59,20 +59,18 @@ public class RlpxAgent { private static final Logger LOG = LogManager.getLogger(); private final LocalNode localNode; - private final PeerRlpxPermissions peerPermissions; private final PeerConnectionEvents connectionEvents; - private final Subscribers connectSubscribers = Subscribers.create(); private final ConnectionInitializer connectionInitializer; - private final boolean limitRemoteWireConnectionsEnabled; - private final double fractionRemoteConnectionsAllowed; + private final Subscribers connectSubscribers = Subscribers.create(); - private final int maxPeers; + private final PeerRlpxPermissions peerPermissions; + private final PeerPrivileges peerPrivileges; + private final int maxConnections; + private final int maxRemotelyInitiatedConnections; @VisibleForTesting final Map connectionsById = new ConcurrentHashMap<>(); - private final PeerProperties peerProperties; - private final AtomicBoolean started = new AtomicBoolean(false); private final AtomicBoolean stopped = new AtomicBoolean(false); @@ -80,22 +78,21 @@ public class RlpxAgent { private RlpxAgent( final LocalNode localNode, - final PeerRlpxPermissions peerPermissions, final PeerConnectionEvents connectionEvents, final ConnectionInitializer connectionInitializer, - final int maxPeers, - final PeerProperties peerProperties, - final MetricsSystem metricsSystem, - final boolean limitRemoteWireConnectionsEnabled, - final double fractionRemoteConnectionsAllowed) { - this.maxPeers = maxPeers; - this.peerProperties = peerProperties; + final PeerRlpxPermissions peerPermissions, + final PeerPrivileges peerPrivileges, + final int maxConnections, + final int maxRemotelyInitiatedConnections, + final MetricsSystem metricsSystem) { this.localNode = localNode; - this.peerPermissions = peerPermissions; this.connectionEvents = connectionEvents; this.connectionInitializer = connectionInitializer; - this.limitRemoteWireConnectionsEnabled = limitRemoteWireConnectionsEnabled; - this.fractionRemoteConnectionsAllowed = fractionRemoteConnectionsAllowed; + this.peerPermissions = peerPermissions; + this.peerPrivileges = peerPrivileges; + this.maxConnections = maxConnections; + this.maxRemotelyInitiatedConnections = + Math.min(maxConnections, maxRemotelyInitiatedConnections); // Setup metrics connectedPeersCounter = @@ -111,7 +108,7 @@ public class RlpxAgent { PantheonMetricCategory.ETHEREUM, "peer_limit", "The maximum number of peers this node allows to connect", - () -> maxPeers); + () -> maxConnections); } public static Builder builder() { @@ -153,7 +150,7 @@ public class RlpxAgent { if (!localNode.isReady()) { return; } - final int availablePeerSlots = Math.max(0, maxPeers - getConnectionCount()); + final int availablePeerSlots = Math.max(0, maxConnections - getConnectionCount()); peerStream .filter(peer -> !connectionsById.containsKey(peer.getId())) .filter(peer -> peer.getEnodeURL().isListening()) @@ -207,10 +204,10 @@ public class RlpxAgent { return peerConnection.get(); } // Check max peers - if (!peerProperties.ignoreMaxPeerLimits(peer) && getConnectionCount() >= maxPeers) { + if (!peerPrivileges.canExceedConnectionLimits(peer) && getConnectionCount() >= maxConnections) { final String errorMsg = "Max peer peer connections established (" - + maxPeers + + maxConnections + "). Cannot connect to peer: " + peer; return FutureUtils.completedExceptionally(new IllegalStateException(errorMsg)); @@ -325,7 +322,12 @@ public class RlpxAgent { return; } // Disconnect if too many peers - if (!peerProperties.ignoreMaxPeerLimits(peer) && getConnectionCount() >= maxPeers) { + if (!peerPrivileges.canExceedConnectionLimits(peer) && getConnectionCount() >= maxConnections) { + peerConnection.disconnect(DisconnectReason.TOO_MANY_PEERS); + return; + } + // Disconnect if too many remotely-initiated connections + if (!peerPrivileges.canExceedConnectionLimits(peer) && remoteConnectionLimitReached()) { peerConnection.disconnect(DisconnectReason.TOO_MANY_PEERS); return; } @@ -335,16 +337,6 @@ public class RlpxAgent { return; } - // Disconnect if the fraction of wire connections initiated by peers is too high to protect - // against eclipse attacks - if (limitRemoteWireConnectionsEnabled && remoteConnectionExceedsLimit()) { - LOG.warn( - "Fraction of remotely initiated connection is too high, rejecting incoming connection. (max ratio allowed: {})", - fractionRemoteConnectionsAllowed); - peerConnection.disconnect(DisconnectReason.TOO_MANY_PEERS); - return; - } - // Track this new connection, deduplicating existing connection if necessary final AtomicBoolean newConnectionAccepted = new AtomicBoolean(false); final RlpxConnection inboundConnection = RlpxConnection.inboundConnection(peerConnection); @@ -387,33 +379,63 @@ public class RlpxAgent { if (newConnectionAccepted.get()) { dispatchConnect(peerConnection); } + // Check remote connections again to control for race conditions + enforceRemoteConnectionLimits(); enforceConnectionLimits(); } - private boolean remoteConnectionExceedsLimit() { - final int remotelyInitiatedConnectionsCount = - Math.toIntExact( - connectionsById.values().stream() - .filter(RlpxConnection::isActive) - .filter(RlpxConnection::initiatedRemotely) - .count()); - final double fractionRemoteConnections = - (double) (remotelyInitiatedConnectionsCount + 1) / (double) maxPeers; - return fractionRemoteConnections > fractionRemoteConnectionsAllowed; + private boolean shouldLimitRemoteConnections() { + return maxRemotelyInitiatedConnections < maxConnections; } - private void enforceConnectionLimits() { - connectionsById.values().stream() + private boolean remoteConnectionLimitReached() { + return shouldLimitRemoteConnections() + && countUntrustedRemotelyInitiatedConnections() >= maxRemotelyInitiatedConnections; + } + + private long countUntrustedRemotelyInitiatedConnections() { + return connectionsById.values().stream() .filter(RlpxConnection::isActive) - .sorted(this::prioritizeConnections) - .skip(maxPeers) - .filter(c -> !peerProperties.ignoreMaxPeerLimits(c.getPeer())) + .filter(RlpxConnection::initiatedRemotely) + .filter(conn -> !peerPrivileges.canExceedConnectionLimits(conn.getPeer())) + .count(); + } + + private void enforceRemoteConnectionLimits() { + if (!shouldLimitRemoteConnections() + || connectionsById.size() < maxRemotelyInitiatedConnections) { + // Nothing to do + return; + } + + getActivePrioritizedConnections() + .filter(RlpxConnection::initiatedRemotely) + .filter(conn -> !peerPrivileges.canExceedConnectionLimits(conn.getPeer())) + .skip(maxRemotelyInitiatedConnections) .forEach(c -> c.disconnect(DisconnectReason.TOO_MANY_PEERS)); } + private void enforceConnectionLimits() { + if (connectionsById.size() < maxConnections) { + // Nothing to do - we're under our limits + return; + } + + getActivePrioritizedConnections() + .skip(maxConnections) + .filter(c -> !peerPrivileges.canExceedConnectionLimits(c.getPeer())) + .forEach(c -> c.disconnect(DisconnectReason.TOO_MANY_PEERS)); + } + + private Stream getActivePrioritizedConnections() { + return connectionsById.values().stream() + .filter(RlpxConnection::isActive) + .sorted(this::prioritizeConnections); + } + private int prioritizeConnections(final RlpxConnection a, final RlpxConnection b) { - final boolean aIgnoresPeerLimits = peerProperties.ignoreMaxPeerLimits(a.getPeer()); - final boolean bIgnoresPeerLimits = peerProperties.ignoreMaxPeerLimits(b.getPeer()); + final boolean aIgnoresPeerLimits = peerPrivileges.canExceedConnectionLimits(a.getPeer()); + final boolean bIgnoresPeerLimits = peerPrivileges.canExceedConnectionLimits(b.getPeer()); if (aIgnoresPeerLimits && !bIgnoresPeerLimits) { return -1; } else if (bIgnoresPeerLimits && !aIgnoresPeerLimits) { @@ -476,7 +498,7 @@ public class RlpxAgent { private KeyPair keyPair; private LocalNode localNode; private RlpxConfiguration config; - private PeerProperties peerProperties; + private PeerPrivileges peerPrivileges; private PeerPermissions peerPermissions; private ConnectionInitializer connectionInitializer; private PeerConnectionEvents connectionEvents; @@ -500,21 +522,20 @@ public class RlpxAgent { new PeerRlpxPermissions(localNode, peerPermissions); return new RlpxAgent( localNode, - rlpxPermissions, connectionEvents, connectionInitializer, + rlpxPermissions, + peerPrivileges, config.getMaxPeers(), - peerProperties, - metricsSystem, - config.isLimitRemoteWireConnectionsEnabled(), - config.getFractionRemoteWireConnectionsAllowed()); + config.getMaxRemotelyInitiatedConnections(), + metricsSystem); } private void validate() { checkState(keyPair != null, "KeyPair must be configured"); checkState(localNode != null, "LocalNode must be configured"); checkState(config != null, "RlpxConfiguration must be set"); - checkState(peerProperties != null, "MaintainedPeers must be configured"); + checkState(peerPrivileges != null, "PeerPrivileges must be configured"); checkState(peerPermissions != null, "PeerPermissions must be configured"); checkState(metricsSystem != null, "MetricsSystem must be configured"); } @@ -543,9 +564,9 @@ public class RlpxAgent { return this; } - public Builder peerProperties(final PeerProperties peerProperties) { - checkNotNull(peerProperties); - this.peerProperties = peerProperties; + public Builder peerPrivileges(final PeerPrivileges peerPrivileges) { + checkNotNull(peerPrivileges); + this.peerPrivileges = peerPrivileges; return this; } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/config/RlpxConfigurationTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/config/RlpxConfigurationTest.java new file mode 100644 index 0000000000..5237078d5d --- /dev/null +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/config/RlpxConfigurationTest.java @@ -0,0 +1,64 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.config; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; + +public class RlpxConfigurationTest { + + @Test + public void getMaxRemotelyInitiatedConnections_remoteLimitsDisabled() { + final RlpxConfiguration config = + RlpxConfiguration.create() + .setFractionRemoteWireConnectionsAllowed(.5) + .setLimitRemoteWireConnectionsEnabled(false) + .setMaxPeers(20); + + assertThat(config.getMaxRemotelyInitiatedConnections()).isEqualTo(20); + } + + @Test + public void getMaxRemotelyInitiatedConnections_remoteLimitsEnabled() { + final RlpxConfiguration config = + RlpxConfiguration.create() + .setFractionRemoteWireConnectionsAllowed(.5) + .setLimitRemoteWireConnectionsEnabled(true) + .setMaxPeers(20); + + assertThat(config.getMaxRemotelyInitiatedConnections()).isEqualTo(10); + } + + @Test + public void getMaxRemotelyInitiatedConnections_remoteLimitsEnabledWithNonIntegerRatio() { + final RlpxConfiguration config = + RlpxConfiguration.create() + .setFractionRemoteWireConnectionsAllowed(.50) + .setLimitRemoteWireConnectionsEnabled(true) + .setMaxPeers(25); + + assertThat(config.getMaxRemotelyInitiatedConnections()).isEqualTo(12); + } + + @Test + public void getMaxRemotelyInitiatedConnections_remoteLimitsEnabledRoundsToZero() { + final RlpxConfiguration config = + RlpxConfiguration.create() + .setFractionRemoteWireConnectionsAllowed(.5) + .setLimitRemoteWireConnectionsEnabled(true) + .setMaxPeers(1); + + assertThat(config.getMaxRemotelyInitiatedConnections()).isEqualTo(0); + } +} diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/MaintainedPeersTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/MaintainedPeersTest.java index e589d736f9..60052b0ef6 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/MaintainedPeersTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/MaintainedPeersTest.java @@ -14,6 +14,7 @@ package tech.pegasys.pantheon.ethereum.p2p.peers; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static tech.pegasys.pantheon.ethereum.p2p.peers.PeerTestHelper.enodeBuilder; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -125,6 +126,18 @@ public class MaintainedPeersTest { assertThat(peers).containsExactlyInAnyOrder(peerA, peerB); } + @Test + public void contains() { + final Peer peerA = createPeer(); + final Peer peerAClone = DefaultPeer.fromEnodeURL(peerA.getEnodeURL()); + final Peer peerB = createPeer(); + + maintainedPeers.add(peerA); + assertThat(maintainedPeers.contains(peerA)).isTrue(); + assertThat(maintainedPeers.contains(peerAClone)).isTrue(); + assertThat(maintainedPeers.contains(peerB)).isFalse(); + } + @Test public void stream_empty() { final List peers = maintainedPeers.streamPeers().collect(Collectors.toList()); @@ -138,8 +151,4 @@ public class MaintainedPeersTest { private Peer nonListeningPeer() { return DefaultPeer.fromEnodeURL(enodeBuilder().disableListening().build()); } - - private EnodeURL.Builder enodeBuilder() { - return EnodeURL.builder().ipAddress("127.0.0.1").useDefaultPorts().nodeId(Peer.randomId()); - } } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/RlpxAgentTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/RlpxAgentTest.java index 2e91bbe339..0c74fa2dde 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/RlpxAgentTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/RlpxAgentTest.java @@ -34,7 +34,7 @@ import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.EnodeURL; import tech.pegasys.pantheon.ethereum.p2p.peers.MutableLocalNode; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerProperties; +import tech.pegasys.pantheon.ethereum.p2p.peers.PeerPrivileges; import tech.pegasys.pantheon.ethereum.p2p.peers.PeerTestHelper; import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions.Action; @@ -61,14 +61,13 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; public class RlpxAgentTest { private static final KeyPair KEY_PAIR = KeyPair.generate(); private final RlpxConfiguration config = RlpxConfiguration.create(); private final TestPeerPermissions peerPermissions = spy(new TestPeerPermissions()); - private final PeerProperties peerProperties = mock(PeerProperties.class); + private final PeerPrivileges peerPrivileges = mock(PeerPrivileges.class); private final MutableLocalNode localNode = createMutableLocalNode(); private final MetricsSystem metrics = new NoOpMetricsSystem(); private final PeerConnectionEvents peerConnectionEvents = new PeerConnectionEvents(metrics); @@ -79,7 +78,7 @@ public class RlpxAgentTest { @Before public void setup() { // Set basic defaults - when(peerProperties.ignoreMaxPeerLimits(any())).thenReturn(false); + when(peerPrivileges.canExceedConnectionLimits(any())).thenReturn(false); config.setMaxPeers(5); } @@ -439,6 +438,42 @@ public class RlpxAgentTest { } } + @Test + public void incomingConnection_succeedsForPrivilegedPeerWhenMaxRemoteConnectionsExceeded() { + final int maxPeers = 5; + final int maxRemotePeers = 3; + final double maxRemotePeersFraction = (double) maxRemotePeers / (double) maxPeers; + config.setLimitRemoteWireConnectionsEnabled(true); + config.setFractionRemoteWireConnectionsAllowed(maxRemotePeersFraction); + startAgentWithMaxPeers(maxPeers); + + // Connect max remote peers + for (int i = 0; i < maxRemotePeers; i++) { + final Peer remotelyInitiatedPeer = createPeer(); + final MockPeerConnection incomingConnection = connection(remotelyInitiatedPeer); + connectionInitializer.simulateIncomingConnection(incomingConnection); + assertThat(incomingConnection.getDisconnectReason()).isEmpty(); + } + // Sanity check + assertThat(agent.getConnectionCount()).isEqualTo(maxRemotePeers); + + final Peer privilegedPeer = createPeer(); + when(peerPrivileges.canExceedConnectionLimits(privilegedPeer)).thenReturn(true); + final MockPeerConnection privilegedConnection = connection(privilegedPeer); + connectionInitializer.simulateIncomingConnection(privilegedConnection); + assertThat(privilegedConnection.isDisconnected()).isFalse(); + + // No peers should be disconnected - exempt connections are ignored when enforcing this limit + assertThat(agent.getConnectionCount()).isEqualTo(maxRemotePeers + 1); + + // The next non-exempt connection should fail + final Peer remotelyInitiatedPeer = createPeer(); + final MockPeerConnection incomingConnection = connection(remotelyInitiatedPeer); + connectionInitializer.simulateIncomingConnection(incomingConnection); + assertThat(agent.getConnectionCount()).isEqualTo(maxRemotePeers + 1); + assertThat(incomingConnection.getDisconnectReason()).contains(DisconnectReason.TOO_MANY_PEERS); + } + @Test public void connect_succeedsForExemptPeerWhenMaxPeersConnected() throws ExecutionException, InterruptedException { @@ -454,7 +489,7 @@ public class RlpxAgentTest { (MockPeerConnection) existingConnectionFuture.get(); final Peer peer = createPeer(); - when(peerProperties.ignoreMaxPeerLimits(peer)).thenReturn(true); + when(peerPrivileges.canExceedConnectionLimits(peer)).thenReturn(true); final CompletableFuture connection = agent.connect(peer); connectionInitializer.completePendingFutures(); @@ -477,8 +512,8 @@ public class RlpxAgentTest { startAgentWithMaxPeers(1); final Peer peerA = createPeer(); final Peer peerB = createPeer(); - when(peerProperties.ignoreMaxPeerLimits(peerA)).thenReturn(true); - when(peerProperties.ignoreMaxPeerLimits(peerB)).thenReturn(true); + when(peerPrivileges.canExceedConnectionLimits(peerA)).thenReturn(true); + when(peerPrivileges.canExceedConnectionLimits(peerB)).thenReturn(true); // Saturate connections final CompletableFuture existingConnection = agent.connect(peerA); @@ -498,12 +533,11 @@ public class RlpxAgentTest { } @Test - @Ignore("Ignore while PAN-1683 is being reworked") public void incomingConnection_maxPeersExceeded_incomingConnectionExemptFromLimits() throws ExecutionException, InterruptedException { final Peer peerA = createPeer(); final Peer peerB = createPeer(); - when(peerProperties.ignoreMaxPeerLimits(peerB)).thenReturn(true); + when(peerPrivileges.canExceedConnectionLimits(peerB)).thenReturn(true); // Saturate connections startAgentWithMaxPeers(1); @@ -530,7 +564,7 @@ public class RlpxAgentTest { throws ExecutionException, InterruptedException { final Peer peerA = createPeer(); final Peer peerB = createPeer(); - when(peerProperties.ignoreMaxPeerLimits(peerA)).thenReturn(true); + when(peerPrivileges.canExceedConnectionLimits(peerA)).thenReturn(true); // Saturate connections startAgentWithMaxPeers(1); @@ -553,13 +587,12 @@ public class RlpxAgentTest { } @Test - @Ignore("Ignore while PAN-1683 is being reworked") public void incomingConnection_maxPeersExceeded_allConnectionsExemptFromLimits() throws ExecutionException, InterruptedException { final Peer peerA = createPeer(); final Peer peerB = createPeer(); - when(peerProperties.ignoreMaxPeerLimits(peerA)).thenReturn(true); - when(peerProperties.ignoreMaxPeerLimits(peerB)).thenReturn(true); + when(peerPrivileges.canExceedConnectionLimits(peerA)).thenReturn(true); + when(peerPrivileges.canExceedConnectionLimits(peerB)).thenReturn(true); // Saturate connections startAgentWithMaxPeers(1); @@ -935,7 +968,7 @@ public class RlpxAgentTest { .keyPair(KEY_PAIR) .config(config) .peerPermissions(peerPermissions) - .peerProperties(peerProperties) + .peerPrivileges(peerPrivileges) .localNode(localNode) .metricsSystem(metrics) .connectionInitializer(connectionInitializer)