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

feat(rpc): get_account #10369

Merged
merged 3 commits into from
Aug 19, 2024
Merged

feat(rpc): get_account #10369

merged 3 commits into from
Aug 19, 2024

Conversation

greged93
Copy link
Contributor

@greged93 greged93 commented Aug 17, 2024

This PR adds the rpc endpoint get_account and solves #9625.

Makes use of the hashed_storage_root method from the StateRootProvider and passes an empty HashedStorage in order to only get the storage root of the current state for the provided account.

@greged93
Copy link
Contributor Author

greged93 commented Aug 17, 2024

I couldn't find a place to add tests to this, I can add in another PR if this is something we would want.

Copy link
Member

@rkrasiuk rkrasiuk left a comment

Choose a reason for hiding this comment

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

account_nonce, account_balance and account_code perform a call to basic_account underneath. moreover, account_code performs an additional database lookup to retrieve code by code hash which is already part of account info, so no need to hash it

@rkrasiuk rkrasiuk added C-enhancement New feature or request A-rpc Related to the RPC implementation labels Aug 17, 2024
Copy link
Member

@rkrasiuk rkrasiuk left a comment

Choose a reason for hiding this comment

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

lgtm

.unwrap_or_default();
let balance = account.balance;
let nonce = account.nonce;
let code_hash = account.bytecode_hash.unwrap_or_default();
Copy link
Member

@rkrasiuk rkrasiuk Aug 19, 2024

Choose a reason for hiding this comment

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

actually, if None this will be 0x000 whereas it should be KECCAK_EMPTY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

of course sorry for missing this

Copy link
Member

@rkrasiuk rkrasiuk left a comment

Choose a reason for hiding this comment

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

lgtm

@rkrasiuk rkrasiuk added this pull request to the merge queue Aug 19, 2024
Merged via the queue into paradigmxyz:main with commit 297934f Aug 19, 2024
34 checks passed
Comment on lines +149 to +153
// Provide a default `HashedStorage` value in order to
// get the storage root hash of the current state.
let storage_root = state
.hashed_storage_root(address, Default::default())
.map_err(Self::Error::from_eth_err)?;
Copy link
Member

Choose a reason for hiding this comment

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

isn't this always the same? can it be const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean, which part could be a const?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants