fix: Barrett reduction bug for primes with large quotient error - #36
Open
quangvdao wants to merge 2 commits into
Open
fix: Barrett reduction bug for primes with large quotient error#36quangvdao wants to merge 2 commits into
quangvdao wants to merge 2 commits into
Conversation
…imes
For primes where 2^{n_p-1}/p + (2^{n_p+63} mod p)/2^{n_p} >= 1 (e.g.,
BLS12-381 Fq, BLS12-377 Fr/Fq, Pallas/Vesta, secp256r1, Curve25519),
the Barrett quotient estimate m can satisfy q - m = 2, producing a
remainder r >= 4p that the existing 3-way conditional subtraction
cannot reduce to canonical form.
The fix uses ceil(c / 2^{n_p}) instead of floor, which makes the two
rounding errors (floor of mu, ceil of c/2^{n_p}) partially cancel.
This keeps |q - m| <= 1 universally, so r is always in (-2p, 4p).
A conditional add-back of 2p handles the possible underflow when m
overestimates q by 1.
A compile-time const BARRETT_NEEDS_CEIL, computed from MODULUS and
BARRETT_MU via exact bigint remainder arithmetic, gates the fix so
that primes where the original algorithm is safe (e.g., BN254 Fr/Fq,
secp256k1) pay zero overhead.
Also adds a const_remainder! macro for computing bigint modular
remainders at compile time.
Includes regression test with a concrete counter-example for BLS12-381
Fq and a 1000-iteration random test over BLS12-381 Fq.
Made-with: Cursor
Reduce the number of Barrett kernel rounds in wide reduction paths and avoid rebuilding temporary N+1 buffers during folding. This removes the scaling cliff in BN254's wide Barrett and mixed Montgomery+Barrett reducers while preserving existing semantics. Made-with: Cursor
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
1 participant
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
mul_u64/barrett_reduce_nplus1_to_ncan underestimate the quotientqby 2 for certain primes (those where2^{n_p-1}/p + (2^{n_p+63} mod p)/2^{n_p} >= 1). This produces a remainderr >= 4p, which the existing 3-way conditional subtraction (handling up to3p) cannot reduce to canonical Montgomery form. Affected primes include BLS12-381 Fq, BLS12-377 Fr/Fq, Pallas/Vesta, secp256r1, and Curve25519. BN254 Fr/Fq and secp256k1 are not affected.ceil(c / 2^{n_p})instead offloorwhen computing the approximate high part ofc. This makes the two rounding errors partially cancel, guaranteeing|q - m| <= 1. When the estimate overestimates (borrow onc - m*2p), we add2pback. The remainder is then always in[0, 4p), which the existing conditional subtraction handles correctly.BARRETT_NEEDS_CEIL, computed fromMODULUSandBARRETT_MUvia exact bigint remainder arithmetic (const_remainder!macro), gates the fix. Primes where the original algorithm is safe (e.g., BN254) pay zero overhead.Changes
ff/src/biginteger/mod.rs: Addedconst_remainder!macro for compile-time bigint modular remainder.ff/src/fields/models/fp/montgomery_backend.rs:BARRETT_NEEDS_CEILcompile-time constant.barrett_reduce_nplus1_to_nto apply the ceil fix when needed.mul_u64test over BLS12-381 Fq.Test plan
mul_u64test over BLS12-381 Fq passesPosted by Cursor assistant (model: claude-4.6-opus-high-thinking) on behalf of the user (Quang Dao) with approval.