From 3598ad35edf44ee6ebf9d307b06d511f7899bc1c Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Mon, 22 Jul 2024 13:39:37 +1000 Subject: [PATCH 1/5] Disable bonsai-limit-trie-logs-enabled if sync-mode=FULL Instead of preventing startup Signed-off-by: Simon Dudley --- .../org/hyperledger/besu/cli/BesuCommand.java | 16 +++++++++++++- .../options/stable/DataStorageOptions.java | 16 +++----------- .../hyperledger/besu/cli/BesuCommandTest.java | 22 ++++++++++++++----- .../stable/DataStorageOptionsTest.java | 8 ------- 4 files changed, 35 insertions(+), 27 deletions(-) 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 198c26fe2a8..b5ad3744311 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.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; @@ -1574,7 +1575,7 @@ private void validateTransactionPoolOptions() { } private void validateDataStorageOptions() { - dataStorageOptions.validate(commandLine, syncMode); + dataStorageOptions.validate(commandLine); } private void validateRequiredOptions() { @@ -2237,6 +2238,19 @@ private DataStorageConfiguration getDataStorageConfiguration() { if (dataStorageConfiguration == null) { dataStorageConfiguration = dataStorageOptions.toDomainObject(); } + if (SyncMode.FULL.equals(syncMode) + && DataStorageFormat.BONSAI.equals(dataStorageConfiguration.getDataStorageFormat()) + && dataStorageConfiguration.getBonsaiLimitTrieLogsEnabled()) { + dataStorageConfiguration = + ImmutableDataStorageConfiguration.copyOf(dataStorageConfiguration) + .withBonsaiLimitTrieLogsEnabled(false); + logger.warn( + "Cannot enable {} with --sync-mode {} and --data-storage-format {}. {} has been automatically disabled.", + DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED, + SyncMode.FULL, + DataStorageFormat.BONSAI, + DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED); + } 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 fc92a7e5dd9..a0403c39420 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 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 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 index efeff412d62..9888e66643b 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.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; @@ -1281,13 +1282,24 @@ public void bonsaiLimitTrieLogsEnabledByDefault() { } @Test - public void parsesInvalidDefaultBonsaiLimitTrieLogsWhenFullSyncEnabled() { + public void bonsaiLimitTrieLogsDisabledWhenFullSyncEnabled() { parseCommand("--sync-mode=FULL"); - 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"); + verify(mockControllerBuilder) + .dataStorageConfiguration(dataStorageConfigurationArgumentCaptor.capture()); + + final DataStorageConfiguration dataStorageConfiguration = + dataStorageConfigurationArgumentCaptor.getValue(); + assertThat(dataStorageConfiguration.getDataStorageFormat()).isEqualTo(BONSAI); + assertThat(dataStorageConfiguration.getBonsaiLimitTrieLogsEnabled()).isFalse(); + verify(mockLogger) + .warn( + "Cannot enable {} with --sync-mode {} and --data-storage-format {}. {} has been automatically disabled.", + "--bonsai-limit-trie-logs-enabled", + SyncMode.FULL, + DataStorageFormat.BONSAI, + "--bonsai-limit-trie-logs-enabled"); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); } @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 5ab2757f888..2086381825f 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 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( From ece5ba2a388fa76e27c8ef8f3821c5c094795a54 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Mon, 22 Jul 2024 15:41:04 +1000 Subject: [PATCH 2/5] Startup error on explicit true Signed-off-by: Simon Dudley --- .../org/hyperledger/besu/cli/BesuCommand.java | 20 ++++++++++++++++--- .../hyperledger/besu/cli/BesuCommandTest.java | 15 ++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) 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 b5ad3744311..a14d5e35cae 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -2238,18 +2238,32 @@ private DataStorageConfiguration getDataStorageConfiguration() { if (dataStorageConfiguration == null) { dataStorageConfiguration = dataStorageOptions.toDomainObject(); } - if (SyncMode.FULL.equals(syncMode) + + 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( - "Cannot enable {} with --sync-mode {} and --data-storage-format {}. {} has been automatically disabled.", + "Cannot enable {} with --sync-mode={} and --data-storage-format={}. {} has been set automatically.", DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED, SyncMode.FULL, DataStorageFormat.BONSAI, - DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED); + DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED + "=false"); } return dataStorageConfiguration; } 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 9888e66643b..7c2697a8a8c 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -1294,14 +1294,25 @@ public void bonsaiLimitTrieLogsDisabledWhenFullSyncEnabled() { assertThat(dataStorageConfiguration.getBonsaiLimitTrieLogsEnabled()).isFalse(); verify(mockLogger) .warn( - "Cannot enable {} with --sync-mode {} and --data-storage-format {}. {} has been automatically disabled.", + "Cannot enable {} with --sync-mode={} and --data-storage-format={}. {} has been set automatically.", "--bonsai-limit-trie-logs-enabled", SyncMode.FULL, DataStorageFormat.BONSAI, - "--bonsai-limit-trie-logs-enabled"); + "--bonsai-limit-trie-logs-enabled=false"); 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 and --data-storage-format=BONSAI. You must set --bonsai-limit-trie-logs-enabled=false or use a different sync-mode"); + } + @Test public void parsesValidBonsaiHistoricalBlockLimitOption() { parseCommand( From fbd43589983bb03c5fde0a8f5fa31fe7c3f6f527 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Mon, 22 Jul 2024 15:46:58 +1000 Subject: [PATCH 3/5] changelog Signed-off-by: Simon Dudley --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c6cb373ba8..ffded48b2d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,10 +23,12 @@ - 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) +- Automatically set 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 ### 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) ## 24.7.0 From 2233664aafbf11d390d718b2e5c750def91d725c Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Mon, 22 Jul 2024 16:23:18 +1000 Subject: [PATCH 4/5] Align wording for "Forcing" options Signed-off-by: Simon Dudley --- .../main/java/org/hyperledger/besu/cli/BesuCommand.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 a14d5e35cae..bf084ab9328 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -2259,11 +2259,10 @@ private DataStorageConfiguration getDataStorageConfiguration() { ImmutableDataStorageConfiguration.copyOf(dataStorageConfiguration) .withBonsaiLimitTrieLogsEnabled(false); logger.warn( - "Cannot enable {} with --sync-mode={} and --data-storage-format={}. {} has been set automatically.", - DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED, + "Forcing {}, since it cannot be enabled with --sync-mode={} and --data-storage-format={}.", + DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED + "=false", SyncMode.FULL, - DataStorageFormat.BONSAI, - DataStorageOptions.BONSAI_LIMIT_TRIE_LOGS_ENABLED + "=false"); + DataStorageFormat.BONSAI); } return dataStorageConfiguration; } From 851c535d999388736c434ce86b7ab907b9197b65 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Mon, 22 Jul 2024 16:25:49 +1000 Subject: [PATCH 5/5] Align forcing wording test and changelog Signed-off-by: Simon Dudley --- CHANGELOG.md | 2 +- .../java/org/hyperledger/besu/cli/BesuCommandTest.java | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffded48b2d4..81cd27b13c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ - 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) -- Automatically set bonsai-limit-trie-logs-enabled=false when sync-mode=FULL instead of startup error [#7357](https://github.com/hyperledger/besu/pull/7357) +- 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 ### Bug fixes 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 7c2697a8a8c..a4efb432927 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -1294,11 +1294,10 @@ public void bonsaiLimitTrieLogsDisabledWhenFullSyncEnabled() { assertThat(dataStorageConfiguration.getBonsaiLimitTrieLogsEnabled()).isFalse(); verify(mockLogger) .warn( - "Cannot enable {} with --sync-mode={} and --data-storage-format={}. {} has been set automatically.", - "--bonsai-limit-trie-logs-enabled", + "Forcing {}, since it cannot be enabled with --sync-mode={} and --data-storage-format={}.", + "--bonsai-limit-trie-logs-enabled=false", SyncMode.FULL, - DataStorageFormat.BONSAI, - "--bonsai-limit-trie-logs-enabled=false"); + DataStorageFormat.BONSAI); assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); }