-
Notifications
You must be signed in to change notification settings - Fork 863
Composite State Store Part 2: EVM database implementation #2755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/evm-ss-router-config
Are you sure you want to change the base?
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feature/evm-ss-router-config #2755 +/- ##
================================================================
- Coverage 41.86% 41.84% -0.02%
================================================================
Files 1022 1022
Lines 84784 84784
================================================================
- Hits 35492 35479 -13
- Misses 45950 45957 +7
- Partials 3342 3348 +6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| for keyStr, value := range keyValues { | ||
| if value != nil { // Skip tombstones | ||
| keys = append(keys, []byte(keyStr)) | ||
| values = append(values, value) | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for storeType, pairs := range changes { | ||
| db := s.databases[storeType] | ||
| if db == nil { | ||
| continue | ||
| } | ||
| if err := db.ApplyBatch(pairs, version); err != nil { | ||
| return fmt.Errorf("failed to apply batch for %s: %w", StoreTypeName(storeType), err) | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for storeType, pairs := range changes { | ||
| db := s.databases[storeType] | ||
| if db == nil { | ||
| continue | ||
| } | ||
|
|
||
| wg.Add(1) | ||
| go func(db *EVMDatabase, pairs []*iavl.KVPair) { | ||
| defer wg.Done() | ||
| if err := db.ApplyBatch(pairs, version); err != nil { | ||
| errCh <- err | ||
| } | ||
| }(db, pairs) | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for _, db := range s.databases { | ||
| if v := db.GetLatestVersion(); v > maxVersion { | ||
| maxVersion = v | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for _, db := range s.databases { | ||
| if err := db.SetLatestVersion(version); err != nil { | ||
| return err | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for _, db := range s.databases { | ||
| v := db.GetEarliestVersion() | ||
| if minVersion < 0 || v < minVersion { | ||
| minVersion = v | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for _, db := range s.databases { | ||
| if err := db.SetEarliestVersion(version); err != nil { | ||
| return err | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for _, db := range s.databases { | ||
| if err := db.Prune(version); err != nil { | ||
| return err | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for _, db := range s.databases { | ||
| if err := db.Close(); err != nil { | ||
| lastErr = err | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| go func(db *EVMDatabase, pairs []*iavl.KVPair) { | ||
| defer wg.Done() | ||
| if err := db.ApplyBatch(pairs, version); err != nil { | ||
| errCh <- err | ||
| } | ||
| }(db, pairs) |
Check notice
Code scanning / CodeQL
Spawning a Go routine Note
Adds the core EVM state store components: - EVMDatabase: Single PebbleDB with DefaultComparer for a specific EVM data type - EVMStateStore: Manages multiple EVMDatabase instances (nonce, code, codehash, codesize, storage) - Versioned key encoding with big-endian version suffix - ApplyBatch and ApplyChangesetParallel for efficient writes - EVMIterator for version-aware iteration over unique keys Uses commonevm.ParseEVMKey directly for key parsing (no router abstraction).
ac83d2f to
0935d03
Compare
7271005 to
7ef1b00
Compare
| func OpenEVMDB(dir string, storeType EVMStoreType) (*EVMDatabase, error) { | ||
| opts := &pebble.Options{ | ||
| Comparer: pebble.DefaultComparer, | ||
| MaxConcurrentCompactions: func() int { return 2 }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 2 enough or shall we bump to numCPU?
| func (db *EVMDatabase) SetLatestVersion(version int64) error { | ||
| buf := make([]byte, 8) | ||
| binary.BigEndian.PutUint64(buf, uint64(version)) | ||
| if err := db.storage.Set([]byte(latestVersionKey), buf, syncWriteOpts); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really wanna sync here? This is gonna trigger fsync which could impact performance
| } | ||
|
|
||
| // EVMStateStore manages multiple EVMDatabase instances, one per EVM data type | ||
| type EVMStateStore struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be moved to a separate file? I feel it would be cleaner if we keep db.go only containing the db related logic. And EVMStateStore seems to be a layer above that, since it needs to manage multiple dbs
| } | ||
|
|
||
| // Prune removes old versions from all databases | ||
| func (s *EVMStateStore) Prune(version int64) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we parallelize this or have each DB run the pruning in the background?
Describe your changes and provide context
Testing performed to validate your change