Skip to content

feat: bindings to getExpectedWithdrawals and native tweaks - #350

Merged
wemeetagain merged 3 commits into
mainfrom
bing/gew
May 6, 2026
Merged

feat: bindings to getExpectedWithdrawals and native tweaks#350
wemeetagain merged 3 commits into
mainfrom
bing/gew

Conversation

@spiral-ladder

@spiral-ladder spiral-ladder commented May 6, 2026

Copy link
Copy Markdown
Member
  • add bindings to it
  • don't heap allocate for withdrawals_results since it is capped at preset.MAX_WITHDRAWALS_PER_PAYLOAD == 16. This is about max ~800 bytes on mainnet preset
  • add assertions to assert the above

extracted from #347

@spiral-ladder spiral-ladder self-assigned this May 6, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 introduces native bindings for computing expected withdrawals and optimizes the underlying withdrawal processing logic. By removing heap allocations in favor of fixed-size stack buffers, the implementation improves performance and memory safety for withdrawal operations. The changes also include necessary updates to the TypeScript definitions to support the new functionality.

Highlights

  • N-API Bindings: Added getExpectedWithdrawals bindings to BeaconStateView to expose withdrawal computation logic to the N-API layer.
  • Memory Optimization: Refactored getExpectedWithdrawals to use stack-allocated buffers instead of heap allocation for withdrawal results, leveraging the fixed MAX_WITHDRAWALS_PER_PAYLOAD constraint.
  • API Refinement: Removed the Allocator parameter from getExpectedWithdrawals across the codebase, simplifying the function signature and enforcing safer memory usage via appendAssumeCapacity.
New Features

🧠 You can now enable Memory (public preview) 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.

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 the 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 counterproductive. 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.

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.

@spiral-ladder
spiral-ladder force-pushed the bing/gew branch 2 times, most recently from 94696ad to 0c456ad Compare May 6, 2026 03:30

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

Copy link
Copy Markdown
Contributor

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 implements the getExpectedWithdrawals method in the NAPI bindings for BeaconStateView and refactors the internal Zig implementation to utilize stack-allocated buffers, removing the requirement for an allocator in the withdrawal processing logic. Review feedback highlights several style guide violations, including a lack of assertions, exceeding the maximum function length, and the use of architecture-specific usize for balances. There is also a recommendation to ensure consistent use of stack-allocated buffers in process_block.zig to align with the PR's performance objectives.

allocator,
epoch_cache,
state,
&withdrawals_result,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The pull request description mentions avoiding heap allocation for withdrawals_results because it is capped at 16. However, the declaration of withdrawals_result at line 65 (outside this diff hunk) still uses initCapacity(allocator, ...). To fulfill the PR's objective and maintain consistency with the implementation in BeaconStateView.zig, please update the declaration to use a stack-allocated buffer and initBuffer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed in new push

/// Returns: { expectedWithdrawals: Withdrawal[], processedPartialWithdrawalsCount, processedValidatorSweepCount,
/// processedBuilderWithdrawalsCount, processedBuildersSweepCount }
/// The latter two are Gloas-only — always 0 here since Zig STF doesn't process Gloas yet.
pub fn getExpectedWithdrawals(self: *const BeaconStateView) !js.Value {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This function lacks assertions. The style guide (line 54) requires an average minimum of two assertions per function to detect programmer errors and enforce invariants. Since this is a public binding method, consider adding assertions to verify the state or the integrity of the results before returning to the JS environment.

References
  1. The assertion density of the code must average a minimum of two assertions per function. (link)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we are asserting in the native fn and already throwing a fork error below

.withdrawals = ct.capella.Withdrawals.Type.initBuffer(&withdrawals_buf),
};

var withdrawal_balances = std.AutoHashMap(ct.primitive.ValidatorIndex.Type, usize).init(allocator);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The use of usize for the value type in withdrawal_balances violates the style guide (line 44), which discourages architecture-specific types for non-memory related values. Since this map stores validator balances (Gwei), u64 should be used instead.

    var withdrawal_balances = std.AutoHashMap(ct.primitive.ValidatorIndex.Type, u64).init(allocator);
References
  1. Use explicitly-sized types like u32 for everything, avoid architecture-specific usize. (link)

/// This list is assumed to be bounded by `preset.MAX_WITHDRAWALS_PER_PAYLOAD`.
///
/// Caller should deinit `withdrawal_balances` with .deinit() after use.
pub fn getExpectedWithdrawals(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This function is approximately 130 lines long, which exceeds the hard limit of 70 lines per function defined in the style guide (line 108). Please refactor this logic by extracting the partial withdrawal processing and the validator sweep into separate helper functions.

References
  1. We enforce a hard limit of 70 lines per function. (link)

- add bindings to it
- don't heap allocate for `withdrawals_results` since it is capped at
  `preset.MAX_WITHDRAWALS_PER_PAYLOAD == 16`. This is about max ~800 bytes
  on mainnet preset
- add assertions to assert the above
@spiral-ladder
spiral-ladder force-pushed the bing/gew branch 2 times, most recently from 3d72b10 to 3d8d475 Compare May 6, 2026 04:08
@spiral-ladder
spiral-ladder marked this pull request as ready for review May 6, 2026 07:35
@spiral-ladder
spiral-ladder requested a review from a team as a code owner May 6, 2026 07:35

@wemeetagain wemeetagain left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good

@wemeetagain
wemeetagain merged commit f47bc66 into main May 6, 2026
30 checks passed
GrapeBaBa added a commit that referenced this pull request May 11, 2026
Bring in #246 (forkchoice + gloas types), #350 (withdrawals bindings),
#342/#348 (bindings refactors), and the other 7 PRs merged since this
branch diverged.

Updates gloas.zig FixedListType/FixedVectorType call sites for the
new opts argument added by this PR:
* balances / inactivity_scores / previous & current_epoch_participation
  now route through phase0.Balances / altair.{Epoch,Inactivity}Scores
  so they inherit chunked_leaf=true (matches fulu BeaconState shape).
* Other 13 FixedListType / 3 FixedVectorType sites take the default
  .{} opts.
wemeetagain pushed a commit that referenced this pull request Jun 17, 2026
This PR aligns the bindings to `BeaconStateView` and its native
implementation with the requirements of the typescript interface found
at
[`IBeaconStateView`](https://github.com/ChainSafe/lodestar/blob/374360e50a5de058b777a94d041089f9999d0726/packages/state-transition/src/stateView/interface.ts#L56).

## Details

This should be ready for a look. This PR mostly aligns `BeaconStateView`
to a 'good enough' state to be consumed by `lodestar` for state
transition.

This PR mainly adds missing functions and fixes the function signatures
of already implemented methods (which did not align with
`IBeaconStateView`

Note that this does not include the following implementations, which are
throw stubs for now:

- gloas related functions (we do pre-gloas STF for now)
- rewards API

Other related work that I broke into smaller PRs for reviewability:

#342, #348, #350, #366, #367, #368
@github-actions github-actions Bot mentioned this pull request Jul 30, 2026
@wemeetagain
wemeetagain deleted the bing/gew branch August 17, 2026 19:40
wemeetagain pushed a commit that referenced this pull request Aug 19, 2026
🤖 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 &lt; 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 -&gt; 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 -&gt; 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>
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