Skip to content

Validate field elements to ensure 254-bit constraint #909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

albertyosef
Copy link

Description:
This PR addresses field element validation to ensure proper bit-width constraints are enforced. Previously, we accepted field elements that fit within 256 bits without properly validating against the BN254 curve's 254-bit constraint.

Key Changes:

  • Replace BLS12-377 with BN254 curve for proper field element handling
  • Introduce fieldElementBitWidth constant (254 bits) for explicit validation
  • Ensure field elements are properly validated against BN254's field size
  • Add test cases to verify 254-bit constraint enforcement

Technical Details:

  • Switch from bls12-377/fr to bn254/fr for correct field arithmetic
  • Explicit validation that field elements are within 254 bits
  • ByteWidth calculation now uses (fieldElementBitWidth+7)/8 for clarity
  • Test cases cover:
    • Valid: 253-bit numbers
    • Boundary: 254-bit numbers
    • Reduction: Numbers > 254 bits (which get reduced mod r)

Testing:

  • Added comprehensive test cases for field element validation
  • Verified proper reduction behavior for the BN254 field
  • All existing tests pass with the new validation

Fixes #282

This introduces a constant for the maximum bit width of field elements (254 bits) and updates the ByteWidth and BitWidth methods to utilize this constant. The Accept method is also modified to ensure values fit within the defined bit width.
This commit introduces a new script, `update_curve.sh`, to automate the replacement of BLS12-377 references with BN254 in all Go files. The script updates both import paths and comments throughout the codebase to reflect the change in elliptic curve used.
Copy link

cla-assistant bot commented May 29, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

cla-assistant bot commented May 29, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@albertyosef
Copy link
Author

@DavePearce hey, could you please review the changes, and check if everythings alright !

@DavePearce
Copy link
Collaborator

Hi @albertyosef --- sorry I'm not following what you are trying to achieve here? Do you have some reason for wanting the BN254 curve?

@albertyosef
Copy link
Author

@DavePearce I apologize for the confusion. I misinterpreted the original issue (#282) about field element validation. I incorrectly assumed we needed to switch to BN254 when the actual requirement was just to ensure proper 254-bit constraint validation for field elements.

Let me revise the PR to focus on the actual issue:

  1. Implement proper validation for 254-bit field elements
  2. Add the fieldElementBitWidth constant for explicit validation
  3. Add test cases to verify the constraints

I'll remove the curve switching changes and submit an updated version that only addresses the field element validation. Thank you for catching this misunderstanding.

…12-377

This commit removes the `update_curve.sh` script, which was used to automate the replacement of BLS12-377 references with BN254 in Go files. Additionally, it updates all relevant import paths and references throughout the codebase to reflect the use of BLS12-377 instead of BN254.
@albertyosef
Copy link
Author

Hi @DavePearce, apologies for the earlier confusion regarding the BN254 curve. I've revised the PR to focus solely on the field element validation as per issue #282. The changes now correctly implement the 254-bit constraint, include the fieldElementBitwidth constant, and have corresponding test cases. Thanks for your understanding! Please do guide me if there is any changes required.

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.

Validation for Field Elements
2 participants