diff --git a/core/state/statedb.go b/core/state/statedb.go index ac847d87b2..48adadf085 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -36,12 +36,6 @@ import ( "github.com/ethereum/go-ethereum/trie/triestate" ) -const ( - // storageDeleteLimit denotes the highest permissible memory allocation - // employed for contract storage deletion. - storageDeleteLimit = 512 * 1024 * 1024 -) - type revision struct { id int journalIndex int @@ -989,130 +983,59 @@ func (s *StateDB) clearJournalAndRefund() { s.validRevisions = s.validRevisions[:0] // Snapshots can be created without journal entries } -// fastDeleteStorage is the function that efficiently deletes the storage trie -// of a specific account. It leverages the associated state snapshot for fast -// storage iteration and constructs trie node deletion markers by creating -// stack trie with iterated slots. -func (s *StateDB) fastDeleteStorage(addrHash common.Hash, root common.Hash) (bool, common.StorageSize, map[common.Hash][]byte, *trienode.NodeSet, error) { - iter, err := s.snaps.StorageIterator(s.originalRoot, addrHash, common.Hash{}) - if err != nil { - return false, 0, nil, nil, err - } - defer iter.Release() - - var ( - size common.StorageSize - nodes = trienode.NewNodeSet(addrHash) - slots = make(map[common.Hash][]byte) - ) - stack := trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { - nodes.AddNode(path, trienode.NewDeleted()) - size += common.StorageSize(len(path)) - }) - for iter.Next() { - if size > storageDeleteLimit { - return true, size, nil, nil, nil - } - slot := common.CopyBytes(iter.Slot()) - if iter.Error() != nil { // error might occur after Slot function - return false, 0, nil, nil, err - } - size += common.StorageSize(common.HashLength + len(slot)) - slots[iter.Hash()] = slot - - if err := stack.Update(iter.Hash().Bytes(), slot); err != nil { - return false, 0, nil, nil, err - } - } - if iter.Error() != nil { // error might occur during iteration - return false, 0, nil, nil, err - } - if stack.Hash() != root { - return false, 0, nil, nil, fmt.Errorf("snapshot is not matched, exp %x, got %x", root, stack.Hash()) - } - return false, size, slots, nodes, nil -} - -// slowDeleteStorage serves as a less-efficient alternative to "fastDeleteStorage," -// employed when the associated state snapshot is not available. It iterates the -// storage slots along with all internal trie nodes via trie directly. -func (s *StateDB) slowDeleteStorage(addr common.Address, addrHash common.Hash, root common.Hash) (bool, common.StorageSize, map[common.Hash][]byte, *trienode.NodeSet, error) { +// deleteStorage iterates the storage trie belongs to the account and mark all +// slots inside as deleted. +func (s *StateDB) deleteStorage(addr common.Address, addrHash common.Hash, root common.Hash) (bool, map[common.Hash][]byte, *trienode.NodeSet, error) { + start := time.Now() tr, err := s.db.OpenStorageTrie(s.originalRoot, addr, root) if err != nil { - return false, 0, nil, nil, fmt.Errorf("failed to open storage trie, err: %w", err) + return false, nil, nil, fmt.Errorf("failed to open storage trie, err: %w", err) } it, err := tr.NodeIterator(nil) if err != nil { - return false, 0, nil, nil, fmt.Errorf("failed to open storage iterator, err: %w", err) + return false, nil, nil, fmt.Errorf("failed to open storage iterator, err: %w", err) } var ( - size common.StorageSize - nodes = trienode.NewNodeSet(addrHash) - slots = make(map[common.Hash][]byte) + set = trienode.NewNodeSet(addrHash) + slots = make(map[common.Hash][]byte) + stateSize common.StorageSize + nodeSize common.StorageSize ) for it.Next(true) { - if size > storageDeleteLimit { - return true, size, nil, nil, nil + // arbitrary stateSize limit, make it configurable + if stateSize+nodeSize > 512*1024*1024 { + log.Info("Skip large storage deletion", "address", addr.Hex(), "states", stateSize, "nodes", nodeSize) + if metrics.EnabledExpensive { + slotDeletionSkip.Inc(1) + } + return true, nil, nil, nil } if it.Leaf() { slots[common.BytesToHash(it.LeafKey())] = common.CopyBytes(it.LeafBlob()) - size += common.StorageSize(common.HashLength + len(it.LeafBlob())) + stateSize += common.StorageSize(common.HashLength + len(it.LeafBlob())) continue } if it.Hash() == (common.Hash{}) { continue } - size += common.StorageSize(len(it.Path())) - nodes.AddNode(it.Path(), trienode.NewDeleted()) + nodeSize += common.StorageSize(len(it.Path())) + set.AddNode(it.Path(), trienode.NewDeleted()) } if err := it.Error(); err != nil { - return false, 0, nil, nil, err - } - return false, size, slots, nodes, nil -} - -// deleteStorage is designed to delete the storage trie of a designated account. -// It could potentially be terminated if the storage size is excessively large, -// potentially leading to an out-of-memory panic. The function will make an attempt -// to utilize an efficient strategy if the associated state snapshot is reachable; -// otherwise, it will resort to a less-efficient approach. -func (s *StateDB) deleteStorage(addr common.Address, addrHash common.Hash, root common.Hash) (bool, map[common.Hash][]byte, *trienode.NodeSet, error) { - var ( - start = time.Now() - err error - aborted bool - size common.StorageSize - slots map[common.Hash][]byte - nodes *trienode.NodeSet - ) - // The fast approach can be failed if the snapshot is not fully - // generated, or it's internally corrupted. Fallback to the slow - // one just in case. - if s.snap != nil { - aborted, size, slots, nodes, err = s.fastDeleteStorage(addrHash, root) - } - if s.snap == nil || err != nil { - aborted, size, slots, nodes, err = s.slowDeleteStorage(addr, addrHash, root) - } - if err != nil { return false, nil, nil, err } if metrics.EnabledExpensive { - if aborted { - slotDeletionSkip.Inc(1) + if int64(len(slots)) > slotDeletionMaxCount.Value() { + slotDeletionMaxCount.Update(int64(len(slots))) } - n := int64(len(slots)) - if n > slotDeletionMaxCount.Value() { - slotDeletionMaxCount.Update(n) - } - if int64(size) > slotDeletionMaxSize.Value() { - slotDeletionMaxSize.Update(int64(size)) + if int64(stateSize+nodeSize) > slotDeletionMaxSize.Value() { + slotDeletionMaxSize.Update(int64(stateSize + nodeSize)) } slotDeletionTimer.UpdateSince(start) - slotDeletionCount.Mark(n) - slotDeletionSize.Mark(int64(size)) + slotDeletionCount.Mark(int64(len(slots))) + slotDeletionSize.Mark(int64(stateSize + nodeSize)) } - return aborted, slots, nodes, nil + return false, slots, set, nil } // handleDestruction processes all destruction markers and deletes the account @@ -1140,13 +1063,7 @@ func (s *StateDB) deleteStorage(addr common.Address, addrHash common.Hash, root // In case (d), **original** account along with its storages should be deleted, // with their values be tracked as original value. func (s *StateDB) handleDestruction(nodes *trienode.MergedNodeSet) (map[common.Address]struct{}, error) { - // Short circuit if geth is running with hash mode. This procedure can consume - // considerable time and storage deletion isn't supported in hash mode, thus - // preemptively avoiding unnecessary expenses. incomplete := make(map[common.Address]struct{}) - if s.db.TrieDB().Scheme() == rawdb.HashScheme { - return incomplete, nil - } for addr, prev := range s.stateObjectsDestruct { // The original account was non-existing, and it's marked as destructed // in the scope of block. It can be case (a) or (b). diff --git a/core/state/statedb_fuzz_test.go b/core/state/statedb_fuzz_test.go index 42381c34ab..aa95236621 100644 --- a/core/state/statedb_fuzz_test.go +++ b/core/state/statedb_fuzz_test.go @@ -31,12 +31,10 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" - "github.com/ethereum/go-ethereum/core/state/snapshot" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie" - "github.com/ethereum/go-ethereum/trie/triedb/pathdb" "github.com/ethereum/go-ethereum/trie/triestate" ) @@ -181,28 +179,16 @@ func (test *stateTest) run() bool { storageList = append(storageList, copy2DSet(states.Storages)) } disk = rawdb.NewMemoryDatabase() - tdb = trie.NewDatabase(disk, &trie.Config{OnCommit: onCommit, PathDB: pathdb.Defaults}) + tdb = trie.NewDatabase(disk, &trie.Config{OnCommit: onCommit}) sdb = NewDatabaseWithNodeDB(disk, tdb) byzantium = rand.Intn(2) == 0 ) - defer disk.Close() - defer tdb.Close() - - var snaps *snapshot.Tree - if rand.Intn(3) == 0 { - snaps, _ = snapshot.New(snapshot.Config{ - CacheSize: 1, - Recovery: false, - NoBuild: false, - AsyncBuild: false, - }, disk, tdb, types.EmptyRootHash) - } for i, actions := range test.actions { root := types.EmptyRootHash if i != 0 { root = roots[len(roots)-1] } - state, err := New(root, sdb, snaps) + state, err := New(root, sdb, nil) if err != nil { panic(err) } diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index ad829a0c8f..869410ff43 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -39,8 +39,6 @@ import ( "github.com/ethereum/go-ethereum/trie" "github.com/ethereum/go-ethereum/trie/triedb/hashdb" "github.com/ethereum/go-ethereum/trie/triedb/pathdb" - "github.com/ethereum/go-ethereum/trie/trienode" - "github.com/holiman/uint256" ) // Tests that updating a state trie does not leak any database writes prior to @@ -1137,57 +1135,3 @@ func TestResetObject(t *testing.T) { t.Fatalf("Unexpected storage slot value %v", slot) } } - -func TestDeleteStorage(t *testing.T) { - var ( - disk = rawdb.NewMemoryDatabase() - tdb = trie.NewDatabase(disk, nil) - db = NewDatabaseWithNodeDB(disk, tdb) - snaps, _ = snapshot.New(snapshot.Config{CacheSize: 10}, disk, tdb, types.EmptyRootHash) - state, _ = New(types.EmptyRootHash, db, snaps) - addr = common.HexToAddress("0x1") - ) - // Initialize account and populate storage - state.SetBalance(addr, big.NewInt(1)) - state.CreateAccount(addr) - for i := 0; i < 1000; i++ { - slot := common.Hash(uint256.NewInt(uint64(i)).Bytes32()) - value := common.Hash(uint256.NewInt(uint64(10 * i)).Bytes32()) - state.SetState(addr, slot, value) - } - root, _ := state.Commit(0, true) - // Init phase done, create two states, one with snap and one without - fastState, _ := New(root, db, snaps) - slowState, _ := New(root, db, nil) - - obj := fastState.GetOrNewStateObject(addr) - storageRoot := obj.data.Root - - _, _, fastNodes, err := fastState.deleteStorage(addr, crypto.Keccak256Hash(addr[:]), storageRoot) - if err != nil { - t.Fatal(err) - } - - _, _, slowNodes, err := slowState.deleteStorage(addr, crypto.Keccak256Hash(addr[:]), storageRoot) - if err != nil { - t.Fatal(err) - } - check := func(set *trienode.NodeSet) string { - var a []string - set.ForEachWithOrder(func(path string, n *trienode.Node) { - if n.Hash != (common.Hash{}) { - t.Fatal("delete should have empty hashes") - } - if len(n.Blob) != 0 { - t.Fatal("delete should have have empty blobs") - } - a = append(a, fmt.Sprintf("%x", path)) - }) - return strings.Join(a, ",") - } - slowRes := check(slowNodes) - fastRes := check(fastNodes) - if slowRes != fastRes { - t.Fatalf("difference found:\nfast: %v\nslow: %v\n", fastRes, slowRes) - } -}