triedb/pathdb: fix state revert on v2 history

triedb/pathdb/database_test.go

@@ -73,10 +73,10 @@ const (
 
 type genctx struct {
 	stateRoot     common.Hash
-	accounts      map[common.Hash][]byte
+	accounts      map[common.Hash][]byte                    // Keyed by the hash of account address
-	storages      map[common.Hash]map[common.Hash][]byte
+	storages      map[common.Hash]map[common.Hash][]byte    // Keyed by the hash of account address and the hash of storage key
-	accountOrigin map[common.Address][]byte
+	accountOrigin map[common.Address][]byte                 // Keyed by the account address
-	storageOrigin map[common.Address]map[common.Hash][]byte
+	storageOrigin map[common.Address]map[common.Hash][]byte // Keyed by the account address and the hash of storage key
 	nodes         *trienode.MergedNodeSet
 }
 
@@ -113,22 +113,23 @@ type tester struct {
 	preimages map[common.Hash][]byte
 
 	// current state set
-	accounts map[common.Hash][]byte
+	accounts map[common.Hash][]byte                 // Keyed by the hash of account address
-	storages map[common.Hash]map[common.Hash][]byte
+	storages map[common.Hash]map[common.Hash][]byte // Keyed by the hash of account address and the hash of storage key
 
 	// state snapshots
-	snapAccounts map[common.Hash]map[common.Hash][]byte
+	snapAccounts map[common.Hash]map[common.Hash][]byte                 // Keyed by the hash of account address
-	snapStorages map[common.Hash]map[common.Hash]map[common.Hash][]byte
+	snapStorages map[common.Hash]map[common.Hash]map[common.Hash][]byte // Keyed by the hash of account address and the hash of storage key
 }
 
-func newTester(t *testing.T, historyLimit uint64, isVerkle bool) *tester {
+func newTester(t *testing.T, historyLimit uint64, isVerkle bool, layers int) *tester {
 	var (
 		disk, _ = rawdb.NewDatabaseWithFreezer(rawdb.NewMemoryDatabase(), t.TempDir(), "", false)
 		db      = New(disk, &Config{
 			StateHistory:    historyLimit,
-			CleanCacheSize:  16 * 1024,
+			CleanCacheSize:  256 * 1024,
-			WriteBufferSize: 16 * 1024,
+			WriteBufferSize: 256 * 1024,
 		}, isVerkle)
+
 		obj = &tester{
 			db:           db,
 			preimages:    make(map[common.Hash][]byte),
@@ -138,7 +139,7 @@ func newTester(t *testing.T, historyLimit uint64, isVerkle bool) *tester {
 			snapStorages: make(map[common.Hash]map[common.Hash]map[common.Hash][]byte),
 		}
 	)
-	for i := 0; i < 12; i++ {
+	for i := 0; i < layers; i++ {
 		var parent = types.EmptyRootHash
 		if len(obj.roots) != 0 {
 			parent = obj.roots[len(obj.roots)-1]
@@ -264,11 +265,11 @@ func (t *tester) generate(parent common.Hash, rawStorageKey bool) (common.Hash,
 			addr := testrand.Address()
 			addrHash := crypto.Keccak256Hash(addr.Bytes())
 
-			// short circuit if the account was already existent
+			// Short circuit if the account was already existent
 			if _, ok := t.accounts[addrHash]; ok {
 				continue
 			}
-			// short circuit if the account has been modified within the same transition
+			// Short circuit if the account has been modified within the same transition
 			if _, ok := dirties[addrHash]; ok {
 				continue
 			}
@@ -448,7 +449,7 @@ func TestDatabaseRollback(t *testing.T) {
 	}()
 
 	// Verify state histories
-	tester := newTester(t, 0, false)
+	tester := newTester(t, 0, false, 32)
 	defer tester.release()
 
 	if err := tester.verifyHistory(); err != nil {
@@ -482,7 +483,7 @@ func TestDatabaseRecoverable(t *testing.T) {
 	}()
 
 	var (
-		tester = newTester(t, 0, false)
+		tester = newTester(t, 0, false, 12)
 		index  = tester.bottomIndex()
 	)
 	defer tester.release()
@@ -526,7 +527,7 @@ func TestDisable(t *testing.T) {
 		maxDiffLayers = 128
 	}()
 
-	tester := newTester(t, 0, false)
+	tester := newTester(t, 0, false, 12)
 	defer tester.release()
 
 	stored := crypto.Keccak256Hash(rawdb.ReadAccountTrieNode(tester.db.diskdb, nil))
@@ -568,7 +569,7 @@ func TestCommit(t *testing.T) {
 		maxDiffLayers = 128
 	}()
 
-	tester := newTester(t, 0, false)
+	tester := newTester(t, 0, false, 12)
 	defer tester.release()
 
 	if err := tester.db.Commit(tester.lastHash(), false); err != nil {
@@ -598,7 +599,7 @@ func TestJournal(t *testing.T) {
 		maxDiffLayers = 128
 	}()
 
-	tester := newTester(t, 0, false)
+	tester := newTester(t, 0, false, 12)
 	defer tester.release()
 
 	if err := tester.db.Journal(tester.lastHash()); err != nil {
@@ -628,7 +629,7 @@ func TestCorruptedJournal(t *testing.T) {
 		maxDiffLayers = 128
 	}()
 
-	tester := newTester(t, 0, false)
+	tester := newTester(t, 0, false, 12)
 	defer tester.release()
 
 	if err := tester.db.Journal(tester.lastHash()); err != nil {
@@ -676,7 +677,7 @@ func TestTailTruncateHistory(t *testing.T) {
 		maxDiffLayers = 128
 	}()
 
-	tester := newTester(t, 10, false)
+	tester := newTester(t, 10, false, 12)
 	defer tester.release()
 
 	tester.db.Close()

triedb/pathdb/disklayer.go

@@ -295,24 +295,6 @@ func (dl *diskLayer) revert(h *history) (*diskLayer, error) {
 	if dl.id == 0 {
 		return nil, fmt.Errorf("%w: zero state id", errStateUnrecoverable)
 	}
-	var (
-		buff     = crypto.NewKeccakState()
-		hashes   = make(map[common.Address]common.Hash)
-		accounts = make(map[common.Hash][]byte)
-		storages = make(map[common.Hash]map[common.Hash][]byte)
-	)
-	for addr, blob := range h.accounts {
-		hash := crypto.HashData(buff, addr.Bytes())
-		hashes[addr] = hash
-		accounts[hash] = blob
-	}
-	for addr, storage := range h.storages {
-		hash, ok := hashes[addr]
-		if !ok {
-			panic(fmt.Errorf("storage history with no account %x", addr))
-		}
-		storages[hash] = storage
-	}
 	// Apply the reverse state changes upon the current state. This must
 	// be done before holding the lock in order to access state in "this"
 	// layer.
@@ -332,6 +314,7 @@ func (dl *diskLayer) revert(h *history) (*diskLayer, error) {
 	// needs to be reverted is not yet flushed and cached in node
 	// buffer, otherwise, manipulate persistent state directly.
 	if !dl.buffer.empty() {
+		accounts, storages := h.stateSet()
 		err := dl.buffer.revertTo(dl.db.diskdb, nodes, accounts, storages)
 		if err != nil {
 			return nil, err

triedb/pathdb/history.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/rawdb"
+	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/log"
 	"golang.org/x/exp/maps"
@@ -68,8 +69,9 @@ const (
 	slotIndexSize    = common.HashLength + 5     // The length of encoded slot index
 	historyMetaSize  = 9 + 2*common.HashLength   // The length of encoded history meta
 
-	stateHistoryV0 = uint8(0) // initial version of state history structure
+	stateHistoryV0 = uint8(0)       // initial version of state history structure
-	stateHistoryV1 = uint8(1) // use the storage slot raw key as the identifier instead of the key hash
+	stateHistoryV1 = uint8(1)       // use the storage slot raw key as the identifier instead of the key hash
+	historyVersion = stateHistoryV1 // the default state history version
 )
 
 // Each state history entry is consisted of five elements:
@@ -258,9 +260,9 @@ func newHistory(root common.Hash, parent common.Hash, block uint64, accounts map
 		slices.SortFunc(slist, common.Hash.Cmp)
 		storageList[addr] = slist
 	}
-	version := stateHistoryV0
+	version := historyVersion
-	if rawStorageKey {
+	if !rawStorageKey {
-		version = stateHistoryV1
+		version = stateHistoryV0
 	}
 	return &history{
 		meta: &meta{
@@ -276,6 +278,35 @@ func newHistory(root common.Hash, parent common.Hash, block uint64, accounts map
 	}
 }
 
+// stateSet returns the state set, keyed by the hash of the account address
+// and the hash of the storage slot key.
+func (h *history) stateSet() (map[common.Hash][]byte, map[common.Hash]map[common.Hash][]byte) {
+	var (
+		buff     = crypto.NewKeccakState()
+		accounts = make(map[common.Hash][]byte)
+		storages = make(map[common.Hash]map[common.Hash][]byte)
+	)
+	for addr, blob := range h.accounts {
+		addrHash := crypto.HashData(buff, addr.Bytes())
+		accounts[addrHash] = blob
+
+		storage, exist := h.storages[addr]
+		if !exist {
+			continue
+		}
+		if h.meta.version == stateHistoryV0 {
+			storages[addrHash] = storage
+		} else {
+			subset := make(map[common.Hash][]byte)
+			for key, slot := range storage {
+				subset[crypto.HashData(buff, key.Bytes())] = slot
+			}
+			storages[addrHash] = subset
+		}
+	}
+	return accounts, storages
+}
+
 // encode serializes the state history and returns four byte streams represent
 // concatenated account/storage data, account/storage indexes respectively.
 func (h *history) encode() ([]byte, []byte, []byte, []byte) {