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

cmd, core, eth, trie: track deleted nodes #576

Conversation

Francesco4203
Copy link
Contributor

@Francesco4203 Francesco4203 commented Sep 17, 2024

Reference: ethereum/go-ethereum#25757

Important changes:

  • A storage trie is now identified by StateRoot (root of state/account trie), Owner, Root. The state/account trie is identified with StateRoot = Root. Those params are now needed for init a trie.

  • Separate list of dirty nodes to updated nodes and deleted node. Also separate the updating/deleting operations.

  • Track previous value of each node (before committing).

  • Apply delete tracking logics to committer.

  • Add trie_reader.go, interface for reaching database layer(s) from trie. Support backward compatibility: can use hash_reader for old hash based scheme, and can use diffLayer for path based scheme in the future PRs.

  • Add metrics for tracking trie commits.

@Francesco4203 Francesco4203 force-pushed the track-deleted-nodes branch 2 times, most recently from ea7255b to 31945a9 Compare September 17, 2024 10:21
// node's original value. The rlp-encoded blob is preferred to be loaded from
// database because it's easy to decode node while complex to encode node to blob.
func (t *Trie) resolveAndTrack(n hashNode, prefix []byte) (node, error) {
blob, err := t.reader.nodeBlob(prefix, common.BytesToHash(n))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to track the encoded node here, so get the blob first for tracer and then return the decoded one.

// getPrev returns the cached original value of the specified node.
func (t *tracer) getPrev(key []byte) []byte {
func (t *tracer) getPrev(path []byte) []byte {
// Don't panic on uninitialized tracer, it's possible in testing.
Copy link
Collaborator

Choose a reason for hiding this comment

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

// Tracer isn't used right now, remove this check later. -> Update it for correcting the status.

trie/utils_test.go Show resolved Hide resolved
case opHash:
tr.Hash()
case opCommit:
hash, nodes, err := tr.Commit(false)
root, nodes, err := tr.Commit(true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why they change to collect Dirty Leaf here too 🤔

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 think since they've just enabled the tracer here, so they just try to commit everything for full coverage.

trie/nodeset.go Outdated Show resolved Hide resolved
@Francesco4203 Francesco4203 force-pushed the track-deleted-nodes branch 2 times, most recently from 89eb9bb to a40e2d1 Compare September 25, 2024 10:19
Copy link
Collaborator

@huyngopt1994 huyngopt1994 left a comment

Choose a reason for hiding this comment

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

then LGTM.

@huyngopt1994 huyngopt1994 merged commit f61a05d into axieinfinity:path-base-implementing Sep 25, 2024
1 check passed
Francesco4203 added a commit that referenced this pull request Oct 17, 2024
* trie: track deleted nodes

* core: track deleted nodes
huyngopt1994 pushed a commit that referenced this pull request Oct 25, 2024
* trie: track deleted nodes

* core: track deleted nodes
huyngopt1994 pushed a commit that referenced this pull request Nov 21, 2024
* trie: track deleted nodes

* core: track deleted nodes
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