Skip to content

chore: bigfield todo fixes#15202

Merged
suyash67 merged 34 commits into
merge-train/barretenbergfrom
sb/bigfield-fixes
Jun 27, 2025
Merged

chore: bigfield todo fixes#15202
suyash67 merged 34 commits into
merge-train/barretenbergfrom
sb/bigfield-fixes

Conversation

@suyash67

@suyash67 suyash67 commented Jun 21, 2025

Copy link
Copy Markdown
Contributor

Bigfield internal audit related cleanup/fixes.

  • Simplified data structs: non_native_multiplication_witnesses and non_native_partial_multiplication_witnesses
  • Reduced code duplication with new functions like get_binary_basis_limb_witness_indices
  • Resolved/removed some of the TODOs

resolves #14662 #14660
resolves AztecProtocol/barretenberg#999
resolves #14658

@suyash67 suyash67 force-pushed the sb/bigfield-fixes branch from 1b3516a to 83ce657 Compare June 22, 2025 11:31
builder.create_add_gate({ a_idx, builder.zero_idx, builder.zero_idx, 1, 0, 0, -fr(e) });
builder.decompose_into_default_range(a_idx, 134);

// odd num bits - divisible by 3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added test case for decompose_into_default_range with odd num of bits.


non_native_field_witnesses<fr> inputs{
a_indices, b_indices, q_indices, r_indices, modulus_limbs, fr(uint256_t(modulus)),
non_native_multiplication_witnesses<fr> inputs{

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renaming, and removed modulus as its not used.

@suyash67 suyash67 requested a review from Copilot June 23, 2025 12:25

Copilot AI 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.

Pull Request Overview

This PR cleans up and audits the bigfield and circuit builder code by resolving several TODOs, reducing duplicate logic, and renaming data structures for clarity. Key changes include:

  • Renaming non_native_field_witnesses to non_native_multiplication_witnesses (with a corresponding partial witness struct) to simplify non‐native field multiplication.
  • Refactoring functions (e.g. pow) with improved fast modular exponentiation and updated gate comments.
  • Removing outdated TODOs and replacing them with detailed NOTE comments that better explain the rationale and potential optimizations.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp Updated non‐native multiplication witness usage and removed modulus parameter.
barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp Renamed witness struct and added a new partial multiplication witness struct, updating function APIs accordingly.
barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp Revised dummy gate comments and simplified auxiliary wire population.
barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp Refactored the subtraction, division, and pow functions with optimized arithmetic and clearer NOTE comments.
barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp Removed stale commented code and improved test assertions.
barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp Added additional assertions and helper methods for limb management.
barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp Updated tests to match new witness struct names and range constraint behaviors.
Comments suppressed due to low confidence (3)

barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp:885

  • Renaming the witness struct and removing the modulus parameter requires confirming that evaluate_non_native_field_multiplication correctly infers or retrieves modulus information from the provided limbs. Please verify that the downstream logic remains accurate without the explicit modulus argument.
        a_indices, b_indices, q_indices, r_indices, modulus_limbs,

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp:24

  • The new naming of the witness struct to non_native_multiplication_witnesses (and introduction of non_native_partial_multiplication_witnesses) improves clarity. Please ensure that all associated functions and call sites are updated consistently to reflect this change.
template <typename FF> struct non_native_multiplication_witnesses {

barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp:1013

  • The new fast modular exponentiation path for constant bigfield elements uses a loop with bitwise shifts and multiplications. Please ensure that edge cases (such as exponent values 0 and 1) are thoroughly tested and that the shift operations correctly update the exponent during each iteration.
    }

@Rumata888 Rumata888 assigned Rumata888 and suyash67 and unassigned Rumata888 Jun 23, 2025
@Rumata888 Rumata888 self-requested a review June 23, 2025 17:30
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp Outdated
suyash67 added 19 commits June 26, 2025 12:54
… those witness indices as uint32_t in the past).
…non_native_field_witnesses has members that are unused.
…` fn in field class. this fn was audited in sergei's internal field audit. further this todo is duplicated in field_conversion.cpp and doesn't seem relevant here anymore. hence removing.
@suyash67 suyash67 changed the base branch from next to merge-train/barretenberg June 27, 2025 12:04
@suyash67 suyash67 merged commit 9f4e391 into merge-train/barretenberg Jun 27, 2025
5 checks passed
@suyash67 suyash67 deleted the sb/bigfield-fixes branch June 27, 2025 13:00
github-merge-queue Bot pushed a commit that referenced this pull request Jul 2, 2025
See
[merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md).

chore: update merge-train documentation
feat!: (sequence of) Apps share a transcript until a kernel is
encountered (#15313)
chore: bigfield todo fixes (#15202)
chore: remove insecure logic from ipa recursion (#14102)
chore: bigfield audit fixes that change circuits (#15205)
feat: stdlib::Proof (#15410)
fix: build bb for bench_ivc script (#15429)
chore: revert "feat: stdlib::Proof (#15410)" (#15431)
chore: unrevert stdlib proof (#15443)
chore: package vk hash with vk (#15318)
chore: fix avm build (#15448)
chore: skip building wasm for bench_ivc when NO_WASM=1 (#15462)
chore: add a link to audit PR template in default PR template (#15463)

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com>
Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com>
Co-authored-by: Lucas Xia <lucasxia01@gmail.com>
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request Jul 16, 2025
See
[merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md).

chore: update merge-train documentation
feat!: (sequence of) Apps share a transcript until a kernel is
encountered (AztecProtocol#15313)
chore: bigfield todo fixes (AztecProtocol#15202)
chore: remove insecure logic from ipa recursion (AztecProtocol#14102)
chore: bigfield audit fixes that change circuits (AztecProtocol#15205)
feat: stdlib::Proof (AztecProtocol#15410)
fix: build bb for bench_ivc script (AztecProtocol#15429)
chore: revert "feat: stdlib::Proof (AztecProtocol#15410)" (AztecProtocol#15431)
chore: unrevert stdlib proof (AztecProtocol#15443)
chore: package vk hash with vk (AztecProtocol#15318)
chore: fix avm build (AztecProtocol#15448)
chore: skip building wasm for bench_ivc when NO_WASM=1 (AztecProtocol#15462)
chore: add a link to audit PR template in default PR template (AztecProtocol#15463)

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com>
Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com>
Co-authored-by: Lucas Xia <lucasxia01@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants