Fixed an issue where providing an incomplete group along with a complete one would throw an error.

Author: wolfmcnally

src/encoding.rs

@@ -246,22 +246,39 @@ fn combine_shares(shares: &[SSKRShare]) -> Result<Secret, SSKRError> {
         }
     }
 
+    // Check that we have enough groups to recover the master secret
     if next_group < group_threshold {
         return Err(SSKRError::NotEnoughGroups);
     }
 
-    // here, all of the shares are unpacked into member groups. Now we go through each
+    // Here, all of the shares are unpacked into member groups. Now we go through each
     // group and recover the group secret, and then use the result to recover the
     // master secret
     let mut master_indexes = Vec::with_capacity(16);
     let mut master_shares = Vec::with_capacity(16);
 
     for group in groups {
-        let group_secret = recover_secret(&group.member_indexes, &group.member_shares)?;
-        master_indexes.push(group.group_index);
-        master_shares.push(group_secret);
+        // Only attempt to recover the group secret if we have enough shares
+        if group.member_indexes.len() < group.member_threshold {
+            continue;
+        }
+        // Recover the group secret
+        if let Ok(group_secret) = recover_secret(&group.member_indexes, &group.member_shares) {
+            master_indexes.push(group.group_index);
+            master_shares.push(group_secret);
+        }
+        // Stop if we have enough groups to recover the master secret
+        if master_indexes.len() == group_threshold {
+            break;
+        }
+    }
+
+    // If we don't have enough groups to recover the master secret, return an error
+    if master_indexes.len() < group_threshold {
+        return Err(SSKRError::NotEnoughGroups);
     }
 
+    // Recover the master secret
     let master_secret = recover_secret(&master_indexes, &master_shares)?;
     let master_secret = Secret::new(master_secret)?;
 

src/lib.rs

@@ -424,4 +424,27 @@ mod tests {
             assert_eq!(from_utf8(result.unwrap().data()).unwrap(), TEXT);
         }
     }
+
+    /// Test fix for [seedtool-cli #6](https://github.com/BlockchainCommons/seedtool-cli-rust/issues/6).
+    #[test]
+    fn example_encode_4() {
+        use crate::{ Secret, GroupSpec, Spec, sskr_generate, sskr_combine };
+        use std::str::from_utf8;
+
+        const TEXT: &str = "my secret belongs to me.";
+        let secret = Secret::new(TEXT).unwrap();
+        let spec = Spec::new(1, vec![GroupSpec::new(2, 3).unwrap(), GroupSpec::new(2, 3).unwrap()]).unwrap();
+        let groupd_shares: Vec<Vec<Vec<u8>>> = sskr_generate(&spec, &secret).unwrap();
+        let flattened_shares = groupd_shares.into_iter().flatten().collect::<Vec<Vec<u8>>>();
+        // The group threshold is 1, but we're providing an additional share from the second group.
+        // This was previously causing an error, because the second group could not be decoded.
+        // The correct behavior is to ignore any group's shares that cannot be decoded.
+        let recovered_share_indexes = [0, 1, 3];
+        let recovered_shares = recovered_share_indexes
+            .iter()
+            .map(|index| flattened_shares[*index].clone())
+            .collect::<Vec<Vec<u8>>>();
+        let recovered_secret = sskr_combine(&recovered_shares).unwrap();
+        assert_eq!(from_utf8(recovered_secret.data()).unwrap(), TEXT);
+    }
 }