feat: get state and storage tries by hash#601
Conversation
| let new_state_root = state | ||
| .database() | ||
| .apply_account_updates(parent_header.number, &account_updates)? | ||
| .apply_account_updates(block.header.parent_hash, &account_updates)? |
There was a problem hiding this comment.
What does default do for a new_state_root?
Not saying this is wrong per-se, but this makes me raise an eyebrow. I think it would be better do unwrap_or_else an explicitly do the default case, since If I'm looking this code, now I also have to double-check what the default trait does for this type, please feel correct me If you disagree.
That being said, this is not part of this PR, so maybe we can make an issue to handle it another time.
| .get_storage_at( | ||
| self.block_number, | ||
| .get_storage_at_hash( | ||
| self.block_hash, |
There was a problem hiding this comment.
Why the change from Block Number to Block Hash?
I've read the PR details but I still do not understand it.
There was a problem hiding this comment.
There might be more than one block with the same block number but different hash. When you get block by number, you are getting the canonical block. The hash is the unique identifier. This change happens because you want to be able to calculate the state transition on non-canonical blocks too.
**Changes** - Change the access to tries in storage engines from number to hash - In `storage.rs`, change methods from numbers to hashes if needed, and add wrappers when not. - Make accesses by hash in `evm_state` construction, the trie db wrapper, blockchain, etc. Regarding the note in the issue: > Endpoints that should accept non canonical blocks: > - newPayload > - call - `newPayload` uses `add_block`, which now uses hash to get the evm state. This will be useful for reorgs too. - call gets called by number or string ids, but not by hash, so the number wrapper is supported. Closes #443
Changes
storage.rs, change methods from numbers to hashes if needed, and add wrappers when not.evm_stateconstruction, the trie db wrapper, blockchain, etc.Regarding the note in the issue:
newPayloadusesadd_block, which now uses hash to get the evm state. This will be useful for reorgs too.Closes #443