Browse Source

Fix return type of verify_share

It was returning Result<bool, &'static str>, which used both the bool
to indicate success or failure and the possibility of Err to indicate
failure.

Instead return Result<(), &'static str>, which uses Ok(()) to indicate
success, and Err to indicate failure.
Ian Goldberg 4 years ago
parent
commit
b61a91a790
1 changed files with 9 additions and 13 deletions
  1. 9 13
      src/vss.rs

+ 9 - 13
src/vss.rs

@@ -70,7 +70,7 @@ pub fn generate_shares(
 }
 
 /// Verify that a share is consistent with a commitment.
-pub fn verify_share(share: &Share, commitment: &Commitment) -> Result<bool, &'static str> {
+pub fn verify_share(share: &Share, commitment: &Commitment) -> Result<(), &'static str> {
     let f_result = RISTRETTO_BASEPOINT_POINT * share.value;
 
     let x = Scalar::from(share.index);
@@ -80,8 +80,11 @@ pub fn verify_share(share: &Share, commitment: &Commitment) -> Result<bool, &'st
         |(x_to_the_i, sum_so_far), comm_i| (x_to_the_i * x, sum_so_far + x_to_the_i * comm_i),
     );
 
-    let is_valid = f_result == result;
-    Ok(is_valid)
+    if f_result == result {
+        Ok(())
+    } else {
+        Err("Share is inconsistent")
+    }
 }
 
 /// Reconstruct the secret from enough (at least the threshold) already-verified shares.
@@ -152,15 +155,11 @@ pub fn apply_share_update(
         value: old_share.value + update.value,
     };
 
-    let share_is_valid = match verify_share(&updated_share, updated_commitment) {
+    match verify_share(&updated_share, updated_commitment) {
         Ok(n) => n,
         Err(_) => return Err("Error when validating share"),
     };
 
-    if !share_is_valid {
-        return Err("Share is invalid");
-    }
-
     Ok(updated_share)
 }
 
@@ -249,7 +248,6 @@ mod tests {
         for share in shares {
             let is_valid = verify_share(&share, &com);
             assert!(is_valid.is_ok());
-            assert!(is_valid.unwrap());
         }
     }
 
@@ -269,8 +267,7 @@ mod tests {
         for share in shares1 {
             // test that commitments to a different set of shares fails
             let is_valid = verify_share(&share, &com2);
-            assert!(is_valid.is_ok());
-            assert_ne!(is_valid.unwrap(), true);
+            assert!(is_valid.is_err());
         }
     }
 
@@ -313,8 +310,7 @@ mod tests {
         // test that the old shares are not valid with the updated commitment
         for share in &shares {
             let is_valid = verify_share(&share, &updated_commitment);
-            assert!(is_valid.is_ok());
-            assert!(!is_valid.unwrap());
+            assert!(is_valid.is_err());
         }
     }