Repository navigation
chore: merge lodestar-z into lodestar - #10513
Merged
Merged
Conversation
## What changed - add `aarch64-unknown-linux-musl` to the Lodestar-Z native package target matrix - pin hashtree-z to the merged PIC-enabled commit from [ChainSafe/hashtree-z#12](ChainSafe/hashtree-z#12) - pin the Zig ZAPI dependency to the ARM64-musl backport on `zapi-v2.x.0` - keep the npm dependency on `@chainsafe/zapi` 2.0.0, apply the matching target-support backport with a pnpm patch, and bundle the patched dependency for consumers ## Why Lodestar currently receives an ARM64 musl binary from `@chainsafe/blst`. Replacing it with Lodestar-Z without an equivalent artifact would regress Alpine Linux support on ARM64. The first real cross-build also exposed non-PIC objects in hashtree-z, which prevented the native Node.js shared library from linking. The pinned prerequisite commit fixes that linker failure. ## Dependency chain - [ChainSafe/zapi#47](ChainSafe/zapi#47) is backported to `zapi-v2.x.0` - [ChainSafe/hashtree-z#12](ChainSafe/hashtree-z#12) is merged; `build.zig.zon` pins its canonical squash commit using `zig fetch --save=hashtree` ## Validation - complete `aarch64-unknown-linux-musl` ReleaseSafe build through the patched ZAPI 2 CLI - native host ReleaseSafe bindings build - frozen pnpm install - `pnpm lint` - `zig fmt --check build.zig.zon` - 236 binding tests passed - npm package dry run confirms the patched ZAPI 2 dependency is bundled > This PR was written with AI assistance. --------- Co-authored-by: matthewkeil <me@matthewkeil.com> Co-authored-by: bing <spiralladder@fastmail.com> Co-authored-by: Cayman <caymannava@gmail.com>
```console ➜ ./lodestar --help Note: no -Dthread-count set, using cgroup-aware CPU count minus 1: 11 ``` `debug.print` prints no matter what so even in upstream lodestar we get this message printed, but this should really be using `std.log` scoped behind proper optimize modes
this - gets rid of bundling zapi as a dep (had to work around musl builds before) - updates zapi usage to be compatible with v3.x indirectly related to and closes ChainSafe/lodestar-z#499
Replaces #488 with the same change on top of the latest `main`, using a GitHub-verified signed commit. ## Summary - Default `Signature.fromHex` `sigInfcheck` to `true` when signature validation is enabled. - Keep the explicit `sigInfcheck = false` opt-out behavior. - Add regression coverage using the compressed G2 point at infinity.
## Summary - convert blst Pippenger scratch sizes from bytes to `u64` element counts - validate caller-provided scratch slices using the same units - allocate exact-minimum G1 and G2 scratch buffers in tests
## Summary - validate `SecretKey.fromHex` input length before copying into its fixed-size buffer - require exactly 32 decoded bytes after stripping an optional `0x` prefix - cover prefixed and unprefixed valid keys, short and long inputs, odd-length hex, and non-ASCII input ## Why Short hex strings previously reached a 32-byte slice operation without a length check, causing a ReleaseSafe bounds panic that aborted the Node process instead of throwing a JavaScript error. Checking the UTF-8 byte length before copying also prevents overlong strings from being silently truncated by N-API. Supersedes #487, which contains unsigned commits.
## Motivation Follow-up to the naming discussion on #469: `blst.lifecycle` was a one-off, while `pool`, `pubkeys`, and `config` already share a `State`/`state` convention for native-only module state. As suggested by @wemeetagain, this adopts that convention for blst. ## Description - Rename `Lifecycle`/`pub var lifecycle` to `State`/`pub var state` in `bindings/napi/blst.zig`, and `initThreadPool`/`deinitThreadPool` to `init(n_workers)`/`deinit`. - Move the module-level `var thread_pool` into `State` as an owned field, matching the shape of the sibling modules (e.g. `pool_rc` in `pool.zig`), not just the name. Internal use sites now read `state.thread_pool`. - `root.zig` now reads uniformly: `blst.state.init(...)` next to `pool.state.init()` / `pubkeys.state.init()` / `config.state.init()`. - Fix (pre-existing): add `errdefer blst.state.deinit()` in `root.zig` `init()`. Previously, if `pool.state.init()` or `pubkeys.state.init()` failed after the BLS pool was created, the `errdefer napi_io.deinit()` tore down the io while the pool's worker threads still referenced it, and `thread_pool` stayed non-null so any retry failed with `PoolExists`. The idempotent sibling states self-heal on retry; blst was the only one that latched. One deliberate divergence kept: `blst.state.init` still returns `error.PoolExists` on double init instead of the siblings' idempotent early-return, since it takes `n_workers` — silently ignoring a second init with a different worker count would hide a bug. ## Validation - `zig build build-lib:bindings` compiles clean. - `pnpm test`: 158 passed; the single failure is a missing local ERA fixture (`fixtures/era/mainnet-01628-47ac89fb.era`), unrelated to this change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
## Summary - route every BLST class extraction through zapi typed conversion so N-API type tags are checked before `napi_unwrap` - pin zapi to the merge commit of [ChainSafe/zapi#50](ChainSafe/zapi#50) until the pointer-argument fix is released - add regression coverage for wrong-class values across signature/public-key aggregation and randomized verification paths ## Motivation Raw `Env.unwrap` accepts any wrapped N-API object. Passing a `SecretKey` where a `PublicKey` or `Signature` is expected can therefore reinterpret a smaller native allocation as a larger structure and read beyond its bounds.
#518) closes #496 (indirectly, since we won't be using a page allocator for aggregation allocations anymore) - 512 as a stack size was arbitrary, and it is a bit ugly to maintain - separate allocators for cache and aggregation, since they're different lifetimes - also scope aggregation allocator (at comptime) appropriately to the `aggregate` fn only to prevent accidental misuse by the cache --------- Co-authored-by: Chen Kai <281165273grape@gmail.com>
deletes the minimal CLAUDE.md for a more comprehensive AGENTS.md that includes some important details like contribution guidelines, zig practices, consensus spec details etc
## Summary - tear down the BLS thread pool if a later N-API pool initialization step fails - tear down the N-API object pool if pubkey cache initialization fails - add a compile-time error-path barrier after the final fallible step so future fallible initialization cannot silently bypass the rollback design ## Motivation The addon initializes several process-global resources in sequence. If a later step fails, the previous implementation only released the N-API I/O context. That can leave an active BLS thread pool referencing a deinitialized I/O backend and prevent a later addon load from recovering because the global pool still exists.
## Motivation `keyGenV45` and `keyGenV5` index `salt[0]` before calling BLST, which traps when an empty salt is provided. The pinned BLST version also cannot safely process a zero-length salt in `blst_keygen_v5`. ## Description - Reject empty salts with `BlstError.BadEncoding` before entering BLST. - Pass validated non-empty salts using the slice pointer. - Add regression coverage for both key generation versions. - Validate with the BLS test suite in Debug and ReleaseSafe, tracked Zig formatting checks, and `pnpm lint`. AI assistance: this implementation was primarily AI-authored.
## Motivation `aggregateWithRandomness` updated `pk_out` before the signature MSM completed. Keeping both output writes after the fallible work avoids exposing a partially updated result when the function returns an error. ## Description Defer both affine output writes until the G1 and G2 MSMs complete successfully.
## Motivation `errorFromInt` currently treats every unrecognized BLST status code as success. This can silently accept unexpected or newly introduced BLST errors. ## Description - Handle `BLST_SUCCESS` explicitly as the only successful status. - Return `BlstError.UnknownError` for unrecognized status codes. - Remove the unused reverse conversion that fabricated a BLST status for `MergeError`. - Add coverage for success, every known BLST error, and an unknown status code. - Validate with the BLS test suite in Debug and ReleaseSafe, tracked Zig formatting checks, and `pnpm lint`. The full `zig build test` command was attempted but exceeded 15 minutes while running the generated spec suites. AI assistance: this implementation was primarily AI-authored.
## Motivation zapi v3.0.0 added a lifecycle-managed `js.io()` to the JS DSL (ChainSafe/zapi#26): `js.exportModule` retains a shared `std.Io.Threaded` before the module's `init` hook and releases it after the `cleanup` hook, refcounted across N-API environments. This is the same lifecycle our hand-rolled `bindings/napi/io.zig` implemented, so the local module can be deleted. ## Summary - bump zapi v2.2.0 → v3.0.0 - delete `bindings/napi/io.zig`; replace all `napi_io.get()` call sites with `js.io()` (`blst`, `pubkeys`, `metrics`, `BeaconStateView`, `root`) - drop manual io init/deinit from the `root.zig` lifecycle hooks — ordering is preserved upstream (`retain()` runs before `init`, `release()` after `cleanup`), so `js.io()` is valid during CPU detection at init and thread pool teardown at cleanup ### zapi v3 breaking change v3.0.0 rejects non-DSL `pub fn`s in `exportModule` (ChainSafe/zapi#38) instead of silently skipping them. Internal decls that the exporter walked are now hidden: - `pool.State`, `config.State`, `pubkeys.State`, `config.chainConfigFromObject`: de-pubbed (cross-file access still works through the pub `state` vars; the exporter skips pub vars) - `pool.PoolRc`: de-pubbed; `BeaconStateView.pool_rc` now uses `@TypeOf(pool.state.pool_rc)` - `blst.initThreadPool`/`deinitThreadPool`: moved behind a pub `lifecycle` var so they stay native-only and are no longer silently exported to JS (`deinitThreadPool`, zero-arg, previously *was* reachable from JS; nothing on the TS side used it) ## Validation - `zig build build-lib:bindings` (Debug and ReleaseSafe) - `pnpm test`: all 233 tests pass, including `teardown.test.ts` (clean process exit through the new env-cleanup → io release path, no panic) - `zig fmt --check` on changed files 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
## Summary - generate aggregate and batch-verification coefficients directly from the shared CSPRNG - reject zero 64-bit coefficients in synchronous aggregation, asynchronous aggregation, and `verifyMultipleAggregateSignatures` - bound zero-coefficient regeneration to eight retries and return `RandomScalarGenerationFailed` if the random source remains unusable ## Why A zero coefficient omits its public key and signature from the random linear combination. The original `@chainsafe/blst` binding generates nonzero 64-bit coefficients in both synchronous and asynchronous aggregation. The synchronous aggregation and batch-verification paths also seeded `DefaultPrng` once with 64 bits and derived every coefficient from that non-cryptographic PRNG. They now bulk-fill coefficient storage directly with `js.io().random`, consistent with the CSPRNG already used by the asynchronous path. Random rejection sampling is bounded so a broken random source fails instead of spinning forever. The asynchronous and batch-verification storage keeps a 32-byte stride per input, while the BLS operations read the first 8 bytes as the coefficient.
## Summary - delete unqueued N-API async work when Promise creation or queueing fails - settle the unreturned Promise on setup failures so Node can release its deferred handle - transfer work/data ownership to the completion callback only after queueing succeeds ## Why Previously, a successful `napi_create_async_work` followed by a failed `napi_queue_async_work` destroyed the callback data without deleting the work handle. This leaked the native handle and left it pointing at freed data.
## Summary - deinitialize the allocating metrics writer on every exit path - preserve the existing `ArrayList` ownership transfer and cleanup ## Why `state_transition.metrics.write()` can fail after `std.Io.Writer.Allocating` has grown its buffer. Previously that error returned before either the writer or an owning `ArrayList` was deinitialized, leaking the buffer. The unconditional `defer aw.deinit()` now releases the buffer when writing fails. On success, `toArrayList()` transfers ownership and resets the writer to an empty state, so the writer defer is safe while the list remains responsible for the transferred allocation. --------- Co-authored-by: Nazar Hussain <nazarhussain@gmail.com>
## Summary - remove 14 stale TODO/comment lines from the epoch cache modules - keep the remaining actionable TODOs unchanged - make no runtime or API changes Follow-up to the request in #278. ## Verification - `zig fmt --check src` - `zig build test:state_transition` - `pnpm lint` (passes with pre-existing warnings) - `git diff --check` The full `zig build test` was also attempted locally. It is blocked in this environment by missing generated spec-test sources and unsupported `io_uring` for clock tests; the targeted state-transition suite passes.
## Motivation Our release process is manual and opaque: versions are hand-bumped in `package.json` via PR, tags and GitHub releases are created by hand, and publishing is a manual `workflow_dispatch` runnable on any ref. This has led to drift — right now `v0.1.2-rc.10` is on npm and GitHub releases, but its tag points to a commit that isn't on `main`, and `main`'s `package.json` still says `0.1.2-rc.9` (see #477 / #478). Resolves #505 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: bing <spiralladder@fastmail.com> Co-authored-by: matthewkeil <me@matthewkeil.com> Co-authored-by: Matthew Keil <github@mail.matthewkeil.com>
## Summary - Replace the binding-owned pubkey caches with one process-wide `PubkeyCache`, defined in state transition and owned by N-API. - Use insertion-ordered `ArrayHashMap` storage, a cache-owned `RwLock`, and bounded parallel decompression. - Thread the shared cache through `EpochCache` and signature construction. - Add ABI-locked PKIX persistence for fast startup. ## Invariants and trust Validator-registry `(index, pubkey)` history is unforkable by design. Every state processed by a node has a registry that is a prefix of the process-wide cache, and an existing index never maps to another pubkey. PKIX is only for application-owned snapshots from the intended network. Loading checks the format, ABI, bounds, exact file size, and checksum. For speed, it intentionally skips semantic validation such as key uniqueness, compressed-to-affine correspondence, and BLS point validity. The checksum detects accidental corruption; it does not authenticate files. ## Impact The JavaScript API remains append-oriented; replaying an existing index is a no-op. `save`, `load`, and `reset` are restricted to the first N-API environment for the lifetime of the shared cache. Previous PKIX formats are not supported. ## AI assistance Implementation and review were primarily AI-assisted, with design direction and iterative review from the author and team. --------- Co-authored-by: Chen Kai <281165273grape@gmail.com> Co-authored-by: bing <spiralladder@fastmail.com>
## Motivation Running the full `process_epoch` benchmark eventually crashed with `SIGSEGV`. The isolated step benchmarks reuse one `EpochTransitionCache` across cloned states, so repeated `processRewardsAndPenalties` calls replaced the owned `cache.balances` slice without freeing the previous value and caused unbounded memory growth. `processEffectiveBalanceUpdates` also mutated ref-counted effective-balance increments shared with the benchmark baseline. ## Description - Transfer the balances slice used to update the state into `cache.balances`, releasing the previous slice only after `setBalances` succeeds. - Call `processRewardsAndPenalties` twice with `std.testing.allocator` so replacing the old cached balances is checked for leaks. - Clone effective-balance increments in the benchmark `before_each` hook, outside the timed section.
## Summary - compute the 384-bit BLST field size independently of pointer width - keep pointer-width sizing only for the `DST` and `DST_len` fields ## Why `Pairing.sizeOf()` divided a 384-bit field width by `@sizeOf(usize)`. This happened to produce the correct 48-byte field size on 64-bit targets, but produced 96 bytes on 32-bit targets. BLST represents the field as either six 64-bit limbs or twelve 32-bit limbs, so its size remains 48 bytes. The old calculation therefore reserved a 6352-byte pairing context on 32-bit targets instead of BLST's 3184 bytes. This was safe but doubled every pairing buffer, including the worker pool's buffer array. This was identified while discussing 32-bit capacity handling in ChainSafe/lodestar-z#522 (comment).
Clarified testing instructions in pre-push checklist.
Member
|
btw i think my comemnts can probably be addressed after the merge, would be cleaner that way, just leaving comments so we remember |
twoeths
previously approved these changes
Oct 30, 2026
nazarhussain
pushed a commit
to ChainSafe/zapi
that referenced
this pull request
Oct 30, 2026
`requireNapiLibrary(packageDir)` currently resolves platform packages from zapi's own module location. With isolated dependencies, the wrapper's platform package can be installed correctly but loading fails with `MODULE_NOT_FOUND`. Hoisting can also expose a different wrapper version's binary. Create the resolver from the supplied wrapper's `package.json`, and normalize local binary paths so relative package directories work too. Local binaries retain priority. Found while reviewing native packaging in ChainSafe/lodestar#10513. > Code, tests, and this PR description were written with OpenAI Codex.
Member
|
|
nazarhussain
left a comment
nazarhussain
left a comment
Contributor
There was a problem hiding this comment.
I don't like mixing up the zig (binding) code in the existing packages/ directory. Our mental model is that any thing in packages/ is old mono repo code. I suggest to have it in napi on the root which have a package lodestar-z.
pnpm can have multiple directories as workspaces, adding another directory will not affect the scripts running with pnpm.
**Motivation** gets the error name. Follow-up to #622. Fixing #622 will require a final PR to add diagnostics to every possible error site, maybe a way to enforce it. **Description** Move diagnostics into the STF module. `stateTransition` clears them on entry and returns errors without logging. `BeaconStateView.stateTransition` puts the diagnostic message in the thrown error while preserving `error.code`. For example: ```text WithdrawalsLengthMismatch expected=1 actual=0 ``` Errors without diagnostics keep their original message. **AI Assistance Disclosure** Implementation primarily AI-authored. (cherry picked from commit ff956b2273a9ceff798b6d19bb0a5e7c1aa7833f)
…odex/combine-lodestar-z
nflaig
approved these changes
Oct 30, 2026
github-project-automation
Bot
moved this from Awaiting Author
to Done
in Lodestar Team Coordination
This was referenced Oct 30, 2026
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
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
The Zig consensus implementation and its TypeScript consumers currently live in separate repositories with separate package versions. Consolidating them in Lodestar allows native interface changes and their consumers to be reviewed, built, and released together.
Description
Import the existing Zig implementation and its Git history into Lodestar.
src/and build files at the repository root, alongside the TypeScript monorepo inpackages/.packages/native/as@lodestar/native, using Lodestar's shared version and preserving the existing export subpaths.The branch adds one integration merge commit joining the original repository histories without rewriting their commits. This PR must not be squash-merged or rebased. The repository currently permits only squash merges, so a history-preserving merge must be arranged before landing it.
npm publishing permissions and trusted-publisher configuration for the new package names also need to be in place before landing, since
unstabletriggers releases.AI Assistance Disclosure
The repository integration was primarily authored with Codex.