core,eth,triedb: route snap/2 completion through pathdb.AdoptSyncedState

cmd/devp2p/internal/ethtest/snap2.go

@@ -337,6 +337,8 @@ func (s *Suite) snapGetAccessLists(t *utesting.T, tc *accessListsTest) error {
 	for _, p := range tc.mustBeEmptyAt {
 		mustEmpty[p] = struct{}{}
 	}
+	head := s.chain.Head().Header()
+	rules := s.chain.config.Rules(head.Number, true, head.Time)
 
 	for it.Next() {
 		raw := it.Value()
@@ -383,7 +385,7 @@ func (s *Suite) snapGetAccessLists(t *utesting.T, tc *accessListsTest) error {
 		if err := rlp.DecodeBytes(raw, &accessList); err != nil {
 			return fmt.Errorf("entry %d: invalid BAL RLP: %v", idx, err)
 		}
-		if err := accessList.Validate(); err != nil {
+		if err := accessList.Validate(rules); err != nil {
 			return fmt.Errorf("entry %d: BAL failed validation: %v", idx, err)
 		}
 		idx++

core/blockchain.go

@@ -1158,7 +1158,7 @@ func (bc *BlockChain) SnapSyncStart() error {
 // given hash, regardless of the chain contents prior to snap sync. It is
 // invoked once snap sync completes and assumes that SnapSyncStart was called
 // previously.
-func (bc *BlockChain) SnapSyncComplete(hash common.Hash) error {
+func (bc *BlockChain) SnapSyncComplete(hash common.Hash, flatStateReady bool) error {
 	// Make sure that both the block as well at its state trie exists
 	block := bc.GetBlockByHash(hash)
 	if block == nil {
@@ -1169,20 +1169,29 @@ func (bc *BlockChain) SnapSyncComplete(hash common.Hash) error {
 	}
 	defer bc.chainmu.Unlock()
 
-	// Reset the trie database with the fresh snap synced state.
+	// Reset the trie database with the fresh snap synced state. Snap/1 needs
+	// a full trie-to-flat regeneration; snap/2 adopts the already-consistent
+	// flat state and skips that work.
 	root := block.Root()
 	if bc.triedb.Scheme() == rawdb.PathScheme {
-		if err := bc.triedb.Enable(root); err != nil {
-			return err
+		if flatStateReady {
+			if err := bc.triedb.AdoptSyncedState(root); err != nil {
+				return err
+			}
+		} else {
+			if err := bc.triedb.Enable(root); err != nil {
+				return err
+			}
 		}
 	}
 	if !bc.HasState(root) {
 		return fmt.Errorf("non existent state [%x..]", root[:4])
 	}
-	// Set up the snapshot tree from the synced flat state. Snap/2 downloads
-	// flat state directly as the snapshot.
-	if bc.snaps != nil {
-		bc.snaps.InitFromSyncedState(root)
+
+	// The legacy snapshot tree needs to be wiped and rebuilt from the trie
+	// after a snap/1 sync.
+	if !flatStateReady && bc.snaps != nil {
+		bc.snaps.Rebuild(root)
 	}
 
 	// If all checks out, manually set the head block.

core/state/snapshot/snapshot.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"sync"
 
-	"github.com/VictoriaMetrics/fastcache"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/core/types"
@@ -727,35 +726,6 @@ func (t *Tree) Rebuild(root common.Hash) {
 	}
 }
 
-// InitFromSyncedState sets up the snapshot tree to use flat state that was
-// already downloaded by snap sync. Unlike Rebuild, it does NOT regenerate the
-// snapshot from the trie.
-func (t *Tree) InitFromSyncedState(root common.Hash) {
-	t.lock.Lock()
-	defer t.lock.Unlock()
-
-	// Delete any recovery flag in the database.
-	rawdb.DeleteSnapshotRecoveryNumber(t.diskdb)
-	rawdb.DeleteSnapshotDisabled(t.diskdb)
-
-	// Write the new root.
-	rawdb.WriteSnapshotRoot(t.diskdb, root)
-
-	// Clear the journal.
-	journalProgress(t.diskdb, nil, nil)
-	log.Info("Setting up snapshot from synced state", "root", root)
-
-	// Replace t.layers with a single diskLayer pointing at the root.
-	t.layers = map[common.Hash]snapshot{
-		root: &diskLayer{
-			diskdb: t.diskdb,
-			triedb: t.triedb,
-			cache:  fastcache.New(t.config.CacheSize * 1024 * 1024),
-			root:   root,
-		},
-	}
-}
-
 // AccountIterator creates a new account iterator for the specified root hash and
 // seeks to a starting account hash.
 func (t *Tree) AccountIterator(root common.Hash, seek common.Hash) (AccountIterator, error) {

eth/downloader/downloader.go

@@ -198,7 +198,7 @@ type BlockChain interface {
 	SnapSyncStart() error
 
 	// SnapSyncComplete directly commits the head block to a certain entity.
-	SnapSyncComplete(common.Hash) error
+	SnapSyncComplete(hash common.Hash, flatStateReady bool) error
 
 	// InsertHeadersBeforeCutoff inserts a batch of headers before the configured
 	// chain cutoff into the ancient store.
@@ -1052,7 +1052,9 @@ func (d *Downloader) commitPivotBlock(result *fetchResult) error {
 	if _, err := d.blockchain.InsertReceiptChain([]*types.Block{block}, []rlp.RawValue{result.Receipts}, d.ancientLimit); err != nil {
 		return err
 	}
-	if err := d.blockchain.SnapSyncComplete(block.Hash()); err != nil {
+
+	// TODO JR: This needs to pass trie for snap/2 and false for snap/1
+	if err := d.blockchain.SnapSyncComplete(block.Hash(), true); err != nil {
 		return err
 	}
 	d.committed.Store(true)

eth/protocols/snap/bal_apply.go

@@ -61,7 +61,7 @@ func (s *Syncer) applyAccessList(b *bal.BlockAccessList) error {
 	batch := s.db.NewBatch()
 
 	// Iterate over all accounts in the access list
-	for _, access := range b.Accesses {
+	for _, access := range *b {
 		addr := common.Address(access.Address)
 		accountHash := crypto.Keccak256Hash(addr[:])
 
@@ -91,8 +91,7 @@ func (s *Syncer) applyAccessList(b *bal.BlockAccessList) error {
 
 		// Apply balance change (last entry = post-block state)
 		if n := len(access.BalanceChanges); n > 0 {
-			raw := access.BalanceChanges[n-1].Balance
-			account.Balance = new(uint256.Int).SetBytes(raw[:])
+			account.Balance = new(uint256.Int).Set(access.BalanceChanges[n-1].Balance)
 		}
 
 		// Apply nonce change (last entry = post-block state)
@@ -116,11 +115,13 @@ func (s *Syncer) applyAccessList(b *bal.BlockAccessList) error {
 		for _, slotWrites := range access.StorageWrites {
 			if n := len(slotWrites.Accesses); n > 0 {
 				value := slotWrites.Accesses[n-1].ValueAfter
-				storageHash := crypto.Keccak256Hash(slotWrites.Slot[:])
-				if value == (common.Hash{}) {
+				slotKey := slotWrites.Slot.Bytes32()
+				storageHash := crypto.Keccak256Hash(slotKey[:])
+				if value.IsZero() {
 					rawdb.DeleteStorageSnapshot(batch, accountHash, storageHash)
 				} else {
-					rawdb.WriteStorageSnapshot(batch, accountHash, storageHash, value[:])
+					valBytes := value.Bytes32()
+					rawdb.WriteStorageSnapshot(batch, accountHash, storageHash, valBytes[:])
 				}
 			}
 		}

eth/protocols/snap/sync_test.go

@@ -666,6 +666,41 @@ func testSync(t *testing.T, scheme string) {
 		t.Fatalf("sync failed: %v", err)
 	}
 	verifyTrie(scheme, syncer.db, sourceAccountTrie.Hash(), t)
+	verifyAdoptedSyncedState(scheme, syncer.db, sourceAccountTrie.Hash(), elems, t)
+}
+
+// verifyAdoptedSyncedState exercises the snap/2 completion contract end-to-end:
+// after a real sync, opening a fresh triedb and calling AdoptSyncedState must
+// (a) succeed and (b) leave flat-state reads serving immediately, with no
+// background regeneration gating them.
+func verifyAdoptedSyncedState(scheme string, db ethdb.KeyValueStore, root common.Hash, elems []*kv, t *testing.T) {
+	t.Helper()
+	if scheme != rawdb.PathScheme {
+		return
+	}
+	tdb := triedb.NewDatabase(rawdb.NewDatabase(db), newDbConfig(scheme))
+	defer tdb.Close()
+
+	if err := tdb.AdoptSyncedState(root); err != nil {
+		t.Fatalf("AdoptSyncedState failed: %v", err)
+	}
+	// Read one of the synced accounts via the public flat-state API. If this
+	// returned errNotCoveredYet we'd know AdoptSyncedState left a generator
+	// gating reads, exactly the bug we're trying to prevent.
+	sr, err := tdb.StateReader(root)
+	if err != nil {
+		t.Fatalf("StateReader: %v", err)
+	}
+	if len(elems) == 0 {
+		return
+	}
+	acc, err := sr.Account(common.BytesToHash(elems[0].k))
+	if err != nil {
+		t.Fatalf("flat-state read failed after AdoptSyncedState: %v", err)
+	}
+	if acc == nil {
+		t.Fatal("flat-state read returned nil account; sync did not populate the snapshot namespace")
+	}
 }
 
 // TestSyncTinyTriePanic tests a basic sync with one peer, and a tiny trie. This caused a