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 getAccountStateTrieNode(final Bytes location, final Bytes return isClosedGet() ? Optional.empty() : super.getAccountStateTrieNode(location, nodeHash); } + @Override + public Optional getTrieNodeUnsafe(final Bytes key) { + return isClosedGet() ? Optional.empty() : super.getTrieNodeUnsafe(key); + } + @Override public Optional 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 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 getAccountStorageTrieNode( - final Hash accountHash, final Bytes location, final Optional 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 getAccountStorageTrieNode( - final Hash accountHash, final Bytes location, final Bytes32 nodeHash) { - return getAccountStorageTrieNode(accountHash, location, Optional.ofNullable(nodeHash)); + public Optional getTrieNodeUnsafe(final Bytes key) { + return composedWorldStateStorage + .get(TRIE_BRANCH_STORAGE, Bytes.concatenate(key).toArrayUnsafe()) + .map(Bytes::wrap); } public Optional 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 getTrieNode(final Bytes32 nodeHash) { } } + @Override + public Optional 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 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 getTrieNodeUnsafe(Bytes key); + Optional 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 getExistingData( final SnapWorldDownloadState downloadState, final WorldStateStorage worldStateStorage) { - Optional accountStorageTrieNode = - worldStateStorage.getAccountStorageTrieNode( - getAccountHash(), - getLocation(), - null); // push null to not check the hash in the getAccountStorageTrieNode method - if (accountStorageTrieNode.isPresent()) { - return accountStorageTrieNode + + final Optional 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
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 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 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(); + } +}