Skip to content

chore: model pre-gloas block index as number#8858

Merged
ensi321 merged 4 commits intonc/epbs-fcfrom
te/nc/epbs-fc/review
Feb 4, 2026
Merged

chore: model pre-gloas block index as number#8858
ensi321 merged 4 commits intonc/epbs-fcfrom
te/nc/epbs-fc/review

Conversation

@twoeths
Copy link
Contributor

@twoeths twoeths commented Feb 4, 2026

Motivation

  • right now we model both pre-gloas and gloas variants as Array, which has some disadvantages:
    • array type is too general, it could be 1000 items while we only have 1/2/3 items
    • item 0 of pre-gloas block is FULL variant index, while item 0 of gloas blocks is PENDING variant index which is a little bit confusing to maintain
    • we check lengh=1 as pre-gloas, which is correct based on the implementation but ideally we can do better typing to know it

Description

  • model pre-gloas variant index as number
  • model gloas variants using tuple: [number, number] or [number, number, number]

part of #8739

cc @ensi321 @nflaig

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @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 improves the type safety and clarity of how block variant indices are managed within the ForkChoice and ProtoArray components. By explicitly typing pre-Gloas blocks with a single number and post-Gloas blocks with tuples, it resolves ambiguities and reduces potential confusion associated with array-based indexing, leading to more robust and maintainable code. This change streamlines the internal representation and handling of block variants, making the codebase easier to understand and less prone to type-related errors.

Highlights

  • Improved Type Safety for Block Variant Indices: The representation of block variant indices has been refactored to explicitly distinguish between 'pre-Gloas' and 'post-Gloas' blocks, enhancing type safety and clarity.
  • Pre-Gloas Variant Index as Number: For 'pre-Gloas' blocks, the variant index is now modeled as a single number instead of a single-element array, simplifying its usage and reducing ambiguity.
  • Post-Gloas Variant Indices as Tuples: For 'post-Gloas' blocks, variant indices are now explicitly typed as tuples, specifically [number, number] or [number, number, number], providing a fixed structure and better type inference.
  • Logic Adaptation for New Types: Existing logic within ForkChoice and ProtoArray classes has been updated to correctly handle the new VariantIndices type, utilizing Array.isArray() checks to differentiate between single numbers and arrays/tuples.

🧠 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
  • packages/fork-choice/src/forkChoice/forkChoice.ts
    • Updated findNodeByBlockHash to correctly process variantIndices which can now be either a number or an array.
    • Modified forwardIterateDescendants to accommodate the new VariantIndices type for blockVariants.
    • Added a TODO comment to forwardIterateDescendants to address its ambiguity regarding consumer payload requirements.
  • packages/fork-choice/src/protoArray/protoArray.ts
    • Introduced new type aliases: PreGloasVariantIndex (number), GloasVariantIndices (tuple types), and VariantIndices (union type) for improved type definition.
    • Changed the indices map from Map<RootHex, number[]> to Map<RootHex, VariantIndices> to leverage the new type system.
    • Refactored getNodeIndexByRootAndStatus, getDefaultVariant, getPayloadStatus, prune, getAncestor, getAncestor (second instance), and getAncestor (third instance) functions to correctly handle the new VariantIndices type using Array.isArray() checks.
    • Simplified the storage mechanism for variant indices in addGloasBlock and addPreGloasBlock to directly assign tuples or numbers, aligning with the new type definitions.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a great step towards improving type safety by modeling pre-Gloas block indices as a number and Gloas variants as tuples. The changes are consistently applied and enhance code clarity. However, I've identified a critical bug in the maybePrune function's logic for adjusting indices, where the wrong variable is used within a map callback. This could lead to incorrect pruning behavior. Please see the specific comment for details and a suggested fix.

@twoeths twoeths marked this pull request as ready for review February 4, 2026 11:01
@twoeths twoeths requested a review from a team as a code owner February 4, 2026 11:01
@ensi321 ensi321 merged commit 1dc21c4 into nc/epbs-fc Feb 4, 2026
6 checks passed
@ensi321 ensi321 deleted the te/nc/epbs-fc/review branch February 4, 2026 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants