fix for review

Signed-off-by: Karim Taam <[email protected]>
hyperledger · Jan 15, 2025 · 103ad88 · 103ad88
1 parent 478b3a8
commit 103ad88
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 65 deletions.
diff --git a/...in/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/BonsaiWorldStateProvider.java b/...in/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/BonsaiWorldStateProvider.java
@@ -25,7 +25,6 @@
 import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.worldview.BonsaiWorldState;
 import org.hyperledger.besu.ethereum.trie.diffbased.common.provider.DiffBasedWorldStateProvider;
 import org.hyperledger.besu.ethereum.trie.diffbased.common.trielog.TrieLogManager;
-import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.WorldStateSharedConfig;
 import org.hyperledger.besu.ethereum.trie.patricia.StoredMerklePatriciaTrie;
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 import org.hyperledger.besu.plugin.ServiceManager;
@@ -60,7 +59,7 @@ public BonsaiWorldStateProvider(
     this.worldStateHealerSupplier = worldStateHealerSupplier;
     provideCachedWorldStorageManager(
         new BonsaiCachedWorldStorageManager(
-            this, worldStateKeyValueStorage, this::cloneBonsaiWorldStateConfig));
+            this, worldStateKeyValueStorage, worldStateSharedConfig));
     loadHeadWorldState(
         new BonsaiWorldState(
             this, worldStateKeyValueStorage, evmConfiguration, worldStateSharedConfig));
@@ -154,10 +153,6 @@ public void prepareStateHealing(final Address address, final Bytes location) {
     getBonsaiWorldStateKeyValueStorage().downgradeToPartialFlatDbMode();
   }
 
-  private WorldStateSharedConfig cloneBonsaiWorldStateConfig() {
-    return WorldStateSharedConfig.newBuilder(worldStateSharedConfig).build();
-  }
-
   @Override
   public void heal(final Optional<Address> maybeAccountToRepair, final Bytes location) {
     worldStateHealerSupplier.get().heal(maybeAccountToRepair, location);

diff --git a/...yperledger/besu/ethereum/trie/diffbased/bonsai/cache/BonsaiCachedWorldStorageManager.java b/...yperledger/besu/ethereum/trie/diffbased/bonsai/cache/BonsaiCachedWorldStorageManager.java
@@ -26,15 +26,13 @@
 import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.WorldStateSharedConfig;
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 
-import java.util.function.Supplier;
-
 public class BonsaiCachedWorldStorageManager extends DiffBasedCachedWorldStorageManager {
 
   public BonsaiCachedWorldStorageManager(
       final BonsaiWorldStateProvider archive,
       final DiffBasedWorldStateKeyValueStorage worldStateKeyValueStorage,
-      final Supplier<WorldStateSharedConfig> worldStateSharedConfigSupplier) {
-    super(archive, worldStateKeyValueStorage, worldStateSharedConfigSupplier);
+      final WorldStateSharedConfig worldStateSharedConfig) {
+    super(archive, worldStateKeyValueStorage, worldStateSharedConfig);
   }
 
   @Override
@@ -46,7 +44,7 @@ public DiffBasedWorldState createWorldState(
         (BonsaiWorldStateProvider) archive,
         (BonsaiWorldStateKeyValueStorage) worldStateKeyValueStorage,
         evmConfiguration,
-        worldStateSharedConfigSupplier.get());
+        WorldStateSharedConfig.newBuilder(worldStateSharedConfig).build());
   }
 
   @Override

diff --git a/.../java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java b/.../java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java
@@ -36,7 +36,6 @@
 import org.hyperledger.besu.ethereum.trie.diffbased.common.trielog.TrieLogManager;
 import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.DiffBasedWorldState;
 import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.WorldStateSharedConfig;
-import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.WorldStateStorageConfig;
 import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.accumulator.DiffBasedWorldStateUpdateAccumulator;
 import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.accumulator.preload.StorageConsumingMap;
 import org.hyperledger.besu.ethereum.trie.patricia.StoredMerklePatriciaTrie;
@@ -67,14 +66,14 @@ public BonsaiWorldState(
       final BonsaiWorldStateProvider archive,
       final BonsaiWorldStateKeyValueStorage worldStateKeyValueStorage,
       final EvmConfiguration evmConfiguration,
-      final WorldStateSharedConfig worldStateSpec) {
+      final WorldStateSharedConfig worldStateConfig) {
     this(
         worldStateKeyValueStorage,
         archive.getCachedMerkleTrieLoader(),
         archive.getCachedWorldStorageManager(),
         archive.getTrieLogManager(),
         evmConfiguration,
-        worldStateSpec);
+        worldStateConfig);
   }
 
   public BonsaiWorldState(
@@ -86,7 +85,7 @@ public BonsaiWorldState(
         worldState.cachedWorldStorageManager,
         worldState.trieLogManager,
         worldState.accumulator.getEvmConfiguration(),
-        WorldStateSharedConfig.newBuilder(worldState.worldStateSpec).build());
+        WorldStateSharedConfig.newBuilder(worldState.worldStateConfig).build());
   }
 
   public BonsaiWorldState(
@@ -95,8 +94,8 @@ public BonsaiWorldState(
       final DiffBasedCachedWorldStorageManager cachedWorldStorageManager,
       final TrieLogManager trieLogManager,
       final EvmConfiguration evmConfiguration,
-      final WorldStateSharedConfig worldStateSpec) {
-    super(worldStateKeyValueStorage, cachedWorldStorageManager, trieLogManager, worldStateSpec);
+      final WorldStateSharedConfig worldStateConfig) {
+    super(worldStateKeyValueStorage, cachedWorldStorageManager, trieLogManager, worldStateConfig);
     this.bonsaiCachedMerkleTrieLoader = bonsaiCachedMerkleTrieLoader;
     this.worldStateKeyValueStorage = worldStateKeyValueStorage;
     this.setAccumulator(
@@ -302,7 +301,7 @@ private void updateAccountStorageState(
                         writeStorageTrieNode(
                             bonsaiUpdater, updatedAddressHash, location, key, value)));
         // only use storage root of the trie when trie is enabled
-        if (!worldStateSpec.isTrieDisabled()) {
+        if (!worldStateConfig.isTrieDisabled()) {
           final Hash newStorageRoot = Hash.wrap(storageTrie.getRootHash());
           accountUpdated.setStorageRoot(newStorageRoot);
         }
@@ -446,14 +445,13 @@ public Map<Bytes32, Bytes> getAllAccountStorage(final Address address, final Has
 
   @Override
   public MutableWorldState freeze() {
-    this.worldStateStorageSpec =
-        WorldStateStorageConfig.newBuilder(worldStateStorageSpec).setFrozen(true).build();
+    this.isStorageFrozen = true;
     this.worldStateKeyValueStorage = new BonsaiWorldStateLayerStorage(getWorldStateStorage());
     return this;
   }
 
   private MerkleTrie<Bytes, Bytes> createTrie(final NodeLoader nodeLoader, final Bytes32 rootHash) {
-    if (worldStateSpec.isTrieDisabled()) {
+    if (worldStateConfig.isTrieDisabled()) {
       return new NoOpMerkleTrie<>();
     } else {
       return new StoredMerklePatriciaTrie<>(

diff --git a/...rledger/besu/ethereum/trie/diffbased/common/cache/DiffBasedCachedWorldStorageManager.java b/...rledger/besu/ethereum/trie/diffbased/common/cache/DiffBasedCachedWorldStorageManager.java
@@ -36,7 +36,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
-import java.util.function.Supplier;
 
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
@@ -50,7 +49,7 @@ public abstract class DiffBasedCachedWorldStorageManager implements StorageSubsc
       LoggerFactory.getLogger(DiffBasedCachedWorldStorageManager.class);
   private final DiffBasedWorldStateProvider archive;
   private final EvmConfiguration evmConfiguration;
-  protected final Supplier<WorldStateSharedConfig> worldStateSharedConfigSupplier;
+  protected final WorldStateSharedConfig worldStateSharedConfig;
   private final Cache<Hash, BlockHeader> stateRootToBlockHeaderCache =
       Caffeine.newBuilder()
           .maximumSize(RETAINED_LAYERS)
@@ -65,25 +64,25 @@ private DiffBasedCachedWorldStorageManager(
       final DiffBasedWorldStateKeyValueStorage worldStateKeyValueStorage,
       final Map<Bytes32, DiffBasedCachedWorldView> cachedWorldStatesByHash,
       final EvmConfiguration evmConfiguration,
-      final Supplier<WorldStateSharedConfig> worldStateSharedConfigSupplier) {
+      final WorldStateSharedConfig worldStateSharedConfig) {
     worldStateKeyValueStorage.subscribe(this);
     this.rootWorldStateStorage = worldStateKeyValueStorage;
     this.cachedWorldStatesByHash = cachedWorldStatesByHash;
     this.archive = archive;
     this.evmConfiguration = evmConfiguration;
-    this.worldStateSharedConfigSupplier = worldStateSharedConfigSupplier;
+    this.worldStateSharedConfig = worldStateSharedConfig;
   }
 
   public DiffBasedCachedWorldStorageManager(
       final DiffBasedWorldStateProvider archive,
       final DiffBasedWorldStateKeyValueStorage worldStateKeyValueStorage,
-      final Supplier<WorldStateSharedConfig> worldStateSharedConfigSupplier) {
+      final WorldStateSharedConfig worldStateSharedConfig) {
     this(
         archive,
         worldStateKeyValueStorage,
         new ConcurrentHashMap<>(),
         EvmConfiguration.DEFAULT,
-        worldStateSharedConfigSupplier);
+        worldStateSharedConfig);
   }
 
   public synchronized void addCachedLayer(

diff --git a/...va/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/DiffBasedWorldState.java b/...va/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/DiffBasedWorldState.java
@@ -62,15 +62,23 @@ public abstract class DiffBasedWorldState
   protected Hash worldStateBlockHash;
 
   // configuration parameters for the world state.
-  protected WorldStateSharedConfig worldStateSpec;
-  // configuration parameters for the storage of the world state.
-  protected WorldStateStorageConfig worldStateStorageSpec;
+  protected WorldStateSharedConfig worldStateConfig;
+
+  /*
+   * Indicates whether the world state is in "frozen" mode.
+   *
+   * When `isStorageFrozen` is true:
+   * - Changes to accounts, code, or slots will not affect the underlying storage.
+   * - The state root can still be recalculated, and a trie log can be generated.
+   * - All modifications are temporary and will be lost once the world state is discarded.
+   */
+  protected boolean isStorageFrozen;
 
   protected DiffBasedWorldState(
       final DiffBasedWorldStateKeyValueStorage worldStateKeyValueStorage,
       final DiffBasedCachedWorldStorageManager cachedWorldStorageManager,
       final TrieLogManager trieLogManager,
-      final WorldStateSharedConfig worldStateSpec) {
+      final WorldStateSharedConfig worldStateConfig) {
     this.worldStateKeyValueStorage = worldStateKeyValueStorage;
     this.worldStateRootHash =
         Hash.wrap(
@@ -81,8 +89,8 @@ protected DiffBasedWorldState(
             Bytes32.wrap(worldStateKeyValueStorage.getWorldStateBlockHash().orElse(Hash.ZERO)));
     this.cachedWorldStorageManager = cachedWorldStorageManager;
     this.trieLogManager = trieLogManager;
-    this.worldStateSpec = worldStateSpec;
-    this.worldStateStorageSpec = WorldStateStorageConfig.newBuilder().build();
+    this.worldStateConfig = worldStateConfig;
+    this.isStorageFrozen = false;
   }
 
   /**
@@ -114,11 +122,10 @@ public Hash getWorldStateRootHash() {
   }
 
   /**
-   * Checks if the current world state is modifying the head of the node.
-   *
-   * <p>This method determines whether the current world state is capable of making changes to the
-   * head of the node. The head of the node will never be of type snapshot or layered. These
-   * instances are copies of the head or of the head at a previous block or other points in time.
+   * Determines whether the current world state is directly modifying the "head" state of the
+   * blockchain. A world state modifying the head directly updates the latest state of the node,
+   * while a world state derived from a snapshot or historical view (e.g., layered or snapshot world
+   * state) does not directly modify the head
    *
    * @return {@code true} if the current world state is modifying the head, {@code false} otherwise.
    */
@@ -127,17 +134,6 @@ public boolean isModifyingHeadWorldState() {
     return isModifyingHeadWorldState(worldStateKeyValueStorage);
   }
 
-  /**
-   * Determines if the provided world state key-value storage is modifying the head of the node.
-   *
-   * <p>This method checks if the given world state key-value storage is an instance that can modify
-   * the head of the node. The head of the node will never be of type {@link
-   * DiffBasedSnapshotWorldStateKeyValueStorage}. These instances are copies of the head or of the
-   * head at a previous block or other points in time.
-   *
-   * @param worldStateKeyValueStorage the world state key-value storage to check.
-   * @return {@code true} if the provided storage is modifying the head, {@code false} otherwise.
-   */
   private boolean isModifyingHeadWorldState(
       final WorldStateKeyValueStorage worldStateKeyValueStorage) {
     return !(worldStateKeyValueStorage instanceof DiffBasedSnapshotWorldStateKeyValueStorage);
@@ -166,9 +162,7 @@ protected Hash unsafeRootHashUpdate(
       final BlockHeader blockHeader,
       final DiffBasedWorldStateKeyValueStorage.Updater stateUpdater) {
     // calling calculateRootHash in order to update the state
-    calculateRootHash(
-        worldStateStorageSpec.isFrozen() ? Optional.empty() : Optional.of(stateUpdater),
-        accumulator);
+    calculateRootHash(isStorageFrozen ? Optional.empty() : Optional.of(stateUpdater), accumulator);
     return blockHeader.getStateRoot();
   }
 
@@ -191,11 +185,10 @@ public void persist(final BlockHeader blockHeader) {
     try {
       final Hash calculatedRootHash;
 
-      if (blockHeader == null || !worldStateSpec.isTrieDisabled()) {
+      if (blockHeader == null || !worldStateConfig.isTrieDisabled()) {
         calculatedRootHash =
             calculateRootHash(
-                worldStateStorageSpec.isFrozen() ? Optional.empty() : Optional.of(stateUpdater),
-                accumulator);
+                isStorageFrozen ? Optional.empty() : Optional.of(stateUpdater), accumulator);
       } else {
         // if the trie is disabled, we cannot calculate the state root, so we directly use the root
         // of the block. It's important to understand that in all networks,
@@ -213,7 +206,7 @@ public void persist(final BlockHeader blockHeader) {
             () -> {
               trieLogManager.saveTrieLog(localCopy, calculatedRootHash, blockHeader, this);
               // not save a frozen state in the cache
-              if (!worldStateStorageSpec.isFrozen()) {
+              if (!isStorageFrozen) {
                 cachedWorldStorageManager.addCachedLayer(blockHeader, calculatedRootHash, this);
               }
             };
@@ -245,7 +238,7 @@ public void persist(final BlockHeader blockHeader) {
   }
 
   protected void verifyWorldStateRoot(final Hash calculatedStateRoot, final BlockHeader header) {
-    if (!worldStateSpec.isTrieDisabled() && !calculatedStateRoot.equals(header.getStateRoot())) {
+    if (!worldStateConfig.isTrieDisabled() && !calculatedStateRoot.equals(header.getStateRoot())) {
       throw new RuntimeException(
           "World State Root does not match expected value, header "
               + header.getStateRoot().toHexString()
@@ -261,7 +254,7 @@ public WorldUpdater updater() {
 
   @Override
   public Hash rootHash() {
-    if (worldStateStorageSpec.isFrozen() && accumulator.isAccumulatorStateChanged()) {
+    if (isStorageFrozen && accumulator.isAccumulatorStateChanged()) {
       worldStateRootHash = calculateRootHash(Optional.empty(), accumulator.copy());
       accumulator.resetAccumulatorStateChanged();
     }
@@ -336,7 +329,7 @@ public void close() {
     try {
       if (!isModifyingHeadWorldState()) {
         this.worldStateKeyValueStorage.close();
-        if (worldStateStorageSpec.isFrozen()) {
+        if (isStorageFrozen) {
           closeFrozenStorage();
         }
       }
@@ -360,6 +353,21 @@ private void closeFrozenStorage() {
   @Override
   public abstract Hash frontierRootHash();
 
+  /*
+   * Configures the current world state to operate in "frozen" mode.
+   *
+   * In this mode:
+   * - Changes (to accounts, code, or slots) are isolated and not applied to the underlying storage.
+   * - The state root can be recalculated, and a trie log can be generated, but updates will not
+   *   affect the world state storage.
+   * - All modifications are temporary and will be lost once the world state is discarded.
+   *
+   * Use Cases:
+   * - Calculating the state root after updates without altering the storage.
+   * - Generating a trie log.
+   *
+   * @return The current world state in "frozen" mode.
+   */
   @Override
   public abstract MutableWorldState freeze();
 

diff --git a/...org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/WorldStateSharedConfig.java b/...org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/WorldStateSharedConfig.java
@@ -14,7 +14,7 @@
  */
 package org.hyperledger.besu.ethereum.trie.diffbased.common.worldview;
 
-/** WorldStateSpec encapsulates the shared configuration parameters for the world state. */
+/** WorldStateSharedConfig encapsulates the shared configuration parameters for the world state. */
 public class WorldStateSharedConfig {
 
   /**
@@ -49,10 +49,11 @@ public void setStateful(final boolean stateful) {
   }
 
   /**
-   * Merges this WorldStateSpec with another WorldStateSpec and returns a new instance.
+   * Merges this WorldStateSharedConfig with another WorldStateSharedConfig and returns a new
+   * instance.
    *
-   * @param other the other WorldStateSpec to merge with
-   * @return a new WorldStateSpec instance with merged values
+   * @param other the other WorldStateSharedConfig to merge with
+   * @return a new WorldStateSharedConfig instance with merged values
    */
   public WorldStateSharedConfig apply(final WorldStateSharedConfig other) {
     return new Builder(this).trieDisabled(other.isTrieDisabled).stateful(other.isStateful).build();
@@ -62,8 +63,8 @@ public static Builder newBuilder() {
     return new Builder();
   }
 
-  public static Builder newBuilder(final WorldStateSharedConfig worldStateSpec) {
-    return new Builder(worldStateSpec);
+  public static Builder newBuilder(final WorldStateSharedConfig worldStateSharedConfig) {
+    return new Builder(worldStateSharedConfig);
   }
 
   public static WorldStateSharedConfig createStatefulConfigWithTrie() {