ipt establisher API: pass a struct to new()

Now new() only has a reasonable number of arguments and removes some
repetition in the mocking arrangements in the IPT Manager.

This is the minimum amount that needs to be done in the commit that
touches both the IPT Establisher and the Manager.

crates/tor-hsservice/src/ipt_mgr.rs

@@ -32,7 +32,7 @@ use tor_rtcompat::Runtime;
 use crate::svc::ipt_establish;
 use crate::timeout_track::{TrackingInstantOffsetNow, TrackingNow};
 use crate::{FatalError, OnionServiceConfig, RendRequest, StartupError};
-use ipt_establish::{IptEstablisher, IptStatus, IptStatusStatus, IptWantsToRetire};
+use ipt_establish::{IptEstablisher, IptParameters, IptStatus, IptStatusStatus, IptWantsToRetire};
 
 use IptStatusStatus as ISS;
 use TrackedStatus as TS;
@@ -305,12 +305,22 @@ impl IptRelay {
     /// Make a new introduction point at this relay
     ///
     /// It becomes the current IPT.
+    #[allow(unreachable_code, clippy::diverging_sub_expression)] // TODO HSS remove
     fn make_new_ipt<R: Runtime, M: Mockable<R>>(
         &mut self,
         imm: &Immutable<R>,
         mockable: &mut M,
     ) -> Result<(), FatalError> {
-        let (establisher, mut watch_rx) = mockable.make_new_ipt(imm, self.relay.clone())?;
+        let params = IptParameters {
+            netdir_provider: imm.dirprovider.clone(),
+            introduce_tx: imm.output_rend_reqs.clone(),
+            // TODO HSS IntroPointId lacks a constructor and maybe should change anyway
+            intro_pt_id: todo!(),
+            target: self.relay.clone(),
+            ipt_sid_keypair: todo!(), // TODO HSS
+            accepting_requests: todo!(), // TODO HSS
+        };
+        let (establisher, mut watch_rx) = mockable.make_new_ipt(imm, params)?;
 
         // we'll treat it as Establishing until we find otherwise
         let status_last = TS::Establishing {
@@ -735,7 +745,7 @@ pub(crate) trait Mockable<R>: Debug + Send + Sync + Sized + 'static {
     fn make_new_ipt(
         &mut self,
         imm: &Immutable<R>,
-        relay: RelayIds,
+        params: IptParameters,
     ) -> Result<(Self::IptEstablisher, watch::Receiver<IptStatus>), FatalError>;
 
     /// Call `Publisher::new_intro_points`
@@ -755,21 +765,15 @@ impl<R: Runtime> Mockable<R> for Real<R> {
         rand::thread_rng()
     }
 
-    #[allow(unreachable_code)] // TODO HSS remove
     fn make_new_ipt(
         &mut self,
         imm: &Immutable<R>,
-        relay: RelayIds,
+        params: IptParameters,
     ) -> Result<(Self::IptEstablisher, watch::Receiver<IptStatus>), FatalError> {
         IptEstablisher::new(
             imm.runtime.clone(),
+            params,
             self.circ_pool.clone(),
-            imm.dirprovider.clone(),
-            imm.output_rend_reqs.clone(),
-            todo!(), // TODO HSS IntroPointId lacks a constructor and maybe should change anyway
-            relay,
-            todo!(), // TODO HSS keypair ought to be provided by our caller
-            todo!(), // TODO HSS RequestDisposition ought to be provided by our caller
         )
     }
 }

crates/tor-hsservice/src/svc/ipt_establish.rs

@@ -151,6 +151,27 @@ impl IptError {
     }
 }
 
+/// Parameters for an introduction point
+///
+/// Consumed by `IptEstablisher::new`.
+/// Primarily serves as a convenient way to bundle the many arguments required.
+///
+/// Does not include:
+///  * The runtime (which would force this struct to have a type parameter)
+///  * The circuit builder (leaving this out makes it possible to use this
+///    struct during mock execution, where we don't call `IptEstablisher::new`).
+#[allow(clippy::missing_docs_in_private_items)] // TODO HSS document these and remove
+pub(crate) struct IptParameters {
+    pub(crate) netdir_provider: Arc<dyn NetDirProvider>,
+    pub(crate) introduce_tx: mpsc::Sender<RendRequest>,
+    pub(crate) intro_pt_id: IntroPointId,
+    // TODO HSS: Should this and the following elements be part of some
+    // configuration object?
+    pub(crate) target: RelayIds,
+    pub(crate) ipt_sid_keypair: HsIntroPtSessionIdKeypair,
+    pub(crate) accepting_requests: RequestDisposition,
+}
+
 impl IptEstablisher {
     /// Try to set up, and maintain, an IPT at `target`.
     ///
@@ -165,19 +186,21 @@ impl IptEstablisher {
     /// The returned `watch::Receiver` will yield `Faulty` if the IPT
     /// establisher is shut down (or crashes).
     // TODO HSS rename to "launch" since it starts the task?
-    #[allow(clippy::too_many_arguments)] // TODO HSS refactor.
     pub(crate) fn new<R: Runtime>(
         runtime: R,
+        params: IptParameters,
         pool: Arc<HsCircPool<R>>,
-        netdir_provider: Arc<dyn NetDirProvider>,
-        introduce_tx: mpsc::Sender<RendRequest>,
-        intro_pt_id: IntroPointId,
-        // TODO HSS: Should this and the following elements be part of some
-        // configuration object?
-        target: RelayIds,
-        ipt_sid_keypair: HsIntroPtSessionIdKeypair,
-        accepting_requests: RequestDisposition,
     ) -> Result<(Self, postage::watch::Receiver<IptStatus>), FatalError> {
+        // This exhaustive deconstruction ensures that we don't
+        // accidentally forget to handle any of our inputs.
+        let IptParameters {
+            netdir_provider,
+            introduce_tx,
+            intro_pt_id,
+            target,
+            ipt_sid_keypair,
+            accepting_requests,
+        } = params;
         if matches!(accepting_requests, RequestDisposition::Shutdown) {
             return Err(bad_api_usage!(
                 "Tried to create a IptEstablisher that that was already shutting down?"