diff --git a/src/errors.rs b/src/errors.rs index 40fe950..b05605e 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -59,16 +59,6 @@ error_chain! { display("The shares are incompatible with each other.") } - IncompatibleThresholds(sets: Vec>) { - description("The shares are incompatible with each other because they do not all have the same threshold.") - display("The shares are incompatible with each other because they do not all have the same threshold.") - } - - IncompatibleDataLengths(sets: Vec>) { - description("The shares are incompatible with each other because they do not all have the same share data length.") - display("The shares are incompatible with each other because they do not all have the same share data length.") - } - MissingShares(provided: usize, required: u8) { description("The number of shares provided is insufficient to recover the secret.") display("{} shares are required to recover the secret, found only {}.", required, provided) @@ -133,10 +123,21 @@ error_chain! { display("This share number ({}) has already been used by a previous share.", share_id) } + InconsistentSecretLengths(id: u8, slen_: usize, ids: Vec, slen: usize) { + description("The shares are incompatible with each other because they do not all have the same secret length.") + display("The share identifier {} had secret length {}, while the secret length {} was found for share identifier(s): {:?}.", id, slen_, slen, ids) + } + InconsistentShares { description("The shares are inconsistent") display("The shares are inconsistent") } + + InconsistentThresholds(id: u8, k_: u8, ids: Vec, k: u8) { + description("The shares are incompatible with each other because they do not all have the same threshold.") + display("The share identifier {} had k = {}, while k = {} was found for share identifier(s): {:?}.", id, k_, k, ids) + } + } foreign_links { diff --git a/src/share/validation.rs b/src/share/validation.rs index 0395da0..9b4e158 100644 --- a/src/share/validation.rs +++ b/src/share/validation.rs @@ -1,5 +1,3 @@ -use std::collections::{HashMap, HashSet}; - use errors::*; use share::{IsShare, IsSignedShare}; @@ -32,11 +30,11 @@ pub(crate) fn validate_shares(shares: &Vec) -> Result<(u8, usize) let shares_count = shares.len(); let mut ids = Vec::with_capacity(shares_count); - let mut k_compatibility_sets = HashMap::new(); - let mut slen_compatibility_sets = HashMap::new(); + let mut threshold = 0; + let mut slen = 0; for share in shares { - let (id, threshold, slen) = ( + let (id, threshold_, slen_) = ( share.get_id(), share.get_threshold(), share.get_data().len(), @@ -44,75 +42,40 @@ pub(crate) fn validate_shares(shares: &Vec) -> Result<(u8, usize) if id < 1 { bail!(ErrorKind::ShareParsingInvalidShareId(id)) - } else if threshold < 2 { + } else if threshold_ < 2 { bail!(ErrorKind::ShareParsingInvalidShareThreshold(threshold, id)) - } else if slen < 1 { + } else if slen_ < 1 { bail!(ErrorKind::ShareParsingErrorEmptyShare(id)) } - k_compatibility_sets - .entry(threshold) - .or_insert_with(HashSet::new); - let k_set = k_compatibility_sets.get_mut(&threshold).unwrap(); - k_set.insert(id); - if ids.iter().any(|&x| x == id) { bail!(ErrorKind::DuplicateShareId(id)); } - slen_compatibility_sets - .entry(slen) - .or_insert_with(HashSet::new); - let slen_set = slen_compatibility_sets.get_mut(&slen).unwrap(); - slen_set.insert(id); + if threshold == 0 { + threshold = threshold_; + } else if threshold_ != threshold { + bail!(ErrorKind::InconsistentThresholds( + id, + threshold_, + ids, + threshold + )) + } + + if slen == 0 { + slen = slen_; + } else if slen_ != slen { + bail!(ErrorKind::InconsistentSecretLengths(id, slen_, ids, slen)) + } ids.push(id); } - // Validate threshold - let k_sets = k_compatibility_sets.keys().count(); - - match k_sets { - 1 => {} // All shares have the same roothash. - _ => { - bail! { - ErrorKind::IncompatibleThresholds( - k_compatibility_sets - .values() - .map(|x| x.to_owned()) - .collect(), - ) - } - } - } - - // It is safe to unwrap because k_sets == 1 - let threshold = k_compatibility_sets.keys().last().unwrap().to_owned(); - if shares_count < threshold as usize { - bail!(ErrorKind::MissingShares(shares_count, threshold)); + bail!(ErrorKind::MissingShares(shares_count, threshold)) } - // Validate share length consistency - let slen_sets = slen_compatibility_sets.keys().count(); - - match slen_sets { - 1 => {} // All shares have the same `data` field len - _ => { - bail! { - ErrorKind::IncompatibleDataLengths( - slen_compatibility_sets - .values() - .map(|x| x.to_owned()) - .collect(), - ) - } - } - } - - // It is safe to unwrap because slen_sets == 1 - let slen = slen_compatibility_sets.keys().last().unwrap().to_owned(); - Ok((threshold, slen)) } diff --git a/tests/recovery_errors.rs b/tests/recovery_errors.rs index c8f13e1..fa862f3 100644 --- a/tests/recovery_errors.rs +++ b/tests/recovery_errors.rs @@ -67,8 +67,8 @@ fn test_recover_duplicate_shares_number() { } #[test] -#[should_panic(expected = "IncompatibleDataLengths")] -fn test_recover_incompatible_data_lengths() { +#[should_panic(expected = "InconsistentSecretLengths")] +fn test_recover_inconsistent_secret_lengths() { let share1 = "2-1-CgnlCxRNtnkzENE".to_string(); let share2 = "2-2-ChbG46L1zRszs0PPn63XnnupmZTcgYJ3".to_string(); @@ -77,6 +77,17 @@ fn test_recover_incompatible_data_lengths() { recover_secret(&shares, false).unwrap(); } +#[test] +#[should_panic(expected = "InconsistentThresholds")] +fn test_inconsistent_thresholds() { + let share1 = "2-1-CgnlCxRNtnkzENE".to_string(); + let share2 = "3-2-CgkAnUgP3lfwjyM".to_string(); + + let shares = vec![share1, share2]; + + recover_secret(&shares, false).unwrap(); +} + #[test] #[should_panic(expected = "MissingShares")] fn test_recover_too_few_shares() {