diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index 561920758e..40524e53c4 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -2108,15 +2108,21 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { // Large contracts could have generated new trie nodes, flush them to disk if res.subTask != nil { if res.subTask.done { - if root, err := res.subTask.genTrie.Commit(); err != nil { + if _, err := res.subTask.genTrie.Commit(); err != nil { log.Error("Failed to commit stack slots", "err", err) - } else if root == res.subTask.root { - // If the chunk's root is an overflown but full delivery, clear the heal request - for i, account := range res.mainTask.res.hashes { - if account == res.accounts[len(res.accounts)-1] { - res.mainTask.needHeal[i] = false - } - } + // The genTrie.Commit()-operation, if initialized from a proof, will + // return the correct root. However, it will also _avoid_ comitting + // nodes along the chunk-lines / boundaries, hence we _do_ need + // to perform healing anyway. + // + //} else if root == res.subTask.root { + // // If the chunk's root is an overflown but full delivery, clear the heal request + // for i, account := range res.mainTask.res.hashes { + // if account == res.accounts[len(res.accounts)-1] { + // res.mainTask.needHeal[i] = false + // fmt.Printf("Marking needHeal as false %x\n", account) + // } + // } } } if res.subTask.genBatch.ValueSize() > ethdb.IdealBatchSize || res.subTask.done { diff --git a/eth/protocols/snap/sync_test.go b/eth/protocols/snap/sync_test.go index 5f4f13ea74..daf5a1d1d8 100644 --- a/eth/protocols/snap/sync_test.go +++ b/eth/protocols/snap/sync_test.go @@ -788,6 +788,49 @@ func testSyncWithStorage(t *testing.T, scheme string) { verifyTrie(scheme, syncer.db, sourceAccountTrie.Hash(), t) } +// TestSyncWithLargeStorage tests basic sync using accounts + storage + code, where +// storage is large enough to trigger chunking. +func TestSyncWithLargeStorage(t *testing.T) { + t.Parallel() + + t.Run("hash", func(t *testing.T) { + testSyncWithLargeStorage(t, rawdb.HashScheme) + }) + t.Run("path", func(t *testing.T) { + testSyncWithLargeStorage(t, rawdb.PathScheme) + }) + //testSyncWithStorage(t, rawdb.PathScheme) +} + +func testSyncWithLargeStorage(t *testing.T, scheme string) { + var ( + once sync.Once + cancel = make(chan struct{}) + term = func() { + once.Do(func() { + close(cancel) + }) + } + ) + nodeScheme, sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(scheme, 3, 30000, true, false) + + mkSource := func(name string) *testPeer { + source := newTestPeer(name, t, term) + source.accountTrie = sourceAccountTrie.Copy() + source.accountValues = elems + source.setStorageTries(storageTries) + source.storageValues = storageElems + return source + } + syncer := setupSyncer(nodeScheme, mkSource("sourceA")) + done := checkStall(t, term) + if err := syncer.Sync(sourceAccountTrie.Hash(), cancel); err != nil { + t.Fatalf("sync failed: %v", err) + } + close(done) + verifyTrie(scheme, syncer.db, sourceAccountTrie.Hash(), t) +} + // TestMultiSyncManyUseless contains one good peer, and many which doesn't return anything valuable at all func TestMultiSyncManyUseless(t *testing.T) { t.Parallel()