Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimizations for eth_getProof #27308

Closed
prestwich opened this issue May 19, 2023 · 0 comments · Fixed by #27310
Closed

Optimizations for eth_getProof #27308

prestwich opened this issue May 19, 2023 · 0 comments · Fixed by #27310

Comments

@prestwich
Copy link
Contributor

prestwich commented May 19, 2023

Encountered while writing #27306

Rationale

eth_getProof is significantly more resource-intensive than it should be

  • eth_getProof implementations performs N+1 deep copies, when only 1 is necessary
  • eth_getProof implementations performs these expensive deep copies before performing cheap hex deserialization
  • GetProof calls state.StorageTrie to see if the account has state
    • This performs a deep copy of the state object
  • Then, for each user-provided key
    • It deserializes the hex string into bytes, failing on error
    • Then calls GetStorageProof
    • state.GetStorageProof immediately calls state.StorageTrie again
      • This performs another deep copy of the same state object,
      • This deep copy is performed for each user-provided hex key until an error is encountered
  • As a result, the RPC expends significant resources on invalid requests, which could be cheaply shortcut

So a correct request will result in N+1 deep copies of the state object (1 before the loop, and N within the loop). It will perform deep copies even if the input is malformatted (invalid hex), repeatedly until it encounters the invalid input.

Implementation

Do you have ideas regarding the implementation of this feature?

Suggestions:

  1. Greedily validate input to prevent deep copying when input is invalid
    • Perform all hex deserialization first at the start of the function, rather than within the for loop [as currently written].
    • shortcut if any hex deser fails
    • this prevents resource expenditure on bad caller input
  2. Do not call state.GetStorageProof within the loop.
    • instead, use the storageTrie reference from outside the loop to generate the storage proof
    • call storageTrie.Prove and perform validation on the result within the loop (inlining the checks in state.GetStorageProof)
    • this cuts out N deep copies of the state object

Are you willing to implement this feature?

Yes

holiman added a commit that referenced this issue May 31, 2023
…mplementation (#27310)

Deserialize hex keys early to shortcut on invalid input, and re-use the account storageTrie for each proof for each proof in the account, preventing repeated deep-copying of the trie.

Closes #27308

 --------

Co-authored-by: Martin Holst Swende <[email protected]>
Co-authored-by: Marius van der Wijden <[email protected]>
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this issue Nov 10, 2023
…mplementation (ethereum#27310)

Deserialize hex keys early to shortcut on invalid input, and re-use the account storageTrie for each proof for each proof in the account, preventing repeated deep-copying of the trie.

Closes ethereum#27308

 --------

Co-authored-by: Martin Holst Swende <[email protected]>
Co-authored-by: Marius van der Wijden <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant