Optimize pivot block selector on PoS networks

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Additions and Improvements
 - Bring GraphQL into compliance with execution-api specs [#4112](https://github.com/hyperledger/besu/pull/4112)
 - Refactor unverified forkchoice event [#4487](https://github.com/hyperledger/besu/pull/4487)
+- Optimize pivot block selector on PoS networks [#4488](https://github.com/hyperledger/besu/pull/4488)
 - Improve UX of initial sync logs, pushing not relevant logs to debug level [#4486](https://github.com/hyperledger/besu/pull/4486)
 
 ### Bug Fixes

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

@@ -20,7 +20,6 @@
 import static java.util.Collections.singletonList;
 import static org.hyperledger.besu.cli.DefaultCommandValues.getDefaultBesuDataPath;
 import static org.hyperledger.besu.cli.config.NetworkName.MAINNET;
-import static org.hyperledger.besu.cli.config.NetworkName.isMergedNetwork;
 import static org.hyperledger.besu.cli.util.CommandLineUtils.DEPENDENCY_WARNING_MSG;
 import static org.hyperledger.besu.cli.util.CommandLineUtils.DEPRECATED_AND_USELESS_WARNING_MSG;
 import static org.hyperledger.besu.cli.util.CommandLineUtils.DEPRECATION_WARNING_MSG;
@@ -508,12 +507,8 @@ private InetAddress autoDiscoverDefaultIP() {
       names = {"--fast-sync-min-peers"},
       paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
       description =
-          "Minimum number of peers required before starting fast sync. (default pre-merge: "
-              + FAST_SYNC_MIN_PEER_COUNT
-              + " and post-merge: "
-              + FAST_SYNC_MIN_PEER_COUNT_POST_MERGE
-              + ")")
-  private final Integer fastSyncMinPeerCount = null;
+          "Minimum number of peers required before starting fast sync. Has only effect on PoW networks. (default: ${DEFAULT-VALUE})")
+  private final Integer fastSyncMinPeerCount = FAST_SYNC_MIN_PEER_COUNT;
 
   @Option(
       names = {"--network"},
@@ -2795,19 +2790,10 @@ private Optional<PkiBlockCreationConfiguration> maybePkiBlockCreationConfigurati
   }
 
   private SynchronizerConfiguration buildSyncConfig() {
-    Integer fastSyncMinPeers = fastSyncMinPeerCount;
-    if (fastSyncMinPeers == null) {
-      if (isMergedNetwork(network)) {
-        fastSyncMinPeers = FAST_SYNC_MIN_PEER_COUNT_POST_MERGE;
-      } else {
-        fastSyncMinPeers = FAST_SYNC_MIN_PEER_COUNT;
-      }
-    }
-
     return unstableSynchronizerOptions
         .toDomainObject()
         .syncMode(syncMode)
-        .fastSyncMinimumPeerCount(fastSyncMinPeers)
+        .fastSyncMinimumPeerCount(fastSyncMinPeerCount)
         .build();
   }
 

besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java

@@ -57,7 +57,6 @@ public interface DefaultCommandValues {
   NatMethod DEFAULT_NAT_METHOD = NatMethod.AUTO;
   JwtAlgorithm DEFAULT_JWT_ALGORITHM = JwtAlgorithm.RS256;
   int FAST_SYNC_MIN_PEER_COUNT = 5;
-  int FAST_SYNC_MIN_PEER_COUNT_POST_MERGE = 1;
   int DEFAULT_MAX_PEERS = 25;
   int DEFAULT_P2P_PEER_LOWER_BOUND = 25;
   int DEFAULT_HTTP_MAX_CONNECTIONS = 80;

besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java

@@ -86,17 +86,4 @@ public boolean isDeprecated() {
   public Optional<String> getDeprecationDate() {
     return Optional.ofNullable(deprecationDate);
   }
-
-  public static boolean isMergedNetwork(final NetworkName networkName) {
-    switch (networkName) {
-      case MAINNET:
-      case GOERLI:
-      case ROPSTEN:
-      case SEPOLIA:
-      case KILN:
-        return true;
-      default:
-        return false;
-    }
-  }
 }

besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java

@@ -63,7 +63,6 @@
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotSelectorFromFinalizedBlock;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotSelectorFromPeers;
-import org.hyperledger.besu.ethereum.eth.sync.fastsync.TransitionPivotSelector;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.checkpoint.Checkpoint;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.checkpoint.ImmutableCheckpoint;
 import org.hyperledger.besu.ethereum.eth.sync.fullsync.SyncTerminationCondition;
@@ -408,7 +407,8 @@ public BesuController build() {
     final Optional<SnapProtocolManager> maybeSnapProtocolManager =
         createSnapProtocolManager(peerValidators, ethPeers, snapMessages, worldStateArchive);
 
-    final PivotBlockSelector pivotBlockSelector = createPivotSelector(protocolContext);
+    final PivotBlockSelector pivotBlockSelector =
+        createPivotSelector(protocolContext, ethContext, syncState);
 
     final Synchronizer synchronizer =
         createSynchronizer(
@@ -493,9 +493,11 @@ protected Synchronizer createSynchronizer(
     return toUse;
   }
 
-  private PivotBlockSelector createPivotSelector(final ProtocolContext protocolContext) {
+  private PivotBlockSelector createPivotSelector(
+      final ProtocolContext protocolContext,
+      final EthContext ethContext,
+      final SyncState syncState) {
 
-    final PivotSelectorFromPeers pivotSelectorFromPeers = new PivotSelectorFromPeers(syncConfig);
     final GenesisConfigOptions genesisConfigOptions = configOptionsSupplier.get();
 
     if (genesisConfigOptions.getTerminalTotalDifficulty().isPresent()) {
@@ -514,15 +516,11 @@ private PivotBlockSelector createPivotSelector(final ProtocolContext protocolCon
             LOG.info("Initial sync done, unsubscribe forkchoice supplier");
           };
 
-      return new TransitionPivotSelector(
-          genesisConfigOptions,
-          unverifiedForkchoiceSupplier,
-          pivotSelectorFromPeers,
-          new PivotSelectorFromFinalizedBlock(
-              genesisConfigOptions, unverifiedForkchoiceSupplier, unsubscribeForkchoiceListener));
+      return new PivotSelectorFromFinalizedBlock(
+          genesisConfigOptions, unverifiedForkchoiceSupplier, unsubscribeForkchoiceListener);
     } else {
       LOG.info("TTD difficulty is not present, creating initial sync phase for PoW");
-      return pivotSelectorFromPeers;
+      return new PivotSelectorFromPeers(ethContext, syncConfig, syncState, metricsSystem);
     }
   }
 

besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java

@@ -847,7 +847,7 @@ public void tomlThatConfiguresEverythingExceptPermissioningToml() throws IOExcep
   }
 
   @Test
-  public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOException {
+  public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() {
     final String configFile = this.getClass().getResource("/partial_config.toml").getFile();
 
     parseCommand("--config-file", configFile);
@@ -882,7 +882,7 @@ public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOExce
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
     assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(1);
+    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(5);
 
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
 
@@ -1709,20 +1709,8 @@ public void helpShouldDisplayFastSyncOptions() {
   }
 
   @Test
-  public void checkValidDefaultFastSyncMinPeersPoS() {
-    parseCommand("--sync-mode", "FAST", "--network", "MAINNET");
-    verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
-
-    final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
-    assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(1);
-    assertThat(commandOutput.toString(UTF_8)).isEmpty();
-    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
-  }
-
-  @Test
-  public void checkValidDefaultFastSyncMinPeersPoW() {
-    parseCommand("--sync-mode", "FAST", "--network", "CLASSIC");
+  public void checkValidDefaultFastSyncMinPeers() {
+    parseCommand("--sync-mode", "FAST");
     verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
@@ -1744,30 +1732,6 @@ public void parsesValidFastSyncMinPeersOption() {
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
 
-  @Test
-  public void parsesValidFastSyncMinPeersOptionPreMerge() {
-    parseCommand("--sync-mode", "FAST", "--network", "CLASSIC", "--fast-sync-min-peers", "11");
-    verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
-
-    final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
-    assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(11);
-    assertThat(commandOutput.toString(UTF_8)).isEmpty();
-    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
-  }
-
-  @Test
-  public void parsesValidFastSyncMinPeersOptionPostMerge() {
-    parseCommand("--sync-mode", "FAST", "--network", "GOERLI", "--fast-sync-min-peers", "11");
-    verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
-
-    final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
-    assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(11);
-    assertThat(commandOutput.toString(UTF_8)).isEmpty();
-    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
-  }
-
   @Test
   public void parsesInvalidFastSyncMinPeersOptionWrongFormatShouldFail() {
 

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/PivotBlockSelector.java

@@ -14,14 +14,16 @@
  */
 package org.hyperledger.besu.ethereum.eth.sync;
 
-import org.hyperledger.besu.ethereum.eth.manager.EthPeer;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState;
 
 import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
 
 public interface PivotBlockSelector {
 
-  Optional<FastSyncState> selectNewPivotBlock(EthPeer peer);
+  Optional<FastSyncState> selectNewPivotBlock();
+
+  CompletableFuture<Void> prepareRetry();
 
   default void close() {
     // do nothing by default