feat(blst): replace blst and pubkeys with lodestar-z#8900
Open
spiral-ladder wants to merge 27 commits intounstablefrom
Open
feat(blst): replace blst and pubkeys with lodestar-z#8900spiral-ladder wants to merge 27 commits intounstablefrom
blst and pubkeys with lodestar-z#8900spiral-ladder wants to merge 27 commits intounstablefrom
Conversation
spiral-ladder
changed the title
blst with blst-z where possible
Contributor
Summary of ChangesHello @spiral-ladder, 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 updates the beacon node's cryptographic backend by replacing the existing 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
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
|
spiral-ladder
changed the title
blst with blst-z where possibleblst with blst-z where possible
![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 replaces the @chainsafe/blst dependency with @chainsafe/blst-z in most of the codebase. The changes are mostly import replacements, but also include an update to the aggregateSignatures function calls, adding a false argument to likely disable signature verification for performance. In two spec test files, aggregateVerify is still imported from the old @chainsafe/blst library, which seems intentional. My review includes suggestions to add comments explaining this mixed dependency for better maintainability.
spiral-ladder
force-pushed
the
bing/blst-z
branch
2 times, most recently
from
5604001 to
59688b7
Compare
Member
|
You should be able to either |
spiral-ladder
force-pushed
the
bing/blst-z
branch
4 times, most recently
from
ec6bb4d to
6c95d69
Compare
wemeetagain
changed the title
blst with blst-z where possibleblst with lodestarz/blst
wemeetagain
changed the title
blst with lodestarz/blstblst with lodestar-z/blst
Contributor
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
nflaig
pushed a commit
that referenced
this pull request
Feb 17, 2026
## Description During finalized sync, blocks are imported in a tight loop without yielding to the event loop. When BLS verification is async (not awaiting the execution engine's `newPayload`), this prevents the checkpoint state cache's `processState()` cleanup from running, causing unbounded state accumulation and OOM on memory-constrained hosts. This adds `nextEventLoop()` after each `importBlock` to allow pending async cleanup (state serialization, cache eviction) to execute between block imports. ### Root Cause In `processBlocks()`, the import loop previously had a natural yield point when `importBlock` awaited the execution engine. With async BLS via `@chainsafe/blst` (PR #8900), `importBlock` no longer blocks on the EL call, so blocks blast through without yielding. The checkpoint state cache's `processState()` — which runs as fire-and-forget async — never gets a chance to serialize and evict old states, causing memory to climb until OOM. ### Fix ```typescript for (const fullyVerifiedBlock of fullyVerifiedBlocks) { await importBlock.call(this, fullyVerifiedBlock, opts); await nextEventLoop(); // Allow processState() cleanup to run } ``` `nextEventLoop()` is an existing utility (`sleep(0)`) that yields to the event loop's timers phase, giving pending microtasks and macrotasks a chance to execute. ### Testing Deployed as commit 983d923 on feat3 infrastructure nodes — resolved OOM crash loops on nodes with sufficient memory. Memory-constrained hosts (~16GB) may need additional tuning (reduced `maxBlockStates`, queue depth limits). Co-authored-by: Cayman <caymannava@gmail.com> --- > [!NOTE] > This PR was authored with AI assistance (Claude). All code has been reviewed and validated. Co-authored-by: lodekeeper <lodekeeper@users.noreply.github.com> Co-authored-by: Cayman <caymannava@gmail.com>
wemeetagain
force-pushed
the
bing/blst-z
branch
from
549bae6 to
9152e82
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9152e822a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
.github/workflows/test-bun.yml
Outdated
Comment on lines
+38
to
+39
| version: ${{ env.ZIG_VERSION }} | ||
| cache-key: ${{ env.ZIG_VERSION }} |
There was a problem hiding this comment.
This workflow now calls mlugg/setup-zig@v2 with version: ${{ env.ZIG_VERSION }} and cache-key: ${{ env.ZIG_VERSION }}, but test-bun.yml does not define ZIG_VERSION at workflow/job scope (unlike the other updated workflows). On Bun CI runs this yields an empty version input, which can make the Zig setup step fail or become non-deterministic by falling back to an implicit default, blocking or destabilizing the Bun test job.
Useful? React with 👍 / 👎.
nazarhussain
requested changes
Feb 23, 2026
nazarhussain
requested changes
Feb 23, 2026
nazarhussain
requested changes
Feb 23, 2026
wemeetagain
force-pushed
the
bing/blst-z
branch
from
b50b073 to
80473b8
Compare
wemeetagain
changed the title
blst with lodestar-z/blstblst and pubkeys with lodestar-z
spiral-ladder
force-pushed
the
bing/blst-z
branch
from
3258fac to
2018db5
Compare
Merged
spiral-ladder
added a commit
that referenced
this pull request
Feb 25, 2026
**Motivation** as previously discussed, remove bun tests entirely. Extracted from #8900 cc @nazarhussain
spiral-ladder
force-pushed
the
bing/blst-z
branch
from
f8fd7c1 to
1961757
Compare
spiral-ladder
force-pushed
the
bing/blst-z
branch
from
1961757 to
ecb2087
Compare
Contributor
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request replaces the @chainsafe/blst dependency with @chainsafe/lodestar-z across the entire codebase. This is a significant refactoring that also introduces a new native pubkey cache for better performance, replacing createPubkeyCache with a singleton getPubkeyCache. The changes are extensive but appear to be applied consistently and correctly. The CI configuration is also updated to install Zig, which is a new build dependency. Overall, this is a great improvement. I have one minor suggestion regarding a pre-existing issue in pnpm-workspace.yaml.
Comment on lines
+30
to
+34
| overrides: | ||
| dns-over-http-resolver": ^2.1.1 | ||
| elliptic: '>=6.6.1' | ||
| loupe": ^2.3.6 | ||
| nan": ^2.19.0 |
Contributor
There was a problem hiding this comment.
It seems there's a pre-existing formatting issue in the overrides section. Several keys have a trailing double quote, which makes them invalid YAML keys. While this PR only moves this block, it would be a good opportunity to fix it. The keys should be properly quoted.
overrides:
"dns-over-http-resolver": ^2.1.1
elliptic: '>=6.6.1'
"loupe": ^2.3.6
"nan": ^2.19.0
Contributor
this benchmark is not great
I wonder why CI still said "no regression". Also I don't think it's that bad when comparing |
spiral-ladder
force-pushed
the
bing/blst-z
branch
from
11921d7 to
52d9206
Compare
spiral-ladder
force-pushed
the
bing/blst-z
branch
from
52d9206 to
740c395
Compare
spiral-ladder
force-pushed
the
bing/blst-z
branch
from
1f63139 to
df75779
Compare
spiral-ladder
force-pushed
the
bing/blst-z
branch
from
94bd51d to
1e2ac8b
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Depends on ChainSafe/lodestar-z#231
Motivation
Replacing usages of
blstwith alodestar-zbindings toblst-zfor usage in the beacon node.