Merge branch 'add-device-check-test' into main

4 files changed, 297 insertions, 221 deletions

diff --git a/docs/relay-selector.md b/docs/relay-selector.md
index 4db6700f1d..a4c4b8a249 100644
--- a/docs/relay-selector.md
+++ b/docs/relay-selector.md
@@ -49,15 +49,9 @@ Endpoints may be filtered by:
 Whilst all user selected constraints are always honored, when the user hasn't selected any specific
 constraints, following default ones will take effect:
 
-- If no tunnel protocol is specified for tunnel endpoints, then the behavior is different on Windows
-  and other platforms.
-  - On MacOS and Linux, first two connection attempts will use WireGuard, over a random port at
-    first and then port 53. From the third attempt onwards, OpenVPN will be used, alternating
-    between UDP on any port and TCP on port 443.
-  - On Windows, a migration to WireGuard is ongoing and a percentage value provided by the API tells
-    clients to randomly decide if they will use WireGuard as a default or OpenVPN as a default.
-    The client's decision will persist over time.
-    If the client decides to use WireGuard it will have the same behavior as MacOS and Linux.
+- If no tunnel protocol is specified, the first two connection attempts will use WireGuard, over a
+  random port at first and then port 53. From the third attempt onwards, OpenVPN will be used,
+  alternating between UDP on any port and TCP on port 443.
 
 - If the tunnel protocol is specified as WireGuard and obfuscation mode is set to _Auto_:
   - First two attempts will be used without _udp2tcp_, using a random port on first attempt, and
diff --git a/mullvad-daemon/src/device/mod.rs b/mullvad-daemon/src/device/mod.rs
index a73d77cbf6..2668e995ee 100644
--- a/mullvad-daemon/src/device/mod.rs
+++ b/mullvad-daemon/src/device/mod.rs
@@ -44,8 +44,8 @@ const VALIDITY_CACHE_TIMEOUT: Duration = Duration::from_secs(10);
 /// How long to wait on logout (device removal) before letting it continue as a background task.
 const LOGOUT_TIMEOUT: Duration = Duration::from_secs(2);
 
-/// Validate the current device once for every `WG_DEVICE_CHECK_THRESHOLD` failed attempts
-/// to set up a WireGuard tunnel.
+/// Validate the current device once for every `WG_DEVICE_CHECK_THRESHOLD` attempt to set up
+/// a WireGuard tunnel.
 const WG_DEVICE_CHECK_THRESHOLD: usize = 2;
 
 #[derive(err_derive::Error, Debug, Clone)]
@@ -1232,7 +1232,7 @@ impl DeviceCacher {
 /// after multiple attempts.
 pub(crate) struct TunnelStateChangeHandler {
     manager: AccountManagerHandle,
-    check_validity: Arc<AtomicBool>,
+    no_more_retries: Arc<AtomicBool>,
     wg_retry_attempt: usize,
 }
 
@@ -1240,51 +1240,215 @@ impl TunnelStateChangeHandler {
     pub fn new(manager: AccountManagerHandle) -> Self {
         Self {
             manager,
-            check_validity: Arc::new(AtomicBool::new(true)),
+            no_more_retries: Arc::new(AtomicBool::new(false)),
             wg_retry_attempt: 0,
         }
     }
 
+    /// Handle state transitions and optionally check the device/account validity. This should be
+    /// called during every tunnel state transition.
     pub fn handle_state_transition(&mut self, new_state: &TunnelStateTransition) {
+        let handle = self.manager.clone();
+
+        let wg_attempt = self.wg_retry_attempt;
+        self.wg_retry_attempt = Self::next_retry_attempt(new_state, self.wg_retry_attempt);
+
+        if self.wg_retry_attempt > wg_attempt {
+            tokio::spawn(Self::maybe_check_validity(
+                wg_attempt,
+                self.no_more_retries.clone(),
+                move || Self::check_validity(handle),
+            ));
+        }
+    }
+
+    /// Return an incremented count for `retry_attempt` if this is another WireGuard connection
+    /// attempt, and zero the counter when leaving the connecting loop.
+    fn next_retry_attempt(new_state: &TunnelStateTransition, retry_attempt: usize) -> usize {
         match new_state {
             TunnelStateTransition::Connecting(endpoint) => {
-                if endpoint.tunnel_type != TunnelType::Wireguard {
-                    return;
-                }
-                self.wg_retry_attempt = self.wg_retry_attempt.wrapping_add(1);
-                if self.wg_retry_attempt % WG_DEVICE_CHECK_THRESHOLD == 0 {
-                    let handle = self.manager.clone();
-                    let check_validity = self.check_validity.clone();
-                    tokio::spawn(async move {
-                        if !check_validity.swap(false, Ordering::SeqCst) {
-                            return;
-                        }
-                        if let Err(error) = Self::check_validity(handle).await {
-                            log::error!(
-                                "{}",
-                                error.display_chain_with_msg(
-                                    "Failed to check device or account validity"
-                                )
-                            );
-                            if error.is_network_error() || error.is_aborted() {
-                                check_validity.store(true, Ordering::SeqCst);
-                            }
-                        }
-                    });
+                if endpoint.tunnel_type == TunnelType::Wireguard {
+                    retry_attempt.wrapping_add(1)
+                } else {
+                    retry_attempt
                 }
             }
             TunnelStateTransition::Error(_)
             | TunnelStateTransition::Connected(_)
-            | TunnelStateTransition::Disconnected => {
-                self.check_validity.store(true, Ordering::SeqCst);
-                self.wg_retry_attempt = 0;
+            | TunnelStateTransition::Disconnected => 0,
+            _ => retry_attempt,
+        }
+    }
+
+    /// Run `validate` when connecting to a WireGuard server, on certain retry attempts.
+    /// If `no_more_retries` is true, no further checks are made. `no_more_retries` is reset
+    /// on the first connection attempt.
+    ///
+    /// This returns whether the device/account validity ran.
+    async fn maybe_check_validity<Validate, ValidateResult>(
+        wg_attempt: usize,
+        no_more_retries: Arc<AtomicBool>,
+        validate: Validate,
+    ) -> bool
+    where
+        Validate: FnOnce() -> ValidateResult + Send + 'static,
+        ValidateResult: Future<Output = Result<(), Error>> + Send + 'static,
+    {
+        if wg_attempt == 0 {
+            // Starting a new connecting loop, so reset the retry state
+            no_more_retries.store(false, Ordering::SeqCst);
+        }
+
+        if !Self::should_check_validity_on_attempt(wg_attempt) {
+            return false;
+        }
+        if no_more_retries.swap(true, Ordering::SeqCst) {
+            // We've either already received the device state or we've given up
+            return false;
+        }
+        match validate().await {
+            Ok(()) => true,
+            Err(error) => {
+                log::error!(
+                    "{}",
+                    error.display_chain_with_msg("Failed to check device or account validity")
+                );
+                if Self::should_continue_retries(error) {
+                    // If the request failed due to a network error, we should continue
+                    // retrying. We give up otherwise, because it means we have a known result or
+                    // the API returned some error.
+                    no_more_retries.store(false, Ordering::SeqCst);
+                }
+                true
             }
-            _ => (),
         }
     }
 
-    pub async fn check_validity(handle: AccountManagerHandle) -> Result<(), Error> {
+    async fn check_validity(handle: AccountManagerHandle) -> Result<(), Error> {
         handle.validate_device().await?;
         handle.check_expiry().await.map(|_expiry| ())
     }
+
+    fn should_check_validity_on_attempt(wg_attempt: usize) -> bool {
+        wg_attempt % WG_DEVICE_CHECK_THRESHOLD == WG_DEVICE_CHECK_THRESHOLD - 1
+    }
+
+    fn should_continue_retries(err: Error) -> bool {
+        err.is_network_error() || err.is_aborted()
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::TunnelStateChangeHandler;
+    use super::{Error, WG_DEVICE_CHECK_THRESHOLD};
+    use mullvad_relay_selector::RelaySelector;
+    use std::sync::{
+        atomic::{AtomicBool, Ordering},
+        Arc,
+    };
+    use talpid_types::net::TunnelType;
+
+    const TIMEOUT_ERROR: Error = Error::OtherRestError(mullvad_api::rest::Error::TimeoutError);
+
+    /// Starting a new connection loop should resume device validity checks
+    #[tokio::test]
+    async fn test_device_check_reset() {
+        let no_more_retries = Arc::new(AtomicBool::new(true));
+
+        TunnelStateChangeHandler::maybe_check_validity(0, no_more_retries.clone(), || async {
+            Ok(())
+        })
+        .await;
+
+        assert!(
+            !no_more_retries.load(Ordering::SeqCst),
+            "expected retry state to be reset on first connection attempt"
+        );
+    }
+
+    /// Retries should stop when a device check succeeds
+    #[tokio::test]
+    async fn test_device_check_on_success() {
+        const ATTEMPT: usize = WG_DEVICE_CHECK_THRESHOLD - 1;
+        assert!(TunnelStateChangeHandler::should_check_validity_on_attempt(
+            ATTEMPT
+        ));
+
+        let no_more_retries = Arc::new(AtomicBool::new(false));
+
+        let check_ran = TunnelStateChangeHandler::maybe_check_validity(
+            ATTEMPT,
+            no_more_retries.clone(),
+            || async { Ok(()) },
+        )
+        .await;
+
+        assert!(check_ran, "expected device check to run");
+
+        let check_ran = TunnelStateChangeHandler::maybe_check_validity(
+            ATTEMPT,
+            no_more_retries.clone(),
+            || async { Ok(()) },
+        )
+        .await;
+
+        assert!(
+            !check_ran,
+            "expected device check to give up after successful check"
+        );
+    }
+
+    /// Retries should continue when a network error occurs
+    #[tokio::test]
+    async fn test_device_check_on_network_error() {
+        const ATTEMPT: usize = WG_DEVICE_CHECK_THRESHOLD - 1;
+        assert!(TunnelStateChangeHandler::should_check_validity_on_attempt(
+            ATTEMPT
+        ));
+
+        let no_more_retries = Arc::new(AtomicBool::new(false));
+
+        let check_ran = TunnelStateChangeHandler::maybe_check_validity(
+            ATTEMPT,
+            no_more_retries.clone(),
+            || async { Err(TIMEOUT_ERROR) },
+        )
+        .await;
+
+        assert!(check_ran, "expected device check to occur");
+
+        let check_ran = TunnelStateChangeHandler::maybe_check_validity(
+            ATTEMPT,
+            no_more_retries.clone(),
+            || async { Err(TIMEOUT_ERROR) },
+        )
+        .await;
+
+        assert!(
+            check_ran,
+            "expected device check to continue after a network error"
+        );
+    }
+
+    /// Test whether the relay selector selects wireguard often enough, given no special
+    /// constraints, to verify that the device is valid
+    #[test]
+    fn test_validates_by_default() {
+        for attempt in 0.. {
+            let should_validate =
+                TunnelStateChangeHandler::should_check_validity_on_attempt(attempt);
+            let (_, _, tunnel_type) =
+                RelaySelector::preferred_tunnel_constraints(attempt.try_into().unwrap());
+            assert_eq!(
+                tunnel_type,
+                TunnelType::Wireguard,
+                "failed on attempt {attempt}"
+            );
+            if should_validate {
+                // Now that we've triggered a device check, we can give up
+                break;
+            }
+        }
+    }
 }
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 0288f9d8c4..58069db4fa 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -2580,14 +2580,11 @@ impl DaemonShutdownHandle {
 }
 
 fn new_selector_config(settings: &Settings) -> SelectorConfig {
-    let default_tunnel_type = TunnelType::Wireguard;
-
     SelectorConfig {
         relay_settings: settings.relay_settings.clone(),
         bridge_state: settings.bridge_state,
         bridge_settings: settings.bridge_settings.clone(),
         obfuscation_settings: settings.obfuscation_settings.clone(),
-        default_tunnel_type,
         custom_lists: settings.custom_lists.clone(),
         relay_overrides: settings.relay_overrides.clone(),
     }
diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs
index d069da4bc0..174454995b 100644
--- a/mullvad-relay-selector/src/lib.rs
+++ b/mullvad-relay-selector/src/lib.rs
@@ -14,6 +14,7 @@ use mullvad_types::{
         SelectedObfuscation, Set, TransportPort, Udp2TcpObfuscationSettings,
     },
     relay_list::{BridgeEndpointData, Relay, RelayEndpointData, RelayList},
+    settings::Settings,
     CustomTunnelEndpoint,
 };
 use parking_lot::{Mutex, MutexGuard};
@@ -223,11 +224,24 @@ pub struct SelectorConfig {
     pub bridge_state: BridgeState,
     pub bridge_settings: BridgeSettings,
     pub obfuscation_settings: ObfuscationSettings,
-    pub default_tunnel_type: TunnelType,
     pub custom_lists: CustomListsSettings,
     pub relay_overrides: Vec<RelayOverride>,
 }
 
+impl Default for SelectorConfig {
+    fn default() -> Self {
+        let default_settings = Settings::default();
+        SelectorConfig {
+            relay_settings: default_settings.relay_settings,
+            bridge_settings: default_settings.bridge_settings,
+            obfuscation_settings: default_settings.obfuscation_settings,
+            bridge_state: default_settings.bridge_state,
+            custom_lists: default_settings.custom_lists,
+            relay_overrides: default_settings.relay_overrides,
+        }
+    }
+}
+
 #[derive(Clone)]
 pub struct RelaySelector {
     config: Arc<Mutex<SelectorConfig>>,
@@ -261,6 +275,17 @@ impl RelaySelector {
         }
     }
 
+    pub fn from_list(config: SelectorConfig, relay_list: RelayList) -> Self {
+        RelaySelector {
+            parsed_relays: Arc::new(Mutex::new(ParsedRelays::from_relay_list(
+                relay_list,
+                SystemTime::now(),
+                &config.relay_overrides,
+            ))),
+            config: Arc::new(Mutex::new(config)),
+        }
+    }
+
     pub fn set_config(&mut self, config: SelectorConfig) {
         let mut parsed_relays = self.parsed_relays.lock();
         parsed_relays.set_overrides(&config.relay_overrides);
@@ -295,7 +320,6 @@ impl RelaySelector {
                     constraints,
                     config.bridge_state,
                     retry_attempt,
-                    config.default_tunnel_type,
                     &config.custom_lists,
                 )?;
                 let bridge = match relay.endpoint {
@@ -337,7 +361,6 @@ impl RelaySelector {
         relay_constraints: &RelayConstraints,
         bridge_state: BridgeState,
         retry_attempt: u32,
-        default_tunnel_type: TunnelType,
         custom_lists: &CustomListsSettings,
     ) -> Result<NormalSelectedRelay, Error> {
         #[cfg(target_os = "android")]
@@ -361,7 +384,6 @@ impl RelaySelector {
                 relay_constraints,
                 bridge_state,
                 retry_attempt,
-                default_tunnel_type,
                 custom_lists,
             ),
         }
@@ -689,14 +711,12 @@ impl RelaySelector {
         relay_constraints: &RelayConstraints,
         bridge_state: BridgeState,
         retry_attempt: u32,
-        default_tunnel_type: TunnelType,
         custom_lists: &CustomListsSettings,
     ) -> Result<NormalSelectedRelay, Error> {
         let preferred_constraints = self.preferred_constraints(
             relay_constraints,
             bridge_state,
             retry_attempt,
-            default_tunnel_type,
             custom_lists,
         );
 
@@ -735,7 +755,6 @@ impl RelaySelector {
         original_constraints: &RelayConstraints,
         bridge_state: BridgeState,
         retry_attempt: u32,
-        default_tunnel_type: TunnelType,
         custom_lists: &CustomListsSettings,
     ) -> RelayConstraints {
         let location = ResolvedLocationConstraint::from_constraint(
@@ -743,12 +762,11 @@ impl RelaySelector {
             custom_lists,
         );
         let (preferred_port, preferred_protocol, preferred_tunnel) = self
-            .preferred_tunnel_constraints(
+            .preferred_tunnel_constraints_for_location(
                 retry_attempt,
-                default_tunnel_type,
                 &location,
                 &original_constraints.providers,
-                &original_constraints.ownership,
+                original_constraints.ownership,
             );
 
         let mut relay_constraints = original_constraints.clone();
@@ -1090,51 +1108,43 @@ impl RelaySelector {
             })
     }
 
-    /// Returns preferred constraints
-    #[allow(unused_variables)]
-    fn preferred_tunnel_constraints(
+    /// Return the preferred constraints, on attempt `retry_attempt`, for matching locations
+    fn preferred_tunnel_constraints_for_location(
         &self,
         retry_attempt: u32,
-        default_tunnel_type: TunnelType,
-        location_constraint: &Constraint<ResolvedLocationConstraint>,
-        providers_constraint: &Constraint<Providers>,
-        ownership_constraint: &Constraint<Ownership>,
+        location: &Constraint<ResolvedLocationConstraint>,
+        providers: &Constraint<Providers>,
+        ownership: Constraint<Ownership>,
     ) -> (Constraint<u16>, TransportProtocol, TunnelType) {
-        match default_tunnel_type {
-            TunnelType::OpenVpn => {
-                let location_supports_openvpn = self.parsed_relays.lock().relays().any(|relay| {
-                    relay.active
-                        && relay.endpoint_data == RelayEndpointData::Openvpn
-                        && location_constraint.matches_with_opts(relay, true)
-                        && providers_constraint.matches(relay)
-                        && ownership_constraint.matches(relay)
-                });
-
-                if location_supports_openvpn {
-                    let (preferred_port, preferred_protocol) =
-                        Self::preferred_openvpn_constraints(retry_attempt);
-                    return (preferred_port, preferred_protocol, TunnelType::OpenVpn);
-                }
+        let parsed_relays = self.parsed_relays.lock();
+        let mut active_location_relays = parsed_relays.relays().filter(|relay| {
+            relay.active
+                && location.matches_with_opts(relay, true)
+                && providers.matches(relay)
+                && ownership.matches(relay)
+        });
+        let location_supports_wg = active_location_relays
+            .clone()
+            .any(|relay| matches!(relay.endpoint_data, RelayEndpointData::Wireguard(_)));
+        let location_supports_openvpn = active_location_relays
+            .any(|relay| matches!(relay.endpoint_data, RelayEndpointData::Openvpn));
+        match (location_supports_wg, location_supports_openvpn) {
+            (true, true) | (false, false) => Self::preferred_tunnel_constraints(retry_attempt),
+            (true, false) => {
+                let port = Self::preferred_wireguard_port(retry_attempt);
+                (port, TransportProtocol::Udp, TunnelType::Wireguard)
             }
-            TunnelType::Wireguard => {
-                let location_supports_wireguard = self.parsed_relays.lock().relays().any(|relay| {
-                    relay.active
-                        && matches!(relay.endpoint_data, RelayEndpointData::Wireguard(_))
-                        && location_constraint.matches_with_opts(relay, true)
-                        && providers_constraint.matches(relay)
-                        && ownership_constraint.matches(relay)
-                });
-
-                // If location does not support WireGuard, defer to preferred OpenVPN tunnel
-                // constraints
-                if !location_supports_wireguard {
-                    let (preferred_port, preferred_protocol) =
-                        Self::preferred_openvpn_constraints(retry_attempt);
-                    return (preferred_port, preferred_protocol, TunnelType::OpenVpn);
-                }
+            (false, true) => {
+                let (port, transport) = Self::preferred_openvpn_constraints(retry_attempt);
+                (port, transport, TunnelType::OpenVpn)
             }
         }
+    }
 
+    /// Return the preferred constraints, on attempt `retry_attempt`, given no other constraints
+    pub const fn preferred_tunnel_constraints(
+        retry_attempt: u32,
+    ) -> (Constraint<u16>, TransportProtocol, TunnelType) {
         // Try out WireGuard in the first two connection attempts, first with any port,
         // afterwards on port 53. Afterwards, connect through OpenVPN alternating between UDP
         // on any port twice and TCP on port 443 once.
@@ -1157,7 +1167,7 @@ impl RelaySelector {
         }
     }
 
-    fn preferred_wireguard_port(retry_attempt: u32) -> Constraint<u16> {
+    const fn preferred_wireguard_port(retry_attempt: u32) -> Constraint<u16> {
         // This ensures that if after the first 2 failed attempts the daemon does not
         // connect, then afterwards 2 of each 4 successive attempts will try to connect
         // on port 53.
@@ -1167,15 +1177,15 @@ impl RelaySelector {
         }
     }
 
-    fn preferred_openvpn_constraints(retry_attempt: u32) -> (Constraint<u16>, TransportProtocol) {
+    const fn preferred_openvpn_constraints(
+        retry_attempt: u32,
+    ) -> (Constraint<u16>, TransportProtocol) {
         // Prefer UDP by default. But if that has failed a couple of times, then try TCP port
         // 443, which works for many with UDP problems. After that, just alternate
         // between protocols.
         // If the tunnel type constraint is set OpenVpn, from the 4th attempt onwards, the first
         // two retry attempts OpenVpn constraints should be set to TCP as a bridge will be used,
-        // and to UDP or TCP for the next two attempts. If the tunnel type is specified to be _Any_
-        // and on not-Windows, the first two tries are used for WireGuard and don't
-        // affect counting here.
+        // and to UDP or TCP for the next two attempts.
         match retry_attempt {
             0 | 1 => (Constraint::Any, TransportProtocol::Udp),
             2 | 3 => (Constraint::Only(443), TransportProtocol::Tcp),
@@ -1339,8 +1349,7 @@ mod test {
     use mullvad_types::{
         custom_list::CustomListsSettings,
         relay_constraints::{
-            BridgeConstraints, GeographicLocationConstraint, RelayConstraints, RelaySettings,
-            WireguardConstraints,
+            GeographicLocationConstraint, RelayConstraints, RelaySettings, WireguardConstraints,
         },
         relay_list::{
             OpenVpnEndpoint, OpenVpnEndpointData, Relay, RelayListCity, RelayListCountry,
@@ -1492,48 +1501,9 @@ mod test {
         },
     });
 
-    fn default_tunnel_type() -> TunnelType {
-        if cfg!(target_os = "windows") {
-            TunnelType::OpenVpn
-        } else {
-            TunnelType::Wireguard
-        }
-    }
-
-    fn new_relay_selector_with_relays(relay_list: RelayList) -> RelaySelector {
-        RelaySelector {
-            parsed_relays: Arc::new(Mutex::new(ParsedRelays::from_relay_list(
-                relay_list,
-                SystemTime::now(),
-                &[],
-            ))),
-            config: Arc::new(Mutex::new(SelectorConfig {
-                relay_settings: RelaySettings::Normal(RelayConstraints {
-                    location: Constraint::Only(LocationConstraint::from(
-                        GeographicLocationConstraint::Country("se".to_owned()),
-                    )),
-                    ..Default::default()
-                }),
-                bridge_settings: BridgeSettings::Normal(BridgeConstraints::default()),
-                obfuscation_settings: ObfuscationSettings {
-                    selected_obfuscation: SelectedObfuscation::Off,
-                    ..Default::default()
-                },
-                bridge_state: BridgeState::Auto,
-                default_tunnel_type: default_tunnel_type(),
-                custom_lists: CustomListsSettings::default(),
-                relay_overrides: vec![],
-            })),
-        }
-    }
-
-    fn new_relay_selector() -> RelaySelector {
-        new_relay_selector_with_relays(RELAYS.clone())
-    }
-
     #[test]
     fn test_preferred_tunnel_protocol() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
         // Prefer WG if the location only supports it
         let location = GeographicLocationConstraint::Hostname(
@@ -1551,7 +1521,6 @@ mod test {
             &relay_constraints,
             BridgeState::Off,
             0,
-            TunnelType::Wireguard,
             &CustomListsSettings::default(),
         );
         assert_eq!(
@@ -1565,7 +1534,6 @@ mod test {
                     &relay_constraints,
                     BridgeState::Off,
                     attempt,
-                    TunnelType::Wireguard,
                     &CustomListsSettings::default()
                 )
                 .is_ok());
@@ -1587,7 +1555,6 @@ mod test {
             &relay_constraints,
             BridgeState::Off,
             0,
-            TunnelType::Wireguard,
             &CustomListsSettings::default(),
         );
         assert_eq!(
@@ -1601,45 +1568,15 @@ mod test {
                     &relay_constraints,
                     BridgeState::Off,
                     attempt,
-                    TunnelType::Wireguard,
                     &CustomListsSettings::default()
                 )
                 .is_ok());
         }
-
-        // Prefer OpenVPN on Windows when possible
-        #[cfg(windows)]
-        {
-            let relay_constraints = RelayConstraints::default();
-            for attempt in 0..10 {
-                let preferred = relay_selector.preferred_constraints(
-                    &relay_constraints,
-                    BridgeState::Off,
-                    attempt,
-                    TunnelType::OpenVpn,
-                    &CustomListsSettings::default(),
-                );
-                assert_eq!(
-                    preferred.tunnel_protocol,
-                    Constraint::Only(TunnelType::OpenVpn)
-                );
-                match relay_selector.get_any_tunnel_endpoint(
-                    &relay_constraints,
-                    BridgeState::Off,
-                    attempt,
-                    TunnelType::OpenVpn,
-                    &CustomListsSettings::default(),
-                ) {
-                    Ok(result) if matches!(result.endpoint, MullvadEndpoint::OpenVpn(_)) => (),
-                    _ => panic!("OpenVPN endpoint was not selected"),
-                }
-            }
-        }
     }
 
     #[test]
     fn test_wg_entry_hostname_collision() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
         let location1 = GeographicLocationConstraint::Hostname(
             "se".to_string(),
@@ -1668,7 +1605,6 @@ mod test {
                 &relay_constraints,
                 BridgeState::Off,
                 0,
-                TunnelType::Wireguard,
                 &CustomListsSettings::default()
             )
             .is_err());
@@ -1682,7 +1618,6 @@ mod test {
                 &relay_constraints,
                 BridgeState::Off,
                 0,
-                TunnelType::Wireguard,
                 &CustomListsSettings::default()
             )
             .is_ok());
@@ -1690,7 +1625,7 @@ mod test {
 
     #[test]
     fn test_wg_entry_filter() -> Result<(), String> {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
         let specific_hostname = "se10-wireguard";
 
@@ -1720,7 +1655,6 @@ mod test {
                 &relay_constraints,
                 BridgeState::Off,
                 0,
-                TunnelType::OpenVpn,
                 &CustomListsSettings::default(),
             )
             .map_err(|error| error.to_string())?
@@ -1741,7 +1675,6 @@ mod test {
                 &relay_constraints,
                 BridgeState::Off,
                 0,
-                TunnelType::Wireguard,
                 &CustomListsSettings::default(),
             )
             .map_err(|error| error.to_string())?;
@@ -1760,7 +1693,7 @@ mod test {
 
     #[test]
     fn test_openvpn_constraints() -> Result<(), String> {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
         const ACTUAL_TCP_PORT: u16 = 443;
         const ACTUAL_UDP_PORT: u16 = 1194;
@@ -1858,7 +1791,6 @@ mod test {
                     &relay_constraints,
                     BridgeState::Auto,
                     retry_attempt,
-                    default_tunnel_type(),
                     &CustomListsSettings::default(),
                 );
 
@@ -1886,7 +1818,7 @@ mod test {
 
     #[test]
     fn test_bridge_constraints() -> Result<(), String> {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
         let location = LocationConstraint::from(GeographicLocationConstraint::Hostname(
             "se".to_string(),
@@ -1907,7 +1839,6 @@ mod test {
             &relay_constraints,
             BridgeState::On,
             0,
-            TunnelType::Wireguard,
             &CustomListsSettings::default(),
         );
         assert_eq!(
@@ -1938,7 +1869,6 @@ mod test {
             &relay_constraints,
             BridgeState::On,
             0,
-            TunnelType::Wireguard,
             &CustomListsSettings::default(),
         );
         assert_eq!(
@@ -1962,7 +1892,6 @@ mod test {
                 &relay_constraints,
                 BridgeState::On,
                 0,
-                TunnelType::Wireguard,
                 &CustomListsSettings::default(),
             );
             assert_eq!(
@@ -1974,7 +1903,6 @@ mod test {
             &relay_constraints,
             BridgeState::On,
             2,
-            TunnelType::Wireguard,
             &CustomListsSettings::default(),
         );
         assert_eq!(
@@ -2005,18 +1933,11 @@ mod test {
             ..RelayConstraints::default()
         };
 
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
-        let result = relay_selector.get_tunnel_endpoint(&relay_constraints, BridgeState::Off, 0, default_tunnel_type(), &CustomListsSettings::default())
+        let result = relay_selector.get_tunnel_endpoint(&relay_constraints, BridgeState::Off, 0, &CustomListsSettings::default())
             .expect("Failed to get relay when tunnel constraints are set to Any and retrying the selection");
-        // Windows will ignore WireGuard until WireGuard is supported well enough
-        // TODO: Remove this caveat once Windows defaults to using WireGuard
-        #[cfg(target_os = "windows")]
-        assert!(
-            matches!(result.endpoint, MullvadEndpoint::OpenVpn(_)) && result.entry_relay.is_none()
-        );
 
-        #[cfg(not(target_os = "windows"))]
         assert!(
             matches!(result.endpoint, MullvadEndpoint::Wireguard(_))
                 && result.entry_relay.is_some()
@@ -2057,9 +1978,9 @@ mod test {
 
     #[test]
     fn test_selecting_wireguard_location_will_consider_multihop() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
-        let result = relay_selector.get_tunnel_endpoint(&WIREGUARD_MULTIHOP_CONSTRAINTS, BridgeState::Off, 0, default_tunnel_type(), &CustomListsSettings::default())
+        let result = relay_selector.get_tunnel_endpoint(&WIREGUARD_MULTIHOP_CONSTRAINTS, BridgeState::Off, 0, &CustomListsSettings::default())
             .expect("Failed to get relay when tunnel constraints are set to default WireGuard multihop constraints");
 
         assert!(result.entry_relay.is_some());
@@ -2068,9 +1989,9 @@ mod test {
 
     #[test]
     fn test_selecting_wg_endpoint_with_udp2tcp_obfuscation() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
-        let result = relay_selector.get_tunnel_endpoint(&WIREGUARD_SINGLEHOP_CONSTRAINTS, BridgeState::Off, 0, default_tunnel_type(), &CustomListsSettings::default())
+        let result = relay_selector.get_tunnel_endpoint(&WIREGUARD_SINGLEHOP_CONSTRAINTS, BridgeState::Off, 0, &CustomListsSettings::default())
             .expect("Failed to get relay when tunnel constraints are set to default WireGuard constraints");
 
         assert!(result.entry_relay.is_none());
@@ -2097,9 +2018,9 @@ mod test {
 
     #[test]
     fn test_selecting_wg_endpoint_with_auto_obfuscation() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
-        let result = relay_selector.get_tunnel_endpoint(&WIREGUARD_SINGLEHOP_CONSTRAINTS, BridgeState::Off, 0, default_tunnel_type(), &CustomListsSettings::default())
+        let result = relay_selector.get_tunnel_endpoint(&WIREGUARD_SINGLEHOP_CONSTRAINTS, BridgeState::Off, 0, &CustomListsSettings::default())
             .expect("Failed to get relay when tunnel constraints are set to default WireGuard constraints");
 
         assert!(result.entry_relay.is_none());
@@ -2128,7 +2049,7 @@ mod test {
 
     #[test]
     fn test_selected_endpoints_use_correct_port_ranges() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
         const TCP2UDP_PORTS: [u16; 3] = [80, 443, 5001];
 
@@ -2143,7 +2064,6 @@ mod test {
                     &WIREGUARD_SINGLEHOP_CONSTRAINTS,
                     BridgeState::Off,
                     attempt,
-                    TunnelType::Wireguard,
                     &CustomListsSettings::default(),
                 )
                 .expect("Failed to select a WireGuard relay");
@@ -2176,7 +2096,7 @@ mod test {
 
     #[test]
     fn test_ownership() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
         let mut constraints = RelayConstraints::default();
         for i in 0..10 {
             constraints.ownership = Constraint::Only(Ownership::MullvadOwned);
@@ -2185,7 +2105,6 @@ mod test {
                     &constraints,
                     BridgeState::Auto,
                     i,
-                    TunnelType::Wireguard,
                     &CustomListsSettings::default(),
                 )
                 .unwrap();
@@ -2203,7 +2122,6 @@ mod test {
                     &constraints,
                     BridgeState::Auto,
                     i,
-                    TunnelType::Wireguard,
                     &CustomListsSettings::default(),
                 )
                 .unwrap();
@@ -2220,7 +2138,7 @@ mod test {
     // Make sure server and port selection varies between retry attempts.
     #[test]
     fn test_load_balancing() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
         for tunnel_protocol in [
             Constraint::Any,
@@ -2268,7 +2186,7 @@ mod test {
     fn test_providers() {
         const EXPECTED_PROVIDERS: [&str; 2] = ["provider0", "provider2"];
 
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
         let mut constraints = RelayConstraints::default();
 
         for i in 0..10 {
@@ -2280,7 +2198,6 @@ mod test {
                     &constraints,
                     BridgeState::Auto,
                     i,
-                    TunnelType::Wireguard,
                     &CustomListsSettings::default(),
                 )
                 .unwrap();
@@ -2297,7 +2214,7 @@ mod test {
     /// to automatic.
     #[test]
     fn test_auto_bridge() {
-        let relay_selector = new_relay_selector();
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone());
 
         {
             let mut config = relay_selector.config.lock();
@@ -2412,27 +2329,31 @@ mod test {
         // If include_in_country is false for all relays, a relay must be selected anyway.
         //
 
-        let relay_selector = new_relay_selector_with_relays(relay_list.clone());
+        let relay_selector =
+            RelaySelector::from_list(SelectorConfig::default(), relay_list.clone());
         assert!(relay_selector.get_relay(0).is_ok());
 
         // If include_in_country is true for some relay, it must always be selected.
         //
 
         relay_list.countries[0].cities[0].relays[0].include_in_country = true;
-        let expected_relay = relay_list.countries[0].cities[0].relays[0].clone();
+        let expected_hostname = relay_list.countries[0].cities[0].relays[0].hostname.clone();
 
-        let relay_selector = new_relay_selector_with_relays(relay_list);
+        let relay_selector = RelaySelector::from_list(SelectorConfig::default(), relay_list);
         let (relay, ..) = relay_selector.get_relay(0).expect("expected match");
 
-        assert!(matches!(
-            relay,
-            SelectedRelay::Normal(NormalSelectedRelay {
-                exit_relay: Relay {
-                    hostname,
+        assert!(
+            matches!(
+                relay,
+                SelectedRelay::Normal(NormalSelectedRelay {
+                    exit_relay: Relay {
+                        ref hostname,
+                        ..
+                    },
                     ..
-                },
-                ..
-            }) if hostname == expected_relay.hostname
-        ))
+                }) if hostname == &expected_hostname,
+            ),
+            "found {relay:?}, expected {expected_hostname:?}",
+        )
     }
 }