Rework validation criterias for accepting relations.

mmaker · mmaker · commit fbfb29a094ff · 2025-09-01T14:34:31.000-07:00
diff --git a/src/composition.rs b/src/composition.rs
@@ -246,14 +246,23 @@ impl<G: PrimeGroup + ConstantTimeEq> ComposedRelation<G> {
             let valid_witness = instances[i].is_witness_valid(w);
             let select_witness = valid_witness & !valid_witness_found;
 
-            let simulated_commitment_ptr  = &simulated_commitment as *const ComposedCommitment<G> as u64;
-            let commitment_ptr  = &commitment as *const ComposedCommitment<G> as u64;
+            let simulated_commitment_ptr =
+                &simulated_commitment as *const ComposedCommitment<G> as u64;
+            let commitment_ptr = &commitment as *const ComposedCommitment<G> as u64;
 
-            let selected_commitment_ptr = ConditionallySelectable::conditional_select(&simulated_commitment_ptr, &commitment_ptr, select_witness);
-            let discarded_commitment_ptr = ConditionallySelectable::conditional_select(&simulated_commitment_ptr, &commitment_ptr, !select_witness);
+            let selected_commitment_ptr = ConditionallySelectable::conditional_select(
+                &simulated_commitment_ptr,
+                &commitment_ptr,
+                select_witness,
+            );
+            let discarded_commitment_ptr = ConditionallySelectable::conditional_select(
+                &simulated_commitment_ptr,
+                &commitment_ptr,
+                !select_witness,
+            );
             let commitment = unsafe { &*(selected_commitment_ptr as *const ComposedCommitment<G>) };
-            let _discarded = unsafe { &*(discarded_commitment_ptr as *const ComposedCommitment<G>) };
-
+            let _discarded =
+                unsafe { &*(discarded_commitment_ptr as *const ComposedCommitment<G>) };
 
             commitments.push(commitment.clone());
             prover_states.push(ComposedOrProverStateEntry(
@@ -318,11 +327,19 @@ impl<G: PrimeGroup + ConstantTimeEq> ComposedRelation<G> {
             let response = instance.prover_response(prover_state, &challenge_i)?;
             let response_ptr = &response as *const ComposedResponse<G> as u64;
             let simulated_response_ptr = &simulated_response as *const ComposedResponse<G> as u64;
-            let selected_response_ptr = ConditionallySelectable::conditional_select(&simulated_response_ptr, &response_ptr, valid_witness);
-            let _discarded_response_ptr = ConditionallySelectable::conditional_select(&simulated_response_ptr, &response_ptr, !valid_witness);
+            let selected_response_ptr = ConditionallySelectable::conditional_select(
+                &simulated_response_ptr,
+                &response_ptr,
+                valid_witness,
+            );
+            let _discarded_response_ptr = ConditionallySelectable::conditional_select(
+                &simulated_response_ptr,
+                &response_ptr,
+                !valid_witness,
+            );
             let response = unsafe { &*(selected_response_ptr as *const ComposedResponse<G>) };
-            let _discarded_response = unsafe { &*(_discarded_response_ptr as *const ComposedResponse<G>) };
-
+            let _discarded_response =
+                unsafe { &*(_discarded_response_ptr as *const ComposedResponse<G>) };
 
             result_challenges.push(challenge_i);
             result_responses.push(response.clone());
@@ -333,9 +350,7 @@ impl<G: PrimeGroup + ConstantTimeEq> ComposedRelation<G> {
     }
 }
 
-impl<G: PrimeGroup + ConstantTimeEq> SigmaProtocol
-    for ComposedRelation<G>
-{
+impl<G: PrimeGroup + ConstantTimeEq> SigmaProtocol for ComposedRelation<G> {
     type Commitment = ComposedCommitment<G>;
     type ProverState = ComposedProverState<G>;
     type Response = ComposedResponse<G>;
@@ -587,9 +602,7 @@ impl<G: PrimeGroup + ConstantTimeEq> SigmaProtocol
     }
 }
 
-impl<G: PrimeGroup + ConstantTimeEq> SigmaProtocolSimulator
-    for ComposedRelation<G>
-{
+impl<G: PrimeGroup + ConstantTimeEq> SigmaProtocolSimulator for ComposedRelation<G> {
     fn simulate_commitment(
         &self,
         challenge: &Self::Challenge,
diff --git a/src/lib.rs b/src/lib.rs
@@ -56,7 +56,6 @@ pub mod errors;
 pub mod linear_relation;
 pub mod traits;
 
-
 pub(crate) mod duplex_sponge;
 pub(crate) mod fiat_shamir;
 pub(crate) mod group;
@@ -69,4 +68,4 @@ pub use fiat_shamir::Nizk;
 pub use linear_relation::LinearRelation;
 
 #[deprecated = "Use sigma_rs::group::serialization instead"]
-pub use group::serialization;
+pub use group::serialization;
diff --git a/src/linear_relation/canonical.rs b/src/linear_relation/canonical.rs
@@ -429,36 +429,40 @@ impl<G: PrimeGroup> TryFrom<&LinearRelation<G>> for CanonicalLinearRelation<G> {
 
         // Process each constraint using the modular helper method
         for (lhs, rhs) in iter::zip(&relation.image, &relation.linear_map.linear_combinations) {
-            let lhs_value = relation
-                .linear_map
-                .group_elements
-                .get(*lhs)
-                .map_err(|_| InvalidInstance::new("Unassigned group variable in image"))?;
-
-            // If the linear combination is trivial, check it directly and skip processing.
-            if rhs.0.iter().all(|weighted| {
-                matches!(weighted.term.scalar, ScalarTerm::Unit)
-                    || weighted.weight.is_zero_vartime()
-            }) {
-                let rhs_value = rhs.0.iter().fold(G::identity(), |acc, weighted| {
-                    acc + relation
-                        .linear_map
-                        .group_elements
-                        .get(weighted.term.elem)
-                        .unwrap_or_else(|_| {
-                            panic!("Unassigned group variable in linear combination")
-                        })
-                        * weighted.weight
-                });
-                if lhs_value == rhs_value {
-                    continue; // Skip processing trivially true constraints
-                }
-                // We know that there is no valid witness for the relation here.
-                // return Err(InvalidInstance::new(
-                //     "Trivial constraint does not hold (LHS != RHS)",
-                // ));
-            } else if lhs_value == G::identity() {
-                return Err(InvalidInstance::new("Image contains identity element"));
+            // If any group element in the image is not assigned, return `InvalidInstance`.
+            let lhs_value = relation.linear_map.group_elements.get(*lhs)?;
+
+            // If any group element in the linear constraints is not assigned, return `InvalidInstance`.
+            let rhs_elements = rhs
+                .0
+                .iter()
+                .map(|weighted| relation.linear_map.group_elements.get(weighted.term.elem))
+                .collect::<Result<Vec<G>, _>>()?;
+
+            // Compute the constant terms on the right-hand side of the equation.
+            let rhs_constants = rhs
+                .0
+                .iter()
+                .map(|element| match element.term.scalar {
+                    ScalarTerm::Unit => element.weight,
+                    _ => G::Scalar::ZERO,
+                })
+                .collect::<Vec<_>>();
+            let rhs_constant_term = G::msm(&rhs_constants, &rhs_elements);
+
+            // The right-hand side is trivial if it contains no scalar variables, or the weight is zero.
+            let is_trivial = rhs.0.iter().all(|term| {
+                matches!(term.term.scalar, ScalarTerm::Unit) || term.weight.is_zero_vartime()
+            });
+
+            // Skip processing trivially true constraints. There's nothing to prove here.
+            if is_trivial && rhs_constant_term == lhs_value {
+                continue;
+            }
+
+            // Disallow non-trivial equations with trivial solutions.
+            if !is_trivial && rhs_constant_term == lhs_value {
+                return Err(InvalidInstance::new("Trivial kernel in this relation"));
             }
 
             canonical.process_constraint(lhs, rhs, relation, &mut weighted_group_cache)?;
diff --git a/src/linear_relation/mod.rs b/src/linear_relation/mod.rs
@@ -196,8 +196,10 @@ impl<G: PrimeGroup> GroupMap<G> {
     pub fn get(&self, var: GroupVar<G>) -> Result<G, InvalidInstance> {
         match self.0.get(var.0) {
             Some(Some(elem)) => Ok(*elem),
-            Some(None) => Err(InvalidInstance::new("unassigned group variable")),
-            None => Err(InvalidInstance::new("unassigned group variable")),
+            Some(None) | None => Err(InvalidInstance::new(format!(
+                "unassigned group variable {}",
+                var.0
+            ))),
         }
     }
 
@@ -409,6 +411,13 @@ impl<G: PrimeGroup> LinearRelation<G> {
         GroupVar(self.linear_map.num_elements - 1, PhantomData)
     }
 
+    /// Allocates a point variable (group element) and sets it immediately to the given value
+    pub fn allocate_element_with(&mut self, element: G) -> GroupVar<G> {
+        let var = self.allocate_element();
+        self.set_element(var, element);
+        var
+    }
+
     /// Allocates `N` point variables (group elements) for use in the linear map.
     ///
     /// # Returns
@@ -431,6 +440,14 @@ impl<G: PrimeGroup> LinearRelation<G> {
         vars
     }
 
+    /// Allocates a point variable (group element) and sets it immediately to the given value.
+    pub fn allocate_elements_with(&mut self, elements: &[G]) -> Vec<GroupVar<G>> {
+        elements
+            .iter()
+            .map(|element| self.allocate_element_with(*element))
+            .collect()
+    }
+
     /// Assign a group element value to a point variable.
     ///
     /// # Parameters
diff --git a/src/linear_relation/ops.rs b/src/linear_relation/ops.rs
@@ -571,9 +571,9 @@ mod sub {
 #[cfg(test)]
 mod tests {
     use crate::linear_relation::{GroupVar, ScalarTerm, ScalarVar, Term};
+    use core::marker::PhantomData;
     use curve25519_dalek::RistrettoPoint as G;
     use curve25519_dalek::Scalar;
-    use core::marker::PhantomData;
 
     fn scalar_var(i: usize) -> ScalarVar<G> {
         ScalarVar(i, PhantomData)
diff --git a/src/tests/test_validation_criteria.rs b/src/tests/test_validation_criteria.rs
@@ -7,6 +7,8 @@
 mod instance_validation {
     use crate::linear_relation::{CanonicalLinearRelation, LinearRelation};
     use bls12_381::{G1Projective as G, Scalar};
+    use ff::Field;
+    use group::Group;
 
     #[test]
     fn test_unassigned_group_vars() {
@@ -67,8 +69,6 @@ mod instance_validation {
     #[test]
     #[allow(non_snake_case)]
     pub fn test_degenerate_equation() {
-        use ff::Field;
-
         // This relation should fail for two reasons:
         // 1. because var_B is not assigned
         let mut relation = LinearRelation::<G>::new();
@@ -137,57 +137,70 @@ mod instance_validation {
         let mut linear_relation = LinearRelation::<G>::new();
         let B_var = linear_relation.allocate_element();
         let C_var = linear_relation.allocate_eq(B_var);
-        linear_relation.set_element(B_var, B);
-        linear_relation.set_element(C_var, C);
+        linear_relation.set_elements([(B_var, B), (C_var, C)]);
         assert!(linear_relation.canonical().is_ok());
 
         // Also in this case, we know that no witness will ever satisfy the relation.
         // Also here, the relation is built even though the prover will never be able to give a valid proof for it.
         // X != B * pub_scalar + A * 3
         let mut linear_relation = LinearRelation::<G>::new();
-        let B_var = linear_relation.allocate_element();
-        let A_var = linear_relation.allocate_element();
+        let [B_var, A_var] = linear_relation.allocate_elements();
         let X_var = linear_relation.allocate_eq(B_var * pub_scalar + A_var * Scalar::from(3));
-
-        linear_relation.set_element(B_var, B);
-        linear_relation.set_element(A_var, A);
-        linear_relation.set_element(X_var, X);
+        linear_relation.set_elements([(B_var, B), (A_var, A), (X_var, X)]);
         assert!(linear_relation.canonical().is_ok());
 
         // The following relation is valid and should pass.
         let mut linear_relation = LinearRelation::<G>::new();
         let B_var = linear_relation.allocate_element();
         let C_var = linear_relation.allocate_eq(B_var);
-        linear_relation.set_element(B_var, B);
-        linear_relation.set_element(C_var, B);
+        linear_relation.set_elements([(B_var, B), (C_var, B)]);
         assert!(linear_relation.canonical().is_ok());
 
         // The following relation is valid and should pass.
         // C = B * pub_scalar + A * 3
         let mut linear_relation = LinearRelation::<G>::new();
-        let B_var = linear_relation.allocate_element();
-        let A_var = linear_relation.allocate_element();
+        let [B_var, A_var] = linear_relation.allocate_elements();
         let C_var = linear_relation.allocate_eq(B_var * pub_scalar + A_var * Scalar::from(3));
-
-        linear_relation.set_element(B_var, B);
-        linear_relation.set_element(A_var, A);
-        linear_relation.set_element(C_var, C);
+        linear_relation.set_elements([(B_var, B), (A_var, A), (C_var, C)]);
         assert!(linear_relation.canonical().is_ok());
 
         // The following relation is for
         // X = B * x + B * pub_scalar + A * 3
         // and should be considered a valid instance.
         let mut linear_relation = LinearRelation::<G>::new();
-
         let x_var = linear_relation.allocate_scalar();
-        let B_var = linear_relation.allocate_element();
-        let A_var = linear_relation.allocate_element();
+        let [B_var, A_var] = linear_relation.allocate_elements();
         let X_var = linear_relation
             .allocate_eq(B_var * x_var + B_var * pub_scalar + A_var * Scalar::from(3));
+        linear_relation.set_elements([(B_var, B), (A_var, A), (X_var, X)]);
+        assert!(linear_relation.canonical().is_ok());
+    }
+
+    // Create a relation with a zero image, such as
+    // 0 = x*A + y*B + C
+    // It should be accepted.
+    #[test]
+    fn test_statement_with_trivial_image() {
+        let mut rng = rand::thread_rng();
+        let mut linear_relation = LinearRelation::new();
+        let [x_var, y_var] = linear_relation.allocate_scalars();
+        let [Z_var, A_var, B_var, C_var] = linear_relation.allocate_elements();
+        linear_relation.append_equation(Z_var, x_var * A_var + y_var * B_var + C_var);
+        let [x, y] = [Scalar::random(&mut rng), Scalar::random(&mut rng)];
+        let Z = G::identity();
+        let A = G::random(&mut rng);
+        let B = G::generator();
+        let C = -x * A - y * B;
+
+        linear_relation.set_elements([(Z_var, Z), (A_var, A), (B_var, B), (C_var, C)]);
+        assert!(linear_relation.canonical().is_ok());
 
-        linear_relation.set_element(B_var, B);
-        linear_relation.set_element(A_var, A);
-        linear_relation.set_element(X_var, X);
+        let F_var = linear_relation.allocate_element();
+        let f_var = linear_relation.allocate_scalar();
+        linear_relation.append_equation(F_var, f_var * A_var);
+        let f = Scalar::random(&mut rng);
+        let F = A * f;
+        linear_relation.set_elements([(F_var, F), (A_var, A)]);
         assert!(linear_relation.canonical().is_ok());
     }
 }
