Conversation
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 significantly refactors the core state management within the beacon node by introducing a new abstraction layer, 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. Changelog
Activity
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 is a significant and well-executed refactoring to introduce and consume the IBeaconStateView interface throughout the beacon-node and fork-choice packages. This change replaces direct access to CachedBeaconState* properties with getter methods and encapsulates state-related logic within the new BeaconStateView class, leading to cleaner, more maintainable, and safer code by promoting immutability.
The changes are extensive but have been applied consistently across the codebase, including API implementations, chain logic, validation, and tests. The new abstraction is a great improvement. I've reviewed the changes carefully and found no issues. The refactoring appears to be sound and improves the overall architecture.
Performance Report✔️ no performance regression detected Full benchmark results
|
0c0f46a to
4b93e37
Compare
4b93e37 to
aab1e14
Compare
…rsion Re-implement two pieces of gloas-specific functionality that were inadvertently removed during the CachedBeaconStateAllForks → IBeaconStateView refactor: 1. Add `processExecutionPayloadEnvelope()` to `IBeaconStateView` interface and `BeaconStateView` implementation, delegating to the existing state-transition function with a fork guard. 2. Restore `computeEnvelopeStateRoot()` in `computeNewStateRoot.ts` using `IBeaconStateView` (no longer requires `CachedBeaconStateGloas` directly). Also restore `postState` in `computeNewStateRoot` return type. 3. Restore gloas block production block in `chain.ts` `produceBlockWrapper`: computes `envelopeStateRoot` and populates `ProduceFullGloas.envelopeStateRoot` for self-built blocks. 4. Fix `getSerializedDataColumnSidecars` regression: replace hardcoded `ssz.fulu.DataColumnSidecar` with fork-aware `sszTypesFor(forkName as ForkPostFulu).DataColumnSidecar` to correctly serialize gloas DataColumnSidecars which have different SSZ fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace number[] with Uint8Array for currentEpochParticipation and previousEpochParticipation in BeaconStateView. Each ParticipationFlags element is a uint8 (1 byte), so serialize() gives identical values at 1/8th the memory cost vs JS number[]. All consumers use indexed access which is fully compatible with Uint8Array. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add getPreviousEpochParticipation/getCurrentEpochParticipation methods that do direct SSZ tree lookups (O(log n)) instead of materializing the full array. Use these in validatorMonitor which only accesses a sparse set of monitored validators, avoiding a full serialize() on each epoch boundary. The cached Uint8Array getters are kept for aggregatedAttestationPool which scans full committees and benefits from amortized bulk access. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Return early in onceEveryEndOfEpoch before creating RootHexCache and iterating per-validator attestation/proposal data when the monitored validator set is empty. Avoids all state access on nodes that don't have any validators registered for monitoring. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Restore byteArrayEquals (over Buffer.compare), await on monitoring.close(), abort-signal comment, and full computeEnvelopeStateRoot JSDoc — all accidentally dropped alongside the IBeaconStateView migration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… in RootCache Add `isCachedBeaconStateAllForks()` type guard and update RootCache constructor to accept a union type, branching block root lookups accordingly. Reverts callers in state-transition from wrapping state in BeaconStateView back to passing directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ve beacon-node duplicate Add `computeAnchorCheckpoint()` to the `IBeaconStateView` interface (already implemented in `BeaconStateView`) and remove the duplicate standalone function from `beacon-node/initState.ts`. Callers in chain, forkChoice, and backfill now use `state.computeAnchorCheckpoint()` directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Motivation
Description
beacon-nodeandfork-choiceinstead of the currentCachedBeaconState*Closes #8650
blocked by #8773