eth_getStorageAt with block number parameter doesn't return correct result when using fork feature

[nebojsa94]

JSON RPC method eth_getStorageAt always returns the latest state when using fork feature regardless of the third parameter

Expected Behavior

When changing the storage value in contract c at key k from v1 to v2 in block b, eth_getStorageAt(c, k, b-1) should return v1.

Current Behavior

eth_getStorageAt(c, k, b-1) returns v2 for any b.
For storage values that didn't change after fork, eth_getStorageAt works as expected

Steps to Reproduce (for bugs)

1. ganache-cli -f mainnet_node --networkId 1
2. truffle migrate
3. eth_getStorageAt("[migration_contract]", "0x1", "0x0")

Your Environment

• Version used: Ganache CLI v6.9.1 (ganache-core: 2.10.2)
• Operating System and version: macOS Catalina 10.15.4

[nebojsa94]

Hey @davidmurdoch, I noticed that there is a TODO that is linked to this issue.
https://github.com/trufflesuite/ganache-core/blob/18f11f75fc6bb048fcc44c9b40b1556a392f31d7/lib/utils/forkedstoragetrie.js#L43

Would you be open to accepting a PR that addresses this problem?

[BogdanHabic]

Hey @seesemichaelj is there anything we can do to help you with resolving this issue? We're open to submitting the PR ourselves if you don't have the capacity to tackle this now 😄

[mikeseese]

Hey @BogdanHabic! I haven't yet scoped out the issue here, but my next priority is to work various forking issues (including this one, #526, #533, #571, and #579). I would definitely appreciate any help you want to provide if you have time to work it now! I can start work on other issues before getting to this one if you want to throw together a PR.

Even if you want to describe a solution without doing the implementation, that would also be helpful

[mikeseese]

@BogdanHabic I'm also happy to collaborate somehow. I know our timezones are not very forgiving for anything realtime, but feel free to use this issue as a means to bounce ideas off me and @davidmurdoch

[BogdanHabic]

@seesemichaelj great to hear that you're tackling forking issues! @nebojsa94 can help out on this issue from our side. He'll submit a description of the solution first and if we get a 👍 from you, we'll submit a PR with the implementation as well.

[mikeseese]

Sounds great!

[nebojsa94]

Thank you for helping us resolve this issue!

There are 2 problems that we need to address here:

1. Even if the key exists in the forked state, we should let the main provider handle the request if the provided blockNumber is lower than forkBlockNumber. This check can be performed here: https://github.com/trufflesuite/ganache-core/blob/a8a4d8f04a357380963c62110fc7659b4f03c4ec/lib/forking/forked_storage_trie.js#L42

2. When blockNumber is greater or equal than forkBlockNumber, we also need to ensure that state trie is at the right root, this can be set before https://github.com/trufflesuite/ganache-core/blob/a8a4d8f04a357380963c62110fc7659b4f03c4ec/lib/forking/forked_blockchain.js#L402

@seesemichaelj @davidmurdoch Do you think this approach is valid. I can submit a PR once you approve.

P.S. I think this will also fix some issues with debug_traceTransaction as this method requires historical state.

[mikeseese]

Hey @nebojsa94, while I was assessing the validity of your solution, I was trying to find a more elegant solution by hacking away. However, I ended up on your proposed solution anyway and opened a PR since it was done. I didn't intend to steal your thunder and hope you didn't spend any time with implementation! Nevertheless, I really appreciate the proposal you made as it saved me discovery time!

[mikeseese]

This was just officially released in [email protected] / [email protected]