Skip to content

Conversation

phrwlk
Copy link
Contributor

@phrwlk phrwlk commented Aug 19, 2025

  • Align constraint evaluation with native evaluation for dim_h == 1 by returning x^2 - h^2 instead of 0.
  • Add a unit test constraints_test_dim1 in src/poly/domain/vanishing_poly.rs.
  • Remove unused import ark_std::ops::Sub to silence the warning.

Test/build pass

@phrwlk phrwlk requested a review from a team as a code owner August 19, 2025 08:17
@phrwlk phrwlk requested review from z-tech, Pratyush and weikengchen and removed request for a team August 19, 2025 08:17
Comment on lines 42 to 44
let mut cur = x.square()?;
cur -= &FpVar::Constant(self.constant_term);
return Ok(cur);
Copy link
Member

Choose a reason for hiding this comment

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

I think you can just remove this if statement, no?

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 can just remove this if statement, no?

Yes sir

@Pratyush
Copy link
Member

Thank you, could you add a test for this setting?

@phrwlk
Copy link
Contributor Author

phrwlk commented Sep 12, 2025

Thank you, could you add a test for this setting?

Sorry, but the test is already there?

#[test]
fn constraints_test_dim1() {
    let mut rng = test_rng();
    let offset = Fr::rand(&mut rng);
    let cs = ConstraintSystem::new_ref();
    let x = Fr::rand(&mut rng);
    let x_var = FpVar::new_witness(ns!(cs, "x_var"), || Ok(x)).unwrap();
    let vp = VanishingPolynomial::new(offset, 1);
    let native = vp.evaluate(&x);
    let result_var = vp.evaluate_constraints(&x_var).unwrap();
    assert!(cs.is_satisfied().unwrap());
    assert_eq!(result_var.value().unwrap(), native);
}

@Pratyush
Copy link
Member

Ah you're right, thanks!

@Pratyush Pratyush enabled auto-merge September 12, 2025 23:16
@Pratyush Pratyush added this pull request to the merge queue Sep 12, 2025
Merged via the queue into arkworks-rs:master with commit 3b12258 Sep 12, 2025
11 checks passed
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