Change default for receipt compaction to be enabled (hyperledger#7450)

Signed-off-by: Jason Frame <jason.frame@consensys.net> Signed-off-by: Ade Lucas <ade.lucas@consensys.net>
cloudspores · Aug 22, 2024 · bd850c6 · bd850c6
1 parent c4f6b00
commit bd850c6
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Upcoming Breaking Changes
 
 ### Breaking Changes
+- Receipt compaction is enabled by default. It will no longer be possible to downgrade Besu to versions prior to 24.5.1.
 
 ### Additions and Improvements
 - Add 'inbound' field to admin_peers JSON-RPC Call [#7461](https://github.com/hyperledger/besu/pull/7461)

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
@@ -95,8 +95,8 @@ public class DataStorageOptions implements CLIOptions<DataStorageConfiguration>
 
   @Option(
       names = "--receipt-compaction-enabled",
-      description = "Enables compact storing of receipts (default: ${DEFAULT-VALUE}).",
-      arity = "1")
+      description = "Enables compact storing of receipts (default: ${DEFAULT-VALUE})",
+      fallbackValue = "true")
   private Boolean receiptCompactionEnabled = DEFAULT_RECEIPT_COMPACTION_ENABLED;
 
   @CommandLine.ArgGroup(validate = false)

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
@@ -126,22 +126,28 @@ public void bonsaiCodeUsingCodeHashEnabledCanBeDisabled() {
         "false");
   }
 
+  @Test
+  public void receiptCompactionCanBeEnabledWithImplicitTrueValue() {
+    internalTestSuccess(
+        dataStorageConfiguration ->
+            assertThat(dataStorageConfiguration.getReceiptCompactionEnabled()).isEqualTo(true),
+        "--receipt-compaction-enabled");
+  }
+
   @Test
   public void receiptCompactionCanBeEnabled() {
     internalTestSuccess(
         dataStorageConfiguration ->
             assertThat(dataStorageConfiguration.getReceiptCompactionEnabled()).isEqualTo(true),
-        "--receipt-compaction-enabled",
-        "true");
+        "--receipt-compaction-enabled=true");
   }
 
   @Test
   public void receiptCompactionCanBeDisabled() {
     internalTestSuccess(
         dataStorageConfiguration ->
             assertThat(dataStorageConfiguration.getReceiptCompactionEnabled()).isEqualTo(false),
-        "--receipt-compaction-enabled",
-        "false");
+        "--receipt-compaction-enabled=false");
   }
 
   @Override

diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/worldstate/DataStorageConfiguration.java b/...core/src/main/java/org/hyperledger/besu/ethereum/worldstate/DataStorageConfiguration.java
@@ -26,7 +26,7 @@ public interface DataStorageConfiguration {
   boolean DEFAULT_BONSAI_LIMIT_TRIE_LOGS_ENABLED = true;
   long MINIMUM_BONSAI_TRIE_LOG_RETENTION_LIMIT = DEFAULT_BONSAI_MAX_LAYERS_TO_LOAD;
   int DEFAULT_BONSAI_TRIE_LOG_PRUNING_WINDOW_SIZE = 5_000;
-  boolean DEFAULT_RECEIPT_COMPACTION_ENABLED = false;
+  boolean DEFAULT_RECEIPT_COMPACTION_ENABLED = true;
 
   DataStorageConfiguration DEFAULT_CONFIG =
       ImmutableDataStorageConfiguration.builder()

diff --git a/...yperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactory.java b/...yperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactory.java
@@ -45,9 +45,7 @@ public class RocksDBKeyValuePrivacyStorageFactory implements PrivacyKeyValueStor
       LoggerFactory.getLogger(RocksDBKeyValuePrivacyStorageFactory.class);
   private static final Set<PrivacyVersionedStorageFormat> SUPPORTED_VERSIONS =
       EnumSet.of(
-          PrivacyVersionedStorageFormat.FOREST_WITH_VARIABLES,
           PrivacyVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION,
-          PrivacyVersionedStorageFormat.BONSAI_WITH_VARIABLES,
           PrivacyVersionedStorageFormat.BONSAI_WITH_RECEIPT_COMPACTION);
   private static final String PRIVATE_DATABASE_PATH = "private";
   private final RocksDBKeyValueStorageFactory publicFactory;
@@ -230,8 +228,12 @@ private Optional<DatabaseMetadata> handleVersionUpgrade(
     // reflect the change to the runtime version, and return it.
 
     // Besu supports both formats of receipts so no upgrade is needed other than updating metadata
-    if (runtimeVersion == PrivacyVersionedStorageFormat.BONSAI_WITH_RECEIPT_COMPACTION
-        || runtimeVersion == PrivacyVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION) {
+    final VersionedStorageFormat existingVersionedStorageFormat =
+        existingPrivacyMetadata.getVersionedStorageFormat();
+    if ((existingVersionedStorageFormat == PrivacyVersionedStorageFormat.BONSAI_WITH_VARIABLES
+            && runtimeVersion == PrivacyVersionedStorageFormat.BONSAI_WITH_RECEIPT_COMPACTION)
+        || (existingVersionedStorageFormat == PrivacyVersionedStorageFormat.FOREST_WITH_VARIABLES
+            && runtimeVersion == PrivacyVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION)) {
       final DatabaseMetadata metadata = new DatabaseMetadata(runtimeVersion);
       try {
         metadata.writeToDirectory(dataDir);
@@ -247,8 +249,8 @@ private Optional<DatabaseMetadata> handleVersionUpgrade(
             "Database unsafe upgrade detect: DB at %s is %s with version %s but version %s is expected. "
                 + "Please check your config and review release notes for supported upgrade procedures.",
             dataDir,
-            existingPrivacyMetadata.getVersionedStorageFormat().getFormat().name(),
-            existingPrivacyMetadata.getVersionedStorageFormat().getVersion(),
+            existingVersionedStorageFormat.getFormat().name(),
+            existingVersionedStorageFormat.getVersion(),
             runtimeVersion.getVersion());
 
     throw new StorageException(error);

diff --git a/...a/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java b/...a/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java
@@ -57,11 +57,7 @@ public class RocksDBKeyValueStorageFactory implements KeyValueStorageFactory {
 
   private static final Logger LOG = LoggerFactory.getLogger(RocksDBKeyValueStorageFactory.class);
   private static final EnumSet<BaseVersionedStorageFormat> SUPPORTED_VERSIONED_FORMATS =
-      EnumSet.of(
-          FOREST_WITH_VARIABLES,
-          FOREST_WITH_RECEIPT_COMPACTION,
-          BONSAI_WITH_VARIABLES,
-          BONSAI_WITH_RECEIPT_COMPACTION);
+      EnumSet.of(FOREST_WITH_RECEIPT_COMPACTION, BONSAI_WITH_RECEIPT_COMPACTION);
   private static final String NAME = "rocksdb";
   private final RocksDBMetricsFactory rocksDBMetricsFactory;
   private DatabaseMetadata databaseMetadata;
@@ -329,8 +325,12 @@ private Optional<DatabaseMetadata> handleVersionUpgrade(
     // reflect the change to the runtime version, and return it.
 
     // Besu supports both formats of receipts so no upgrade is needed other than updating metadata
-    if (runtimeVersion == BONSAI_WITH_RECEIPT_COMPACTION
-        || runtimeVersion == FOREST_WITH_RECEIPT_COMPACTION) {
+    final VersionedStorageFormat existingVersionedStorageFormat =
+        existingMetadata.getVersionedStorageFormat();
+    if ((existingVersionedStorageFormat == BONSAI_WITH_VARIABLES
+            && runtimeVersion == BONSAI_WITH_RECEIPT_COMPACTION)
+        || (existingVersionedStorageFormat == FOREST_WITH_VARIABLES
+            && runtimeVersion == FOREST_WITH_RECEIPT_COMPACTION)) {
       final DatabaseMetadata metadata = new DatabaseMetadata(runtimeVersion);
       try {
         metadata.writeToDirectory(dataDir);
@@ -346,8 +346,8 @@ private Optional<DatabaseMetadata> handleVersionUpgrade(
             "Database unsafe downgrade detect: DB at %s is %s with version %s but version %s is expected. "
                 + "Please check your config and review release notes for supported downgrade procedures.",
             dataDir,
-            existingMetadata.getVersionedStorageFormat().getFormat().name(),
-            existingMetadata.getVersionedStorageFormat().getVersion(),
+            existingVersionedStorageFormat.getFormat().name(),
+            existingVersionedStorageFormat.getVersion(),
             runtimeVersion.getVersion());
 
     throw new StorageException(error);

diff --git a/...ledger/besu/plugin/services/storage/rocksdb/configuration/BaseVersionedStorageFormat.java b/...ledger/besu/plugin/services/storage/rocksdb/configuration/BaseVersionedStorageFormat.java
@@ -63,14 +63,8 @@ public enum BaseVersionedStorageFormat implements VersionedStorageFormat {
   public static BaseVersionedStorageFormat defaultForNewDB(
       final DataStorageConfiguration configuration) {
     return switch (configuration.getDatabaseFormat()) {
-      case FOREST ->
-          configuration.getReceiptCompactionEnabled()
-              ? FOREST_WITH_RECEIPT_COMPACTION
-              : FOREST_WITH_VARIABLES;
-      case BONSAI ->
-          configuration.getReceiptCompactionEnabled()
-              ? BONSAI_WITH_RECEIPT_COMPACTION
-              : BONSAI_WITH_VARIABLES;
+      case FOREST -> FOREST_WITH_RECEIPT_COMPACTION;
+      case BONSAI -> BONSAI_WITH_RECEIPT_COMPACTION;
     };
   }
 

diff --git a/.../org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java b/.../org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java
@@ -74,7 +74,7 @@ public static DatabaseMetadata defaultForNewDb(final BesuConfiguration besuConfi
    * @return the metadata to use for new db
    */
   public static DatabaseMetadata defaultForNewPrivateDb() {
-    return new DatabaseMetadata(PrivacyVersionedStorageFormat.FOREST_WITH_VARIABLES);
+    return new DatabaseMetadata(PrivacyVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION);
   }
 
   /**

diff --git a/...ger/besu/plugin/services/storage/rocksdb/configuration/PrivacyVersionedStorageFormat.java b/...ger/besu/plugin/services/storage/rocksdb/configuration/PrivacyVersionedStorageFormat.java
@@ -32,7 +32,7 @@ public enum PrivacyVersionedStorageFormat implements VersionedStorageFormat {
    * Current Forest version, with receipts using compaction, in order to make Receipts use less disk
    * space
    */
-  FOREST_WITH_RECEIPT_COMPACTION(BaseVersionedStorageFormat.FOREST_WITH_VARIABLES, 2),
+  FOREST_WITH_RECEIPT_COMPACTION(BaseVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION, 2),
   /** Original Bonsai version, not used since replace by BONSAI_WITH_VARIABLES */
   BONSAI_ORIGINAL(BaseVersionedStorageFormat.BONSAI_ORIGINAL, 1),
   /**
@@ -64,14 +64,8 @@ public enum PrivacyVersionedStorageFormat implements VersionedStorageFormat {
   public static VersionedStorageFormat defaultForNewDB(
       final DataStorageConfiguration configuration) {
     return switch (configuration.getDatabaseFormat()) {
-      case FOREST ->
-          configuration.getReceiptCompactionEnabled()
-              ? FOREST_WITH_RECEIPT_COMPACTION
-              : FOREST_WITH_VARIABLES;
-      case BONSAI ->
-          configuration.getReceiptCompactionEnabled()
-              ? BONSAI_WITH_RECEIPT_COMPACTION
-              : BONSAI_WITH_VARIABLES;
+      case FOREST -> FOREST_WITH_RECEIPT_COMPACTION;
+      case BONSAI -> BONSAI_WITH_RECEIPT_COMPACTION;
     };
   }
 

diff --git a/...ledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactoryTest.java b/...ledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactoryTest.java
@@ -77,7 +77,7 @@ public void shouldDetectVersion1MetadataIfPresent() throws Exception {
     try (final var storage = storageFactory.create(segment, commonConfiguration, metricsSystem)) {
 
       assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersionedStorageFormat())
-          .isEqualTo(PrivacyVersionedStorageFormat.FOREST_WITH_VARIABLES);
+          .isEqualTo(PrivacyVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION);
     }
   }
 
@@ -97,7 +97,7 @@ public void shouldCreateCorrectMetadataFileForLatestVersion() throws Exception {
     // Side effect is creation of the Metadata version file
     try (final var storage = storageFactory.create(segment, commonConfiguration, metricsSystem)) {
       assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersionedStorageFormat())
-          .isEqualTo(PrivacyVersionedStorageFormat.FOREST_WITH_VARIABLES);
+          .isEqualTo(PrivacyVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION);
     }
   }
 
@@ -116,8 +116,8 @@ public void shouldUpdateCorrectMetadataFileForLatestVersion(
     try (final var storage = storageFactory.create(segment, commonConfiguration, metricsSystem)) {
       final BaseVersionedStorageFormat expectedBaseVersion =
           dataStorageFormat == BONSAI
-              ? BaseVersionedStorageFormat.BONSAI_WITH_VARIABLES
-              : BaseVersionedStorageFormat.FOREST_WITH_VARIABLES;
+              ? BaseVersionedStorageFormat.BONSAI_WITH_RECEIPT_COMPACTION
+              : BaseVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION;
       assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersionedStorageFormat())
           .isEqualTo(expectedBaseVersion);
     }
@@ -130,8 +130,8 @@ public void shouldUpdateCorrectMetadataFileForLatestVersion(
         privacyStorageFactory.create(segment, commonConfiguration, metricsSystem)) {
       final PrivacyVersionedStorageFormat expectedPrivacyVersion =
           dataStorageFormat == BONSAI
-              ? PrivacyVersionedStorageFormat.BONSAI_WITH_VARIABLES
-              : PrivacyVersionedStorageFormat.FOREST_WITH_VARIABLES;
+              ? PrivacyVersionedStorageFormat.BONSAI_WITH_RECEIPT_COMPACTION
+              : PrivacyVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION;
       assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersionedStorageFormat())
           .isEqualTo(expectedPrivacyVersion);
     }
@@ -145,7 +145,6 @@ public void shouldUpdateCorrectMetadataFileForLatestVersionWithReceiptCompaction
     final Path tempDataDir = temporaryFolder.resolve("data");
     final Path tempDatabaseDir = temporaryFolder.resolve("db");
     mockCommonConfiguration(tempDataDir, tempDatabaseDir, dataStorageFormat);
-    when(dataStorageConfiguration.getReceiptCompactionEnabled()).thenReturn(true);
 
     final RocksDBKeyValueStorageFactory storageFactory =
         new RocksDBKeyValueStorageFactory(

diff --git a/...g/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java b/...g/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java
@@ -76,8 +76,8 @@ public void shouldCreateCorrectMetadataFileForLatestVersionForNewDb(
       // Side effect is creation of the Metadata version file
       final BaseVersionedStorageFormat expectedVersion =
           dataStorageFormat == BONSAI
-              ? BaseVersionedStorageFormat.BONSAI_WITH_VARIABLES
-              : BaseVersionedStorageFormat.FOREST_WITH_VARIABLES;
+              ? BaseVersionedStorageFormat.BONSAI_WITH_RECEIPT_COMPACTION
+              : BaseVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION;
       assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersionedStorageFormat())
           .isEqualTo(expectedVersion);
     }
@@ -90,7 +90,6 @@ public void shouldCreateCorrectMetadataFileForLatestVersionForNewDbWithReceiptCo
     final Path tempDataDir = temporaryFolder.resolve("data");
     final Path tempDatabaseDir = temporaryFolder.resolve("db");
     mockCommonConfiguration(tempDataDir, tempDatabaseDir, dataStorageFormat);
-    when(dataStorageConfiguration.getReceiptCompactionEnabled()).thenReturn(true);
 
     final RocksDBKeyValueStorageFactory storageFactory =
         new RocksDBKeyValueStorageFactory(
@@ -129,7 +128,7 @@ public void shouldFailIfDbExistsAndNoMetadataFileFound() throws Exception {
   }
 
   @Test
-  public void shouldDetectCorrectMetadataV1() throws Exception {
+  public void shouldDetectCorrectMetadataV1AndUpgrade() throws Exception {
     final Path tempDataDir = temporaryFolder.resolve("data");
     final Path tempDatabaseDir = temporaryFolder.resolve("db");
     Files.createDirectories(tempDataDir);
@@ -143,7 +142,7 @@ public void shouldDetectCorrectMetadataV1() throws Exception {
 
     try (final var storage = storageFactory.create(segment, commonConfiguration, metricsSystem)) {
       assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersionedStorageFormat())
-          .isEqualTo(BaseVersionedStorageFormat.BONSAI_WITH_VARIABLES);
+          .isEqualTo(BaseVersionedStorageFormat.BONSAI_WITH_RECEIPT_COMPACTION);
       assertThat(storageFactory.isSegmentIsolationSupported()).isTrue();
     }
   }
@@ -240,7 +239,7 @@ public void shouldDetectCorrectMetadataV2AndSetSegmentationFieldDuringCreation()
             () -> rocksDbConfiguration, segments, RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS);
     try (final var storage = storageFactory.create(segment, commonConfiguration, metricsSystem)) {
       assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersionedStorageFormat())
-          .isEqualTo(BaseVersionedStorageFormat.FOREST_WITH_VARIABLES);
+          .isEqualTo(BaseVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION);
       assertThatCode(storageFactory::isSegmentIsolationSupported).doesNotThrowAnyException();
     }
   }
@@ -299,7 +298,7 @@ public void shouldCreateDBCorrectlyIfSymlink() throws Exception {
     // created correctly
     try (final var storage = storageFactory.create(segment, commonConfiguration, metricsSystem)) {
       assertThat(DatabaseMetadata.lookUpFrom(tempRealDataDir).getVersionedStorageFormat())
-          .isEqualTo(BaseVersionedStorageFormat.FOREST_WITH_VARIABLES);
+          .isEqualTo(BaseVersionedStorageFormat.FOREST_WITH_RECEIPT_COMPACTION);
     }
   }