Conversation
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Summary of ChangesHello @GrapeBaBa, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a SlashingsCache to optimize checking for slashed validators, which is a commendable performance enhancement. The implementation using a DynamicBitSet is efficient, and the cache is well-integrated into the block processing logic. I've identified some opportunities to reduce code duplication in the cache initialization logic, which will improve maintainability. Additionally, there's a minor issue with an import path that could lead to a compilation error. Overall, this is a solid contribution.
| _ = @import("utils/pubkey_index_map.zig"); | ||
| _ = @import("utils/reference_count.zig"); | ||
| _ = @import("utils/root_cache.zig"); | ||
| _ = @import("cache/RootCache.zig"); |
There was a problem hiding this comment.
The import path cache/RootCache.zig seems to have a typo. According to the repository's style guide, filenames should be in snake_case. The file is named root_cache.zig, so the import path should reflect that. This typo will likely cause a compilation error.
_ = @import("cache/root_cache.zig");
References
- The style guide specifies that file names should use
snake_case. The importcache/RootCache.zigusesCamelCase, which violates this rule. (link)
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a cached view of slashed validators (SlashingsCache) and threads it through block/operation processing so that slashing-related checks can avoid repeatedly reading from the SSZ tree, while staying consistent with the beacon state.
Changes:
- Add
SlashingsCachewith initialization, cloning, and incremental update APIs, and attach it toCachedBeaconState. - Wire the cache through block processing (
processBlock,processOperations, proposer/attester slashing, attestations) and update altair attestation processing to use the cache for unslashed-balance accounting. - Update tests, spec runners, and benchmarks to pass and initialize the new cache where needed, and expose a helper to build the cache from a state.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
test/spec/runner/operations.zig |
Passes cached_state.slashings_cache into attestation, attester slashing, and proposer slashing spec test runners to exercise the new cache-aware APIs. |
src/state_transition/state_transition.zig |
Extends the main stateTransition block path to pass post_cached_state.slashings_cache into processBlock, making the cache part of normal block processing. |
src/state_transition/root.zig |
Re-exports buildSlashingsCacheFromStateIfNeeded so callers (e.g., benchmarks) can initialize the cache from an arbitrary state. |
src/state_transition/cache/state_cache.zig |
Adds a SlashingsCache field to CachedBeaconState, wires it through creation/cloning/deinit, and exposes helper methods to query/update the cache. |
src/state_transition/cache/slashings_cache.zig |
Introduces SlashingsCache (bitset of slashed validators keyed by latest block slot) and a buildFromStateIfNeeded helper to rebuild the cache from the state’s validators when required. |
src/state_transition/block/slash_validator.zig |
Extends slashValidator to accept a SlashingsCache pointer and record a validator slashing in the cache in sync with the state’s slashed flag. |
src/state_transition/block/process_proposer_slashing.zig |
Adds allocator and SlashingsCache parameters, builds the cache on demand, and calls slashValidator with the cache. |
src/state_transition/block/process_operations.zig |
Threads SlashingsCache through processOperations and down into proposer/attester slashing and attestation processing. |
src/state_transition/block/process_block.zig |
Makes processBlock cache-aware: builds the slashings cache once per state header when needed, keeps its slot in sync, and forwards it into processOperations. |
src/state_transition/block/process_attester_slashing.zig |
Adds SlashingsCache usage, ensuring the cache is built and updated when slashing validators from attester slashings. |
src/state_transition/block/process_attestations.zig |
Extends the API with a SlashingsCache pointer and ensures the cache is initialized before delegating to phase0/altair-specific attestation logic. |
src/state_transition/block/process_attestation_altair.zig |
Uses SlashingsCache.isSlashed instead of reading the slashed flag from the tree when updating target unslashed balance increments, reducing tree reads. |
bench/state_transition/process_block.zig |
Updates block/operations benchmarks to pass a SlashingsCache and builds it once from the benchmark state before running bench cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
**Motivation** This PR introduces a slashing cache which to eliminate the slash status checking hotspot during the process attestation. It has been tested which could reduce a lot of time cost using `bench_process_block`. Refer to the lighthouse https://github.com/sigp/lighthouse/blob/stable/consensus/types/src/beacon_state.rs#L704. Fix #179 --------- Signed-off-by: Chen Kai <281165273grape@gmail.com> Co-authored-by: Cayman <caymannava@gmail.com>
Motivation
This PR introduces a slashing cache which to eliminate the slash status checking hotspot during the process attestation. It has been tested which could reduce a lot of time cost using
bench_process_block. Refer to the lighthouse https://github.com/sigp/lighthouse/blob/stable/consensus/types/src/beacon_state.rs#L704.Fix #179