Skip to content

fix(ci): commit-independent toolchain hash; generate-once VK cache - #25111

Merged
fcarreiro merged 1 commit into
nextfrom
cl/fix-vk-cache-generation
Aug 5, 2026
Merged

fix(ci): commit-independent toolchain hash; generate-once VK cache#25111
fcarreiro merged 1 commit into
nextfrom
cl/fix-vk-cache-generation

Conversation

@charlielye

Copy link
Copy Markdown
Contributor

The incident

PR #24306 failed aztec-up/scripts/run_test.sh bridge_and_claim deterministically — the identical BBApiException: Failed to verify the generated proof! ~2.8s into the only ClientIVC prove of the test, across multiple "retries", while the same test was 60/60 green on next. The retries could never help: the failure was frozen inside cached build artifacts (aztec-up-test-image-… and yarn-project-….tar.gz), which are content-addressed by tree hash and therefore reused by every run of the same tree.

Tracing how a wrong artifact could sit under a correct content key led to two independent defects that compose:

Defect 1: the toolchain hash changes on every commit (regression, #25078 / #25057 / #25047)

labs-aztec-toolchain/bootstrap.sh hash computed the toolchain identity as hash_str $(git hash-object <built binaries>). But inject_version (barretenberg/cpp/bootstrap.sh) stamps git rev-parse --short HEAD into bb/bb-avm bytes on any non-release build — so the binary bytes encode the commit, and byte-identical source trees produce a different toolchain hash on every commit.

Observed directly: two CI runs of byte-identical trees (commits 5041b6bb / fafdd723) had toolchain hashes f2e3f37015b9cdfa vs d714a9fc28a62cff, zero overlap between their 84 contract-*.tar.gz cache names, and bb logged ~/.bb/5041b6b/vk_cache vs ~/.bb/fafdd72/vk_cache — the injected commit even keys the runtime VK cache directory.

Since every noir-contracts cache key mixes in AZTEC_TOOLCHAIN_HASH, the practical effect is: every commit of every PR recompiles all ~84 contracts and mass-regenerates their VKs in parallel into a brand-new, empty, shared VK cache. Before this scheme (pre-Aug-3), contract keys were built from source rebuild patterns over bb/noir/transpiler — identical trees reused artifacts.

It is also conceptually circular: the binaries themselves rebuild if and only if their source hash changes (their artifact names are the source hash — both runs above shared barretenberg-clang20-11cccdad4e78d0a2.zst), so deriving downstream rebuild keys from the binary bytes answers "did the toolchain change?" with information that is only available after deciding exactly that.

Fix: the toolchain hash now composes the providers' source content hashes — barretenberg/cpp/bootstrap.sh hash + noir/bootstrap.sh hash — plus the presence of the optional binaries (bb-avm/acvm, whose availability is part of the toolchain's identity but whose content is already covered by the provider hashes). Rebuild decisions at every level of the graph now key on the same inputs. A future labs-repo provisioning mode (build_labs, currently stubbed) should use the released toolchain's version string as its fixed identity.

Defect 2: the shared VK cache is not generate-once (latent since ~June)

get_or_generate_cached_app_vk (barretenberg/cpp/src/barretenberg/api/aztec_process.cpp) did exists() → read_file, and on miss wrote the VK directly to the final path — no locking, no temp-file+rename. The cache directory is shared by all concurrent bb processes (the noir-projects builds run one per contract under GNU parallel), and distinct contracts can carry byte-identical functions: the simulated account contracts stub out signature verification, so simulated_schnorr_account and simulated_ecdsa_account produce the same bytecode hash → the same cache path, wanted simultaneously by two unrelated processes.

The failing run's logs show the race firing three times (same VK hash "Generating…" concurrently from two contracts), plus other processes reading entries moments after a different process generated them. Two simultaneous writers on one path — or a reader consuming a half-written file — silently embeds a truncated VK into a contract artifact. Account-contract VKs are exactly what ClientIVC private-kernel recursion verifies, matching the observed deterministic proof-verification failure; the corrupted artifact then flowed into the yarn-project tarball and the aztec-up test image, both content-keyed, freezing the failure for every subsequent run of that tree.

Fix: each cache entry now takes an exclusive flock on a <entry>.lock sidecar for the duration of check-generate-write — one process generates while the rest block, then read the completed entry (generate-once, which is also the economical behavior: previously colliding processes each did the full ChonkComputeVk). Writes go via temp file + rename, so even a reader without the advisory lock (the Windows fallback, where flock is unavailable) can never observe a partial entry; a lost rename race there is benign and resolved in favor of the completed equivalent entry. The file is already excluded from WASM builds.

Why the fixes fix it

  • Defect 1's fix removes the trigger: identical trees reuse contract artifacts again, so the parallel mass-VK-regen (and its race window) happens only when bb/noir/transpiler genuinely change — and recovers the associated CI compute wasted on every commit since Aug 3.
  • Defect 2's fix removes the vulnerability: when a legitimate mass regeneration does happen (any PR that really changes bb), concurrent generation of shared-bytecode VKs is serialized and atomic, so a corrupt artifact can no longer be produced, and therefore can no longer be frozen into content-addressed caches.

Validation

  • bb builds clean with the C++ change; bash -n on the bootstrap change.
  • Both provider hash commands verified to produce stable content hashes; the new toolchain hash is commit-independent by construction (inputs are source content hashes + a presence string).
  • The poisoned artifacts from the incident (aztec-up-test-image-3747b48e27303118.zst) were separately rebuilt from a from-source local build and force-uploaded; this PR prevents recurrence.

The labs-aztec-toolchain hash hashed the bytes of the built binaries, but
inject_version stamps the current commit into bb/bb-avm on non-release builds,
so byte-identical trees produced a different toolchain hash on every commit.
Every noir-contracts cache key mixes that hash in, so every commit recompiled
all ~84 contracts and mass-regenerated their VKs in parallel into a fresh
shared vk_cache. The toolchain hash now composes the providers' source content
hashes (the same inputs that decide whether the binaries rebuild) plus the
presence of the optional binaries.

That mass regeneration exposed a latent race: get_or_generate_cached_app_vk
checked existence then wrote the final path directly, with no locking — and
distinct contracts can carry byte-identical functions (the simulated account
contracts), so concurrent bb processes double-generated the same entry and
could read a partially written VK. One corrupted account-contract VK baked
into the yarn-project artifact and the aztec-up test image caches, making
bridge_and_claim fail deterministically on every retry of PR #24306. VK cache
entries now take a per-entry flock (generate-once: one process generates,
the rest block then read) and are written via temp file + rename.
@nchamo
nchamo self-requested a review August 5, 2026 15:01

@nchamo nchamo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, great catch!

@charlielye
charlielye added this pull request to the merge queue Aug 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 5, 2026
done
hash_str $(git hash-object "${files[@]}")
hash_str \
$("$ROOT"/barretenberg/cpp/bootstrap.sh hash) \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I will come back to this PR when I get back from OOO next week, but this part is wrong. It makes the labs repo depend on the foundation repo.

@fcarreiro
fcarreiro added this pull request to the merge queue Aug 5, 2026
Merged via the queue into next with commit df0891a Aug 5, 2026
22 checks passed
@fcarreiro
fcarreiro deleted the cl/fix-vk-cache-generation branch August 5, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants