Skip to content

Commit

Permalink
core, eth/protocols/snap, trie: fix cause for snap-sync corruption, i…
Browse files Browse the repository at this point in the history 
…mplement gentrie (ethereum#29313)

This pull request defines a gentrie for snap sync purpose.

The stackTrie is used to generate the merkle tree nodes upon receiving a state batch. Several additional options have been added into stackTrie to handle incomplete states (either missing states before or after).

In this pull request, these options have been relocated from stackTrie to genTrie, which serves as a wrapper for stackTrie specifically for snap sync purposes.

Further, the logic for managing incomplete state has been enhanced in this change. Originally, there are two cases handled:

-    boundary node filtering
-    internal (covered by extension node) node clearing

This changes adds one more:

- Clearing leftover nodes on the boundaries.

This feature is necessary if there are leftover trie nodes in database, otherwise node inconsistency may break the state healing.
  • Loading branch information
@rjl493456442 @RogerLamTd
rjl493456442 authored and RogerLamTd committed Dec 11, 2024
1 parent 5d20d55 commit 0e497d3
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 191 deletions.
50 changes: 8 additions & 42 deletions eth/protocols/snap/gentrie.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ type genTrie interface {
// update inserts the state item into generator trie.
update(key, value []byte) error

// delete removes the state item from the generator trie.
delete(key []byte) error

// commit flushes the right boundary nodes if complete flag is true. This
// function must be called before flushing the associated database batch.
commit(complete bool) common.Hash
Expand Down Expand Up @@ -116,7 +113,7 @@ func (t *pathTrie) onTrieNode(path []byte, hash common.Hash, blob []byte) {
// removed because it's a sibling of the nodes we want to commit, not
// the parent or ancestor.
for i := 0; i < len(path); i++ {
t.deleteNode(path[:i], false)
t.delete(path[:i], false)
}
}
return
Expand All @@ -135,11 +132,11 @@ func (t *pathTrie) onTrieNode(path []byte, hash common.Hash, blob []byte) {
//
// The extension node is detected if its path is the prefix of last committed
// one and path gap is larger than one. If the path gap is only one byte,
// the current node could either be a full node, or an extension with single
// the current node could either be a full node, or a extension with single
// byte key. In either case, no gaps will be left in the path.
if t.last != nil && bytes.HasPrefix(t.last, path) && len(t.last)-len(path) > 1 {
for i := len(path) + 1; i < len(t.last); i++ {
t.deleteNode(t.last[:i], true)
t.delete(t.last[:i], true)
}
}
t.write(path, blob)
Expand Down Expand Up @@ -167,7 +164,7 @@ func (t *pathTrie) deleteAccountNode(path []byte, inner bool) {
} else {
accountOuterLookupGauge.Inc(1)
}
if !rawdb.HasAccountTrieNode(t.db, path) {
if !rawdb.ExistsAccountTrieNode(t.db, path) {
return
}
if inner {
Expand All @@ -184,7 +181,7 @@ func (t *pathTrie) deleteStorageNode(path []byte, inner bool) {
} else {
storageOuterLookupGauge.Inc(1)
}
if !rawdb.HasStorageTrieNode(t.db, t.owner, path) {
if !rawdb.ExistsStorageTrieNode(t.db, t.owner, path) {
return
}
if inner {
Expand All @@ -195,8 +192,8 @@ func (t *pathTrie) deleteStorageNode(path []byte, inner bool) {
rawdb.DeleteStorageTrieNode(t.batch, t.owner, path)
}

// deleteNode commits the node deletion to provided database batch in path mode.
func (t *pathTrie) deleteNode(path []byte, inner bool) {
// delete commits the node deletion to provided database batch in path mode.
func (t *pathTrie) delete(path []byte, inner bool) {
if t.owner == (common.Hash{}) {
t.deleteAccountNode(path, inner)
} else {
Expand All @@ -210,34 +207,6 @@ func (t *pathTrie) update(key, value []byte) error {
return t.tr.Update(key, value)
}

// delete implements genTrie interface, deleting the item from the stack trie.
func (t *pathTrie) delete(key []byte) error {
// Commit the trie since the right boundary is incomplete because
// of the deleted item. This will implicitly discard the last inserted
// item and clean some ancestor trie nodes of the last committed
// item in the database.
t.commit(false)

// Reset the trie and all the internal trackers
t.first = nil
t.last = nil
t.tr.Reset()

// Explicitly mark the left boundary as incomplete, as the left-side
// item of the next one has been deleted. Be aware that the next item
// to be inserted will be ignored from committing as well as it's on
// the left boundary.
t.skipLeftBoundary = true

// Explicitly delete the potential leftover nodes on the specific
// path from the database.
tkey := t.tr.TrieKey(key)
for i := 0; i <= len(tkey); i++ {
t.deleteNode(tkey[:i], false)
}
return nil
}

// commit implements genTrie interface, flushing the right boundary if it's
// considered as complete. Otherwise, the nodes on the right boundary are
// discarded and cleaned up.
Expand Down Expand Up @@ -286,7 +255,7 @@ func (t *pathTrie) commit(complete bool) common.Hash {
// with no issues as they are actually complete. Also, from a database
// perspective, first deleting and then rewriting is a valid data update.
for i := 0; i < len(t.last); i++ {
t.deleteNode(t.last[:i], false)
t.delete(t.last[:i], false)
}
return common.Hash{} // the hash is meaningless for incomplete commit
}
Expand All @@ -309,9 +278,6 @@ func (t *hashTrie) update(key, value []byte) error {
return t.tr.Update(key, value)
}

// delete implements genTrie interface, ignoring the state item for deleting.
func (t *hashTrie) delete(key []byte) error { return nil }

// commit implements genTrie interface, committing the nodes on right boundary.
func (t *hashTrie) commit(complete bool) common.Hash {
if !complete {
Expand Down
142 changes: 0 additions & 142 deletions eth/protocols/snap/gentrie_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,145 +551,3 @@ func TestTinyPartialTree(t *testing.T) {
}
}
}

func TestTrieDelete(t *testing.T) {
var entries []*kv
for i := 0; i < 1024; i++ {
entries = append(entries, &kv{
k: testrand.Bytes(32),
v: testrand.Bytes(32),
})
}
slices.SortFunc(entries, (*kv).cmp)

nodes := make(map[string]common.Hash)
tr := trie.NewStackTrie(func(path []byte, hash common.Hash, blob []byte) {
nodes[string(path)] = hash
})
for i := 0; i < len(entries); i++ {
tr.Update(entries[i].k, entries[i].v)
}
tr.Hash()

check := func(index []int) {
var (
db = rawdb.NewMemoryDatabase()
batch = db.NewBatch()
marks = map[int]struct{}{}
neighbors = map[int]struct{}{}
)
for _, n := range index {
marks[n] = struct{}{}
}
for _, n := range index {
if n != 0 {
if _, ok := marks[n-1]; !ok {
neighbors[n-1] = struct{}{}
}
}
if n != len(entries)-1 {
if _, ok := neighbors[n+1]; !ok {
neighbors[n+1] = struct{}{}
}
}
}
// Write the junk nodes as the dangling
var injects []string
for _, n := range index {
nibbles := byteToHex(entries[n].k)
for i := 0; i <= len(nibbles); i++ {
injects = append(injects, string(nibbles[:i]))
}
}
for _, path := range injects {
rawdb.WriteAccountTrieNode(db, []byte(path), testrand.Bytes(32))
}
tr := newPathTrie(common.Hash{}, false, db, batch)
for i := 0; i < len(entries); i++ {
if _, ok := marks[i]; ok {
tr.delete(entries[i].k)
} else {
tr.update(entries[i].k, entries[i].v)
}
}
tr.commit(true)

r := newBatchReplay()
batch.Replay(r)
batch.Write()

for _, path := range injects {
if rawdb.HasAccountTrieNode(db, []byte(path)) {
t.Fatalf("Unexpected leftover node %v", []byte(path))
}
}

// ensure all the written nodes match with the complete tree
set := make(map[string]common.Hash)
for path, hash := range r.modifies() {
if hash == (common.Hash{}) {
continue
}
n, ok := nodes[path]
if !ok {
t.Fatalf("Unexpected trie node: %v", []byte(path))
}
if n != hash {
t.Fatalf("Unexpected trie node content: %v, want: %x, got: %x", []byte(path), n, hash)
}
set[path] = hash
}

// ensure all the missing nodes either on the deleted path, or
// on the neighbor paths.
isMissing := func(path []byte) bool {
for n := range marks {
key := byteToHex(entries[n].k)
if bytes.HasPrefix(key, path) {
return true
}
}
for n := range neighbors {
key := byteToHex(entries[n].k)
if bytes.HasPrefix(key, path) {
return true
}
}
return false
}
for path := range nodes {
if _, ok := set[path]; ok {
continue
}
if !isMissing([]byte(path)) {
t.Fatalf("Missing node %v", []byte(path))
}
}
}
var cases = []struct {
index []int
}{
// delete the first
{[]int{0}},

// delete the last
{[]int{len(entries) - 1}},

// delete the first two
{[]int{0, 1}},

// delete the last two
{[]int{len(entries) - 2, len(entries) - 1}},

{[]int{
0, 2, 4, 6,
len(entries) - 1,
len(entries) - 3,
len(entries) - 5,
len(entries) - 7,
}},
}
for _, c := range cases {
check(c.index)
}
}
7 changes: 0 additions & 7 deletions eth/protocols/snap/sync.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2427,13 +2427,6 @@ func (s *Syncer) forwardAccountTask(task *accountTask) {
panic(err) // Really shouldn't ever happen
}
task.genTrie.update(hash[:], full)
} else {
// If the storage task is incomplete, explicitly delete the corresponding
// account item from the account trie to ensure that all nodes along the
// path to the incomplete storage trie are cleaned up.
if err := task.genTrie.delete(hash[:]); err != nil {
panic(err) // Really shouldn't ever happen
}
}
}
// Flush anything written just now and update the stats
Expand Down

0 comments on commit 0e497d3

Please sign in to comment.