test(bb): run every root rollup check nightly, including the pinned vk - #25165
Open
AztecBot wants to merge 1 commit into
Open
test(bb): run every root rollup check nightly, including the pinned vk#25165AztecBot wants to merge 1 commit into
AztecBot wants to merge 1 commit into
Conversation
Moving the heavy recursion suites to nightly kept PinnedVKRootRollup per merge as the cheap change-detector for the root rollup circuit. The first post-merge merge-queue run shows it is not cheap: it took 261s, making it the longest barretenberg test in the merge queue, against a default 600s per-test timeout. The cost is constructing the ~6.35M-gate circuit at all, not the checks layered on top, so there is no cheaper per-merge detector to keep behind. Per merge that circuit went from 29s (GateCountRootRollup) to 261s while 241s of vk-independence and tampering work moved to nightly — a wash for the merge queue, and a longer critical path. Drop the PinnedVKRootRollup carve-out so the whole of HonkRecursionConstraintTestWithoutPredicate/2 is emitted by test_cmds_nightly: the pinned vk hash and gate count now run there alongside GenerateVKFromConstraints and Tampering. The trade is that a change to the root rollup circuit is caught by the next nightly rather than in the merge queue. The GateCount carve-out and the debug-build carve-out for WithoutPredicate/1.GenerateVKFromConstraints are unchanged, so the smaller recursion circuits keep their per-merge detectors. The WithoutPredicate/2 resource-prefix branch is removed with it: nightly tests get a fixed CPUS=16:MEM=32g:TIMEOUT=60m, so the branch could no longer be reached.
iakovenkos
self-requested a review
August 10, 2026 17:51
iakovenkos
marked this pull request as ready for review
August 10, 2026 17:51
iakovenkos
enabled auto-merge
August 10, 2026 17:51
iakovenkos
approved these changes
Aug 10, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 10, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 11, 2026
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
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.
Summary
#24799 moved the heavy recursion suites to nightly but kept
PinnedVKRootRollupper merge as the cheap change-detector for the root rollup circuit. The first merge-queue run after it landed shows it is not cheap.From run 31409209377 (
ci-full-no-test-cache, headc38ea15f, test-engine log6b948df91b0fd544):That is the longest barretenberg test in the merge queue, against a default 600s per-test timeout — for a circuit whose full checks had already been deferred to nightly.
Comparing against the last pre-#24799 merge-queue run (31395115064, head
02217d37, test-engine logb96eb5be8d914778), the per-merge cost of the root rollup circuit went:…/2.GateCountRootRollup→…/2.PinnedVKRootRollup…/2.GenerateVKFromConstraints…/2.TamperingA wash — and with a longer critical path, since the work is now concentrated in one serial test rather than spread across three. The cost is constructing the ~6.35M-gate circuit at all, not the checks layered on top of it, so there is no cheaper per-merge detector to keep behind.
What this does
Drops the
*.PinnedVKRootRollupcarve-out inruns_nightly_only, so the whole ofHonkRecursionConstraintTestWithoutPredicate/2— pinned vk hash, pinned gate count,GenerateVKFromConstraints,Tampering— is emitted bytest_cmds_nightlyatCPUS=16:MEM=32g:TIMEOUT=60m.Unchanged, so the smaller recursion circuits keep their per-merge detectors:
*.GateCount*carve-out —HonkRecursionConstraintTestWithPredicate/*.GateCountSingleHonkRecursionandChonkRecursionConstraintTest.GateCountChonkRecursionstill run per mergeWithoutPredicate/1.GenerateVKFromConstraints, the only case exercising the debug-onlynative_verification_debugpathAlso removes the
HonkRecursionConstraintTestWithoutPredicate/2.*resource-prefix branch. Nightly tests get a fixed prefix before that code is reached, and no/2case is emitted per merge any more, so the branch became unreachable; leaving it would have been a comment asserting a per-merge policy that no longer exists. The three comments that still described the pinned vk as the per-merge cover are updated.The trade-off
The pinned gate count and vk hash become nightly-only, so a PR that changes the root rollup circuit merges green and the nightly goes red the next morning — the failure lands on
nextrather than on the PR that caused it. That is the deliberate cost of not paying 261s per merge for a circuit whose real checks already run nightly.Worth knowing: the nightly job itself has never run.
barretenberg-nightly-slow-tests.ymllanded with #24799 at 14:33 UTC today and its cron is0 6 * * *, sototal_countfor that workflow is still 0. It hasworkflow_dispatch— a manual trigger before this merges would validate the nightly emission end to end.Testing
nightly_only_tests+runs_nightly_onlyinto a harness and classified the real test names taken from the two merge-queue logs above, under bothclang20and a debug preset:WithoutPredicate/2cases → nightly, in both presetsGateCountSingleHonkRecursion,GateCountChonkRecursion→ per mergeWithoutPredicate/1.GenerateVKFromConstraints→ per merge in debug onlyAvmRecursiveTests.TranscriptOperations,IPARecursiveTests.*,ChonkTests.Basic→ per merge, unaffectedbash -n barretenberg/cpp/bootstrap.shCreated by claudebox · group:
slackbot· requested by Sergei · Slack thread