Disable bonsai-limit-trie-logs-enabled if sync-mode=FULL (#7357)

There is still a startup error when bonsai-limit-trie-logs-enabled is explicitly set to true --------- Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
hyperledger · Jul 25, 2024 · 4a670a1 · 4a670a1
1 parent c182ba1
commit 4a670a1
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,12 +23,13 @@
 - Added EIP-7702 [#7237](https://github.com/hyperledger/besu/pull/7237)
 - Implement gnark-crypto for eip-196 [#7262](https://github.com/hyperledger/besu/pull/7262)
 - Add trie log pruner metrics [#7352](https://github.com/hyperledger/besu/pull/7352)
+- Force bonsai-limit-trie-logs-enabled=false when sync-mode=FULL instead of startup error [#7357](https://github.com/hyperledger/besu/pull/7357)
 - `--Xbonsai-parallel-tx-processing-enabled` option enables executing transactions in parallel during block processing for Bonsai nodes
-
 - Add option `--poa-discovery-retry-bootnodes` for PoA networks to always use bootnodes during peer refresh, not just on first start [#7314](https://github.com/hyperledger/besu/pull/7314) 
 
 ### Bug fixes
 - Fix `eth_call` deserialization to correctly ignore unknown fields in the transaction object. [#7323](https://github.com/hyperledger/besu/pull/7323)
+- Prevent Besu from starting up with sync-mode=FULL and bonsai-limit-trie-logs-enabled=true for private networks [#7357](https://github.com/hyperledger/besu/pull/7357)
 - Avoid executing pruner preload during trie log subcommands [#7366](https://github.com/hyperledger/besu/pull/7366)
 
 ## 24.7.0

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -139,6 +139,7 @@
 import org.hyperledger.besu.ethereum.storage.keyvalue.KeyValueStorageProviderBuilder;
 import org.hyperledger.besu.ethereum.transaction.TransactionSimulator;
 import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration;
+import org.hyperledger.besu.ethereum.worldstate.ImmutableDataStorageConfiguration;
 import org.hyperledger.besu.evm.precompile.AbstractAltBnPrecompiledContract;
 import org.hyperledger.besu.evm.precompile.BigIntegerModularExponentiationPrecompiledContract;
 import org.hyperledger.besu.evm.precompile.KZGPointEvalPrecompiledContract;
@@ -1587,7 +1588,7 @@ private void validateTransactionPoolOptions() {
   }
 
   private void validateDataStorageOptions() {
-    dataStorageOptions.validate(commandLine, syncMode);
+    dataStorageOptions.validate(commandLine);
   }
 
   private void validateRequiredOptions() {
@@ -2255,6 +2256,32 @@ public DataStorageConfiguration getDataStorageConfiguration() {
     if (dataStorageConfiguration == null) {
       dataStorageConfiguration = dataStorageOptions.toDomainObject();
     }
+
+    if (SyncMode.FULL.equals(getDefaultSyncModeIfNotSet())
+        && DataStorageFormat.BONSAI.equals(dataStorageConfiguration.getDataStorageFormat())
+        && dataStorageConfiguration.getBonsaiLimitTrieLogsEnabled()) {
+
+      if (CommandLineUtils.isOptionSet(
+          commandLine, DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED)) {
+        throw new ParameterException(
+            commandLine,
+            String.format(
+                "Cannot enable %s with --sync-mode=%s and --data-storage-format=%s. You must set %s or use a different sync-mode",
+                DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED,
+                SyncMode.FULL,
+                DataStorageFormat.BONSAI,
+                DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED + "=false"));
+      }
+
+      dataStorageConfiguration =
+          ImmutableDataStorageConfiguration.copyOf(dataStorageConfiguration)
+              .withBonsaiLimitTrieLogsEnabled(false);
+      logger.warn(
+          "Forcing {}, since it cannot be enabled with --sync-mode={} and --data-storage-format={}.",
+          DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED + "=false",
+          SyncMode.FULL,
+          DataStorageFormat.BONSAI);
+    }
     return dataStorageConfiguration;
   }
 

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java
@@ -24,7 +24,6 @@
 
 import org.hyperledger.besu.cli.options.CLIOptions;
 import org.hyperledger.besu.cli.util.CommandLineUtils;
-import org.hyperledger.besu.ethereum.eth.sync.SyncMode;
 import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration;
 import org.hyperledger.besu.ethereum.worldstate.ImmutableDataStorageConfiguration;
 import org.hyperledger.besu.plugin.services.storage.DataStorageFormat;
@@ -63,7 +62,8 @@ public class DataStorageOptions implements CLIOptions<DataStorageConfiguration>
       arity = "1")
   private Long bonsaiMaxLayersToLoad = DEFAULT_BONSAI_MAX_LAYERS_TO_LOAD;
 
-  private static final String BONSAI_LIMIT_TRIE_LOGS_ENABLED = "--bonsai-limit-trie-logs-enabled";
+  /** The bonsai limit trie logs enabled option name */
+  public static final String BONSAI_LIMIT_TRIE_LOGS_ENABLED = "--bonsai-limit-trie-logs-enabled";
 
   /** The bonsai trie logs pruning window size. */
   public static final String BONSAI_TRIE_LOG_PRUNING_WINDOW_SIZE =
@@ -147,20 +147,10 @@ public static DataStorageOptions create() {
    * Validates the data storage options
    *
    * @param commandLine the full commandLine to check all the options specified by the user
-   * @param syncMode the sync mode
    */
-  public void validate(final CommandLine commandLine, final SyncMode syncMode) {
+  public void validate(final CommandLine commandLine) {
     if (DataStorageFormat.BONSAI == dataStorageFormat) {
       if (bonsaiLimitTrieLogsEnabled) {
-        if (SyncMode.FULL == syncMode) {
-          throw new CommandLine.ParameterException(
-              commandLine,
-              String.format(
-                  "Cannot enable %s with sync-mode %s. You must set %s or use a different sync-mode",
-                  BONSAI_LIMIT_TRIE_LOGS_ENABLED,
-                  SyncMode.FULL,
-                  BONSAI_LIMIT_TRIE_LOGS_ENABLED + "=false"));
-        }
         if (bonsaiMaxLayersToLoad < MINIMUM_BONSAI_TRIE_LOG_RETENTION_LIMIT) {
           throw new CommandLine.ParameterException(
               commandLine,

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -65,6 +65,7 @@
 import org.hyperledger.besu.metrics.StandardMetricCategory;
 import org.hyperledger.besu.metrics.prometheus.MetricsConfiguration;
 import org.hyperledger.besu.plugin.data.EnodeURL;
+import org.hyperledger.besu.plugin.services.storage.DataStorageFormat;
 import org.hyperledger.besu.util.number.Fraction;
 import org.hyperledger.besu.util.number.Percentage;
 import org.hyperledger.besu.util.number.PositiveNumber;
@@ -1303,13 +1304,34 @@ public void bonsaiLimitTrieLogsEnabledByDefault() {
   }
 
   @Test
-  public void parsesInvalidDefaultBonsaiLimitTrieLogsWhenFullSyncEnabled() {
+  public void bonsaiLimitTrieLogsDisabledWhenFullSyncEnabled() {
     parseCommand("--sync-mode=FULL");
 
+    verify(mockControllerBuilder)
+        .dataStorageConfiguration(dataStorageConfigurationArgumentCaptor.capture());
+
+    final DataStorageConfiguration dataStorageConfiguration =
+        dataStorageConfigurationArgumentCaptor.getValue();
+    assertThat(dataStorageConfiguration.getDataStorageFormat()).isEqualTo(BONSAI);
+    assertThat(dataStorageConfiguration.getBonsaiLimitTrieLogsEnabled()).isFalse();
+    verify(mockLogger)
+        .warn(
+            "Forcing {}, since it cannot be enabled with --sync-mode={} and --data-storage-format={}.",
+            "--bonsai-limit-trie-logs-enabled=false",
+            SyncMode.FULL,
+            DataStorageFormat.BONSAI);
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void parsesInvalidWhenFullSyncAndBonsaiLimitTrieLogsExplicitlyTrue() {
+    parseCommand("--sync-mode=FULL", "--bonsai-limit-trie-logs-enabled=true");
+
     Mockito.verifyNoInteractions(mockRunnerBuilder);
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8))
-        .contains("Cannot enable --bonsai-limit-trie-logs-enabled with sync-mode FULL");
+        .contains(
+            "Cannot enable --bonsai-limit-trie-logs-enabled with --sync-mode=FULL and --data-storage-format=BONSAI. You must set --bonsai-limit-trie-logs-enabled=false or use a different sync-mode");
   }
 
   @Test

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/DataStorageOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/stable/DataStorageOptionsTest.java
@@ -55,14 +55,6 @@ public void bonsaiTrieLogsEnabled_explicitlySetToFalse() {
         "--bonsai-limit-trie-logs-enabled=false");
   }
 
-  @Test
-  public void bonsaiTrieLogPruningWindowSizeShouldBePositive2() {
-    internalTestFailure(
-        "Cannot enable --bonsai-limit-trie-logs-enabled with sync-mode FULL. You must set --bonsai-limit-trie-logs-enabled=false or use a different sync-mode",
-        "--sync-mode",
-        "FULL");
-  }
-
   @Test
   public void bonsaiTrieLogPruningWindowSizeShouldBePositive() {
     internalTestFailure(