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

snapshotter/tests: verify snapdb post-state against trie #20812

Merged
merged 3 commits into from
Mar 31, 2020

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Mar 25, 2020

This PR implements a snapshot vs trie reconciliation after running blockchain tests, to ensure that the state-according-to-snapshot is consistent with the state-according-to-trie.

}

// SlimToFull converts data on the 'slim RLP' format into the full RLP-format
func SlimToFull(data []byte) []byte {
Copy link
Member

Choose a reason for hiding this comment

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

Please return an error if rlp.Decode fails. We'll need it when parsing data from the snap protocol.

Copy link
Member

Choose a reason for hiding this comment

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

I think I'd move this and conversionAccount into conversion.go until it's clear where it goes. Don't want to litter the package. I'm unsure this is a method we want to keep long term, seems weird to do three ops in on (decode, convert, recode).

"github.com/ethereum/go-ethereum/trie"
)

type leaf struct {
Copy link
Member

Choose a reason for hiding this comment

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

This is not a leaf. Its a key/value entry in the trie.


// StdGenerate is a very basic hexary trie builder which uses the same Trie
// as the rest of geth, with no enhancements or optimizations
func StdGenerate(in chan (leaf), out chan (common.Hash)) {
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, not in this PR. I'm changing it.
It was public because we later might want to expose it to the geth cli command, so we can do snaphash.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

SGTM

@holiman holiman merged commit 76eed9e into ethereum:master Mar 31, 2020
@karalabe karalabe added this to the 1.9.13 milestone Mar 31, 2020
enriquefynn pushed a commit to enriquefynn/go-ethereum that referenced this pull request Mar 10, 2021
)

* core/state/snapshot: basic trie-to-hash implementation

* tests: validate snapshot after test

* core/state/snapshot: fix review concerns
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