diff --git a/crates/tor-hsservice/src/ipt_mgr.rs b/crates/tor-hsservice/src/ipt_mgr.rs index d956671dd..f856905cb 100644 --- a/crates/tor-hsservice/src/ipt_mgr.rs +++ b/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>( &mut self, imm: &Immutable, 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: Debug + Send + Sync + Sized + 'static { fn make_new_ipt( &mut self, imm: &Immutable, - relay: RelayIds, + params: IptParameters, ) -> Result<(Self::IptEstablisher, watch::Receiver), FatalError>; /// Call `Publisher::new_intro_points` @@ -755,21 +765,15 @@ impl Mockable for Real { rand::thread_rng() } - #[allow(unreachable_code)] // TODO HSS remove fn make_new_ipt( &mut self, imm: &Immutable, - relay: RelayIds, + params: IptParameters, ) -> Result<(Self::IptEstablisher, watch::Receiver), 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 ) } } diff --git a/crates/tor-hsservice/src/svc/ipt_establish.rs b/crates/tor-hsservice/src/svc/ipt_establish.rs index 7157b1ebb..1e642c344 100644 --- a/crates/tor-hsservice/src/svc/ipt_establish.rs +++ b/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, + pub(crate) introduce_tx: mpsc::Sender, + 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( runtime: R, + params: IptParameters, pool: Arc>, - netdir_provider: Arc, - introduce_tx: mpsc::Sender, - 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), 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?"