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

@@ -93,11 +93,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

@@ -19,7 +19,11 @@ 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)
@@ -30,7 +34,11 @@ pub fn print_state(state: &TunnelState, verbose: bool) {
                 }
             }
         }
-        Connecting { endpoint, location } => {
+        Connecting {
+            endpoint,
+            location,
+            feature_indicators: _,
+        } => {
             let ellipsis = if !verbose { "..." } else { "" };
             println!(
                 "Connecting to {}{ellipsis}",

mullvad-daemon/src/lib.rs

@@ -397,9 +397,6 @@ pub(crate) enum InternalDaemonEvent {
     /// 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"))]
@@ -571,9 +568,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> {
@@ -751,11 +745,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 {
@@ -961,7 +950,6 @@ where
             LocationEvent(location_data) => self.handle_location_event(location_data),
             #[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
     }
@@ -980,12 +968,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)
@@ -1039,8 +1029,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();
     }
 
@@ -1420,11 +1408,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>,
@@ -2893,13 +2876,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

@@ -1158,15 +1158,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

@@ -202,8 +202,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; }
 
@@ -649,7 +655,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)),
-            ),
         }
     }
 }

mullvad-management-interface/src/types/conversions/features.rs

@@ -43,12 +43,10 @@ impl From<proto::FeatureIndicator> for mullvad_types::features::FeatureIndicator
 
 impl From<proto::FeatureIndicators> for mullvad_types::features::FeatureIndicators {
     fn from(features: proto::FeatureIndicators) -> Self {
-        Self {
-            active_features: features
-                .active_features()
-                .map(mullvad_types::features::FeatureIndicator::from)
-                .collect(),
-        }
+        features
+            .active_features()
+            .map(mullvad_types::features::FeatureIndicator::from)
+            .collect()
     }
 }
 
@@ -57,8 +55,7 @@ impl From<mullvad_types::features::FeatureIndicators> for proto::FeatureIndicato
         let mut proto_features = Self::default();
 
         features
-            .active_features
-            .into_iter()
+            .active_features()
             .map(proto::FeatureIndicator::from)
             .for_each(|feature| proto_features.push_active_features(feature));
 

mullvad-management-interface/src/types/conversions/states.rs

@@ -39,22 +39,28 @@ impl From<mullvad_types::states::TunnelState> for proto::TunnelState {
                 disconnected_location: disconnected_location.map(proto::GeoIpLocation::from),
                 locked_down,
             }),
-            MullvadTunnelState::Connecting { endpoint, location } => {
-                proto::tunnel_state::State::Connecting(proto::tunnel_state::Connecting {
-                    relay_info: Some(proto::TunnelStateRelayInfo {
-                        tunnel_endpoint: Some(proto::TunnelEndpoint::from(endpoint)),
-                        location: location.map(proto::GeoIpLocation::from),
-                    }),
-                })
-            }
-            MullvadTunnelState::Connected { endpoint, location } => {
-                proto::tunnel_state::State::Connected(proto::tunnel_state::Connected {
-                    relay_info: Some(proto::TunnelStateRelayInfo {
-                        tunnel_endpoint: Some(proto::TunnelEndpoint::from(endpoint)),
-                        location: location.map(proto::GeoIpLocation::from),
-                    }),
-                })
-            }
+            MullvadTunnelState::Connecting {
+                endpoint,
+                location,
+                feature_indicators: features_indicators,
+            } => proto::tunnel_state::State::Connecting(proto::tunnel_state::Connecting {
+                relay_info: Some(proto::TunnelStateRelayInfo {
+                    tunnel_endpoint: Some(proto::TunnelEndpoint::from(endpoint)),
+                    location: location.map(proto::GeoIpLocation::from),
+                }),
+                feature_indicators: Some(proto::FeatureIndicators::from(features_indicators)),
+            }),
+            MullvadTunnelState::Connected {
+                endpoint,
+                location,
+                feature_indicators: features_indicators,
+            } => proto::tunnel_state::State::Connected(proto::tunnel_state::Connected {
+                relay_info: Some(proto::TunnelStateRelayInfo {
+                    tunnel_endpoint: Some(proto::TunnelEndpoint::from(endpoint)),
+                    location: location.map(proto::GeoIpLocation::from),
+                }),
+                feature_indicators: Some(proto::FeatureIndicators::from(features_indicators)),
+            }),
             MullvadTunnelState::Disconnecting(after_disconnect) => {
                 proto::tunnel_state::State::Disconnecting(proto::tunnel_state::Disconnecting {
                     after_disconnect: match after_disconnect {
@@ -229,23 +235,36 @@ impl TryFrom<proto::TunnelState> for mullvad_types::states::TunnelState {
                         tunnel_endpoint: Some(tunnel_endpoint),
                         location,
                     }),
+                feature_indicators,
             })) => MullvadState::Connecting {
                 endpoint: talpid_net::TunnelEndpoint::try_from(tunnel_endpoint)?,
                 location: location
                     .map(mullvad_types::location::GeoIpLocation::try_from)
                     .transpose()?,
+                feature_indicators: feature_indicators
+                    .map(mullvad_types::features::FeatureIndicators::from)
+                    .ok_or(FromProtobufTypeError::InvalidArgument(
+                        "Missing feature indicators",
+                    ))?,
             },
             Some(proto::tunnel_state::State::Connected(proto::tunnel_state::Connected {
                 relay_info:
                     Some(proto::TunnelStateRelayInfo {
                         tunnel_endpoint: Some(tunnel_endpoint),
                         location,
                     }),
+                feature_indicators,
             })) => MullvadState::Connected {
                 endpoint: talpid_net::TunnelEndpoint::try_from(tunnel_endpoint)?,
                 location: location
                     .map(mullvad_types::location::GeoIpLocation::try_from)
                     .transpose()?,
+
+                feature_indicators: feature_indicators
+                    .map(mullvad_types::features::FeatureIndicators::from)
+                    .ok_or(FromProtobufTypeError::InvalidArgument(
+                        "Missing feature indicators",
+                    ))?,
             },
             Some(proto::tunnel_state::State::Disconnecting(
                 proto::tunnel_state::Disconnecting { after_disconnect },

mullvad-types/src/features.rs

@@ -7,8 +7,18 @@ use serde::{Deserialize, Serialize};
 ///
 /// Note that the feature indicators are not ordered.
 #[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-pub struct FeatureIndicators {
-    pub active_features: HashSet<FeatureIndicator>,
+pub struct FeatureIndicators(HashSet<FeatureIndicator>);
+
+impl FeatureIndicators {
+    pub fn active_features(&self) -> impl Iterator<Item = FeatureIndicator> {
+        self.0.clone().into_iter()
+    }
+}
+
+impl FromIterator<FeatureIndicator> for FeatureIndicators {
+    fn from_iter<T: IntoIterator<Item = FeatureIndicator>>(iter: T) -> Self {
+        Self(iter.into_iter().collect())
+    }
 }
 
 /// All possible feature indicators. These represent a subset of all VPN settings in a

mullvad-types/src/states.rs

@@ -1,4 +1,4 @@
-use crate::location::GeoIpLocation;
+use crate::{features::FeatureIndicators, location::GeoIpLocation};
 use serde::{Deserialize, Serialize};
 use std::fmt;
 use talpid_types::{
@@ -38,10 +38,12 @@ pub enum TunnelState {
     Connecting {
         endpoint: TunnelEndpoint,
         location: Option<GeoIpLocation>,
+        feature_indicators: FeatureIndicators,
     },
     Connected {
         endpoint: TunnelEndpoint,
         location: Option<GeoIpLocation>,
+        feature_indicators: FeatureIndicators,
     },
     Disconnecting(ActionAfterDisconnect),
     Error(ErrorState),