feat: use zig backed lodestar-z shuffle - #9829
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c3cfb0d97
ℹ️ 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".
| nodeLinker: isolated | ||
|
|
||
| catalog: | ||
| "@chainsafe/lodestar-z": "github:ChainSafe/lodestar-z#nh/swap-or-not-shuffle-parity" |
There was a problem hiding this comment.
Pin the published shuffle dependency to a commit
When @lodestar/state-transition is packed or published, pnpm replaces its catalog: dependency with this catalog value, so downstream users receive the mutable nh/swap-or-not-shuffle-parity branch rather than the commit recorded in this repository's lockfile. A later branch update could therefore change consensus-critical native shuffling code without a Lodestar change or lockfile review. Use the resolved commit hash or a released package version in the catalog.
Useful? React with 👍 / 👎.
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
twoeths
left a comment
There was a problem hiding this comment.
need to wait for a @chainsafe/lodestar-z release
Review feedback: the async variants are unused (ChainSafe/lodestar#9829 calls unshuffleList, computeProposerIndex and computeSyncCommitteeIndices), so drop asyncShuffleList/asyncUnshuffleList and the async_task helper -- the zapi AsyncTask work waits for a future release. Also unbind the other unused package exports (forward shuffleList, ComputeShuffledIndex, the Electra wrappers, SHUFFLE_ROUNDS_* and ByteCount); the Zig module still implements them, so re-binding later is a few lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Motivation
Migrate shuffling to the new zig-based implementation from ChainSafe/lodestar-z#559, removing the rust-based
@chainsafe/swap-or-not-shuffledependency.Description
@chainsafe/lodestar-z(pinned via pnpm catalog) and remove@chainsafe/swap-or-not-shufflefrom state-transition and beacon-nodeunshuffleList/asyncUnshuffleList/computeProposerIndex/computeSyncCommitteeIndicescall sites tobindings.shuffle— API is identical, no logic changesVerified with build, check-types, lint, state-transition unit tests, and a shuffle/unshuffle roundtrip smoke test.
🤖 AI-assisted (Claude Code)