getTrieNodes and test coverage and rebase on main

Signed-off-by: garyschulte <[email protected]>
hyperledger · Sep 29, 2023 · 6790640 · 6790640
1 parent ce8fc8b
commit 6790640
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 35 deletions.
diff --git a/...e/src/main/java/org/hyperledger/besu/ethereum/bonsai/cache/CachedWorldStorageManager.java b/...e/src/main/java/org/hyperledger/besu/ethereum/bonsai/cache/CachedWorldStorageManager.java
@@ -203,6 +203,11 @@ public Optional<BonsaiWorldState> getHeadWorldState(
             });
   }
 
+  @Override
+  public Optional<CachedBonsaiWorldView> getStorageByRootHash(final Hash rootHash) {
+    return Optional.ofNullable(cachedWorldStatesByHash.get(rootHash));
+  }
+
   @Override
   public void reset() {
     this.cachedWorldStatesByHash.clear();

diff --git a/...in/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java b/...in/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java
@@ -196,9 +196,7 @@ public Optional<Bytes> getAccountStorageTrieNode(
 
   @Override
   public Optional<Bytes> getTrieNodeUnsafe(final Bytes key) {
-    return composedWorldStateStorage
-        .get(TRIE_BRANCH_STORAGE, Bytes.concatenate(key).toArrayUnsafe())
-        .map(Bytes::wrap);
+    return composedWorldStateStorage.get(TRIE_BRANCH_STORAGE, key.toArrayUnsafe()).map(Bytes::wrap);
   }
 
   public Optional<byte[]> getTrieLog(final Hash blockHash) {

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/trielog/TrieLogManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/trielog/TrieLogManager.java
@@ -16,6 +16,7 @@
 package org.hyperledger.besu.ethereum.bonsai.trielog;
 
 import org.hyperledger.besu.datatypes.Hash;
+import org.hyperledger.besu.ethereum.bonsai.cache.CachedBonsaiWorldView;
 import org.hyperledger.besu.ethereum.bonsai.worldview.BonsaiWorldState;
 import org.hyperledger.besu.ethereum.bonsai.worldview.BonsaiWorldStateUpdateAccumulator;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
@@ -45,6 +46,8 @@ void addCachedLayer(
   Optional<BonsaiWorldState> getHeadWorldState(
       final Function<Hash, Optional<BlockHeader>> hashBlockHeaderFunction);
 
+  Optional<CachedBonsaiWorldView> getStorageByRootHash(Hash rootHash);
+
   long getMaxLayersToLoad();
 
   void reset();

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
@@ -15,6 +15,8 @@
 package org.hyperledger.besu.ethereum.eth.manager.snap;
 
 import org.hyperledger.besu.datatypes.Hash;
+import org.hyperledger.besu.ethereum.bonsai.BonsaiWorldStateProvider;
+import org.hyperledger.besu.ethereum.bonsai.cache.CachedBonsaiWorldView;
 import org.hyperledger.besu.ethereum.eth.manager.EthMessages;
 import org.hyperledger.besu.ethereum.eth.messages.snap.AccountRangeMessage;
 import org.hyperledger.besu.ethereum.eth.messages.snap.ByteCodesMessage;
@@ -41,7 +43,6 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import kotlin.Pair;
 import kotlin.collections.ArrayDeque;
@@ -69,8 +70,13 @@ class SnapServer {
 
   SnapServer(final EthMessages snapMessages, final WorldStateArchive archive) {
     this.snapMessages = snapMessages;
-    // TODO implement worldstate storage retrieval by root hash in WorldStateArchive:
-    this.worldStateStorageProvider = __ -> null;
+    // TODO remove dirty bonsai cast:
+    this.worldStateStorageProvider =
+        rootHash ->
+            ((BonsaiWorldStateProvider) archive)
+                .getTrieLogManager()
+                .getStorageByRootHash(rootHash)
+                .map(CachedBonsaiWorldView::getWorldStateStorage);
   }
 
   SnapServer(
@@ -204,7 +210,7 @@ private MessageData constructGetBytecodesResponse(final MessageData message) {
     return ByteCodesMessage.create(new ArrayDeque<>());
   }
 
-  private MessageData constructGetTrieNodesResponse(final MessageData message) {
+  MessageData constructGetTrieNodesResponse(final MessageData message) {
     final GetTrieNodesMessage getTrieNodesMessage = GetTrieNodesMessage.readFrom(message);
     final GetTrieNodesMessage.TrieNodesPaths triePaths = getTrieNodesMessage.paths(true);
     // TODO: drop to TRACE
@@ -214,33 +220,29 @@ private MessageData constructGetTrieNodesResponse(final MessageData message) {
         .addArgument(() -> triePaths.paths().size())
         .log();
 
+    //TODO: implement limits
     return worldStateStorageProvider
         .apply(triePaths.worldStateRootHash())
         .map(
             storage -> {
               ArrayList<Bytes> trieNodes = new ArrayList<>();
               for (var triePath : triePaths.paths()) {
+                // first element is paths is account
+                if (triePath.size() == 1) {
+                  // if there is only one path, presume it should be compact encoded account path
+                  storage.getTrieNodeUnsafe(triePath.get(0)).ifPresent(trieNodes::add);
+                } else {
+                  // otherwise the first element should be account hash, and subsequent paths
+                  // are compact encoded account storage paths
 
-                // first element is path in account trie
-                final Stream<Bytes> pathStream = triePath.stream();
-                final Optional<Bytes> accountPath = pathStream.findFirst();
+                  final Bytes accountPrefix = triePath.get(0);
 
-                if (triePaths.paths().size() == 1) {
-                  // if we are only requesting the account node, return it
-                  // TODO: confirm whether this is binary or compact encoding for account
-                  accountPath.flatMap(storage::getTrieNodeUnsafe).ifPresent(trieNodes::add);
-                } else {
-                  // otherwise return the storage from this account from the subsequent paths:
-                  pathStream
-                      .map(this::applyPathMagic)
+                  triePath.subList(1, triePath.size()).stream()
                       .forEach(
-                          pathPair -> {
-                            // then return it:
-                            storage.getAccountStorageTrieNode(
-                                null, // TODO: get accountHash
-                                pathPair.getSecond(),
-                                pathPair.getFirst());
-                          });
+                          path ->
+                              storage
+                                  .getTrieNodeUnsafe(Bytes.concatenate(accountPrefix, path))
+                                  .ifPresent(trieNodes::add));
                 }
               }
               return TrieNodesMessage.create(trieNodes);
@@ -293,12 +295,4 @@ Hash getAccountStorageRoot(final Bytes32 accountHash, final WorldStateStorage st
         .map(Hash::hash)
         .orElse(Hash.EMPTY_TRIE_HASH);
   }
-
-  Pair<Bytes32, Bytes> applyPathMagic(Bytes path) {
-    // TODO: write me.  Determine whether we have a partial path with compact encoding
-    //  or full path binary encoded. return location and hash pair
-
-    // !!this impl 100% broken!!
-    return new Pair(Bytes32.wrap(path), path);
-  }
 }
diff --git a/...reum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServerTest.java b/...reum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServerTest.java
@@ -24,11 +24,14 @@
 import org.hyperledger.besu.ethereum.eth.messages.snap.AccountRangeMessage;
 import org.hyperledger.besu.ethereum.eth.messages.snap.GetAccountRangeMessage;
 import org.hyperledger.besu.ethereum.eth.messages.snap.GetStorageRangeMessage;
+import org.hyperledger.besu.ethereum.eth.messages.snap.GetTrieNodesMessage;
 import org.hyperledger.besu.ethereum.eth.messages.snap.StorageRangeMessage;
+import org.hyperledger.besu.ethereum.eth.messages.snap.TrieNodesMessage;
 import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
 import org.hyperledger.besu.ethereum.rlp.RLP;
 import org.hyperledger.besu.ethereum.storage.keyvalue.KeyValueStorageProvider;
+import org.hyperledger.besu.ethereum.trie.CompactEncoding;
 import org.hyperledger.besu.ethereum.trie.MerkleTrie;
 import org.hyperledger.besu.ethereum.trie.patricia.SimpleMerklePatriciaTrie;
 import org.hyperledger.besu.ethereum.trie.patricia.StoredMerklePatriciaTrie;
@@ -166,6 +169,39 @@ public void assertStorageForSingleAccount() {
     //     .isTrue();
   }
 
+  @Test
+  public void assertAccountTriePathRequest() {
+    insertTestAccounts(acct1, acct2, acct3, acct4);
+    var partialPathToAcct2 = CompactEncoding.bytesToPath(acct2.addressHash).slice(0, 1);
+    var partialPathToAcct1 = Bytes.fromHexString("0x01"); // first nibble is 1
+    var trieNodeRequest =
+        requestTrieNodes(
+            storageTrie.getRootHash(),
+            List.of(List.of(partialPathToAcct2), List.of(partialPathToAcct1)));
+    assertThat(trieNodeRequest).isNotNull();
+    List<Bytes> trieNodes = trieNodeRequest.nodes(false);
+    assertThat(trieNodes).isNotNull();
+    assertThat(trieNodes.size()).isEqualTo(2);
+  }
+
+  @Test
+  public void assertStorageTriePathRequest() {
+    insertTestAccounts(acct1, acct2, acct3, acct4);
+    var pathToSlot11 = Bytes.fromHexStringLenient("0x0101");
+    var pathToSlot12 = Bytes.fromHexStringLenient("0x0102");
+    var pathToSlot1a = Bytes.fromHexStringLenient("0x010A"); // not present
+    var trieNodeRequest =
+        requestTrieNodes(
+            storageTrie.getRootHash(),
+            List.of(
+                List.of(acct3.addressHash, pathToSlot11, pathToSlot12, pathToSlot1a),
+                List.of(acct4.addressHash, pathToSlot11, pathToSlot12, pathToSlot1a)));
+    assertThat(trieNodeRequest).isNotNull();
+    List<Bytes> trieNodes = trieNodeRequest.nodes(false);
+    assertThat(trieNodes).isNotNull();
+    assertThat(trieNodes.size()).isEqualTo(4);
+  }
+
   static SnapTestAccount createTestAccount(final String hexAddr) {
     return new SnapTestAccount(
         Hash.wrap(Bytes32.rightPad(Bytes.fromHexString(hexAddr))),
@@ -193,7 +229,7 @@ static SnapTestAccount createTestContractAccount(
         .boxed()
         .forEach(
             i -> {
-              Bytes32 mockBytes32 = Bytes32.fromHexStringLenient(i.toString());
+              Bytes32 mockBytes32 = Bytes32.rightPad(Bytes.fromHexString(i.toString()));
               trie.put(mockBytes32, mockBytes32);
               flatdb.putFlatAccountStorageValueByStorageSlotHash(
                   updater.getWorldStateTransaction(),
@@ -271,6 +307,13 @@ StorageRangeMessage requestStorageRange(
                 .wrapMessageData(BigInteger.ONE));
   }
 
+  TrieNodesMessage requestTrieNodes(final Bytes32 rootHash, final List<List<Bytes>> trieNodesList) {
+    return (TrieNodesMessage)
+        snapServer.constructGetTrieNodesResponse(
+            GetTrieNodesMessage.create(Hash.wrap(rootHash), trieNodesList)
+                .wrapMessageData(BigInteger.ONE));
+  }
+
   AccountRangeMessage.AccountRangeData getAndVerifyAcountRangeData(
       final AccountRangeMessage range, final int expectedSize) {
     assertThat(range).isNotNull();

diff --git a/...main/java/org/hyperledger/besu/ethereum/referencetests/BonsaiReferenceTestWorldState.java b/...main/java/org/hyperledger/besu/ethereum/referencetests/BonsaiReferenceTestWorldState.java
@@ -16,6 +16,7 @@
 
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Hash;
+import org.hyperledger.besu.ethereum.bonsai.cache.CachedBonsaiWorldView;
 import org.hyperledger.besu.ethereum.bonsai.cache.CachedMerkleTrieLoader;
 import org.hyperledger.besu.ethereum.bonsai.storage.BonsaiPreImageProxy;
 import org.hyperledger.besu.ethereum.bonsai.storage.BonsaiWorldStateKeyValueStorage;
@@ -167,6 +168,11 @@ public Optional<BonsaiWorldState> getHeadWorldState(
       return Optional.empty();
     }
 
+    @Override
+    public Optional<CachedBonsaiWorldView> getStorageByRootHash(final Hash rootHash) {
+      return Optional.empty();
+    }
+
     @Override
     public long getMaxLayersToLoad() {
       return 0;

diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle
@@ -69,7 +69,7 @@ Calculated : ${currentHash}
 tasks.register('checkAPIChanges', FileStateChecker) {
   description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
   files = sourceSets.main.allJava.files
-  knownHash = 'yJgCLn/XmaOwyIlpSw/6gbsM5eNNQQs6hmpTMvkezqk='
+  knownHash = 'ON5/4jw14IPAL/Civ3ld6tvwrLsGS9eI38w5C0xRzdY='
 }
 check.dependsOn('checkAPIChanges')