From 4a670a1dbe1da35f31fce75d1e760f195a5b1059 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Fri, 26 Jul 2024 09:16:49 +1000 Subject: [PATCH] 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 --- CHANGELOG.md | 3 +- .../org/hyperledger/besu/cli/BesuCommand.java | 29 ++++++++++++++++++- .../options/stable/DataStorageOptions.java | 16 ++-------- .../hyperledger/besu/cli/BesuCommandTest.java | 26 +++++++++++++++-- .../stable/DataStorageOptionsTest.java | 8 ----- 5 files changed, 57 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b999bca71..c5754da1c9 100644 --- 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 index 1eaab20bd2..d300fbea45 100644 --- 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.KeyValueStorageProvider; 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 @@ public class BesuCommand implements DefaultCommandValues, Runnable { } private void validateDataStorageOptions() { - dataStorageOptions.validate(commandLine, syncMode); + dataStorageOptions.validate(commandLine); } private void validateRequiredOptions() { @@ -2255,6 +2256,32 @@ public class BesuCommand implements DefaultCommandValues, Runnable { 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 index fc92a7e5dd..a0403c3942 100644 --- 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 static org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration. 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 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 class DataStorageOptions implements CLIOptions * 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 index c892ffe7fe..d0c42f6718 100644 --- 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.evm.precompile.KZGPointEvalPrecompiledContract; 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 class BesuCommandTest extends CommandTestAbstract { } @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 index 5ab2757f88..2086381825 100644 --- 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 class DataStorageOptionsTest "--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(