chore: Native curve audit by federicobarbacovi · Pull Request #20936 · AztecProtocol/aztec-packages

federicobarbacovi · 2026-02-27T07:25:05Z

🧾 Audit Context

First part of the audit of ecc/curves. This PR is concerned with curve fields: we restructure testing, standardise struct format, and add some more constant checks.

🛠️ Changes Made

Refactor field params structs so that they all have the same format
Refactor testing: move some testing to generic field testing, standardise testing for all field params struct, add tests for constants

✅ Checklist

Audited all methods of the relevant module/class
Audited the interface of the module/class with other (relevant) components
Documented existing functionality and any changes made (as per Doxygen requirements)
Resolved and/or closed all issues/TODOs pertaining to the audited files
Confirmed and documented any security or other issues found (if applicable)
Verified that tests cover all critical paths (and added tests if necessary)
Updated audit tracking for the files audited (check the start of each file you audited)

📌 Notes for Reviewers

federicobarbacovi · 2026-02-27T07:25:37Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/constants.test.cpp

 TEST(FqConstants, RInv)
 {
    // r_inv = -q^{-1} mod 2^64
-    uint512_t r{ 0, 1 };


This was using r = 1 << 256

federicobarbacovi · 2026-02-27T07:26:09Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq2.hpp

+    static constexpr fq frobenius_on_twisted_curve_y_1{
        0xa1d77ce45ffe77c7UL, 0x07affd117826d1dbUL, 0x6d16bd27bb7edc6bUL, 0x2c87200285defeccUL
    };
-    static constexpr fq twist_cube_root_0{


federicobarbacovi · 2026-02-27T07:26:33Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq2.hpp

    static constexpr fq twist_coeff_b_1{
        0x38e7ecccd1dcff67UL, 0x65f0b37d93ce0d3eUL, 0xd749d0dd22ac00aaUL, 0x0141b9ce4a688d4dUL
    };
-    static constexpr fq twist_mul_by_q_x_0{


Changed names to make them more descriptive

federicobarbacovi · 2026-02-27T20:19:17Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/bn254.hpp

@@ -25,18 +25,16 @@ class BN254 {
    using TargetField = bb::fq12;

    static constexpr const char* name = "BN254";


Old issue, closed

federicobarbacovi · 2026-02-27T20:20:41Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq.hpp

+    static constexpr uint64_t primitive_root_3 = 0UL;
+
+    // Coset generators in Montgomery form for R=2^256 mod Modulus. Used in FFT-based proving systems
+    static constexpr uint64_t coset_generator_0 = 0x7a17caa950ad28d7ULL;


We need only the first coset generator, I removed the other ones from all the fields

federicobarbacovi · 2026-02-27T20:21:18Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq.test.cpp

-    constexpr fq b = uint256_t{ 0x744fc10aec23e56aUL, 0x5dea4788a3b936a6UL, 0xa0a89f4a8af01df1UL, 0x72ae28836807df3UL };
-    constexpr fq expected =
-        uint256_t{ 0x6c0a789c0028fd09UL, 0xca9520d84c684efaUL, 0xcbf3f7b023a852b4UL, 0x1b2e4dac41400621UL };
+    constexpr fq a{ 0x83aa80986c4f06f8, 0xbd01cce5e3b3afc3, 0x1cba208cb70aa13b, 0x2a582eb35a932e0d };


Prior to this change the elements were in non-Montgomery form. I changed them so that we use Montgomery form in all tests

much better, thank you!

federicobarbacovi · 2026-02-27T20:21:35Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq.test.cpp

    constexpr fq inv = a.invert();
    // Verify a * a^-1 = 1
    static_assert(a * inv == fq::one());
 }


Moved to generic field testing

federicobarbacovi · 2026-02-27T20:26:06Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fr.test.cpp

    k1.self_to_montgomery_form();
    k2.self_to_montgomery_form();

+    EXPECT_LT(uint256_t(k1).get_msb(), 128);


They were present in other tests so I added them here as well

federicobarbacovi · 2026-02-27T20:27:27Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq6.test.cpp

@@ -3,75 +3,6 @@

 using namespace bb;


I don't see the value of these tests

federicobarbacovi · 2026-02-27T20:29:10Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq12.test.cpp

@@ -403,22 +307,6 @@ TEST(fq12, SqrCheckAgainstConstants)
    EXPECT_EQ(result, expected);
 }


Inverse moved to generic field testing; unitary inverse is below

federicobarbacovi · 2026-02-27T20:56:20Z

barretenberg/cpp/src/barretenberg/ecc/fields/general_field.test.cpp


    EXPECT_EQ(result, expected);
 }
+


Add some generic testing

federicobarbacovi · 2026-02-27T20:56:51Z

barretenberg/cpp/src/barretenberg/ecc/fields/field_docs.md

 Prime field documentation    {#field_docs}
 ===
-Barretenberg has its own implementation of finite field arithmetic. The implementation targets 254-bit (bn254, grumpkin) and 256-bit (secp256k1, secp256r1) fields. Internally the field is represented as a little-endian C-array of 4 uint64_t limbs. For 254-bit fields, the internal representation must be in the range \f$[0, 2p)\f$ (which we refer to as the _coarse representation_), while for 256-bit fields the internal representation is an arbitrary `uint256_t`.
+Barretenberg has its own implementation of finite field arithmetic. The implementation targets 254-bit (bn254, grumpkin) and 256-bit (secp256k1, secp256r1) fields. Internally the field is represented as a little-endian C-array of 4 uint64_t limbs. For 254-bit fields, the internal representation must be in the range $[0, 2p)$ (which we refer to as the _coarse representation_), while for 256-bit fields the internal representation is an arbitrary `uint256_t`.


Changed formatting to markdown math instead of Doxygen math

federicobarbacovi · 2026-02-27T20:57:16Z

barretenberg/cpp/src/barretenberg/ecc/fields/field_declarations.hpp

+            Params::coset_generator_3,
        };
 #else
        const field result{


Removed some unused code

federicobarbacovi · 2026-02-27T20:57:38Z

barretenberg/cpp/src/barretenberg/ecc/fields/field12.hpp

    }

-    constexpr field12 from_montgomery_form()
+    constexpr field12 from_montgomery_form() const


Needed for uniform testing

notnotraju · 2026-03-04T14:10:04Z

barretenberg/cpp/src/barretenberg/ecc/curves/field_params_constants.test.cpp

+    // References: https://eips.ethereum.org/EIPS/eip-196, https://hackmd.io/@jpw/bn254
+    static constexpr const char* expected_modulus_decimal =
+        "21888242871839275222246405745257275088696311157297823662689037894645226208583";
+    static constexpr bool has_cube_root = true;


I like these flags!

notnotraju · 2026-03-04T14:10:33Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/constants.test.cpp

@@ -1,289 +0,0 @@
-/**


Thanks for refactoring out of the bn254 folder, cleaning up, and incorporating all fields

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/bn254.hpp

notnotraju · 2026-03-04T14:17:19Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq.hpp

+    // divide by 2^256 by taking the highest four limbs. See field_docs.hpp for more details.
+    static constexpr uint64_t r_inv = 0x87d20782e4866389UL;
+
+    // 2^(-64) mod Modulus


This is a nit, but I think it's better to cite the field docs here. The main reason we cite the Ingoyama hackmd is for what we call "Yuval reduction", which we use for WASM.

(so, we cite it when we do 2^(-29) below)

notnotraju · 2026-03-05T09:43:27Z

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq12.test.cpp

-}
-
-TEST(fq12, MulSqrConsistency)
+TEST(fq12, FrobeniusMapSixIsConjugation)


good tests!

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq2.hpp

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq.hpp

barretenberg/cpp/src/barretenberg/ecc/curves/grumpkin/grumpkin.hpp

notnotraju

Looks good, thanks! Left a few trivial comments.

…; mark from_montgomery_form as const in fq12

…ore general testing

…ield params

The `FromMontgomeryForm` test in `general_field.test.cpp` (added by #20936) was comparing raw limbs after `from_montgomery_form()`, expecting exactly `{1, 0, 0, 0}`. However, the WASM Montgomery multiplication returns coarse results in `[0, 2p)`, so `from_montgomery_form(one())` can return `p+1` instead of `1` — both represent the same field element, but the raw limbs differ. This caused `ecc_tests` to fail in WASM builds (`FieldTest/0.FromMontgomeryForm` for BN254 Fq and `FieldTest/1.FromMontgomeryForm` for BN254 Fr), which dequeued merge-train/barretenberg PR #21084 from the merge queue. Fix: use `from_montgomery_form_reduced()` for base fields to ensure full reduction to `[0, p)` before raw limb comparison. Extension fields already use `from_montgomery_form_reduced()` internally on their components, so they are unaffected. ClaudeBox log: http://ci.aztec-labs.com/5f9c459fdf9e2366-1

BEGIN_COMMIT_OVERRIDE fix: add -g0 to zig presets to eliminate 11GB debug info bloat (#21071) fix: resolve flaky p2p_client test race condition on ARM64 (#21088) chore: remove domain iteration macros and address backing memory race (#20988) fix: [ECCVM] added domain separation for the multiset equality check. (#20352) feat: hybrid CRS hash verification — 8MB chunks, parallel, span-based (#21113) chore: unify splitting scalars interface (#20805) chore: add a unique id to each origin tag (#20924) chore: Native curve audit (#20936) chore: Update bootstrap in test vk haven't changed script (#21153) fix: use reduced form in WASM FromMontgomeryForm test (#21164) chore: erase ephemeral secrets from memory in schnorr and aes (#21106) chore: suppress clangd target triple version diagnostic (#21180) feat: Optimise new claim calculation (#21179) docs: add Quick Start build instructions to barretenberg README (#20951) feat: batched chonk verification (#21083) fix: link libc++ instead of libstdc++ for Rust FFI on Linux (#21203) fix: [ECCVM] in the transcript table, no-ops force the next accumulator to be 0. (#20849) fix: resolve merge-train conflict with next (zig wrapper scripts + -g0) (#21201) fix: [ECCVM] rare edge case completeness issue when `z1 == 0` but `z2 != 0` (#20858) fix: use actual data extent for CommitmentKey in HypernovaDeciderProver (#21206) END_COMMIT_OVERRIDE

federicobarbacovi commented Feb 27, 2026

View reviewed changes

federicobarbacovi force-pushed the fb/native_curve_audit branch from f7702a3 to a003ee8 Compare February 27, 2026 20:16

federicobarbacovi commented Feb 27, 2026

View reviewed changes

federicobarbacovi marked this pull request as ready for review February 27, 2026 21:01

federicobarbacovi self-assigned this Feb 27, 2026

federicobarbacovi requested a review from notnotraju February 27, 2026 21:40

notnotraju reviewed Mar 4, 2026

View reviewed changes

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/bn254.hpp Outdated Show resolved Hide resolved

notnotraju reviewed Mar 4, 2026

View reviewed changes

notnotraju self-requested a review March 5, 2026 09:41

notnotraju reviewed Mar 5, 2026

View reviewed changes

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq2.hpp Show resolved Hide resolved

notnotraju reviewed Mar 5, 2026

View reviewed changes

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq.hpp Show resolved Hide resolved

notnotraju reviewed Mar 5, 2026

View reviewed changes

barretenberg/cpp/src/barretenberg/ecc/curves/grumpkin/grumpkin.hpp Outdated Show resolved Hide resolved

notnotraju self-requested a review March 5, 2026 10:01

notnotraju approved these changes Mar 5, 2026

View reviewed changes

Improve constant testing + formatting changes

21b884d

federicobarbacovi added 18 commits March 5, 2026 10:43

Improve testing + remove old code about coset generators

dccc281

Re order constants

65c2bf3

Fr

5f46828

Fq (bn254)

7a1bcf2

Fr tests

72c3dee

Move some testing to general field testing; standardise fq/fr testing…

76e348b

…; mark from_montgomery_form as const in fq12

Fix python script + Fq2 testing + Fq2 (remove cube root of unity) + M…

3e3d4d6

…ore general testing

Improve naming

8d2c02a

Fq6

2cd05c8

Fq12

0312cdd

BN254

e7e3ef3

Grumpkin

786a65b

Fixes

71f6884

Uniform tests for curve field parameters + standardize structure of f…

be7a5d1

…ield params

Final stuff

fc0cb57

Fixes + add test for generator and coeffs

26773b8

Address comments

9857c04

Remove commented out code

3332452

federicobarbacovi force-pushed the fb/native_curve_audit branch from c46da72 to 3332452 Compare March 5, 2026 10:45

federicobarbacovi added 2 commits March 5, 2026 11:01

More address comments

f18889f

Update header

3ded1f0

federicobarbacovi enabled auto-merge (squash) March 5, 2026 11:06

federicobarbacovi merged commit cc23f3e into merge-train/barretenberg Mar 5, 2026
10 checks passed

federicobarbacovi deleted the fb/native_curve_audit branch March 5, 2026 11:17

This was referenced Mar 5, 2026

feat: merge-train/barretenberg #21084

Merged

fix: use reduced form in WASM FromMontgomeryForm test #21164

Merged

		@@ -25,18 +25,16 @@ class BN254 {
		using TargetField = bb::fq12;

		static constexpr const char* name = "BN254";

		@@ -403,22 +307,6 @@ TEST(fq12, SqrCheckAgainstConstants)
		EXPECT_EQ(result, expected);
		}

Conversation

federicobarbacovi commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧾 Audit Context

🛠️ Changes Made

✅ Checklist

📌 Notes for Reviewers

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

notnotraju left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

federicobarbacovi commented Feb 27, 2026 •

edited

Loading