From a840e9b59f28427d3c65de9f028853e075438201 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Wed, 22 Jan 2025 21:06:36 +0800 Subject: [PATCH] triedb/pathdb: fix state revert on v2 history (#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. --- triedb/pathdb/database_test.go | 43 +++++++++++++++++----------------- triedb/pathdb/disklayer.go | 22 ++++------------- triedb/pathdb/history.go | 41 ++++++++++++++++++++++++++++---- 3 files changed, 62 insertions(+), 44 deletions(-) diff --git a/triedb/pathdb/database_test.go b/triedb/pathdb/database_test.go index f4b3fcec23..c85d7832ee 100644 --- a/triedb/pathdb/database_test.go +++ b/triedb/pathdb/database_test.go @@ -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 } @@ -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), @@ -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, 32) 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() diff --git a/triedb/pathdb/disklayer.go b/triedb/pathdb/disklayer.go index 5e678dbdee..184f6430a2 100644 --- a/triedb/pathdb/disklayer.go +++ b/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. @@ -320,6 +302,10 @@ func (dl *diskLayer) revert(h *history) (*diskLayer, error) { 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() diff --git a/triedb/pathdb/history.go b/triedb/pathdb/history.go index 9fb7d9e153..d8fb99ade4 100644 --- a/triedb/pathdb/history.go +++ b/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 - 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: @@ -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{ @@ -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) {