Forking needs to be blockNumber aware for its gets (and deletes)

[davidmurdoch]

Basically this TODO needs to go bye-bye:

https://github.com/trufflesuite/ganache-core/blob/9b5a54b8e47a8ef1a94e7f102520cb45f22c617f/lib/utils/forkedstoragetrie.js#L41-L43

Also need to make sure deletion (.del) is blockNumber aware, as well as getCode and getAccount (they might be already).

[mikeseese]

@davidmurdoch

1. The TODO is a duplicate of eth_getStorageAt with block number parameter doesn't return correct result when using fork feature #573 and handled by fix(forking): leverage the requested block number when getting forked storage #613
2. getCode is already working, deduced by these passing tests
3. I'm not sure what you mean by getAccount, do you mean getAccounts? there's no getAccount (or similar) for either Web3 or RPC. If you do mean getAccounts, it's not block dependent. Outside of that, you may be referring to things like ForkedBlockchain.getLookupAccount() which I believe is tested through these tests
4. I believe .del has been preliminary handled in fix(forking): invalidate _deleted in ForkedStorageTrie if a subsequent put happens #612 and prior fixes

I propose we close this issue as obsolete and duplicate

[davidmurdoch]

1. eth_getStorageAt with block number parameter doesn't return correct result when using fork feature #573 is a duplicate of this issue :-p
2. ✔️
3. Yes, I meant ForkedBlockchain.prototype.getAccount. There might be a separate bug in this fn, I haven't checked if there are tests for this scenario yet:

• contract account A exists at block 9 on the original chain. it has a non-zero balance
• we fork at block 10
• we selfdestruct() the contract on our fork at block 11, thus transferring its value to another address.
• getAccount(account A, block 12) may return the account from the original chain. A call to eth_getBalance may return the non-zero account value.

No immediate need to look into this, I just wanted to document some thoughts.

4. Another separate thought, I wonder if we can handle evm_snapshot + .del + evm_revert. 👀

[mikeseese]

👍 I'll open new issues for both 3 and 4 and close this one after that

[mikeseese]

Closing in favor of #623 and #624