chore: remove merge transition code - #359
Merged
spiral-ladder merged 47 commits intoJun 17, 2026
Merged
Conversation
Contributor
Summary of ChangesHello, 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 cleans up the codebase by removing dead code related to the Ethereum merge transition. Since all supported networks have long passed the Bellatrix upgrade, these legacy checks are no longer necessary. The changes simplify state transition logic while preserving essential functionality for post-Bellatrix edge cases. Highlights
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 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 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 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
|
![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 removes legacy pre-merge logic by deleting the pre_merge status from the ExecutionPayloadStatus enum, removing the isMergeTransitionBlock utility, and simplifying related checks in the state transition logic. Feedback indicates that the changes violate several repository style guidelines: processBlobKzgCommitments fails to handle both positive and negative spaces and lacks sufficient assertion density, while processExecutionPayload exceeds the 70-line function length limit and also requires more assertions.
Comment on lines
+4
to
+5
| if (external_data.execution_payload_status == .invalid) { | ||
| return error.InvalidExecutionPayload; |
Contributor
There was a problem hiding this comment.
The new implementation uses an if statement that only handles the negative case (.invalid). According to the repository style guide (Lines 134-138), it is preferred to handle or assert both positive and negative spaces. Additionally, this function lacks the required minimum of two assertions per function (Line 54). Consider using a switch statement to explicitly handle all enum variants and adding assertions for the function arguments to improve safety and clarity.
| if (external_data.execution_payload_status == .pre_merge) { | ||
| return error.ExecutionPayloadStatusPreMerge; | ||
| } else if (external_data.execution_payload_status == .invalid) { | ||
| if (external_data.execution_payload_status == .invalid) { |
Contributor
There was a problem hiding this comment.
The processExecutionPayload function (lines 16-94) violates the repository style guide in two ways:
- Function Length: At 79 lines, it exceeds the hard limit of 70 lines per function (Line 108).
- Assertion Density: It does not meet the requirement of an average of two assertions per function (Line 54).
While the removal of the .pre_merge check is correct, this function should be refactored into smaller helpers with appropriate assertions for function arguments and invariants to adhere to the project's safety and maintainability standards.
Contributor
Author
|
Adapted bindings, should pass the checks |
In zapi, [local path takes preference](https://github.com/ChainSafe/zapi/blob/e522fa4beb5eacfb8ee0f9a964b435a88bb8f2ba/ts/lib.ts#L218) so we want to not publish with that entry under `files`, otherwise `lodestar` will attempt to load an architecture incompatible library on `dlopen`. Locally testing after pulling from a published `lodestar-z` is failing because my machine (aarch64-apple-darwin) is trying to load from an x86_64-linux binary, so we'd need to re-publish after we fix this
Depends on ChainSafe/zapi#30 (we need to release zapi and update the dep) This is one of possible likely causes for increased GC pressure on experiments to swap out blst-ts for lodestar-z/bls, as observed on feat2 and feat3 deployments in [this PR](ChainSafe/lodestar#9342). With external array buffers, V8 is only aware of the pointer to the backing memory, instead of having to track both the pointer and the backing memory. This means that during marking phase the GC does not have to walk the backing memory to mark it as 'live' - the frequency of the GC firing off is still the same, but each cycle does less work. This of course comes with a tradeoff, we need a **finalizer** to let V8 know how much external memory is in native heap so that the GC tells the native impl to free the useless memory. Though, regardless of the effect, we should still probably do this anyway, since [napi-rs does the same](https://github.com/napi-rs/napi-rs/blob/159395b365c583a6642ad481edc5708d9f36a24b/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs#L175), and only defaults to V8 managed array buffers if it is disallowed (like in Electron).
for testing signature validation + new zapi externalBuffer api
**Motivation** It need `loadState` API to integrate state-transition-z to lodestar **Description** - Implement `loadState()` migration semantics aligned with Lodestar: migrate a new BeaconState from SSZ bytes using a seed state, returning the migrated state plus the list of modified validator indices. - Optimize validator/inactivity_scores handling by reusing seed subtrees where bytes are unchanged, and by computing modified indices via recursive byte-level diff (avoids full SSZ decoding for comparison). Fix ChainSafe#159
- Motivation: ChainSafe#360 (comment) Afaik, zig-out/lib/* must be in package.json `files` in order for the library to be part of the packed package when installing lodestar-z as a git dependency. Unfortunately, that also allows it to be part of the package when _published_!! which breaks cross-platform usage of the library, short circuiting the correct library from being loaded. To that end, this PR attempts to get the intended behavior for both cases: - keep zig-out/lib in `files` so git-dependency-installed lodestar-z packs a freshly-built library - but crucially suppress the prepare script from being run during publish via `--ignore-scripts` so npm-dependency-installed lodestar-z uses a platform-specific published library
extracted from ChainSafe#347 Support both native + binding
will contain ChainSafe#371 for consumption by ChainSafe/lodestar#8900
for static decls
## Motivation `ThreadPool` worker hot loops use `.acquire` on `err_flag.load()` where `.monotonic` suffices — the flag is a pure early-exit signal with no data dependency on the setter's other writes. BLS verification is CPU-intensive, so relaxing this in the inner loop avoids unnecessary memory-fence cost. Matches the pattern already used in `src/state_transition/cache/pubkey_cache.zig`. The earlier NAPI init-mutex changes from this branch have been dropped after merging main, because main moved to zapi-managed lifecycle (`js.exportModule` with `init`/`cleanup` hooks). The concurrent-register race they were guarding against has been filed against zapi upstream: ChainSafe/zapi#31. ## Description `src/bls/ThreadPool.zig`: - `err_flag.load(.acquire)` → `.monotonic` in `VerifyMultiWorkItem.exec` and `AggVerifyWorkItem.exec` worker loops. - Setter side (`err_flag.store(true, .release)` on pairing failure) is unchanged — release semantics on the producer side carry no obligation on the consumer to also be `.acquire` when the consumer doesn't depend on the producer's other writes.
as title says, we were printing 0 because the print was misplaced
…ition (ChainSafe#377) ## Summary Addresses the ChainSafe#357 memory-safety review. The findings are error-path bugs that the happy path never exercises: `errdefer`/`defer` cleanup running over `undefined` or already-moved memory, OOM leaving aliased refcounted pointers, orphaned pool nodes on rollback, and an out-of-bounds proposer index from untrusted input. All fixes are allocate/validate-before-commit so a failure can't leave shared state corrupt. ## Changes **persistent_merkle_tree** - Zero-fill node-id buffers at `errdefer` sites so a mid-build error's `unref`/`free` no-ops on the unbuilt tail instead of unref-ing stack garbage (C1) - `Pool.alloc` rolls back already-popped slots on a preheat OOM (C2) - `createBranch` rolls back an applied child ref on `RefCountOverflow` (M2) - `FillWithContentsIterator` reclaims both orphaned `left` and `carry` on a `createBranch` OOM, guarding the all-default aliased-node case (M10) - `setNodes*` start `unfinalized_parents` all-null so the unref loop can't read an undefined `?Id` (H1); empty-`indices` no-op guarded before the ascending assert - `View.destroy` asserts no live children + poisons the recycled slot (H2) **ssz tree views** - `commit`/`set` reserve capacity up front so the stores are infallible (H4/M5) - `setValue` no longer double-frees the child view on `set`'s OOM; documented the borrowed-pointer invalidation contract on `get`/`set`/`getReadonly`/`clone` (C5) - `sliceTo` unrefs its intermediate orphan roots (H6) - `ReadonlyIterator.nextValue` initializes `out` before `toValue` (H5) **state_transition** - Epoch-cache shuffling rotation and effective-balance/pubkey updates allocate before mutating shared `Rc`/maps, so OOM can't leave an aliased double-unref (C4) - `epoch_transition_cache` errdefers for its temporary lists (M8) - Proposer signature set bounds-checks the proposer index (`verifyProposerSignature` runs before `processBlockHeader` validates it) (C3) - `effectiveBalanceIncrementsSet` allocates the buffer with the same allocator the owning `Rc` frees it with - Signature-set `out` params changed from by-value `ArrayList` to `*ArrayList`
## bench: move state clones out of benchmark run functions Closes ChainSafe#164 ### Problem Benchmark `run` functions were cloning and freeing `CachedBeaconState` on every iteration. This clone/deinit cost was included in the timed measurement, skewing results for `processBlock`, `processEpoch`, and their individual sub-step benchmarks. This is visible through the `runImpl` inside `zbench`. This change should also increase readability of the affected files. ### Solution Moved state cloning into zbench's `before_each`/`after_each` lifecycle hooks using module-level variables: - `beforeEach` clones the cached state before each timed iteration - `afterEach` frees the clone after the iteration completes - `run` functions now operate directly on the pre-cloned instance Also removed the now-unused `cached_state` field from all benchmark structs and cleaned up the corresponding `addParam` calls. Applied to `process_block.zig` and `process_epoch.zig`. *AI disclosure: Claude was consulted for reviewing the approach and drafting this description. All code changes were authored manually.*
introduces - `sleeping_workers`: mark itself as asleep when no work is available, mark itself as not a `sleeping_worker` (i.e. `sleeping_workers -= 1` when it sees work - use `signal` instead of `broadcast` I suspect contention on threads causing regression on bls workload. This at least metrics wise on `feat3-sas` has some good effects (white line is redeployment): <img width="1656" height="446" alt="Screenshot 2026-05-28 at 9 30 20 PM" src="https://github.com/user-attachments/assets/2b06593d-f331-45ce-9a87-be9d35952fd8" /> Job wait time is also trending down: <img width="1691" height="450" alt="Screenshot 2026-05-28 at 9 32 18 PM" src="https://github.com/user-attachments/assets/d3de0992-37cb-49fa-a5c3-9f3a23506122" /> --------- Co-authored-by: Cayman <caymannava@gmail.com>
extracted from ChainSafe#347 Support both native call + binding
…hainSafe#387) `Signature.fromBytes` should do infinity check by default if not provided as an argument (in other words provided as `null`) source: https://github.com/ChainSafe/blst-ts/blob/86c49590d37d4e1dd44b3b5ba604132f3b51d99d/src/lib.rs#L301
instead of allocating 5 smaller arrays, we just use fixed size buffers (capped at `bls.MAX_AGGREGATE_PER_JOB`) within `AsyncAggRandData` and create that struct to send to worker threads @twoeths suggested this in a previous PR review but i wrongly shot it down (sorry :p )
for parity with rust, which also uses the pool only
By default --release on cargo build also compiles with a lot less guarantee checks, so we should do the same on the blst level
…regateSignatures (ChainSafe#389) We don't actually need to count the number of the results, we just write into the bufs sequentially
…hainSafe#346) ## Motivation State transition is dominated by PMT operations on `BeaconState`'s large basic-element lists (`Balances`, `EpochParticipation`, `InactivityScores`, ~1.4M items each) and per-field tree access for struct-shaped containers (`Validator`). On a mainnet fulu state with 2.18M validators, these account for the bulk of `processEpoch` and `processBlock` runtime. ## Description Five PMT-level changes that compose: 1. **`u64` payload column** — collapses every node kind's payload (branch left/right Ids, chunked_leaf pointer, container_struct vtable pointer, free-list link) into one machine word. State packs `[free_bit:1 | kind:3 | ref_count:28]` in `u32`. Cache validity moves out of `kind` into a `0xFF…` sentinel in the `root` column. Hot tree-walk visits touch only `state` (1 B) + 4-8 B from `payload`, fitting one cache line. 2. **`chunked_leaf`** — opt-in via `opts.chunked_leaf=true` on `FixedListType` / `FixedVectorType`. Bottom `k_log2 = 6` levels of the chunks subtree fold into one `*ChunkedLeaf` heap blob holding K=64 chunks. For 1M-item `List<u64>` pool metadata drops ~64× (256K Node.Id → 4096 ChunkedLeaves + 4096 heap blobs). Bulk read/write get SIMD-batched root recomputation and amortized CoW (one 2 KB memcpy per dirty leaf instead of 6 path clones per dirty chunk). 3. **`container_struct`** (originally ChainSafe#232) — a node kind whose payload is `*ContainerStructRef` (vtable + caller-allocated `T`). Backs `StructContainerType` for `Validator` etc. Field access = O(1) struct read instead of per-field tree walk; `hashTreeRoot` calls type's cached `get_root` directly. 4. **Pool dual-allocator** — `Pool` keeps two allocators routed by allocation kind: `page_allocator` for the MultiArrayList node columns (one large, infrequent allocation), and `allocator` (default `c_allocator`) for every per-node out-of-line heap blob — `ContainerStructRef`, `WrappedT`, and the 2 KB chunked_leaf blobs. Page-per-alloc on the small lane wastes ~70 GB of virtual address space at 2.18M validators on macOS arm64 and thrashes the TLB; the bucket allocator packs them densely. `Pool.init` switched to options-struct shape (`Pool.init(.{})` for production defaults). This unblocks `serializeValidators` / `getEffectiveBalanceIncrementsZeroInactive` / `getSingleProof` binding tests at mainnet scale (24 s → ~500 ms, 50× speedup, equal to main). 5. **Zero-copy validator access** — completes the container_struct value chain. PR ChainSafe#232 added `pool.getStructPtr(node, T)` but no list-iteration API was built on top, so callers still cloned the full 263 MB validators slice per epoch transition. This PR adds: - `StructContainerType.tree.getValuePtr(node, pool) -> *const T` — direct typed pointer into the pool's container_struct payload. - `ListCompositeTreeView.ReadonlyIterator.nextValuePtr() -> *const Element.Type` — list iteration that hands out per-element pointers as the depth-iterator walks the tree. - `BeaconState.validatorsPtrSlice(allocator) -> []*const Validator.Type` — random-access pointer slice for callers that need sort / parallel workers / multi-pass. The two APIs are complementary: iterator wins for single forward read passes (`epoch_transition_cache.init`, `getEffectiveBalanceIncrementsZeroInactive`); pointer slice wins for sort + random index access + parallel workers (`epoch_cache.init` calling `syncPubkeys`, `slashings_cache.buildFromStateIfNeeded`, `upgrade_state_to_altair`). 8 of 9 hot callers migrated; the last (`upgrade_state_to_electra`) keeps the value slice because its mutate-then-reread pattern would invalidate pointers. ## Bench `bench_process_epoch` and `bench_process_block` on mainnet era, fulu fork, slot 13336576 (**2.18M validators**), ReleaseFast, Apple Silicon. Both branches run with the same bench harness using `c_allocator` (no DebugAllocator overhead) for apples-to-apples comparison. `*_total` rows exclude the final `hashTreeRoot` (state-root recompute), which the bench tracks as its own segment. ### Process epoch (segmented breakdown, ms/run averaged over 50 runs) | step | main | this branch | speedup | |------|------|-------------|---------| | **epoch_total** | **418.0** | **74.9** | **5.58×** | | `before_process_epoch` | 193.8 | 39.9 | **4.86×** | | `inactivity_updates` | 58.6 | 4.0 | **14.7×** | | `rewards_and_penalties` | 80.8 | 14.2 | **5.69×** | | `effective_balance_updates` | 67.2 | 5.7 | **11.8×** | | `proposer_lookahead` | 16.5 | 10.9 | 1.51× | `before_process_epoch` (`EpochTransitionCache.init`) drops 4.86×: container_struct gives O(1) per-field reads on validators, and the `nextValuePtr` iterator skips the 263 MB clone that `validatorsSlice` used to do every epoch. `inactivity_updates`, `rewards_and_penalties`, `effective_balance_updates` get 5-15× from chunked_leaf making bulk reads/writes on `Balances` / `InactivityScores` / `EpochParticipation` SIMD-friendly + amortized CoW. ### Process block (segmented breakdown, ms/run averaged over 50 runs) | step | main | this branch | speedup | |------|------|-------------|---------| | **block_total** | **166.1** | **67.3** | **2.47×** | | `operations` | 162.9 | 64.0 | **2.55×** | | `block_header` | 0.243 | 0.244 | ~same | | `withdrawals` | 0.021 | 0.021 | ~same | | `execution_payload` | 0.201 | 0.196 | ~same | | `randao` | 1.088 | 1.140 | ~same | | `sync_aggregate` | 1.675 | 1.492 | ~same | `operations` (bulk of block processing) gets 2.55× — chunked_leaf on the balance writes plus zero-copy validator reads in `slashings_cache.buildFromStateIfNeeded`. `sync_aggregate`'s scattered sync-committee balance writes CoW a 2 KB `ChunkedLeaf` blob; its residual cost is BLS aggregate verification (~1.1 ms fixed, identical across branches). ### Linux verification (AMD EPYC 9V74, 16 vCPU codespace, ReleaseFast) Same fixture (mainnet era, fulu fork, slot 13336576, **2.18M validators**), 50 runs/step. Speedup ratios reproduce on Linux/x86; absolute numbers are higher than Apple Silicon due to per-core differences. #### Process epoch (segmented breakdown, ms/run) | step | main | this branch | speedup | |------|------|-------------|---------| | **epoch_total** | **666.0** | **123.0** | **5.41×** | | `before_process_epoch` | 344.8 | 69.7 | **4.95×** | | `inactivity_updates` | 67.6 | 4.4 | **15.4×** | | `rewards_and_penalties` | 112.9 | 24.1 | **4.69×** | | `effective_balance_updates` | 93.0 | 6.2 | **15.0×** | | `proposer_lookahead` | 43.8 | 15.5 | 2.83× | #### Process block (segmented breakdown, ms/run) | step | main | this branch | speedup | |------|------|-------------|---------| | **block_total** | **350.8** | **110.7** | **3.17×** | | `operations` | 346.0 | 106.4 | **3.25×** | #### Process block (end-to-end fused, ms/run) | variant | main | this branch | speedup | |---------|------|-------------|---------| | `process_block` (with BLS) | 49.3 | 38.5 | 1.28× | | `process_block_no_sig` | 10.63 | 2.79 | **3.81×** | `process_block_no_sig` (BLS bypassed) drops 3.81× — the optimizations land cleanly on the non-BLS portion. The fused 1.28× reflects ~36 ms going to BLS aggregate signature verification per block, which is unaffected by PMT changes. potentially fix ChainSafe#243
…hainSafe#367 merge skew (ChainSafe#394) ## Problem `main` CI is red: the `build & test` job fails to **compile** `test:state_transition`: \`\`\` src/state_transition/sync_committees_witness.zig:148:33: error: expected 1 argument(s), found 2 pub fn init(opts: InitOptions) Error!Pool { \`\`\` ## Root cause — a semantic merge conflict (merge skew) - **ChainSafe#346** (chunked-leaf) changed `Node.Pool.init(allocator, pool_size)` → `Node.Pool.init(opts: InitOptions)` (2 positional args → 1 options struct). - **ChainSafe#367** (`getSyncCommitteesWitness`) landed `sync_committees_witness.zig` in parallel, still calling the **old 2-arg** form: `Node.Pool.init(allocator, 500_000)`. Both PR branches were green because neither tree contained the *combination*: ChainSafe#346's branch didn't have `sync_committees_witness.zig` (it predates ChainSafe#367 and was never updated to the latest `main`), and ChainSafe#367's base still had the old `Pool.init`. Merging ChainSafe#346 into a `main` that already had ChainSafe#367 produced code git merged cleanly (different files, no textual conflict) but which no longer compiles. ## Fix One line — update the stale call site to the new `InitOptions` form (matching every other PMT test in the repo, both fields pinned to the testing allocator for leak tracking). `git grep` confirms this is the only remaining old-style call site. ## Verification `zig build test:state_transition` → **96/96 tests passed** (was: compile error). ## Prevention Consider enabling **"Require branches to be up to date before merging"** or a **GitHub merge queue** so PR CI runs against the real post-merge tree and catches this class of logical conflict that git can't see.
rework allocations around `verifyMultipleAggregateSignatures`. - we're batching on average about ~30 signature sets per batch on our highest load fleet (according to metrics), so a cap of about 32 makes sense for stack allocations. Anything beyond that, use heap allocations. - avoid copies for `msgs` which was unnecessary
…afe#386) ## Motivation `std.Thread.getCpuCount()` only reads the CPU affinity mask, so under a cgroup CPU quota (`docker --cpus=N` / k8s `limits.cpu`) it reports the host core count rather than the quota. That over-sizes the BLS verification thread pool at NAPI init, which can cause thread oversubscription and CFS throttling in CPU-limited containers. ## Changes - Add `src/cpu_count.zig` — `getNumCpus(gpa, io)` returns `min(cgroup quota, affinity)`, locating the cpu controller via `/proc/self/{cgroup,mountinfo}` and reading `cpu.max` / `cpu.cfs_quota_us` (cgroup v1 + v2). Parsing logic ported from the `num_cpus` crate. - The quota is the **minimum along the cgroup ancestor chain** (leaf up to the mount point), matching Rust `std::thread::available_parallelism`: the kernel enforces every level but each level's file reports only its own limit, so leaf-only reads miss LXC/Proxmox `cpulimit`, systemd `CPUQuota=` on a parent slice, and sub-cgroups inside a limited container. - Hardened beyond `num_cpus`: cgroup paths containing `:` (containerd's `…slice:cri-containerd:<id>` cgroupfs naming) are kept intact, and `..` paths (process outside its cgroupns root, `cgroup_namespaces(7)`) fall back cleanly instead of erroring. - Use it at NAPI thread-pool init (`bindings/napi/root.zig`) instead of `std.Thread.getCpuCount()`. - Run `test:cpu_count` in CI (the build-test job enumerates module test steps explicitly). Notes: - `/proc` pseudo-files report size 0, so they are streamed to EOF (`readerStreaming` + `allocRemaining`), not read by stat size (`readFileAlloc` reads empty). - Error policy: `getNumCpus` itself is fail-fast — a genuinely **absent** quota (non-Linux, no cpu controller, no cgroup mount, unresolvable path, unlimited) is `null` → affinity fallback, while a **broken read** of an existing resource (unreadable `/proc` or quota file, unopenable cgroup dir, malformed content) is an error, so a readable quota is never silently masked. The NAPI call site catches detection errors, logs a warning, and sizes by the affinity count — a sizing probe must not prevent the module from loading. - `ceilDiv` is overflow-safe; the allocator is passed in by the caller (the module stays allocator-agnostic / libc-free). ## Testing - 22 unit tests covering the parse-and-resolve pipeline and the ancestor walk; test vectors adapted from `seanmonstar/num_cpus` (MIT). - Verified end-to-end in Linux containers (OrbStack): `--cpus=2` → 2, `--cpus=4` → 4, `--cpus=1.5` → 2 (ceil), unlimited → host count. The old `getCpuCount()` returned the host count in all cases. - Ancestor walk verified end-to-end: with `--cpus=2` and the process moved into an unconstrained child cgroup (`0::/child`, no `cpu.max` of its own), the walk finds the limit at the cgroupns root and returns 2 — leaf-only reads return the host count here. - Colon handling verified end-to-end: a process in `0::/x.slice:cri-containerd:y` resolves its cgroup dir and returns the quota. --- 🤖 This PR was developed with AI assistance (Claude Code).
we were missing the small MSM codepath previously, though this did not affect perf at all when deployed. Still, let's implement it for parity.
…nSafe#397) Instead of getting the pk from cache and then passing it back through napi, we just pass the already known indices into the native layer and let the cache do the work. On `feat3-super`, this halved pk aggregation time (8s to 4s): <img width="731" height="290" alt="Screenshot 2026-06-09 at 5 26 16 PM" src="https://github.com/user-attachments/assets/0d4d0f1a-7097-4dac-bbf1-5b18db641c55" /> Notably, sig set verification time per set went up (530us -> 600us), but I suspect this is due to it becoming the bottleneck now. Job wait time has gone down (37-38ms to about 31-32ms), and we're doing less verifications in batches (88% to 87% roughly) <img width="1463" height="777" alt="Screenshot 2026-06-09 at 5 27 09 PM" src="https://github.com/user-attachments/assets/4b68ec69-f29f-4620-9ebb-26691e7dd451" /> block processing looks better with this: <img width="737" height="573" alt="Screenshot 2026-06-09 at 5 29 44 PM" src="https://github.com/user-attachments/assets/2f6bc9a7-36fc-4bbd-8b30-4baa3007281e" /> verification metrics across the board trending down with this: <img width="1460" height="829" alt="Screenshot 2026-06-09 at 5 30 39 PM" src="https://github.com/user-attachments/assets/7ceeeb99-fd9a-4a2b-93ec-3736078c5b47" />
final release for BLS testing
contains changes from ChainSafe/zbuild#9
Contains zbuild update
while working on bls bindings and checking for usages here I noticed some inconsistencies, notably we pass `null` to decide if sigs/pks should be validated. Then we do some logic to decide defaults. Instead put these in a `BlsOpts` struct which we set defaults for, so we just pass empty structs for when we don't want validation. Also added doc comments.
Update gloas types to match v1.7.0-alpha.10 Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…r set (ChainSafe#432) also add a test
…ty (ChainSafe#400) ## Motivation A review of the chunked-leaf packing and zero-copy tree-view paths surfaced a handful of memory-safety issues. ## Description - **Composite `set`/`push`/`setValue` ownership.** Make them caller-retains-on-failure (the std/Ghostty model): `chunks.set` no longer deinits the passed view on its own reservation OOM, and `setValue`/`pushValue` carry an `errdefer` over the view they build. Fixes a double-free in `load_state` (`applyModifiedValidators` / `appendNewValidators`), where the caller's `errdefer` and `set`'s self-free both ran on the `ensureUnusedCapacity` OOM path. - **ChunkedLeaf root recompute.** `getRoot`'s `.chunked_leaf` arm uses a reused Pool scratch field + `computeRoot` instead of `computeRootAllocating`, removing the only `@panic("OOM")` in `src/` (aborted the Node.js host on OOM) and the per-recompute malloc/free on the hashTreeRoot path. A Pool field rather than a stack buffer because `getRoot` recurses to tree depth (~47 on a mainnet validators path), and chunked_leaf is a recursion leaf so one shared scratch is always safe. - **`sumTargetUnslashedBalanceIncrements`.** Assert `participations.len == validators.len`; the zero-copy validator pointer slice turns a cross-list length mismatch into a garbage-pointer dereference. - **`ContainerTreeView.deserialize`.** Add the `errdefer pool.unref(root)` its two siblings already carry, so an `init` OOM no longer strands the deserialized subtree. - **Delete dead `fillToLength` / `fillToDepth`.** Pool-corrupting on first use, zero callers, superseded by `fillWithContents`. - **`ChunkedLeaf.computeRoot` trailing-zero assert.** Assert chunks past `len` are zero — a violated invariant would silently hash stale data into a wrong (consensus-divergent) root. - **`getChunkedLeafPtr` exclusive-ownership assert.** Assert `refCount() == 0` before handing out a mutable blob pointer; in-place mutation of a shared node corrupts every tree referencing it. - **List `setLength` → `growTo`, grow-only.** New positions read as zero (the data subtree is already the virtual zero subtree), so growing is O(1) and correct by construction; a bare length *cut* only rewrites the length mix-in, leaving stale chunk data in the merkleized root — a silent wrong hashTreeRoot. Now asserted (`new_length >= _len`) and documented: shrinking must go through `sliceTo`. All production callers grow (upgrade-to-altair); the one shrink user (the loadState trim test generator) now truncates a value-level state, keeping the test fixture independent of `sliceTo`, which loadState itself uses to trim. - **`ContainerTreeView.getFieldRoot` per-call pool-node leak.** On a dirty basic field it built a temporary node from the cached value and never unref'd it — one orphaned pool slot per call, invisible to leak detectors (`Pool.deinit` frees every in-use slot on teardown). Mirrors the fix its `StructContainerTreeView` sibling already carries: copy the hash into a per-field backing store, unref the node, return a pointer into the store. Pinned by a `getNodesInUse`-baseline test (10 calls leaked 10 slots before; baseline-stable after). - **Cloning a dirty tree view — two latent bugs.** A transfer-clone deliberately *drops* uncommitted writes (the rc-0 staged nodes are exclusively owned and can't be shared in the refcount model). The composite path handles this correctly; the basic-list path had two gaps. (1) **Leak:** `TreeViewState.clone` dropped the staged `children_nodes` entries *without* `unref`, orphaning a pool slot (and any chunked_leaf blob) per dropped write — invisible to leak detectors because `Pool.deinit` frees every in-use slot on teardown; now caught by a `getNodesInUse` baseline. (2) **`_len` skew:** the clone kept the uncommitted `_len`, so a dropped push left length N+1 over an N-element tree → wrong root on commit; the clone now reflects the committed length. Both latent (callers commit before cloning). - **`StructContainerTreeView.clone` semantics.** It committed the source first, so uncommitted writes survived into both views and `clone()` mutated the source's root — the opposite of every other view's drop semantics. It now clones the committed state and drops uncommitted writes (from the source too on transfer). - **`ProofFixture` dangling Pool (sync-committee witness tests).** The fixture returned its `Pool` by value after handing `&pool` to the views, leaving them pointing at a dead stack frame; the tests passed only by stack-layout luck. The fixture now initializes in place. - **Allocator-lane routing.** Two transient buffers (the chunked-leaf serialize Id scratch, the compact-multiproof arena) allocated from the page-allocator lane reserved for the pool's node columns; they now use the general allocator lane.
…nSafe#347) 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). 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:
`config.zig` was broken in various places: - `getValueUint64()` is a non-existent API, this shouldn't even have been merged (i was the reviewer so it's my bad) - we were unnecessarily using an allocator when we could've just had fixed sized buffers for `config_name` and `blob_schedule`, both of which probably won't change that frequently anyway
Contributor
Author
|
bindings test fails in the same manner as on |
spiral-ladder
approved these changes
Jun 17, 2026
spiral-ladder
left a comment
spiral-ladder
left a comment
Member
There was a problem hiding this comment.
changes look good, the gemini comments are a bit pedantic. Thanks for the PR! Let me investigate the CI failures
Contributor
Author
|
Thanks @spiral-ladder, I skipped them in order not to expand the scope by applying the rule that was generally not applied at the time (at least that's what I remember). I can take care of that now if needed. |
Merged
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 < 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>
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
6 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.
chore: remove merge transition code
Closes #130 (supersedes stale PR #145)
Context
The merge transition (Bellatrix → PoS activation) happened on mainnet in September 2022. All live networks have long passed it, and lodestar-z doesn't support pre-Bellatrix sync. The related code paths are dead.
PR #145 targeted the old runtime-dispatched version of these functions, which was replaced by the comptime fork refactor in #190. This PR applies the removal against current
main.Changes
pre_mergevariant fromExecutionPayloadStatusenum.pre_mergecheck inprocess_blob_kzg_commitments.zig.pre_mergecheck inprocess_execution_payload.zigisMergeTransitionBlockfunction fromexecution.zig(zero callers)isMergeTransitionCompleteis intentionally kept — it's still used byisExecutionEnabledfor the edge case where a post-Bellatrix state hasn't yet seen an execution payload.AI disclosure: Claude was consulted for reviewing the diff and drafting this description. All code changes were authored manually.