Skip to content

Commit 7561d18

Browse files
Fixup comments
1 parent b88ac2f commit 7561d18

File tree

5 files changed

+149
-19
lines changed

5 files changed

+149
-19
lines changed

Diff for: mullvad-relay-selector/src/relay_selector/detailer.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
//! OpenVPN relay chosen by the relay selector.
33
//!
44
//! [`MullvadEndpoint`] contains all the necessary information for establishing a connection
5-
//! between the client and Mullvad VPN. It is the daemon's responsibillity of actually establishing
6-
//! this connection.
5+
//! between the client and Mullvad VPN. It is the daemon's responsibility to establish this
6+
//! connection.
77
//!
8-
//! TODO(markus): Revise comments
8+
//! [`MullvadEndpoint`]: mullvad_types::endpoint::MullvadEndpoint
99
1010
use std::net::{IpAddr, SocketAddr, SocketAddrV4};
1111

Diff for: mullvad-relay-selector/src/relay_selector/helpers.rs

+12
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,20 @@ pub fn pick_random_relay_fn<RelayType>(
4545
if total_weight == 0 {
4646
relays.choose(&mut rng)
4747
} else {
48+
// Assign each relay a subset of the range 0..total_weight with size equal to its weight.
4849
// Pick a random number in the range 1..=total_weight. This choses the relay with a
4950
// non-zero weight.
51+
//
52+
// rng(1..=total_weight)
53+
// |
54+
// v
55+
// _____________________________i___________________________________________________
56+
// 0|_____________|__________________________|___________|_____|___________|__________| total_weight
57+
// ^ ^ ^ ^ ^
58+
// | | | | |
59+
// ------------------------------------------ ------------
60+
// | | |
61+
// weight(relay 0) weight(relay 1) .. .. .. weight(relay n)
5062
let mut i: u64 = rng.gen_range(1..=total_weight);
5163
Some(
5264
relays

Diff for: mullvad-relay-selector/src/relay_selector/mod.rs

+10-2
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,8 @@ impl RelaySelector {
535535
/// * An `Err` if no entry relay can be chosen (if multihop is enabled on `query`)
536536
/// * an `Err` if no [`MullvadEndpoint`] can be derived from the selected relay(s).
537537
/// * `Ok(GetRelay::Wireguard)` otherwise
538+
///
539+
/// [`MullvadEndpoint`]: mullvad_types::endpoint::MullvadEndpoint
538540
fn get_wireguard_relay(
539541
query: &RelayQuery,
540542
config: &NormalSelectorConfig<'_>,
@@ -623,7 +625,9 @@ impl RelaySelector {
623625
})
624626
}
625627

626-
/// Constructs a `MullvadEndpoint` with details for how to connect to `relay`.
628+
/// Constructs a [`MullvadEndpoint`] with details for how to connect to `relay`.
629+
///
630+
/// [`MullvadEndpoint`]: mullvad_types::endpoint::MullvadEndpoint
627631
fn get_wireguard_endpoint(
628632
query: &RelayQuery,
629633
parsed_relays: &ParsedRelays,
@@ -671,6 +675,8 @@ impl RelaySelector {
671675
/// * An `Err` if no entry bridge can be chosen (if bridge mode is enabled on `query`)
672676
/// * an `Err` if no [`MullvadEndpoint`] can be derived from the selected relay
673677
/// * `Ok(GetRelay::OpenVpn)` otherwise
678+
///
679+
/// [`MullvadEndpoint`]: mullvad_types::endpoint::MullvadEndpoint
674680
#[cfg(not(target_os = "android"))]
675681
fn get_openvpn_relay(
676682
query: &RelayQuery,
@@ -689,7 +695,9 @@ impl RelaySelector {
689695
})
690696
}
691697

692-
/// Constructs a `MullvadEndpoint` with details for how to connect to `relay`.
698+
/// Constructs a [`MullvadEndpoint`] with details for how to connect to `relay`.
699+
///
700+
/// [`MullvadEndpoint`]: mullvad_types::endpoint::MullvadEndpoint
693701
#[cfg(not(target_os = "android"))]
694702
fn get_openvpn_endpoint(
695703
query: &RelayQuery,

Diff for: mullvad-relay-selector/src/relay_selector/query.rs

+87-11
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,20 @@ pub struct RelayQuery {
7979
}
8080

8181
impl RelayQuery {
82-
/// Create a new [`RelayQuery`] with no opinionated defaults. This
83-
/// should be the const equivalent to [`Default::default`].
82+
/// Create a new [`RelayQuery`] with no opinionated defaults. This query matches every relay
83+
/// with any configuration by setting each of its fields to [`Constraint::Any`]. Should be the
84+
/// const equivalent to [`Default::default`].
85+
///
86+
/// Note that the following identity applies for any `other_query`:
87+
/// ```rust
88+
/// # use mullvad_relay_selector::query::RelayQuery;
89+
/// # use crate::mullvad_relay_selector::query::Intersection;
90+
///
91+
/// # let other_query = RelayQuery::new();
92+
/// assert_eq!(RelayQuery::new().intersection(other_query.clone()), Some(other_query));
93+
/// # let other_query = RelayQuery::new();
94+
/// assert_eq!(other_query.clone().intersection(RelayQuery::new()), Some(other_query));
95+
/// ```
8496
pub const fn new() -> RelayQuery {
8597
RelayQuery {
8698
location: Constraint::Any,
@@ -94,20 +106,84 @@ impl RelayQuery {
94106
}
95107

96108
impl Intersection for RelayQuery {
97-
/// `intersection` defines a cautious merge strategy between two
98-
/// [`RelayQuery`].
109+
/// Return a new [`RelayQuery`] which matches the intersected queries.
99110
///
100-
/// * If two [`RelayQuery`] differ in any configuration such that no
101-
/// consensus can be reached, the two [`RelayQuery`] are said to be
102-
/// incompatible and `intersection` returns [`Option::None`].
111+
/// * If two [`RelayQuery`]s differ such that no relay matches both, [`Option::None`] is returned:
112+
/// ```rust
113+
/// # use mullvad_relay_selector::query::builder::RelayQueryBuilder;
114+
/// # use crate::mullvad_relay_selector::query::Intersection;
115+
/// let query_a = RelayQueryBuilder::new().wireguard().build();
116+
/// let query_b = RelayQueryBuilder::new().openvpn().build();
117+
/// assert_eq!(query_a.intersection(query_b), None);
118+
/// ```
103119
///
104120
/// * Otherwise, a new [`RelayQuery`] is returned where each constraint is
105121
/// as specific as possible. See [`Constraint`] for further details.
122+
/// ```rust
123+
/// # use crate::mullvad_relay_selector::*;
124+
/// # use crate::mullvad_relay_selector::query::*;
125+
/// # use crate::mullvad_relay_selector::query::builder::*;
126+
/// # use mullvad_types::relay_list::*;
127+
/// # use talpid_types::net::wireguard::PublicKey;
128+
///
129+
/// // The relay list used by `relay_selector` in this example
130+
/// let relay_list = RelayList {
131+
/// # etag: None,
132+
/// # openvpn: OpenVpnEndpointData { ports: vec![] },
133+
/// # bridge: BridgeEndpointData {
134+
/// # shadowsocks: vec![],
135+
/// # },
136+
/// # wireguard: WireguardEndpointData {
137+
/// # port_ranges: vec![(53, 53), (4000, 33433), (33565, 51820), (52000, 60000)],
138+
/// # ipv4_gateway: "10.64.0.1".parse().unwrap(),
139+
/// # ipv6_gateway: "fc00:bbbb:bbbb:bb01::1".parse().unwrap(),
140+
/// # udp2tcp_ports: vec![],
141+
/// # },
142+
/// countries: vec![RelayListCountry {
143+
/// name: "Sweden".to_string(),
144+
/// # code: "Sweden".to_string(),
145+
/// cities: vec![RelayListCity {
146+
/// name: "Gothenburg".to_string(),
147+
/// # code: "Gothenburg".to_string(),
148+
/// # latitude: 57.70887,
149+
/// # longitude: 11.97456,
150+
/// relays: vec![Relay {
151+
/// hostname: "se9-wireguard".to_string(),
152+
/// ipv4_addr_in: "185.213.154.68".parse().unwrap(),
153+
/// # ipv6_addr_in: Some("2a03:1b20:5:f011::a09f".parse().unwrap()),
154+
/// # include_in_country: false,
155+
/// # active: true,
156+
/// # owned: true,
157+
/// # provider: "31173".to_string(),
158+
/// # weight: 1,
159+
/// # endpoint_data: RelayEndpointData::Wireguard(WireguardRelayEndpointData {
160+
/// # public_key: PublicKey::from_base64(
161+
/// # "BLNHNoGO88LjV/wDBa7CUUwUzPq/fO2UwcGLy56hKy4=",
162+
/// # )
163+
/// # .unwrap(),
164+
/// # }),
165+
/// # location: None,
166+
/// }],
167+
/// }],
168+
/// }],
169+
/// };
170+
///
171+
/// # let relay_selector = RelaySelector::from_list(SelectorConfig::default(), relay_list.clone());
172+
/// # let city = |country, city| GeographicLocationConstraint::city(country, city);
173+
///
174+
/// let query_a = RelayQueryBuilder::new().wireguard().build();
175+
/// let query_b = RelayQueryBuilder::new().location(city("Sweden", "Gothenburg")).build();
176+
///
177+
/// let result = relay_selector.get_relay_by_query(query_a.intersection(query_b).unwrap());
178+
/// assert!(result.is_ok());
179+
/// ```
180+
///
181+
/// This way, if the mullvad app wants to check if the user's relay settings
182+
/// are compatible with any other [`RelayQuery`], for examples those defined by
183+
/// [`RETRY_ORDER`] , taking the intersection between them will never result in
184+
/// a situation where the app can override the user's preferences.
106185
///
107-
/// This way, if the mullvad app wants to check if the user's configured
108-
/// [`RelayQuery`] are compatible with any other [`RelayQuery`], taking the
109-
/// intersection between them will never result in a situation where the app
110-
/// can override the user's preferences.
186+
/// [`RETRY_ORDER`]: crate::RETRY_ORDER
111187
fn intersection(self, other: Self) -> Option<Self>
112188
where
113189
Self: PartialEq,

Diff for: mullvad-types/src/relay_list.rs

+37-3
Original file line numberDiff line numberDiff line change
@@ -107,16 +107,50 @@ pub struct Relay {
107107
}
108108

109109
impl PartialEq for Relay {
110+
/// Hostnames are assumed to be unique per relay, i.e. a relay can be uniquely identified by its hostname.
111+
///
112+
/// # Example
113+
///
114+
/// ```rust
115+
/// # use mullvad_types::{relay_list::Relay, relay_list::{RelayEndpointData, WireguardRelayEndpointData}};
116+
/// # use talpid_types::net::wireguard::PublicKey;
117+
///
118+
/// let relay = Relay {
119+
/// hostname: "se9-wireguard".to_string(),
120+
/// ipv4_addr_in: "185.213.154.68".parse().unwrap(),
121+
/// # ipv6_addr_in: None,
122+
/// # include_in_country: true,
123+
/// # active: true,
124+
/// # owned: true,
125+
/// # provider: "provider0".to_string(),
126+
/// # weight: 1,
127+
/// # endpoint_data: RelayEndpointData::Wireguard(WireguardRelayEndpointData {
128+
/// # public_key: PublicKey::from_base64(
129+
/// # "BLNHNoGO88LjV/wDBa7CUUwUzPq/fO2UwcGLy56hKy4=",
130+
/// # )
131+
/// # .unwrap(),
132+
/// # }),
133+
/// # location: None,
134+
/// };
135+
///
136+
/// let mut different_relay = relay.clone();
137+
/// // Modify the relay's IPv4 address - should not matter for the equality check
138+
/// different_relay.ipv4_addr_in = "1.3.3.7".parse().unwrap();
139+
/// assert_eq!(relay, different_relay);
140+
///
141+
/// // What matter's for the equality check is the hostname of the relay
142+
/// different_relay.hostname = "dk-cph-wg-001".to_string();
143+
/// assert_ne!(relay, different_relay);
144+
/// ```
110145
fn eq(&self, other: &Self) -> bool {
111-
// Hostnames are assumed to be unique per relay, i.e. a relay can be uniquely identified by its hostname.
112146
self.hostname == other.hostname
113147
}
114148
}
115149

116-
// See uniqueness argument in the implementation of [`PartialEq`] for [`Relay`].
150+
/// Hostnames are assumed to be unique per relay, i.e. a relay can be uniquely identified by its hostname.
117151
impl Eq for Relay {}
118152

119-
// See uniqueness argument in the implementation of [`PartialEq`] for [`Relay`].
153+
/// Hostnames are assumed to be unique per relay, i.e. a relay can be uniquely identified by its hostname.
120154
impl std::hash::Hash for Relay {
121155
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
122156
self.hostname.hash(state)

0 commit comments

Comments
 (0)