From b3e64c4f7c1237e40d56f6c994169d36e2a6b8fe Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Thu, 24 Jan 2019 09:36:15 +1000 Subject: [PATCH] [NC-1273] Consider peer count insufficient until minimum peers for fast sync are connected (#629) Makes Synchronizer responsible for deciding if it has enough peers or not rather than ProtocolManager. Signed-off-by: Adrian Sutton --- .../consensus/ibft/protocol/IbftProtocolManager.java | 5 ----- .../pegasys/pantheon/ethereum/core/Synchronizer.java | 2 ++ .../ethereum/eth/manager/EthProtocolManager.java | 5 ----- .../ethereum/eth/sync/DefaultSynchronizer.java | 11 +++++++++++ .../ethereum/eth/sync/SynchronizerConfiguration.java | 2 +- .../pantheon/ethereum/eth/transactions/TestNode.java | 2 +- .../pantheon/ethereum/p2p/api/ProtocolManager.java | 3 +-- .../p2p/discovery/internal/PeerRequirement.java | 6 ------ .../java/tech/pegasys/pantheon/RunnerBuilder.java | 8 ++++---- 9 files changed, 20 insertions(+), 24 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/protocol/IbftProtocolManager.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/protocol/IbftProtocolManager.java index b00e25281e..f3f7b800a4 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/protocol/IbftProtocolManager.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/protocol/IbftProtocolManager.java @@ -95,9 +95,4 @@ public class IbftProtocolManager implements ProtocolManager { final boolean initiatedByPeer) { peers.remove(peerConnection); } - - @Override - public boolean hasSufficientPeers() { - return true; - } } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Synchronizer.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Synchronizer.java index 1cb53d29fc..3194d11b13 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Synchronizer.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Synchronizer.java @@ -24,4 +24,6 @@ public interface Synchronizer { * empty */ Optional getSyncStatus(); + + boolean hasSufficientPeers(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthProtocolManager.java index cd3cacbc53..fe63aa31c0 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthProtocolManager.java @@ -237,11 +237,6 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver { } } - @Override - public boolean hasSufficientPeers() { - return ethPeers.availablePeerCount() > 0; - } - private void handleStatusMessage(final EthPeer peer, final MessageData data) { final StatusMessage status = StatusMessage.readFrom(data); try { diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/DefaultSynchronizer.java index 2d05bc8fdb..e79f7c71d5 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/DefaultSynchronizer.java @@ -37,6 +37,8 @@ public class DefaultSynchronizer 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 blockPropagationManager; @@ -50,6 +52,8 @@ public class DefaultSynchronizer implements Synchronizer { final EthContext ethContext, final SyncState syncState, final LabelledMetric ethTasksTimer) { + this.syncConfig = syncConfig; + this.ethContext = ethContext; this.syncState = syncState; this.blockPropagationManager = new BlockPropagationManager<>( @@ -121,4 +125,11 @@ public class DefaultSynchronizer implements Synchronizer { } return Optional.of(syncState.syncStatus()); } + + @Override + public boolean hasSufficientPeers() { + final int requiredPeerCount = + fastSyncDownloader.isPresent() ? syncConfig.getFastSyncMinimumPeerCount() : 1; + return ethContext.getEthPeers().availablePeerCount() >= requiredPeerCount; + } } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java index 997f0d91e6..a5dba7f7c0 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java @@ -32,7 +32,7 @@ public class SynchronizerConfiguration { public static int DEFAULT_PIVOT_DISTANCE_FROM_HEAD = 500; public static float DEFAULT_FULL_VALIDATION_RATE = .1f; public static int DEFAULT_FAST_SYNC_MINIMUM_PEERS = 5; - private static final Duration DEFAULT_FAST_SYNC_MAXIMUM_PEER_WAIT_TIME = Duration.ofSeconds(3); + private static final Duration DEFAULT_FAST_SYNC_MAXIMUM_PEER_WAIT_TIME = Duration.ofMinutes(3); // Fast sync config private final int fastSyncPivotDistance; diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java index 036e0cb9f9..e3dbae8728 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java @@ -115,7 +115,7 @@ public class TestNode implements Closeable { this.kp, networkingConfiguration, capabilities, - ethProtocolManager, + () -> true, new PeerBlacklist(), new NoOpMetricsSystem(), new NodeWhitelistController(PermissioningConfiguration.createDefault()))) diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/ProtocolManager.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/ProtocolManager.java index 43f45f9cc3..6a49196823 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/ProtocolManager.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/ProtocolManager.java @@ -12,14 +12,13 @@ */ package tech.pegasys.pantheon.ethereum.p2p.api; -import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerRequirement; import tech.pegasys.pantheon.ethereum.p2p.wire.Capability; import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; import java.util.List; /** Represents an object responsible for managing a wire subprotocol. */ -public interface ProtocolManager extends AutoCloseable, PeerRequirement { +public interface ProtocolManager extends AutoCloseable { String getSupportedProtocol(); diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerRequirement.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerRequirement.java index 8213fd4cca..b641ffde44 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerRequirement.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerRequirement.java @@ -12,14 +12,8 @@ */ package tech.pegasys.pantheon.ethereum.p2p.discovery.internal; -import java.util.Collection; - @FunctionalInterface public interface PeerRequirement { boolean hasSufficientPeers(); - - static PeerRequirement aggregateOf(final Collection peers) { - return () -> peers.stream().allMatch(PeerRequirement::hasSufficientPeers); - } } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java index 4da6d6d64f..21c729017d 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java @@ -46,7 +46,6 @@ import tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration; import tech.pegasys.pantheon.ethereum.p2p.config.NetworkingConfiguration; import tech.pegasys.pantheon.ethereum.p2p.config.RlpxConfiguration; import tech.pegasys.pantheon.ethereum.p2p.config.SubProtocolConfiguration; -import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerRequirement; import tech.pegasys.pantheon.ethereum.p2p.netty.NettyP2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.ethereum.p2p.permissioning.NodeWhitelistController; @@ -213,9 +212,11 @@ public class RunnerBuilder { new PeerBlacklist( bannedNodeIds.stream().map(BytesValue::fromHexString).collect(Collectors.toSet())); - NodeWhitelistController nodeWhitelistController = + final NodeWhitelistController nodeWhitelistController = new NodeWhitelistController(permissioningConfiguration); + final Synchronizer synchronizer = pantheonController.getSynchronizer(); + final NetworkRunner networkRunner = NetworkRunner.builder() .protocolManagers(protocolManagers) @@ -228,7 +229,7 @@ public class RunnerBuilder { keyPair, networkConfig, caps, - PeerRequirement.aggregateOf(protocolManagers), + synchronizer::hasSufficientPeers, peerBlacklist, metricsSystem, nodeWhitelistController) @@ -236,7 +237,6 @@ public class RunnerBuilder { .metricsSystem(metricsSystem) .build(); - final Synchronizer synchronizer = pantheonController.getSynchronizer(); final TransactionPool transactionPool = pantheonController.getTransactionPool(); final MiningCoordinator miningCoordinator = pantheonController.getMiningCoordinator();