From f38b5e642d4d39e69542b0f3ccfd2a50abd22565 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 18 Jul 2024 11:03:18 -0500 Subject: [PATCH] Drop support for older DAP protocol versions (#1179) --- client/src/protocol.rs | 8 - src/clients/aggregator_client/api_types.rs | 1 - src/entity/aggregator/protocol.rs | 25 +--- src/entity/task/vdaf.rs | 161 +++++++-------------- test-support/src/fixtures.rs | 2 +- tests/integration/vdaf.rs | 39 +---- 6 files changed, 60 insertions(+), 176 deletions(-) diff --git a/client/src/protocol.rs b/client/src/protocol.rs index ab3aa35b..59d8a883 100644 --- a/client/src/protocol.rs +++ b/client/src/protocol.rs @@ -7,10 +7,6 @@ use std::{ #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum Protocol { - #[serde(rename = "DAP-04")] - Dap04, - #[serde(rename = "DAP-07")] - Dap07, #[serde(rename = "DAP-09")] Dap09, } @@ -18,8 +14,6 @@ pub enum Protocol { impl AsRef for Protocol { fn as_ref(&self) -> &str { match self { - Self::Dap04 => "DAP-04", - Self::Dap07 => "DAP-07", Self::Dap09 => "DAP-09", } } @@ -37,8 +31,6 @@ impl FromStr for Protocol { type Err = UnrecognizedProtocol; fn from_str(s: &str) -> Result { match &*s.to_lowercase() { - "dap-04" => Ok(Self::Dap04), - "dap-07" => Ok(Self::Dap07), "dap-09" => Ok(Self::Dap09), unrecognized => Err(UnrecognizedProtocol(unrecognized.to_string())), } diff --git a/src/clients/aggregator_client/api_types.rs b/src/clients/aggregator_client/api_types.rs index 5354bfc9..c842e53d 100644 --- a/src/clients/aggregator_client/api_types.rs +++ b/src/clients/aggregator_client/api_types.rs @@ -369,7 +369,6 @@ pub struct AggregatorApiConfig { pub role: AggregatorRole, pub vdafs: VdafNameSet, pub query_types: QueryTypeNameSet, - #[serde(default)] pub protocol: Protocol, #[serde(default)] pub features: Features, diff --git a/src/entity/aggregator/protocol.rs b/src/entity/aggregator/protocol.rs index 2969c24e..8cfb422c 100644 --- a/src/entity/aggregator/protocol.rs +++ b/src/entity/aggregator/protocol.rs @@ -7,32 +7,17 @@ use std::{ str::FromStr, }; -#[derive( - Debug, Clone, Copy, PartialEq, Eq, EnumIter, DeriveActiveEnum, Serialize, Deserialize, Default, -)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, EnumIter, DeriveActiveEnum, Serialize, Deserialize)] #[sea_orm(rs_type = "String", db_type = "String(None)")] pub enum Protocol { - #[sea_orm(string_value = "DAP-04")] - #[serde(rename = "DAP-04")] - #[default] - Dap04, - - #[sea_orm(string_value = "DAP-07")] - #[serde(rename = "DAP-07")] - Dap07, - #[sea_orm(string_value = "DAP-09")] #[serde(rename = "DAP-09")] Dap09, } impl Distribution for Standard { - fn sample(&self, rng: &mut R) -> Protocol { - match rng.gen_range(0..=2) { - 0 => Protocol::Dap04, - 1 => Protocol::Dap07, - _ => Protocol::Dap09, - } + fn sample(&self, _rng: &mut R) -> Protocol { + Protocol::Dap09 } } @@ -45,8 +30,6 @@ impl Display for Protocol { impl AsRef for Protocol { fn as_ref(&self) -> &str { match self { - Self::Dap04 => "DAP-04", - Self::Dap07 => "DAP-07", Self::Dap09 => "DAP-09", } } @@ -68,8 +51,6 @@ impl FromStr for Protocol { fn from_str(s: &str) -> Result { match &*s.to_lowercase() { - "dap-04" => Ok(Self::Dap04), - "dap-07" => Ok(Self::Dap07), "dap-09" => Ok(Self::Dap09), unrecognized => Err(UnrecognizedProtocol(unrecognized.to_string())), } diff --git a/src/entity/task/vdaf.rs b/src/entity/task/vdaf.rs index 5112730c..8bf631d8 100644 --- a/src/entity/task/vdaf.rs +++ b/src/entity/task/vdaf.rs @@ -41,54 +41,15 @@ impl Histogram { fn representation_for_protocol( &self, - protocol: &Protocol, + _protocol: &Protocol, ) -> Result { - match (protocol, self) { - (Protocol::Dap07 | Protocol::Dap09, histogram) => { - if let Some(chunk_length) = histogram.chunk_length() { - Ok(AggregatorVdaf::Prio3Histogram(HistogramType::Opaque { - length: histogram.length(), - chunk_length: Some(chunk_length), - })) - } else { - panic!("chunk_length was not populated"); - } - } - - ( - Protocol::Dap04, - Self::Continuous(ContinuousBuckets { - buckets: Some(buckets), - chunk_length: None, - }), - ) => Ok(AggregatorVdaf::Prio3Histogram(HistogramType::Buckets { - buckets: buckets.clone(), - chunk_length: None, - })), - - ( - Protocol::Dap04, - Self::Continuous(ContinuousBuckets { - buckets: _, - chunk_length: Some(_), - }), - ) => { - let mut errors = ValidationErrors::new(); - errors.add("chunk_length", ValidationError::new("not-allowed")); - Err(errors) - } - - (Protocol::Dap04, Self::Categorical(_)) => { - let mut errors = ValidationErrors::new(); - errors.add("buckets", ValidationError::new("must-be-numbers")); - Err(errors) - } - - (Protocol::Dap04, _) => { - let mut errors = ValidationErrors::new(); - errors.add("buckets", ValidationError::new("required")); - Err(errors) - } + if let Some(chunk_length) = self.chunk_length() { + Ok(AggregatorVdaf::Prio3Histogram(HistogramType::Opaque { + length: self.length(), + chunk_length: Some(chunk_length), + })) + } else { + panic!("chunk_length was not populated"); } } } @@ -237,47 +198,32 @@ impl Vdaf { } } - pub fn populate_chunk_length(&mut self, protocol: &Protocol) { - match (self, protocol) { + pub fn populate_chunk_length(&mut self, _protocol: &Protocol) { + match self { // Chunk length was already populated, don't change it. - ( - Self::Histogram(Histogram::Continuous(ContinuousBuckets { - chunk_length: Some(_), - .. - })), - _, - ) - | ( - Self::Histogram(Histogram::Opaque(BucketLength { - chunk_length: Some(_), - .. - })), - _, - ) - | ( - Self::Histogram(Histogram::Categorical(CategoricalBuckets { - chunk_length: Some(_), - .. - })), - _, - ) - | ( - Self::CountVec(CountVec { - chunk_length: Some(_), - .. - }), - _, - ) - | ( - Self::SumVec(SumVec { - chunk_length: Some(_), - .. - }), - _, - ) => {} - - // Select a chunk length if the protocol version needs it and it isn't set yet. - (Self::Histogram(histogram), Protocol::Dap07 | Protocol::Dap09) => { + Self::Histogram(Histogram::Continuous(ContinuousBuckets { + chunk_length: Some(_), + .. + })) + | Self::Histogram(Histogram::Opaque(BucketLength { + chunk_length: Some(_), + .. + })) + | Self::Histogram(Histogram::Categorical(CategoricalBuckets { + chunk_length: Some(_), + .. + })) + | Self::CountVec(CountVec { + chunk_length: Some(_), + .. + }) + | Self::SumVec(SumVec { + chunk_length: Some(_), + .. + }) => {} + + // Select a chunk length if it isn't set yet. + Self::Histogram(histogram) => { let length = histogram.length(); match histogram { Histogram::Opaque(BucketLength { chunk_length, .. }) @@ -288,35 +234,26 @@ impl Vdaf { } } - ( - Self::CountVec(CountVec { - length: Some(length), - chunk_length: chunk_length @ None, - }), - Protocol::Dap07 | Protocol::Dap09, - ) => *chunk_length = Some(optimal_chunk_length(*length as usize) as u64), - - ( - Self::SumVec(SumVec { - bits: Some(bits), - length: Some(length), - chunk_length: chunk_length @ None, - }), - Protocol::Dap07 | Protocol::Dap09, - ) => { + Self::CountVec(CountVec { + length: Some(length), + chunk_length: chunk_length @ None, + }) => *chunk_length = Some(optimal_chunk_length(*length as usize) as u64), + + Self::SumVec(SumVec { + bits: Some(bits), + length: Some(length), + chunk_length: chunk_length @ None, + }) => { *chunk_length = Some(optimal_chunk_length(*bits as usize * *length as usize) as u64) } // Invalid, missing parameters, do nothing. - (Self::CountVec(CountVec { length: None, .. }), Protocol::Dap07 | Protocol::Dap09) - | (Self::SumVec(SumVec { bits: None, .. }), Protocol::Dap07 | Protocol::Dap09) - | (Self::SumVec(SumVec { length: None, .. }), Protocol::Dap07 | Protocol::Dap09) => {} - - // Chunk length is not applicable, either due to VDAF choice or protocol version. - (Self::Count, _) - | (Self::Sum { .. }, _) - | (Self::Unrecognized, _) - | (_, Protocol::Dap04) => {} + Self::CountVec(CountVec { length: None, .. }) + | Self::SumVec(SumVec { bits: None, .. }) + | Self::SumVec(SumVec { length: None, .. }) => {} + + // Chunk length is not applicable due to VDAF choice. + Self::Count | Self::Sum { .. } | Self::Unrecognized => {} } } } diff --git a/test-support/src/fixtures.rs b/test-support/src/fixtures.rs index 222f428a..85fb9efb 100644 --- a/test-support/src/fixtures.rs +++ b/test-support/src/fixtures.rs @@ -179,7 +179,7 @@ pub async fn aggregator(app: &DivviupApi, account: Option<&Account>) -> Aggregat role: Role::Either, query_types: Default::default(), vdafs: Default::default(), - protocol: Protocol::Dap07, + protocol: Protocol::Dap09, features: Features::from(Feature::TokenHash).into(), } .into_active_model() diff --git a/tests/integration/vdaf.rs b/tests/integration/vdaf.rs index e33e3c28..a9ab83bd 100644 --- a/tests/integration/vdaf.rs +++ b/tests/integration/vdaf.rs @@ -3,44 +3,19 @@ use test_support::{assert_eq, test, *}; #[test] pub fn histogram_representations() { let scenarios = [ - ( - json!({"type": "histogram", "buckets": ["a", "b", "c"]}), - Protocol::Dap04, - Err(json!({"buckets": [{"code": "must-be-numbers", "message": null, "params": {}}]})), - ), - ( - json!({"type": "histogram", "buckets": ["a", "b", "c"], "chunk_length": 1}), - Protocol::Dap04, - Err(json!({"buckets": [{"code": "must-be-numbers", "message": null, "params": {}}]})), - ), ( json!({"type": "histogram", "buckets": ["a", "b", "c"], "chunk_length": 1}), - Protocol::Dap07, + Protocol::Dap09, Ok(json!({"Prio3Histogram": {"length": 3, "chunk_length": 1}})), ), - ( - json!({"type": "histogram", "buckets": [1, 2, 3]}), - Protocol::Dap04, - Ok(json!({"Prio3Histogram": {"buckets": [1, 2, 3], "chunk_length": null}})), - ), - ( - json!({"type": "histogram", "buckets": [1, 2, 3], "chunk_length": 2}), - Protocol::Dap04, - Err(json!({"chunk_length": [{"code": "not-allowed", "message": null, "params": {}}]})), - ), ( json!({"type": "histogram", "buckets": [1, 2, 3], "chunk_length": 2}), - Protocol::Dap07, + Protocol::Dap09, Ok(json!({"Prio3Histogram": {"length": 4, "chunk_length": 2}})), ), - ( - json!({"type": "histogram", "length": 3}), - Protocol::Dap04, - Err(json!({"buckets": [{"code": "required", "message": null, "params":{}}]})), - ), ( json!({"type": "histogram", "length": 3, "chunk_length": 1}), - Protocol::Dap07, + Protocol::Dap09, Ok(json!({"Prio3Histogram": {"length": 3, "chunk_length": 1}})), ), ]; @@ -59,20 +34,20 @@ pub fn histogram_representations() { #[test] #[should_panic] -fn histogram_representation_dap_07_no_chunk_length_1() { +fn histogram_representation_dap_09_no_chunk_length_1() { let _ = Vdaf::Histogram(Histogram::Categorical(CategoricalBuckets { buckets: Some(Vec::from(["a".to_owned(), "b".to_owned(), "c".to_owned()])), chunk_length: None, })) - .representation_for_protocol(&Protocol::Dap07); + .representation_for_protocol(&Protocol::Dap09); } #[test] #[should_panic] -fn histogram_representation_dap_07_no_chunk_length_2() { +fn histogram_representation_dap_09_no_chunk_length_2() { let _ = Vdaf::Histogram(Histogram::Opaque(BucketLength { length: 3, chunk_length: None, })) - .representation_for_protocol(&Protocol::Dap07); + .representation_for_protocol(&Protocol::Dap09); }