diff --git a/core/src/task.rs b/core/src/task.rs index 0228097ca..f7de447eb 100644 --- a/core/src/task.rs +++ b/core/src/task.rs @@ -30,8 +30,8 @@ pub enum VdafInstance { Prio3Sum { bits: usize }, /// A vector of `Prio3` sums. Prio3SumVec { bits: usize, length: usize }, - /// A `Prio3` histogram. - Prio3Histogram { buckets: usize }, + /// A `Prio3` histogram with `length` buckets in it. + Prio3Histogram { length: usize }, /// A `Prio3` 16-bit fixed point vector sum with bounded L2 norm. #[cfg(feature = "fpvec_bounded_l2")] Prio3FixedPoint16BitBoundedL2VecSum { length: usize }, @@ -84,7 +84,10 @@ impl TryFrom<&taskprov::VdafType> for VdafInstance { bits: *bits as usize, }), taskprov::VdafType::Prio3Histogram { buckets } => Ok(Self::Prio3Histogram { - buckets: buckets.clone(), + // taskprov does not yet deal with the VDAF-06 representation of histograms. In the + // meantime, we translate the bucket boundaries to a length that Janus understands. + // https://github.com/wangshan/draft-wang-ppm-dap-taskprov/issues/33 + length: buckets.len() + 1, // +1 to account for the top bucket extending to infinity }), taskprov::VdafType::Poplar1 { bits } => Ok(Self::Poplar1 { bits: *bits as usize, @@ -94,10 +97,6 @@ impl TryFrom<&taskprov::VdafType> for VdafInstance { } } -fn bucket_count(buckets: &Vec, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "[{} buckets]", buckets.len() + 1) -} - /// Internal implementation details of [`vdaf_dispatch`](crate::vdaf_dispatch). #[macro_export] macro_rules! vdaf_dispatch_impl_base { @@ -171,8 +170,8 @@ macro_rules! vdaf_dispatch_impl_base { $body } - ::janus_core::task::VdafInstance::Prio3Histogram { buckets } => { - let $vdaf = ::prio::vdaf::prio3::Prio3::new_histogram(2, *buckets)?; + ::janus_core::task::VdafInstance::Prio3Histogram { length } => { + let $vdaf = ::prio::vdaf::prio3::Prio3::new_histogram(2, *length)?; type $Vdaf = ::prio::vdaf::prio3::Prio3Histogram; const $VERIFY_KEY_LENGTH: usize = ::janus_core::task::PRIO3_VERIFY_KEY_LENGTH; $body @@ -765,14 +764,14 @@ mod tests { ], ); assert_tokens( - &VdafInstance::Prio3Histogram { buckets: 6 }, + &VdafInstance::Prio3Histogram { length: 6 }, &[ Token::StructVariant { name: "VdafInstance", variant: "Prio3Histogram", len: 1, }, - Token::Str("buckets"), + Token::Str("length"), Token::U64(6), Token::StructVariantEnd, ], diff --git a/integration_tests/src/divviup_api_client.rs b/integration_tests/src/divviup_api_client.rs index b7d293239..80d35cf29 100644 --- a/integration_tests/src/divviup_api_client.rs +++ b/integration_tests/src/divviup_api_client.rs @@ -21,7 +21,7 @@ pub enum ApiVdaf { /// Corresponds to Prio3Count Count, Histogram { - buckets: usize, + buckets: Vec, }, Sum { bits: usize, @@ -35,8 +35,14 @@ impl TryFrom<&VdafInstance> for ApiVdaf { match vdaf { VdafInstance::Prio3Count => Ok(ApiVdaf::Count), VdafInstance::Prio3Sum { bits } => Ok(ApiVdaf::Sum { bits: *bits }), - VdafInstance::Prio3Histogram { buckets } => { - Ok(ApiVdaf::Histogram { buckets: *buckets }) + VdafInstance::Prio3Histogram { length } => { + // divviup-api does not yet support the new Prio3Histogram representation. Until it + // does, we synthesize fake bucket boundaries that will yield the number of buckets + // we want. + // https://github.com/divviup/divviup-api/issues/410 + Ok(ApiVdaf::Histogram { + buckets: (0..=length).collect(), + }) } _ => Err(anyhow!("unsupported VDAF: {vdaf:?}")), } diff --git a/interop_binaries/src/lib.rs b/interop_binaries/src/lib.rs index 5b6e5ed45..926b8535a 100644 --- a/interop_binaries/src/lib.rs +++ b/interop_binaries/src/lib.rs @@ -117,7 +117,7 @@ pub enum VdafObject { length: NumberAsString, }, Prio3Histogram { - buckets: NumberAsString, + length: NumberAsString, }, #[cfg(feature = "fpvec_bounded_l2")] Prio3FixedPoint16BitBoundedL2VecSum { @@ -151,8 +151,8 @@ impl From for VdafObject { length: NumberAsString(length), }, - VdafInstance::Prio3Histogram { buckets } => VdafObject::Prio3Histogram { - buckets: NumberAsString(buckets), + VdafInstance::Prio3Histogram { length } => VdafObject::Prio3Histogram { + length: NumberAsString(length), }, #[cfg(feature = "fpvec_bounded_l2")] @@ -196,8 +196,8 @@ impl From for VdafInstance { length: length.0, }, - VdafObject::Prio3Histogram { buckets } => { - VdafInstance::Prio3Histogram { buckets: buckets.0 } + VdafObject::Prio3Histogram { length } => { + VdafInstance::Prio3Histogram { length: length.0 } } #[cfg(feature = "fpvec_bounded_l2")] diff --git a/messages/src/taskprov.rs b/messages/src/taskprov.rs index 3d9a4f276..72036498c 100644 --- a/messages/src/taskprov.rs +++ b/messages/src/taskprov.rs @@ -9,10 +9,7 @@ use prio::codec::{ decode_u16_items, decode_u24_items, decode_u8_items, encode_u16_items, encode_u24_items, encode_u8_items, CodecError, Decode, Encode, }; -use std::{ - fmt::{self, Debug, Formatter}, - io::Cursor, -}; +use std::{fmt::Debug, io::Cursor}; /// Defines all parameters necessary to configure an aggregator with a new task. /// Provided by taskprov participants in all requests incident to task execution. @@ -325,8 +322,9 @@ pub enum VdafType { bits: u8, }, Prio3Histogram { - /// List of buckets. - #[derivative(Debug(format_with = "fmt_histogram"))] + /// Number of buckets in the histogram + // This may change as the taskprov draft adapts to VDAF-06 + // https://github.com/wangshan/draft-wang-ppm-dap-taskprov/issues/33 buckets: Vec, }, Poplar1 { @@ -342,10 +340,6 @@ impl VdafType { const POPLAR1: u32 = 0x00001000; } -fn fmt_histogram(buckets: &Vec, f: &mut Formatter) -> Result<(), fmt::Error> { - write!(f, "num_buckets: {}", buckets.len()) -} - impl Encode for VdafType { fn encode(&self, bytes: &mut Vec) { match self { diff --git a/tools/src/bin/collect.rs b/tools/src/bin/collect.rs index 737fe8157..eaf8c57d8 100644 --- a/tools/src/bin/collect.rs +++ b/tools/src/bin/collect.rs @@ -320,21 +320,13 @@ struct Options { display_order = 0 )] vdaf: VdafType, - /// Number of vector elements, for use with --vdaf=countvec and --vdaf=sumvec + /// Number of vector elements, when used with --vdaf=countvec and --vdaf=sumvec or number of + /// histogram buckets, when used with --vdaf=histogram #[clap(long, help_heading = "VDAF Algorithm and Parameters")] length: Option, /// Bit length of measurements, for use with --vdaf=sum and --vdaf=sumvec #[clap(long, help_heading = "VDAF Algorithm and Parameters")] bits: Option, - /// Comma-separated list of bucket boundaries, for use with --vdaf=histogram - #[clap( - long, - required = false, - num_args = 1, - action = ArgAction::Set, - help_heading = "VDAF Algorithm and Parameters" - )] - buckets: Option, #[clap(flatten)] query: QueryOptions, @@ -412,40 +404,40 @@ where options.hpke_private_key.clone(), ); let http_client = default_http_client().map_err(|err| Error::Anyhow(err.into()))?; - match (options.vdaf, options.length, options.bits, options.buckets) { - (VdafType::Count, None, None, None) => { + match (options.vdaf, options.length, options.bits) { + (VdafType::Count, None, None) => { let vdaf = Prio3::new_count(2).map_err(|err| Error::Anyhow(err.into()))?; run_collection_generic(parameters, vdaf, http_client, query, &()) .await .map_err(|err| Error::Anyhow(err.into())) } - (VdafType::CountVec, Some(length), None, None) => { + (VdafType::CountVec, Some(length), None) => { let vdaf = Prio3::new_sum_vec(2, 1, length).map_err(|err| Error::Anyhow(err.into()))?; run_collection_generic(parameters, vdaf, http_client, query, &()) .await .map_err(|err| Error::Anyhow(err.into())) } - (VdafType::Sum, None, Some(bits), None) => { + (VdafType::Sum, None, Some(bits)) => { let vdaf = Prio3::new_sum(2, bits).map_err(|err| Error::Anyhow(err.into()))?; run_collection_generic(parameters, vdaf, http_client, query, &()) .await .map_err(|err| Error::Anyhow(err.into())) } - (VdafType::SumVec, Some(length), Some(bits), None) => { + (VdafType::SumVec, Some(length), Some(bits)) => { let vdaf = Prio3::new_sum_vec(2, bits, length).map_err(|err| Error::Anyhow(err.into()))?; run_collection_generic(parameters, vdaf, http_client, query, &()) .await .map_err(|err| Error::Anyhow(err.into())) } - (VdafType::Histogram, None, None, Some(buckets)) => { - let vdaf = Prio3::new_histogram(2, buckets).map_err(|err| Error::Anyhow(err.into()))?; + (VdafType::Histogram, Some(length), None) => { + let vdaf = Prio3::new_histogram(2, length).map_err(|err| Error::Anyhow(err.into()))?; run_collection_generic(parameters, vdaf, http_client, query, &()) .await .map_err(|err| Error::Anyhow(err.into())) } #[cfg(feature = "fpvec_bounded_l2")] - (VdafType::FixedPoint16BitBoundedL2VecSum, Some(length), None, None) => { + (VdafType::FixedPoint16BitBoundedL2VecSum, Some(length), None) => { let vdaf: Prio3FixedPointBoundedL2VecSumMultithreaded> = Prio3::new_fixedpoint_boundedl2_vec_sum_multithreaded(2, length) .map_err(|err| Error::Anyhow(err.into()))?; @@ -454,7 +446,7 @@ where .map_err(|err| Error::Anyhow(err.into())) } #[cfg(feature = "fpvec_bounded_l2")] - (VdafType::FixedPoint32BitBoundedL2VecSum, Some(length), None, None) => { + (VdafType::FixedPoint32BitBoundedL2VecSum, Some(length), None) => { let vdaf: Prio3FixedPointBoundedL2VecSumMultithreaded> = Prio3::new_fixedpoint_boundedl2_vec_sum_multithreaded(2, length) .map_err(|err| Error::Anyhow(err.into()))?; @@ -463,7 +455,7 @@ where .map_err(|err| Error::Anyhow(err.into())) } #[cfg(feature = "fpvec_bounded_l2")] - (VdafType::FixedPoint64BitBoundedL2VecSum, Some(length), None, None) => { + (VdafType::FixedPoint64BitBoundedL2VecSum, Some(length), None) => { let vdaf: Prio3FixedPointBoundedL2VecSumMultithreaded> = Prio3::new_fixedpoint_boundedl2_vec_sum_multithreaded(2, length) .map_err(|err| Error::Anyhow(err.into()))?; @@ -597,7 +589,6 @@ mod tests { vdaf: VdafType::Count, length: None, bits: None, - buckets: None, query: QueryOptions { batch_interval_start: Some(1_000_000), batch_interval_duration: Some(1_000), @@ -682,30 +673,6 @@ mod tests { "1000".to_string(), ]); - let mut bad_arguments = base_arguments.clone(); - bad_arguments.extend(["--vdaf=count".to_string(), "--buckets=4".to_string()]); - let bad_options = Options::try_parse_from(bad_arguments).unwrap(); - assert_matches!( - run(bad_options).await.unwrap_err(), - Error::Clap(err) => assert_eq!(err.kind(), ErrorKind::ArgumentConflict) - ); - - let mut bad_arguments = base_arguments.clone(); - bad_arguments.extend(["--vdaf=sum".to_string(), "--buckets=4".to_string()]); - let bad_options = Options::try_parse_from(bad_arguments).unwrap(); - assert_matches!( - run(bad_options).await.unwrap_err(), - Error::Clap(err) => assert_eq!(err.kind(), ErrorKind::ArgumentConflict) - ); - - let mut bad_arguments = base_arguments.clone(); - bad_arguments.extend(["--vdaf=countvec".to_string(), "--buckets=4".to_string()]); - let bad_options = Options::try_parse_from(bad_arguments).unwrap(); - assert_matches!( - run(bad_options).await.unwrap_err(), - Error::Clap(err) => assert_eq!(err.kind(), ErrorKind::ArgumentConflict) - ); - let mut bad_arguments = base_arguments.clone(); bad_arguments.extend(["--vdaf=countvec".to_string(), "--bits=3".to_string()]); let bad_options = Options::try_parse_from(bad_arguments).unwrap(); @@ -751,10 +718,7 @@ mod tests { } let mut bad_arguments = base_arguments.clone(); - bad_arguments.extend([ - "--vdaf=histogram".to_string(), - "--buckets=apple".to_string(), - ]); + bad_arguments.extend(["--vdaf=histogram".to_string(), "--length=apple".to_string()]); assert_eq!( Options::try_parse_from(bad_arguments).unwrap_err().kind(), ErrorKind::ValueValidation @@ -793,7 +757,7 @@ mod tests { Options::try_parse_from(good_arguments).unwrap(); let mut good_arguments = base_arguments.clone(); - good_arguments.extend(["--vdaf=histogram".to_string(), "--buckets=4".to_string()]); + good_arguments.extend(["--vdaf=histogram".to_string(), "--length=4".to_string()]); Options::try_parse_from(good_arguments).unwrap(); #[cfg(feature = "fpvec_bounded_l2")] @@ -846,7 +810,6 @@ mod tests { vdaf: VdafType::Count, length: None, bits: None, - buckets: None, query: QueryOptions { batch_interval_start: None, batch_interval_duration: None, @@ -886,7 +849,6 @@ mod tests { vdaf: VdafType::Count, length: None, bits: None, - buckets: None, query: QueryOptions { batch_interval_start: None, batch_interval_duration: None, diff --git a/tools/tests/cmd/collect_fpvec_bounded_l2.trycmd b/tools/tests/cmd/collect_fpvec_bounded_l2.trycmd index 2f388204d..243c53644 100644 --- a/tools/tests/cmd/collect_fpvec_bounded_l2.trycmd +++ b/tools/tests/cmd/collect_fpvec_bounded_l2.trycmd @@ -33,7 +33,7 @@ Authorization: [env: DAP_AUTH_TOKEN=] --authorization-bearer-token - Authentication token for the "Authorization: Bearer ..." HTTP header, in base64 + Authentication token for the "Authorization: Bearer ..." HTTP header [env: AUTHORIZATION_BEARER_TOKEN=] @@ -52,14 +52,11 @@ VDAF Algorithm and Parameters: - fixedpoint64bitboundedl2vecsum: Prio3FixedPoint64BitBoundedL2VecSum --length - Number of vector elements, for use with --vdaf=countvec and --vdaf=sumvec + Number of vector elements, when used with --vdaf=countvec and --vdaf=sumvec or number of histogram buckets, when used with --vdaf=histogram --bits Bit length of measurements, for use with --vdaf=sum and --vdaf=sumvec - --buckets - Comma-separated list of bucket boundaries, for use with --vdaf=histogram - Collect Request Parameters (Time Interval): --batch-interval-start Start of the collection batch interval, as the number of seconds since the Unix epoch