Refresh peer table while we have fewer than maxPeers connected (#1142)

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Synchronizer.java

@@ -25,8 +25,6 @@ public interface Synchronizer {
    */
   Optional<SyncStatus> getSyncStatus();
 
-  boolean hasSufficientPeers();
-
   long observeSyncStatus(final SyncStatusListener listener);
 
   boolean removeObserver(long observerId);

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/DefaultSynchronizer.java

@@ -40,8 +40,6 @@ public class DefaultSynchronizer<C> implements Synchronizer {
 
   private static final Logger LOG = LogManager.getLogger();
 
-  private final SynchronizerConfiguration syncConfig;
-  private final EthContext ethContext;
   private final SyncState syncState;
   private final AtomicBoolean started = new AtomicBoolean(false);
   private final BlockPropagationManager<C> blockPropagationManager;
@@ -58,8 +56,6 @@ public class DefaultSynchronizer<C> implements Synchronizer {
       final SyncState syncState,
       final Path dataDirectory,
       final MetricsSystem metricsSystem) {
-    this.syncConfig = syncConfig;
-    this.ethContext = ethContext;
     this.syncState = syncState;
 
     this.blockPropagationManager =
@@ -151,13 +147,6 @@ public class DefaultSynchronizer<C> implements Synchronizer {
     return Optional.of(syncState.syncStatus());
   }
 
-  @Override
-  public boolean hasSufficientPeers() {
-    final int requiredPeerCount =
-        fastSynchronizer.isPresent() ? syncConfig.getFastSyncMinimumPeerCount() : 1;
-    return ethContext.getEthPeers().availablePeerCount() >= requiredPeerCount;
-  }
-
   @Override
   public long observeSyncStatus(final SyncStatusListener listener) {
     checkNotNull(listener);

ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java

@@ -120,7 +120,6 @@ public class TestNode implements Closeable {
                         this.kp,
                         networkingConfiguration,
                         capabilities,
-                        () -> true,
                         new PeerBlacklist(),
                         new NoOpMetricsSystem(),
                         Optional.empty(),

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java

@@ -15,6 +15,7 @@ package tech.pegasys.pantheon.ethereum.p2p.netty;
 import static com.google.common.base.Preconditions.checkState;
 
 import tech.pegasys.pantheon.crypto.SECP256K1;
+import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
 import tech.pegasys.pantheon.ethereum.chain.BlockAddedEvent;
 import tech.pegasys.pantheon.ethereum.chain.Blockchain;
 import tech.pegasys.pantheon.ethereum.p2p.PeerNotPermittedException;
@@ -28,7 +29,6 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryAgent;
 import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent;
 import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent;
 import tech.pegasys.pantheon.ethereum.p2p.discovery.VertxPeerDiscoveryAgent;
-import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerRequirement;
 import tech.pegasys.pantheon.ethereum.p2p.peers.Endpoint;
 import tech.pegasys.pantheon.ethereum.p2p.peers.Peer;
 import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist;
@@ -173,10 +173,9 @@ public class NettyP2PNetwork implements P2PNetwork {
 
   public NettyP2PNetwork(
       final Vertx vertx,
-      final SECP256K1.KeyPair keyPair,
+      final KeyPair keyPair,
       final NetworkingConfiguration config,
       final List<Capability> supportedCapabilities,
-      final PeerRequirement peerRequirement,
       final PeerBlacklist peerBlacklist,
       final MetricsSystem metricsSystem,
       final Optional<NodeLocalConfigPermissioningController> nodeWhitelistController,
@@ -186,7 +185,6 @@ public class NettyP2PNetwork implements P2PNetwork {
         keyPair,
         config,
         supportedCapabilities,
-        peerRequirement,
         peerBlacklist,
         metricsSystem,
         nodeWhitelistController,
@@ -206,7 +204,6 @@ public class NettyP2PNetwork implements P2PNetwork {
    * @param config The network configuration to use.
    * @param supportedCapabilities The wire protocol capabilities to advertise to connected peers.
    * @param peerBlacklist The peers with which this node will not connect
-   * @param peerRequirement Queried to determine if enough peers are currently connected.
    * @param metricsSystem The metrics system to capture metrics with.
    * @param nodeLocalConfigPermissioningController local file config for permissioning
    * @param nodePermissioningController Controls node permissioning.
@@ -217,13 +214,13 @@ public class NettyP2PNetwork implements P2PNetwork {
       final SECP256K1.KeyPair keyPair,
       final NetworkingConfiguration config,
       final List<Capability> supportedCapabilities,
-      final PeerRequirement peerRequirement,
       final PeerBlacklist peerBlacklist,
       final MetricsSystem metricsSystem,
       final Optional<NodeLocalConfigPermissioningController> nodeLocalConfigPermissioningController,
       final Optional<NodePermissioningController> nodePermissioningController,
       final Blockchain blockchain) {
 
+    maxPeers = config.getRlpx().getMaxPeers();
     connections = new PeerConnectionRegistry(metricsSystem);
     this.peerBlacklist = peerBlacklist;
     this.peerMaintainConnectionList = new HashSet<>();
@@ -232,7 +229,7 @@ public class NettyP2PNetwork implements P2PNetwork {
             vertx,
             keyPair,
             config.getDiscovery(),
-            peerRequirement,
+            () -> connections.size() >= maxPeers,
             peerBlacklist,
             nodeLocalConfigPermissioningController,
             nodePermissioningController);
@@ -262,7 +259,6 @@ public class NettyP2PNetwork implements P2PNetwork {
     subscribeDisconnect(peerBlacklist);
     subscribeDisconnect(connections);
 
-    maxPeers = config.getRlpx().getMaxPeers();
     this.keyPair = keyPair;
     this.subProtocols = config.getSupportedProtocols();
 

ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java

@@ -118,7 +118,6 @@ public final class NettyP2PNetworkTest {
                     .setSupportedProtocols(subProtocol())
                     .setRlpx(RlpxConfiguration.create().setBindPort(0)),
                 singletonList(cap),
-                () -> false,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -132,7 +131,6 @@ public final class NettyP2PNetworkTest {
                     .setRlpx(RlpxConfiguration.create().setBindPort(0))
                     .setDiscovery(noDiscovery),
                 singletonList(cap),
-                () -> false,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -174,7 +172,6 @@ public final class NettyP2PNetworkTest {
                     .setDiscovery(noDiscovery)
                     .setRlpx(RlpxConfiguration.create().setBindPort(0)),
                 capabilities,
-                () -> true,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -188,7 +185,6 @@ public final class NettyP2PNetworkTest {
                     .setRlpx(RlpxConfiguration.create().setBindPort(0))
                     .setDiscovery(noDiscovery),
                 capabilities,
-                () -> true,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -245,7 +241,6 @@ public final class NettyP2PNetworkTest {
                     .setRlpx(RlpxConfiguration.create().setBindPort(0).setMaxPeers(maxPeers))
                     .setSupportedProtocols(subProtocol),
                 cap,
-                () -> true,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -259,7 +254,6 @@ public final class NettyP2PNetworkTest {
                     .setRlpx(RlpxConfiguration.create().setBindPort(0))
                     .setSupportedProtocols(subProtocol),
                 cap,
-                () -> true,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -273,7 +267,6 @@ public final class NettyP2PNetworkTest {
                     .setRlpx(RlpxConfiguration.create().setBindPort(0))
                     .setSupportedProtocols(subProtocol),
                 cap,
-                () -> true,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -329,7 +322,6 @@ public final class NettyP2PNetworkTest {
                     .setSupportedProtocols(subprotocol1)
                     .setRlpx(RlpxConfiguration.create().setBindPort(0)),
                 singletonList(cap1),
-                () -> false,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -343,7 +335,6 @@ public final class NettyP2PNetworkTest {
                     .setRlpx(RlpxConfiguration.create().setBindPort(0))
                     .setDiscovery(noDiscovery),
                 singletonList(cap2),
-                () -> false,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -386,7 +377,6 @@ public final class NettyP2PNetworkTest {
                     .setSupportedProtocols(subprotocol)
                     .setRlpx(RlpxConfiguration.create().setBindPort(0)),
                 singletonList(cap),
-                () -> false,
                 localBlacklist,
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -400,7 +390,6 @@ public final class NettyP2PNetworkTest {
                     .setRlpx(RlpxConfiguration.create().setBindPort(0))
                     .setDiscovery(noDiscovery),
                 singletonList(cap),
-                () -> false,
                 remoteBlacklist,
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -481,7 +470,6 @@ public final class NettyP2PNetworkTest {
                     .setSupportedProtocols(subprotocol)
                     .setRlpx(RlpxConfiguration.create().setBindPort(0)),
                 singletonList(cap),
-                () -> false,
                 localBlacklist,
                 new NoOpMetricsSystem(),
                 Optional.of(localWhitelistController),
@@ -496,7 +484,6 @@ public final class NettyP2PNetworkTest {
                     .setRlpx(RlpxConfiguration.create().setBindPort(0))
                     .setDiscovery(noDiscovery),
                 singletonList(cap),
-                () -> false,
                 remoteBlacklist,
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -867,7 +854,6 @@ public final class NettyP2PNetworkTest {
         keyPair,
         networkingConfiguration,
         singletonList(cap),
-        () -> false,
         new PeerBlacklist(),
         new NoOpMetricsSystem(),
         Optional.empty(),

ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NetworkingServiceLifecycleTest.java

@@ -55,7 +55,6 @@ public class NetworkingServiceLifecycleTest {
             keyPair,
             config,
             emptyList(),
-            () -> true,
             new PeerBlacklist(),
             new NoOpMetricsSystem(),
             Optional.empty(),
@@ -86,7 +85,6 @@ public class NetworkingServiceLifecycleTest {
             keyPair,
             config,
             emptyList(),
-            () -> true,
             new PeerBlacklist(),
             new NoOpMetricsSystem(),
             Optional.empty(),
@@ -107,7 +105,6 @@ public class NetworkingServiceLifecycleTest {
             keyPair,
             config,
             emptyList(),
-            () -> true,
             new PeerBlacklist(),
             new NoOpMetricsSystem(),
             Optional.empty(),
@@ -128,7 +125,6 @@ public class NetworkingServiceLifecycleTest {
             keyPair,
             config,
             emptyList(),
-            () -> true,
             new PeerBlacklist(),
             new NoOpMetricsSystem(),
             Optional.empty(),
@@ -145,7 +141,6 @@ public class NetworkingServiceLifecycleTest {
             null,
             configWithRandomPorts(),
             emptyList(),
-            () -> true,
             new PeerBlacklist(),
             new NoOpMetricsSystem(),
             Optional.empty(),
@@ -163,7 +158,6 @@ public class NetworkingServiceLifecycleTest {
             keyPair,
             configWithRandomPorts(),
             emptyList(),
-            () -> true,
             new PeerBlacklist(),
             new NoOpMetricsSystem(),
             Optional.empty(),
@@ -183,7 +177,6 @@ public class NetworkingServiceLifecycleTest {
                 keyPair,
                 configWithRandomPorts(),
                 emptyList(),
-                () -> true,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -194,7 +187,6 @@ public class NetworkingServiceLifecycleTest {
                 keyPair,
                 configWithRandomPorts(),
                 emptyList(),
-                () -> true,
                 new PeerBlacklist(),
                 new NoOpMetricsSystem(),
                 Optional.empty(),
@@ -215,7 +207,6 @@ public class NetworkingServiceLifecycleTest {
             keyPair,
             configWithRandomPorts(),
             emptyList(),
-            () -> true,
             new PeerBlacklist(),
             new NoOpMetricsSystem(),
             Optional.empty(),
@@ -231,7 +222,6 @@ public class NetworkingServiceLifecycleTest {
               keyPair,
               config,
               emptyList(),
-              () -> true,
               new PeerBlacklist(),
               new NoOpMetricsSystem(),
               Optional.empty(),
@@ -262,7 +252,6 @@ public class NetworkingServiceLifecycleTest {
             keyPair,
             configWithRandomPorts(),
             emptyList(),
-            () -> true,
             new PeerBlacklist(),
             new NoOpMetricsSystem(),
             Optional.empty(),

pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java

@@ -270,7 +270,6 @@ public class RunnerBuilder {
                             keyPair,
                             networkConfig,
                             caps,
-                            synchronizer::hasSufficientPeers,
                             peerBlacklist,
                             metricsSystem,
                             nodeWhitelistController,