Bug/is 1247#2309
Open
olehnikolaiev wants to merge 36 commits into
Open
Conversation
|
SUGGESTIONS BEFORE MERGE:
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements a global LRU cache for storage key-value pairs to reduce database read operations and adds improved error handling for database operations. The changes address internal-support issue #1247.
Key changes:
- Adds a global LRU cache (100K entries) for storage operations to avoid redundant database reads
- Enhances error handling with a catch-all exception handler for database commits
- Protects database deletion operations with proper locking
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| libdevcore/LruCache.h | Enhanced LRU cache implementation with custom hash support, insertOrUpdate method, and std::any-based get method |
| libskale/State.h | Added StorageKey type, custom hash function, and global LRU cache static member declaration with required includes |
| libskale/State.cpp | Initialized global cache, integrated cache population on commit, and added cache lookup in storage retrieval path |
| libskale/OverlayDB.cpp | Added catch-all exception handler for database commit failures |
| libdevcore/LevelDB.cpp | Added locking protection around Delete() operations to prevent concurrent access issues |
| libethereum/ClientBase.cpp | Changed to use read-only block copy for gas estimation |
| libethereum/Block.cpp | Removed debugging state copy operation |
| libevm/LegacyVM.h | Added trace logger member for VM operations |
| libevm/LegacyVM.cpp | Fixed braces formatting in updateSSGasPreEIP1283 method |
| libethereum/Executive.cpp | Fixed braces formatting in go method |
| deps/build.sh | Pinned double-conversion to specific commit hash |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PropzSaladaz
approved these changes
Oct 29, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes https://github.com/skalenetwork/internal-support/issues/1247
during catchup skaled produces 5-10 blocks per second. this produces a lot of writes to level-0 files in LevelDB, triggering compaction. because we use sync=false and background compaction, it is possible that during the read operation LevelDB returns stale key value from lower level, while higher levels are being compacted.
the bug is most likely to reproduce during catchup of a big batch of blocks on a chain with high average txns count.
in the proposed solution a new write-cache is added to State class. when a new key-value storage pair is committed into the database, it's added into the cache as well (or the cache is updated with a new value for a given key). when a contract's storage is read, the cache is checked first before making a request to the db.
only State instance with write rights has access to this cache - readonly copies always read the data from the database's snapshot.