Refactor sync-peer-count internal variable to match name, add experim…

…ental flag to enabled snap + BFT

Signed-off-by: Matthew Whitehead <[email protected]>

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

@@ -2766,6 +2766,7 @@ private String generateConfigurationOverview() {
     }
 
     builder.setSnapServerEnabled(this.unstableSynchronizerOptions.isSnapsyncServerEnabled());
+    builder.setSnapSyncBftEnabled(this.unstableSynchronizerOptions.isSnapSyncBftEnabled());
 
     builder.setTxPoolImplementation(buildTransactionPoolConfiguration().getTxPoolImplementation());
     builder.setWorldStateUpdateMode(unstableEvmOptions.toDomainObject().worldUpdaterMode());

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

@@ -56,6 +56,7 @@ public class ConfigurationOverviewBuilder {
   private long trieLogRetentionLimit = 0;
   private Integer trieLogsPruningWindowSize = null;
   private boolean isSnapServerEnabled = false;
+  private boolean isSnapSyncBftEnabled = false;
   private TransactionPoolConfiguration.Implementation txPoolImplementation;
   private EvmConfiguration.WorldUpdaterMode worldStateUpdateMode;
   private Map<String, String> environment;
@@ -233,6 +234,17 @@ public ConfigurationOverviewBuilder setSnapServerEnabled(final boolean snapServe
     return this;
   }
 
+  /**
+   * Sets snap sync BFT enabled/disabled
+   *
+   * @param snapSyncBftEnabled bool to indicate if snap sync for BFT is enabled
+   * @return the builder
+   */
+  public ConfigurationOverviewBuilder setSnapSyncBftEnabled(final boolean snapSyncBftEnabled) {
+    isSnapSyncBftEnabled = snapSyncBftEnabled;
+    return this;
+  }
+
   /**
    * Sets trie logs pruning window size
    *
@@ -357,6 +369,10 @@ public String build() {
       lines.add("Experimental Snap Sync server enabled");
     }
 
+    if (isSnapSyncBftEnabled) {
+      lines.add("Experimental Snap Sync for BFT enabled");
+    }
+
     if (isBonsaiLimitTrieLogsEnabled) {
       final StringBuilder trieLogPruningString = new StringBuilder();
       trieLogPruningString

besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java

@@ -85,6 +85,8 @@ public class SynchronizerOptions implements CLIOptions<SynchronizerConfiguration
 
   private static final String CHECKPOINT_POST_MERGE_FLAG = "--Xcheckpoint-post-merge-enabled";
 
+  private static final String SNAP_SYNC_BFT_ENABLED_FLAG = "--Xsnapsync-bft-enabled";
+
   /**
    * Parse block propagation range.
    *
@@ -304,6 +306,14 @@ public void parseBlockPropagationRange(final String arg) {
   private Boolean checkpointPostMergeSyncEnabled =
       SynchronizerConfiguration.DEFAULT_CHECKPOINT_POST_MERGE_ENABLED;
 
+  @CommandLine.Option(
+      names = SNAP_SYNC_BFT_ENABLED_FLAG,
+      hidden = true,
+      paramLabel = "<Boolean>",
+      arity = "0..1",
+      description = "Snap sync enabled for BFT chains (default: ${DEFAULT-VALUE})")
+  private Boolean snapsyncBftEnabled = SnapSyncConfiguration.DEFAULT_SNAP_SYNC_BFT_ENABLED;
+
   private SynchronizerOptions() {}
 
   /**
@@ -315,6 +325,15 @@ public boolean isSnapsyncServerEnabled() {
     return snapsyncServerEnabled;
   }
 
+  /**
+   * Flag to know whether the Snap sync should be enabled for a BFT chain
+   *
+   * @return true if snap sync for BFT is enabled
+   */
+  public boolean isSnapSyncBftEnabled() {
+    return snapsyncBftEnabled;
+  }
+
   /**
    * Create synchronizer options.
    *
@@ -365,6 +384,7 @@ public static SynchronizerOptions fromConfig(final SynchronizerConfiguration con
         config.getSnapSyncConfiguration().getLocalFlatStorageCountToHealPerRequest();
     options.checkpointPostMergeSyncEnabled = config.isCheckpointPostMergeEnabled();
     options.snapsyncServerEnabled = config.getSnapSyncConfiguration().isSnapServerEnabled();
+    options.snapsyncBftEnabled = config.getSnapSyncConfiguration().isSnapSyncBftEnabled();
     return options;
   }
 
@@ -397,6 +417,7 @@ public SynchronizerConfiguration.Builder toDomainObject() {
             .localFlatAccountCountToHealPerRequest(snapsyncFlatAccountHealedCountPerRequest)
             .localFlatStorageCountToHealPerRequest(snapsyncFlatStorageHealedCountPerRequest)
             .isSnapServerEnabled(snapsyncServerEnabled)
+            .isSnapSyncBftEnabled(snapsyncBftEnabled)
             .build());
     builder.checkpointPostMergeEnabled(checkpointPostMergeSyncEnabled);
 
@@ -454,7 +475,9 @@ public List<String> getCLIOptions() {
             SNAP_FLAT_STORAGE_HEALED_COUNT_PER_REQUEST_FLAG,
             OptionParser.format(snapsyncFlatStorageHealedCountPerRequest),
             SNAP_SERVER_ENABLED_FLAG,
-            OptionParser.format(snapsyncServerEnabled));
+            OptionParser.format(snapsyncServerEnabled),
+            SNAP_SYNC_BFT_ENABLED_FLAG,
+            OptionParser.format(snapsyncBftEnabled));
     return value;
   }
 }

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

@@ -1166,7 +1166,7 @@ public void checkValidDefaultFastSyncMinPeers() {
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
     assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(5);
+    assertThat(syncConfig.getSyncMinimumPeerCount()).isEqualTo(5);
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
@@ -1178,7 +1178,7 @@ public void parsesValidFastSyncMinPeersOption() {
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
     assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(11);
+    assertThat(syncConfig.getSyncMinimumPeerCount()).isEqualTo(11);
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
@@ -1190,7 +1190,7 @@ public void parsesValidSnapSyncMinPeersOption() {
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
     assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.SNAP);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(11);
+    assertThat(syncConfig.getSyncMinimumPeerCount()).isEqualTo(11);
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
@@ -1202,7 +1202,7 @@ public void parsesValidSyncMinPeersOption() {
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
     assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(11);
+    assertThat(syncConfig.getSyncMinimumPeerCount()).isEqualTo(11);
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }

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

@@ -138,7 +138,7 @@ public void overrideDefaultValuesIfKeyIsPresentInConfigFile(final @TempDir File
     verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
 
     assertThat(syncConfigurationCaptor.getValue().getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfigurationCaptor.getValue().getFastSyncMinimumPeerCount()).isEqualTo(13);
+    assertThat(syncConfigurationCaptor.getValue().getSyncMinimumPeerCount()).isEqualTo(13);
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
@@ -182,7 +182,7 @@ public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() {
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
     assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.SNAP);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(5);
+    assertThat(syncConfig.getSyncMinimumPeerCount()).isEqualTo(5);
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();

besu/src/test/java/org/hyperledger/besu/cli/options/SynchronizerOptionsTest.java

@@ -40,7 +40,7 @@ protected SynchronizerConfiguration.Builder createCustomizedDomainObject() {
     return SynchronizerConfiguration.builder()
         .fastSyncPivotDistance(SynchronizerConfiguration.DEFAULT_PIVOT_DISTANCE_FROM_HEAD + 10)
         .fastSyncFullValidationRate(SynchronizerConfiguration.DEFAULT_FULL_VALIDATION_RATE / 2)
-        .fastSyncMinimumPeerCount(SynchronizerConfiguration.DEFAULT_FAST_SYNC_MINIMUM_PEERS + 2)
+        .fastSyncMinimumPeerCount(SynchronizerConfiguration.DEFAULT_SYNC_MINIMUM_PEERS + 2)
         .worldStateHashCountPerRequest(
             SynchronizerConfiguration.DEFAULT_WORLD_STATE_HASH_COUNT_PER_REQUEST + 2)
         .worldStateRequestParallelism(

consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/BFTPivotSelectorFromPeers.java

@@ -21,7 +21,6 @@
 import org.hyperledger.besu.ethereum.core.Util;
 import org.hyperledger.besu.ethereum.eth.manager.EthContext;
 import org.hyperledger.besu.ethereum.eth.manager.EthPeer;
-import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.NoSyncRequiredException;
@@ -73,7 +72,7 @@ public Optional<FastSyncState> selectNewPivotBlock() {
 
       return bestPeer.flatMap(this::fromBestPeer);
     } else {
-      // Treat single-validator as a special case. We are the only node that can produce
+      // Treat us being the only validator as a special case. We are the only node that can produce
       // blocks so we won't wait to sync with a non-validator node that may or may not exist
       if (validatorProvider.getValidatorsAtHead().size() == 1
           && validatorProvider
@@ -82,20 +81,30 @@ public Optional<FastSyncState> selectNewPivotBlock() {
         throw new NoSyncRequiredException();
       }
 
-      // Treat the case where we have min-peer-count peers who don't have a chain-head estimate but who are all validators as not needing to sync
-      // This is effectively handling the "new chain with N validators" case, but speaks more generally to the BFT case where a BFT chain
-      // prioritises information from other validators over waiting for non-validator peers to respond.
-      AtomicInteger peerValidatorCount = new AtomicInteger();
-      EthPeers theList = ethContext.getEthPeers();
-      theList.getAllActiveConnections().forEach(peer -> {
-        if (validatorProvider
-              .getValidatorsAtHead().contains(peer.getPeerInfo().getAddress())) {
-          peerValidatorCount.getAndIncrement();
-        }
-      });
-      if (peerValidatorCount.get() >= syncConfig.getFastSyncMinimumPeerCount()) {
-        // We have sync-min-peers x validators connected, all of whom have no head estimate. We'll assume this is a new chain
-        // and skip waiting for any more peers to sync with.
+      // Treat the case where we have sync-min-peers peers, who don't have a chain-head estimate but
+      // who are all validators, as not needing to sync
+      // This is effectively handling the "new chain with N validators" case, but speaks more
+      // generally to the BFT case where a BFT chain
+      // prioritises information from other validators over waiting for non-validator peers to
+      // respond.
+      final AtomicInteger peerValidatorCount = new AtomicInteger();
+      ethContext
+          .getEthPeers()
+          .getAllActiveConnections()
+          .forEach(
+              peer -> {
+                if (validatorProvider
+                    .getValidatorsAtHead()
+                    .contains(peer.getPeerInfo().getAddress())) {
+                  peerValidatorCount.getAndIncrement();
+                }
+              });
+
+      if (peerValidatorCount.get() >= syncConfig.getSyncMinimumPeerCount()) {
+        // We have sync-min-peers x validators connected, all of whom have no head estimate. We'll
+        // assume this is a new chain
+        // and skip waiting for any more peers to sync with. The worst case is this puts us into
+        // full sync mode.
         throw new NoSyncRequiredException();
       }
     }

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

@@ -29,7 +29,7 @@ public class SynchronizerConfiguration {
 
   public static final int DEFAULT_PIVOT_DISTANCE_FROM_HEAD = 50;
   public static final float DEFAULT_FULL_VALIDATION_RATE = .1f;
-  public static final int DEFAULT_FAST_SYNC_MINIMUM_PEERS = 5;
+  public static final int DEFAULT_SYNC_MINIMUM_PEERS = 5;
   public static final int DEFAULT_WORLD_STATE_HASH_COUNT_PER_REQUEST = 384;
   public static final int DEFAULT_WORLD_STATE_REQUEST_PARALLELISM = 10;
   public static final int DEFAULT_WORLD_STATE_MAX_REQUESTS_WITHOUT_PROGRESS = 1000;
@@ -55,7 +55,7 @@ public class SynchronizerConfiguration {
   // Fast sync config
   private final int fastSyncPivotDistance;
   private final float fastSyncFullValidationRate;
-  private final int fastSyncMinimumPeerCount;
+  private final int syncMinimumPeerCount;
   private final int worldStateHashCountPerRequest;
   private final int worldStateRequestParallelism;
   private final int worldStateMaxRequestsWithoutProgress;
@@ -89,7 +89,7 @@ public class SynchronizerConfiguration {
   private SynchronizerConfiguration(
       final int fastSyncPivotDistance,
       final float fastSyncFullValidationRate,
-      final int fastSyncMinimumPeerCount,
+      final int syncMinimumPeerCount,
       final int worldStateHashCountPerRequest,
       final int worldStateRequestParallelism,
       final int worldStateMaxRequestsWithoutProgress,
@@ -111,7 +111,7 @@ private SynchronizerConfiguration(
       final boolean checkpointPostMergeEnabled) {
     this.fastSyncPivotDistance = fastSyncPivotDistance;
     this.fastSyncFullValidationRate = fastSyncFullValidationRate;
-    this.fastSyncMinimumPeerCount = fastSyncMinimumPeerCount;
+    this.syncMinimumPeerCount = syncMinimumPeerCount;
     this.worldStateHashCountPerRequest = worldStateHashCountPerRequest;
     this.worldStateRequestParallelism = worldStateRequestParallelism;
     this.worldStateMaxRequestsWithoutProgress = worldStateMaxRequestsWithoutProgress;
@@ -222,8 +222,8 @@ public float getFastSyncFullValidationRate() {
     return fastSyncFullValidationRate;
   }
 
-  public int getFastSyncMinimumPeerCount() {
-    return fastSyncMinimumPeerCount;
+  public int getSyncMinimumPeerCount() {
+    return syncMinimumPeerCount;
   }
 
   public int getWorldStateHashCountPerRequest() {
@@ -256,7 +256,7 @@ public long getPropagationManagerGetBlockTimeoutMillis() {
 
   public static class Builder {
     private SyncMode syncMode = SyncMode.FULL;
-    private int fastSyncMinimumPeerCount = DEFAULT_FAST_SYNC_MINIMUM_PEERS;
+    private int syncMinimumPeerCount = DEFAULT_SYNC_MINIMUM_PEERS;
     private int maxTrailingPeers = Integer.MAX_VALUE;
     private Range<Long> blockPropagationRange = DEFAULT_BLOCK_PROPAGATION_RANGE;
     private long downloaderChangeTargetThresholdByHeight =
@@ -358,7 +358,7 @@ public Builder computationParallelism(final int computationParallelism) {
     }
 
     public Builder fastSyncMinimumPeerCount(final int fastSyncMinimumPeerCount) {
-      this.fastSyncMinimumPeerCount = fastSyncMinimumPeerCount;
+      this.syncMinimumPeerCount = fastSyncMinimumPeerCount;
       return this;
     }
 
@@ -408,7 +408,7 @@ public SynchronizerConfiguration build() {
       return new SynchronizerConfiguration(
           fastSyncPivotDistance,
           fastSyncFullValidationRate,
-          fastSyncMinimumPeerCount,
+          syncMinimumPeerCount,
           worldStateHashCountPerRequest,
           worldStateRequestParallelism,
           worldStateMaxRequestsWithoutProgress,

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActions.java

@@ -142,7 +142,7 @@ private CompletableFuture<FastSyncState> internalDownloadPivotBlockHeader(
                                     ethContext,
                                     metricsSystem,
                                     currentState.getPivotBlockNumber().getAsLong(),
-                                    syncConfig.getFastSyncMinimumPeerCount(),
+                                    syncConfig.getSyncMinimumPeerCount(),
                                     syncConfig.getFastSyncPivotDistance())
                                 .downloadPivotBlockHeader()));
   }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotSelectorFromPeers.java

@@ -66,7 +66,7 @@ public CompletableFuture<Void> prepareRetry() {
                     conservativelyEstimatedPivotBlock(), syncConfig.getMaxTrailingPeers()));
     trailingPeerLimiter.enforceTrailingPeerLimit();
 
-    return waitForPeers(syncConfig.getFastSyncMinimumPeerCount());
+    return waitForPeers(syncConfig.getSyncMinimumPeerCount());
   }
 
   @Override
@@ -96,7 +96,7 @@ protected Optional<EthPeer> selectBestPeer() {
 
   private boolean enoughFastSyncPeersArePresent() {
     final long peerCount = countPeersThatCanDeterminePivotBlock();
-    final int minPeerCount = syncConfig.getFastSyncMinimumPeerCount();
+    final int minPeerCount = syncConfig.getSyncMinimumPeerCount();
     if (peerCount < minPeerCount) {
       LOG.info(
           "Waiting for valid peers with chain height information.  {} / {} required peers currently available.",

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapSyncConfiguration.java

@@ -38,6 +38,8 @@ public class SnapSyncConfiguration {
 
   public static final Boolean DEFAULT_SNAP_SERVER_ENABLED = Boolean.FALSE;
 
+  public static final Boolean DEFAULT_SNAP_SYNC_BFT_ENABLED = Boolean.FALSE;
+
   public static SnapSyncConfiguration getDefault() {
     return ImmutableSnapSyncConfiguration.builder().build();
   }
@@ -81,4 +83,9 @@ public int getLocalFlatStorageCountToHealPerRequest() {
   public Boolean isSnapServerEnabled() {
     return DEFAULT_SNAP_SERVER_ENABLED;
   }
+
+  @Value.Default
+  public Boolean isSnapSyncBftEnabled() {
+    return DEFAULT_SNAP_SYNC_BFT_ENABLED;
+  }
 }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActionsTest.java

@@ -113,7 +113,7 @@ public void setUp(final DataStorageFormat storageFormat) {
   public void waitForPeersShouldSucceedIfEnoughPeersAreFound(
       final DataStorageFormat storageFormat) {
     setUp(storageFormat);
-    for (int i = 0; i < syncConfig.getFastSyncMinimumPeerCount(); i++) {
+    for (int i = 0; i < syncConfig.getSyncMinimumPeerCount(); i++) {
       EthProtocolManagerTestUtil.createPeer(
           ethProtocolManager, syncConfig.getFastSyncPivotDistance() + i + 1);
     }
@@ -435,7 +435,7 @@ public void selectPivotBlockShouldRetryIfBestPeerChainIsEqualToPivotDistance(
     final long pivotDistance = syncConfig.getFastSyncPivotDistance();
     EthProtocolManagerTestUtil.disableEthSchedulerAutoRun(ethProtocolManager);
     // Create peers with chains that are too short
-    for (int i = 0; i < syncConfig.getFastSyncMinimumPeerCount(); i++) {
+    for (int i = 0; i < syncConfig.getSyncMinimumPeerCount(); i++) {
       EthProtocolManagerTestUtil.createPeer(ethProtocolManager, pivotDistance);
     }