types, node: parallelize per-att_data aggregate FFI (#907)

[ch4r10t33r]

Summary

• Refactor computeAggregatedSignatures to prepare each AttestationData serially, then run independent recursive STARK FFI calls in parallel when the aggregator thread pool is available and more than one entry needs proving.
• Sort att_data keys deterministically before building output (stable block/aggregate ordering across runs).
• Wire the aggregator path in forkchoice.zig through computeAggregatedSignaturesWithPool.

Complements #919 (parallel aggregate submission / in_flight skips). That PR improves slot throughput; this one targets the ~11s per-FFI wall clock when a slot window contains multiple att_data entries (~2 per worker call per #907 finding 1).

What this does not fix

• Single att_data with 7–8 gossip sigs still pays one full ~11s STARK (cost is input-shape independent in leanMultisig). Trivial 1-sig cases remain covered by node, types: skip recursive aggregation for 1-sig + 0-child trivial inputs (#907) #908 aggregator pre-filter.
• Does not address Investigate slow slot_interval / tick duration (event-loop starvation vs nominal 0.8s) #863 event-loop backlog or peer aggregate merge timing (timely=none).

Test plan

• zig build test --summary all
• Devnet: zeam_aggregate_worker_duration_seconds p50 drops when slot window has 2 att_data (expect ~1× STARK time instead of ~2×)
• Cross-check with node, chain: parallel aggregate submission for #907 #919 on aggregator image for combined slot skip + worker latency improvement

[zclawz]

Critical review result: requesting changes / not approving yet.

I found one blocker in the new parallel aggregation path:

Blocking: aggregate_max_inflight > 1 can produce duplicate aggregates from identical snapshots

This PR removes the previous aggregate_group.concurrent_limit=1 guard and allows multiple aggregate workers to run concurrently (aggregate_max_inflight is set to up to 3 for aggregators). The three-phase snapshot/compute/merge contract prevents direct map races, but it does not prevent two in-flight workers from snapshotting the same attestation_signatures before either one reaches commit.

Concrete sequence:

1. interval N starts worker A; A snapshots attestation signatures for {slot-1, slot} and enters the ~11s FFI phase.
2. before A commits/removes those live signature entries, interval N+1 starts worker B; B snapshots the same live entries.
3. A commits, appends latest_new_aggregated_payloads, removes the live gossip entries, and returns/publishes aggregate(s).
4. B later commits from its stale snapshot. The removal loop is harmless/no-op because A already removed those vids, but B still appends its locally computed payloads and returns/publishes the same aggregate(s).

The current merge code unconditionally appends new_payloads_local and results from the worker's snapshot; it does not re-check under signatures_mutex that the live map still contains any consumed participants for that att_data. So concurrent workers are memory-safe, but not semantically idempotent.

This is especially likely here because aggregate workers are expected to take ~11s while interval ticks are sub-slot, so the new in-flight cap enables exactly this overlap.

Suggested fixes:

• keep only one outer aggregate worker in flight, and parallelize only the per-att_data FFI work inside that worker; or
• under signatures_mutex at commit, suppress both latest_new_aggregated_payloads insertion and returned/published results for an att_data if none of the snapshot-consumed live vids are still present; or
• add an explicit per-slot/att_data in-progress claim before compute so later workers cannot snapshot/compute the same work.

I also tried to run zig build test --summary all, but this checkout’s default zig is 0.15.2 while the cached dependencies/build scripts expect the repo’s pinned toolchain/API (b.graph.io etc.), so local validation is blocked by toolchain mismatch in this environment rather than by a PR-specific test failure.

[zclawz]

Follow-up review on 9e51d6e7b58a281734a1d07f2e9f7736147695f2: code review LGTM.

The blocker I raised is addressed:

• aggregator nodes now keep aggregate_max_inflight = 1, so only one outer interval aggregate worker can snapshot/commit at a time;
• the expensive part is still parallelized inside that single worker across per-att_data FFI calls;
• the added commit-time duplicate suppression is a reasonable defensive backstop for any future/non-aggregator aggregate_max_inflight > 1 use.

Validation I could do locally:

• git diff --check origin/main...pr-920 passes.
• I re-read the updated snapshot/compute/commit path and the new suppression helper/tests.

I could not run zig build test --summary all locally because this host has Zig 0.15.2 while build.zig.zon now requires 0.16.0. GitHub CI is still in progress at the time of this comment, with lint already green.

Per standing project guidance, this is a regular PR comment rather than a formal GitHub “Approve” review.

[zclawz]

Approved after review. The parallel per-att_data aggregation refactor looks sound to me: prep stays serial, FFI work is scoped through the shared ThreadPool, output order is deterministic, and the aggregator path is wired through the pool while keeping outer aggregate in-flight capped. Local validation is blocked by this workspace Zig/dependency mismatch, but CI lint/macOS build are green and remaining Linux/test jobs are still running with no failures at approval time.