fix snapsync issue with forest

CHANGELOG.md

@@ -15,6 +15,7 @@
 - Add type to PendingTransactionDetail, fix eth_subscribe [#5729](https://github.com/hyperledger/besu/pull/5729)
 - EvmTool "run" mode did not reflect contracts created within the transaction. [#5755](https://github.com/hyperledger/besu/pull/5755)
 - Update native libraries that have JPMS friendly module names [#5749](https://github.com/hyperledger/besu/pull/5749)
+- Fixing snapsync issue with forest during the heal step [#5776](https://github.com/hyperledger/besu/pull/5776)
 
 ### Download Links
 

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiSnapshotWorldStateKeyValueStorage.java

@@ -95,6 +95,11 @@ public Optional<Bytes> getAccountStateTrieNode(final Bytes location, final Bytes
     return isClosedGet() ? Optional.empty() : super.getAccountStateTrieNode(location, nodeHash);
   }
 
+  @Override
+  public Optional<Bytes> getUnSafeTrieNode(final Bytes key) {
+    return isClosedGet() ? Optional.empty() : super.getUnSafeTrieNode(key);
+  }
+
   @Override
   public Optional<Bytes> getAccountStorageTrieNode(
       final Hash accountHash, final Bytes location, final Bytes32 nodeHash) {

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java

@@ -166,32 +166,28 @@ public Optional<Bytes> getAccountStateTrieNode(final Bytes location, final Bytes
     }
   }
 
-  /**
-   * Retrieves the storage trie node associated with the specified account and location, if
-   * available.
-   *
-   * @param accountHash The hash of the account.
-   * @param location The location within the storage trie.
-   * @param maybeNodeHash The optional hash of the storage trie node to validate the retrieved data
-   *     against.
-   * @return The optional bytes of the storage trie node.
-   */
+  @Override
   public Optional<Bytes> getAccountStorageTrieNode(
-      final Hash accountHash, final Bytes location, final Optional<Bytes32> maybeNodeHash) {
-    if (maybeNodeHash.filter(hash -> hash.equals(MerkleTrie.EMPTY_TRIE_NODE_HASH)).isPresent()) {
+      final Hash accountHash, final Bytes location, final Bytes32 nodeHash) {
+    if (nodeHash.equals(MerkleTrie.EMPTY_TRIE_NODE_HASH)) {
       return Optional.of(MerkleTrie.EMPTY_TRIE_NODE);
     } else {
       return composedWorldStateStorage
           .get(TRIE_BRANCH_STORAGE, Bytes.concatenate(accountHash, location).toArrayUnsafe())
           .map(Bytes::wrap)
-          .filter(data -> maybeNodeHash.map(hash -> Hash.hash(data).equals(hash)).orElse(true));
+          .filter(b -> Hash.hash(b).equals(nodeHash));
     }
   }
 
   @Override
-  public Optional<Bytes> getAccountStorageTrieNode(
-      final Hash accountHash, final Bytes location, final Bytes32 nodeHash) {
-    return getAccountStorageTrieNode(accountHash, location, Optional.ofNullable(nodeHash));
+  public Optional<Bytes> getUnSafeTrieNode(final Bytes key) {
+    /*This method allows obtaining a TrieNode in an unsafe manner,
+    without verifying the consistency of the obtained node. Checks such as
+    node hash verification are not performed here.
+     */
+    return composedWorldStateStorage
+        .get(TRIE_BRANCH_STORAGE, Bytes.concatenate(key).toArrayUnsafe())
+        .map(Bytes::wrap);
   }
 
   public Optional<byte[]> getTrieLog(final Hash blockHash) {

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/WorldStateKeyValueStorage.java

@@ -78,6 +78,11 @@ private Optional<Bytes> getTrieNode(final Bytes32 nodeHash) {
     }
   }
 
+  @Override
+  public Optional<Bytes> getUnSafeTrieNode(final Bytes key) {
+    return keyValueStorage.get(key.toArrayUnsafe()).map(Bytes::wrap);
+  }
+
   @Override
   public FlatDbMode getFlatDbMode() {
     return FlatDbMode.NO_FLATTENED;

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/WorldStateStorage.java

@@ -33,6 +33,8 @@ public interface WorldStateStorage {
 
   Optional<Bytes> getAccountStorageTrieNode(Hash accountHash, Bytes location, Bytes32 nodeHash);
 
+  Optional<Bytes> getUnSafeTrieNode(Bytes key);
+
   Optional<Bytes> getNodeData(Bytes location, Bytes32 hash);
 
   FlatDbMode getFlatDbMode();

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequest.java

@@ -22,6 +22,7 @@
 import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.SnapDataRequest;
 import org.hyperledger.besu.ethereum.trie.CompactEncoding;
 import org.hyperledger.besu.ethereum.trie.MerkleTrie;
+import org.hyperledger.besu.ethereum.worldstate.DataStorageFormat;
 import org.hyperledger.besu.ethereum.worldstate.FlatDbMode;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage.Updater;
@@ -59,13 +60,17 @@ protected int doPersist(
   @Override
   public Optional<Bytes> getExistingData(
       final SnapWorldDownloadState downloadState, final WorldStateStorage worldStateStorage) {
-    Optional<Bytes> accountStorageTrieNode =
-        worldStateStorage.getAccountStorageTrieNode(
-            getAccountHash(),
-            getLocation(),
-            null); // push null to not check the hash in the getAccountStorageTrieNode method
-    if (accountStorageTrieNode.isPresent()) {
-      return accountStorageTrieNode
+
+    final Optional<Bytes> storageTrieNode;
+    if (worldStateStorage.getDataStorageFormat().equals(DataStorageFormat.FOREST)) {
+      storageTrieNode = worldStateStorage.getUnSafeTrieNode(getNodeHash());
+    } else {
+      storageTrieNode =
+          worldStateStorage.getUnSafeTrieNode(Bytes.concatenate(getAccountHash(), getLocation()));
+    }
+
+    if (storageTrieNode.isPresent()) {
+      return storageTrieNode
           .filter(node -> Hash.hash(node).equals(getNodeHash()))
           .or(
               () -> { // if we have a storage in database but not the good one we will need to fix

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequestTest.java

@@ -0,0 +1,113 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal;
+
+import org.hyperledger.besu.datatypes.Address;
+import org.hyperledger.besu.datatypes.Hash;
+import org.hyperledger.besu.ethereum.bonsai.storage.BonsaiWorldStateKeyValueStorage;
+import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider;
+import org.hyperledger.besu.ethereum.core.TrieGenerator;
+import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapWorldDownloadState;
+import org.hyperledger.besu.ethereum.rlp.RLP;
+import org.hyperledger.besu.ethereum.storage.StorageProvider;
+import org.hyperledger.besu.ethereum.storage.keyvalue.WorldStateKeyValueStorage;
+import org.hyperledger.besu.ethereum.trie.MerkleTrie;
+import org.hyperledger.besu.ethereum.worldstate.DataStorageFormat;
+import org.hyperledger.besu.ethereum.worldstate.StateTrieAccountValue;
+import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage;
+import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem;
+import org.hyperledger.besu.services.kvstore.InMemoryKeyValueStorage;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.tuweni.bytes.Bytes;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.ArgumentsProvider;
+import org.junit.jupiter.params.provider.ArgumentsSource;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class StorageTrieNodeHealingRequestTest {
+
+  @Mock private SnapWorldDownloadState downloadState;
+  final List<Address> accounts =
+      List.of(
+          Address.fromHexString("0xdeadbeef"),
+          Address.fromHexString("0xdeadbeee"),
+          Address.fromHexString("0xdeadbeea"),
+          Address.fromHexString("0xdeadbeeb"));
+
+  private WorldStateStorage worldStateStorage;
+
+  private Hash account0Hash;
+  private Hash account0StorageRoot;
+
+  static class StorageFormatArguments implements ArgumentsProvider {
+    @Override
+    public Stream<? extends Arguments> provideArguments(final ExtensionContext context) {
+      return Stream.of(
+          Arguments.of(DataStorageFormat.BONSAI), Arguments.of(DataStorageFormat.FOREST));
+    }
+  }
+
+  public void setup(final DataStorageFormat storageFormat) {
+    if (storageFormat.equals(DataStorageFormat.FOREST)) {
+      worldStateStorage = new WorldStateKeyValueStorage(new InMemoryKeyValueStorage());
+    } else {
+      final StorageProvider storageProvider = new InMemoryKeyValueStorageProvider();
+      worldStateStorage =
+          new BonsaiWorldStateKeyValueStorage(storageProvider, new NoOpMetricsSystem());
+    }
+    final MerkleTrie<Bytes, Bytes> trie =
+        TrieGenerator.generateTrie(
+            worldStateStorage,
+            accounts.stream().map(Address::addressHash).collect(Collectors.toList()));
+    account0Hash = accounts.get(0).addressHash();
+    account0StorageRoot =
+        trie.get(account0Hash)
+            .map(RLP::input)
+            .map(StateTrieAccountValue::readFrom)
+            .map(StateTrieAccountValue::getStorageRoot)
+            .orElseThrow();
+  }
+
+  @ParameterizedTest
+  @ArgumentsSource(StorageFormatArguments.class)
+  void shouldDetectExistingData(final DataStorageFormat storageFormat) {
+    setup(storageFormat);
+    final StorageTrieNodeHealingRequest request =
+        new StorageTrieNodeHealingRequest(
+            account0StorageRoot, account0Hash, Hash.EMPTY, Bytes.EMPTY);
+
+    Assertions.assertThat(request.getExistingData(downloadState, worldStateStorage)).isPresent();
+  }
+
+  @ParameterizedTest
+  @ArgumentsSource(StorageFormatArguments.class)
+  void shouldDetectMissingData(final DataStorageFormat storageFormat) {
+    setup(storageFormat);
+    final StorageTrieNodeHealingRequest request =
+        new StorageTrieNodeHealingRequest(Hash.EMPTY, account0Hash, Hash.EMPTY, Bytes.EMPTY);
+
+    Assertions.assertThat(request.getExistingData(downloadState, worldStateStorage)).isEmpty();
+  }
+}