Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rust 1.78 clippy warnings #1035

Merged
merged 1 commit into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 1 addition & 65 deletions ipa-core/src/bin/ipa_bench/models.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
use std::{
fmt::{Debug, Formatter},
ops::Range,
};
use std::fmt::Debug;

use serde::{Deserialize, Serialize};

// Type aliases to indicate whether the parameter should be encrypted, secret shared, etc.
// Underlying types are temporalily assigned for PoC.
type PlainText = String;
pub type MatchKey = u64;
pub type Number = u32;

Expand Down Expand Up @@ -150,66 +146,6 @@ enum Node {
Helper3,
}

#[derive(Serialize)]
struct IPAQuery {
/// Caller authentication token.
auth_token: PlainText,

/// Initial MPC helper node to send the data to.
leader_node: Node,

/// List of match key providers to be used by the source and trigger events during an epoch.
mk_providers: Vec<String>,

/// Source-fanout or Trigger-fanout.
query_type: QueryType,

/// Percentage of epoch-level privacy budget this query should consume. Likely 1-100.
privacy_budget: u8,

/// A collection of source events. At least 100 (TBD) unique source events must be provided.
reports: Vec<GenericReport>,
}

#[derive(Serialize)]
struct SourceFanoutQuery {
query: IPAQuery,

/// The maximum number of attributed conversion events that a single person can contribute
/// towards the final output. We could also express this using sum of trigger values.
/// We'll leave it for the future spec to decide.
cap: u8,
}

#[cfg(feature = "debug")]
impl Debug for SourceFanoutQuery {
fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
write!(
f,
"SourceFanoutQuery:\n {} events",
&self.query.reports.len(),
)
}
}

#[derive(Serialize)]
struct TriggerFanoutQuery {
query: IPAQuery,

/// The range within which all the trigger event values must lie.
value_range: Range<u32>,
}

impl Debug for TriggerFanoutQuery {
fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
write!(
f,
"TriggerFanoutQuery:\n {} events",
&self.query.reports.len(),
)
}
}

#[cfg(all(test, unit_test))]
mod tests {
use super::{Epoch, EventTimestamp};
Expand Down
8 changes: 3 additions & 5 deletions ipa-core/src/bin/test_mpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,9 @@ async fn main() -> Result<(), Box<dyn Error>> {
Ok(())
}

async fn multiply_in_field<F: Field + U128Conversions>(
args: &Args,
helper_clients: &[MpcHelperClient; 3],
) where
F: Field + IntoShares<AdditiveShare<F>>,
async fn multiply_in_field<F>(args: &Args, helper_clients: &[MpcHelperClient; 3])
where
F: Field + U128Conversions + IntoShares<AdditiveShare<F>>,
<F as Serializable>::Size: Add<<F as Serializable>::Size>,
<<F as Serializable>::Size as Add<<F as Serializable>::Size>>::Output: ArrayLength,
{
Expand Down
2 changes: 1 addition & 1 deletion ipa-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ impl Debug for Http1Configurator {
#[derive(Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct Http2Configurator {
/// Enable [`PING`] frames to keep connection alive. Default value is 90 seconds to match [`Hyper`] value
/// for SO_KEEPALIVE. Note that because
/// for `SO_KEEPALIVE`. Note that because
/// IPA builds [`http`] connector manually, keep-alive is not enabled by Hyper. It is somewhat
/// confusing that Hyper turns it on inside [`build_http`] method only.
///
Expand Down
2 changes: 1 addition & 1 deletion ipa-core/src/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ pub enum TotalRecords {
Specified(NonZeroUsize),

/// Total number of records is not well-determined. When the record ID is
/// counting solved_bits attempts. The total record count for solved_bits
/// counting `solved_bits` attempts. The total record count for `solved_bits`
/// depends on the number of failures.
///
/// The purpose of this is to waive the warning that there is a known
Expand Down
2 changes: 1 addition & 1 deletion ipa-core/src/helpers/transport/in_memory/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl<I: TransportIdentity> Transport for Weak<InMemoryTransport<I>> {

/// Convenience struct to support heterogeneous in-memory streams
pub struct InMemoryStream {
/// There is only one reason for this to have dynamic dispatch: tests that use from_iter method.
/// There is only one reason for this to have dynamic dispatch: tests that use `from_iter` method.
inner: Pin<Box<dyn Stream<Item = StreamItem> + Send>>,
}

Expand Down
2 changes: 1 addition & 1 deletion ipa-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub(crate) mod task {
pub use shuttle::future::{JoinError, JoinHandle};
}

#[cfg(feature = "shuttle")]
#[cfg(all(feature = "multi-threading", feature = "shuttle"))]
pub(crate) mod shim {
use std::any::Any;

Expand Down
38 changes: 19 additions & 19 deletions ipa-core/src/net/server/handlers/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ mod results;
mod status;
mod step;

use std::any::Any;

use axum::{
response::{IntoResponse, Response},
Router,
Expand All @@ -15,7 +13,7 @@ use futures_util::{
future::{ready, Either, Ready},
FutureExt,
};
use hyper::{http::request, Request, StatusCode};
use hyper::{Request, StatusCode};
use tower::{layer::layer_fn, Service};

use crate::{
Expand Down Expand Up @@ -100,29 +98,31 @@ impl<B, S: Service<Request<B>, Response = Response>> Service<Request<B>>
}
}

/// Helper trait for optionally adding an extension to a request.
trait MaybeExtensionExt {
fn maybe_extension<T: Any + Send + Sync + 'static>(self, extension: Option<T>) -> Self;
}

impl MaybeExtensionExt for request::Builder {
fn maybe_extension<T: Any + Send + Sync + 'static>(self, extension: Option<T>) -> Self {
if let Some(extension) = extension {
self.extension(extension)
} else {
self
}
}
}

#[cfg(all(test, unit_test))]
pub mod test_helpers {
use std::any::Any;

use futures_util::future::poll_immediate;
use hyper::{service::Service, StatusCode};
use hyper::{http::request, service::Service, StatusCode};
use tower::ServiceExt;

use crate::net::test::TestServer;

/// Helper trait for optionally adding an extension to a request.
pub trait MaybeExtensionExt {
fn maybe_extension<T: Any + Send + Sync + 'static>(self, extension: Option<T>) -> Self;
}

impl MaybeExtensionExt for request::Builder {
fn maybe_extension<T: Any + Send + Sync + 'static>(self, extension: Option<T>) -> Self {
if let Some(extension) = extension {
self.extension(extension)
} else {
self
}
}
}

/// types that implement `IntoFailingReq` are intended to induce some failure in the process of
/// axum routing. Pair with `assert_req_fails_with` to detect specific [`StatusCode`] failures.
pub trait IntoFailingReq {
Expand Down
4 changes: 1 addition & 3 deletions ipa-core/src/net/server/handlers/query/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ pub fn router(transport: Arc<HttpTransport>) -> Router {

#[cfg(all(test, unit_test))]
mod tests {

use axum::{http::Request, Extension};
use hyper::{Body, StatusCode};

Expand All @@ -55,8 +54,7 @@ mod tests {
server::{
handlers::query::{
prepare::handler,
test_helpers::{assert_req_fails_with, IntoFailingReq},
MaybeExtensionExt,
test_helpers::{assert_req_fails_with, IntoFailingReq, MaybeExtensionExt},
},
ClientIdentity,
},
Expand Down
5 changes: 2 additions & 3 deletions ipa-core/src/net/server/handlers/query/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ mod tests {
use crate::{
helpers::{HelperIdentity, MESSAGE_PAYLOAD_SIZE_BYTES},
net::{
server::handlers::query::{
test_helpers::{assert_req_fails_with, IntoFailingReq},
MaybeExtensionExt,
server::handlers::query::test_helpers::{
assert_req_fails_with, IntoFailingReq, MaybeExtensionExt,
},
test::TestServer,
},
Expand Down
3 changes: 3 additions & 0 deletions ipa-core/src/protocol/basics/mul/sparse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ pub(super) trait MultiplyWork {
/// Determine the work that is required for the identified role.
/// Return value is who is sending relative to the given role [self, left, right].
fn work_for(self, role: Role) -> [bool; 3];

#[cfg(all(test, unit_test))]
/// Determines where there are known zeros in the output of a multiplication.
fn output(self) -> ZeroPositions;
}
Expand All @@ -186,6 +188,7 @@ impl MultiplyWork for MultiplyZeroPositions {
[need_to_recv, need_to_send, need_random_right]
}

#[cfg(all(test, unit_test))]
fn output(self) -> ZeroPositions {
ZeroPositions::mul_output(self)
}
Expand Down
6 changes: 3 additions & 3 deletions ipa-core/src/protocol/ipa_prf/malicious_security/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ where
Self { u, v }
}

pub fn compute_proof<λ: ArrayLength, I, J>(
pub fn compute_proof<λ, I, J>(
u: I,
v: J,
lagrange_table: &LagrangeTable<F, λ, <λ as Sub<U1>>::Output>,
Expand Down Expand Up @@ -106,7 +106,7 @@ where
ZeroKnowledgeProof::new(proof)
}

pub fn compute_final_proof<λ: ArrayLength, I, J>(
pub fn compute_final_proof<λ, I, J>(
u: I,
v: J,
p_0: F,
Expand Down Expand Up @@ -154,7 +154,7 @@ where
ZeroKnowledgeProof::new(proof)
}

pub fn gen_challenge_and_recurse<λ: ArrayLength, I, J>(
pub fn gen_challenge_and_recurse<λ, I, J>(
proof_left: &GenericArray<F, TwoNMinusOne<λ>>,
proof_right: &GenericArray<F, TwoNMinusOne<λ>>,
u: I,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ where
Self { u_or_v, out_share }
}

pub fn verify_proof<λ: ArrayLength>(
pub fn verify_proof<λ>(
&self,
zkp: &ZeroKnowledgeProof<F, TwoNMinusOne<λ>>,
r: F,
Expand Down
2 changes: 1 addition & 1 deletion ipa-core/src/protocol/ipa_prf/shuffle/sharded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ enum ShuffleStep {
Cardinality,
/// Send all the shares from helper on the left to the helper on the right.
LeftToRight,
/// H2 and H3 interaction - Exchange C_1 and C_2.
/// H2 and H3 interaction - Exchange `C_1` and `C_2`.
C,
/// Apply a mask to the given set of shares. Masking values come from PRSS.
Mask,
Expand Down
2 changes: 1 addition & 1 deletion ipa-core/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub struct RunningQuery {
/// The join handle is only useful for the purpose of aborting the query. Tasks started with
/// `tokio::spawn` run to completion whether or not anything waits on the handle.
///
/// We could return the result via the JoinHandle, except that we want to check the status
/// We could return the result via the `JoinHandle`, except that we want to check the status
/// of the task, and shuttle doesn't implement `JoinHandle::is_finished`.
pub join_handle: JoinHandle<()>,
}
Expand Down
2 changes: 1 addition & 1 deletion ipa-core/src/seq_join/multi_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ where
let mut result = Vec::with_capacity(scope.len());
while let Some(item) = scope.next().await {
// join error is nothing we can do about
result.push(item.expect("parallel_join: received JoinError")?)
result.push(item.expect("parallel_join: received JoinError")?);
}
Ok(result)
}
Expand Down
2 changes: 1 addition & 1 deletion pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ check "Web tests" \
cargo test -p ipa-core --no-default-features --features "cli web-app real-world-infra test-fixture descriptive-gate"

check "Concurrency tests" \
cargo test -p ipa-core --release --features shuttle
cargo test -p ipa-core --release --features "shuttle multi-threading"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this is an accidental change or no - I don't have a preference which test to run as long as we run both on CI

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intentional, trying to make pre-commit closer to what CI is running.


check "IPA benchmark" \
cargo bench --bench oneshot_ipa --no-default-features --features="enable-benches descriptive-gate" -- -n 62 -c 16
Expand Down