Skip to content

chore: Stdlib curves audit#21177

Merged
federicobarbacovi merged 4 commits into
merge-train/barretenbergfrom
fb/stdlib_curve_audit
Mar 11, 2026
Merged

chore: Stdlib curves audit#21177
federicobarbacovi merged 4 commits into
merge-train/barretenbergfrom
fb/stdlib_curve_audit

Conversation

@federicobarbacovi

Copy link
Copy Markdown
Contributor

🧾 Audit Context

Stdlib curve audit

🛠️ Changes Made

Only refactoring: remove some unused aliases in the structs.

✅ 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 federicobarbacovi self-assigned this Mar 5, 2026
@federicobarbacovi federicobarbacovi marked this pull request as ready for review March 5, 2026 16:44
using Element = Group;

// Additional types with no analog in the native description of the curve
using witness_ct = witness_t<CircuitBuilder>;

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.

I removed these aliases because it didn't look like they belonged here

using byte_array_ct = byte_array<Builder>;
using bool_ct = bool_t<Builder>;

using fq_ct = bigfield<Builder, typename ::bb::secp256k1::FqParams>;

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.

Using ScalarField/BaseField/Group in the codebase. These aliases were duplicate

using byte_array_ct = byte_array<CircuitBuilder>;
using bool_ct = bool_t<CircuitBuilder>;

using bigfr_ct = bigfield<CircuitBuilder, bb::Bn254FrParams>;

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.

Removed as it was confusing to have a bigfield representation of the scalar field. I modified the test for biggroup so that the case in which the scalar field is instantiated via biggroup is still tested

@suyash67 suyash67 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.

Looks good, thanks for the cleanup!

@federicobarbacovi federicobarbacovi enabled auto-merge (squash) March 11, 2026 08:12
@federicobarbacovi federicobarbacovi merged commit 707994e into merge-train/barretenberg Mar 11, 2026
10 checks passed
@federicobarbacovi federicobarbacovi deleted the fb/stdlib_curve_audit branch March 11, 2026 08:26
github-merge-queue Bot pushed a commit that referenced this pull request Mar 11, 2026
BEGIN_COMMIT_OVERRIDE
chore: use free witness for offset generator in `batch_mul` (#21040)
chore: Stdlib curves audit (#21177)
END_COMMIT_OVERRIDE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants