From 2bced4ebf96b04f5c7bccc3d013a31f1618700e9 Mon Sep 17 00:00:00 2001
From: matkt <karim.t2am@gmail.com>
Date: Mon, 21 Aug 2023 16:12:16 +0200
Subject: [PATCH] fix snapsync issue with forest (#5776)
Fixing an issue during snapsync with forest related to the heal step
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
---
CHANGELOG.md | 1 +
...nsaiSnapshotWorldStateKeyValueStorage.java | 5 +
.../BonsaiWorldStateKeyValueStorage.java | 24 ++--
.../keyvalue/WorldStateKeyValueStorage.java | 5 +
.../worldstate/WorldStateStorage.java | 9 ++
.../heal/StorageTrieNodeHealingRequest.java | 19 +--
.../StorageTrieNodeHealingRequestTest.java | 113 ++++++++++++++++++
7 files changed, 153 insertions(+), 23 deletions(-)
create mode 100644 ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequestTest.java
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5080389666c..c46edf12c61 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiSnapshotWorldStateKeyValueStorage.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiSnapshotWorldStateKeyValueStorage.java
index 2e89e0f791a..44c79c46b8d 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiSnapshotWorldStateKeyValueStorage.java
+++ b/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> getTrieNodeUnsafe(final Bytes key) {
+ return isClosedGet() ? Optional.empty() : super.getTrieNodeUnsafe(key);
+ }
+
@Override
public Optional<Bytes> getAccountStorageTrieNode(
final Hash accountHash, final Bytes location, final Bytes32 nodeHash) {
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java
index 04c4aefe66d..4ceb63ef219 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java
@@ -166,32 +166,24 @@ 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> getTrieNodeUnsafe(final Bytes key) {
+ return composedWorldStateStorage
+ .get(TRIE_BRANCH_STORAGE, Bytes.concatenate(key).toArrayUnsafe())
+ .map(Bytes::wrap);
}
public Optional<byte[]> getTrieLog(final Hash blockHash) {
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/WorldStateKeyValueStorage.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/WorldStateKeyValueStorage.java
index 007a65f2138..74051b5e9b7 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/WorldStateKeyValueStorage.java
+++ b/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> getTrieNodeUnsafe(final Bytes key) {
+ return keyValueStorage.get(key.toArrayUnsafe()).map(Bytes::wrap);
+ }
+
@Override
public FlatDbMode getFlatDbMode() {
return FlatDbMode.NO_FLATTENED;
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/WorldStateStorage.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/WorldStateStorage.java
index 87527ed75b5..3bb6a0dfe78 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/WorldStateStorage.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/WorldStateStorage.java
@@ -33,6 +33,15 @@ public interface WorldStateStorage {
Optional<Bytes> getAccountStorageTrieNode(Hash accountHash, Bytes location, Bytes32 nodeHash);
+ /**
+ * 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.
+ *
+ * @param key of the trie node
+ * @return value of the trie node
+ */
+ Optional<Bytes> getTrieNodeUnsafe(Bytes key);
+
Optional<Bytes> getNodeData(Bytes location, Bytes32 hash);
FlatDbMode getFlatDbMode();
diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequest.java
index 092925b955d..779a5a72c8a 100644
--- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequest.java
+++ b/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.getTrieNodeUnsafe(getNodeHash());
+ } else {
+ storageTrieNode =
+ worldStateStorage.getTrieNodeUnsafe(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
diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequestTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequestTest.java
new file mode 100644
index 00000000000..14e4d551f72
--- /dev/null
+++ b/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();
+ }
+}