Skip to content

Polynomial and NTT interfaces for Rhizomes#1399

Merged
tgeoghegan merged 5 commits intomainfrom
timg/rhizomes-preliminaries
Mar 10, 2026
Merged

Polynomial and NTT interfaces for Rhizomes#1399
tgeoghegan merged 5 commits intomainfrom
timg/rhizomes-preliminaries

Conversation

@tgeoghegan
Copy link
Contributor

Some preliminary work ahead of #1394.

  • mod polynomial: rename functions to make it clear which basis inputs and outputs are in
  • add polynomial::poly_mul_lagrange, extend_values_to_power_of_2, double_evaluations
  • test_poly_eval_lagrange_batched now requires all input polynomials to have equal length, per spec. Further assertions will follow in Evaluate polynomials in Lagrange basis (Rhizomes) #1394.
  • add ntt::ntt_set_s and various convenience function to mod ntt
  • take lists of polynomials as &[P] where F: FieldElement, P: AsRef<[F]> where Rust allows (we can't change methods on trait Gadget because it needs to be dyn compatible)

Part of #1394

Some preliminary work ahead of #1394.

- `mod polynomial`: rename functions to make it clear which basis inputs
  and outputs are in
- add `polynomial::poly_mul_lagrange, extend_values_to_power_of_2`,
  `double_evaluations`
- `test_poly_eval_lagrange_batched` now requires all input polynomials
  to have equal length, per spec. Further assertions will follow in
  #1394.
- add `ntt::ntt_set_s` and various convenience function to `mod ntt`

Part of #1394
@tgeoghegan tgeoghegan requested a review from a team as a code owner March 9, 2026 19:48
test_poly_eval_batched_with_lengths(&[1, 2, 4, 16, 64]);
// arbitrary
test_poly_eval_batched_with_lengths(&[1, 6, 3, 9]);
test_poly_eval_lagrange_batched_with_lengths(&[64, 64, 64, 64, 64]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was batch evaluating polynomials of various lengths together, but per spec that isn't allowed:

def poly_eval_batched(self, polys: list[list[F]], x: F) -> list[F]:
        """Evaluate each polynomial in the Lagrange basis at x."""
        assert len({len(p) for p in polys}) == 1 # <-- polynomials must have equal length
        n = len(polys[0])
        assert_power_of_2(n)

        nodes = self.field.nth_root_powers(n)
        k = self.field(1)
        u = [p[0] for p in polys]
        d = nodes[0] - x
        for i in range(1, n):
            k *= d
            d = nodes[i] - x
            t = k * nodes[i]
            for j, p in enumerate(polys):
                u[j] *= d
                if i < len(p):
                    u[j] += t * p[i]

        factor = self.field(-1)**(n-1) * self.field(n).inv()
        for i in range(len(u)):
            u[i] *= factor
        return u

https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-vdaf-18#section-6.1.3.2

// TODO(#1394): enable these assertions once `Flp.query` is fixed to match
// https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-vdaf-18#section-7.3.4
// assert!(polynomials[0].as_ref().len().is_power_of_two());
// assert_eq!(roots.len(), poly_len, "incorrect number of roots provided");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH it's odd to me that the roots get passed into this function, because that just allows the caller to compute the wrong number of them. AFAICT from the reference implementation, there are no cases where we would re-use a computed set of roots across multiple calls to poly_eval_lagrange_batched so I'm tempted to remove the parameter roots and do let roots = nth_root_powers(poly_len); here. But I can't do that until the call side in Flp::query is cleaned up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it could be useful to take the roots as an argument if we precompute and store them at initialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but we don't, so as it stands, this is just something that's easy to get wrong. Since these are crate-internal interfaces, we could always add such an optimization in the future.

src/ntt.rs Outdated
/// basis.
///
/// Interpreting the input as the coefficients of a polynomial, the output is equal to the input
/// evaluated at points `p^0, p^1, ... p^(size-1)`, where `p` is the `2^size`-th principal root of
Copy link
Contributor

Choose a reason for hiding this comment

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

This talks about the 2^size-th ... root, but size is already a power of two (e.g., 128), so would this be just the size-th root, like in the next comment down on line 48?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you're right. I also updated the comment on ntt_set_s to say p is a size-th root and s is a 2size-th root.

src/flp.rs Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

just trying to re-orient here I saw these call_poly references, but those are now eval_poly, so we might need another pass of cleanup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I did another project wide search for call and call_poly and cleaned up a few more.

ntt_inv(front, evaluations, evaluations.len())?;
ntt_set_s(back, front, evaluations.len())?;

// Interleave the input (even indices) with the back half of output (odd indices), into output
Copy link
Contributor

Choose a reason for hiding this comment

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

reading and writing to output in the same loop feels worthy of slightly more comment imo. It's safe because of the even/odd-ness, but perhaps something like "due to the even/odd split, we're never reading and writing the same memory at the same time" ?

src/flp.rs Outdated
@@ -614,11 +614,11 @@ pub trait Gadget<F: NttFriendlyFieldElement>: Debug {
fn eval_poly(&mut self, outp: &mut [F], inp: &[Vec<F>]) -> Result<(), FlpError>;

/// Returns the arity of the gadget. This is the length of `inp` passed to `call` or
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: another comment to update post-gadget method renaming

Suggested change
/// Returns the arity of the gadget. This is the length of `inp` passed to `call` or
/// Returns the arity of the gadget. This is the length of `inp` passed to `eval` or

Comment on lines +85 to +86
/// same length, which must be a power of 2. For input polynomials of length `k` and degree `k - 1`,
/// the output polynomial will have length `2k` and degree `2k - 1`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The product of the polynomials will have degree 2k - 2 (the sum of the degrees of the multiplicands) but we will still store it as 2k evaluations, i.e. with one over-determined coordinate, for compatibility with NTT-based algorithms.

// TODO(#1394): enable these assertions once `Flp.query` is fixed to match
// https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-vdaf-18#section-7.3.4
// assert!(polynomials[0].as_ref().len().is_power_of_two());
// assert_eq!(roots.len(), poly_len, "incorrect number of roots provided");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it could be useful to take the roots as an argument if we precompute and store them at initialization.

@tgeoghegan tgeoghegan enabled auto-merge (squash) March 10, 2026 20:57
@tgeoghegan tgeoghegan merged commit 02ce5db into main Mar 10, 2026
6 checks passed
@tgeoghegan tgeoghegan deleted the timg/rhizomes-preliminaries branch March 10, 2026 21:01
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.

3 participants