Skip to content

Commit

Permalink
triedb/pathdb: fix state revert on v2 history (ethereum#31060)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@rjl493456442
rjl493456442 authored Jan 22, 2025
1 parent d10c61c commit a840e9b
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 44 deletions.
43 changes: 22 additions & 21 deletions triedb/pathdb/database_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ const (

type genctx struct {
stateRoot common.Hash
accounts map[common.Hash][]byte
storages map[common.Hash]map[common.Hash][]byte
accountOrigin map[common.Address][]byte
storageOrigin map[common.Address]map[common.Hash][]byte
accounts map[common.Hash][]byte // Keyed by the hash of account address
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 // Keyed by the account address
storageOrigin map[common.Address]map[common.Hash][]byte // Keyed by the account address and the hash of storage key
nodes *trienode.MergedNodeSet
}

Expand Down Expand Up @@ -113,22 +113,23 @@ type tester struct {
preimages map[common.Hash][]byte

// current state set
accounts map[common.Hash][]byte
storages map[common.Hash]map[common.Hash][]byte
accounts map[common.Hash][]byte // Keyed by the hash of account address
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
snapStorages map[common.Hash]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 // 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,
WriteBufferSize: 16 * 1024,
CleanCacheSize: 256 * 1024,
WriteBufferSize: 256 * 1024,
}, isVerkle)

obj = &tester{
db: db,
preimages: make(map[common.Hash][]byte),
Expand All @@ -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]
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -526,7 +527,7 @@ func TestDisable(t *testing.T) {
maxDiffLayers = 128
}()

tester := newTester(t, 0, false)
tester := newTester(t, 0, false, 32)
defer tester.release()

stored := crypto.Keccak256Hash(rawdb.ReadAccountTrieNode(tester.db.diskdb, nil))
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down
22 changes: 4 additions & 18 deletions triedb/pathdb/disklayer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,31 +295,17 @@ 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.
nodes, err := apply(dl.db, h.meta.parent, h.meta.root, h.meta.version != stateHistoryV0, h.accounts, h.storages)
if err != nil {
return nil, err
}
// Derive the state modification set from the history, keyed by the hash
// of the account address and the storage key.
accounts, storages := h.stateSet()

// Mark the diskLayer as stale before applying any mutations on top.
dl.lock.Lock()
defer dl.lock.Unlock()
Expand Down
41 changes: 36 additions & 5 deletions triedb/pathdb/history.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
stateHistoryV1 = uint8(1) // use the storage slot raw key as the identifier instead of the key hash
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
historyVersion = stateHistoryV1 // the default state history version
)

// Each state history entry is consisted of five elements:
Expand Down Expand Up @@ -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
if rawStorageKey {
version = stateHistoryV1
version := historyVersion
if !rawStorageKey {
version = stateHistoryV0
}
return &history{
meta: &meta{
Expand All @@ -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) {
Expand Down

0 comments on commit a840e9b

Please sign in to comment.