move address discovery role from an enum to a simplified struct

Author: divagant-martian

quinn-proto/src/address_discovery.rs

@@ -7,120 +7,48 @@ use crate::VarInt;
 ///
 /// When enabled, this is reported as a transport parameter.
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Default)]
-pub(crate) enum Role {
-    /// Is able to report observer addresses to other peers, but it's not interested in receiving
-    /// reports about its own address.
-    SendOnly,
-    /// Is interested on reports about its own observed address, but will not report back to other
-    /// peers.
-    ReceiveOnly,
-    /// Will both report and receive reports of observed addresses.
-    Both,
-    /// Address discovery is disabled.
-    #[default]
-    Disabled,
+pub(crate) struct Role {
+    pub(crate) send_reports: bool,
+    pub(crate) receive_reports: bool,
 }
 
 impl TryFrom<VarInt> for Role {
     type Error = crate::transport_parameters::Error;
 
     fn try_from(value: VarInt) -> Result<Self, Self::Error> {
+        let mut role = Self::default();
         match value.0 {
-            0 => Ok(Self::SendOnly),
-            1 => Ok(Self::ReceiveOnly),
-            2 => Ok(Self::Both),
-            _ => Err(crate::transport_parameters::Error::IllegalValue),
+            0 => role.send_reports = true,
+            1 => role.receive_reports = true,
+            2 => {
+                role.send_reports = true;
+                role.receive_reports = true;
+            }
+            _ => return Err(crate::transport_parameters::Error::IllegalValue),
         }
+
+        Ok(role)
     }
 }
 
 impl Role {
     /// Whether address discovery is disabled.
     pub(crate) fn is_disabled(&self) -> bool {
-        matches!(self, Self::Disabled)
-    }
-
-    /// Whether this peer's role allows for address reporting to other peers.
-    fn is_reporter(&self) -> bool {
-        matches!(self, Self::SendOnly | Self::Both)
-    }
-
-    /// Whether this peer's role accepts observed address reports.
-    fn receives_reports(&self) -> bool {
-        matches!(self, Self::ReceiveOnly | Self::Both)
+        !self.receive_reports && !self.send_reports
     }
 
     /// Whether this peer should report observed addresses to the other peer.
     pub(crate) fn should_report(&self, other: &Self) -> bool {
-        self.is_reporter() && other.receives_reports()
-    }
-
-    /// Sets whether this peer should provide observed addresses to other peers.
-    pub(crate) fn send_reports_to_peers(&mut self, provide: bool) {
-        if provide {
-            self.enable_sending_reports_to_peers()
-        } else {
-            self.disable_sending_reports_to_peers()
-        }
-    }
-
-    /// Enables sending reports of observed addresses to other peers.
-    fn enable_sending_reports_to_peers(&mut self) {
-        match self {
-            Self::SendOnly => {} // already enabled
-            Self::ReceiveOnly => *self = Self::Both,
-            Self::Both => {} // already enabled
-            Self::Disabled => *self = Self::SendOnly,
-        }
-    }
-
-    /// Disables sending reports of observed addresses to other peers.
-    fn disable_sending_reports_to_peers(&mut self) {
-        match self {
-            Self::SendOnly => *self = Self::Disabled,
-            Self::ReceiveOnly => {} // already disabled
-            Self::Both => *self = Self::ReceiveOnly,
-            Self::Disabled => {} // already disabled
-        }
-    }
-
-    /// Sets whether this peer should accept received reports of observed addresses from other
-    /// peers.
-    pub(crate) fn receive_reports_from_peers(&mut self, receive: bool) {
-        if receive {
-            self.enable_receiving_reports_from_peers()
-        } else {
-            self.disable_receiving_reports_from_peers()
-        }
-    }
-
-    /// Enables receiving reports of observed addresses from other peers.
-    fn enable_receiving_reports_from_peers(&mut self) {
-        match self {
-            Self::SendOnly => *self = Self::Both,
-            Self::ReceiveOnly => {} // already enabled
-            Self::Both => {}        // already enabled
-            Self::Disabled => *self = Self::ReceiveOnly,
-        }
-    }
-
-    /// Disables receiving reports of observed addresses from other peers.
-    fn disable_receiving_reports_from_peers(&mut self) {
-        match self {
-            Self::SendOnly => {} // already disabled
-            Self::ReceiveOnly => *self = Self::Disabled,
-            Self::Both => *self = Self::SendOnly,
-            Self::Disabled => {} // already disabled
-        }
+        self.send_reports && other.receive_reports
     }
 
     /// Gives the [`VarInt`] representing this [`Role`] as a transport parameter.
     pub(crate) fn as_transport_parameter(&self) -> Option<VarInt> {
-        match self {
-            Self::SendOnly => Some(VarInt(0)),
-            Self::ReceiveOnly => Some(VarInt(1)),
-            Self::Both => Some(VarInt(2)),
-            Self::Disabled => None,
+        match (self.send_reports, self.receive_reports) {
+            (true, true) => Some(VarInt(2)),
+            (true, false) => Some(VarInt(0)),
+            (false, true) => Some(VarInt(1)),
+            (false, false) => None,
         }
     }
 }

quinn-proto/src/config/transport.rs

@@ -325,7 +325,7 @@ impl TransportConfig {
     /// This will aid peers in inferring their reachable address, which in most NATd networks
     /// will not be easily available to them.
     pub fn send_observed_address_reports(&mut self, enabled: bool) -> &mut Self {
-        self.address_discovery_role.send_reports_to_peers(enabled);
+        self.address_discovery_role.send_reports = enabled;
         self
     }
 
@@ -336,8 +336,7 @@ impl TransportConfig {
     /// reports cannot be trusted. This, however, can aid the current endpoint in inferring its
     /// reachable address, which in most NATd networks will not be easily available.
     pub fn receive_observed_address_reports(&mut self, enabled: bool) -> &mut Self {
-        self.address_discovery_role
-            .receive_reports_from_peers(enabled);
+        self.address_discovery_role.receive_reports = enabled;
         self
     }
 }

quinn-proto/src/tests/mod.rs

@@ -3191,15 +3191,21 @@ fn address_discovery() {
 
     let server = ServerConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..server_config()
     };
     let mut pair = Pair::new(Default::default(), server);
     let client_config = ClientConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..client_config()
@@ -3236,7 +3242,10 @@ fn address_discovery_zero_rtt_accepted() {
     let _guard = subscribe();
     let server = ServerConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..server_config()
@@ -3246,16 +3255,16 @@ fn address_discovery_zero_rtt_accepted() {
     pair.server.incoming_connection_behavior = IncomingConnectionBehavior::Validate;
     let client_cfg = ClientConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..client_config()
     };
     let alt_client_cfg = ClientConfig {
-        transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Disabled,
-            ..TransportConfig::default()
-        }),
+        transport: Arc::new(TransportConfig::default()),
         ..client_cfg.clone()
     };
 
@@ -3317,23 +3326,26 @@ fn address_discovery_zero_rtt_accepted() {
 fn address_discovery_zero_rtt_rejection() {
     let _guard = subscribe();
     let server_cfg = ServerConfig {
-        transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Disabled,
-            ..TransportConfig::default()
-        }),
+        transport: Default::default(),
         ..server_config()
     };
     let alt_server_cfg = ServerConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::SendOnly,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                ..Default::default()
+            },
             ..TransportConfig::default()
         }),
         ..server_cfg.clone()
     };
     let mut pair = Pair::new(Default::default(), server_cfg);
     let client_cfg = ClientConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..client_config()
@@ -3387,15 +3399,21 @@ fn address_discovery_retransmission() {
 
     let server = ServerConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..server_config()
     };
     let mut pair = Pair::new(Default::default(), server);
     let client_config = ClientConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..client_config()
@@ -3423,15 +3441,21 @@ fn address_discovery_rebind_retransmission() {
 
     let server = ServerConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..server_config()
     };
     let mut pair = Pair::new(Default::default(), server);
     let client_config = ClientConfig {
         transport: Arc::new(TransportConfig {
-            address_discovery_role: crate::address_discovery::Role::Both,
+            address_discovery_role: crate::address_discovery::Role {
+                send_reports: true,
+                receive_reports: true,
+            },
             ..TransportConfig::default()
         }),
         ..client_config()

quinn-proto/src/transport_parameters.rs

@@ -138,7 +138,7 @@ macro_rules! make_struct {
                     grease_transport_parameter: None,
                     write_order: None,
 
-                    address_discovery_role: address_discovery::Role::Disabled,
+                    address_discovery_role: address_discovery::Role::default(),
                 }
             }
         }
@@ -778,7 +778,10 @@ mod test {
             }),
             grease_quic_bit: true,
             min_ack_delay: Some(2_000u32.into()),
-            address_discovery_role: address_discovery::Role::SendOnly,
+            address_discovery_role: address_discovery::Role {
+                send_reports: true,
+                ..Default::default()
+            },
             ..TransportParameters::default()
         };
         params.write(&mut buf);