refactor SnapServer storage proving conditions

Signed-off-by: garyschulte <[email protected]>
hyperledger · Nov 6, 2023 · 035e23b · 035e23b
1 parent b9e7b08
commit 035e23b
Showing 1 changed file with 29 additions and 18 deletions.
diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java
@@ -42,9 +42,11 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.NavigableMap;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
@@ -146,7 +148,7 @@ public SnapServer stop() {
 
   private void primeWorldStateArchive(
       final Optional<CachedWorldStorageManager> storageManager, final Blockchain blockchain) {
-    // at startup, prime the latest worldstates' by roothash:
+    // at startup, prime the latest worldstates by roothash:
     storageManager.ifPresent(
         cache -> cache.primeRootToBlockHashCache(blockchain, PRIME_STATE_ROOT_CACHE_LIMIT));
   }
@@ -221,6 +223,13 @@ MessageData constructGetAccountRangeResponse(final MessageData message) {
                           range.worldStateRootHash(), Hash.wrap(accounts.lastKey())));
                 }
                 var resp = AccountRangeMessage.create(accounts, proof);
+                if (accounts.isEmpty()) {
+                  LOGGER.warn(
+                      "returned empty account range message for {} to  {}, proof count {}",
+                      range.startKeyHash(),
+                      range.endKeyHash(),
+                      proof.size());
+                }
                 LOGGER.debug(
                     "returned account range message with {} accounts and {} proofs",
                     accounts.size(),
@@ -296,7 +305,7 @@ MessageData constructGetStorageRangeResponse(final MessageData message) {
                 }
 
                 ArrayDeque<NavigableMap<Bytes32, Bytes>> collectedStorages = new ArrayDeque<>();
-                ArrayList<Bytes> proofNodes = new ArrayList<>();
+                Set<Bytes> proofNodes = new LinkedHashSet<>();
                 final var worldStateProof = new WorldStateProofProvider(storage);
 
                 for (var forAccountHash : range.hashes()) {
@@ -306,26 +315,26 @@ MessageData constructGetStorageRangeResponse(final MessageData message) {
                           Hash.wrap(forAccountHash), startKeyBytes, endKeyBytes, statefulPredicate);
                   collectedStorages.add(accountStorages);
 
-                  // if this was a partial range, send a proof for the left side:
-                  if (!startKeyBytes.equals(Hash.ZERO)) {
-                    proofNodes.addAll(
-                        worldStateProof.getStorageProofRelatedNodes(
-                            getAccountStorageRoot(forAccountHash, storage),
-                            forAccountHash,
-                            Hash.wrap(startKeyBytes)));
+                  // TODO: for debugging only, remove me
+                  if (accountStorages.isEmpty()) {
+                    LOGGER.warn(
+                        "returned empty storage range message for account hash {}, {} to  {}",
+                        forAccountHash.toHexString(),
+                        range.startKeyHash(),
+                        range.endKeyHash());
                   }
 
-                  // if this was a partial ending range, send a proof for the right side:
-                  if (!startKeyBytes.equals(Hash.ZERO)) {
+                  // if this a partial storage range was requested, or we interrupted storage due
+                  // to request limits, send proofs:
+                  if (!(startKeyBytes.equals(Hash.ZERO) && endKeyBytes.equals(HASH_LAST))
+                      || !statefulPredicate.shouldGetMore()) {
+                    // send a proof for the left side
                     proofNodes.addAll(
                         worldStateProof.getStorageProofRelatedNodes(
                             getAccountStorageRoot(forAccountHash, storage),
                             forAccountHash,
-                            Hash.wrap(endKeyBytes)));
-                  }
-
-                  // add last key proof, in case it was partial storage
-                  if (!statefulPredicate.shouldGetMore() && accountStorages.size() > 0) {
+                            Hash.wrap(startKeyBytes)));
+                    // and last account:
                     proofNodes.addAll(
                         worldStateProof.getStorageProofRelatedNodes(
                             getAccountStorageRoot(forAccountHash, storage),
@@ -335,8 +344,10 @@ MessageData constructGetStorageRangeResponse(final MessageData message) {
                   }
                 }
 
-                var resp = StorageRangeMessage.create(collectedStorages, proofNodes);
-                LOGGER.info(
+                var resp =
+                    StorageRangeMessage.create(collectedStorages, proofNodes.stream().toList());
+
+                LOGGER.debug(
                     "returned storage range message with {} storages and {} proofs",
                     collectedStorages.size(),
                     proofNodes.size());