feat: add support for `debug_storageRangeAt` RPC #755

eshaben · 2021-01-26T08:47:43Z

Resolves #682

eshaben · 2021-01-26T08:50:54Z

Need to clean up the code (copy pasta from debug_traceTransaction), clean up/add typings, add annotations for documentation in api, and probably more... but the logic is there, this is close.

eshaben · 2021-02-09T04:00:53Z

Hahaha... what my last comment said but hopefully for real this time

eshaben · 2021-02-11T09:07:59Z

Not the prettiest way of implementing this rpc method but it gets the job done 🤷

Basically we are storing the raw, un-hashed version of storage keys along with their hashed keys so we can return both hashed and raw keys to the user for the debug_storageRangeAt method. Then after we replay the transaction, we use the trie to create a read stream and get all of our storage keys from the transaction. Then we sort the keys before we can filter through the ones we need, using the hashed key to get the raw key from the db, and finally return the desired range of storage data.

Findings: geth stores their hashed keys along with the raw, un-hashed keys. geth also sorts their keys after getting them out of the database (see here: https://gist.github.com/eshaben/ae99036dc15a3784ed3ea50856cd733d -- this is the ordering of keys returned from the trie read stream) 🤷

This is ready for initial review!

If anyone wants to test against geth, I used the following command to start up a geth console: geth --rpc --rpccorsdomain="https://remix.ethereum.org" --rpcapi web3,eth,debug,personal,net --vmdebug --datadir test-chain-dir --dev console
☝️ this command requires you create a directory called test-chain-dir for storing chaindata.

Then I went to remix and deployed my contracts and sent transactions. After sending transactions I went back to geth console and tried to use debug_storageRangeAt method, like so...

debug.storageRangeAt(
"0x29a4daa5e2a33843a5967e7f6014f95862772fa6077afeddeebc9b85e966dbda", // blockHash
 0, // txIndex
"0xe33983B0ab2E5987e6b88E0a860E859061F08C68", // contractAddress
"0x405787fa12a823e0f2b7631cc41b3ba8828b3321ca811111fa75cd3aa3bb5ace", // startKey
 2 // maxResults
);

If you have any questions or want help configuring test environment for this, let me know!

davidmurdoch

Looking good and loving the new tests! Made some comments and suggestions. I'll do another review once you've done the clean up you mentioned.

davidmurdoch · 2021-02-16T23:34:32Z

src/chains/ethereum/ethereum/src/api.ts

+   * @param maxResult
+   * @returns returns a storage object with the keys being keccak-256 hashes of the storage keys,
+   * and the values being the raw, unhashed key and value for that specific storage slot. Also
+   * retuns a next key which is the keccak-256 hash of the next key in storage for continuous downloading.


Suggested change

* retuns a next key which is the keccak-256 hash of the next key in storage for continuous downloading.

* returns a next key which is the keccak-256 hash of the next key in storage for continuous downloading.

davidmurdoch · 2021-02-16T23:37:37Z

src/chains/ethereum/ethereum/src/blockchain.ts

@@ -365,6 +370,11 @@ export default class Blockchain extends Emittery.Typed<
        );
      });

+      // save storage keys to the database one at a time


Suggested change

// save storage keys to the database one at a time

// save storage keys to the database

Because this loop happens within the batch callback, teeeecchhhhnically they'll all get saved at the same time :-)

davidmurdoch · 2021-02-16T23:38:04Z

src/chains/ethereum/ethereum/src/blockchain.ts

+   * Strategy:
+   *
+   *  1. Get transaction hash
+   *  2. Replay transaction
+   *  3. Create read stream to get the storage keys from the transaction
+   *  4. Sort storage keys
+   *  5. Filter, assemble, and send storage data for the given range back


davidmurdoch · 2021-02-16T23:50:27Z

src/chains/ethereum/ethereum/src/blockchain.ts

+    try {
+      await this.traceTransaction(transactionHash, {
+        disableMemory: true,
+        disableStack: false,
+        disableStorage: false
+      });
+    } catch (e) {
+      throw new Error(e);
+    }


traceTransaction shouldn't (:sweat_smile:) have any side effects, so this bit can probably be removed.

davidmurdoch · 2021-02-18T22:32:49Z

src/chains/ethereum/ethereum/src/api.ts

+   * @param blockHash
+   * @param txIndex
+   * @param contractAddress
+   * @param startKey
+   * @param maxResult


Can you add descriptions for each of these parameters?

davidmurdoch · 2021-02-19T20:50:20Z

src/chains/ethereum/ethereum/tests/api/debug/debug_storageRangeAt.test.ts

+        const txReceipt1 = await provider.send("eth_getTransactionReceipt", [
+          tx1
+        ]);
+        const txReceipt2 = await provider.send("eth_getTransactionReceipt", [
+          tx2
+        ]);
+        const txReceipt3 = await provider.send("eth_getTransactionReceipt", [
+          tx3
+        ]);


a slightly neater way:

Suggested change

const txReceipt1 = await provider.send("eth_getTransactionReceipt", [

tx1

]);

const txReceipt2 = await provider.send("eth_getTransactionReceipt", [

tx2

]);

const txReceipt3 = await provider.send("eth_getTransactionReceipt", [

tx3

]);

const [txReceipt1, txReceipt2, txReceipt3] = await Promise.all([

provider.send("eth_getTransactionReceipt", [tx1]),

provider.send("eth_getTransactionReceipt", [tx2]),

provider.send("eth_getTransactionReceipt", [tx3])

]);

davidmurdoch · 2021-02-19T20:56:01Z

src/chains/ethereum/ethereum/src/blockchain.ts

+          });
+      });
+    };
+    await getStorageKeys();


Can you move the let keys: Buffer[] = []; declaration inside the getStorageKeys function and then:

Suggested change

await getStorageKeys();

const keys = await getStorageKeys();

davidmurdoch · 2021-02-19T20:56:09Z

src/chains/ethereum/ethereum/src/blockchain.ts

+      return new Promise((resolve, reject) => {
+        trie
+          .createReadStream()
+          .on("data", async data => {


Suggested change

.on("data", async data => {

.on("data", data => {

davidmurdoch · 2021-02-19T20:56:16Z

src/chains/ethereum/ethereum/src/blockchain.ts

+          .on("data", async data => {
+            keys.push(data.key);
+          })
+          .on("end", async () => {


Suggested change

.on("end", async () => {

.on("end", () => {

davidmurdoch · 2021-02-19T20:59:11Z

src/chains/ethereum/ethereum/src/miner/miner.ts

+      type StepEvent = {
+        gasLeft: BN;
+        memory: Array<number>; // Not officially sure the type. Not a buffer or uint8array
+        stack: Array<BN>;
+        depth: number;
+        opcode: {
+          name: string;
+        };
+        pc: number;
+        address: Buffer;
+      };


Maybe we can move this type from here (and from the traceTransaction) to a common place, maybe @ganache/ethereum-utils can export this type?

…the keys from the trie stream

davidmurdoch

Only one more little quick fix and I think it's good to go! Awesome work @eshaben !

src/chains/ethereum/ethereum/src/miner/miner.ts

Co-authored-by: David Murdoch <[email protected]>

eshaben marked this pull request as ready for review February 11, 2021 08:53

eshaben requested a review from davidmurdoch February 11, 2021 09:11

davidmurdoch changed the base branch from next to develop February 17, 2021 04:06

davidmurdoch suggested changes Feb 19, 2021

View reviewed changes

davidmurdoch changed the title ~~debug_storageRangeAt implementation~~ feat: add support for debug_storageRangeAt RPC Feb 22, 2021

eshaben added 22 commits February 24, 2021 23:46

create debug storage contract for testing

cf1c093

add initial debug_storageRangeAt entry point to api

cce0f48

create initial test

5af3f4f

add debug_traceTransaction logic so we can recompute txs

b6fa4b0

use contract address to set state root

2299c0f

use startKey and maxResult to iterate over storage entries

c1318ac

oops remove left over comment

57c8940

get nextKey and format return data correctly

0c3caef

return result and update test assertions

a118a50

add more tests

10a9b39

add keccak hash key to storage output

61eeed6

remove irrelevant logic

7ab7c86

create an in memory map for storage keys and write it to block

522672d

fill in memory storage map with raw and hashed version of storage keys

14b462e

create storageKeys db

abdff2e

save storage keys to the database one at a time

cf5d306

remove irrelevant comment

49eed1d

create function to get storage keys

d142bbf

fix tests

b7939c5

check if the startKey matches any of the keys from the trie stream

1b8b8d6

try hashing the startKey and seeing if the hashed key matches any of …

d014318

…the keys from the trie stream

handle arbitrary hexadecimal strings as startKeys

9fbd8b9

eshaben added 23 commits February 24, 2021 23:46

update comment for accuracy

166db81

remove unnecessary async keyword

e90dd00

update keys declaration

9b516b6

remove unnecessary try/catch

3e9ae3c

clean up test

ca4124e

create reusable getFromTrie function

2c5e1b3

create internal traceTransaction function

c062285

remove .only

045074f

pass storageKeys to runtimeBlock.finalize

21b9e03

move StepEvent type to utils

760e8ba

use assert.rejects in test

733d314

wrap test in snapshot and revert

2f970bb

use Promise.all in test

ea431f5

update storageKeys key to be a string instead of a buffer

0c71c3b

remove .only on test

3d21b85

update error message for tx index out of range

468c623

create test for transaction index out of range

a6187d7

remove unnecessary async keyword

62ad4f1

update comments

726bfa8

fix typing of keys array

487c80f

throw error instead of console.log it

b5cc898

remove unneccessary for loop

d9966c4

clean up promises

20ab776

eshaben force-pushed the debug_storageRangeAt branch from 9db8596 to 20ab776 Compare February 25, 2021 06:51

eshaben requested a review from davidmurdoch February 25, 2021 06:51

davidmurdoch suggested changes Feb 26, 2021

View reviewed changes

src/chains/ethereum/ethereum/src/miner/miner.ts Outdated Show resolved Hide resolved

update keccak import

c383f1c

Co-authored-by: David Murdoch <[email protected]>

davidmurdoch merged commit be84335 into develop Feb 26, 2021

davidmurdoch deleted the debug_storageRangeAt branch February 26, 2021 02:17

sambacha pushed a commit to contractshark/ganache-core that referenced this pull request Apr 15, 2021

feat: add support for debug_storageRangeAt RPC (trufflesuite#755)

fb874f1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for `debug_storageRangeAt` RPC #755

feat: add support for `debug_storageRangeAt` RPC #755

eshaben commented Jan 26, 2021

eshaben commented Jan 26, 2021

eshaben commented Feb 9, 2021

eshaben commented Feb 11, 2021 •

edited

Loading

davidmurdoch left a comment

davidmurdoch Feb 16, 2021

davidmurdoch Feb 16, 2021

davidmurdoch Feb 16, 2021

davidmurdoch Feb 16, 2021

davidmurdoch Feb 18, 2021

davidmurdoch Feb 19, 2021

davidmurdoch Feb 19, 2021

davidmurdoch Feb 19, 2021

davidmurdoch Feb 19, 2021

davidmurdoch Feb 19, 2021

davidmurdoch left a comment

	* retuns a next key which is the keccak-256 hash of the next key in storage for continuous downloading.
	* returns a next key which is the keccak-256 hash of the next key in storage for continuous downloading.

	// save storage keys to the database one at a time
	// save storage keys to the database

-        const txReceipt1 = await provider.send("eth_getTransactionReceipt", [
-          tx1
-        ]);
-        const txReceipt2 = await provider.send("eth_getTransactionReceipt", [
-          tx2
-        ]);
-        const txReceipt3 = await provider.send("eth_getTransactionReceipt", [
-          tx3
-        ]);
+        const [txReceipt1, txReceipt2, txReceipt3] = await Promise.all([
+          provider.send("eth_getTransactionReceipt", [tx1]),
+          provider.send("eth_getTransactionReceipt", [tx2]),
+          provider.send("eth_getTransactionReceipt", [tx3])
+        ]);

feat: add support for debug_storageRangeAt RPC #755

feat: add support for debug_storageRangeAt RPC #755

Conversation

eshaben commented Jan 26, 2021

eshaben commented Jan 26, 2021

eshaben commented Feb 9, 2021

eshaben commented Feb 11, 2021 • edited Loading

davidmurdoch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidmurdoch left a comment

Choose a reason for hiding this comment

feat: add support for `debug_storageRangeAt` RPC #755

feat: add support for `debug_storageRangeAt` RPC #755

eshaben commented Feb 11, 2021 •

edited

Loading