refactor: generalize state repositories #8732
Open
Conversation
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
Contributor
Summary of ChangesHello @twoeths, 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 focuses on generalizing state repositories to work with raw byte arrays instead of specific BeaconState types. This change facilitates future migrations to a more generic state view and improves error detection by enforcing repository method usage at compile time. The updates involve creating a 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request effectively generalizes the state repositories (StateArchiveRepository and CheckpointStateRepository) by introducing a BinaryRepository base class. This decouples them from BeaconStateAllForks and allows them to work with raw Uint8Array data, which is a great improvement for flexibility and compile-time type safety. The refactoring in abstractRepository.ts is clean and separates concerns well. The consumer code has been updated accordingly, for instance by removing initStateFromDb and adjusting how states are retrieved. I have one suggestion to improve the type safety and readability of the decodeKey implementation in StateArchiveRepository.
twoeths
commented
Jan 9, 2026
| /** | ||
| * Restore the latest beacon state from db | ||
| */ | ||
| export async function initStateFromDb( |
Contributor
Author
There was a problem hiding this comment.
remove unused function
twoeths
commented
Jan 9, 2026
| // returning 1 per 100s of states that are persisted in the archive state is not useful enough | ||
| // and it causes consumers having to loadState and createCachedBeaconState again | ||
| // if state is finalized, consumers need to use getHistoricalStateBySlot() api instead | ||
| return null; |
Contributor
Author
There was a problem hiding this comment.
this is to simplify code, same to https://github.com/ChainSafe/lodestar/pull/8728/changes#diff-29e6daed96db2b885b1c4f516eb55bf7e6916e298d50f78bd2f8c57811481d2fR590
twoeths
commented
Jan 9, 2026
|
|
||
| const state = states[0]; | ||
| const stateBytes = stateBytesArr[0]; | ||
| const state = getStateTypeFromBytes(config, stateBytes).deserializeToViewDU(stateBytes); |
Contributor
Author
There was a problem hiding this comment.
this code make it ready for us to migrate to BeaconStateView later
Contributor
Performance Report✔️ no performance regression detected Full benchmark results
|
twoeths
commented
Jan 9, 2026
| import {getRootIndex, getRootIndexKey, storeRootIndex} from "./stateArchiveIndex.js"; | ||
|
|
||
| export class StateArchiveRepository extends Repository<Slot, BeaconStateAllForks> { | ||
| export type BeaconStateArchive = { |
Contributor
Author
There was a problem hiding this comment.
generic type to work with the current CachedBeaconStateAllForks and the future BeaconStateView
twoeths
commented
Jan 9, 2026
| * By default, SSZ-encoded values, | ||
| * indexed by root | ||
| */ | ||
| export abstract class Repository<I extends Id, T> extends BinaryRepository<I> { |
Contributor
Author
There was a problem hiding this comment.
nothing change here, just a refactor to move methods without value (T) to BinaryRepository
no need to change anything at the consumer side
twoeths
changed the title
nflaig
reviewed
Jan 22, 2026
| // returning 1 per 100s of states that are persisted in the archive state is not useful enough | ||
| // and it causes consumers having to loadState and createCachedBeaconState again | ||
| // if state is finalized, consumers need to use getHistoricalStateBySlot() api instead | ||
| return null; |
nflaig
reviewed
Jan 22, 2026
| optsBuff.lt = this.maxKey; | ||
| } | ||
|
|
||
| // Set at least on max key |
Member
There was a problem hiding this comment.
Suggested change
| // Set at least on max key | |
| // Set at least one max key |
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.
Motivation
BeaconStateAllForkswhich make it hard when we migrate to a generic BeaconStateView (to be come later, see Decouple beacon-node and state-transition #8650)Description
BinaryRepositoryStateArchiveRepository + CheckpointStateRepositoryto be extended fromBinaryRepositoryBinaryRepository, it's a compile time check, vs calling methods inRepositoryand throw runtime error which make it harder to detect errorsCloses #8729