SNAP and CHECKPOINT sync modes - now production ready (#6405)

* removed X flag from SNAP sync mode
* and X_CHECKPOINT sync-mode

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>


---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

CHANGELOG.md

@@ -11,6 +11,7 @@
 
 
 ### Deprecations
+- X_SNAP and X_CHECKPOINT are marked for deprecation and will be removed in 24.4.0 in favor of SNAP and CHECKPOINT [#6405](https://github.com/hyperledger/besu/pull/6405)
 - `--Xsnapsync-synchronizer-flat-db-healing-enabled` is deprecated (always enabled). [#6499](https://github.com/hyperledger/besu/pull/6499)
 - `--Xp2p-peer-lower-bound` [#6501](https://github.com/hyperledger/besu/pull/6501)
 
@@ -21,6 +22,8 @@
 - Log blob count when importing a block via Engine API [#6466](https://github.com/hyperledger/besu/pull/6466)
 - Introduce `--Xbonsai-limit-trie-logs-enabled` experimental feature which by default will only retain the latest 512 trie logs, saving about 3GB per week in database growth [#5390](https://github.com/hyperledger/besu/issues/5390)
 - Introduce `besu storage x-trie-log prune` experimental offline subcommand which will prune all redundant trie logs except the latest 512 [#6303](https://github.com/hyperledger/besu/pull/6303)
+- SNAP and CHECKPOINT sync - early access flag removed so now simply SNAP and CHECKPOINT [#6405](https://github.com/hyperledger/besu/pull/6405)
+  - X_SNAP and X_CHECKPOINT are marked for deprecation and will be removed in 24.4.0
 - Github Actions based build.
 - Introduce caching mechanism to optimize Keccak hash calculations for account storage slots during block processing [#6452](https://github.com/hyperledger/besu/pull/6452)
 - Added configuration options for `pragueTime` to genesis file for Prague fork development [#6473](https://github.com/hyperledger/besu/pull/6473)

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

@@ -1608,8 +1608,8 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
 
     CommandLineUtils.failIfOptionDoesntMeetRequirement(
         commandLine,
-        "--Xcheckpoint-post-merge-enabled can only be used with X_CHECKPOINT sync-mode",
-        SyncMode.X_CHECKPOINT.equals(getDefaultSyncModeIfNotSet()),
+        "--Xcheckpoint-post-merge-enabled can only be used with CHECKPOINT sync-mode",
+        SyncMode.isCheckpointSync(getDefaultSyncModeIfNotSet()),
         singletonList("--Xcheckpoint-post-merge-enabled"));
 
     if (!securityModuleName.equals(DEFAULT_SECURITY_MODULE)

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

@@ -1147,8 +1147,7 @@ public abstract class BesuControllerBuilder implements MiningParameterOverrides
 
     final CheckpointConfigOptions checkpointConfigOptions =
         genesisConfig.getConfigOptions(genesisConfigOverrides).getCheckpointOptions();
-    if (SyncMode.X_CHECKPOINT.equals(syncConfig.getSyncMode())
-        && checkpointConfigOptions.isValid()) {
+    if (SyncMode.isCheckpointSync(syncConfig.getSyncMode()) && checkpointConfigOptions.isValid()) {
       validators.add(
           new CheckpointBlocksPeerValidator(
               protocolSchedule,

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

@@ -1134,7 +1134,7 @@ public class BesuCommandTest extends CommandTestAbstract {
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8))
         .contains(
-            "Invalid value for option '--sync-mode': expected one of [FULL, FAST, X_SNAP, X_CHECKPOINT] (case-insensitive) but was 'bogus'");
+            "Invalid value for option '--sync-mode': expected one of [FULL, FAST, SNAP, CHECKPOINT, X_SNAP, X_CHECKPOINT] (case-insensitive) but was 'bogus'");
   }
 
   @Test
@@ -1187,11 +1187,11 @@ public class BesuCommandTest extends CommandTestAbstract {
 
   @Test
   public void parsesValidSnapSyncMinPeersOption() {
-    parseCommand("--sync-mode", "X_SNAP", "--sync-min-peers", "11");
+    parseCommand("--sync-mode", "SNAP", "--sync-min-peers", "11");
     verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
-    assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.X_SNAP);
+    assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.SNAP);
     assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(11);
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
@@ -3192,7 +3192,7 @@ public class BesuCommandTest extends CommandTestAbstract {
         "--genesis-file",
         genesisFile.toString(),
         "--sync-mode",
-        "X_CHECKPOINT",
+        "CHECKPOINT",
         "--Xcheckpoint-post-merge-enabled");
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
@@ -3203,7 +3203,7 @@ public class BesuCommandTest extends CommandTestAbstract {
   @Test
   public void checkpointPostMergeShouldFailWhenGenesisUsesCheckpointFromPreMerge() {
     // using the default genesis which has a checkpoint sync block prior to the merge
-    parseCommand("--sync-mode", "X_CHECKPOINT", "--Xcheckpoint-post-merge-enabled");
+    parseCommand("--sync-mode", "CHECKPOINT", "--Xcheckpoint-post-merge-enabled");
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8))
@@ -3214,9 +3214,9 @@ public class BesuCommandTest extends CommandTestAbstract {
   @Test
   public void checkpointPostMergeShouldFailWhenSyncModeIsNotCheckpoint() {
 
-    parseCommand("--sync-mode", "X_SNAP", "--Xcheckpoint-post-merge-enabled");
+    parseCommand("--sync-mode", "SNAP", "--Xcheckpoint-post-merge-enabled");
     assertThat(commandErrorOutput.toString(UTF_8))
-        .contains("--Xcheckpoint-post-merge-enabled can only be used with X_CHECKPOINT sync-mode");
+        .contains("--Xcheckpoint-post-merge-enabled can only be used with CHECKPOINT sync-mode");
   }
 
   @Test
@@ -3231,7 +3231,7 @@ public class BesuCommandTest extends CommandTestAbstract {
         "--genesis-file",
         genesisFile.toString(),
         "--sync-mode",
-        "X_CHECKPOINT",
+        "CHECKPOINT",
         "--Xcheckpoint-post-merge-enabled");
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
@@ -3250,7 +3250,7 @@ public class BesuCommandTest extends CommandTestAbstract {
         "--genesis-file",
         genesisFile.toString(),
         "--sync-mode",
-        "X_CHECKPOINT",
+        "CHECKPOINT",
         "--Xcheckpoint-post-merge-enabled");
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
@@ -3269,7 +3269,7 @@ public class BesuCommandTest extends CommandTestAbstract {
         "--genesis-file",
         genesisFile.toString(),
         "--sync-mode",
-        "X_CHECKPOINT",
+        "CHECKPOINT",
         "--Xcheckpoint-post-merge-enabled");
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();

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

@@ -130,7 +130,7 @@ public class BesuControllerTest {
 
     final BesuControllerBuilder besuControllerBuilder =
         new BesuController.Builder()
-            .fromGenesisConfig(postMergeGenesisFile, Collections.emptyMap(), SyncMode.X_CHECKPOINT);
+            .fromGenesisConfig(postMergeGenesisFile, Collections.emptyMap(), SyncMode.CHECKPOINT);
 
     assertThat(besuControllerBuilder).isInstanceOf(MergeBesuControllerBuilder.class);
   }
@@ -147,7 +147,7 @@ public class BesuControllerTest {
 
     final BesuControllerBuilder besuControllerBuilder =
         new BesuController.Builder()
-            .fromGenesisConfig(mergeAtGenesisFile, Collections.emptyMap(), SyncMode.X_CHECKPOINT);
+            .fromGenesisConfig(mergeAtGenesisFile, Collections.emptyMap(), SyncMode.CHECKPOINT);
 
     assertThat(besuControllerBuilder).isInstanceOf(TransitionBesuControllerBuilder.class);
   }
@@ -156,7 +156,7 @@ public class BesuControllerTest {
   public void preMergeCheckpointSyncUsesTransitionControllerBuilder() {
     final BesuControllerBuilder besuControllerBuilder =
         new BesuController.Builder()
-            .fromGenesisConfig(genesisConfigFile, Collections.emptyMap(), SyncMode.X_CHECKPOINT);
+            .fromGenesisConfig(genesisConfigFile, Collections.emptyMap(), SyncMode.CHECKPOINT);
 
     assertThat(besuControllerBuilder).isInstanceOf(TransitionBesuControllerBuilder.class);
   }
@@ -165,7 +165,7 @@ public class BesuControllerTest {
   public void nonCheckpointSyncUsesTransitionControllerBuild() {
     final BesuControllerBuilder besuControllerBuilder =
         new BesuController.Builder()
-            .fromGenesisConfig(genesisConfigFile, Collections.emptyMap(), SyncMode.X_SNAP);
+            .fromGenesisConfig(genesisConfigFile, Collections.emptyMap(), SyncMode.SNAP);
 
     assertThat(besuControllerBuilder).isInstanceOf(TransitionBesuControllerBuilder.class);
   }

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

@@ -142,7 +142,7 @@ public class DefaultSynchronizer implements Synchronizer, UnverifiedForkchoiceLi
                   worldStateStorage,
                   syncState,
                   clock);
-    } else if (SyncMode.X_CHECKPOINT.equals(syncConfig.getSyncMode())) {
+    } else if (SyncMode.isCheckpointSync(syncConfig.getSyncMode())) {
       this.fastSyncFactory =
           () ->
               CheckpointDownloaderFactory.createCheckpointDownloader(

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

@@ -24,8 +24,11 @@ public enum SyncMode {
   // Perform light validation on older blocks, and switch to full validation for more recent blocks
   FAST,
   // Perform snapsync
-  X_SNAP,
+  SNAP,
   // Perform snapsync but starting from a checkpoint instead of starting from genesis
+  CHECKPOINT,
+  // Deprecated and will be removed in 24.4.0 (X_SNAP and X_CHECKPOINT)
+  X_SNAP,
   X_CHECKPOINT;
 
   public String normalize() {
@@ -38,10 +41,16 @@ public enum SyncMode {
   }
 
   public static boolean isFullSync(final SyncMode syncMode) {
-    return !EnumSet.of(SyncMode.FAST, SyncMode.X_SNAP, SyncMode.X_CHECKPOINT).contains(syncMode);
+    return !EnumSet.of(
+            SyncMode.FAST,
+            SyncMode.SNAP,
+            SyncMode.X_SNAP,
+            SyncMode.CHECKPOINT,
+            SyncMode.X_CHECKPOINT)
+        .contains(syncMode);
   }
 
   public static boolean isCheckpointSync(final SyncMode syncMode) {
-    return syncMode.equals(X_CHECKPOINT);
+    return X_CHECKPOINT.equals(syncMode) || CHECKPOINT.equals(syncMode);
   }
 }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java

@@ -1152,9 +1152,9 @@ public final class EthProtocolManagerTest {
 
   @Test
   public void shouldUseRightCapabilityDependingOnSyncMode() {
-    assertHighestCapability(SyncMode.X_SNAP, EthProtocol.ETH68);
+    assertHighestCapability(SyncMode.SNAP, EthProtocol.ETH68);
     assertHighestCapability(SyncMode.FULL, EthProtocol.ETH68);
-    assertHighestCapability(SyncMode.X_CHECKPOINT, EthProtocol.ETH68);
+    assertHighestCapability(SyncMode.CHECKPOINT, EthProtocol.ETH68);
     /* Eth67 does not support fast sync, see EIP-4938 */
     assertHighestCapability(SyncMode.FAST, EthProtocol.ETH66);
   }
@@ -1166,9 +1166,9 @@ public final class EthProtocolManagerTest {
     final EthProtocolConfiguration configuration =
         EthProtocolConfiguration.builder().maxEthCapability(EthProtocolVersion.V65).build();
 
-    assertHighestCapability(SyncMode.X_SNAP, EthProtocol.ETH65, configuration);
+    assertHighestCapability(SyncMode.SNAP, EthProtocol.ETH65, configuration);
     assertHighestCapability(SyncMode.FULL, EthProtocol.ETH65, configuration);
-    assertHighestCapability(SyncMode.X_CHECKPOINT, EthProtocol.ETH65, configuration);
+    assertHighestCapability(SyncMode.CHECKPOINT, EthProtocol.ETH65, configuration);
     /* Eth67 does not support fast sync, see EIP-4938 */
     assertHighestCapability(SyncMode.FAST, EthProtocol.ETH65, configuration);
   }
@@ -1180,7 +1180,7 @@ public final class EthProtocolManagerTest {
     final EthProtocolConfiguration configuration =
         EthProtocolConfiguration.builder().minEthCapability(EthProtocolVersion.V64).build();
 
-    final EthProtocolManager ethManager = createEthManager(SyncMode.X_SNAP, configuration);
+    final EthProtocolManager ethManager = createEthManager(SyncMode.SNAP, configuration);
 
     assertThat(ethManager.getSupportedCapabilities()).contains(EthProtocol.ETH64);
     assertThat(ethManager.getSupportedCapabilities()).doesNotContain(EthProtocol.ETH63);
@@ -1193,9 +1193,9 @@ public final class EthProtocolManagerTest {
     final EthProtocolConfiguration configuration =
         EthProtocolConfiguration.builder().maxEthCapability(EthProtocolVersion.V67).build();
 
-    assertHighestCapability(SyncMode.X_SNAP, EthProtocol.ETH67, configuration);
+    assertHighestCapability(SyncMode.SNAP, EthProtocol.ETH67, configuration);
     assertHighestCapability(SyncMode.FULL, EthProtocol.ETH67, configuration);
-    assertHighestCapability(SyncMode.X_CHECKPOINT, EthProtocol.ETH67, configuration);
+    assertHighestCapability(SyncMode.CHECKPOINT, EthProtocol.ETH67, configuration);
     /* Eth67 does not support fast sync, see EIP-4938 */
     assertHighestCapability(SyncMode.FAST, EthProtocol.ETH66, configuration);
   }