-
Notifications
You must be signed in to change notification settings - Fork 817
Cache subnetIDs in cachedState #4402
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
Conversation
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.
Pull Request Overview
Adds an LRU cache for subnet IDs keyed by blockchain ID to avoid repeated underlying (potentially gRPC) lookups and renames the existing validator set cache field for clarity. Also introduces unit tests validating cache population, hits, and error handling paths.
- Introduces subnetIDsCache (LRU) with configurable size.
- Renames internal validator set cache field and adds GetSubnetID caching logic.
- Adds comprehensive test for subnet ID caching behavior including error path.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| snow/validators/state.go | Adds subnet ID cache, renames validator set cache field, implements cached GetSubnetID. |
| snow/validators/state_test.go | Adds unit test covering subnet ID cache miss/hit and error handling behavior. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Michael Kaplan <[email protected]>
|
@StephenButtolph called out that this does not actually improve the worst case verification times/cost for flows dependent on It's strictly a best case performance optimization, but should not be relied upon for gas price calculations. |
…kaplan13/cached-subnet-ids
Why this should be merged
Optimizes look ups of the subnet ID for a given blockchain ID, particularly for chains with
validatorStateinstances that would otherwise require going over grpc for the look up each time (i.e.subnet-evminstances).See discussion here.
How this works
Uses an LRU cache for subnet IDs for given blockchain IDs.
How this was tested
Unit tests
Need to be documented in RELEASES.md?
No