From 103ad884b2701577aff52d0b96e380e6de6f0d53 Mon Sep 17 00:00:00 2001 From: Karim Taam Date: Wed, 15 Jan 2025 17:39:02 +0100 Subject: [PATCH] fix for review Signed-off-by: Karim Taam --- .../bonsai/BonsaiWorldStateProvider.java | 7 +- .../BonsaiCachedWorldStorageManager.java | 8 +-- .../bonsai/worldview/BonsaiWorldState.java | 18 +++-- .../DiffBasedCachedWorldStorageManager.java | 11 ++- .../common/worldview/DiffBasedWorldState.java | 72 ++++++++++--------- .../worldview/WorldStateSharedConfig.java | 13 ++-- 6 files changed, 64 insertions(+), 65 deletions(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/BonsaiWorldStateProvider.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/BonsaiWorldStateProvider.java index d26cd585e6f..879a9bb5c75 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/BonsaiWorldStateProvider.java +++ b/ethereum/core/src/main/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
maybeAccountToRepair, final Bytes location) { worldStateHealerSupplier.get().heal(maybeAccountToRepair, location); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/cache/BonsaiCachedWorldStorageManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/cache/BonsaiCachedWorldStorageManager.java index 9f585ea3c0c..fdccbe8ef73 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/cache/BonsaiCachedWorldStorageManager.java +++ b/ethereum/core/src/main/java/org/hyperledger/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 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/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java index 9558f9ef508..7a078a9cf33 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java +++ b/ethereum/core/src/main/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 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 createTrie(final NodeLoader nodeLoader, final Bytes32 rootHash) { - if (worldStateSpec.isTrieDisabled()) { + if (worldStateConfig.isTrieDisabled()) { return new NoOpMerkleTrie<>(); } else { return new StoredMerklePatriciaTrie<>( diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/cache/DiffBasedCachedWorldStorageManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/cache/DiffBasedCachedWorldStorageManager.java index 020df26a706..b61e7f104e8 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/cache/DiffBasedCachedWorldStorageManager.java +++ b/ethereum/core/src/main/java/org/hyperledger/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 worldStateSharedConfigSupplier; + protected final WorldStateSharedConfig worldStateSharedConfig; private final Cache stateRootToBlockHeaderCache = Caffeine.newBuilder() .maximumSize(RETAINED_LAYERS) @@ -65,25 +64,25 @@ private DiffBasedCachedWorldStorageManager( final DiffBasedWorldStateKeyValueStorage worldStateKeyValueStorage, final Map cachedWorldStatesByHash, final EvmConfiguration evmConfiguration, - final Supplier 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 worldStateSharedConfigSupplier) { + final WorldStateSharedConfig worldStateSharedConfig) { this( archive, worldStateKeyValueStorage, new ConcurrentHashMap<>(), EvmConfiguration.DEFAULT, - worldStateSharedConfigSupplier); + worldStateSharedConfig); } public synchronized void addCachedLayer( diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/DiffBasedWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/DiffBasedWorldState.java index e03f5281cba..3045ba849d7 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/DiffBasedWorldState.java +++ b/ethereum/core/src/main/java/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. - * - *

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. - * - *

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/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/WorldStateSharedConfig.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/WorldStateSharedConfig.java index c7ed45f0501..ea6f4542e83 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/WorldStateSharedConfig.java +++ b/ethereum/core/src/main/java/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() {