perf(blobs): derive the blob fracs by halving the roots of unity - #25444
Merged
Conversation
Collaborator
Author
|
⏳ Claude Review — starting review session... |
Collaborator
Author
|
✅ Adversarial verdict: PASS ✅ Claude Review — completed VERDICT: PASS
|
Collaborator
Author
|
Claude Review: Starting automated code review... workflow run |
Collaborator
Author
|
Claude Review: Starting automated code review... workflow run |
AztecBot
changed the base branch from
cb/unconstrained-blob-eval-speedup
to
next
September 10, 2026 07:47
AztecBot
force-pushed
the
cb/blob-fracs-halving
branch
from
September 10, 2026 07:47
442b9ee to
c944189
Compare
Collaborator
Author
|
Claude Review: Starting automated code review... workflow run |
TomAFrench
marked this pull request as ready for review
September 10, 2026 08:14
TomAFrench
requested review from
IlyasRidhuan,
iAmMichaelConnor and
ledwards2225
as code owners
September 10, 2026 08:14
iAmMichaelConnor
approved these changes
Sep 10, 2026
iAmMichaelConnor
left a comment
Contributor
There was a problem hiding this comment.
Approved pending merge of #25450 into this branch
TomAFrench
pushed a commit
that referenced
this pull request
Sep 10, 2026
…s coverage (#25450) Follow-up to #25444, targeting its branch `cb/blob-fracs-halving`. One squashed commit; the diff touches only `noir-projects/fnd/noir-protocol-circuits/crates/blob/src/blob.nr`. The vendored bignum crate is untouched. ## Why The halving in `__compute_fracs` derives all 4096 inverses from one inversion of `z^d - 1`, so it requires `z^d != 1`. #25444's comment and body say a root-of-unity challenge "behaves as before". It does not: `batch_invert` skipped the single zero denominator and returned the other 4095 fractions intact, whereas the halving returns all zeros. The difference is unobservable (at a root `factor = 0` so `y_i = 0` either way, and `validate_fracs` has no solution there), but the claim was wrong and the precondition was implicit. ## What changes - `compute_factor` already has `z^d` in-circuit, so it asserts `z^d != 1` there with bignum's `assert_is_not_equal` (a few gates, sound against the `z^d = 1 + MOD` alias). `validate_fracs` is unsatisfiable at a root anyway, so this changes no accepted statement; it names the precondition in one place. It is a small ACIR change, so #25444's "The ACIR is untouched" no longer holds once this lands. - The `__compute_fracs` hint asserts the same condition on its own `z^d` with the message `blob challenge z is a d-th root of unity` rather than returning an all-zero array. Its doc comment states the precondition and drops the equivalence claim. ## Tests - The all-entry defining-relation check now runs over a corpus of eleven challenges (zero, small values, both sides of each 120-bit limb boundary, `-2` and `-3` at the top of the field, and a Poseidon2-sized value) instead of one. `-1` is deliberately absent: it is `ROOTS[1]`, a root of unity. - Every fraction is `-1` at `z = 0`. - The hint rejects `z = 1`, `-1` and `ROOTS[1000]` with its own message (`should_fail_with`), and `compute_factor` rejects a root in-circuit with `assert_is_not_equal fail`. - `validate_fracs` rejects a root even when handed an array that is correct at every other index (built with `batch_invert`, exactly the old hint), showing the constraint itself has no solution independent of the hint. - The squared-parent property `ROOTS[2u]^2 == ROOTS[u]`, which the final level of the halving relies on, is pinned next to the existing adjacent-negation check. - A corrupted fraction at the last index is rejected, not only at index 0. Rejections that fail inside bignum's `evaluate_quadratic_expression` stay plain `should_fail`: that assertion carries no message, and giving it one belongs in a bignum change rather than here. Ran the full `blob` crate with the repository-pinned compiler (Noir 1.0.0-rc.0, `fae3abca7e51`): 53 tests pass, up from 45 at the #25444 head. Analysis and derivation: https://gist.github.com/AztecBot/bfe643f0f99557cf0f6266eefb433c55 --- *Created by [claudebox](https://claudebox.work/v2/sessions/a85ec915f2c7003c/jobs/10) · group: `slackbot` · requested by Mike (@iAmMichaelConnor) · [Slack thread](https://aztecfoundation.slack.com/archives/D0B2N7W1WJD/p1789036805405059?thread_ts=1789036805.405059&cid=D0B2N7W1WJD)*
Collaborator
Author
|
Claude Review: Starting automated code review... workflow run |
Refs #10323. Independent of the rest of that work — it lands on `next` on its own. `__compute_fracs` builds `w^i / (z - w^i)` for all 4096 blob positions. It was a 4096-element Montgomery batch inversion (3 multiplications per element) plus a pass to apply the numerator: ~4d multiplications and one inversion. The roots of unity make almost all of that unnecessary. ## The identity Write `z_k = z^(2^k)` and `w_k = w^(2^k)`, and let `I_k[t] = 1 / (z_k - w_k^t)` over `t = 0 .. d/2^k - 1`. The top level is a single entry, `1 / (z^d - 1)` — the one inversion. Each level below follows by a difference of two squares, since `w_{k-1}^(t+m) = -w_{k-1}^t` for `m = d/2^k`: ``` (z_{k-1} - w_{k-1}^t)(z_{k-1} + w_{k-1}^t) = z_k - w_k^t ``` so one parent yields two children for two multiplications: ``` I_{k-1}[t] = (z_{k-1} + w_{k-1}^t) * I_k[t] I_{k-1}[t + m] = (z_{k-1} - w_{k-1}^t) * I_k[t] ``` Levels halve going up, so reaching `I_1` costs about `d` multiplications in total. The last level folds the `w^i` numerator in as it descends, sharing the `z*w^t` product across each pair — 3 multiplications per pair rather than the 4 a separate numerator pass needs. Total: ~2.5d multiplications and one inversion, against ~4d and one inversion. ## The indexing falls out of the EIP-4844 layout `ROOTS` is stored bit-reversed. That turns out to make the recursion trivial to index: a parent at array index `u` has its children at `2u` and `2u + 1`, and the root it needs is `ROOTS[2u]` at every level. At the final level `ROOTS[2u]` is `w^t` and `ROOTS[u]` is `w^2t`, so both terms are plain lookups. I got this wrong first time by assuming natural order — the halves are `i` and `i + d/2` there, not `2u` and `2u + 1` — and it produced fracs that failed the defining relation. `test_roots_are_stored_adjacent_to_their_negations` pins the property the indexing rests on, because the failure mode is silent corruption of half the array. ## Measurements `compute_fracs` alone, native ACVM, harness overhead subtracted, min of 5 on a loaded shared host: | | time | |---|---| | batch inversion | 0.915s | | roots-of-unity halving | 0.585s | **1.56x**, saving ~0.33s. This is a hint used by *both* execution paths, so it should take roughly the same ~0.33s off constrained witness generation as well — `__compute_fracs` is ~0.86s of the ~1.4s of Brillig hint work inside the ~11s constrained `rollup-checkpoint-root` witness generation. I have not measured the constrained side (it needs an 8-minute circuit compile per arm); worth confirming before quoting that number. It also matters for #10323: with the oracle removed (#25442) this takes a checkpoint-root simulation from ~1.40s to ~1.05s. ## Correctness The ACIR is untouched — this only changes how the hint is produced, and `validate_fracs` constrains the result exactly as before. The existing `test_validate_fracs_*` tests pass, which is the real contract: the new hint still satisfies the circuit's constraints. New `test_compute_fracs_satisfies_defining_relation_everywhere` checks `fracs[i] * (z - ROOTS[i]) == ROOTS[i]` for **all** 4096 entries rather than spot-checking a few indices, because an error at one level of the recursion corrupts a whole contiguous run of the output and a spot check would miss it. Degenerate input behaves as before: if `z` is itself a d-th root of unity the single inversion is zero and the array degenerates to zeros, exactly as `batch_invert` mapped a zero denominator to zero. `validate_fracs` rejects that either way, and reaching it requires a Poseidon2 preimage. Ran: the `blob` crate's `fracs` and `validate_fracs` tests (6 passing) plus the new invariant test. Not the full suite — CI is the complete check. The `blob` and `bignum` crates are byte-identical between the commit those runs were made against and current `next`, so the rebase did not move the code under the measurements or the test results. --- *Created by [claudebox](https://claudebox.work/v2/sessions/99aba4482349eaa5/jobs/9) · group: `slackbot` · requested by Tom (@TomAFrench) · [Slack thread](https://aztecfoundation.slack.com/archives/D0B586H14KG/p1788969921421879?thread_ts=1788969921.421879&cid=D0B586H14KG)*
AztecBot
force-pushed
the
cb/blob-fracs-halving
branch
from
September 10, 2026 14:26
8a155dc to
070b810
Compare
AztecBot
enabled auto-merge
September 10, 2026 14:26
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.
Refs #10323. Independent of the rest of that work — it lands on
nexton its own.__compute_fracsbuildsw^i / (z - w^i)for all 4096 blob positions. It was a4096-element Montgomery batch inversion (3 multiplications per element) plus a
pass to apply the numerator: ~4d multiplications and one inversion. The roots of
unity make almost all of that unnecessary.
The identity
Write
z_k = z^(2^k)andw_k = w^(2^k), and letI_k[t] = 1 / (z_k - w_k^t)over
t = 0 .. d/2^k - 1. The top level is a single entry,1 / (z^d - 1)— theone inversion. Each level below follows by a difference of two squares, since
w_{k-1}^(t+m) = -w_{k-1}^tform = d/2^k:so one parent yields two children for two multiplications:
Levels halve going up, so reaching
I_1costs aboutdmultiplications intotal. The last level folds the
w^inumerator in as it descends, sharing thez*w^tproduct across each pair — 3 multiplications per pair rather than the 4 aseparate numerator pass needs.
Total: ~2.5d multiplications and one inversion, against ~4d and one inversion.
The indexing falls out of the EIP-4844 layout
ROOTSis stored bit-reversed. That turns out to make the recursion trivial toindex: a parent at array index
uhas its children at2uand2u + 1, and theroot it needs is
ROOTS[2u]at every level. At the final levelROOTS[2u]isw^tandROOTS[u]isw^2t, so both terms are plain lookups.I got this wrong first time by assuming natural order — the halves are
iandi + d/2there, not2uand2u + 1— and it produced fracs that failed thedefining relation.
test_roots_are_stored_adjacent_to_their_negationspins theproperty the indexing rests on, because the failure mode is silent corruption of
half the array.
Measurements
compute_fracsalone, native ACVM, harness overhead subtracted, min of 5 on aloaded shared host:
1.56x, saving ~0.33s.
This is a hint used by both execution paths, so it should take roughly the same
~0.33s off constrained witness generation as well —
__compute_fracsis ~0.86sof the ~1.4s of Brillig hint work inside the ~11s constrained
rollup-checkpoint-rootwitness generation. I have not measured the constrainedside (it needs an 8-minute circuit compile per arm); worth confirming before
quoting that number.
It also matters for #10323: with the oracle removed (#25442) this takes a
checkpoint-root simulation from ~1.40s to ~1.05s.
Correctness
The ACIR is untouched — this only changes how the hint is produced, and
validate_fracsconstrains the result exactly as before. The existingtest_validate_fracs_*tests pass, which is the real contract: the new hintstill satisfies the circuit's constraints.
New
test_compute_fracs_satisfies_defining_relation_everywherechecksfracs[i] * (z - ROOTS[i]) == ROOTS[i]for all 4096 entries rather thanspot-checking a few indices, because an error at one level of the recursion
corrupts a whole contiguous run of the output and a spot check would miss it.
Degenerate input behaves as before: if
zis itself a d-th root of unity thesingle inversion is zero and the array degenerates to zeros, exactly as
batch_invertmapped a zero denominator to zero.validate_fracsrejects thateither way, and reaching it requires a Poseidon2 preimage.
Ran: the
blobcrate'sfracsandvalidate_fracstests (6 passing) plus thenew invariant test. Not the full suite — CI is the complete check. The
blobandbignumcrates are byte-identical between the commit those runs were madeagainst and current
next, so the rebase did not move the code under themeasurements or the test results.
Created by claudebox · group:
slackbot· requested by Tom (@TomAFrench) · Slack thread