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.

Sign up for GitHub

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

Jump to bottom

implement get proof rpc endpoint #13291

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

implement get proof rpc endpoint #13291

wants to merge 6 commits into from
+199 −107

Conversation

shotasilagadze
Copy link
Contributor

@shotasilagadze shotasilagadze commented Dec 31, 2024 •
edited
Loading

@shotasilagadze shotasilagadze marked this pull request as draft December 31, 2024 18:44
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch 4 times, most recently from a19d2f9 to 91ff2b0 Compare January 1, 2025 11:46
@shotasilagadze shotasilagadze marked this pull request as ready for review January 1, 2025 11:46
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch 2 times, most recently from fee4a79 to c76029b Compare January 1, 2025 12:04
@shotasilagadze shotasilagadze linked an issue Jan 1, 2025 that may be closed by this pull request
Implement eth_getProof for "latest" Tag in Erigon 3 #13201
Open
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch from c76029b to c3f515d Compare January 1, 2025 12:23
@Giulio2002
Copy link
Contributor

Add unit tests + remove printlns. @awskii will give a more meaningful review tho. this is not my domain

@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch 2 times, most recently from 0433019 to 3b4808f Compare January 3, 2025 06:45
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch 3 times, most recently from a2c6086 to 970d2c1 Compare January 6, 2025 07:26
awskii
awskii reviewed Jan 6, 2025
View reviewed changes
turbo/jsonrpc/eth_call.go Outdated Show resolved Hide resolved
awskii
awskii reviewed Jan 6, 2025
View reviewed changes
turbo/jsonrpc/eth_call.go Outdated Show resolved Hide resolved
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch 3 times, most recently from 9337a4b to b3bc88c Compare January 7, 2025 09:13
antonis19
antonis19 reviewed Jan 7, 2025
View reviewed changes
erigon-lib/trie/proof.go
//
// If the trie does not contain a value for key, the returned proof contains all
// nodes of the longest existing prefix of the key (at least the root node), ending
// with the node that proves the absence of the key.
func (t *Trie) Prove(key []byte, fromLevel int, storage bool) ([][]byte, error) {
func (t *Trie) Prove(key []byte, fromLevel int, storage bool) ([][]byte, []byte, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Not necessary to rely on Prove to get the storage value as it can easily be retrieved via stateReader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm not sure so we can directly access value here, why not returning it. We wouldn't need stateReader and would have a ready value 🤔

Isn't this one easier?

Copy link
Member

Choose a reason for hiding this comment

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

As we discussed, I's suggest not returning the value bytes since we don't want the function to do more than it should. If you need the value either use a StateReader, or write another function to traverse the trie and get the value. I did this on my branch:

func (t *Trie) GetAccountNode(key []byte) (value *AccountNode, gotValue bool) {
	hex := keybytesToHex(key)
	return t.getAccount(t.RootNode, hex, 0)
}

antonis19
antonis19 reviewed Jan 7, 2025
View reviewed changes
turbo/jsonrpc/eth_call.go Outdated Show resolved Hide resolved
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch 12 times, most recently from c26e569 to 650dc19 Compare January 7, 2025 16:09
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch from 650dc19 to 89a473b Compare January 7, 2025 16:19
@shotasilagadze shotasilagadze requested a review from awskii January 7, 2025 16:20
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch from a9663ca to 786cbc8 Compare January 9, 2025 08:18
awskii
awskii reviewed Jan 9, 2025
View reviewed changes
turbo/jsonrpc/eth_call.go
// shouldn't happen, but check anyway
return nil, fmt.Errorf("block number is in the future latest=%d requested=%d", latestBlock, blockNr)
}
return api.getProof(ctx, address, storageKeys, rpc.BlockNumberOrHashWithNumber(rpc.BlockNumber(latestBlock)), api.db, api.logger)
Copy link
Member

Choose a reason for hiding this comment

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

why not to pass already opened RoTx?

Copy link
Contributor Author

@shotasilagadze shotasilagadze Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can I use only one and the same of RoTx everywhere ? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@awskii

awskii
awskii reviewed Jan 9, 2025
View reviewed changes
turbo/jsonrpc/eth_call.go Outdated Show resolved Hide resolved
awskii
awskii requested changes Jan 9, 2025
View reviewed changes
turbo/jsonrpc/eth_call.go Outdated Show resolved Hide resolved
turbo/jsonrpc/eth_call.go Outdated Show resolved Hide resolved
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch from 786cbc8 to ee64763 Compare January 10, 2025 09:50
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch from ee64763 to 064242e Compare January 10, 2025 09:58
@shotasilagadze shotasilagadze requested a review from awskii January 11, 2025 12:12
@zzzckck zzzckck mentioned this pull request Jan 14, 2025
Closed
@shotasilagadze shotasilagadze force-pushed the implement_eth_get_proof branch from 06f3e13 to d044cdb Compare January 14, 2025 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antonis19 antonis19 antonis19 left review comments

@Giulio2002 Giulio2002 Awaiting requested review from Giulio2002

@awskii awskii Awaiting requested review from awskii

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement eth_getProof for "latest" Tag in Erigon 3
5 participants
@shotasilagadze @Giulio2002 @awskii @antonis19 @shotasilagadzetaal