From 15cbd8423b6d85b781abb1e7a9636cb8e65569bf Mon Sep 17 00:00:00 2001 From: Karim TAAM Date: Tue, 26 Mar 2024 14:21:52 +0100 Subject: [PATCH] Fix issues during snap sync (#6802) The purpose of this commit is to fix some bugs detected in the snap sync. This should allow for greater stability with the snap sync and the healing of the flat DB. --------- Signed-off-by: Karim Taam --- .../eth/sync/snapsync/RequestDataStep.java | 1 + .../sync/snapsync/SnapSyncMetricsManager.java | 1 + .../sync/snapsync/SnapWorldDownloadState.java | 1 + .../snapsync/SnapWorldStateDownloader.java | 1 + .../ethereum/eth/sync/snapsync/StackTrie.java | 13 +- .../request/AccountRangeDataRequest.java | 6 +- .../request/StorageRangeDataRequest.java | 22 +-- ...ccountFlatDatabaseHealingRangeRequest.java | 6 +- ...torageFlatDatabaseHealingRangeRequest.java | 4 +- .../snap/GetAccountRangeMessageTest.java | 2 +- .../snap/GetStorageRangeMessageTest.java | 2 +- .../snapsync/AccountHealingTrackingTest.java | 2 +- .../eth/sync/snapsync/RangeManagerTest.java | 1 + .../snapsync/SnapWorldDownloadStateTest.java | 1 + .../eth/sync/snapsync/StackTrieTest.java | 126 ++++++++++++++++ .../eth/sync/snapsync/TaskGenerator.java | 1 + ...ntFlatDatabaseHealingRangeRequestTest.java | 2 +- ...geFlatDatabaseHealingRangeRequestTest.java | 2 +- .../trie/InnerNodeDiscoveryManager.java | 41 ++---- .../besu/ethereum/trie}/RangeManager.java | 44 +++++- .../besu/ethereum/trie/SnapCommitVisitor.java | 138 ++++++++++++++++++ .../besu/ethereum/trie/SnapPutVisitor.java | 52 ------- .../ethereum/trie/SnapCommitVisitorTest.java | 127 ++++++++++++++++ .../ethereum/trie/SnapPutVisitorTest.java | 129 ---------------- 24 files changed, 483 insertions(+), 242 deletions(-) rename ethereum/{eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync => trie/src/main/java/org/hyperledger/besu/ethereum/trie}/RangeManager.java (74%) create mode 100644 ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/SnapCommitVisitor.java delete mode 100644 ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/SnapPutVisitor.java create mode 100644 ethereum/trie/src/test/java/org/hyperledger/besu/ethereum/trie/SnapCommitVisitorTest.java delete mode 100644 ethereum/trie/src/test/java/org/hyperledger/besu/ethereum/trie/SnapPutVisitorTest.java diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java index fdde0a5eea9..adbb8b736c1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java @@ -31,6 +31,7 @@ import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal.StorageFlatDatabaseHealingRangeRequest; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal.TrieNodeHealingRequest; import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.worldstate.FlatDbMode; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorageCoordinator; import org.hyperledger.besu.plugin.services.MetricsSystem; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapSyncMetricsManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapSyncMetricsManager.java index da7607de7fd..b8e907b2eb7 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapSyncMetricsManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapSyncMetricsManager.java @@ -18,6 +18,7 @@ import static org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncMetricsManager.Step.HEAL_TRIE; import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.metrics.BesuMetricCategory; import org.hyperledger.besu.plugin.services.MetricsSystem; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadState.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadState.java index cb6ed53ce69..000badc25d7 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadState.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadState.java @@ -29,6 +29,7 @@ import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal.AccountFlatDatabaseHealingRangeRequest; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal.StorageFlatDatabaseHealingRangeRequest; import org.hyperledger.besu.ethereum.eth.sync.worldstate.WorldDownloadState; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; import org.hyperledger.besu.ethereum.worldstate.FlatDbMode; import org.hyperledger.besu.ethereum.worldstate.WorldStateKeyValueStorage; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldStateDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldStateDownloader.java index b3dae73d934..61219882741 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldStateDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldStateDownloader.java @@ -27,6 +27,7 @@ import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.AccountRangeDataRequest; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.SnapDataRequest; import org.hyperledger.besu.ethereum.eth.sync.worldstate.WorldStateDownloader; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorageCoordinator; import org.hyperledger.besu.metrics.BesuMetricCategory; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/StackTrie.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/StackTrie.java index 01f17eb79b4..f9e8ea2eb7d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/StackTrie.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/StackTrie.java @@ -15,12 +15,12 @@ package org.hyperledger.besu.ethereum.eth.sync.snapsync; import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.trie.CommitVisitor; import org.hyperledger.besu.ethereum.trie.InnerNodeDiscoveryManager; import org.hyperledger.besu.ethereum.trie.MerkleTrie; import org.hyperledger.besu.ethereum.trie.Node; import org.hyperledger.besu.ethereum.trie.NodeUpdater; -import org.hyperledger.besu.ethereum.trie.SnapPutVisitor; +import org.hyperledger.besu.ethereum.trie.RangeManager; +import org.hyperledger.besu.ethereum.trie.SnapCommitVisitor; import org.hyperledger.besu.ethereum.trie.patricia.StoredMerklePatriciaTrie; import java.util.ArrayList; @@ -122,7 +122,7 @@ public void commit(final FlatDatabaseUpdater flatDatabaseUpdater, final NodeUpda Function.identity(), Function.identity(), startKeyHash, - keys.lastKey(), + proofs.isEmpty() ? RangeManager.MAX_RANGE : keys.lastKey(), true); final MerkleTrie trie = @@ -130,14 +130,17 @@ public void commit(final FlatDatabaseUpdater flatDatabaseUpdater, final NodeUpda snapStoredNodeFactory, proofs.isEmpty() ? MerkleTrie.EMPTY_TRIE_NODE_HASH : rootHash); for (Map.Entry entry : keys.entrySet()) { - trie.put(entry.getKey(), new SnapPutVisitor<>(snapStoredNodeFactory, entry.getValue())); + trie.put(entry.getKey(), entry.getValue()); } keys.forEach(flatDatabaseUpdater::update); trie.commit( nodeUpdater, - (new CommitVisitor<>(nodeUpdater) { + (new SnapCommitVisitor<>( + nodeUpdater, + startKeyHash, + proofs.isEmpty() ? RangeManager.MAX_RANGE : keys.lastKey()) { @Override public void maybeStoreNode(final Bytes location, final Node node) { if (!node.isHealNeeded()) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/AccountRangeDataRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/AccountRangeDataRequest.java index 7829ad75eb8..76897e18505 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/AccountRangeDataRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/AccountRangeDataRequest.java @@ -14,12 +14,12 @@ */ package org.hyperledger.besu.ethereum.eth.sync.snapsync.request; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.MAX_RANGE; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.MIN_RANGE; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.findNewBeginElementInRange; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RequestType.ACCOUNT_RANGE; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncMetricsManager.Step.DOWNLOAD; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.StackTrie.FlatDatabaseUpdater.noop; +import static org.hyperledger.besu.ethereum.trie.RangeManager.MAX_RANGE; +import static org.hyperledger.besu.ethereum.trie.RangeManager.MIN_RANGE; +import static org.hyperledger.besu.ethereum.trie.RangeManager.findNewBeginElementInRange; import static org.hyperledger.besu.ethereum.worldstate.WorldStateStorageCoordinator.applyForStrategy; import org.hyperledger.besu.datatypes.Hash; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java index 0cb70b54635..646161a42e2 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java @@ -14,16 +14,15 @@ */ package org.hyperledger.besu.ethereum.eth.sync.snapsync.request; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.MAX_RANGE; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.MIN_RANGE; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.findNewBeginElementInRange; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.getRangeCount; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RequestType.STORAGE_RANGE; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.StackTrie.FlatDatabaseUpdater.noop; +import static org.hyperledger.besu.ethereum.trie.RangeManager.MAX_RANGE; +import static org.hyperledger.besu.ethereum.trie.RangeManager.MIN_RANGE; +import static org.hyperledger.besu.ethereum.trie.RangeManager.findNewBeginElementInRange; +import static org.hyperledger.besu.ethereum.trie.RangeManager.getRangeCount; import static org.hyperledger.besu.ethereum.worldstate.WorldStateStorageCoordinator.applyForStrategy; import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncConfiguration; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncProcessState; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapWorldDownloadState; @@ -31,6 +30,7 @@ import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider; import org.hyperledger.besu.ethereum.trie.CompactEncoding; import org.hyperledger.besu.ethereum.trie.NodeUpdater; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; import org.hyperledger.besu.ethereum.worldstate.FlatDbMode; import org.hyperledger.besu.ethereum.worldstate.WorldStateKeyValueStorage; @@ -62,7 +62,7 @@ public class StorageRangeDataRequest extends SnapDataRequest { private final Bytes32 startKeyHash; private final Bytes32 endKeyHash; - private StackTrie stackTrie; + private final StackTrie stackTrie; private Optional isProofValid; protected StorageRangeDataRequest( @@ -77,7 +77,7 @@ protected StorageRangeDataRequest( this.startKeyHash = startKeyHash; this.endKeyHash = endKeyHash; this.isProofValid = Optional.empty(); - addStackTrie(Optional.empty()); + this.stackTrie = new StackTrie(Hash.wrap(getStorageRoot()), startKeyHash); LOG.trace( "create get storage range data request for account {} with root hash={} from {} to {}", accountHash, @@ -183,7 +183,6 @@ public Stream getChildRequests( final StorageRangeDataRequest storageRangeDataRequest = createStorageRangeDataRequest( getRootHash(), accountHash, storageRoot, key, value); - storageRangeDataRequest.addStackTrie(Optional.of(stackTrie)); childRequests.add(storageRangeDataRequest); }); if (startKeyHash.equals(MIN_RANGE) && endKeyHash.equals(MAX_RANGE)) { @@ -225,11 +224,4 @@ public void clear() { public void setProofValid(final boolean isProofValid) { this.isProofValid = Optional.of(isProofValid); } - - public void addStackTrie(final Optional maybeStackTrie) { - stackTrie = - maybeStackTrie - .filter(StackTrie::addSegment) - .orElse(new StackTrie(Hash.wrap(getStorageRoot()), 1, 3, startKeyHash)); - } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequest.java index 05bbeed3da5..76d6f903f12 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequest.java @@ -14,12 +14,11 @@ */ package org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.MAX_RANGE; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.MIN_RANGE; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncMetricsManager.Step.HEAL_FLAT; +import static org.hyperledger.besu.ethereum.trie.RangeManager.MAX_RANGE; +import static org.hyperledger.besu.ethereum.trie.RangeManager.MIN_RANGE; import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager; import org.hyperledger.besu.ethereum.eth.sync.snapsync.RequestType; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncConfiguration; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncProcessState; @@ -29,6 +28,7 @@ import org.hyperledger.besu.ethereum.rlp.RLP; import org.hyperledger.besu.ethereum.trie.CompactEncoding; import org.hyperledger.besu.ethereum.trie.MerkleTrie; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.RangeStorageEntriesCollector; import org.hyperledger.besu.ethereum.trie.TrieIterator; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequest.java index ac554439e91..26610572533 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequest.java @@ -14,17 +14,17 @@ */ package org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.getRangeCount; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RequestType.STORAGE_RANGE; +import static org.hyperledger.besu.ethereum.trie.RangeManager.getRangeCount; import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncConfiguration; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncProcessState; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapWorldDownloadState; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.SnapDataRequest; import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider; import org.hyperledger.besu.ethereum.trie.MerkleTrie; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.RangeStorageEntriesCollector; import org.hyperledger.besu.ethereum.trie.TrieIterator; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/snap/GetAccountRangeMessageTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/snap/GetAccountRangeMessageTest.java index c8ef62a6d51..a486608eff1 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/snap/GetAccountRangeMessageTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/snap/GetAccountRangeMessageTest.java @@ -15,10 +15,10 @@ package org.hyperledger.besu.ethereum.eth.messages.snap; import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.AbstractSnapMessageData; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.RawMessage; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.apache.tuweni.bytes.Bytes32; import org.assertj.core.api.Assertions; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/snap/GetStorageRangeMessageTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/snap/GetStorageRangeMessageTest.java index 4ff1b09b9a0..2492528a89b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/snap/GetStorageRangeMessageTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/snap/GetStorageRangeMessageTest.java @@ -15,10 +15,10 @@ package org.hyperledger.besu.ethereum.eth.messages.snap; import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.AbstractSnapMessageData; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.RawMessage; +import org.hyperledger.besu.ethereum.trie.RangeManager; import java.util.List; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java index 28001a2a377..296952cf029 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java @@ -14,7 +14,7 @@ */ package org.hyperledger.besu.ethereum.eth.sync.snapsync; -import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.MAX_RANGE; +import static org.hyperledger.besu.ethereum.trie.RangeManager.MAX_RANGE; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManagerTest.java index 40b967096a7..8ee3784d2cf 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManagerTest.java @@ -20,6 +20,7 @@ import org.hyperledger.besu.ethereum.core.TrieGenerator; import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider; import org.hyperledger.besu.ethereum.trie.MerkleTrie; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.RangeStorageEntriesCollector; import org.hyperledger.besu.ethereum.trie.TrieIterator; import org.hyperledger.besu.ethereum.trie.forest.storage.ForestWorldStateKeyValueStorage; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadStateTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadStateTest.java index 0dcc204426a..18ab5f02fa7 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadStateTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadStateTest.java @@ -38,6 +38,7 @@ import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.BytecodeRequest; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.SnapDataRequest; import org.hyperledger.besu.ethereum.eth.sync.worldstate.WorldStateDownloadProcess; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; import org.hyperledger.besu.ethereum.trie.forest.storage.ForestWorldStateKeyValueStorage; import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/StackTrieTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/StackTrieTest.java index b1b3800a1be..4fcb21d9252 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/StackTrieTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/StackTrieTest.java @@ -18,12 +18,15 @@ import org.hyperledger.besu.ethereum.core.TrieGenerator; import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider; import org.hyperledger.besu.ethereum.trie.MerkleTrie; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.RangeStorageEntriesCollector; import org.hyperledger.besu.ethereum.trie.TrieIterator; import org.hyperledger.besu.ethereum.trie.forest.storage.ForestWorldStateKeyValueStorage; +import org.hyperledger.besu.ethereum.trie.patricia.StoredMerklePatriciaTrie; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorageCoordinator; import org.hyperledger.besu.services.kvstore.InMemoryKeyValueStorage; +import java.util.ArrayList; import java.util.List; import java.util.TreeMap; @@ -143,4 +146,127 @@ public void shouldSaveTheRootWhenComplete() { worldStateKeyValueStorage.getAccountStateTrieNode(accountStateTrie.getRootHash())) .isPresent(); } + + @Test + public void shouldNotSaveNodeWithChildNotInTheRange() { + final ForestWorldStateKeyValueStorage worldStateStorage = + new ForestWorldStateKeyValueStorage(new InMemoryKeyValueStorage()); + final WorldStateStorageCoordinator worldStateStorageCoordinator = + new WorldStateStorageCoordinator(worldStateStorage); + + final MerkleTrie trie = + new StoredMerklePatriciaTrie<>( + (location, hash) -> worldStateStorage.getAccountStateTrieNode(hash).map(Bytes::wrap), + b -> b, + b -> b); + + trie.put(Bytes32.rightPad(Bytes.of(0x10)), Bytes.of(0x01)); + trie.put(Bytes32.rightPad(Bytes.of(0x11)), Bytes.of(0x01)); + trie.put(Bytes32.rightPad(Bytes.of(0x20)), Bytes.of(0x01)); + trie.put(Bytes32.rightPad(Bytes.of(0x21)), Bytes.of(0x01)); + trie.put(Bytes32.rightPad(Bytes.of(0x01)), Bytes.of(0x02)); + trie.put(Bytes32.rightPad(Bytes.of(0x02)), Bytes.of(0x03)); + trie.put(Bytes32.rightPad(Bytes.of(0x03)), Bytes.of(0x04)); + + final ForestWorldStateKeyValueStorage.Updater updater = worldStateStorage.updater(); + trie.commit((location, hash, value) -> updater.putAccountStateTrieNode(hash, value)); + updater.commit(); + + final Bytes32 startRange = Bytes32.rightPad(Bytes.of(0x02)); + + final RangeStorageEntriesCollector collector = + RangeStorageEntriesCollector.createCollector( + startRange, RangeManager.MAX_RANGE, 15, Integer.MAX_VALUE); + final TrieIterator visitor = RangeStorageEntriesCollector.createVisitor(collector); + final TreeMap entries = + (TreeMap) + trie.entriesFrom( + root -> + RangeStorageEntriesCollector.collectEntries( + collector, visitor, root, startRange)); + + final WorldStateProofProvider worldStateProofProvider = + new WorldStateProofProvider(worldStateStorageCoordinator); + + // generate the proof + final List proofs = + worldStateProofProvider.getAccountProofRelatedNodes( + Hash.wrap(trie.getRootHash()), startRange); + proofs.addAll( + worldStateProofProvider.getAccountProofRelatedNodes( + Hash.wrap(trie.getRootHash()), entries.lastKey())); + + // try to commit with stack trie + final ForestWorldStateKeyValueStorage recreatedWorldStateStorage = + new ForestWorldStateKeyValueStorage(new InMemoryKeyValueStorage()); + final StackTrie stackTrie = new StackTrie(Hash.wrap(trie.getRootHash()), 0, 256, startRange); + stackTrie.addSegment(); + stackTrie.addElement(Bytes32.random(), proofs, entries); + final ForestWorldStateKeyValueStorage.Updater updaterStackTrie = + recreatedWorldStateStorage.updater(); + stackTrie.commit( + (location, hash, value) -> updaterStackTrie.putAccountStateTrieNode(hash, value)); + updaterStackTrie.commit(); + + // verify the state of the db + Assertions.assertThat(worldStateStorage.getAccountStateTrieNode(trie.getRootHash())) + .isPresent(); + Assertions.assertThat(recreatedWorldStateStorage.getAccountStateTrieNode(trie.getRootHash())) + .isNotPresent(); + } + + @Test + public void shouldSaveNodeWithAllChildsInTheRange() { + final ForestWorldStateKeyValueStorage worldStateStorage = + new ForestWorldStateKeyValueStorage(new InMemoryKeyValueStorage()); + + final MerkleTrie trie = + new StoredMerklePatriciaTrie<>( + (location, hash) -> worldStateStorage.getAccountStateTrieNode(hash).map(Bytes::wrap), + b -> b, + b -> b); + + trie.put(Bytes32.rightPad(Bytes.of(0x10)), Bytes.of(0x01)); + trie.put(Bytes32.rightPad(Bytes.of(0x11)), Bytes.of(0x01)); + trie.put(Bytes32.rightPad(Bytes.of(0x20)), Bytes.of(0x01)); + trie.put(Bytes32.rightPad(Bytes.of(0x21)), Bytes.of(0x01)); + trie.put(Bytes32.rightPad(Bytes.of(0x01)), Bytes.of(0x02)); + trie.put(Bytes32.rightPad(Bytes.of(0x02)), Bytes.of(0x03)); + trie.put(Bytes32.rightPad(Bytes.of(0x03)), Bytes.of(0x04)); + + final ForestWorldStateKeyValueStorage.Updater updater = worldStateStorage.updater(); + trie.commit((location, hash, value) -> updater.putAccountStateTrieNode(hash, value)); + updater.commit(); + + final Bytes32 startRange = Bytes32.rightPad(Bytes.of(0x00)); + + final RangeStorageEntriesCollector collector = + RangeStorageEntriesCollector.createCollector( + startRange, RangeManager.MAX_RANGE, 15, Integer.MAX_VALUE); + final TrieIterator visitor = RangeStorageEntriesCollector.createVisitor(collector); + final TreeMap entries = + (TreeMap) + trie.entriesFrom( + root -> + RangeStorageEntriesCollector.collectEntries( + collector, visitor, root, startRange)); + + // try to commit with stack trie + final ForestWorldStateKeyValueStorage recreatedWorldStateStorage = + new ForestWorldStateKeyValueStorage(new InMemoryKeyValueStorage()); + final StackTrie stackTrie = new StackTrie(Hash.wrap(trie.getRootHash()), 0, 256, startRange); + stackTrie.addSegment(); + stackTrie.addElement(Bytes32.random(), new ArrayList<>(), entries); + final ForestWorldStateKeyValueStorage.Updater updaterStackTrie = + recreatedWorldStateStorage.updater(); + stackTrie.commit( + (location, hash, value) -> updaterStackTrie.putAccountStateTrieNode(hash, value)); + updaterStackTrie.commit(); + + // verify the state of the db + Assertions.assertThat(worldStateStorage.getAccountStateTrieNode(trie.getRootHash())) + .isPresent(); + Assertions.assertThat(recreatedWorldStateStorage.getAccountStateTrieNode(trie.getRootHash())) + .isPresent(); + } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java index 63895f4e6c5..9b5d9355168 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java @@ -24,6 +24,7 @@ import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider; import org.hyperledger.besu.ethereum.rlp.RLP; import org.hyperledger.besu.ethereum.trie.MerkleTrie; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.RangeStorageEntriesCollector; import org.hyperledger.besu.ethereum.trie.TrieIterator; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequestTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequestTest.java index 2799d631926..6c3ba6f0daa 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequestTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequestTest.java @@ -17,7 +17,6 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider; import org.hyperledger.besu.ethereum.core.TrieGenerator; -import org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncConfiguration; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncMetricsManager; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncProcessState; @@ -27,6 +26,7 @@ import org.hyperledger.besu.ethereum.storage.StorageProvider; import org.hyperledger.besu.ethereum.trie.CompactEncoding; import org.hyperledger.besu.ethereum.trie.MerkleTrie; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.RangeStorageEntriesCollector; import org.hyperledger.besu.ethereum.trie.TrieIterator; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequestTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequestTest.java index 41d02651308..eb6eedfb4c4 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequestTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequestTest.java @@ -20,7 +20,6 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider; import org.hyperledger.besu.ethereum.core.TrieGenerator; -import org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncConfiguration; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncProcessState; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapWorldDownloadState; @@ -29,6 +28,7 @@ import org.hyperledger.besu.ethereum.rlp.RLP; import org.hyperledger.besu.ethereum.storage.StorageProvider; import org.hyperledger.besu.ethereum.trie.MerkleTrie; +import org.hyperledger.besu.ethereum.trie.RangeManager; import org.hyperledger.besu.ethereum.trie.RangeStorageEntriesCollector; import org.hyperledger.besu.ethereum.trie.TrieIterator; import org.hyperledger.besu.ethereum.trie.bonsai.storage.BonsaiWorldStateKeyValueStorage; diff --git a/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/InnerNodeDiscoveryManager.java b/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/InnerNodeDiscoveryManager.java index 5190ada7ef9..d20b47e9e59 100644 --- a/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/InnerNodeDiscoveryManager.java +++ b/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/InnerNodeDiscoveryManager.java @@ -14,6 +14,9 @@ */ package org.hyperledger.besu.ethereum.trie; +import static org.hyperledger.besu.ethereum.trie.RangeManager.createPath; +import static org.hyperledger.besu.ethereum.trie.RangeManager.isInRange; + import org.hyperledger.besu.ethereum.rlp.RLPInput; import org.hyperledger.besu.ethereum.trie.patricia.BranchNode; import org.hyperledger.besu.ethereum.trie.patricia.ExtensionNode; @@ -21,7 +24,6 @@ import org.hyperledger.besu.ethereum.trie.patricia.StoredNodeFactory; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.function.Function; @@ -37,7 +39,7 @@ public class InnerNodeDiscoveryManager extends StoredNodeFactory { private final List innerNodes = new ArrayList<>(); - private final Bytes startKeyHash, endKeyHash; + private final Bytes startKeyPath, endKeyPath; private final boolean allowMissingElementInRange; @@ -49,8 +51,8 @@ public InnerNodeDiscoveryManager( final Bytes32 endKeyHash, final boolean allowMissingElementInRange) { super(nodeLoader, valueSerializer, valueDeserializer); - this.startKeyHash = createPath(startKeyHash); - this.endKeyHash = createPath(endKeyHash); + this.startKeyPath = createPath(startKeyHash); + this.endKeyPath = createPath(endKeyHash); this.allowMissingElementInRange = allowMissingElementInRange; } @@ -62,7 +64,7 @@ protected Node decodeExtension( final Supplier errMessage) { final ExtensionNode vNode = (ExtensionNode) super.decodeExtension(location, path, valueRlp, errMessage); - if (isInRange(Bytes.concatenate(location, Bytes.of(0)))) { + if (isInRange(Bytes.concatenate(location, Bytes.of(0)), startKeyPath, endKeyPath)) { innerNodes.add( ImmutableInnerNode.builder() .location(location) @@ -78,7 +80,7 @@ protected BranchNode decodeBranch( final BranchNode vBranchNode = super.decodeBranch(location, nodeRLPs, errMessage); final List> children = vBranchNode.getChildren(); for (int i = 0; i < children.size(); i++) { - if (isInRange(Bytes.concatenate(location, Bytes.of(i)))) { + if (isInRange(Bytes.concatenate(location, Bytes.of(i)), startKeyPath, endKeyPath)) { innerNodes.add( ImmutableInnerNode.builder() .location(location) @@ -97,7 +99,7 @@ protected LeafNode decodeLeaf( final Supplier errMessage) { final LeafNode vLeafNode = super.decodeLeaf(location, path, valueRlp, errMessage); final Bytes concatenatePath = Bytes.concatenate(location, path); - if (isInRange(concatenatePath.slice(0, concatenatePath.size() - 1))) { + if (isInRange(concatenatePath.slice(0, concatenatePath.size() - 1), startKeyPath, endKeyPath)) { innerNodes.add(ImmutableInnerNode.builder().location(location).path(path).build()); } return vLeafNode; @@ -106,10 +108,16 @@ protected LeafNode decodeLeaf( @Override public Optional> retrieve(final Bytes location, final Bytes32 hash) throws MerkleTrieException { + return super.retrieve(location, hash) + .map( + vNode -> { + vNode.markDirty(); + return vNode; + }) .or( () -> { - if (!allowMissingElementInRange && isInRange(location)) { + if (!allowMissingElementInRange && isInRange(location, startKeyPath, endKeyPath)) { return Optional.empty(); } return Optional.of(new MissingNode<>(hash, location)); @@ -120,23 +128,6 @@ public List getInnerNodes() { return List.copyOf(innerNodes); } - private boolean isInRange(final Bytes location) { - return !location.isEmpty() - && Arrays.compare(location.toArrayUnsafe(), startKeyHash.toArrayUnsafe()) >= 0 - && Arrays.compare(location.toArrayUnsafe(), endKeyHash.toArrayUnsafe()) <= 0; - } - - private Bytes createPath(final Bytes bytes) { - final MutableBytes path = MutableBytes.create(bytes.size() * 2); - int j = 0; - for (int i = 0; i < bytes.size(); i += 1, j += 2) { - final byte b = bytes.get(i); - path.set(j, (byte) ((b >>> 4) & 0x0f)); - path.set(j + 1, (byte) (b & 0x0f)); - } - return path; - } - public static Bytes32 decodePath(final Bytes bytes) { final MutableBytes32 decoded = MutableBytes32.create(); final MutableBytes path = MutableBytes.create(Bytes32.SIZE * 2); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManager.java b/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/RangeManager.java similarity index 74% rename from ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManager.java rename to ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/RangeManager.java index 83f58c10fbf..dbc7c10932a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManager.java +++ b/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/RangeManager.java @@ -12,14 +12,13 @@ * * SPDX-License-Identifier: Apache-2.0 */ -package org.hyperledger.besu.ethereum.eth.sync.snapsync; +package org.hyperledger.besu.ethereum.trie; import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.trie.InnerNodeDiscoveryManager; -import org.hyperledger.besu.ethereum.trie.MerkleTrieException; import org.hyperledger.besu.ethereum.trie.patricia.StoredMerklePatriciaTrie; import java.math.BigInteger; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -29,6 +28,7 @@ import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; +import org.apache.tuweni.bytes.MutableBytes; /** * This class helps to generate ranges according to several parameters (the start and the end of the @@ -152,4 +152,42 @@ public static Optional findNewBeginElementInRange( private static Bytes32 format(final BigInteger data) { return Bytes32.leftPad(Bytes.of(data.toByteArray()).trimLeadingZeros()); } + + /** + * Checks if a given location is within a specified range. This method determines whether a given + * location (represented as {@link Bytes}) falls within the range defined by a start key path and + * an end key path. + * + * @param location The location to check, represented as {@link Bytes}. + * @param startKeyPath The start of the range as path, represented as {@link Bytes}. + * @param endKeyPath The end of the range as path, represented as {@link Bytes}. + * @return {@code true} if the location is within the range (inclusive); {@code false} otherwise. + */ + public static boolean isInRange( + final Bytes location, final Bytes startKeyPath, final Bytes endKeyPath) { + final MutableBytes path = MutableBytes.create(Bytes32.SIZE * 2); + path.set(0, location); + return !location.isEmpty() + && Arrays.compare(path.toArrayUnsafe(), startKeyPath.toArrayUnsafe()) >= 0 + && Arrays.compare(path.toArrayUnsafe(), endKeyPath.toArrayUnsafe()) <= 0; + } + + /** + * Transforms a list of bytes into a path. This method processes a sequence of bytes, expanding + * each byte into two separate bytes to form a new path. The resulting path will have twice the + * length of the input byte sequence. + * + * @param bytes The byte sequence to be transformed into a path. + * @return A {@link Bytes} object representing the newly formed path. + */ + public static Bytes createPath(final Bytes bytes) { + final MutableBytes path = MutableBytes.create(bytes.size() * 2); + int j = 0; + for (int i = 0; i < bytes.size(); i += 1, j += 2) { + final byte b = bytes.get(i); + path.set(j, (byte) ((b >>> 4) & 0x0f)); + path.set(j + 1, (byte) (b & 0x0f)); + } + return path; + } } diff --git a/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/SnapCommitVisitor.java b/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/SnapCommitVisitor.java new file mode 100644 index 00000000000..099674e226b --- /dev/null +++ b/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/SnapCommitVisitor.java @@ -0,0 +1,138 @@ +/* + * 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.trie; + +import static org.hyperledger.besu.ethereum.trie.RangeManager.createPath; + +import org.hyperledger.besu.ethereum.trie.patricia.BranchNode; +import org.hyperledger.besu.ethereum.trie.patricia.ExtensionNode; + +import java.util.Arrays; + +import org.apache.tuweni.bytes.Bytes; +import org.apache.tuweni.bytes.Bytes32; +import org.apache.tuweni.bytes.MutableBytes; + +/** + * Implements a visitor for persisting changes to nodes during a snap synchronization process, + * focusing specifically on nodes that are marked as "dirty" but not as "heal needed". + * + *

This visitor plays a crucial role in the snap synchronization by identifying nodes that have + * been modified and require persistence ("dirty" nodes). The key functionality of this visitor is + * its selective persistence approach: it only persists changes to dirty nodes that are not marked + * as "heal needed". This strategy is designed to prevent future inconsistencies within the tree by + * ensuring that only nodes that are both modified and currently consistent with the rest of the + * structure are persisted. Nodes marked as "heal needed" are excluded from immediate persistence to + * allow for their proper healing in a controlled manner, ensuring the integrity and consistency of + * the data structure. + */ +public class SnapCommitVisitor extends CommitVisitor implements LocationNodeVisitor { + + private final Bytes startKeyPath; + private final Bytes endKeyPath; + + public SnapCommitVisitor( + final NodeUpdater nodeUpdater, final Bytes32 startKeyHash, final Bytes32 endKeyHash) { + super(nodeUpdater); + this.startKeyPath = createPath(startKeyHash); + this.endKeyPath = createPath(endKeyHash); + } + + /** + * Visits an extension node during a traversal operation. + * + *

This method is called when visiting an extension node. It checks if the node is marked as + * "dirty" (indicating changes that have not been persisted). If the node is clean, the method + * returns immediately. For dirty nodes, it recursively visits any dirty child nodes, + * concatenating the current location with the extension node's path to form the full path to the + * child. + * + *

Additionally, it checks if the child node requires healing (e.g., if it's falls outside the + * specified range defined by {@code startKeyPath} and {@code endKeyPath}). If healing is needed, + * the extension node is marked accordingly. + * + *

Finally, it attempts to persist the extension node if applicable. + * + * @param location The current location represented as {@link Bytes}. + * @param extensionNode The extension node being visited. + */ + @Override + public void visit(final Bytes location, final ExtensionNode extensionNode) { + if (!extensionNode.isDirty()) { + return; + } + + final Node child = extensionNode.getChild(); + if (child.isDirty()) { + child.accept(Bytes.concatenate(location, extensionNode.getPath()), this); + } + if (child.isHealNeeded() + || !isInRange( + Bytes.concatenate(location, extensionNode.getPath()), startKeyPath, endKeyPath)) { + extensionNode.markHealNeeded(); // not save an incomplete node + } + + maybeStoreNode(location, extensionNode); + } + + /** + * Visits a branch node during a traversal operation. + * + *

This method is invoked when visiting a branch node. It first checks if the branch node is + * marked as "dirty" (indicating changes that have not been persisted). If the node is clean, the + * method returns immediately. + * + *

For dirty branch nodes, it iterates through each child node. For each child, if the child is + * dirty, it recursively visits the child, passing along the concatenated path (current location + * plus the child's index) to the child's accept method. + * + *

Additionally, it checks if the child node requires healing (e.g., if it's falls outside the + * specified range of interest defined by {@code startKeyPath} and {@code endKeyPath}). If healing + * is needed, the branch node is marked accordingly. + * + *

Finally, it attempts to persist the branch node if applicable. + * + * @param location The current location represented as {@link Bytes}. + * @param branchNode The branch node being visited. + */ + @Override + public void visit(final Bytes location, final BranchNode branchNode) { + if (!branchNode.isDirty()) { + return; + } + + for (int i = 0; i < branchNode.maxChild(); ++i) { + Bytes index = Bytes.of(i); + final Node child = branchNode.child((byte) i); + if (child.isDirty()) { + child.accept(Bytes.concatenate(location, index), this); + } + if (child.isHealNeeded() + || !isInRange(Bytes.concatenate(location, index), startKeyPath, endKeyPath)) { + branchNode.markHealNeeded(); // not save an incomplete node + } + } + + maybeStoreNode(location, branchNode); + } + + private boolean isInRange( + final Bytes location, final Bytes startKeyPath, final Bytes endKeyPath) { + final MutableBytes path = MutableBytes.create(Bytes32.SIZE * 2); + path.set(0, location); + return Arrays.compare(path.toArrayUnsafe(), startKeyPath.toArrayUnsafe()) >= 0 + && Arrays.compare(path.toArrayUnsafe(), endKeyPath.toArrayUnsafe()) <= 0; + } +} diff --git a/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/SnapPutVisitor.java b/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/SnapPutVisitor.java deleted file mode 100644 index 0cce55dcf4a..00000000000 --- a/ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/SnapPutVisitor.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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.trie; - -import org.hyperledger.besu.ethereum.trie.patricia.BranchNode; -import org.hyperledger.besu.ethereum.trie.patricia.ExtensionNode; -import org.hyperledger.besu.ethereum.trie.patricia.PutVisitor; - -import org.apache.tuweni.bytes.Bytes; - -public class SnapPutVisitor extends PutVisitor { - - public SnapPutVisitor(final NodeFactory nodeFactory, final V value) { - super(nodeFactory, value); - } - - @Override - public Node visit(final BranchNode branchNode, final Bytes path) { - final Node visit = super.visit(branchNode, path); - for (Node child : visit.getChildren()) { - if (child.isHealNeeded() || (child instanceof StoredNode && child.getValue().isEmpty())) { - visit.markHealNeeded(); // not save an incomplete node - return visit; - } - } - return visit; - } - - @Override - public Node visit(final ExtensionNode extensionNode, final Bytes path) { - final Node visit = super.visit(extensionNode, path); - for (Node child : visit.getChildren()) { - if (child.isHealNeeded() || (child instanceof StoredNode && child.getValue().isEmpty())) { - visit.markHealNeeded(); // not save an incomplete node - return visit; - } - } - return visit; - } -} diff --git a/ethereum/trie/src/test/java/org/hyperledger/besu/ethereum/trie/SnapCommitVisitorTest.java b/ethereum/trie/src/test/java/org/hyperledger/besu/ethereum/trie/SnapCommitVisitorTest.java new file mode 100644 index 00000000000..5b281529893 --- /dev/null +++ b/ethereum/trie/src/test/java/org/hyperledger/besu/ethereum/trie/SnapCommitVisitorTest.java @@ -0,0 +1,127 @@ +/* + * 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.trie; + +import static org.mockito.Mockito.mock; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.trie.patricia.BranchNode; +import org.hyperledger.besu.ethereum.trie.patricia.ExtensionNode; +import org.hyperledger.besu.ethereum.trie.patricia.StoredNodeFactory; + +import java.util.ArrayList; +import java.util.Optional; +import java.util.function.Function; + +import org.apache.tuweni.bytes.Bytes; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; + +@SuppressWarnings("unchecked") +public class SnapCommitVisitorTest { + + @Test + public void shouldDetectValidBranch() { + final StoredNodeFactory storedNodeFactory = mock(StoredNodeFactory.class); + final ArrayList> children = new ArrayList<>(); + for (int i = 0; i < 16; i++) { + children.add( + new StoredNode<>( + storedNodeFactory, Bytes.concatenate(Bytes.of(0x00), Bytes.of(i)), Hash.ZERO)); + } + final BranchNode validBranchNode = + new BranchNode<>( + Bytes.of(0x00), + children, + Optional.of(Bytes.of(0x00)), + storedNodeFactory, + Function.identity()); + validBranchNode.markDirty(); + final SnapCommitVisitor snapCommitVisitor = + new SnapCommitVisitor<>( + (location, hash, value) -> {}, RangeManager.MIN_RANGE, RangeManager.MAX_RANGE); + Assertions.assertThat(validBranchNode.isHealNeeded()).isFalse(); + snapCommitVisitor.visit(validBranchNode.getLocation().get(), validBranchNode); + Assertions.assertThat(validBranchNode.isHealNeeded()).isFalse(); + } + + @Test + public void shouldDetectBranchWithChildrenNotInTheRange() { + final StoredNodeFactory storedNodeFactory = mock(StoredNodeFactory.class); + final ArrayList> children = new ArrayList<>(); + for (int i = 0; i < 16; i++) { + children.add( + new StoredNode<>( + storedNodeFactory, Bytes.concatenate(Bytes.of(0x01), Bytes.of(i)), Hash.ZERO)); + } + + final BranchNode invalidBranchNode = + new BranchNode<>( + Bytes.of(0x01), + children, + Optional.of(Bytes.of(0x00)), + storedNodeFactory, + Function.identity()); + invalidBranchNode.markDirty(); + final SnapCommitVisitor snapCommitVisitor = + new SnapCommitVisitor<>( + (location, hash, value) -> {}, + RangeManager.MIN_RANGE, + Hash.fromHexString( + "0x1effffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")); + Assertions.assertThat(invalidBranchNode.isHealNeeded()).isFalse(); + snapCommitVisitor.visit(invalidBranchNode.getLocation().get(), invalidBranchNode); + Assertions.assertThat(invalidBranchNode.isHealNeeded()).isTrue(); + } + + @Test + public void shouldDetectValidExtension() { + final StoredNodeFactory storedNodeFactory = mock(StoredNodeFactory.class); + final ExtensionNode validExtensionNode = + new ExtensionNode<>( + Bytes.of(0x00), + Bytes.of(0x01), + new StoredNode<>(storedNodeFactory, Bytes.of((byte) 0x00, (byte) 0x01), Hash.ZERO), + storedNodeFactory); + validExtensionNode.markDirty(); + final SnapCommitVisitor snapCommitVisitor = + new SnapCommitVisitor<>( + (location, hash, value) -> {}, RangeManager.MIN_RANGE, RangeManager.MAX_RANGE); + Assertions.assertThat(validExtensionNode.isHealNeeded()).isFalse(); + snapCommitVisitor.visit(validExtensionNode.getLocation().get(), validExtensionNode); + Assertions.assertThat(validExtensionNode.isHealNeeded()).isFalse(); + } + + @Test + public void shouldDetectExtensionWithChildNotInRange() { + final StoredNodeFactory storedNodeFactory = mock(StoredNodeFactory.class); + final ExtensionNode inValidExtensionNode = + new ExtensionNode<>( + Bytes.of(0x00), + Bytes.of(0x03), + new StoredNode<>(storedNodeFactory, Bytes.of((byte) 0x00, (byte) 0x03), Hash.ZERO), + storedNodeFactory); + inValidExtensionNode.markDirty(); + final SnapCommitVisitor snapCommitVisitor = + new SnapCommitVisitor<>( + (location, hash, value) -> {}, + RangeManager.MIN_RANGE, + Hash.fromHexString( + "0x02ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")); + Assertions.assertThat(inValidExtensionNode.isHealNeeded()).isFalse(); + snapCommitVisitor.visit(inValidExtensionNode.getLocation().get(), inValidExtensionNode); + Assertions.assertThat(inValidExtensionNode.isHealNeeded()).isTrue(); + } +} diff --git a/ethereum/trie/src/test/java/org/hyperledger/besu/ethereum/trie/SnapPutVisitorTest.java b/ethereum/trie/src/test/java/org/hyperledger/besu/ethereum/trie/SnapPutVisitorTest.java deleted file mode 100644 index cbedbe9a288..00000000000 --- a/ethereum/trie/src/test/java/org/hyperledger/besu/ethereum/trie/SnapPutVisitorTest.java +++ /dev/null @@ -1,129 +0,0 @@ -/* - * 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.trie; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyByte; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.trie.patricia.BranchNode; -import org.hyperledger.besu.ethereum.trie.patricia.ExtensionNode; -import org.hyperledger.besu.ethereum.trie.patricia.LeafNode; -import org.hyperledger.besu.ethereum.trie.patricia.StoredNodeFactory; - -import java.util.ArrayList; -import java.util.Optional; -import java.util.function.Function; - -import org.apache.tuweni.bytes.Bytes; -import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.Test; - -@SuppressWarnings("unchecked") -public class SnapPutVisitorTest { - - @Test - public void shouldDetectValidBranch() { - final StoredNodeFactory storedNodeFactory = mock(StoredNodeFactory.class); - when(storedNodeFactory.createBranch(any(), any())) - .thenReturn( - new LeafNode( - Bytes.EMPTY, Bytes.of(0x00), storedNodeFactory, Function.identity())); - final ArrayList> children = new ArrayList<>(); - for (int i = 0; i < 16; i++) { - children.add(new StoredNode<>(storedNodeFactory, Bytes.EMPTY, Hash.ZERO)); - } - final BranchNode invalidBranchNode = - new BranchNode<>( - Bytes.EMPTY, - children, - Optional.of(Bytes.of(0x00)), - storedNodeFactory, - Function.identity()); - final SnapPutVisitor snapPutVisitor = - new SnapPutVisitor<>(storedNodeFactory, Bytes.EMPTY); - Node visit = - snapPutVisitor.visit(invalidBranchNode, Bytes.of(CompactEncoding.LEAF_TERMINATOR)); - Assertions.assertThat(visit.isHealNeeded()).isFalse(); - } - - @Test - public void shouldDetectBranchWithMissingChildren() { - final StoredNodeFactory storedNodeFactory = mock(StoredNodeFactory.class); - when(storedNodeFactory.createBranch(any(), any())) - .thenReturn(new MissingNode<>(Hash.ZERO, Bytes.EMPTY)); - final ArrayList> children = new ArrayList<>(); - for (int i = 0; i < 16; i++) { - children.add(new StoredNode<>(storedNodeFactory, Bytes.EMPTY, Hash.ZERO)); - } - final BranchNode invalidBranchNode = - new BranchNode<>( - Bytes.EMPTY, - children, - Optional.of(Bytes.of(0x00)), - storedNodeFactory, - Function.identity()); - final SnapPutVisitor snapPutVisitor = - new SnapPutVisitor<>(storedNodeFactory, Bytes.EMPTY); - Node visit = - snapPutVisitor.visit(invalidBranchNode, Bytes.of(CompactEncoding.LEAF_TERMINATOR)); - Assertions.assertThat(visit.isHealNeeded()).isTrue(); - } - - @Test - public void shouldDetectValidExtension() { - final StoredNodeFactory storedNodeFactory = mock(StoredNodeFactory.class); - when(storedNodeFactory.createBranch(any(), any())) - .thenReturn( - new LeafNode<>(Bytes.EMPTY, Bytes.of(0x00), storedNodeFactory, Function.identity())); - final ArrayList> children = new ArrayList<>(); - for (int i = 0; i < 16; i++) { - children.add(new StoredNode<>(storedNodeFactory, Bytes.EMPTY, Hash.ZERO)); - } - final BranchNode invalidBranchNode = - new BranchNode<>( - Bytes.EMPTY, - children, - Optional.of(Bytes.of(0x00)), - storedNodeFactory, - Function.identity()); - final SnapPutVisitor snapPutVisitor = - new SnapPutVisitor<>(storedNodeFactory, Bytes.EMPTY); - Node visit = - snapPutVisitor.visit(invalidBranchNode, Bytes.of(CompactEncoding.LEAF_TERMINATOR)); - Assertions.assertThat(visit.isHealNeeded()).isFalse(); - } - - @Test - public void shouldDetectExtensionWithMissingChildren() { - final StoredNodeFactory storedNodeFactory = mock(StoredNodeFactory.class); - when(storedNodeFactory.createBranch(anyByte(), any(), anyByte(), any())) - .thenReturn(new MissingNode<>(Hash.ZERO, Bytes.EMPTY)); - when(storedNodeFactory.createLeaf(any(), any())) - .thenReturn(new MissingNode<>(Hash.ZERO, Bytes.EMPTY)); - final ExtensionNode invalidBranchNode = - new ExtensionNode<>( - Bytes.of(0x00), - new StoredNode<>(storedNodeFactory, Bytes.EMPTY, Hash.ZERO), - storedNodeFactory); - final SnapPutVisitor snapPutVisitor = - new SnapPutVisitor<>(storedNodeFactory, Bytes.EMPTY); - Node visit = - snapPutVisitor.visit(invalidBranchNode, Bytes.of(CompactEncoding.LEAF_TERMINATOR)); - Assertions.assertThat(visit.isHealNeeded()).isTrue(); - } -}