Move FeatureIndicators to TunnelState

Do not broadcast new feature indicators as a daemon event. Rather, it is
now part of the tunnel state, which ties their presence to the tunnel
states in which we would be concerned to present them to the user.

android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt

@@ -224,7 +224,6 @@ class ManagementService(
                         }
                         ManagementInterface.DaemonEvent.EventCase.REMOVE_DEVICE -> {}
                         ManagementInterface.DaemonEvent.EventCase.EVENT_NOT_SET -> {}
-                        ManagementInterface.DaemonEvent.EventCase.FEATURE_INDICATORS -> {}
                     }
                 }
             }

mullvad-cli/src/cmds/status.rs

@@ -54,9 +54,15 @@ impl Status {
                                 // Do print an updated state if the lockdown setting was changed
                             ) if was_locked_down == locked_down => continue,
                             (
-                                Some(TunnelState::Connected { .. }),
-                                TunnelState::Connected { .. },
-                            ) => continue,
+                                Some(TunnelState::Connected {
+                                    feature_indicators: old_feature_indicators,
+                                    ..
+                                }),
+                                TunnelState::Connected {
+                                    feature_indicators, ..
+                                },
+                                // Do print an updated state if the feature indicators changed
+                            ) if old_feature_indicators == feature_indicators => continue,
                             _ => {}
                         }
                         format::print_state(&new_state, args.verbose);
@@ -93,11 +99,6 @@ impl Status {
                         println!("New access method: {access_method:#?}");
                     }
                 }
-                DaemonEvent::FeatureIndicators(feature_indicators) => {
-                    if args.debug {
-                        println!("New feature indicators: {feature_indicators:#?}");
-                    }
-                }
             }
         }
         Ok(())

mullvad-cli/src/format.rs

@@ -1,4 +1,8 @@
-use mullvad_types::{auth_failed::AuthFailed, location::GeoIpLocation, states::TunnelState};
+use itertools::Itertools;
+use mullvad_types::{
+    auth_failed::AuthFailed, features::FeatureIndicators, location::GeoIpLocation,
+    states::TunnelState,
+};
 use talpid_types::{
     net::{Endpoint, TunnelEndpoint},
     tunnel::ErrorState,
@@ -19,18 +23,27 @@ pub fn print_state(state: &TunnelState, verbose: bool) {
 
     match state {
         Error(error) => print_error_state(error),
-        Connected { endpoint, location } => {
+        Connected {
+            endpoint,
+            location,
+            feature_indicators,
+        } => {
             println!(
                 "Connected to {}",
                 format_relay_connection(endpoint, location.as_ref(), verbose)
             );
             if verbose {
+                println!("Using {}", format_feature_indicators(feature_indicators));
                 if let Some(tunnel_interface) = &endpoint.tunnel_interface {
                     println!("Tunnel interface: {tunnel_interface}")
                 }
             }
         }
-        Connecting { endpoint, location } => {
+        Connecting {
+            endpoint,
+            location,
+            feature_indicators: _,
+        } => {
             let ellipsis = if !verbose { "..." } else { "" };
             println!(
                 "Connecting to {}{ellipsis}",
@@ -166,24 +179,6 @@ fn format_relay_connection(
     } else {
         String::new()
     };
-    let quantum_resistant = if !verbose {
-        ""
-    } else if endpoint.quantum_resistant {
-        "\nQuantum resistant tunnel: yes"
-    } else {
-        "\nQuantum resistant tunnel: no"
-    };
-
-    #[cfg(daita)]
-    let daita = if !verbose {
-        ""
-    } else if endpoint.daita {
-        "\nDAITA: yes"
-    } else {
-        "\nDAITA: no"
-    };
-    #[cfg(not(daita))]
-    let daita = "";
 
     let mut bridge_type = String::new();
     let mut obfuscator_type = String::new();
@@ -197,13 +192,21 @@ fn format_relay_connection(
     }
 
     format!(
-        "{exit_endpoint}{first_hop}{bridge}{obfuscator}{tunnel_type}{quantum_resistant}{daita}{bridge_type}{obfuscator_type}",
+        "{exit_endpoint}{first_hop}{bridge}{obfuscator}{tunnel_type}{bridge_type}{obfuscator_type}",
         first_hop = first_hop.unwrap_or_default(),
         bridge = bridge.unwrap_or_default(),
         obfuscator = obfuscator.unwrap_or_default(),
     )
 }
 
+fn format_feature_indicators(feature_indicators: &FeatureIndicators) -> String {
+    feature_indicators
+        .active_features()
+        // Sort the features alphabetically (Just to have some order, arbitrarily chosen)
+        .sorted_by_key(|feature| feature.to_string())
+        .join(", ")
+}
+
 fn format_endpoint(hostname: Option<&str>, endpoint: &Endpoint, verbose: bool) -> String {
     match (hostname, verbose) {
         (Some(hostname), true) => format!("{hostname} ({endpoint})"),

mullvad-daemon/src/lib.rs

@@ -86,7 +86,7 @@ use talpid_types::android::AndroidContext;
 #[cfg(target_os = "windows")]
 use talpid_types::split_tunnel::ExcludedProcess;
 use talpid_types::{
-    net::{IpVersion, ObfuscationType, TunnelEndpoint, TunnelType},
+    net::{IpVersion, ObfuscationType, TunnelType},
     tunnel::{ErrorStateCause, TunnelStateTransition},
     ErrorExt,
 };
@@ -396,12 +396,11 @@ pub(crate) enum InternalDaemonEvent {
     DeviceMigrationEvent(Result<PrivateAccountAndDevice, device::Error>),
     /// A geographical location has has been received from am.i.mullvad.net
     LocationEvent(LocationEventData),
+    /// A generic event for when any settings change.
+    SettingsChanged,
     /// The split tunnel paths or state were updated.
     #[cfg(any(windows, target_os = "android", target_os = "macos"))]
     ExcludedPathsEvent(ExcludedPathsUpdate, oneshot::Sender<Result<(), Error>>),
-
-    /// A setting in the [SettingsPersister] was changed.
-    SettingsChanged,
 }
 
 #[cfg(any(windows, target_os = "android", target_os = "macos"))]
@@ -573,9 +572,6 @@ pub trait EventListener: Clone + Send + Sync + 'static {
 
     /// Notify that the api access method changed.
     fn notify_new_access_method_event(&self, new_access_method: AccessMethodSetting);
-
-    /// Notify that the feature indicators have changed.
-    fn notify_feature_indicators(&self, feature_indicators: FeatureIndicators);
 }
 
 pub struct Daemon<L: EventListener> {
@@ -753,11 +749,6 @@ where
             settings.tunnel_options.clone(),
         );
 
-        let settings_daemon_tx = internal_event_tx.clone();
-        settings.register_change_listener(move |_| {
-            let _ = settings_daemon_tx.send(InternalDaemonEvent::SettingsChanged);
-        });
-
         let param_gen = parameters_generator.clone();
         let (param_gen_tx, mut param_gen_rx) = mpsc::unbounded();
         tokio::spawn(async move {
@@ -769,6 +760,13 @@ where
             let _ = param_gen_tx.unbounded_send(settings.tunnel_options.to_owned());
         });
 
+        // Register a listener for generic settings changes.
+        // This is useful for example for updating feature indicators when the settings change.
+        let settings_changed_event_sender = internal_event_tx.clone();
+        settings.register_change_listener(move |_settings| {
+            let _ = settings_changed_event_sender.send(InternalDaemonEvent::SettingsChanged);
+        });
+
         let (offline_state_tx, offline_state_rx) = mpsc::unbounded();
         #[cfg(target_os = "windows")]
         let (volume_update_tx, volume_update_rx) = mpsc::unbounded();
@@ -961,9 +959,11 @@ where
             } => self.handle_access_method_event(event, endpoint_active_tx),
             DeviceMigrationEvent(event) => self.handle_device_migration_event(event),
             LocationEvent(location_data) => self.handle_location_event(location_data),
+            SettingsChanged => {
+                self.handle_feature_indicator_event();
+            }
             #[cfg(any(windows, target_os = "android", target_os = "macos"))]
             ExcludedPathsEvent(update, tx) => self.handle_new_excluded_paths(update, tx).await,
-            SettingsChanged => self.handle_settings_changed(),
         }
         should_stop
     }
@@ -982,12 +982,14 @@ where
                 locked_down,
             },
             TunnelStateTransition::Connecting(endpoint) => TunnelState::Connecting {
-                location: self.parameters_generator.get_last_location().await,
                 endpoint,
+                location: self.parameters_generator.get_last_location().await,
+                feature_indicators: self.get_feature_indicators(),
             },
             TunnelStateTransition::Connected(endpoint) => TunnelState::Connected {
-                location: self.parameters_generator.get_last_location().await,
                 endpoint,
+                location: self.parameters_generator.get_last_location().await,
+                feature_indicators: self.get_feature_indicators(),
             },
             TunnelStateTransition::Disconnecting(after_disconnect) => {
                 TunnelState::Disconnecting(after_disconnect)
@@ -1041,8 +1043,6 @@ where
 
         self.tunnel_state = tunnel_state.clone();
         self.event_listener.notify_new_state(tunnel_state);
-        self.event_listener
-            .notify_feature_indicators(self.get_feature_indicators());
         self.fetch_am_i_mullvad();
     }
 
@@ -1114,6 +1114,23 @@ where
             .notify_new_state(self.tunnel_state.clone());
     }
 
+    /// Update the set of feature indicators.
+    fn handle_feature_indicator_event(&mut self) {
+        // Note: If the current tunnel state carries information about active feature indicators,
+        // we should care to update the known set of feature indicators (i.e. in the connecting /
+        // connected state). Otherwise, we can just skip broadcasting a new tunnel state.
+        if let Some(current_feature_indicators) = self.tunnel_state.get_feature_indicators() {
+            let new_feature_indicators = self.get_feature_indicators();
+            if *current_feature_indicators != new_feature_indicators {
+                // Make sure to update the daemon's actual tunnel state. Otherwise feature indicator changes won't be persisted.
+                self.tunnel_state
+                    .set_feature_indicators(new_feature_indicators);
+                self.event_listener
+                    .notify_new_state(self.tunnel_state.clone());
+            }
+        }
+    }
+
     fn reset_rpc_sockets_on_tunnel_state_transition(
         &mut self,
         tunnel_state_transition: &TunnelStateTransition,
@@ -1423,11 +1440,6 @@ where
         let _ = tx.send(save_result.map(|_| ()));
     }
 
-    fn handle_settings_changed(&mut self) {
-        self.event_listener
-            .notify_feature_indicators(self.get_feature_indicators())
-    }
-
     async fn on_set_target_state(
         &mut self,
         tx: oneshot::Sender<bool>,
@@ -2783,30 +2795,15 @@ where
         }
     }
 
-    fn get_connected_tunnel_type(&self) -> Option<TunnelType> {
-        if let TunnelState::Connected {
-            endpoint: TunnelEndpoint { tunnel_type, .. },
-            ..
-        } = self.tunnel_state
-        {
-            Some(tunnel_type)
-        } else {
-            None
+    const fn get_connected_tunnel_type(&self) -> Option<TunnelType> {
+        match self.tunnel_state.get_tunnel_type() {
+            Some(tunnel_type) if self.tunnel_state.is_connected() => Some(tunnel_type),
+            Some(_) | None => None,
         }
     }
 
-    fn get_target_tunnel_type(&self) -> Option<TunnelType> {
-        match self.tunnel_state {
-            TunnelState::Connected {
-                endpoint: TunnelEndpoint { tunnel_type, .. },
-                ..
-            }
-            | TunnelState::Connecting {
-                endpoint: TunnelEndpoint { tunnel_type, .. },
-                ..
-            } => Some(tunnel_type),
-            _ => None,
-        }
+    const fn get_target_tunnel_type(&self) -> Option<TunnelType> {
+        self.tunnel_state.get_tunnel_type()
     }
 
     fn send_tunnel_command(&self, command: TunnelCommand) {
@@ -2896,13 +2893,11 @@ where
         };
 
         // use the booleans to filter into a list of only the active features
-        let active_features = generic_features
+        generic_features
             .into_iter()
             .chain(protocol_features)
             .filter_map(|(active, feature)| active.then_some(feature))
-            .collect();
-
-        FeatureIndicators { active_features }
+            .collect()
     }
 }
 

mullvad-daemon/src/management_interface.rs

@@ -1166,15 +1166,6 @@ impl EventListener for ManagementInterfaceEventBroadcaster {
             )),
         })
     }
-
-    fn notify_feature_indicators(&self, features: mullvad_types::features::FeatureIndicators) {
-        log::debug!("Broadcasting new feature indicators");
-        self.notify(types::DaemonEvent {
-            event: Some(daemon_event::Event::FeatureIndicators(
-                types::FeatureIndicators::from(features),
-            )),
-        })
-    }
 }
 
 impl ManagementInterfaceEventBroadcaster {

mullvad-management-interface/proto/management_interface.proto

@@ -203,8 +203,14 @@ message TunnelState {
     GeoIpLocation disconnected_location = 1;
     bool locked_down = 2;
   }
-  message Connecting { TunnelStateRelayInfo relay_info = 1; }
-  message Connected { TunnelStateRelayInfo relay_info = 1; }
+  message Connecting {
+    TunnelStateRelayInfo relay_info = 1;
+    FeatureIndicators feature_indicators = 2;
+  }
+  message Connected {
+    TunnelStateRelayInfo relay_info = 1;
+    FeatureIndicators feature_indicators = 2;
+  }
   message Disconnecting { AfterDisconnect after_disconnect = 1; }
   message Error { ErrorState error_state = 1; }
 
@@ -650,7 +656,6 @@ message DaemonEvent {
     DeviceEvent device = 5;
     RemoveDeviceEvent remove_device = 6;
     AccessMethodSetting new_access_method = 7;
-    FeatureIndicators feature_indicators = 8;
   }
 }
 

mullvad-management-interface/src/client.rs

@@ -8,7 +8,6 @@ use mullvad_types::wireguard::DaitaSettings;
 use mullvad_types::{
     access_method::AccessMethodSetting,
     device::{DeviceEvent, RemoveDeviceEvent},
-    features::FeatureIndicators,
     relay_list::RelayList,
     settings::Settings,
     states::TunnelState,
@@ -21,6 +20,7 @@ use mullvad_types::{
     account::{AccountData, AccountToken, VoucherSubmission},
     custom_list::{CustomList, Id},
     device::{Device, DeviceId, DeviceState},
+    features::FeatureIndicators,
     relay_constraints::{
         BridgeSettings, BridgeState, ObfuscationSettings, RelayOverride, RelaySettings,
     },
@@ -51,7 +51,6 @@ pub enum DaemonEvent {
     Device(DeviceEvent),
     RemoveDevice(RemoveDeviceEvent),
     NewAccessMethod(AccessMethodSetting),
-    FeatureIndicators(FeatureIndicators),
 }
 
 impl TryFrom<types::daemon_event::Event> for DaemonEvent {
@@ -82,9 +81,6 @@ impl TryFrom<types::daemon_event::Event> for DaemonEvent {
                     .map(DaemonEvent::NewAccessMethod)
                     .map_err(Error::InvalidResponse)
             }
-            types::daemon_event::Event::FeatureIndicators(event) => Ok(
-                DaemonEvent::FeatureIndicators(FeatureIndicators::from(event)),
-            ),
         }
     }
 }