Skip to content

Commit 4956f83

Browse files
rjl493456442gballet
authored andcommitted
triedb/pathdb: fix state revert on v2 history (ethereum#31060)
State history v2 has been shipped and will take effect after the Cancun fork. However, the state revert function does not differentiate between v1 and v2, instead blindly using the storage map key for state reversion. This mismatch between the keys of the live state set and the state history can trigger a panic: `non-existent storage slot for reverting`. This flaw has been fixed in this PR.
1 parent 2910bee commit 4956f83

File tree

3 files changed

+62
-44
lines changed

3 files changed

+62
-44
lines changed

triedb/pathdb/database_test.go

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,10 @@ const (
7373

7474
type genctx struct {
7575
stateRoot common.Hash
76-
accounts map[common.Hash][]byte
77-
storages map[common.Hash]map[common.Hash][]byte
78-
accountOrigin map[common.Address][]byte
79-
storageOrigin map[common.Address]map[common.Hash][]byte
76+
accounts map[common.Hash][]byte // Keyed by the hash of account address
77+
storages map[common.Hash]map[common.Hash][]byte // Keyed by the hash of account address and the hash of storage key
78+
accountOrigin map[common.Address][]byte // Keyed by the account address
79+
storageOrigin map[common.Address]map[common.Hash][]byte // Keyed by the account address and the hash of storage key
8080
nodes *trienode.MergedNodeSet
8181
}
8282

@@ -113,22 +113,23 @@ type tester struct {
113113
preimages map[common.Hash][]byte
114114

115115
// current state set
116-
accounts map[common.Hash][]byte
117-
storages map[common.Hash]map[common.Hash][]byte
116+
accounts map[common.Hash][]byte // Keyed by the hash of account address
117+
storages map[common.Hash]map[common.Hash][]byte // Keyed by the hash of account address and the hash of storage key
118118

119119
// state snapshots
120-
snapAccounts map[common.Hash]map[common.Hash][]byte
121-
snapStorages map[common.Hash]map[common.Hash]map[common.Hash][]byte
120+
snapAccounts map[common.Hash]map[common.Hash][]byte // Keyed by the hash of account address
121+
snapStorages map[common.Hash]map[common.Hash]map[common.Hash][]byte // Keyed by the hash of account address and the hash of storage key
122122
}
123123

124-
func newTester(t *testing.T, historyLimit uint64, isVerkle bool) *tester {
124+
func newTester(t *testing.T, historyLimit uint64, isVerkle bool, layers int) *tester {
125125
var (
126126
disk, _ = rawdb.NewDatabaseWithFreezer(rawdb.NewMemoryDatabase(), t.TempDir(), "", false)
127127
db = New(disk, &Config{
128128
StateHistory: historyLimit,
129-
CleanCacheSize: 16 * 1024,
130-
WriteBufferSize: 16 * 1024,
129+
CleanCacheSize: 256 * 1024,
130+
WriteBufferSize: 256 * 1024,
131131
}, isVerkle)
132+
132133
obj = &tester{
133134
db: db,
134135
preimages: make(map[common.Hash][]byte),
@@ -138,7 +139,7 @@ func newTester(t *testing.T, historyLimit uint64, isVerkle bool) *tester {
138139
snapStorages: make(map[common.Hash]map[common.Hash]map[common.Hash][]byte),
139140
}
140141
)
141-
for i := 0; i < 12; i++ {
142+
for i := 0; i < layers; i++ {
142143
var parent = types.EmptyRootHash
143144
if len(obj.roots) != 0 {
144145
parent = obj.roots[len(obj.roots)-1]
@@ -264,11 +265,11 @@ func (t *tester) generate(parent common.Hash, rawStorageKey bool) (common.Hash,
264265
addr := testrand.Address()
265266
addrHash := crypto.Keccak256Hash(addr.Bytes())
266267

267-
// short circuit if the account was already existent
268+
// Short circuit if the account was already existent
268269
if _, ok := t.accounts[addrHash]; ok {
269270
continue
270271
}
271-
// short circuit if the account has been modified within the same transition
272+
// Short circuit if the account has been modified within the same transition
272273
if _, ok := dirties[addrHash]; ok {
273274
continue
274275
}
@@ -448,7 +449,7 @@ func TestDatabaseRollback(t *testing.T) {
448449
}()
449450

450451
// Verify state histories
451-
tester := newTester(t, 0, false)
452+
tester := newTester(t, 0, false, 32)
452453
defer tester.release()
453454

454455
if err := tester.verifyHistory(); err != nil {
@@ -482,7 +483,7 @@ func TestDatabaseRecoverable(t *testing.T) {
482483
}()
483484

484485
var (
485-
tester = newTester(t, 0, false)
486+
tester = newTester(t, 0, false, 12)
486487
index = tester.bottomIndex()
487488
)
488489
defer tester.release()
@@ -526,7 +527,7 @@ func TestDisable(t *testing.T) {
526527
maxDiffLayers = 128
527528
}()
528529

529-
tester := newTester(t, 0, false)
530+
tester := newTester(t, 0, false, 32)
530531
defer tester.release()
531532

532533
stored := crypto.Keccak256Hash(rawdb.ReadAccountTrieNode(tester.db.diskdb, nil))
@@ -568,7 +569,7 @@ func TestCommit(t *testing.T) {
568569
maxDiffLayers = 128
569570
}()
570571

571-
tester := newTester(t, 0, false)
572+
tester := newTester(t, 0, false, 12)
572573
defer tester.release()
573574

574575
if err := tester.db.Commit(tester.lastHash(), false); err != nil {
@@ -598,7 +599,7 @@ func TestJournal(t *testing.T) {
598599
maxDiffLayers = 128
599600
}()
600601

601-
tester := newTester(t, 0, false)
602+
tester := newTester(t, 0, false, 12)
602603
defer tester.release()
603604

604605
if err := tester.db.Journal(tester.lastHash()); err != nil {
@@ -628,7 +629,7 @@ func TestCorruptedJournal(t *testing.T) {
628629
maxDiffLayers = 128
629630
}()
630631

631-
tester := newTester(t, 0, false)
632+
tester := newTester(t, 0, false, 12)
632633
defer tester.release()
633634

634635
if err := tester.db.Journal(tester.lastHash()); err != nil {
@@ -676,7 +677,7 @@ func TestTailTruncateHistory(t *testing.T) {
676677
maxDiffLayers = 128
677678
}()
678679

679-
tester := newTester(t, 10, false)
680+
tester := newTester(t, 10, false, 12)
680681
defer tester.release()
681682

682683
tester.db.Close()

triedb/pathdb/disklayer.go

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -295,31 +295,17 @@ func (dl *diskLayer) revert(h *history) (*diskLayer, error) {
295295
if dl.id == 0 {
296296
return nil, fmt.Errorf("%w: zero state id", errStateUnrecoverable)
297297
}
298-
var (
299-
buff = crypto.NewKeccakState()
300-
hashes = make(map[common.Address]common.Hash)
301-
accounts = make(map[common.Hash][]byte)
302-
storages = make(map[common.Hash]map[common.Hash][]byte)
303-
)
304-
for addr, blob := range h.accounts {
305-
hash := crypto.HashData(buff, addr.Bytes())
306-
hashes[addr] = hash
307-
accounts[hash] = blob
308-
}
309-
for addr, storage := range h.storages {
310-
hash, ok := hashes[addr]
311-
if !ok {
312-
panic(fmt.Errorf("storage history with no account %x", addr))
313-
}
314-
storages[hash] = storage
315-
}
316298
// Apply the reverse state changes upon the current state. This must
317299
// be done before holding the lock in order to access state in "this"
318300
// layer.
319301
nodes, err := apply(dl.db, h.meta.parent, h.meta.root, h.meta.version != stateHistoryV0, h.accounts, h.storages)
320302
if err != nil {
321303
return nil, err
322304
}
305+
// Derive the state modification set from the history, keyed by the hash
306+
// of the account address and the storage key.
307+
accounts, storages := h.stateSet()
308+
323309
// Mark the diskLayer as stale before applying any mutations on top.
324310
dl.lock.Lock()
325311
defer dl.lock.Unlock()

triedb/pathdb/history.go

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
"github.com/ethereum/go-ethereum/common"
2828
"github.com/ethereum/go-ethereum/core/rawdb"
29+
"github.com/ethereum/go-ethereum/crypto"
2930
"github.com/ethereum/go-ethereum/ethdb"
3031
"github.com/ethereum/go-ethereum/log"
3132
"golang.org/x/exp/maps"
@@ -68,8 +69,9 @@ const (
6869
slotIndexSize = common.HashLength + 5 // The length of encoded slot index
6970
historyMetaSize = 9 + 2*common.HashLength // The length of encoded history meta
7071

71-
stateHistoryV0 = uint8(0) // initial version of state history structure
72-
stateHistoryV1 = uint8(1) // use the storage slot raw key as the identifier instead of the key hash
72+
stateHistoryV0 = uint8(0) // initial version of state history structure
73+
stateHistoryV1 = uint8(1) // use the storage slot raw key as the identifier instead of the key hash
74+
historyVersion = stateHistoryV1 // the default state history version
7375
)
7476

7577
// 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
258260
slices.SortFunc(slist, common.Hash.Cmp)
259261
storageList[addr] = slist
260262
}
261-
version := stateHistoryV0
262-
if rawStorageKey {
263-
version = stateHistoryV1
263+
version := historyVersion
264+
if !rawStorageKey {
265+
version = stateHistoryV0
264266
}
265267
return &history{
266268
meta: &meta{
@@ -276,6 +278,35 @@ func newHistory(root common.Hash, parent common.Hash, block uint64, accounts map
276278
}
277279
}
278280

281+
// stateSet returns the state set, keyed by the hash of the account address
282+
// and the hash of the storage slot key.
283+
func (h *history) stateSet() (map[common.Hash][]byte, map[common.Hash]map[common.Hash][]byte) {
284+
var (
285+
buff = crypto.NewKeccakState()
286+
accounts = make(map[common.Hash][]byte)
287+
storages = make(map[common.Hash]map[common.Hash][]byte)
288+
)
289+
for addr, blob := range h.accounts {
290+
addrHash := crypto.HashData(buff, addr.Bytes())
291+
accounts[addrHash] = blob
292+
293+
storage, exist := h.storages[addr]
294+
if !exist {
295+
continue
296+
}
297+
if h.meta.version == stateHistoryV0 {
298+
storages[addrHash] = storage
299+
} else {
300+
subset := make(map[common.Hash][]byte)
301+
for key, slot := range storage {
302+
subset[crypto.HashData(buff, key.Bytes())] = slot
303+
}
304+
storages[addrHash] = subset
305+
}
306+
}
307+
return accounts, storages
308+
}
309+
279310
// encode serializes the state history and returns four byte streams represent
280311
// concatenated account/storage data, account/storage indexes respectively.
281312
func (h *history) encode() ([]byte, []byte, []byte, []byte) {

0 commit comments

Comments
 (0)