core/state, trie: add period to node encoding#560
core/state, trie: add period to node encoding#560weiihann wants to merge 12 commits intogballet:archival-state-expiryfrom
Conversation
# Conflicts: # trie/verkle.go
triedb/pathdb/generate.go
Outdated
| tr.Update(key, result.vals[i]) | ||
| } | ||
| _, nodes := tr.Commit(false) | ||
| _, nodes := tr.Commit(false, 0) |
There was a problem hiding this comment.
this is going to be a problem, because it means that we can't generate the snapshot after period 0 or periods will be wrong. I guess it's ok for the test, but it means that we can't merge into master
There was a problem hiding this comment.
hmm from how I see it, it might not be a problem. The node set is used purely as a temporary in-memory hash->blob lookup table for the resolver. The nodes are never written to the disk, so the period value is irrelevant.
trie/trie.go
Outdated
| nodes := trienode.NewNodeSet(t.owner) | ||
| for _, path := range paths { | ||
| nodes.AddNode(path, trienode.NewDeletedWithPrev(t.prevalueTracer.Get(path))) | ||
| nodes.AddNode(path, trienode.NewDeletedWithPrev(t.prevalueTracer.Get(path), period)) |
There was a problem hiding this comment.
not that this is incorrect, but it poses a tough question: if a piece of the state is accessed and then reorged, and not touched in the new fork, should it be resurrected?
no, because it is not part of the official history, and so a fullsync would get the block wrong. But this might be tricky, because we need to save the period history of each nodes, or at least the previous one.
This is only important if this becomes in-protocol of course. Otherwise, I can think of a few tricks so that it doesn't matter.
There was a problem hiding this comment.
For out-of-protocol, I think falsely resurrecting nodes due to reorgs is acceptable.
Yes we have to deal with this if it's in-protocol. But we have more problems to deal with if it's a solution where we have to add metadata to every single trie node.
So I'd keep this in mind and leave it for now.
weiihann
force-pushed
the
archive-expiry/trie-node
branch
from
0e9bced to
668fc8e
Compare
2 participants
No description provided.