Skip to content
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

Polynomial ring operations #19

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

Polynomial ring operations #19

wants to merge 25 commits into from

Conversation

mattsuffern
Copy link
Collaborator

@mattsuffern mattsuffern commented Feb 13, 2025

Module Description

This module provides the implementation of various operations in the polynomial ring:

$$ R = \mathbb{Z}_q[X] / (X^d + 1) $$

Based on the "modular arithmetic primitive using wrapping operations" module by @pycckuu (unchanged)

Implemented Functions:

  • Polynomial Addition: add()
  • Polynomial Multiplication: mul()
  • Inner Product / Dot Product: inner_product()
  • Polynomial Subtraction: sub()
  • Polynomial Negation: neg()
  • Scalar Multiplication: scalar_mul()
  • Division by Monomials: div_by_monomial()
  • Polynomial Evaluation: eval()
  • Zero Check: is_zero()
  • Polynomial Equality Check: is_equal()

(closes #5)

mattsuffern and others added 9 commits February 12, 2025 16:22
Implement Zq struct representing elements in ℤ/(2³²)ℤ ring arithmetic
with native u32 operations. Key features:

-  Fully derived Debug, Clone, Copy, PartialEq, Eq, and Default traits
-  Implements Add/Sub/Mul operator traits with implicit modulo reduction
-  Provides Assign variants for in-place operations (AddAssign, etc)
-  Macro-generated trait impls ensuring DRY principle adherence
-  Display trait for formatted output in user interfaces
-  Extensive test coverage including edge cases:
  - Additive/multiplicative identity properties
  - Wrapping overflow/underflow behavior
  - Assignment operator correctness
  - u32 conversion invariants
  - Display formatting checks

Enables safe, zero-cost abstraction for cryptographic primitives
requiring modular arithmetic (e.g., FHE schemes, lattice-based crypto).
Leverages:
-  Rust's wrapping arithmetic for constant-time operations
-  Type-safe API preventing raw integer misuse

Performance characteristics:
-  All operations compile to single CPU instructions
-  No heap allocations

Issue-URL: #17
Copy link
Collaborator

@pycckuu pycckuu left a comment

Choose a reason for hiding this comment

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

Great Work Matteo! 🚀 See my suggestions 😄

Additionally, I suggest:

  1. Rename the struct to Rq to align with cryptographical terms and rename poly_rings.rs to rq.rs

  2. Implement operator traits for natural arithmetic syntax. This will enable idiomatic usage like let sum = &rq1 + &rq2; and if rq1 == rq2 while maintaining ring arithmetic correctness.

  3. I noticed you went beyond, but the original task only required:

    • Addition
    • Multiplication
    • Modular Reduction

Not sure if we need the additional operations. Maybe @NiDimi can confirm.

Copy link
Contributor

@maksimryndin maksimryndin left a comment

Choose a reason for hiding this comment

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

Very good job Mateo! 👍 You're doing a great progress with Rust! 🔥

Igor has made a thorough review 👍 I've only added some minors. Will review again later.

@NiDimi
Copy link
Contributor

NiDimi commented Feb 14, 2025

Not sure if we need the additional operations. Maybe @NiDimi can confirm.

@pycckuu My reasoning for adding it was that we need it for all the summation operations on the paper. For example for Ajtai:

The prover has to compute the commitment vector t as:

$t = As \in R$

The vector is computed using matrix-vector multiplication:

$t_i = \sum_j{A_{i,j} \cdot s_j}$

That requires adding the results of the polynomial multiplication.

Of course this is a suggestion if you think it can be done better feel free to suggest how to do it.

Copy link
Contributor

@NiDimi NiDimi left a comment

Choose a reason for hiding this comment

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

  1. Left some comments. 😄
  2. You need to consider what happens when the polynomials do not have the same number of coefficient. Right now If I am not mistaken the code will just fail.
  3. In rust commonly we use // for line comment and /// for doc comments. What that means is that we should use /// when explaining what the function does.

Overall a very good starting step 🔥

@mattsuffern
Copy link
Collaborator Author

Thank you very much, @pycckuu, @maksimryndin, @NiDimi, for all the help and comments! I think all the issues are now resolved, but if there's any other change I should make, please let me know

Copy link
Collaborator

@pycckuu pycckuu left a comment

Choose a reason for hiding this comment

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

@mattsuffern Code looks great! See my very minor suggestions!

Copy link
Contributor

@maksimryndin maksimryndin left a comment

Choose a reason for hiding this comment

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

Awesome job! 🥇
let's finish some minor things and move further! 👍

Copy link
Contributor

@maksimryndin maksimryndin left a comment

Choose a reason for hiding this comment

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

Awesome, @mattsuffern ! 👍 great progress!

@maksimryndin
Copy link
Contributor

Not sure if we need the additional operations. Maybe @NiDimi can confirm.

@pycckuu My reasoning for adding it was that we need it for all the summation operations on the paper. For example for Ajtai:

The prover has to compute the commitment vector t as:

t = A s ∈ R

The vector is computed using matrix-vector multiplication:

t i = ∑ j A i , j ⋅ s j

That requires adding the results of the polynomial multiplication.

Of course this is a suggestion if you think it can be done better feel free to suggest how to do it.

@NiDimi I believe this is already resolved. Could you please take a final look? :)

@maksimryndin maksimryndin requested a review from NiDimi February 20, 2025 13:56
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.

Define and Implement Polynomial Ring Operations
4 participants