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

Access StorageData for eth_getBalance #43

Closed

Conversation

tgmichel
Copy link
Contributor

rel #7 eth_getBalance

  • Build StorageKey for EVM::Accounts.
  • Use client.storage to access EVM::Accounts for a given BlockId.
  • Implement account_decode for Runtime API to Decode StorageData back to pallet-evm::Account type.

@tgmichel
Copy link
Contributor Author

Still TODO the support for all BlockNumber variants, that task probably deserves a unified approach for the whole module and a big refactor PR for it.

In this PR I just adapted the previously supported Latest and Num to locate the block hash using the HeaderBackend.

rpc/src/lib.rs Outdated
return Ok(
self.client
.runtime_api()
.account_decode(&BlockId::Hash(header.hash()), data.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are reading the raw account data, what's the reason still calling into runtime?

Copy link
Contributor Author

@tgmichel tgmichel Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not adding the pallet-evm as a dependency for accessing the EvmAccount (as it's a dependency for the Runtime anyways).

If we are OK adding that dep, then it would make no sense of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also as a side note, Decode::decode cannot infer the data type, in this case pallet-evm::Account needs to be declared as the result type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The round trip call to runtime API is the main consumption of performance. If we still call in runtime I'm not sure how this PR brings improvements, TBH.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. As in the RPC module we already have the frontier-rpc-primitives dep, we could do a public re-export of the pallet-evm::Account from there, and avoid calling the Runtime doing that. Let me update the PR with that.

@tgmichel tgmichel requested a review from sorpaas June 18, 2020 15:39
@tgmichel
Copy link
Contributor Author

As talked with @sorpaas, we probably are good just calling the Runtime Api impls passing the past BlockId as an argument. It is less efficient but way more easy to implement and it just fulfills our requirements.

Closing.

@tgmichel tgmichel closed this Jun 18, 2020
@tgmichel tgmichel deleted the tgmichel-balance-b branch October 7, 2020 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants