chore: merge next into monorepo-split/labs - #25190
Open
fcarreiro wants to merge 28 commits into
Open
Conversation
…couple vm2 + AVM fuzzer from world_state Introduces a standalone world_state_reference component (ns bb::world_state): a minimal in-memory reference of the Aztec world-state trees plus the world-state vocabulary (MerkleTreeId, WorldStateRevision, getMerkleTreeName), vm2-free and single-threaded so it builds under the fuzzing preset. A thin adapter in vm2 implements the AVM's LowLevelMerkleDBInterface by wrapping it. The WorldState-backed PureRawMerkleDB and the simulate_*_with_existing_ws entry points are deleted; the AVM fuzzer and test tester run on the reference, production continues through the WsdbIpcMerkleDB IPC client. The fidelity gate (reference vs an ephemeral real WorldState) moves into world_state_tests, so vm2 no longer references world_state at all, tests included.
## The coupling `cycle_group` took its MSM offset generators from `crypto::generator_data` — the pedersen generator store — via a `GeneratorContext` parameter threaded down `batch_mul` / `fixed_batch_mul`. That store is shared, mutable, and grows in place. Its own header documents it as not thread-safe: > This is not thread-safe. Each process that uses a `generator_data` object may extend `generator_data` if more generators are required ... either each process must use an independent `generator_data` object or the author must KNOW that `generator_data` will not be extended by any process. It also returns a `std::span` into a `std::vector` that a later call for the same domain reallocates, so a view handed to one caller can be freed underneath it. Both halves of that contract are violated in practice. `cycle_group::batch_mul` holds one of those spans across an entire MSM build, and `bb aztec_process` derives verification keys for several circuits on concurrent `std::thread`s against the shared singleton. The result is a rare, unreproducible `cycle_group: Point is not on curve` abort during `noir-contracts` bootstrap — [an example run](http://ci.aztec-labs.com/1786023406574272), [the failing contract](http://ci.aztec-labs.com/44c19c0d4f4041c4). The `GeneratorContext` parameter carrying this dependency was never once supplied. Every call site in `stdlib/` and `dsl/` defaulted it, and `ipa.hpp` passed a literal `{}` purely to reach the `table_bits` argument sitting behind it. ## What this does Adds `stdlib/primitives/group/cycle_group_offset_generators.hpp`, a store that owns exactly this one concern: ```cpp template <typename Curve> class cycle_group_offset_generators { public: static constexpr std::string_view DOMAIN_SEPARATOR = "cycle_group_offset_generator"; static const cycle_group_offset_generators& default_generators(); std::span<const AffineElement> get(size_t num_generators) const; }; ``` The design aims at removing the brittleness rather than guarding it: - **Prefix-only.** No offset or slice parameter. Generator `i` depends on nothing but the domain separator and `i`, so no request can change what another request sees. That is all cycle_group ever needed from the general store. - **Append-only blocks.** Growth allocates a new block and retains the superseded one, so a span already handed out stays valid however many times other threads call `get` afterwards. Growth doubles, so the retained set stays at a handful of entries. - **An instance, not a hidden global.** Circuit construction shares `default_generators()`, but the store is an ordinary object. This is what lets the tests below be non-vacuous — see the note on that. - **Locked.** A plain `std::mutex`; the critical section is a size check plus a rare derivation. `GeneratorContext` disappears from cycle_group's API, `fixed_batch_mul(points, scalars, {}, 8)` in `ipa.hpp` becomes `fixed_batch_mul(points, scalars, 8)`, and the now-unused `crypto/pedersen_commitment/pedersen.hpp` include comes out of both cycle_group files. **`crypto::generator_data` and the pedersen path are untouched.** ## Why memoize Measured here: `derive_generators` costs roughly 65us per generator — 550us for 8, 4.3ms for 65, 84ms for 1024. Deriving per `batch_mul` would be a real regression, so the store memoizes, and doubling blocks keep a circuit made of many differently sized MSMs to a handful of derivations rather than one per MSM. ## Verification keys are unaffected Offset generator `i` is the same point as before: same domain separator, same derivation, same prefix semantics — the previous path also requested offset 0. `MatchesDerivation` and `PrefixIsStableAcrossRequestSizes` pin exactly that, and the circuit tests below carry fixed expected values. ## Tests `stdlib/primitives/group/cycle_group_offset_generators.test.cpp`: | Test | What it pins | | --- | --- | | `MatchesDerivation` | Values equal `derive_generators` for the domain | | `PrefixIsStableAcrossRequestSizes` | Generator `i` does not depend on the request size | | `GetKeepsPreviouslyReturnedGeneratorsValid` | A span stays valid after a larger request grows the store | | `ConcurrentGetReturnsOnCurvePoints` | Concurrent `get` never yields an off-curve point | These were checked against a deliberately reintroduced bug — replacing the block on growth instead of retaining it. `PrefixIsStableAcrossRequestSizes` and `GetKeepsPreviouslyReturnedGeneratorsValid` go red deterministically, and green again once the retain is restored. Two details are worth knowing before editing these tests, because both produced a silently passing suite first time round: - The store has to be an **instance**. With function-local statics the cache is process-global, the growth path is live only for the first few assertions, and everything afterwards passes trivially — the first draft passed even with the bug present. - The deterministic test **sprays the freed size range** before re-reading. Without that, a stale span reads its own old contents back and passes while pointing at released memory. `ConcurrentGetReturnsOnCurvePoints` is the weakest of the four and is not the primary regression net: with the map race gone the residual failure is a dangling read, and freed-but-unreused memory usually still reads back correct. It is kept as the symptom-level reproduction and as an ASan/TSan target, with the two deterministic tests doing the real work. ## Run Full `cmake --build build` clean, including `bb`, `bb-avm` and `bb-avm-sim`. ``` stdlib_primitives_tests 1349 passed, 0 failed (128 cycle_group) dsl_tests 551 passed, 0 failed commitment_schemes_tests (IPA) 30 passed, 0 failed crypto_pedersen_commitment_tests 2 passed, 0 failed crypto_pedersen_hash_tests 6 passed, 0 failed ``` ## Note on scope This removes cycle_group's dependence on the shared store; it does not fix the store itself. `generator_data::get` still hands out spans it can later invalidate, which remains a hazard for the pedersen path and for any future caller, and is worth addressing separately. --- *Created by [claudebox](https://claudebox.work/v2/sessions/877d88cba221b585/jobs/8) · group: `slackbot` · requested by Sergei · [Slack thread](https://aztecfoundation.slack.com/archives/C04ALP0UQHH/p1786024347064329?thread_ts=1786024347.064329&cid=C04ALP0UQHH)*
## Summary The nightly debug build failed in `AcirComponentsCheckTest.DetectsUnconstrainedWitnesses` because the test intentionally shrinks `builder.real_variable_index` to simulate a missing ACIR witness. In debug STL mode, `StaticAnalyzer` then indexes trace witness `10` through that shortened vector and aborts before `ComponentsChecker` can report the intended `UNCONSTRAINED` error. This PR makes `ComponentsChecker` detect ACIR witnesses missing from the builder-side `real_variable_index` immediately after building the ACIR graph and return `UNCONSTRAINED` errors before constructing `StaticAnalyzer`. It also avoids indexing out-of-range range-list entries during classification and tightens the regression test to assert the missing witness is named in the error. Failed run investigated: https://github.com/AztecProtocol/aztec-packages/actions/runs/29720683064 ## Testing - `cmake --preset debug -DAVM_TRANSPILER_LIB=` - `cmake --build --preset debug --target acir_components_check_tests` - `NATIVE_PRESET=debug barretenberg/cpp/scripts/run_test.sh acir_components_check_tests AcirComponentsCheckTest.DetectsUnconstrainedWitnesses` - `./barretenberg/cpp/build-debug/bin/acir_components_check_tests` - `git diff --check` --- *Created by [claudebox](https://claudebox.work/v2/sessions/0ee4283d1eda583c/jobs/1) · group: `slackbot` · [Slack thread](https://aztecprotocol.slack.com/archives/C04ALP0UQHH/p1784527672929669?thread_ts=1784527672.929669&cid=C04ALP0UQHH)*
Removes `client11` from the mainnet RPC consumers. The Terraform removal was already applied with a guarded plan of 0 added, 0 changed, and 2 destroyed (KongConsumer and ExternalSecret). The GCP secret was deleted after verifying the Kubernetes resources were gone.
See [merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md). This is a merge-train.
…db/bb-avm-sim services Hardens the generated IPC packages' spawn-and-connect path against the failure family behind #24802, and makes bb-avm-sim process failures invisible to everything above the AVM pool. Servers listen before heavy init: aztec-wsdb creates its socket before WorldState construction and bb-avm-sim before its upstream wsdb/CDB connects, so clients connect into the accept backlog immediately and the connect backstop only covers exec + linking + reaching listen(). bb-avm-sim installs SIGUSR1 and lifecycle handlers (incl. parent-death monitoring) before the socket is reachable; upstream connect budget 5s -> 60s. ipc-runtime gains SpawnedProcessBackend, extracted from the codegen template: liveness-based connect raced against child death, kill-on-expiry backstop, log capture (async fs throughout — sync fs here would stall the event loop on exactly the degraded machines this path runs on), SIGTERM -> SIGKILL teardown, and opt-in lazy respawn (next call after a death gets a fresh process; stable ipc path; no eager crash-loop). Errors are typed (IpcTransportError, IpcProcessExitedError, IpcSpawnError) with a retry flag distinguishing process death from configuration errors; call failures that race the child's exit event are attributed to the death after a short grace. The generated package shrinks to binary resolution + backend config and passes through a respawn option; the call surface gains nothing. The AVM pool spawns services with respawn enabled and is the only interpreter of the retry flag — callers keep exact pre-IPC semantics (result, tx failure, or their own deadline). Environmental spawn failures retry indefinitely on a flat 1s cadence (no backoff: sequencers live on ~6s slots), bounded by the caller's abort signal, which threads through checkout, the spawn-retry loop, and full-pool waits. A simulation whose process dies is re-issued once; a second death is attributed to the input so a simulator-crashing tx is evicted instead of retried forever. Cancellation escalates SIGUSR1 -> (5s) -> SIGKILL, reclaiming the pool slot from a wedged simulation. Configuration errors surface fatally at the boot-time prewarm. wsdb keeps fail-fast semantics: a respawned wsdb would lose all forks and uncommitted state. New tests: SpawnedProcessBackend unit tests (script-based fake servers), echo ts_package reliability tests (slow-listen, die-before-listen, wedged-then-killed, missing-binary classification, respawn), AVM pool tests (indefinite abort-aware spawn retry, config fast-fail, re-issue-once, input attribution), and wsdb/avm server compile + startup-order checks.
The root rollup circuit has ~6.35M gates and is too large to construct more than once within the per-test timeout, so its vk independence check kept exceeding 600s in the merge queue. Per merge, PinnedVKRootRollup now constructs the circuit once and checks its vk hash and gate count together, reporting both current values whenever either drifts so that neither gets re-pinned while the other is left stale. This replaces GateCountRootRollup, whose gate count pin it subsumes. GenerateVKFromConstraints and Tampering for that circuit move to a nightly barretenberg job, so vk independence, tampering and the circuit checker still run daily. Per merge the circuit is checked for change only, not for satisfiability. Both root rollup checks use the non-zk UltraFlavor: the circuit closes its IPA accumulator in-circuit and is proved as a standard, non-rollup UltraHonk.
The recursive verifier suites and the Chonk max capacity test dominate the per-merge suite's wall time, at tens of seconds to minutes per case. They are now emitted by test_cmds_nightly, driven by a single nightly_only_tests pattern so that the per-merge skip and the nightly emission cannot drift apart. This pattern subsumes the debug-build skip list, which covered the same suites. Kept per merge as the cheap detectors of unintended changes to these circuits: the gate count cases and PinnedVKRootRollup, each of which builds its circuit once to compare against a pinned value. Debug builds also keep WithoutPredicate/1.GenerateVKFromConstraints, the only case exercising the debug-only native_verification_debug path, as the old skip list did. The nightly job keeps AVM enabled: the AVM recursive verifier tests are only built with it, so disabling it would leave them running nowhere, and with it the build hash matches the per-merge one and the build is a cache pull.
…25151) Unifies `labs-aztec-toolchain/bootstrap.sh` so one file serves the three contexts that need it: the monorepo today, the standalone labs repo after the split, and the foundation repo after the split (which deletes the labs components, consumes them as a submodule, and runs labs e2e against its own locally built bb/noir — the pre-split flow). Today the next and monorepo-split/labs lines carry divergent copies of this file that conflict on every sync merge; this converges them so the only intended difference is one committed default. ## The mode protocol A single variable selects the provisioning mode: - **Foundation mode** (`FND_ROOT` non-empty): symlink the binaries built inside the checkout at `FND_ROOT` (`barretenberg/cpp`, the `noir` submodule), and derive the toolchain identity from that tree's source hashes. This is `build_monorepo` + the #25111 hash, with the root parameterized. - **Pinned mode** (`FND_ROOT` empty): download released binaries at the pinned `BB_VERSION`/`NOIR_VERSION` (the monorepo-split/labs flow: bbup/noirup, cached acvm source build, `.pin` provenance record, `check_pin_drift`), and derive the identity from this directory's committed content. The committed default on this line is `FND_ROOT=$(git rev-parse --show-toplevel)` — foundation mode, today's behavior — so a bare invocation keeps linking the local build. `AZTEC_TOOLCHAIN_FND_ROOT` overrides either way: export it empty to force pinned mode, or point it at a foundation checkout root (how the post-split foundation repo will drive its labs submodule). The labs line will carry the same file with an empty default (follow-up PR against monorepo-split/labs). ## Hash semantics - **Foundation mode**: byte-identical inputs to the current (post-#25111) hash — providers' source hashes plus observed optional binaries. Verified the value is unchanged on this tree (`ce12057b4ea229fb` before and after), so **no cache invalidation on this line**. - **Pinned mode**: `cache_content_hash "^labs-aztec-toolchain/"` plus the declaratively expected optionals (bb-avm iff released for this platform, acvm iff cargo exists). This replaces the labs line's current byte-hashing of `bin/`, completing the identity/verification separation #25111 started: the hash is a pure function of the committed tree (computable on a fresh checkout — today `hash` fails until `build` has run, which downstream hash compositions trip over), a corrupted `bin/` can no longer mint a fresh valid-looking cache key (byte verification stays in the `.pin` record at provision time), a pin bump still moves the hash before any binary is refreshed, and a dirty toolchain dir propagates `disabled-cache` instead of laundering it into a stable-looking key. ## Other behavior notes - Foundation-mode `build` now starts from an empty `bin/` and writes the `.pin` record (previously labs-line-only), so switching between modes in one checkout fully re-provisions, and `noir_version` can report the exact submodule tag (e.g. `nightly-2026-07-31`) instead of nargo's base cargo version; it falls back to the binary when no record exists. - Pinned mode on this line is exercisable via the override but `check_pin_drift` will legitimately fail while next's Nargo.toml/docs/yarn-project pins differ from the labs line's — expected until the split content converges. - Call-site interface (`hash`, `build`, `noir_version`, `bin/*` paths) is unchanged. ## Validation - `bash -n` clean; fnd-mode `hash` byte-identical to the old script on the same tree (`ce12057b4ea229fb`). - Pinned-mode `hash` identical with `bin/` present and absent (`bd71460d78038ae5`), returns `disabled-cache` on a dirty toolchain dir locally, and fails (exit 1) under `CI=1` instead of hashing an empty string. - Foundation `build` run against this checkout's local bb/noir builds: symlinks all five binaries, writes the pin record, hash stable across re-provisioning. - Invalid `AZTEC_TOOLCHAIN_FND_ROOT` fails with a clear message. ## Follow-up - Apply the same file to `monorepo-split/labs` with an empty committed `FND_ROOT` default (its toolchain hash value changes once: contract/yarn-project caches on that line rebuild one time). - After the split, the foundation repo's labs-e2e driver exports `AZTEC_TOOLCHAIN_FND_ROOT=$(git rev-parse --show-toplevel)` before entering the submodule; the committed default divergence disappears.
…25073) Hardens the spawn-and-connect path used by the generated IPC packages (`@aztec/wsdb`, `@aztec/bb-avm-sim`), and makes bb-avm-sim failures non-fatal to the prover. Reviewing this stack against the bb socket incident fixed in #24802 showed the specific shared-budget bug from that incident does not reproduce here, but the same failure *family* was present — in the connect path, and in how environmental failures were classified downstream. ## Commit 1 — connect reliability **Servers listen before heavy init.** `aztec-wsdb` previously constructed its entire `WorldState` (LMDB open, genesis prefill) *before* `listen()`, and `bb-avm-sim` connected to its upstream wsdb/CDB servers before creating its own socket — so the client-side connect timeout was a bet on how fast a loaded machine can do real, unbounded work. Both servers now listen first: clients connect straight into the kernel accept backlog and first requests wait in the socket buffer until the reactor starts, so the connect backstop only ever covers exec + linking + reaching `listen()`. A server that dies during init surfaces its real exit cause instead of a spurious connect timeout. `bb-avm-sim` also installs its SIGUSR1 cancellation handler before the socket is reachable (the default disposition is process termination) and its lifecycle handlers — including parent-death monitoring — before the potentially-long upstream waits, whose budget goes from a hard-coded 5s to a 60s backstop. **Client-side liveness-based connect.** The connect wait is raced against child death (a dead server fails immediately with its exit code/signal); the backstop (60s) is purely a broken-process detector and now **kills** the wedged child instead of orphaning it (an orphaned wsdb holds LMDB locks on its data dir, poisoning any respawn). Every spawn-failure path reaps the child, unlinks the ipc path, and points at the child's captured log. Hard connect errors (EACCES etc.) fail immediately instead of being retried until the deadline; `EAGAIN` (momentarily full accept backlog under simultaneous pool spawns) is retryable. `destroy()` escalates SIGTERM → SIGKILL after 5s. One behavioural consequence: spawn resolving no longer implies the server finished initializing. All consumers issue their first call immediately after spawn and simply block until init completes; an init failure surfaces on that first call with the child's exit code and log path. ## Commits 2+3 — the backend owns process lifecycle; callers keep pre-IPC semantics Previously a bb-avm-sim spawn failure or crash was fatal to the prover: the error was rewrapped into `SimulationError`, the tx reported failed, the checkpoint prover failed, and the session manager blocks that epoch until a prune replaces the failed prover — i.e. forever (`hasFailedProver`). The pool also never evicted dead processes, so one crash permanently poisoned a slot; and the sequencer drops failed txs from P2P, so environmental failures evicted innocent txs. The design principle: **process lifecycle belongs to the layer that owns the process, and never appears above it.** Pre-IPC (in-process NAPI), callers could assume every failure was an actual tx failure, with the processor's deadline as the only environmental bound; that contract is preserved exactly. - **ipc-runtime** gains `SpawnedProcessBackend`, extracted from the codegen template (which embedded ~200 lines of process machinery per generated package, untestable except through them). It owns spawn, connect, death detection, teardown — and opt-in **lazy respawn**: the next `call()` after a death transparently gets a fresh process (one shared respawn attempt, stable ipc path, lazy-only so a crashing binary can't respawn-loop unprompted). Errors are typed — `IpcTransportError`, `IpcProcessExitedError`, `IpcSpawnError` — and carry a `retry` flag distinguishing process death from configuration errors. Call failures that race the child's `exit` event (the socket breaks first) are attributed to the death after a short grace, so deaths are never misreported as bare transport errors. - **Generated packages** shrink to binary resolution + backend config, plus a `respawn?: boolean` spawn option. The call surface gains nothing. - **The AVM pool** spawns its services with `respawn: true` (each simulation is self-contained; state is routed via WSDB/CDB by fork id) and is the *only* interpreter of the backend's error flags. It absorbs environmental trouble outright: spawn failures retry indefinitely on a flat 1s cadence — no backoff: sequencers live on ~6s slots, so sleeping longer after a failure costs whole blocks while the machine may have recovered, and slow attempts self-pace inside the backend connect backstop, bounded by the **caller's own deadline** — the abort signal threads through checkout, the spawn-retry loop, and full-pool waits, all of which stop promptly on abort. For the sequencer that bound is the slot deadline (`execWithSignal`); for the prover, the epoch deadline — meaning a checkpoint prover riding out a load spike just waits (loudly logged) and the epoch fails only at its true deadline, rather than being permanently poisoned after a fixed budget. Configuration errors (missing binary) still fail fast, at the boot-time prewarm. - **Poison-tx bound**: a simulation whose process dies is re-issued once on the respawned process; a second death for the same input is attributed to the input and surfaces as an ordinary failed tx — so a simulator-crashing tx is evicted from the mempool instead of burning a process per slot forever. (Pre-IPC, the same event killed the whole node.) - **Guaranteed cancellation cleanup**: on deadline abort, SIGUSR1 asks the C++ process to cancel at its next checkpoint; if it doesn't respond within 5s, it is SIGKILLed. Killing is safe because the service respawns lazily — a wedged simulation can no longer leak a pool slot. - **Upstream layers are untouched relative to pre-IPC**: no retry classification in the public tx simulator, public processor, or foundation. A simulation produces a result, fails on its own merits, or runs until deadlined out. **wsdb deliberately keeps fail-fast semantics**: a respawned wsdb loses all forks, checkpoints, and uncommitted state, so clients holding forkIds would silently read wrong state; its death remains a node-level event, surfaced with typed cause and log path. Deliberately out of scope: SHM readiness/call timeouts (shm has no readiness handshake; nothing deploys shm-wsdb today — it's exercised only by a parameterized world-state test). ## Tests - `ipc-runtime`: unit tests for `SpawnedProcessBackend` against script-based fake servers — death → typed exit error, lazy respawn gets a fresh pid, missing binary flagged as configuration, dies-before-listen fails promptly with the exit code, wedged process killed at the backstop, destroy during a pending respawn leaks nothing. - Echo `ts_package` reliability tests (run in ipc-codegen CI, uds + shm): slow-to-listen, dies-before-listen, wedged-then-killed (fails on the pre-#25073 code by construction), missing-binary classification, death-without-respawn, respawn-recreates-process. - Simulator: 9 pool tests — indefinite flat-cadence spawn retry, abort stops the retry loop and full-pool waits, config errors propagate immediately, re-issue-once on process death, second death attributed to the input (no environmental flag escapes), destroy semantics. Public processor suite unchanged from pre-IPC semantics; full `public_tx_simulator` suite green. - Verified end-to-end locally: `aztec-wsdb`/`bb-avm-sim` compile, a manual wsdb run shows `listening on` before `Creating WorldState` with a clean socket unlink on failed init, both generated packages rebuilt, full yarn-project TS build passes.
## Summary The nightly debug build failed in `AcirComponentsCheckTest.DetectsUnconstrainedWitnesses` because the test intentionally shrinks `builder.real_variable_index` to simulate a missing ACIR witness. In debug STL mode, `StaticAnalyzer` then indexes trace witness `10` through that shortened vector and aborts before `ComponentsChecker` can report the intended `UNCONSTRAINED` error. This PR makes `ComponentsChecker` detect ACIR witnesses missing from the builder-side `real_variable_index` immediately after building the ACIR graph and return `UNCONSTRAINED` errors before constructing `StaticAnalyzer`. It also avoids indexing out-of-range range-list entries during classification and tightens the regression test to assert the missing witness is named in the error. Failed run investigated: https://github.com/AztecProtocol/aztec-packages/actions/runs/29720683064 ## Testing - `cmake --preset debug -DAVM_TRANSPILER_LIB=` - `cmake --build --preset debug --target acir_components_check_tests` - `NATIVE_PRESET=debug barretenberg/cpp/scripts/run_test.sh acir_components_check_tests AcirComponentsCheckTest.DetectsUnconstrainedWitnesses` - `./barretenberg/cpp/build-debug/bin/acir_components_check_tests` - `git diff --check` --- *Created by [claudebox](https://claudebox.work/v2/sessions/0ee4283d1eda583c/jobs/1) · group: `slackbot` · [Slack thread](https://aztecprotocol.slack.com/archives/C04ALP0UQHH/p1784527672929669?thread_ts=1784527672.929669&cid=C04ALP0UQHH)*
## The coupling `cycle_group` took its MSM offset generators from `crypto::generator_data` — the pedersen generator store — via a `GeneratorContext` parameter threaded down `batch_mul` / `fixed_batch_mul`. That store is shared, mutable, and grows in place. Its own header documents it as not thread-safe: > This is not thread-safe. Each process that uses a `generator_data` object may extend `generator_data` if more generators are required ... either each process must use an independent `generator_data` object or the author must KNOW that `generator_data` will not be extended by any process. It also returns a `std::span` into a `std::vector` that a later call for the same domain reallocates, so a view handed to one caller can be freed underneath it. Both halves of that contract are violated in practice. `cycle_group::batch_mul` holds one of those spans across an entire MSM build, and `bb aztec_process` derives verification keys for several circuits on concurrent `std::thread`s against the shared singleton. The result is a rare, unreproducible `cycle_group: Point is not on curve` abort during `noir-contracts` bootstrap — [an example run](http://ci.aztec-labs.com/1786023406574272), [the failing contract](http://ci.aztec-labs.com/44c19c0d4f4041c4). The `GeneratorContext` parameter carrying this dependency was never once supplied. Every call site in `stdlib/` and `dsl/` defaulted it, and `ipa.hpp` passed a literal `{}` purely to reach the `table_bits` argument sitting behind it. ## What this does Adds `stdlib/primitives/group/cycle_group_offset_generators.hpp`, a store that owns exactly this one concern: ```cpp template <typename Curve> class cycle_group_offset_generators { public: static constexpr std::string_view DOMAIN_SEPARATOR = "cycle_group_offset_generator"; static const cycle_group_offset_generators& default_generators(); std::span<const AffineElement> get(size_t num_generators) const; }; ``` The design aims at removing the brittleness rather than guarding it: - **Prefix-only.** No offset or slice parameter. Generator `i` depends on nothing but the domain separator and `i`, so no request can change what another request sees. That is all cycle_group ever needed from the general store. - **Append-only blocks.** Growth allocates a new block and retains the superseded one, so a span already handed out stays valid however many times other threads call `get` afterwards. Growth doubles, so the retained set stays at a handful of entries. - **An instance, not a hidden global.** Circuit construction shares `default_generators()`, but the store is an ordinary object. This is what lets the tests below be non-vacuous — see the note on that. - **Locked.** A plain `std::mutex`; the critical section is a size check plus a rare derivation. `GeneratorContext` disappears from cycle_group's API, `fixed_batch_mul(points, scalars, {}, 8)` in `ipa.hpp` becomes `fixed_batch_mul(points, scalars, 8)`, and the now-unused `crypto/pedersen_commitment/pedersen.hpp` include comes out of both cycle_group files. **`crypto::generator_data` and the pedersen path are untouched.** ## Why memoize Measured here: `derive_generators` costs roughly 65us per generator — 550us for 8, 4.3ms for 65, 84ms for 1024. Deriving per `batch_mul` would be a real regression, so the store memoizes, and doubling blocks keep a circuit made of many differently sized MSMs to a handful of derivations rather than one per MSM. ## Verification keys are unaffected Offset generator `i` is the same point as before: same domain separator, same derivation, same prefix semantics — the previous path also requested offset 0. `MatchesDerivation` and `PrefixIsStableAcrossRequestSizes` pin exactly that, and the circuit tests below carry fixed expected values. ## Tests `stdlib/primitives/group/cycle_group_offset_generators.test.cpp`: | Test | What it pins | | --- | --- | | `MatchesDerivation` | Values equal `derive_generators` for the domain | | `PrefixIsStableAcrossRequestSizes` | Generator `i` does not depend on the request size | | `GetKeepsPreviouslyReturnedGeneratorsValid` | A span stays valid after a larger request grows the store | | `ConcurrentGetReturnsOnCurvePoints` | Concurrent `get` never yields an off-curve point | These were checked against a deliberately reintroduced bug — replacing the block on growth instead of retaining it. `PrefixIsStableAcrossRequestSizes` and `GetKeepsPreviouslyReturnedGeneratorsValid` go red deterministically, and green again once the retain is restored. Two details are worth knowing before editing these tests, because both produced a silently passing suite first time round: - The store has to be an **instance**. With function-local statics the cache is process-global, the growth path is live only for the first few assertions, and everything afterwards passes trivially — the first draft passed even with the bug present. - The deterministic test **sprays the freed size range** before re-reading. Without that, a stale span reads its own old contents back and passes while pointing at released memory. `ConcurrentGetReturnsOnCurvePoints` is the weakest of the four and is not the primary regression net: with the map race gone the residual failure is a dangling read, and freed-but-unreused memory usually still reads back correct. It is kept as the symptom-level reproduction and as an ASan/TSan target, with the two deterministic tests doing the real work. ## Run Full `cmake --build build` clean, including `bb`, `bb-avm` and `bb-avm-sim`. ``` stdlib_primitives_tests 1349 passed, 0 failed (128 cycle_group) dsl_tests 551 passed, 0 failed commitment_schemes_tests (IPA) 30 passed, 0 failed crypto_pedersen_commitment_tests 2 passed, 0 failed crypto_pedersen_hash_tests 6 passed, 0 failed ``` ## Note on scope This removes cycle_group's dependence on the shared store; it does not fix the store itself. `generator_data::get` still hands out spans it can later invalidate, which remains a hazard for the pedersen path and for any future caller, and is worth addressing separately. --- *Created by [claudebox](https://claudebox.work/v2/sessions/877d88cba221b585/jobs/8) · group: `slackbot` · requested by Sergei · [Slack thread](https://aztecfoundation.slack.com/archives/C04ALP0UQHH/p1786024347064329?thread_ts=1786024347.064329&cid=C04ALP0UQHH)*
BEGIN_COMMIT_OVERRIDE chore(ci): dual-mode labs-aztec-toolchain provisioning and hashing (#25151) END_COMMIT_OVERRIDE
Some quality of life improvements to the avm transpiler
…couple vm2 + AVM fuzzer from world_state (#24306) ## Summary Introduces a standalone **`world_state_reference`** component: a minimal, self-contained in-memory reference of the Aztec world-state trees (`bb::world_state::MemoryMerkleDB`) that faithfully reproduces `world_state::WorldState`'s tree rules (genesis prefill, zero-hashes, indexed/append-only semantics, checkpointing) — and uses it to remove the AVM simulator's dependency on the in-process `WorldState`. The heights, behaviour, and genesis data are all protocol decisions, so this reference implementation is foundation-owned code in barretenberg. It is deliberately vm2-free and single-threaded (it builds under the fuzzing preset), and it carries the world-state vocabulary (`MerkleTreeId`, `WorldStateRevision`, `getMerkleTreeName`) as its own — the lmdb `world_state` now shares that single definition instead of re-exporting it from elsewhere. After this PR, production AVM (`bb-avm-sim`) talks to world state only via the generated IPC client (`WsdbIpcMerkleDB`), and the AVM fuzzer runs entirely on the in-memory reference. `vm2` no longer references `world_state` at all — including its tests. This is the precursor to extracting `world_state`/`lmdblib`/persistent merkle out of barretenberg into a top-level `native-packages/` (stacked PR), where the reference becomes the conformance spec that external world-state implementations test against. ## What changes - **`world_state_reference/`** (new component, ns `bb::world_state`): the world-state vocabulary (`merkle_tree_id.{hpp,cpp}`, moved out of `world_state/types.hpp` — it's store-agnostic vocabulary, not lmdb storage) plus the reference trees (`memory_merkle_db.{hpp,cpp}`, `sparse_memory_tree.hpp`): a faithful, full-height, sparse in-memory reference of the four AVM trees. Replaces the WorldState-backed `PureRawMerkleDB` (deleted, along with the `simulate_*_with_existing_ws` entry points that existed only to construct it). - **vm2 adapter** (`vm2/simulation/lib/memory_merkle_db.{hpp,cpp}`): a thin `bb::avm2::simulation::MemoryMerkleDB` implementing the AVM's `LowLevelMerkleDBInterface` by wrapping the reference; the only translation is the tree-roots type (plain `TreeRoots` → the AVM's column-serialisable `TreeSnapshots`). Consumers keep the same class name. - **Fidelity gate** (`world_state/memory_merkle_db.test.cpp`, runs under `world_state_tests`): constructs an ephemeral `WorldState` and the reference with identical genesis, applies an identical sequence of appends/inserts/updates/pads/checkpoints, and asserts roots, sibling paths, low-leaf lookups, preimages, and leaf values match at every step. It lives with the lmdb world_state — the component asserting the reference reproduces it — so `vm2_tests` no longer links `world_state` even for tests. - **`SequentialInsertionResult`/`BatchInsertionResult`** move from `world_state/world_state.hpp` to `crypto/merkle_tree/response.hpp`, next to the witness types they're composed of, so vm2 can use them without world_state includes. - **AVM fuzzer decoupled**: the C++ side simulates on the in-memory reference; the TS differential simulator self-bootstraps its own world state (`NativeWorldStateService.tmp()`, identical genesis by construction) instead of reading a shared on-disk lmdb. `FuzzerWorldStateManager` drops its `WorldState` member and singleton lifecycle. ## Validation - Fidelity gate: 7/7 green (genesis, appends, pad, nullifier inserts, public-data insert+update, nested checkpoints, mixed sequence) against a real ephemeral `WorldState`; full `world_state_tests` green. - `vm2_tests` green; downstream linkers (`bb-avm-sim`, `wsdb_ipc_merkle_db`, `nodejs_module`, monolithic archive) build. - `FUZZING_AVM=ON`: all fuzzer targets compile/link; `prover.fuzzer` runs (simulate → check_circuit → prove → verify) with no divergence. - `grep world_state` over `vm2/` (tests included) is clean; `grep vm2` over `world_state_reference/` is clean (the reference is vm2-free). ## One divergence found + fixed `MemoryIndexedTree` reported the freshly-inserted leaf in `insertion_witness_data[0].leaf` where `ContentAddressedIndexedTree` reports the empty pre-write leaf; corrected to match.
fcarreiro
added
ci-no-fail-fast
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring alerts on:
|
Contributor
Author
@SocketSecurity ignore npm/@aztec/protocol-circuits-artifacts@6.0.0-nightly.20260812 |
Contributor
Author
|
@SocketSecurity ignore npm/@aztec/protocol-circuits-artifacts@6.0.0-nightly.20260812 |
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
7 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.
Merges
origin/next(5ab94a8) intomonorepo-split/labs. Must be merged with the merge commit method (ci-no-squash) to preserve the merge parentage — squashing would make every future next merge re-conflict.Conflict resolutions:
barretenberg/,avm-transpiler/,ipc-codegen/,ipc-runtime/) resolved as deleted, including files next added since the last merge (world_state_reference, cycle_group offset generators, ipc spawned-backend, the new barretenberg-nightly-slow-tests workflow).bootstrap.sh/ci.sh: kept labs' deletion of the barretenberg/chonk CI modes; next's modifications to those entries only concern the foundation side.labs-aztec-toolchain/bootstrap.sh: kept labs' committedFND_ROOTdefault (empty → pinned mode); the checkout-pointing default is the monorepo/foundation side's.What lands: the ci3/workflow updates, the dead-code removals from next's cleanup (old grafana Terraform, fund-sepolia workflow), spartan alert/env fixes, and the yarn-project changes from the trains (avm simulator pool + fuzzer updates, wsdb ipc client tweak, two new tests).
yarn-project/package.jsonresolutions are untouched.Follow-up commit: the merged yarn-project code uses the
respawnservice option introduced by the ipc reliability fix (#25073), which the pinned6.0.0-nightly.20260807foundation packages predate. All foundation pins (yarn-project resolutions, docs, Nargo.toml git deps,labs-aztec-toolchainBB_VERSION) move to6.0.0-nightly.20260812— the first published nightly containing that fix (.20260811was never published). The noir submodule didn't move between the two cuts, soNOIR_VERSIONstays at1.0.0-beta.26.check_pin_driftpasses and both lockfile diffs contain only the version moves.