refactor(bindings): use zapi js.io() instead of local io module - #469
Conversation
Bump zapi to v3.0.0, whose js.exportModule retains a shared std.Io.Threaded before the init hook and releases it after cleanup — same lifecycle bindings/napi/io.zig implemented by hand, plus refcounting across N-API environments. v3.0.0 also rejects non-DSL pub decls in exportModule instead of silently skipping them: de-pub internal State types and move the blst thread pool lifecycle behind a pub var so they stay native-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| /// Initialized lazily on first use, torn down via `deinitThreadPool`. | ||
| var thread_pool: ?*ThreadPool = null; | ||
|
|
||
| pub fn initThreadPool(n_workers: u16) !void { |
There was a problem hiding this comment.
We could have kept the function as it is by using following signature.
pub fn initThreadPool(n_workers: js.Number) !voidBut then it would have been expose to JS surface, which we don't want, as it's an internal life cycle function only used at root with the number of cpu count.
| } | ||
| /// Native-only thread pool lifecycle, reached from `root.zig` through the | ||
| /// pub `lifecycle` var so it is not part of the JS module surface. | ||
| const Lifecycle = struct { |
There was a problem hiding this comment.
This pattern is safety hatch for above mentioned problem. Now we can access the lifecycle functions in zig but not in the js surface.
There was a problem hiding this comment.
this seems reasonable and cleaner than #483 , but thinking about future modules we expose, might get confusing with mixed use in the same file, but this might be overthinking, we can reconsider again when/if we encounter this issue because the blast radius here is not big
nit: is LifeCycle a good name though, seems like blst.lifecycle... is not immediately obvious
Resolve zapi to v3.1.0: superset of both sides — js.io() (v3.0.0, this branch) plus ARM64 musl target support (main's #485 backport). Port main's new stateTransition.zig binding and pubkeys growth_step changes to js.io() / non-pub State. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| } | ||
| /// Native-only thread pool lifecycle, reached from `root.zig` through the | ||
| /// pub `lifecycle` var so it is not part of the JS module surface. | ||
| const Lifecycle = struct { |
There was a problem hiding this comment.
this seems reasonable and cleaner than #483 , but thinking about future modules we expose, might get confusing with mixed use in the same file, but this might be overthinking, we can reconsider again when/if we encounter this issue because the blast radius here is not big
nit: is LifeCycle a good name though, seems like blst.lifecycle... is not immediately obvious
Suggest any other name, which you feel much aligned.
Ideally we should not have any interface in a binding file that we actually want to hide from the JS runtime. |
|
@nazarhussain i don't have a better name yet :) hence approving now, we can come up with one if someone other than me finds it confusnig |
wemeetagain
left a comment
There was a problem hiding this comment.
Well another option is to use the standard of the other bindings modules: State / state.
eg: blst.state.deinit
@wemeetagain Yes I agree to it and opened a separate PR to address this change #516 to not mix with the |
## 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>
# Conflicts: # bindings/napi/blst.zig
|
I have a question about why zapi should expose an IO implementation, it looks like it create a separate thread pool and not use the application's thread pool, I thought it is better to let application managing the unified thread pool and concurrency strategy, more control for tuning |
|
Short answer to your question, the pool isn't actually created up front, nothing forces you to use it, and in a Node addon there is no "application thread pool" to unify with. Key points are:
One point that shared instance is hardcoded to |
Yes, this is my thought. |
|
@GrapeBaBa I feel we should do that enhancement as separate PR in the future. What do you think? |
Sure |
🤖 I have created a release *beep* *boop* --- ## [1.0.0](v0.1.2...v1.0.0) (2026-08-19) ### Features * add `state.getBuildersLength()` binding ([#472](#472)) ([be2b5ab](be2b5ab)) * **beacon-node:** add block state cache and checkpoint datastore ([#452](#452)) ([2145faa](2145faa)) * bindings to `getExpectedWithdrawals` and native tweaks ([#350](#350)) ([f47bc66](f47bc66)) * **bindings:** add pubkey cache syncPubkeys ([#537](#537)) ([542779f](542779f)) * **bindings:** aggregate cached public keys by validator index ([#397](#397)) ([2f90603](2f90603)) * **bindings:** align `BeaconStateView` with `IBeaconStateView` ([#347](#347)) ([b8ec273](b8ec273)) * **bindings:** configurable pubkey cache growth step ([#481](#481)) ([133ef24](133ef24)) * **bindings:** expose more APIs for STF ([#444](#444)) ([7fe2609](7fe2609)) * **bls:** add small MSM for npoints < 32 ([#393](#393)) ([b430638](b430638)) * **blst:** use external buffers for blst operations ([#358](#358)) ([78e4678](78e4678)) * **ci:** conditionally publish bindings with tag ([#355](#355)) ([ea77919](ea77919)) * **clock:** add clock module for slot/epoch timing ([#354](#354)) ([385b077](385b077)) * **fork_choice:** add Prometheus metrics module ([#309](#309)) ([cbc9d8d](cbc9d8d)) * **forkchoice:** implement the forkchoice module ([#246](#246)) ([7c62a9b](7c62a9b)) * getSyncCommitteesWitness ([#367](#367)) ([ef77649](ef77649)) * implement `loadState` API and binding ([#165](#165)) ([f903519](f903519)), closes [#159](#159) * **metrics:** metrics bindings ([#455](#455)) ([dd41999](dd41999)) * migrate blst,pubkeys to use zapi js dsl ([#331](#331)) ([fcd26ca](fcd26ca)) * **pubkeys:** add getPubkeyBytes binding ([#555](#555)) ([4ca51cf](4ca51cf)) * publish ARM64 musl bindings ([#482](#482)) ([ac764c9](ac764c9)) * **shuffle:** add swap-or-not shuffling module and binding ([#559](#559)) ([c2db37c](c2db37c)) * split nextValue fn ([#464](#464)) ([b47faeb](b47faeb)) * support getLatestWeakSubjectivityCheckpointEpoch ([#366](#366)) ([dcf3883](dcf3883)) * update fulu deposit processing ([#442](#442)) ([064335c](064335c)) ### Bug Fixes * avoid set ([#484](#484)) ([2e25d97](2e25d97)) * better generation of rand scalar ([#388](#388)) ([74dce77](74dce77)) * **bindings:** accept `dontTransferCache` in processSlots for backward compatibility ([#460](#460)) ([65df5af](65df5af)) * **bindings:** check signature infinity by default ([#509](#509)) ([2f5f281](2f5f281)) * **bindings:** clean up failed async BLS work ([#527](#527)) ([1111b00](1111b00)) * **bindings:** free metrics writer on scrape failure ([#529](#529)) ([4c8d94a](4c8d94a)) * **bindings:** harden random aggregate scalars ([#528](#528)) ([8e89a63](8e89a63)) * **bindings:** log level for missing fields ([#435](#435)) ([08faf41](08faf41)) * **bindings:** misordering of print for cpu count ([#381](#381)) ([752a972](752a972)) * **bindings:** populate epoch participation for test fixtures ([#436](#436)) ([8dbdd2e](8dbdd2e)) * **bindings:** refcount Pool to fix teardown panic ([#352](#352)) ([23b2f68](23b2f68)) * **bindings:** roll back partial N-API initialization ([#491](#491)) ([31c5ebb](31c5ebb)) * **bindings:** size BLS thread pool by cgroup-aware CPU count ([#386](#386)) ([3ae9522](3ae9522)) * **bindings:** validate class types before unwrap ([#514](#514)) ([2fd2ad5](2fd2ad5)) * **bindings:** validate secret key hex length ([#517](#517)) ([136e415](136e415)) * **bls:** align PublicKey.uncompress validation with Signature.uncompress ([#508](#508)) ([5a8dbe9](5a8dbe9)) * **bls:** bound randomized aggregation inputs ([#548](#548)) ([779d0bf](779d0bf)), closes [#542](#542) * **bls:** clean up partial thread pool initialization ([#490](#490)) ([d55e598](d55e598)) * **bls:** convert pippenger scratch bytes to element counts ([#513](#513)) ([a12ca92](a12ca92)) * **bls:** enforce 32-byte signing roots ([#545](#545)) ([72fd308](72fd308)) * **bls:** make batch cardinality structural ([#547](#547)) ([a06d8b2](a06d8b2)) * **bls:** preserve aggregate outputs on failure ([#521](#521)) ([e0b6dd1](e0b6dd1)) * **bls:** reject empty keygen salts ([#524](#524)) ([d2a9c86](d2a9c86)) * **bls:** reject unknown BLST error codes ([#525](#525)) ([9e4a6ad](9e4a6ad)) * **bls:** size pairing buffers for 32-bit targets ([#531](#531)) ([dc64a27](dc64a27)) * **blst:** default signature infinity check to true if not provided ([#387](#387)) ([021cdcb](021cdcb)) * **build:** remove `zig-out` from `files` ([#360](#360)) ([c52af09](c52af09)) * **ci:** fix caching spec test version ([#439](#439)) ([96885a1](96885a1)) * dangling state pointer in loadOtherState ([#450](#450)) ([81cbd5f](81cbd5f)) * **epoch_cache:** compute missing `next_proposers` ([#447](#447)) ([0088a29](0088a29)) * **epoch_cache:** populate decision roots in afterProcessEpoch ([#453](#453)) ([4b70a5e](4b70a5e)) * export asyncAggregateWithRandomness through napi binding ([#371](#371)) ([1d04c2b](1d04c2b)) * harden memory safety across PMT, SSZ tree views, and state transition ([#377](#377)) ([d6f5897](d6f5897)) * improve atomic ordering in ThreadPool and NAPI init ([#310](#310)) ([4b0a1cc](4b0a1cc)) * interface compatbility with NativeBeaconStateView ([#445](#445)) ([89e13d1](89e13d1)) * missing deinits in loadOtherState ([#459](#459)) ([094d278](094d278)) * missing state commits ([#454](#454)) ([a432b55](a432b55)) * no-op when syncPubkeys run on a pk cache with shrinking validator set ([#432](#432)) ([ed05a99](ed05a99)) * param order in BeaconBlockBody ([#348](#348)) ([d8b9c06](d8b9c06)) * pendingConsolidations bindings ([#449](#449)) ([b9c497e](b9c497e)) * **pmt,ssz:** harden chunked-leaf and zero-copy tree-view memory safety ([#400](#400)) ([de50c53](de50c53)) * populate cache balances during rewards/penalties processing ([#474](#474)) ([5bf23dc](5bf23dc)) * re-expose sizes ([#369](#369)) ([64b81f3](64b81f3)) * remove `slashValidator` gating on active status ([#448](#448)) ([d319a0d](d319a0d)) * **ssz:** drop redundant default-init pass in fixed-list decode ([#468](#468)) ([0c757be](0c757be)) * **ssz:** publish child cache entries after lookup ([#565](#565)) ([21e78c9](21e78c9)) * state transition binding exports ([#456](#456)) ([895982c](895982c)) * **state-transition:** group-check signature sets ([#515](#515)) ([42774e9](42774e9)), closes [#502](#502) * **state-transition:** isolate epoch step cache mutations ([#535](#535)) ([a83741a](a83741a)) * **state-transition:** repair Pool.init call broken by [#346](https://github.com/ChainSafe/lodestar-z/issues/346)×[#367](https://github.com/ChainSafe/lodestar-z/issues/367) merge skew ([#394](#394)) ([b42944f](b42944f)) * various fixes around config ([#433](#433)) ([c4f082c](c4f082c)) ### Performance Improvements * **bindings:** drop TS BLS comparison benches and report benchmarks on PRs ([#552](#552)) ([c909c6f](c909c6f)) * **bls:** add cache-aware signature verifier ([#562](#562)) ([063857e](063857e)) * **bls:** bypass worker queue for small batches ([#553](#553)) ([3f8a6df](3f8a6df)) * **epoch:** replace AutoHashMap with array lookup in reward/penalty caches ([#286](#286)) ([e4e181b](e4e181b)), closes [#243](#243) * **pmt:** chunked-leaf packing for basic lists and container_struct ([#346](#346)) ([ba156c4](ba156c4)) ### Code Refactoring * allocate `AsyncAggRandData` in one obj ([#384](#384)) ([459750f](459750f)) * **bindings/pubkeys:** simplify allocation strategy for aggregate ([#518](#518)) ([b82750f](b82750f)) * **bindings:** rename blst Lifecycle to State ([#516](#516)) ([0a9c179](0a9c179)) * **bindings:** use zapi js.io() instead of local io module ([#469](#469)) ([2b34cc0](2b34cc0)) * **bindings:** wake only required number of workers ([#383](#383)) ([1db57f1](1db57f1)) * **bls:** allocations around VMAS ([#395](#395)) ([dfda58c](dfda58c)) * **bls:** clean up bls ([#398](#398)) ([e0f3b9b](e0f3b9b)) * **bls:** remove need for tracking results for verifyMultipleAggregateSignatures ([#389](#389)) ([6fe5c3f](6fe5c3f)) * **bls:** remove single-threaded fallback ([#390](#390)) ([e057713](e057713)) * **clock:** single public Clock; internalize SlotClock ([#463](#463)) ([fbab1fa](fbab1fa)) * make XXXDecisionRoot fns return `js.String` ([#342](#342)) ([aef4420](aef4420)) * move shuffle into swap_or_not_shuffle module ([#558](#558)) ([e56efb2](e56efb2)) * **pubkeys:** centralize the process-wide cache ([#522](#522)) ([dc9669d](dc9669d)) ### Miscellaneous Chores * avoid slow tests in AGENTS.md ([#546](#546)) ([c60f2a9](c60f2a9)) * bump zapi to include musl build ([#485](#485)) ([0b488cc](0b488cc)) * **ci:** pin github actions with sha hashes ([#507](#507)) ([167b8f5](167b8f5)) * deprecate unused blst APIs ([#575](#575)) ([7b547fa](7b547fa)) * **deps:** bump zapi v2.1.0 -> v2.2.0 ([#376](#376)) ([0c240d8](0c240d8)) * **deps:** bump zbuild ([#403](#403)) ([e2545de](e2545de)) * **deps:** compile blst with ReleaseFast ([#391](#391)) ([753a896](753a896)) * **deps:** update zapi to 3.1.0 ([#483](#483)) ([f3e5827](f3e5827)) * **deps:** use zapi v2.1.0 ([#372](#372)) ([88f403a](88f403a)) * disable gemini auto code review ([#382](#382)) ([63e42a4](63e42a4)), closes [#380](#380) * **docs:** add comments section in AGENTS.md ([#566](#566)) ([0c09750](0c09750)) * move state clones out of benchmark run functions ([#324](#324)) ([e4035de](e4035de)) * prepare 1.0.0 release ([#576](#576)) ([20b657b](20b657b)) * release v0.1.2-rc.3 ([#370](#370)) ([e4fc551](e4fc551)) * **release:** 0.1.2-rc.2 ([#365](#365)) ([7046128](7046128)) * **release:** v0.1.2-rc.10 ([#477](#477)) ([9a4fad5](9a4fad5)) * **release:** v0.1.2-rc.4 ([#373](#373)) ([09468f1](09468f1)) * **release:** v0.1.2-rc.5 ([#374](#374)) ([f344efa](f344efa)) * **release:** v0.1.2-rc.6 ([#375](#375)) ([bdf5b67](bdf5b67)) * **release:** v0.1.2-rc.8 ([#401](#401)) ([06f91c2](06f91c2)) * **release:** v0.1.2-rc.9 ([#404](#404)) ([6024800](6024800)) * remove merge transition code ([#359](#359)) ([09b175d](09b175d)) * remove stale epoch cache TODOs ([#534](#534)) ([27a547a](27a547a)) * rename era shortHistoricalRoot to shortEraRoot ([#473](#473)) ([c75a4d3](c75a4d3)) * **scripts:** build bindings with preset ([#434](#434)) ([a1b5ef7](a1b5ef7)) * silence debug log when used in release builds ([#486](#486)) ([c5377d7](c5377d7)) * support dev workflow ([#364](#364)) ([fcb9a78](fcb9a78)) * update gloas types to align with the latest specs ([#431](#431)) ([1f065b5](1f065b5)) * update spec test version to v1.7.0-alpha.11 ([#451](#451)) ([5875660](5875660)) * update spec-test-version: v1.6.0-beta.2 -> v1.7.0-alpha.10 ([#441](#441)) ([f932b1c](f932b1c)) * update zapi to 4.0.0 ([#571](#571)) ([de8e3fd](de8e3fd)) ### Documentation * document security threat model ([#557](#557)) ([e678b87](e678b87)) * more comprehensive AGENTS.md ([#520](#520)) ([c74b386](c74b386)) * **pkix:** document load provenance requirement ([#556](#556)) ([37e0aa2](37e0aa2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Motivation
zapi v3.0.0 added a lifecycle-managed
js.io()to the JS DSL (ChainSafe/zapi#26):js.exportModuleretains a sharedstd.Io.Threadedbefore the module'sinithook and releases it after thecleanuphook, refcounted across N-API environments. This is the same lifecycle our hand-rolledbindings/napi/io.zigimplemented, so the local module can be deleted.Summary
bindings/napi/io.zig; replace allnapi_io.get()call sites withjs.io()(blst,pubkeys,metrics,BeaconStateView,root)root.ziglifecycle hooks — ordering is preserved upstream (retain()runs beforeinit,release()aftercleanup), sojs.io()is valid during CPU detection at init and thread pool teardown at cleanupzapi v3 breaking change
v3.0.0 rejects non-DSL
pub fns inexportModule(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 pubstatevars; the exporter skips pub vars)pool.PoolRc: de-pubbed;BeaconStateView.pool_rcnow uses@TypeOf(pool.state.pool_rc)blst.initThreadPool/deinitThreadPool: moved behind a publifecyclevar 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, includingteardown.test.ts(clean process exit through the new env-cleanup → io release path, no panic)zig fmt --checkon changed files🤖 Generated with Claude Code