avoid Result<bool>, fix a bug in vess.verify

Co-authored-by: Anders Konring <[email protected]>

Author: alxiong

timeboost-crypto/src/feldman.rs

@@ -89,7 +89,7 @@ impl<C: CurveGroup> VerifiableSecretSharing for FeldmanVss<C> {
         node_idx: usize,
         share: &Self::SecretShare,
         commitment: &Self::Commitment,
-    ) -> Result<bool, VssError> {
+    ) -> Result<(), VssError> {
         let n = pp.n.get() as usize;
         let t = pp.t.get() as usize;
 
@@ -113,7 +113,11 @@ impl<C: CurveGroup> VerifiableSecretSharing for FeldmanVss<C> {
             VssError::InternalError("commitments and powers mismatched length".to_string())
         })?;
 
-        Ok(C::generator().mul(share) == eval_in_exp)
+        if C::generator().mul(share) == eval_in_exp {
+            Ok(())
+        } else {
+            Err(VssError::FailedVerification)
+        }
     }
 
     fn reconstruct(
@@ -167,29 +171,27 @@ mod tests {
             let (shares, commitment) = FeldmanVss::<C>::share(&pp, rng, secret);
             for (node_idx, s) in shares.iter().enumerate() {
                 // happy path
-                assert!(FeldmanVss::<C>::verify(&pp, node_idx, s, &commitment).unwrap());
+                assert!(FeldmanVss::<C>::verify(&pp, node_idx, s, &commitment).is_ok());
 
                 // sad path
                 // wrong node_idx should fail
-                assert!(
-                    !FeldmanVss::<C>::verify(&pp, node_idx + 1, s, &commitment).unwrap_or(false)
-                );
+                assert!(FeldmanVss::<C>::verify(&pp, node_idx + 1, s, &commitment).is_err());
 
                 // wrong secret share should fail
                 assert!(
-                    !FeldmanVss::<C>::verify(
+                    FeldmanVss::<C>::verify(
                         &pp,
                         node_idx,
                         &C::ScalarField::rand(rng),
                         &commitment,
                     )
-                    .unwrap()
+                    .is_err()
                 );
 
                 // wrong commitment should fail
                 let mut bad_comm = commitment.clone();
                 bad_comm[1] = C::Affine::default();
-                assert!(!FeldmanVss::<C>::verify(&pp, node_idx, s, &bad_comm).unwrap());
+                assert!(FeldmanVss::<C>::verify(&pp, node_idx, s, &bad_comm).is_err());
 
                 // incomplete/dropped commitment should fail
                 bad_comm.pop();

timeboost-crypto/src/traits/dkg.rs

@@ -36,13 +36,13 @@ pub trait VerifiableSecretSharing {
     /// - `share`: the secret share to verify
     /// - `commitment`: the global commitment (if any)
     ///
-    /// Returns Ok(true) if valid, Ok(false) if invalid, or an appropriate `VssError` otherwise.
+    /// Returns Ok(()) if valid, or an appropriate `VssError` otherwise.
     fn verify(
         pp: &Self::PublicParam,
         node_idx: usize,
         share: &Self::SecretShare,
         commitment: &Self::Commitment,
-    ) -> Result<bool, VssError>;
+    ) -> Result<(), VssError>;
 
     /// Reconstructs the original secret from a set of (index, share) pairs.
     ///
@@ -66,6 +66,8 @@ pub enum VssError {
     InvalidShare(usize, String),
     #[error("invalid VSS commitment")]
     InvalidCommitment,
+    #[error("failed verification: share does not match commitment")]
+    FailedVerification,
     #[error("failed to reconstruct: {0}")]
     FailedReconstruction(String),
     #[error("internal err: {0}")]

timeboost-crypto/src/vess.rs

@@ -372,7 +372,7 @@ impl<C: CurveGroup> ShoupVess<C> {
         ct: &VessCiphertext,
         comm: &<FeldmanVss<C> as VerifiableSecretSharing>::Commitment,
         aad: &[u8],
-    ) -> Result<bool, VessError> {
+    ) -> Result<(), VessError> {
         let mut verifier_state = self.io_pattern(aad).to_verifier_state(&ct.transcript);
 
         // verifier logic until Step 4b
@@ -407,10 +407,14 @@ impl<C: CurveGroup> ShoupVess<C> {
                             .expect("subset_size > 0, so is shifted_polys.len()")
                             .as_ref(),
                     );
+
+                    let mut unshifted_comm = vec![];
                     for (shifted, delta) in shifted_comm.into_iter().zip(comm.iter()) {
                         // g^omega'' / C in paper
-                        hasher.update(serialize_to_vec![shifted - delta]?)
+                        unshifted_comm.push(shifted - delta);
                     }
+                    let unshifted_comm = C::normalize_batch(&unshifted_comm);
+                    hasher.update(serialize_to_vec![unshifted_comm]?);
 
                     let mre_ct = mre_cts
                         .pop_front()
@@ -435,7 +439,11 @@ impl<C: CurveGroup> ShoupVess<C> {
         debug_assert!(mre_cts.is_empty());
         debug_assert!(seeds.is_empty());
 
-        Ok(h != hasher.finalize().as_slice())
+        if h == hasher.finalize().as_slice() {
+            Ok(())
+        } else {
+            Err(VessError::FailedVerification)
+        }
     }
 
     /// Decrypt with a decryption key `recv_sk` (labeled with node_idx, see `LabeledDecryptionKey`)
@@ -473,7 +481,7 @@ impl<C: CurveGroup> ShoupVess<C> {
             let share = shifted_eval - unshifted_eval;
 
             // check correctness
-            if FeldmanVss::<C>::verify(&self.vss_pp, node_idx, &share, &comm)? {
+            if FeldmanVss::<C>::verify(&self.vss_pp, node_idx, &share, &comm).is_ok() {
                 return Ok(share);
             }
         }
@@ -643,6 +651,8 @@ pub enum VessError {
     IndexOutOfBound(usize, usize),
     #[error("wrong vss commitment supplied")]
     WrongCommitment,
+    #[error("failed verification: proof verification failed")]
+    FailedVerification,
     #[error("decryption fail")]
     DecryptionFailed,
 }
@@ -692,10 +702,10 @@ mod tests {
         let aad = b"Associated data";
         let (ct, comm) = vess.encrypted_shares(&recv_pks, secret, aad).unwrap();
 
-        assert!(vess.verify(&recv_pks, &ct, &comm, aad).unwrap());
+        assert!(vess.verify(&recv_pks, &ct, &comm, aad).is_ok());
         for labeled_recv_sk in labeled_sks {
             let share = vess.decrypt_share(&labeled_recv_sk, &ct, aad).unwrap();
-            assert!(Vss::verify(&vess.vss_pp, labeled_recv_sk.node_idx, &share, &comm).unwrap());
+            assert!(Vss::verify(&vess.vss_pp, labeled_recv_sk.node_idx, &share, &comm).is_ok());
         }
     }