Skip to content

Commit

Permalink
fix snapsync issue with forest (#5776)
Browse files Browse the repository at this point in the history
Fixing an issue during snapsync with forest related to the heal step

Signed-off-by: Karim TAAM <[email protected]>
  • Loading branch information
matkt authored Aug 21, 2023
1 parent 92a3c5b commit 2bced4e
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}

0 comments on commit 2bced4e

Please sign in to comment.