build, cli, rust: ThinLTO multisig-release + --rayon-threads + leanMultisig 5eba3b1 bump

[ch4r10t33r]

Summary

Three build/runtime tuning levers the #899 investigation has not yet exercised on the production aggregator image. All are conservative defaults — behaviour is unchanged unless an operator opts in (items 1–2) or unchanged on the spec level (item 3).

1. [profile.multisig-release] Cargo profile

The shared [profile.release] is forced to lto = false, codegen-units = 16 to avoid sys_alloc_aligned-style symbol collisions between risc0 and openvm. That constraint does not apply to the --prover=dummy build — it links neither risc0 nor openvm (--features libp2p,hashsig,multisig), so cross-crate LTO is safe.

The new multisig-release profile inherits release and sets lto = "thin", codegen-units = 1 so the leanMultisig prover hot path gets the same level of inlining that single-prover risc0/openvm builds already get. opt-level stays at the inherited 3 — switching to s/z with CGU=1 miscompiles the prover on x86_64 Linux (see [profile.openvm-release] comment in rust/Cargo.toml, zeam#734, leanEthereum/leanVM#198).

build.zig is updated so -Dprover=dummy cargo-builds with --profile multisig-release and the static lib is read from rust/target/multisig-release/libzeam_glue.a.

Observed locally (fresh build, x86-64-v3 default target-cpu):

	Before (release)	After (multisig-release)
libzeam_glue.a	91M	27M

3.4x size reduction is consistent with cross-crate dead-code elimination actually running; we expect a similar shape on prover throughput.

2. --rayon-threads N CLI flag

pkgs/cli/src/node.zig splits the post-system-thread budget roughly half-and-half between the Zig thread pool and the rayon pool used by the multisig prover. That split is appropriate for non-aggregator nodes (where rayon is also entered from verification on Zig workers), but on a CPU-rich aggregator the produce-path FFI is the bottleneck and a larger rayon pool measurably shortens per-pass build time. Today the only way to bump rayon is a rebuild.

This adds --rayon-threads N (default unset = current behaviour) so operators can re-tune the split per-deployment without rebuilding. We also log cpu_count, the zig worker count, and the chosen rayon thread count at startup so the same number is visible from logs without cat-ing /proc/$PID/status:

[node] thread pools: cpu_count=16 zig_workers=6 rayon_threads=12 (rayon override via --rayon-threads)

Operator-typo guard (added after review feedback): if --rayon-threads exceeds cpu_count, we log a warn line. We don't reject because cgroup-limited containers commonly see getCpuCount over-report relative to the effective quota — see the comment in pkgs/cli/src/node.zig for the cases that motivated warn-not-reject.

3. leanMultisig pin bump 2eb4b9d → 5eba3b1 (devnet4 head)

Folded in from #905 (closed). Devnet operators reported ~16 sig/s aggregation throughput on the previous pin (Apr 17) versus ~37 sig/s on the leanMultisig benchmarks against 5eba3b1 (May 12) on the same hardware. The dominant contributor is leanMultisig commit 939a767, which removed superfluous #[inline] annotations in eq_mle.rs for a measured +10% on AVX-512 (Hetzner AX42-U, same hardware class as the devnet aggregator hosts) with no NEON regression. Splitting this out would have forced two image cuts and two redeploys to capture the full aggregator-perf win, so it belongs with items 1–2.

Commits picked up (2eb4b9d → 5eba3b1):

sha	what
0fbf27c	fix #198
89e0320 → 6a0d8fa	"eprint 055 ordering" commit + revert (no net diff)
d853f1c	update deps
939a767	remove #[inline] in eq_mle.rs (+10% AVX-512, neutral NEON)
7a71c0f	eq_mle base-case correctness fix (was using packing_width instead of log_packing_width)
5fbd5bf	mirror Plonky3 PR #1600: NEON carry-critical dot-product regression tests
e5c2183	leanSig dep flipped to main (devnet4 merged into main)
5eba3b1	rec_aggregation: expose structured BenchmarkReport (port from devnet5)

API adjustment in hashsig-glue. e5c2183 pulls in leanSig:main, which renamed SchemeAbortingTargetSumLifetime32Dim46Base8 → SIGAbortingTargetSumLifetime32Dim46Base8 in the production lifetime_2_to_the_32 instantiation. The test-only lifetime_2_to_the_8 kept the original Scheme... name, so only the production config import needs to flip; the test-config and test_scheme paths are untouched. No other zeam code references the old type name.

Out of scope: recursive-aggregation review. The same review observation that flagged this bump also noted that, per spec, aggregators run recursive aggregation whenever helper payloads are available — 1.5–6 s wall — versus <600 ms for the non-recursive (gossip-only) fast-path that zeam already takes when selected_children.items.len <= 1 (pkgs/types/src/block.zig lines 651–665). Whether to widen that fast-path to always skip recursion on the produce path is a spec/operational design choice and is intentionally not part of this PR. Tracked separately.

Test plan

• zig fmt --check .
• cargo fmt --manifest-path rust/Cargo.toml --all -- --check
• cargo clippy --manifest-path rust/Cargo.toml --workspace --no-default-features --features=libp2p,hashsig,multisig -- -D warnings
• zig build -Dprover=dummy — clean build with multisig-release profile; produces rust/target/multisig-release/libzeam_glue.a at 27M
• zig build test --summary all — all targets pass (xmss FFI test at 53 s, the one that exercises any ABI break from the dep bump)
• zig build simtest --summary all — passed (combined branch)
• CI verification of -Dprover=risc0 / -Dprover=openvm / -Dprover=all builds — those arms are unchanged but worth confirming the dispatch in addRustGlueLib still picks the right per-prover artifact
• Re-run benchmark on aggregator host with the new image; expect ~2x in lean_committee_signatures_aggregation_time_seconds p50
• Watch zeam_aggregate_skip_total{reason="in_flight"} — should fall toward 0 since fewer passes will spill across the next slot
• Rolling-deploy on one zeam_8 aggregator first; compare per-slot publish counts to a control before propagating

Related

• node, aggregation: zeam aggregate-build is 5–7x slower than ethlambda on devnet (2–3s vs 0.4s) #899 — parent investigation
• Limit aggregate signature builds to active slot #900 — slot-window the att_data set (already merged); this PR is its build-tuning sibling
• node, metrics: surface aggregator-publish counter and gossip-sig coverage #902 — aggregator observability counters (companion PR in this series)
• rust, hashsig-glue: bump leanMultisig to devnet4 5eba3b1 (~2x aggregation throughput) #905 — closed, folded into this PR (commit e99573b8)

[zclawz]

Adversarial review started from Telegram request. I’ll inspect the diff and post findings here.

[zclawz]

Adversarial review — no blocking defects, but several things worth naming explicitly.

---

✅ What is correct and well-reasoned

[profile.multisig-release]

• The symbol-collision constraint that forces lto = false, codegen-units = 16 on [profile.release] genuinely does not apply to --features libp2p,hashsig,multisig (neither risc0 nor openvm is linked). ThinLTO here is safe.
• opt-level is correctly kept at the inherited 3. The opt-level = "z" + codegen-units = 1 miscompile on x86_64 Linux is real (zeam#734) and the comment is accurate.
• The 3.4× binary size drop reported locally is plausible from dead-code elimination kicking in. Runtime improvement won't be that dramatic but should be measurable.

--rayon-threads flag

• @max(1, override) correctly clamps --rayon-threads 0 to 1; the log line states the effective count and whether it came from the override, which is good for post-mortem debugging.
• setRayonThreads is still called before setupVerifier, preserving the required ordering.
• rayon_threads is set in the start_options struct literal in main.zig (line 853), not in getStartNodeOptions / buildStartOptions. That is the right place — buildStartOptions fills derived/loaded values; this comes straight from the CLI arg.

---

⚠️ Issues worth addressing

1. CI does not exercise the new multisig-release artifact path (medium)

zig build run -Dprover=dummy in CI does implicitly trigger the dummy build, so the compile path is hit. However, the Rust cache key for that job (cargo-test-${{ hashFiles('**/Cargo.lock') }}) is shared with the test and simtest jobs, and the build-all-provers job uses cargo-all-provers-* as its key. Neither key distinguishes multisig-release vs release artifacts. On a warm cache hit, CI will try to link against a stale or absent rust/target/multisig-release/libzeam_glue.a and fail in a confusing way.

The PR test plan already calls out "CI verification of risc0/openvm/all builds — those arms are unchanged but worth confirming". I'd extend that: the dummy path is now materially different from what CI has cached before (new profile, new output directory), so a first-run CI failure due to cache pollution is likely unless the cache is busted. Consider adding multisig-release to the Swatinem cache key suffix for the dummy-prover job, or noting this explicitly.

2. Dockerfile / auto-release don't pass -Dprover (default = dummy) — this is fine, but worth a note

The Dockerfile and auto-release.yml both call zig build without -Dprover, so they default to .dummy and will now compile with multisig-release. The switch is correct and intentional, but it is invisible in both files — someone reading the Dockerfile alone has no indication the Rust profile changed. A one-liner comment would help future readers. Not blocking.

3. --rayon-threads accepts values > cpu_count with no warning (low)

--rayon-threads 256 on a 4-vCPU box is silently accepted and logs rayon_threads=256. Rayon's scheduler handles over-subscription at runtime, so it won't crash, but an operator typo (e.g. confusing thread count with CPU index) produces a misleading log line. A if override > cpu_count warning would cost three lines and prevent confusion. Not blocking, but consider it.

4. --rayon-threads not plumbed through getStartNodeOptions / buildStartOptions — intended, but document why

This is correct (the value comes from the CLI arg directly, not from config YAML). However, buildStartOptions sets opts.chain_worker_enabled, opts.is_aggregator, etc. from node_cmd. rayon_threads does the same but is set in the struct literal rather than by the helper. That is consistent with how chain-worker was wired but could confuse a reader who expects all node_cmd.* → opts.* assignments in buildStartOptions. A one-line comment in buildStartOptions like // --rayon-threads is set in the NodeOptions struct literal in main.zig would close the loop.

---

✅ Not a bug

• --rayon-threads 0 → clamped to 1, correct.
• zig fmt --check passes on all three changed Zig files (verified locally).
• risc0, openvm, and all prover arms in build_rust_project and addRustGlueLib are untouched — they still use their respective artifact directories.
• The multisig-release profile correctly inherits from release, so strip = "symbols" is still in effect (binary is stripped).

---

Summary

Two genuine concerns: CI cache pollution risk on first run after merge (medium, worth a PR checklist item or cache key tweak), and the lack of a >cpu_count warning on --rayon-threads (low, nice-to-have). Everything else is well-structured. No correctness defects found.

Per repo convention this is a regular PR comment, not a formal GitHub Approve.

[ch4r10t33r]

Thanks @zclawz — both items addressed in 4d5ae4d.

1. Cache pollution. Bumped every Swatinem/rust-cache@v2 key in .github/workflows/ci.yml (and auto-release.yml) to hash rust/Cargo.toml in addition to Cargo.lock:

key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock', 'rust/Cargo.toml') }}

Six cache scopes touched (lint, build, build-all-provers, test, dummy-prove, docker-build in ci.yml + the cargo-docker key in auto-release.yml). Picked this over a hard -v2 suffix because it self-busts on any future profile add/remove/rename, not just this one. The merge of #903 itself flips the hash, so the first post-merge run starts from a clean cache and the test / simtest / dummy-prove jobs all populate the new multisig-release/ artifact together — no half-restored target dir to trip the linker. Left risc0.yml alone since it already hashes **/Cargo.toml.

2. --rayon-threads over-subscription. Added a warn (not reject) right after the existing thread-pool info log in pkgs/cli/src/node.zig:

if (options.rayon_threads) |override| {
    if (@as(usize, override) > cpu_count) {
        self.logger.warn(
            "--rayon-threads {d} exceeds detected cpu_count={d}; rayon over-subscription typically reduces throughput. Verify this is intentional.",
            .{ override, cpu_count },
        );
    }
}

Warn rather than hard-reject because cgroup-limited containers commonly see std.Thread.getCpuCount over-report relative to the effective quota, so rejecting would break a legitimate use case. The two log lines now surface side-by-side at startup, making the typo case unmistakable.

Pre-commit checks all green: cargo fmt --check, cargo clippy --features=libp2p,hashsig,multisig -- -D warnings, zig fmt --check ., zig build test --summary all, zig build simtest --summary all.

[zclawz]

Re-review after 4d5ae4d: LGTM from code review.

The two concerns from my adversarial review are addressed:

• CI/Rust cache keys now include rust/Cargo.toml everywhere relevant (ci.yml and auto-release.yml), so the new multisig-release profile/output dir should bust caches instead of restoring a target dir that lacks rust/target/multisig-release/libzeam_glue.a.
• --rayon-threads > cpu_count now emits a startup warning while still allowing intentional oversubscription. Good tradeoff.

I re-checked the full diff and don’t see new correctness issues:

• .dummy build path consistently uses --profile multisig-release and links rust/target/multisig-release/libzeam_glue.a.
• .all, .risc0, and .openvm paths remain on their existing profiles/artifact dirs.
• setRayonThreads still runs before setupVerifier.
• --rayon-threads 0 still clamps to 1.
• zig fmt --check build.zig pkgs/cli/src/main.zig pkgs/cli/src/node.zig passes locally.

CI is still in progress on the latest commit at the time of this comment, but from code review I’m happy with the PR. Per repo convention, this is a regular LGTM comment rather than a formal GitHub Approve review.

[zclawz]

Thanks — this looks good to me. Hashing rust/Cargo.toml into the cache keys is the better self-busting fix than a one-off suffix, and the warn-not-reject behavior for --rayon-threads handles the over-subscription typo case without blocking legitimate container setups. The green pre-commit checks are reassuring too.

[anshalshukla]

Approving it although I don't like how we are using rayon threads mingled with zig thread pool but that has already moved into the codebase so maybe a proper cleanup can be done later with proper profiling of threads