revert back to children enqueue for bottom-up saving of trie. known b…

…roken persist test Signed-off-by: garyschulte <[email protected]>
hyperledger · Jan 18, 2024 · 7cff6d0 · 7cff6d0
1 parent 6d3f362
commit 7cff6d0
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 23 deletions.
diff --git a/...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStep.java b/...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStep.java
@@ -56,6 +56,19 @@ public List<Task<SnapDataRequest>> persist(final List<Task<SnapDataRequest>> tas
       final WorldStateStorage.Updater updater = worldStateStorage.updater();
       for (Task<SnapDataRequest> task : tasks) {
         if (task.getData().isResponseReceived()) {
+          // enqueue child requests
+          final Stream<SnapDataRequest> childRequests =
+              task.getData().getChildRequests(downloadState, worldStateStorage, snapSyncState);
+          if (!(task.getData() instanceof TrieNodeHealingRequest)) {
+            enqueueChildren(childRequests);
+          } else {
+            if (!task.getData().isExpired(snapSyncState)) {
+              enqueueChildren(childRequests);
+            } else {
+              continue;
+            }
+          }
+
           // persist nodes
           final int persistedNodes =
               task.getData()
@@ -72,19 +85,6 @@ public List<Task<SnapDataRequest>> persist(final List<Task<SnapDataRequest>> tas
               downloadState.getMetricsManager().notifyNodesGenerated(persistedNodes);
             }
           }
-
-          // enqueue child requests
-          final Stream<SnapDataRequest> childRequests =
-              task.getData().getChildRequests(downloadState, worldStateStorage, snapSyncState);
-          if (!(task.getData() instanceof TrieNodeHealingRequest)) {
-            enqueueChildren(childRequests);
-          } else {
-            if (!task.getData().isExpired(snapSyncState)) {
-              enqueueChildren(childRequests);
-            } else {
-              continue;
-            }
-          }
         }
       }
       updater.commit();

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RangeManager.java
@@ -38,8 +38,7 @@
 public class RangeManager {
 
   public static final Hash MIN_RANGE = Hash.wrap(Bytes32.ZERO);
-  public static final Hash MAX_RANGE =
-      Hash.fromHexString("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
+  public static final Hash MAX_RANGE = Hash.LAST;
 
   private RangeManager() {}
 

diff --git a/...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java b/...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java
@@ -143,13 +143,12 @@ public CompletableFuture<List<Task<SnapDataRequest>>> requestStorage(
                   }
                 }
 
-                // if we received no slots, but do do get a proof of exclusion, add as a response:
+                // if we received no slots, but do get a proof of exclusion, add as a response:
                 else if (response.proofs().size() > 0 && requestTasks.size() > 0) {
                   requestTasks.stream()
-                      .findFirst()
                       .filter(task -> task instanceof StorageRangeDataRequest)
                       .map(StorageRangeDataRequest.class::cast)
-                      .ifPresent(
+                      .forEach(
                           storageRequest ->
                               storageRequest.addResponse(
                                   downloadState,

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
@@ -125,7 +125,7 @@ public void commit(final FlatDatabaseUpdater flatDatabaseUpdater, final NodeUpda
               Function.identity(),
               Function.identity(),
               startKeyHash,
-              keys.size() > 0 ? keys.lastKey() : Hash.LAST,
+              keys.size() > 0 ? keys.lastKey() : RangeManager.MAX_RANGE,
               true);
 
       final MerkleTrie<Bytes, Bytes> trie =

diff --git a/...java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java b/...java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java
@@ -97,6 +97,7 @@ protected int doPersist(
     final NodeUpdater nodeUpdater =
         (location, hash, value) -> {
           updater.putAccountStorageTrieNode(accountHash, location, hash, value);
+          nbNodesSaved.getAndIncrement();
         };
 
     StackTrie.FlatDatabaseUpdater flatDatabaseUpdater = noop();

diff --git a/...th/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java b/...th/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java
@@ -39,7 +39,7 @@
 public class PersistDataStepTest {
 
   private final WorldStateStorage worldStateStorage =
-      new InMemoryKeyValueStorageProvider().createWorldStateStorage(DataStorageFormat.FOREST);
+      new InMemoryKeyValueStorageProvider().createWorldStateStorage(DataStorageFormat.BONSAI);
   private final SnapSyncProcessState snapSyncState = mock(SnapSyncProcessState.class);
   private final SnapWorldDownloadState downloadState = mock(SnapWorldDownloadState.class);
 

diff --git a/.../org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequestTest.java b/.../org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequestTest.java
@@ -17,6 +17,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import org.hyperledger.besu.datatypes.Hash;
+import org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager;
 import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapWorldDownloadState;
 import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage;
@@ -43,7 +44,11 @@ public void assertEmptySlotsWithProofOfExclusionCompletes() {
 
     var storageRangeRequest =
         new StorageRangeDataRequest(
-            Hash.EMPTY_TRIE_HASH, Bytes32.ZERO, Hash.EMPTY_TRIE_HASH, Bytes32.ZERO, Hash.LAST);
+            Hash.EMPTY_TRIE_HASH,
+            Bytes32.ZERO,
+            Hash.EMPTY_TRIE_HASH,
+            Bytes32.ZERO,
+            RangeManager.MAX_RANGE);
 
     var proofs = new ArrayDeque<Bytes>();
     proofs.add(0, Hash.EMPTY_TRIE_HASH);
@@ -59,7 +64,11 @@ public void assertEmptySlotsWithProofOfExclusionCompletes() {
   public void assertEmptySlotsWithInvalidProofCompletes() {
     var storageRangeRequest =
         new StorageRangeDataRequest(
-            Hash.ZERO, Bytes32.fromHexString("0x01"), Bytes32.ZERO, Bytes32.ZERO, Hash.LAST);
+            Hash.ZERO,
+            Bytes32.fromHexString("0x01"),
+            Bytes32.ZERO,
+            Bytes32.ZERO,
+            RangeManager.MAX_RANGE);
 
     var proofs = new ArrayDeque<Bytes>();
     proofs.add(0, Hash.ZERO);

diff --git a/...reum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/InnerNodeDiscoveryManager.java b/...reum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/InnerNodeDiscoveryManager.java
@@ -97,7 +97,8 @@ protected LeafNode<V> decodeLeaf(
       final Supplier<String> errMessage) {
     final LeafNode<V> vLeafNode = super.decodeLeaf(location, path, valueRlp, errMessage);
     final Bytes concatenatePath = Bytes.concatenate(location, path);
-    if (isInRange(concatenatePath.slice(0, concatenatePath.size() - 1))) {
+    if (!concatenatePath.isEmpty()
+        && isInRange(concatenatePath.slice(0, concatenatePath.size() - 1))) {
       innerNodes.add(ImmutableInnerNode.builder().location(location).path(path).build());
     }
     return vLeafNode;