snapshot: resolve zkStateRoot to mptStateRoot for generation and loading #302
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,278 @@ | ||
| // Copyright 2024 The go-ethereum Authors | ||
| // This file is part of the go-ethereum library. | ||
| // | ||
| // The go-ethereum library is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU Lesser General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
| // | ||
| // The go-ethereum library is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU Lesser General Public License for more details. | ||
| // | ||
| // You should have received a copy of the GNU Lesser General Public License | ||
| // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| // Tests for ZK-MPT compatibility fixes: | ||
| // - generateSnapshot resolves zkStateRoot → mptStateRoot via ReadDiskStateRoot | ||
| // - loadSnapshot accepts a snapshot whose disk root is the MPT translation of | ||
| // the requested zkStateRoot | ||
| // - Rebuild stores the disk layer under base.root (the possibly-translated root) | ||
| // as the map key so that Snapshot() lookups work correctly after translation | ||
|
|
||
| package snapshot | ||
|
|
||
| import ( | ||
| "math/big" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/morph-l2/go-ethereum/common" | ||
| "github.com/morph-l2/go-ethereum/core/rawdb" | ||
| "github.com/morph-l2/go-ethereum/ethdb" | ||
| "github.com/morph-l2/go-ethereum/ethdb/memorydb" | ||
| "github.com/morph-l2/go-ethereum/rlp" | ||
| "github.com/morph-l2/go-ethereum/trie" | ||
| ) | ||
|
|
||
| // buildMPTTrie creates a small MPT account trie in diskdb/triedb and returns its root. | ||
| func buildMPTTrie(t *testing.T, diskdb *memorydb.Database) (common.Hash, *trie.Database) { | ||
| t.Helper() | ||
| triedb := trie.NewDatabase(diskdb) | ||
| accTrie, _ := trie.NewSecure(common.Hash{}, triedb) | ||
| for _, key := range []string{"acc-1", "acc-2", "acc-3"} { | ||
| acc := &Account{ | ||
| Balance: big.NewInt(1), | ||
| Root: emptyRoot.Bytes(), | ||
| KeccakCodeHash: emptyKeccakCode.Bytes(), | ||
| PoseidonCodeHash: emptyPoseidonCode.Bytes(), | ||
| } | ||
| val, _ := rlp.EncodeToBytes(acc) | ||
| accTrie.Update([]byte(key), val) | ||
| } | ||
| mptRoot, _, _ := accTrie.Commit(nil) | ||
| triedb.Commit(mptRoot, false, nil) | ||
| return mptRoot, triedb | ||
|
Comment on lines
+40
to
+56
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Verify trie/RLP APIs used here actually return errors that should be handled.
rg -nP --type go 'func\s+NewSecure\(' -C2
rg -nP --type go 'func\s+\(.*\)\s+Update\([^)]*\)\s+error' -C2
rg -nP --type go 'func\s+\(.*\)\s+Commit\([^)]*\)\s*\([^)]*error' -C2
rg -nP --type go 'func\s+EncodeToBytes\(' -C2Repository: morph-l2/go-ethereum Length of output: 4538 🏁 Script executed: # Find SecureTrie.Update method definition
rg -nP --type go 'func\s+\(\w+\s+\*SecureTrie\)\s+Update' -C3Repository: morph-l2/go-ethereum Length of output: 485 🏁 Script executed: # Find trie.Database.Commit method definition
rg -nP --type go 'func\s+\(\w+\s+\*Database\)\s+Commit' -C3Repository: morph-l2/go-ethereum Length of output: 665 🏁 Script executed: # Read the specific lines of the test file to confirm context
sed -n '40,70p' core/state/snapshot/zk_mpt_compat_test.goRepository: morph-l2/go-ethereum Length of output: 1265 Fix error handling in test fixture builders. The helpers currently discard returned errors from setup operations, which can hide initialization failures and produce misleading test failures. The 🔧 Proposed fix func buildMPTTrie(t *testing.T, diskdb *memorydb.Database) (common.Hash, *trie.Database) {
t.Helper()
triedb := trie.NewDatabase(diskdb)
- accTrie, _ := trie.NewSecure(common.Hash{}, triedb)
+ accTrie, err := trie.NewSecure(common.Hash{}, triedb)
+ if err != nil {
+ t.Fatalf("failed to create secure trie: %v", err)
+ }
for _, key := range []string{"acc-1", "acc-2", "acc-3"} {
acc := &Account{
Balance: big.NewInt(1),
Root: emptyRoot.Bytes(),
KeccakCodeHash: emptyKeccakCode.Bytes(),
PoseidonCodeHash: emptyPoseidonCode.Bytes(),
}
- val, _ := rlp.EncodeToBytes(acc)
+ val, err := rlp.EncodeToBytes(acc)
+ if err != nil {
+ t.Fatalf("failed to encode account: %v", err)
+ }
accTrie.Update([]byte(key), val)
}
- mptRoot, _, _ := accTrie.Commit(nil)
- triedb.Commit(mptRoot, false, nil)
+ mptRoot, _, err := accTrie.Commit(nil)
+ if err != nil {
+ t.Fatalf("failed to commit trie: %v", err)
+ }
+ if err := triedb.Commit(mptRoot, false, nil); err != nil {
+ t.Fatalf("failed to commit trie db: %v", err)
+ }
return mptRoot, triedb
}
-func writeDoneGenerator(db ethdb.KeyValueWriter) {
- blob, _ := rlp.EncodeToBytes(&journalGenerator{Done: true})
+func writeDoneGenerator(t *testing.T, db ethdb.KeyValueWriter) {
+ t.Helper()
+ blob, err := rlp.EncodeToBytes(&journalGenerator{Done: true})
+ if err != nil {
+ t.Fatalf("failed to encode generator journal: %v", err)
+ }
rawdb.WriteSnapshotGenerator(db, blob)
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| // writeDoneGenerator writes a completed (Done=true) snapshot generator record | ||
| // into the given db — required by loadAndParseJournal before loadSnapshot | ||
| // proceeds to the head-root comparison. | ||
| func writeDoneGenerator(db ethdb.KeyValueWriter) { | ||
| blob, _ := rlp.EncodeToBytes(&journalGenerator{Done: true}) | ||
| rawdb.WriteSnapshotGenerator(db, blob) | ||
| } | ||
|
|
||
| // ---- generate.go: generateSnapshot root translation ------------------------- | ||
|
|
||
| // TestGenerateSnapshotTranslatesZkRoot verifies that when a DiskStateRoot | ||
| // mapping (zkRoot → mptRoot) exists, generateSnapshot uses mptRoot for both | ||
| // WriteSnapshotRoot and the trie walk, allowing generation to complete. | ||
| func TestGenerateSnapshotTranslatesZkRoot(t *testing.T) { | ||
| diskdb := memorydb.New() | ||
| mptRoot, triedb := buildMPTTrie(t, diskdb) | ||
|
|
||
| // Simulate ZK-era: block header carries zkRoot, local trie is at mptRoot. | ||
| zkRoot := common.HexToHash("0x1111111111111111111111111111111111111111111111111111111111111111") | ||
| rawdb.WriteDiskStateRoot(diskdb, zkRoot, mptRoot) | ||
|
|
||
| snap := generateSnapshot(diskdb, triedb, 16, zkRoot) | ||
|
|
||
| // diskLayer.root must be mptRoot, not zkRoot. | ||
| if snap.root != mptRoot { | ||
| t.Fatalf("diskLayer.root: got %x, want mptRoot %x", snap.root, mptRoot) | ||
| } | ||
| // SnapshotRoot persisted to DB must be mptRoot. | ||
| if stored := rawdb.ReadSnapshotRoot(diskdb); stored != mptRoot { | ||
| t.Fatalf("SnapshotRoot in DB: got %x, want mptRoot %x", stored, mptRoot) | ||
| } | ||
| // Generation must complete successfully. | ||
| select { | ||
| case <-snap.genPending: | ||
| case <-time.After(3 * time.Second): | ||
| t.Fatal("snapshot generation timed out — trie walk likely used zkRoot instead of mptRoot") | ||
| } | ||
| // The generated snapshot data must reproduce mptRoot exactly. | ||
| checkSnapRoot(t, snap, mptRoot) | ||
|
|
||
| stop := make(chan *generatorStats) | ||
| snap.genAbort <- stop | ||
| <-stop | ||
| } | ||
|
Comment on lines
+80
to
+102
Contributor
There was a problem hiding this comment. Use Manual shutdown at the end of each test is skipped if an earlier 🔧 Proposed fix+func stopGeneratorOnCleanup(t *testing.T, dl *diskLayer) {
+ t.Helper()
+ t.Cleanup(func() {
+ stop := make(chan *generatorStats)
+ dl.genAbort <- stop
+ <-stop
+ })
+}
func TestGenerateSnapshotTranslatesZkRoot(t *testing.T) {
...
snap := generateSnapshot(diskdb, triedb, 16, zkRoot)
+ stopGeneratorOnCleanup(t, snap)
...
- stop := make(chan *generatorStats)
- snap.genAbort <- stop
- <-stop
}
func TestGenerateSnapshotNoTranslation(t *testing.T) {
...
snap := generateSnapshot(diskdb, triedb, 16, mptRoot)
+ stopGeneratorOnCleanup(t, snap)
...
- stop := make(chan *generatorStats)
- snap.genAbort <- stop
- <-stop
}
func TestRebuildZkRootMapKeyConsistency(t *testing.T) {
...
snaps.Rebuild(zkRoot)
+ stopGeneratorOnCleanup(t, snaps.disklayer())
...
- stop := make(chan *generatorStats)
- snaps.disklayer().genAbort <- stop
- <-stop
}
func TestRebuildWithoutTranslation(t *testing.T) {
...
snaps.Rebuild(mptRoot)
+ stopGeneratorOnCleanup(t, snaps.disklayer())
...
- stop := make(chan *generatorStats)
- snaps.disklayer().genAbort <- stop
- <-stop
}Also applies to: 112-130, 201-244, 260-278 🤖 Prompt for AI Agents |
||
|
|
||
| // TestGenerateSnapshotNoTranslation verifies that when no DiskStateRoot mapping | ||
| // exists (post-Jade-fork MPT blocks where mptRoot IS the block root), root is | ||
| // used unchanged and generation still succeeds. | ||
| func TestGenerateSnapshotNoTranslation(t *testing.T) { | ||
| diskdb := memorydb.New() | ||
| mptRoot, triedb := buildMPTTrie(t, diskdb) | ||
| // No WriteDiskStateRoot call — no mapping. | ||
|
|
||
| snap := generateSnapshot(diskdb, triedb, 16, mptRoot) | ||
|
|
||
| if snap.root != mptRoot { | ||
| t.Fatalf("diskLayer.root: got %x, want %x", snap.root, mptRoot) | ||
| } | ||
| if stored := rawdb.ReadSnapshotRoot(diskdb); stored != mptRoot { | ||
| t.Fatalf("SnapshotRoot in DB: got %x, want %x", stored, mptRoot) | ||
| } | ||
| select { | ||
| case <-snap.genPending: | ||
| case <-time.After(3 * time.Second): | ||
| t.Fatal("snapshot generation timed out") | ||
| } | ||
| checkSnapRoot(t, snap, mptRoot) | ||
|
|
||
| stop := make(chan *generatorStats) | ||
| snap.genAbort <- stop | ||
| <-stop | ||
| } | ||
|
|
||
| // ---- journal.go: loadSnapshot ZK/MPT mismatch tolerance -------------------- | ||
|
|
||
| // TestLoadSnapshotAcceptsZkMptMismatch verifies that loadSnapshot succeeds when | ||
| // the snapshot was stored under mptRoot but is requested with the corresponding | ||
| // zkRoot (MPT node syncing ZK-era blocks, snapshot already generated with fix). | ||
| func TestLoadSnapshotAcceptsZkMptMismatch(t *testing.T) { | ||
| db := rawdb.NewMemoryDatabase() | ||
| triedb := trie.NewDatabase(db) | ||
|
|
||
| mptRoot := common.HexToHash("0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | ||
| zkRoot := common.HexToHash("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") | ||
|
|
||
| // Snapshot is stored at mptRoot (written by generateSnapshot after translation). | ||
| rawdb.WriteSnapshotRoot(db, mptRoot) | ||
| writeDoneGenerator(db) | ||
| // The ZK→MPT mapping tells loadSnapshot they correspond. | ||
| rawdb.WriteDiskStateRoot(db, zkRoot, mptRoot) | ||
|
|
||
| snap, disabled, err := loadSnapshot(db, triedb, 16, zkRoot, false /* recovery */) | ||
| if err != nil { | ||
| t.Fatalf("loadSnapshot with ZK/MPT mismatch should succeed, got: %v", err) | ||
| } | ||
| if disabled { | ||
| t.Fatal("snapshot should not be disabled") | ||
| } | ||
| if snap.Root() != mptRoot { | ||
| t.Fatalf("snapshot root: got %x, want mptRoot %x", snap.Root(), mptRoot) | ||
| } | ||
| } | ||
|
|
||
| // TestLoadSnapshotRejectsTrueMismatch verifies that a genuine root mismatch | ||
| // (no DiskStateRoot mapping, and not in recovery mode) is still rejected so | ||
| // that the ZK/MPT tolerance does not become a security hole. | ||
| func TestLoadSnapshotRejectsTrueMismatch(t *testing.T) { | ||
| db := rawdb.NewMemoryDatabase() | ||
| triedb := trie.NewDatabase(db) | ||
|
|
||
| storedRoot := common.HexToHash("0xcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc") | ||
| requestedRoot := common.HexToHash("0xdddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd") | ||
|
|
||
| // Snapshot stored at storedRoot, no DiskStateRoot mapping for requestedRoot. | ||
| rawdb.WriteSnapshotRoot(db, storedRoot) | ||
| writeDoneGenerator(db) | ||
| // Intentionally no WriteDiskStateRoot call. | ||
|
|
||
| _, _, err := loadSnapshot(db, triedb, 16, requestedRoot, false /* recovery */) | ||
| if err == nil { | ||
| t.Fatal("loadSnapshot should return error for genuine root mismatch without DiskStateRoot mapping") | ||
| } | ||
| } | ||
|
|
||
| // ---- snapshot.go: Rebuild uses base.root as map key ------------------------ | ||
|
|
||
| // TestRebuildZkRootMapKeyConsistency verifies that after Rebuild(zkRoot), the | ||
| // snapshot tree's layers map is keyed by mptRoot (base.root after translation), | ||
| // not zkRoot, so that Snapshot(mptRoot) and DiskRoot() return correct results. | ||
| func TestRebuildZkRootMapKeyConsistency(t *testing.T) { | ||
| diskdb := memorydb.New() | ||
| mptRoot, triedb := buildMPTTrie(t, diskdb) | ||
|
|
||
| zkRoot := common.HexToHash("0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee") | ||
| rawdb.WriteDiskStateRoot(diskdb, zkRoot, mptRoot) | ||
|
|
||
| snaps := &Tree{ | ||
| diskdb: diskdb, | ||
| triedb: triedb, | ||
| cache: 16, | ||
| layers: make(map[common.Hash]snapshot), | ||
| } | ||
| snaps.Rebuild(zkRoot) | ||
|
|
||
| // Wait for generation to complete. | ||
| select { | ||
| case <-snaps.disklayer().genPending: | ||
| case <-time.After(3 * time.Second): | ||
| t.Fatal("snapshot generation timed out") | ||
| } | ||
|
|
||
| // DiskRoot() must return mptRoot. | ||
| if got := snaps.DiskRoot(); got != mptRoot { | ||
| t.Fatalf("DiskRoot(): got %x, want mptRoot %x", got, mptRoot) | ||
| } | ||
| // The layers map must be keyed by mptRoot so Snapshot(mptRoot) finds it. | ||
| if snaps.Snapshot(mptRoot) == nil { | ||
| t.Fatal("Snapshot(mptRoot) returned nil — map key was not translated from zkRoot") | ||
| } | ||
| // Snapshot(zkRoot) should NOT return a layer (key is mptRoot, not zkRoot). | ||
| if snaps.Snapshot(zkRoot) != nil { | ||
| t.Fatal("Snapshot(zkRoot) returned non-nil — map should be keyed by mptRoot only") | ||
| } | ||
| // Verify layers map has exactly one entry (the disk layer at mptRoot). | ||
| snaps.lock.RLock() | ||
| count := len(snaps.layers) | ||
| snaps.lock.RUnlock() | ||
| if count != 1 { | ||
| t.Fatalf("layers map has %d entries, want 1", count) | ||
| } | ||
|
|
||
| // Cap/Update should work correctly using mptRoot as parent. | ||
| diffRoot := common.HexToHash("0xff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00ff00") | ||
| if err := snaps.Update(diffRoot, mptRoot, nil, map[common.Hash][]byte{ | ||
| common.HexToHash("0xab"): randomAccount(), | ||
| }, nil); err != nil { | ||
| t.Fatalf("Update after Rebuild failed: %v", err) | ||
| } | ||
| if snaps.Snapshot(diffRoot) == nil { | ||
| t.Fatal("diffLayer not found after Update — parent lookup using mptRoot failed") | ||
| } | ||
|
|
||
| stop := make(chan *generatorStats) | ||
| snaps.disklayer().genAbort <- stop | ||
| <-stop | ||
| } | ||
|
|
||
| // TestRebuildWithoutTranslation verifies that when no DiskStateRoot mapping | ||
| // exists, Rebuild(mptRoot) stores the disk layer under mptRoot as before — | ||
| // ensuring no regression for the post-Jade-fork (pure MPT) case. | ||
| func TestRebuildWithoutTranslation(t *testing.T) { | ||
| diskdb := memorydb.New() | ||
| mptRoot, triedb := buildMPTTrie(t, diskdb) | ||
| // No DiskStateRoot mapping — pure MPT case. | ||
|
|
||
| snaps := &Tree{ | ||
| diskdb: diskdb, | ||
| triedb: triedb, | ||
| cache: 16, | ||
| layers: make(map[common.Hash]snapshot), | ||
| } | ||
| snaps.Rebuild(mptRoot) | ||
|
|
||
| select { | ||
| case <-snaps.disklayer().genPending: | ||
| case <-time.After(3 * time.Second): | ||
| t.Fatal("snapshot generation timed out") | ||
| } | ||
|
|
||
| if got := snaps.DiskRoot(); got != mptRoot { | ||
| t.Fatalf("DiskRoot(): got %x, want %x", got, mptRoot) | ||
| } | ||
| if snaps.Snapshot(mptRoot) == nil { | ||
| t.Fatal("Snapshot(mptRoot) returned nil after Rebuild(mptRoot)") | ||
| } | ||
|
|
||
| stop := make(chan *generatorStats) | ||
| snaps.disklayer().genAbort <- stop | ||
| <-stop | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 15914
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 454
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 1379
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 3162
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 2194
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 2019
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 1779
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 7832
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 2068
🏁 Script executed:
Repository: morph-l2/go-ethereum
Length of output: 2543
Validate translated root before persisting to prevent corrupted snapshot state.
At lines 114–120,
ReadDiskStateRootreturns the translated root without validating it. If the mapping is malformed or stale, line 120 persists an invalid snapshot root that causes all subsequentsnapshot.Newcalls to fail on restart. The same vulnerability exists incore/state/snapshot/generate.go:162–173.Add validation to confirm the translated root points to an actual trie node before persisting:
Suggested fix
if mptRoot, err2 := rawdb.ReadDiskStateRoot(db, snapDiskRoot); err2 == nil { + if mptRoot == (common.Hash{}) || len(rawdb.ReadTrieNode(db, mptRoot)) == 0 { + log.Warn("Pruner: translated MPT root is not usable, keeping persisted snapshot root", + "zkRoot", snapDiskRoot, "mptRoot", mptRoot) + } else { log.Info("Pruner: resolved snapshot ZK root to MPT root", "zkRoot", snapDiskRoot, "mptRoot", mptRoot) snapDiskRoot = mptRoot rawdb.WriteSnapshotRoot(db, snapDiskRoot) + } }