diff --git a/src/error.rs b/src/error.rs index de41eda4b..a58205dde 100644 --- a/src/error.rs +++ b/src/error.rs @@ -53,7 +53,7 @@ pub enum Error { #[error("Value truncation error: {0}")] FieldValueTruncation(String), #[error("Invalid query parameter: {0}")] - InvalidQueryParameter(String), + InvalidQueryParameter(BoxError), #[error("invalid report: {0}")] InvalidReport(#[from] InvalidReportError), #[error("unsupported: {0}")] diff --git a/src/ff/galois_field.rs b/src/ff/galois_field.rs index d7d6e8312..0773755ca 100644 --- a/src/ff/galois_field.rs +++ b/src/ff/galois_field.rs @@ -463,7 +463,7 @@ macro_rules! bit_array_impl { } #[test] - #[should_panic] + #[should_panic(expected = "index < usize::try_from")] pub fn out_of_count_index() { let s = $name::try_from(1_u128).unwrap(); // Below assert doesn't matter. The indexing should panic diff --git a/src/protocol/attribution/credit_capping.rs b/src/protocol/attribution/credit_capping.rs index 4b9e91362..9d4c0f120 100644 --- a/src/protocol/attribution/credit_capping.rs +++ b/src/protocol/attribution/credit_capping.rs @@ -47,7 +47,7 @@ where if (u128::from(cap) * 2) >= F::PRIME.into() { return Err(crate::error::Error::InvalidQueryParameter(format!( "The cap {cap} must be less than 1/2 of the prime modulus to make overflow detectable, and propagable." - ))); + ).into())); } // @@ -527,7 +527,9 @@ mod tests { } #[tokio::test] - #[should_panic] + #[should_panic( + expected = "must be less than 1/2 of the prime modulus to make overflow detectable, and propagable" + )] pub async fn invalid_cap_value() { // Input doesn't matter here, since the test should panic before the computation starts. let input: Vec> = credit_capping_test_input!( diff --git a/src/protocol/basics/apply_permutation.rs b/src/protocol/basics/apply_permutation.rs index 571cd081f..4544c59db 100644 --- a/src/protocol/basics/apply_permutation.rs +++ b/src/protocol/basics/apply_permutation.rs @@ -30,6 +30,9 @@ pub fn apply(permutation: &[u32], values: &mut [T]) { /// reordered into (D, C, A, B). /// /// ![Apply inv steps][applyinv] +/// +/// ## Panics +/// If permutation size and values size do not match. pub fn apply_inv(permutation: &[u32], values: &mut [T]) { assert_eq!(permutation.len(), values.len()); let mut permuted = bitvec![0; permutation.len()]; diff --git a/src/protocol/basics/mul/sparse.rs b/src/protocol/basics/mul/sparse.rs index 97796931e..9f1ad9943 100644 --- a/src/protocol/basics/mul/sparse.rs +++ b/src/protocol/basics/mul/sparse.rs @@ -322,21 +322,21 @@ pub(in crate::protocol) mod test { #[test] #[cfg(debug_assertions)] - #[should_panic] + #[should_panic(expected = "attempting to do a multiplication that can be performed locally")] fn no_work_vzz() { (ZeroPositions::Pvzz, ZeroPositions::Pvzz).work_for(Role::H1); } #[test] #[cfg(debug_assertions)] - #[should_panic] + #[should_panic(expected = "attempting to do a multiplication that can be performed locally")] fn no_work_vzv() { (ZeroPositions::Pzvz, ZeroPositions::Pzvz).work_for(Role::H2); } #[test] #[cfg(debug_assertions)] - #[should_panic] + #[should_panic(expected = "attempting to do a multiplication that can be performed locally")] fn no_work_zzv() { (ZeroPositions::Pzzv, ZeroPositions::Pzzv).work_for(Role::H3); } diff --git a/src/protocol/ipa_prf/prf_sharding/bucket.rs b/src/protocol/ipa_prf/prf_sharding/bucket.rs index a1e62d1a2..3ea16d9d5 100644 --- a/src/protocol/ipa_prf/prf_sharding/bucket.rs +++ b/src/protocol/ipa_prf/prf_sharding/bucket.rs @@ -38,6 +38,22 @@ impl From for BucketStep { } } +#[derive(thiserror::Error, Debug)] +pub enum MoveToBucketError { + #[error("Bad value for the breakdown key: {0}")] + InvalidBreakdownKey(String), +} + +impl From for Error { + fn from(error: MoveToBucketError) -> Self { + match error { + e @ MoveToBucketError::InvalidBreakdownKey(_) => { + Error::InvalidQueryParameter(Box::new(e)) + } + } + } +} + #[embed_doc_image("tree-aggregation", "images/tree_aggregation.png")] /// This function moves a single value to a correct bucket using tree aggregation approach /// @@ -57,6 +73,9 @@ impl From for BucketStep { /// extra processing to ensure contribution doesn't end up in a wrong bucket. However, this requires extra multiplications. /// This would potentially not be needed in IPA (as the breakdown key is provided by the report collector, so a bad value only spoils their own result) but useful for PAM. /// This can be by passing `robust` as true. +/// +/// ## Errors +/// If `breakdown_count` does not fit into `BK` bits or greater than or equal to $2^9$ pub async fn move_single_value_to_bucket( ctx: C, record_id: RecordId, @@ -71,17 +90,23 @@ where S: LinearSecretSharing + Serializable + SecureMul, F: PrimeField + ExtendableField, { + const MAX_BREAKDOWNS: usize = 512; // constrained by the compact step ability to generate dynamic steps let mut step: usize = 1 << BK::BITS; - assert!( - breakdown_count <= 1 << BK::BITS, - "Asking for more buckets ({breakdown_count}) than bits in the key ({}) allow", - BK::BITS - ); - assert!( - breakdown_count <= 512, - "Our step implementation (BucketStep) cannot go past 256" - ); + if breakdown_count > step { + Err(MoveToBucketError::InvalidBreakdownKey(format!( + "Asking for more buckets ({breakdown_count}) than bits in the breakdown key ({}) allow", + BK::BITS + )))?; + } + + if breakdown_count > MAX_BREAKDOWNS { + Err(MoveToBucketError::InvalidBreakdownKey( + "Our step implementation (BucketStep) cannot go past {MAX_BREAKDOWNS} breakdown keys" + .to_string(), + ))?; + } + let mut row_contribution = vec![value; breakdown_count]; for (tree_depth, bit_of_bdkey) in bd_key.iter().enumerate().rev() { @@ -222,7 +247,7 @@ pub mod tests { } #[test] - #[should_panic] + #[should_panic(expected = "Asking for more buckets")] fn move_out_of_range_too_many_buckets_type() { run(move || async move { _ = move_to_bucket(MAX_BREAKDOWN_COUNT + 1, 0, false).await; @@ -230,7 +255,7 @@ pub mod tests { } #[test] - #[should_panic] + #[should_panic(expected = "Asking for more buckets")] fn move_out_of_range_too_many_buckets_steps() { run(move || async move { let breakdown_key_bits = get_bits::(0, Gf9Bit::BITS); diff --git a/src/protocol/prss/mod.rs b/src/protocol/prss/mod.rs index 2102fc659..383c9765c 100644 --- a/src/protocol/prss/mod.rs +++ b/src/protocol/prss/mod.rs @@ -492,7 +492,9 @@ pub mod test { } #[test] - #[should_panic] + #[should_panic( + expected = "Attempt access a sequential PRSS for protocol/test after another access" + )] fn indexed_then_sequential() { let [p1, _p2, _p3] = participants(); @@ -502,7 +504,9 @@ pub mod test { } #[test] - #[should_panic] + #[should_panic( + expected = "Attempt to get an indexed PRSS for protocol/test after retrieving a sequential PRSS" + )] fn sequential_then_indexed() { let [p1, _p2, _p3] = participants(); @@ -526,7 +530,9 @@ pub mod test { #[test] #[cfg(debug_assertions)] - #[should_panic] + #[should_panic( + expected = "Generated randomness for index '100' twice using the same key 'protocol/test'" + )] fn indexed_rejects_the_same_index() { let [p1, _p2, _p3] = participants(); let step = Gate::default().narrow("test"); diff --git a/src/protocol/sort/apply_sort/shuffle.rs b/src/protocol/sort/apply_sort/shuffle.rs index 22a354cec..91919c44f 100644 --- a/src/protocol/sort/apply_sort/shuffle.rs +++ b/src/protocol/sort/apply_sort/shuffle.rs @@ -73,6 +73,9 @@ where /// The Shuffle object receives a step function and appends a `ShuffleStep` to form a concrete step /// /// ![Shuffle steps][shuffle] +/// +/// ## Errors +/// If the underlying multiplicaion protocol fails on any of the input records. pub async fn shuffle_shares( input: Vec, random_permutations: (&[u32], &[u32]),