Skip to content

Commit

Permalink
Drop support for older DAP protocol versions (#1179)
Browse files Browse the repository at this point in the history
  • Loading branch information
@divergentdave
divergentdave authored Jul 18, 2024
1 parent de2897f commit f38b5e6
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 176 deletions.
8 changes: 0 additions & 8 deletions client/src/protocol.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,13 @@ 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,
}

impl AsRef<str> for Protocol {
fn as_ref(&self) -> &str {
match self {
Self::Dap04 => "DAP-04",
Self::Dap07 => "DAP-07",
Self::Dap09 => "DAP-09",
}
}
Expand All @@ -37,8 +31,6 @@ impl FromStr for Protocol {
type Err = UnrecognizedProtocol;
fn from_str(s: &str) -> Result<Self, Self::Err> {
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())),
}
Expand Down
1 change: 0 additions & 1 deletion src/clients/aggregator_client/api_types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
25 changes: 3 additions & 22 deletions src/entity/aggregator/protocol.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Protocol> for Standard {
fn sample<R: rand::Rng + ?Sized>(&self, rng: &mut R) -> Protocol {
match rng.gen_range(0..=2) {
0 => Protocol::Dap04,
1 => Protocol::Dap07,
_ => Protocol::Dap09,
}
fn sample<R: rand::Rng + ?Sized>(&self, _rng: &mut R) -> Protocol {
Protocol::Dap09
}
}

Expand All @@ -45,8 +30,6 @@ impl Display for Protocol {
impl AsRef<str> for Protocol {
fn as_ref(&self) -> &str {
match self {
Self::Dap04 => "DAP-04",
Self::Dap07 => "DAP-07",
Self::Dap09 => "DAP-09",
}
}
Expand All @@ -68,8 +51,6 @@ impl FromStr for Protocol {

fn from_str(s: &str) -> Result<Self, Self::Err> {
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())),
}
Expand Down
161 changes: 49 additions & 112 deletions src/entity/task/vdaf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,54 +41,15 @@ impl Histogram {

fn representation_for_protocol(
&self,
protocol: &Protocol,
_protocol: &Protocol,
) -> Result<AggregatorVdaf, ValidationErrors> {
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");
}
}
}
Expand Down Expand Up @@ -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, .. })
Expand All @@ -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 => {}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test-support/src/fixtures.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
39 changes: 7 additions & 32 deletions tests/integration/vdaf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}})),
),
];
Expand All @@ -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);
}

0 comments on commit f38b5e6

Please sign in to comment.