revert "7536 use head for snap sync (#7718)" (#7767)

This reverts commit a7e1f6a.

Signed-off-by: Simon Dudley <[email protected]>

Author: siladu

CHANGELOG.md

@@ -29,7 +29,6 @@
 - Interrupt pending transaction processing on block creation timeout [#7673](https://github.com/hyperledger/besu/pull/7673)
 - Align gas cap calculation for transaction simulation to Geth approach [#7703](https://github.com/hyperledger/besu/pull/7703)
 - Expose chainId in the `BlockchainService` [7702](https://github.com/hyperledger/besu/pull/7702)
-- Add `--Xsnapsync-to-head-enabled` feature to use head block instead of safe block for snap sync [7536](https://github.com/hyperledger/besu/issues/7536)
 - Add support for `chainId` in `CallParameters` [#7720](https://github.com/hyperledger/besu/pull/7720)
 - Add `--ephemery` network support for Ephemery Testnet [#7563](https://github.com/hyperledger/besu/pull/7563) thanks to [@gconnect](https://github.com/gconnect)
 - Add configuration of Consolidation Request Contract Address via genesis configuration [#7647](https://github.com/hyperledger/besu/pull/7647)

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

@@ -2739,7 +2739,6 @@ && getDataStorageConfiguration().getBonsaiLimitTrieLogsEnabled()) {
 
     builder.setSnapServerEnabled(this.unstableSynchronizerOptions.isSnapsyncServerEnabled());
     builder.setSnapSyncBftEnabled(this.unstableSynchronizerOptions.isSnapSyncBftEnabled());
-    builder.setSnapSyncToHeadEnabled(this.unstableSynchronizerOptions.isSnapSyncToHeadEnabled());
 
     builder.setTxPoolImplementation(buildTransactionPoolConfiguration().getTxPoolImplementation());
     builder.setWorldStateUpdateMode(unstableEvmOptions.toDomainObject().worldUpdaterMode());

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

@@ -58,7 +58,6 @@ public class ConfigurationOverviewBuilder {
   private Integer trieLogsPruningWindowSize = null;
   private boolean isSnapServerEnabled = false;
   private boolean isSnapSyncBftEnabled = false;
-  private boolean isSnapSyncToHeadEnabled = true;
   private TransactionPoolConfiguration.Implementation txPoolImplementation;
   private EvmConfiguration.WorldUpdaterMode worldStateUpdateMode;
   private Map<String, String> environment;
@@ -258,18 +257,6 @@ public ConfigurationOverviewBuilder setSnapSyncBftEnabled(final boolean snapSync
     return this;
   }
 
-  /**
-   * Sets snap sync to head enabled/disabled
-   *
-   * @param snapSyncToHeadEnabled bool to indicate if snap sync to head is enabled
-   * @return the builder
-   */
-  public ConfigurationOverviewBuilder setSnapSyncToHeadEnabled(
-      final boolean snapSyncToHeadEnabled) {
-    isSnapSyncToHeadEnabled = snapSyncToHeadEnabled;
-    return this;
-  }
-
   /**
    * Sets trie logs pruning window size
    *
@@ -402,10 +389,6 @@ public String build() {
       lines.add("Experimental Snap Sync for BFT enabled");
     }
 
-    if (isSnapSyncToHeadEnabled) {
-      lines.add("Snap Sync to Head enabled");
-    }
-
     if (isBonsaiLimitTrieLogsEnabled) {
       final StringBuilder trieLogPruningString = new StringBuilder();
       trieLogPruningString

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

@@ -87,8 +87,6 @@ public class SynchronizerOptions implements CLIOptions<SynchronizerConfiguration
 
   private static final String SNAP_SYNC_BFT_ENABLED_FLAG = "--Xsnapsync-bft-enabled";
 
-  private static final String SNAP_SYNC_TO_HEAD_ENABLED_FLAG = "--Xsnapsync-to-head-enabled";
-
   /**
    * Parse block propagation range.
    *
@@ -316,15 +314,6 @@ public void parseBlockPropagationRange(final String arg) {
       description = "Snap sync enabled for BFT chains (default: ${DEFAULT-VALUE})")
   private Boolean snapsyncBftEnabled = SnapSyncConfiguration.DEFAULT_SNAP_SYNC_BFT_ENABLED;
 
-  @CommandLine.Option(
-      names = SNAP_SYNC_TO_HEAD_ENABLED_FLAG,
-      hidden = true,
-      paramLabel = "<Boolean>",
-      arity = "0..1",
-      description = "Snap sync to head enabled (default: ${DEFAULT-VALUE})")
-  private Boolean snapsyncToHeadEnabled =
-      SnapSyncConfiguration.DEFAULT_SNAP_SYNC_TO_HEAD_ENABLED_FLAG;
-
   @CommandLine.Option(
       names = {"--Xpeertask-system-enabled"},
       hidden = true,
@@ -352,15 +341,6 @@ public boolean isSnapSyncBftEnabled() {
     return snapsyncBftEnabled;
   }
 
-  /**
-   * Flag to know if Snap sync should sync to head instead of safe block
-   *
-   * @return true if snap sync should sync to head
-   */
-  public boolean isSnapSyncToHeadEnabled() {
-    return snapsyncToHeadEnabled;
-  }
-
   /**
    * Flag to indicate whether the peer task system should be used where available
    *
@@ -421,7 +401,6 @@ public static SynchronizerOptions fromConfig(final SynchronizerConfiguration con
     options.checkpointPostMergeSyncEnabled = config.isCheckpointPostMergeEnabled();
     options.snapsyncServerEnabled = config.getSnapSyncConfiguration().isSnapServerEnabled();
     options.snapsyncBftEnabled = config.getSnapSyncConfiguration().isSnapSyncBftEnabled();
-    options.snapsyncToHeadEnabled = config.getSnapSyncConfiguration().isSnapSyncToHeadEnabled();
     return options;
   }
 
@@ -455,7 +434,6 @@ public SynchronizerConfiguration.Builder toDomainObject() {
             .localFlatStorageCountToHealPerRequest(snapsyncFlatStorageHealedCountPerRequest)
             .isSnapServerEnabled(snapsyncServerEnabled)
             .isSnapSyncBftEnabled(snapsyncBftEnabled)
-            .isSnapSyncToHeadEnabled(snapsyncToHeadEnabled)
             .build());
     builder.checkpointPostMergeEnabled(checkpointPostMergeSyncEnabled);
     builder.isPeerTaskSystemEnabled(isPeerTaskSystemEnabled);
@@ -515,9 +493,7 @@ public List<String> getCLIOptions() {
             SNAP_SERVER_ENABLED_FLAG,
             OptionParser.format(snapsyncServerEnabled),
             SNAP_SYNC_BFT_ENABLED_FLAG,
-            OptionParser.format(snapsyncBftEnabled),
-            SNAP_SYNC_TO_HEAD_ENABLED_FLAG,
-            OptionParser.format(snapsyncToHeadEnabled));
+            OptionParser.format(snapsyncBftEnabled));
     return value;
   }
 }

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

@@ -65,7 +65,6 @@
 import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector;
 import org.hyperledger.besu.ethereum.eth.sync.SyncMode;
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
-import org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotSelectorFromHeadBlock;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotSelectorFromPeers;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotSelectorFromSafeBlock;
 import org.hyperledger.besu.ethereum.eth.sync.fastsync.checkpoint.Checkpoint;
@@ -891,27 +890,14 @@ private PivotBlockSelector createPivotSelector(
             LOG.info("Initial sync done, unsubscribe forkchoice supplier");
           };
 
-      if (syncConfig.getSnapSyncConfiguration().isSnapSyncToHeadEnabled()) {
-        LOG.info("Using head block for sync.");
-        return new PivotSelectorFromHeadBlock(
-            protocolContext,
-            protocolSchedule,
-            ethContext,
-            metricsSystem,
-            genesisConfigOptions,
-            unverifiedForkchoiceSupplier,
-            unsubscribeForkchoiceListener);
-      } else {
-        LOG.info("Using safe block for sync.");
-        return new PivotSelectorFromSafeBlock(
-            protocolContext,
-            protocolSchedule,
-            ethContext,
-            metricsSystem,
-            genesisConfigOptions,
-            unverifiedForkchoiceSupplier,
-            unsubscribeForkchoiceListener);
-      }
+      return new PivotSelectorFromSafeBlock(
+          protocolContext,
+          protocolSchedule,
+          ethContext,
+          metricsSystem,
+          genesisConfigOptions,
+          unverifiedForkchoiceSupplier,
+          unsubscribeForkchoiceListener);
     } else {
       LOG.info("TTD difficulty is not present, creating initial sync phase for PoW");
       return new PivotSelectorFromPeers(ethContext, syncConfig, syncState, metricsSystem);

besu/src/test/java/org/hyperledger/besu/controller/MergeBesuControllerBuilderTest.java

@@ -45,8 +45,6 @@
 import org.hyperledger.besu.ethereum.core.PrivacyParameters;
 import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration;
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
-import org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotSelectorFromHeadBlock;
-import org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotSelectorFromSafeBlock;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
 import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket;
@@ -76,15 +74,12 @@
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.units.bigints.UInt256;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.io.TempDir;
 import org.mockito.Answers;
 import org.mockito.Mock;
-import org.mockito.MockedConstruction;
-import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
@@ -95,10 +90,7 @@ public class MergeBesuControllerBuilderTest {
 
   @Mock GenesisConfigFile genesisConfigFile;
   @Mock GenesisConfigOptions genesisConfigOptions;
-
-  @Mock(answer = Answers.RETURNS_DEEP_STUBS)
-  SynchronizerConfiguration synchronizerConfiguration;
-
+  @Mock SynchronizerConfiguration synchronizerConfiguration;
   @Mock EthProtocolConfiguration ethProtocolConfiguration;
   @Mock CheckpointConfigOptions checkpointConfigOptions;
 
@@ -154,9 +146,6 @@ public void setup() {
     lenient().when(synchronizerConfiguration.getDownloaderParallelism()).thenReturn(1);
     lenient().when(synchronizerConfiguration.getTransactionsParallelism()).thenReturn(1);
     lenient().when(synchronizerConfiguration.getComputationParallelism()).thenReturn(1);
-    lenient()
-        .when(synchronizerConfiguration.getSnapSyncConfiguration().isSnapSyncToHeadEnabled())
-        .thenReturn(false);
 
     lenient()
         .when(synchronizerConfiguration.getBlockPropagationRange())
@@ -302,32 +291,6 @@ public void assertFinalizedBlockIsPresentWhenStored() {
     assertThat(mergeContext.getFinalized().get()).isEqualTo(finalizedHeader);
   }
 
-  @Test
-  public void assertPivotSelectorFromSafeBlockIsCreated() {
-    MockedConstruction<PivotSelectorFromSafeBlock> mocked =
-        Mockito.mockConstruction(PivotSelectorFromSafeBlock.class);
-    lenient()
-        .when(synchronizerConfiguration.getSnapSyncConfiguration().isSnapSyncToHeadEnabled())
-        .thenReturn(false);
-
-    visitWithMockConfigs(new MergeBesuControllerBuilder()).build();
-
-    Assertions.assertEquals(1, mocked.constructed().size());
-  }
-
-  @Test
-  public void assertPivotSelectorFromHeadBlockIsCreated() {
-    MockedConstruction<PivotSelectorFromHeadBlock> mocked =
-        Mockito.mockConstruction(PivotSelectorFromHeadBlock.class);
-    lenient()
-        .when(synchronizerConfiguration.getSnapSyncConfiguration().isSnapSyncToHeadEnabled())
-        .thenReturn(true);
-
-    visitWithMockConfigs(new MergeBesuControllerBuilder()).build();
-
-    Assertions.assertEquals(1, mocked.constructed().size());
-  }
-
   private BlockHeader finalizedBlockHeader() {
     final long blockNumber = 42;
     final Hash magicHash = Hash.wrap(Bytes32.leftPad(Bytes.ofUnsignedInt(42)));