Upgrade to Rust 1.93

ipa-core/benches/ct/dzkp.rs

@@ -66,7 +66,7 @@ fn benchmark_proof(c: &mut Criterion) {
                 (a, b)
             },
             |(a, b): (Vec<BA>, Vec<BA>)| async move {
-                TestWorld::default()
+                let _ = TestWorld::default()
                     .malicious((a.into_iter(), b.into_iter()), |ctx, (a, b)| async move {
                         let batch_size = non_zero_prev_power_of_two(
                             TARGET_PROOF_SIZE / usize::try_from(BA::BITS).unwrap(),

ipa-core/src/bin/helper.rs

@@ -291,7 +291,7 @@ async fn server(args: ServerArgs, logging_handle: LoggingHandle) -> Result<(), B
 
     join(server_handle, shard_server_handle).await;
 
-    [query_runtime, http_runtime].map(Runtime::shutdown_background);
+    let _ = [query_runtime, http_runtime].map(Runtime::shutdown_background);
 
     Ok(())
 }

ipa-core/src/bin/ipa_bench/models.rs

@@ -133,19 +133,6 @@ pub enum GenericReport {
     },
 }
 
-#[derive(Serialize, Deserialize)]
-enum QueryType {
-    SourceFanout,
-    TriggerFanout,
-}
-
-#[derive(Serialize, Deserialize)]
-enum Node {
-    Helper1,
-    Helper2,
-    Helper3,
-}
-
 #[cfg(all(test, unit_test))]
 mod tests {
     use super::{Epoch, EventTimestamp};

ipa-core/src/cli/clientconf.rs

@@ -63,7 +63,7 @@ pub fn setup(args: ConfGenArgs) -> Result<(), BoxError> {
         .map(|(id, (host, (port, shard_port)))| {
             let id: u8 = u8::try_from(id).unwrap() + 1;
             HelperClientConf {
-                host: host.to_string(),
+                host: host.clone(),
                 port,
                 shard_port,
                 tls_cert_file: args.keys_dir.helper_tls_cert(id),

ipa-core/src/cli/config_parse.rs

@@ -361,8 +361,8 @@ pub fn sharded_server_from_toml_str(
         let url = myself.url.to_string();
         let pos = url.rfind(':');
         let port = shard_port.expect("Shard port should be set");
-        let new_url = if pos.is_some() {
-            format!("{}{port}", &url[..=pos.unwrap()])
+        let new_url = if let Some(pos) = pos {
+            format!("{}{port}", &url[..=pos])
         } else {
             format!("{}:{port}", &url)
         };
@@ -374,7 +374,7 @@ pub fn sharded_server_from_toml_str(
         };
         Ok((mpc_network, shard_network))
     } else {
-        return Err(Error::MissingShardUrls(missing_urls));
+        Err(Error::MissingShardUrls(missing_urls))
     }
 }
 

ipa-core/src/cli/keygen.rs

@@ -103,11 +103,11 @@ pub fn keygen_tls<R: Rng + CryptoRng>(args: &KeygenArgs, rng: &mut R) -> Result<
 fn keygen_matchkey<R: Rng + CryptoRng>(args: &KeygenArgs, mut rng: &mut R) -> Result<(), BoxError> {
     let keypair = crate::hpke::KeyPair::r#gen(&mut rng);
 
-    if args.mk_public_key.is_some() && args.mk_private_key.is_some() {
-        create_new(args.mk_public_key.as_ref().unwrap())?
-            .write_all(hex::encode(keypair.pk_bytes()).as_bytes())?;
-        create_new(args.mk_private_key.as_ref().unwrap())?
-            .write_all(hex::encode(keypair.sk_bytes()).as_bytes())?;
+    if let (Some(mk_public_key), Some(mk_private_key)) =
+        (args.mk_public_key.as_ref(), args.mk_private_key.as_ref())
+    {
+        create_new(mk_public_key)?.write_all(hex::encode(keypair.pk_bytes()).as_bytes())?;
+        create_new(mk_private_key)?.write_all(hex::encode(keypair.sk_bytes()).as_bytes())?;
     }
 
     Ok(())

ipa-core/src/cli/metric_collector.rs

@@ -8,9 +8,9 @@ use tokio::runtime::Builder;
 
 /// Holds a reference to metrics controller and producer
 pub struct CollectorHandle {
-    thread_handle: JoinHandle<()>,
+    thread_handle: Option<JoinHandle<()>>,
     /// This will be used once we start consuming metrics
-    controller: MetricsCollectorController,
+    controller: Option<MetricsCollectorController>,
     producer: MetricsProducer,
 }
 
@@ -26,16 +26,23 @@ pub fn install_collector() -> io::Result<CollectorHandle> {
     tracing::info!("Metrics engine is enabled");
 
     Ok(CollectorHandle {
-        thread_handle: handle,
-        controller,
+        thread_handle: Some(handle),
+        controller: Some(controller),
         producer,
     })
 }
 
 impl Drop for CollectorHandle {
     fn drop(&mut self) {
-        if !thread::panicking() && !self.thread_handle.is_finished() {
-            tracing::warn!("Metrics thread is still running");
+        if thread::panicking() {
+            return; // avoid potential deadlock during panic unwind
+        }
+        // Drop controller first to disconnect the command channel.
+        // This causes the collector thread's event_loop to exit.
+        drop(self.controller.take());
+        // Wait for the collector thread to finish.
+        if let Some(handle) = self.thread_handle.take() {
+            let _ = handle.join();
         }
     }
 }
@@ -61,7 +68,12 @@ impl CollectorHandle {
     /// If metrics is not initialized
     #[must_use]
     pub fn scrape_metrics(&self) -> Vec<u8> {
-        let mut store = self.controller.snapshot().expect("Metrics must be set up");
+        let mut store = self
+            .controller
+            .as_ref()
+            .expect("Metrics must be set up")
+            .snapshot()
+            .expect("Metrics snapshot failed");
         let mut buff = Vec::new();
         store.export(&mut buff);
 

ipa-core/src/cli/playbook/mod.rs

@@ -222,7 +222,7 @@ pub async fn make_clients(
     // Note: This closure is only called when the selected action uses clients.
 
     let clients = IpaHttpClient::from_conf(&IpaRuntime::current(), &network, &ClientIdentity::None);
-    wait_for_servers(wait, &[clients.clone()]).await;
+    wait_for_servers(wait, std::slice::from_ref(&clients)).await;
     (clients, network)
 }
 

ipa-core/src/cli/test_setup.rs

@@ -174,7 +174,7 @@ fn make_client_configs(
             keygen(&keygen_args)?;
 
             Ok(HelperClientConf {
-                host: localhost.to_string(),
+                host: localhost.clone(),
                 port: mpc_port,
                 shard_port,
                 tls_cert_file: keygen_args.tls_cert,

ipa-core/src/error.rs

@@ -22,13 +22,14 @@ use crate::{
 ///  * `ipa::ff::Error`, for finite field routines
 ///  * `ipa::net::Error`, for the HTTP transport
 ///  * `ipa::app::Error`, for the report collector query APIs
-#[derive(Error, Debug)]
+#[derive(Error, Debug, Default)]
 pub enum Error {
     #[error("already exists")]
     AlreadyExists,
     #[error("already setup")]
     AlreadySetup,
     #[error("internal")]
+    #[default]
     Internal,
     #[error("invalid id found: {0}")]
     InvalidId(String),
@@ -106,12 +107,6 @@ pub enum Error {
     DuplicateBytes(usize),
 }
 
-impl Default for Error {
-    fn default() -> Self {
-        Self::Internal
-    }
-}
-
 impl Error {
     #[must_use]
     pub fn path_parse_error(source: &str) -> Error {

ipa-core/src/helpers/buffers/ordering_sender.rs

@@ -9,7 +9,7 @@ use std::{
     task::{Context, Poll},
 };
 
-use futures::{Future, Stream, task::Waker};
+use futures::{Future, task::Waker};
 
 use crate::{
     helpers::{Message, buffers::circular::CircularBuf},
@@ -493,11 +493,13 @@ impl Future for Close<'_> {
 /// the next stream that happens to be polled.  Ordinarily streams require a
 /// mutable reference so that they have exclusive access to the underlying state.
 /// To avoid that happening, don't make more than one stream.
+#[cfg(all(test, any(unit_test, feature = "shuttle")))]
 pub struct OrderedStream<B: Borrow<OrderingSender>> {
     sender: B,
 }
 
-impl<B: Borrow<OrderingSender> + Unpin> Stream for OrderedStream<B> {
+#[cfg(all(test, any(unit_test, feature = "shuttle")))]
+impl<B: Borrow<OrderingSender> + Unpin> futures::Stream for OrderedStream<B> {
     type Item = Vec<u8>;
 
     fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {

ipa-core/src/helpers/stream/chunks.rs

@@ -277,7 +277,7 @@ where
                 ChunkType::Partial(remainder_len),
             ))
         } else {
-            return None;
+            None
         }
     }
 }

ipa-core/src/helpers/transport/handler.rs

@@ -177,7 +177,7 @@ pub enum Error {
 }
 
 /// Trait for custom-handling different request types made against MPC helper parties.
-/// There is a limitation for RPITIT that traits can't be made object-safe, hence the use of async_trait
+/// There is a limitation for RPITIT that traits can't be made object-safe, hence the use of `async_trait`
 #[async_trait]
 pub trait RequestHandler<I: TransportIdentity>: Send + Sync {
     /// Handle the incoming request with metadata/headers specified in [`Addr`] and body encoded as

ipa-core/src/helpers/transport/receive.rs

@@ -1,5 +1,5 @@
 use std::{
-    pin::{Pin, pin},
+    pin::Pin,
     task::{Context, Poll},
 };
 

ipa-core/src/net/http_serde.rs

@@ -69,12 +69,6 @@ pub mod echo {
 }
 
 pub mod metrics {
-
-    use serde::{Deserialize, Serialize};
-
-    #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)]
-    pub struct Request {}
-
     pub const AXUM_PATH: &str = "/metrics";
 }
 

ipa-core/src/net/server/mod.rs

@@ -670,7 +670,7 @@ mod e2e_tests {
     }
 
     /// Ensures that server tracks number of requests it received and emits a corresponding metric.
-    /// In order for this test not to be flaky, we rely on tokio::test macro to set up a
+    /// In order for this test not to be flaky, we rely on `tokio::test` macro to set up a
     /// new runtime per test (which it currently does) and set up metric recorders per thread (done
     /// by this test). It is also tricky to make it work in a multi-threaded environment - I haven't
     /// tested that, so better to stick with default behavior of tokio:test macro

ipa-core/src/protocol/basics/reshare.rs

@@ -29,8 +29,8 @@ use crate::{
 /// 1. While calculating for a helper, we call pseudo random secret sharing (prss) to get random values which match
 ///    with those generated by other helpers (say `rand_left`, `rand_right`)
 ///    `to_helper.left` knows `rand_left` (named r1) and `to_helper.right` knows `rand_right` (named r0)
-/// 2. `to_helper.left` calculates part1 = (a1 + a2) - r2 = Same as (input.left() + input.right()) - r1 from helper POV
-///    `to_helper.right` calculates part2 = (a3 - r3) = Same as (input.left() - r0) from helper POV
+/// 2. `to_helper.left` calculates part1 = (a1 + a2) - r2 = Same as (`input.left()` + `input.right()`) - r1 from helper POV
+///    `to_helper.right` calculates part2 = (a3 - r3) = Same as (`input.left()` - r0) from helper POV
 /// 3. `to_helper.left` and `to_helper.right` exchange their calculated shares
 /// 4. Everyone sets their shares
 ///    `to_helper.left`  = (part1 + part2, `rand_left`)  = (part1 + part2, r1)

ipa-core/src/protocol/boolean/step.rs

@@ -19,4 +19,5 @@ pub struct TwoHundredFiftySixBitOpStep(usize);
 #[cfg(test)]
 #[derive(CompactStep)]
 #[step(count = 256, name = "bit")]
+#[allow(dead_code)] // used as a type parameter, not constructed directly
 pub struct DefaultBitStep(usize);

ipa-core/src/protocol/context/dzkp_validator.rs

@@ -684,7 +684,7 @@ pub trait DZKPValidator: Send + Sync {
     fn context(&self) -> Self::Context;
 
     /// Sets the validator's total number of records field. This is required when using
-    /// the validate_record API, if it wasn't already set on the context used to create
+    /// the `validate_record` API, if it wasn't already set on the context used to create
     /// the validator.
     fn set_total_records<T: Into<TotalRecords>>(&mut self, total_records: T);
 
@@ -860,7 +860,7 @@ impl<'a, B: ShardBinding> DZKPValidator for MaliciousDZKPValidator<'a, B> {
     }
 
     /// `is_verified` checks that there are no `MultiplicationInputs` that have not been verified.
-    /// This function is called by drop() to ensure that the validator is safe to be dropped.
+    /// This function is called by `drop()` to ensure that the validator is safe to be dropped.
     ///
     /// ## Errors
     /// Errors when there are `MultiplicationInputs` that have not been verified.

ipa-core/src/protocol/context/validator.rs

@@ -423,7 +423,7 @@ mod tests {
     /// This is the simplest arithmetic circuit that allows us to test all of the pieces of this validator
     /// A -
     ///     \
-    ///      Mult_Gate -> A*B
+    ///      `Mult_Gate` -> A*B
     ///     /
     /// B -
     ///

ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs

@@ -375,7 +375,7 @@ mod test {
     const BENCH_COUNT: usize = 131_072;
 
     #[test]
-    #[ignore] // benchmark
+    #[ignore = "benchmark"]
     #[cfg(not(coverage))]
     fn semi_honest_compare_gt_novec() {
         run(|| async move {
@@ -430,7 +430,7 @@ mod test {
     }
 
     #[test]
-    #[ignore] // benchmark
+    #[ignore = "benchmark"]
     #[cfg(not(coverage))]
     fn semi_honest_compare_gt_vec() {
         run(|| async move {

ipa-core/src/protocol/ipa_prf/oprf_padding/mod.rs

@@ -582,7 +582,7 @@ mod tests {
     }
 
     #[test]
-    #[ignore]
+    #[ignore = "manual execution only"]
     pub fn table_of_padding_parameters() {
         // see output https://docs.google.com/spreadsheets/d/1N0WEUkarP_6nd-7W8O9r-Xurh9OImESgAC1Jd_6OfWw/edit?gid=0#gid=0
         let epsilon_values = [0.01, 0.1, 1.0, 5.0, 10.0];

ipa-core/src/query/processor.rs

@@ -1128,9 +1128,9 @@ mod tests {
         use crate::{helpers::query::CompareStatusRequest, protocol::QueryId};
 
         /// * From the standpoint of leader shard in Helper 1
-        /// * On query_status
+        /// * On `query_status`
         ///
-        /// The min state should be returned. In this case, if I, as leader, am in AwaitingInputs
+        /// The min state should be returned. In this case, if I, as leader, am in `AwaitingInputs`
         /// state and shards report that they are further ahead (Completed and Running), then my
         /// state is returned.
         #[tokio::test]
@@ -1188,9 +1188,9 @@ mod tests {
         }
 
         /// * From the standpoint of leader shard in Helper 1
-        /// * On query_status
+        /// * On `query_status`
         ///
-        /// If one of my shards hasn't received the query yet (NoSuchQuery) the leader should
+        /// If one of my shards hasn't received the query yet (`NoSuchQuery`) the leader should
         /// return an error despite other shards returning their status
         #[tokio::test]
         #[should_panic(

ipa-core/src/query/runner/reshard_tag.rs

@@ -1,5 +1,5 @@
 use std::{
-    pin::{Pin, pin},
+    pin::Pin,
     task::{Context, Poll},
 };
 

ipa-core/src/test_fixture/sharing.rs

@@ -212,6 +212,7 @@ impl Reconstruct<()> for [(); 3] {
     fn reconstruct(&self) {}
 }
 
+#[allow(dead_code)] // used as a trait bound; not all feature combinations call its methods
 pub trait ValidateMalicious<F: ExtendableField> {
     fn validate(&self, r: F::ExtendedField);
 }

ipa-step-test/src/lib.rs

@@ -1,5 +1,8 @@
+#[cfg(test)]
 mod basic_step;
+#[cfg(test)]
 mod complex_step;
+#[cfg(test)]
 mod module;
 
 #[cfg(test)]