diff --git a/core/blockchain_reader.go b/core/blockchain_reader.go index 8f1da82a2b..cf72e98cb2 100644 --- a/core/blockchain_reader.go +++ b/core/blockchain_reader.go @@ -344,10 +344,7 @@ func (bc *BlockChain) stateRecoverable(root common.Hash) bool { // ContractCodeWithPrefix retrieves a blob of data associated with a contract // hash either from ephemeral in-memory cache, or from persistent storage. -// -// If the code doesn't exist in the in-memory cache, check the storage with -// new code scheme. -func (bc *BlockChain) ContractCodeWithPrefix(hash common.Hash) ([]byte, error) { +func (bc *BlockChain) ContractCodeWithPrefix(hash common.Hash) []byte { // TODO(rjl493456442) The associated account address is also required // in Verkle scheme. Fix it once snap-sync is supported for Verkle. return bc.statedb.ContractCodeWithPrefix(common.Address{}, hash) diff --git a/core/state/database.go b/core/state/database.go index 0d8acec35a..fff7f1519f 100644 --- a/core/state/database.go +++ b/core/state/database.go @@ -17,7 +17,6 @@ package state import ( - "errors" "fmt" "github.com/ethereum/go-ethereum/common" @@ -55,12 +54,6 @@ type Database interface { // OpenStorageTrie opens the storage trie of an account. OpenStorageTrie(stateRoot common.Hash, address common.Address, root common.Hash, trie Trie) (Trie, error) - // ContractCode retrieves a particular contract's code. - ContractCode(addr common.Address, codeHash common.Hash) ([]byte, error) - - // ContractCodeSize retrieves a particular contracts code's size. - ContractCodeSize(addr common.Address, codeHash common.Hash) (int, error) - // PointCache returns the cache holding points used in verkle tree key computation PointCache() *utils.PointCache @@ -180,7 +173,7 @@ func NewDatabaseForTesting() *CachingDB { // Reader returns a state reader associated with the specified state root. func (db *CachingDB) Reader(stateRoot common.Hash) (Reader, error) { - var readers []Reader + var readers []StateReader // Set up the state snapshot reader if available. This feature // is optional and may be partially useful if it's not fully @@ -188,7 +181,7 @@ func (db *CachingDB) Reader(stateRoot common.Hash) (Reader, error) { if db.snap != nil { snap := db.snap.Snapshot(stateRoot) if snap != nil { - readers = append(readers, newStateReader(snap)) // snap reader is optional + readers = append(readers, newFlatReader(snap)) } } // Set up the trie reader, which is expected to always be available @@ -199,7 +192,11 @@ func (db *CachingDB) Reader(stateRoot common.Hash) (Reader, error) { } readers = append(readers, tr) - return newMultiReader(readers...) + combined, err := newMultiStateReader(readers...) + if err != nil { + return nil, err + } + return newReader(newCachingCodeReader(db.disk, db.codeCache, db.codeSizeCache), combined), nil } // OpenTrie opens the main account trie at a specific root hash. @@ -229,45 +226,20 @@ func (db *CachingDB) OpenStorageTrie(stateRoot common.Hash, address common.Addre return tr, nil } -// ContractCode retrieves a particular contract's code. -func (db *CachingDB) ContractCode(address common.Address, codeHash common.Hash) ([]byte, error) { - code, _ := db.codeCache.Get(codeHash) - if len(code) > 0 { - return code, nil - } - code = rawdb.ReadCode(db.disk, codeHash) - if len(code) > 0 { - db.codeCache.Add(codeHash, code) - db.codeSizeCache.Add(codeHash, len(code)) - return code, nil - } - return nil, errors.New("not found") -} - // ContractCodeWithPrefix retrieves a particular contract's code. If the // code can't be found in the cache, then check the existence with **new** // db scheme. -func (db *CachingDB) ContractCodeWithPrefix(address common.Address, codeHash common.Hash) ([]byte, error) { +func (db *CachingDB) ContractCodeWithPrefix(address common.Address, codeHash common.Hash) []byte { code, _ := db.codeCache.Get(codeHash) if len(code) > 0 { - return code, nil + return code } code = rawdb.ReadCodeWithPrefix(db.disk, codeHash) if len(code) > 0 { db.codeCache.Add(codeHash, code) db.codeSizeCache.Add(codeHash, len(code)) - return code, nil } - return nil, errors.New("not found") -} - -// ContractCodeSize retrieves a particular contracts code's size. -func (db *CachingDB) ContractCodeSize(addr common.Address, codeHash common.Hash) (int, error) { - if cached, ok := db.codeSizeCache.Get(codeHash); ok { - return cached, nil - } - code, err := db.ContractCode(addr, codeHash) - return len(code), err + return code } // TrieDB retrieves any intermediate trie-node caching layer. diff --git a/core/state/iterator.go b/core/state/iterator.go index 83c552ca1a..5ea52c6183 100644 --- a/core/state/iterator.go +++ b/core/state/iterator.go @@ -136,10 +136,13 @@ func (it *nodeIterator) step() error { } if !bytes.Equal(account.CodeHash, types.EmptyCodeHash.Bytes()) { it.codeHash = common.BytesToHash(account.CodeHash) - it.code, err = it.state.db.ContractCode(address, common.BytesToHash(account.CodeHash)) + it.code, err = it.state.reader.Code(address, common.BytesToHash(account.CodeHash)) if err != nil { return fmt.Errorf("code %x: %v", account.CodeHash, err) } + if len(it.code) == 0 { + return fmt.Errorf("code is not found: %x", account.CodeHash) + } } it.accountHash = it.stateIt.Parent() return nil diff --git a/core/state/reader.go b/core/state/reader.go index 85842adde8..a0f15dfcc8 100644 --- a/core/state/reader.go +++ b/core/state/reader.go @@ -18,11 +18,13 @@ package state import ( "errors" - "maps" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/lru" + "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie" "github.com/ethereum/go-ethereum/trie/utils" @@ -30,9 +32,26 @@ import ( "github.com/ethereum/go-ethereum/triedb/database" ) -// Reader defines the interface for accessing accounts and storage slots +// ContractCodeReader defines the interface for accessing contract code. +type ContractCodeReader interface { + // Code retrieves a particular contract's code. + // + // - Returns nil code along with nil error if the requested contract code + // doesn't exist + // - Returns an error only if an unexpected issue occurs + Code(addr common.Address, codeHash common.Hash) ([]byte, error) + + // CodeSize retrieves a particular contracts code's size. + // + // - Returns zero code size along with nil error if the requested contract code + // doesn't exist + // - Returns an error only if an unexpected issue occurs + CodeSize(addr common.Address, codeHash common.Hash) (int, error) +} + +// StateReader defines the interface for accessing accounts and storage slots // associated with a specific state. -type Reader interface { +type StateReader interface { // Account retrieves the account associated with a particular address. // // - Returns a nil account if it does not exist @@ -47,32 +66,84 @@ type Reader interface { // - Returns an error only if an unexpected issue occurs // - The returned storage slot is safe to modify after the call Storage(addr common.Address, slot common.Hash) (common.Hash, error) - - // Copy returns a deep-copied state reader. - Copy() Reader } -// stateReader wraps a database state reader. -type stateReader struct { +// Reader defines the interface for accessing accounts, storage slots and contract +// code associated with a specific state. +type Reader interface { + ContractCodeReader + StateReader +} + +// cachingCodeReader implements ContractCodeReader, accessing contract code either in +// local key-value store or the shared code cache. +type cachingCodeReader struct { + db ethdb.KeyValueReader + + // These caches could be shared by multiple code reader instances, + // they are natively thread-safe. + codeCache *lru.SizeConstrainedCache[common.Hash, []byte] + codeSizeCache *lru.Cache[common.Hash, int] +} + +// newCachingCodeReader constructs the code reader. +func newCachingCodeReader(db ethdb.KeyValueReader, codeCache *lru.SizeConstrainedCache[common.Hash, []byte], codeSizeCache *lru.Cache[common.Hash, int]) *cachingCodeReader { + return &cachingCodeReader{ + db: db, + codeCache: codeCache, + codeSizeCache: codeSizeCache, + } +} + +// Code implements ContractCodeReader, retrieving a particular contract's code. +// If the contract code doesn't exist, no error will be returned. +func (r *cachingCodeReader) Code(addr common.Address, codeHash common.Hash) ([]byte, error) { + code, _ := r.codeCache.Get(codeHash) + if len(code) > 0 { + return code, nil + } + code = rawdb.ReadCode(r.db, codeHash) + if len(code) > 0 { + r.codeCache.Add(codeHash, code) + r.codeSizeCache.Add(codeHash, len(code)) + } + return code, nil +} + +// CodeSize implements ContractCodeReader, retrieving a particular contracts code's size. +// If the contract code doesn't exist, no error will be returned. +func (r *cachingCodeReader) CodeSize(addr common.Address, codeHash common.Hash) (int, error) { + if cached, ok := r.codeSizeCache.Get(codeHash); ok { + return cached, nil + } + code, err := r.Code(addr, codeHash) + if err != nil { + return 0, err + } + return len(code), nil +} + +// flatReader wraps a database state reader. +type flatReader struct { reader database.StateReader buff crypto.KeccakState } -// newStateReader constructs a state reader with on the given state root. -func newStateReader(reader database.StateReader) *stateReader { - return &stateReader{ +// newFlatReader constructs a state reader with on the given state root. +func newFlatReader(reader database.StateReader) *flatReader { + return &flatReader{ reader: reader, buff: crypto.NewKeccakState(), } } -// Account implements Reader, retrieving the account specified by the address. +// Account implements StateReader, retrieving the account specified by the address. // // An error will be returned if the associated snapshot is already stale or // the requested account is not yet covered by the snapshot. // // The returned account might be nil if it's not existent. -func (r *stateReader) Account(addr common.Address) (*types.StateAccount, error) { +func (r *flatReader) Account(addr common.Address) (*types.StateAccount, error) { account, err := r.reader.Account(crypto.HashData(r.buff, addr.Bytes())) if err != nil { return nil, err @@ -95,14 +166,14 @@ func (r *stateReader) Account(addr common.Address) (*types.StateAccount, error) return acct, nil } -// Storage implements Reader, retrieving the storage slot specified by the +// Storage implements StateReader, retrieving the storage slot specified by the // address and slot key. // // An error will be returned if the associated snapshot is already stale or // the requested storage slot is not yet covered by the snapshot. // // The returned storage slot might be empty if it's not existent. -func (r *stateReader) Storage(addr common.Address, key common.Hash) (common.Hash, error) { +func (r *flatReader) Storage(addr common.Address, key common.Hash) (common.Hash, error) { addrHash := crypto.HashData(r.buff, addr.Bytes()) slotHash := crypto.HashData(r.buff, key.Bytes()) ret, err := r.reader.Storage(addrHash, slotHash) @@ -123,15 +194,7 @@ func (r *stateReader) Storage(addr common.Address, key common.Hash) (common.Hash return value, nil } -// Copy implements Reader, returning a deep-copied snap reader. -func (r *stateReader) Copy() Reader { - return &stateReader{ - reader: r.reader, - buff: crypto.NewKeccakState(), - } -} - -// trieReader implements the Reader interface, providing functions to access +// trieReader implements the StateReader interface, providing functions to access // state from the referenced trie. type trieReader struct { root common.Hash // State root which uniquely represent a state @@ -167,7 +230,7 @@ func newTrieReader(root common.Hash, db *triedb.Database, cache *utils.PointCach }, nil } -// Account implements Reader, retrieving the account specified by the address. +// Account implements StateReader, retrieving the account specified by the address. // // An error will be returned if the trie state is corrupted. An nil account // will be returned if it's not existent in the trie. @@ -184,7 +247,7 @@ func (r *trieReader) Account(addr common.Address) (*types.StateAccount, error) { return account, nil } -// Storage implements Reader, retrieving the storage slot specified by the +// Storage implements StateReader, retrieving the storage slot specified by the // address and slot key. // // An error will be returned if the trie state is corrupted. An empty storage @@ -227,48 +290,32 @@ func (r *trieReader) Storage(addr common.Address, key common.Hash) (common.Hash, return value, nil } -// Copy implements Reader, returning a deep-copied trie reader. -func (r *trieReader) Copy() Reader { - tries := make(map[common.Address]Trie) - for addr, tr := range r.subTries { - tries[addr] = mustCopyTrie(tr) - } - return &trieReader{ - root: r.root, - db: r.db, - buff: crypto.NewKeccakState(), - mainTrie: mustCopyTrie(r.mainTrie), - subRoots: maps.Clone(r.subRoots), - subTries: tries, - } +// multiStateReader is the aggregation of a list of StateReader interface, +// providing state access by leveraging all readers. The checking priority +// is determined by the position in the reader list. +type multiStateReader struct { + readers []StateReader // List of state readers, sorted by checking priority } -// multiReader is the aggregation of a list of Reader interface, providing state -// access by leveraging all readers. The checking priority is determined by the -// position in the reader list. -type multiReader struct { - readers []Reader // List of readers, sorted by checking priority -} - -// newMultiReader constructs a multiReader instance with the given readers. The -// priority among readers is assumed to be sorted. Note, it must contain at least -// one reader for constructing a multiReader. -func newMultiReader(readers ...Reader) (*multiReader, error) { +// newMultiStateReader constructs a multiStateReader instance with the given +// readers. The priority among readers is assumed to be sorted. Note, it must +// contain at least one reader for constructing a multiStateReader. +func newMultiStateReader(readers ...StateReader) (*multiStateReader, error) { if len(readers) == 0 { return nil, errors.New("empty reader set") } - return &multiReader{ + return &multiStateReader{ readers: readers, }, nil } -// Account implementing Reader interface, retrieving the account associated with -// a particular address. +// Account implementing StateReader interface, retrieving the account associated +// with a particular address. // // - Returns a nil account if it does not exist // - Returns an error only if an unexpected issue occurs // - The returned account is safe to modify after the call -func (r *multiReader) Account(addr common.Address) (*types.StateAccount, error) { +func (r *multiStateReader) Account(addr common.Address) (*types.StateAccount, error) { var errs []error for _, reader := range r.readers { acct, err := reader.Account(addr) @@ -280,13 +327,13 @@ func (r *multiReader) Account(addr common.Address) (*types.StateAccount, error) return nil, errors.Join(errs...) } -// Storage implementing Reader interface, retrieving the storage slot associated -// with a particular account address and slot key. +// Storage implementing StateReader interface, retrieving the storage slot +// associated with a particular account address and slot key. // // - Returns an empty slot if it does not exist // - Returns an error only if an unexpected issue occurs // - The returned storage slot is safe to modify after the call -func (r *multiReader) Storage(addr common.Address, slot common.Hash) (common.Hash, error) { +func (r *multiStateReader) Storage(addr common.Address, slot common.Hash) (common.Hash, error) { var errs []error for _, reader := range r.readers { slot, err := reader.Storage(addr, slot) @@ -298,11 +345,16 @@ func (r *multiReader) Storage(addr common.Address, slot common.Hash) (common.Has return common.Hash{}, errors.Join(errs...) } -// Copy implementing Reader interface, returning a deep-copied state reader. -func (r *multiReader) Copy() Reader { - var readers []Reader - for _, reader := range r.readers { - readers = append(readers, reader.Copy()) - } - return &multiReader{readers: readers} +// reader is the wrapper of ContractCodeReader and StateReader interface. +type reader struct { + ContractCodeReader + StateReader +} + +// newReader constructs a reader with the supplied code reader and state reader. +func newReader(codeReader ContractCodeReader, stateReader StateReader) *reader { + return &reader{ + ContractCodeReader: codeReader, + StateReader: stateReader, + } } diff --git a/core/state/state_object.go b/core/state/state_object.go index b659bf7ff2..2d542e5005 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -510,10 +510,13 @@ func (s *stateObject) Code() []byte { if bytes.Equal(s.CodeHash(), types.EmptyCodeHash.Bytes()) { return nil } - code, err := s.db.db.ContractCode(s.address, common.BytesToHash(s.CodeHash())) + code, err := s.db.reader.Code(s.address, common.BytesToHash(s.CodeHash())) if err != nil { s.db.setError(fmt.Errorf("can't load code hash %x: %v", s.CodeHash(), err)) } + if len(code) == 0 { + s.db.setError(fmt.Errorf("code is not found %x", s.CodeHash())) + } s.code = code return code } @@ -528,10 +531,13 @@ func (s *stateObject) CodeSize() int { if bytes.Equal(s.CodeHash(), types.EmptyCodeHash.Bytes()) { return 0 } - size, err := s.db.db.ContractCodeSize(s.address, common.BytesToHash(s.CodeHash())) + size, err := s.db.reader.CodeSize(s.address, common.BytesToHash(s.CodeHash())) if err != nil { s.db.setError(fmt.Errorf("can't load code size %x: %v", s.CodeHash(), err)) } + if size == 0 { + s.db.setError(fmt.Errorf("code is not found %x", s.CodeHash())) + } return size } diff --git a/core/state/statedb.go b/core/state/statedb.go index 9cc91c9332..b0603db7f0 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -650,10 +650,11 @@ func (s *StateDB) CreateContract(addr common.Address) { // Snapshots of the copied state cannot be applied to the copy. func (s *StateDB) Copy() *StateDB { // Copy all the basic fields, initialize the memory ones + reader, _ := s.db.Reader(s.originalRoot) // impossible to fail state := &StateDB{ db: s.db, trie: mustCopyTrie(s.trie), - reader: s.reader.Copy(), + reader: reader, originalRoot: s.originalRoot, stateObjects: make(map[common.Address]*stateObject, len(s.stateObjects)), stateObjectsDestruct: make(map[common.Address]*stateObject, len(s.stateObjectsDestruct)), diff --git a/core/state/sync_test.go b/core/state/sync_test.go index b2c75e72fe..efa56f8860 100644 --- a/core/state/sync_test.go +++ b/core/state/sync_test.go @@ -210,14 +210,18 @@ func testIterativeStateSync(t *testing.T, count int, commit bool, bypath bool, s if err != nil { t.Fatalf("state is not existent, %#x", srcRoot) } + cReader, err := srcDb.Reader(srcRoot) + if err != nil { + t.Fatalf("state is not existent, %#x", srcRoot) + } for len(nodeElements)+len(codeElements) > 0 { var ( nodeResults = make([]trie.NodeSyncResult, len(nodeElements)) codeResults = make([]trie.CodeSyncResult, len(codeElements)) ) for i, element := range codeElements { - data, err := srcDb.ContractCode(common.Address{}, element.code) - if err != nil { + data, err := cReader.Code(common.Address{}, element.code) + if err != nil || len(data) == 0 { t.Fatalf("failed to retrieve contract bytecode for hash %x", element.code) } codeResults[i] = trie.CodeSyncResult{Hash: element.code, Data: data} @@ -329,6 +333,10 @@ func testIterativeDelayedStateSync(t *testing.T, scheme string) { if err != nil { t.Fatalf("state is not existent, %#x", srcRoot) } + cReader, err := srcDb.Reader(srcRoot) + if err != nil { + t.Fatalf("state is not existent, %#x", srcRoot) + } for len(nodeElements)+len(codeElements) > 0 { // Sync only half of the scheduled nodes var nodeProcessed int @@ -336,8 +344,8 @@ func testIterativeDelayedStateSync(t *testing.T, scheme string) { if len(codeElements) > 0 { codeResults := make([]trie.CodeSyncResult, len(codeElements)/2+1) for i, element := range codeElements[:len(codeResults)] { - data, err := srcDb.ContractCode(common.Address{}, element.code) - if err != nil { + data, err := cReader.Code(common.Address{}, element.code) + if err != nil || len(data) == 0 { t.Fatalf("failed to retrieve contract bytecode for %x", element.code) } codeResults[i] = trie.CodeSyncResult{Hash: element.code, Data: data} @@ -433,13 +441,17 @@ func testIterativeRandomStateSync(t *testing.T, count int, scheme string) { if err != nil { t.Fatalf("state is not existent, %#x", srcRoot) } + cReader, err := srcDb.Reader(srcRoot) + if err != nil { + t.Fatalf("state is not existent, %#x", srcRoot) + } for len(nodeQueue)+len(codeQueue) > 0 { // Fetch all the queued nodes in a random order if len(codeQueue) > 0 { results := make([]trie.CodeSyncResult, 0, len(codeQueue)) for hash := range codeQueue { - data, err := srcDb.ContractCode(common.Address{}, hash) - if err != nil { + data, err := cReader.Code(common.Address{}, hash) + if err != nil || len(data) == 0 { t.Fatalf("failed to retrieve node data for %x", hash) } results = append(results, trie.CodeSyncResult{Hash: hash, Data: data}) @@ -526,6 +538,10 @@ func testIterativeRandomDelayedStateSync(t *testing.T, scheme string) { if err != nil { t.Fatalf("state is not existent, %#x", srcRoot) } + cReader, err := srcDb.Reader(srcRoot) + if err != nil { + t.Fatalf("state is not existent, %#x", srcRoot) + } for len(nodeQueue)+len(codeQueue) > 0 { // Sync only half of the scheduled nodes, even those in random order if len(codeQueue) > 0 { @@ -533,8 +549,8 @@ func testIterativeRandomDelayedStateSync(t *testing.T, scheme string) { for hash := range codeQueue { delete(codeQueue, hash) - data, err := srcDb.ContractCode(common.Address{}, hash) - if err != nil { + data, err := cReader.Code(common.Address{}, hash) + if err != nil || len(data) == 0 { t.Fatalf("failed to retrieve node data for %x", hash) } results = append(results, trie.CodeSyncResult{Hash: hash, Data: data}) @@ -631,6 +647,10 @@ func testIncompleteStateSync(t *testing.T, scheme string) { if err != nil { t.Fatalf("state is not available %x", srcRoot) } + cReader, err := srcDb.Reader(srcRoot) + if err != nil { + t.Fatalf("state is not existent, %#x", srcRoot) + } nodeQueue := make(map[string]stateElement) codeQueue := make(map[common.Hash]struct{}) paths, nodes, codes := sched.Missing(1) @@ -649,8 +669,8 @@ func testIncompleteStateSync(t *testing.T, scheme string) { if len(codeQueue) > 0 { results := make([]trie.CodeSyncResult, 0, len(codeQueue)) for hash := range codeQueue { - data, err := srcDb.ContractCode(common.Address{}, hash) - if err != nil { + data, err := cReader.Code(common.Address{}, hash) + if err != nil || len(data) == 0 { t.Fatalf("failed to retrieve node data for %x", hash) } results = append(results, trie.CodeSyncResult{Hash: hash, Data: data}) @@ -713,6 +733,11 @@ func testIncompleteStateSync(t *testing.T, scheme string) { // Sanity check that removing any node from the database is detected for _, node := range addedCodes { val := rawdb.ReadCode(dstDb, node) + if len(val) == 0 { + t.Logf("no code: %v", node) + } else { + t.Logf("has code: %v", node) + } rawdb.DeleteCode(dstDb, node) if err := checkStateConsistency(dstDb, ndb.Scheme(), srcRoot); err == nil { t.Errorf("trie inconsistency not caught, missing: %x", node) diff --git a/eth/protocols/snap/handler.go b/eth/protocols/snap/handler.go index d36f9621b1..8164973c20 100644 --- a/eth/protocols/snap/handler.go +++ b/eth/protocols/snap/handler.go @@ -454,7 +454,7 @@ func ServiceGetByteCodesQuery(chain *core.BlockChain, req *GetByteCodesPacket) [ // Peers should not request the empty code, but if they do, at // least sent them back a correct response without db lookups codes = append(codes, []byte{}) - } else if blob, err := chain.ContractCodeWithPrefix(hash); err == nil { + } else if blob := chain.ContractCodeWithPrefix(hash); len(blob) > 0 { codes = append(codes, blob) bytes += uint64(len(blob)) }