Don't plumb current time through endpoint events

This was only needed to determine the base time for the next
CID retirement timer, for which the marginal delay caused by fetching
a new `Instant::now()` on demand is harmless.

Author: Ralith

quinn-proto/src/connection/mod.rs

@@ -956,7 +956,7 @@ impl Connection {
     /// Will execute protocol logic upon receipt of a connection event, in turn preparing signals
     /// (including application `Event`s, `EndpointEvent`s and outgoing datagrams) that should be
     /// extracted through the relevant methods.
-    pub fn handle_event(&mut self, event: ConnectionEvent) {
+    pub fn handle_event(&mut self, event: ConnectionEvent, now: Instant) {
         use self::ConnectionEventInner::*;
         match event.0 {
             Datagram {
@@ -1001,7 +1001,7 @@ impl Connection {
                     self.set_loss_detection_timer(now);
                 }
             }
-            NewIdentifiers(ids, now) => {
+            NewIdentifiers(ids) => {
                 self.local_cid_state.new_cids(&ids, now);
                 ids.into_iter().rev().for_each(|frame| {
                     self.spaces[SpaceId::Data].pending.new_cids.push(frame);
@@ -1071,7 +1071,7 @@ impl Connection {
                             self.local_cid_state.retire_prior_to()
                         );
                         self.endpoint_events
-                            .push_back(EndpointEventInner::NeedIdentifiers(now, num_new_cid));
+                            .push_back(EndpointEventInner::NeedIdentifiers(num_new_cid));
                     }
                 }
                 Timer::MaxAckDelay => {
@@ -2354,7 +2354,7 @@ impl Connection {
                             .push_back(EndpointEventInner::ResetToken(self.path.remote, token));
                     }
                     self.handle_peer_params(params)?;
-                    self.issue_first_cids(now);
+                    self.issue_first_cids();
                 } else {
                     // Server-only
                     self.spaces[SpaceId::Data].pending.handshake_done = true;
@@ -2401,7 +2401,7 @@ impl Connection {
                                 reason: "transport parameters missing".into(),
                             })?;
                     self.handle_peer_params(params)?;
-                    self.issue_first_cids(now);
+                    self.issue_first_cids();
                     self.init_0rtt();
                 }
                 Ok(())
@@ -2662,7 +2662,6 @@ impl Connection {
                         .on_cid_retirement(sequence, self.peer_params.issue_cids_limit())?;
                     self.endpoint_events
                         .push_back(EndpointEventInner::RetireConnectionId(
-                            now,
                             sequence,
                             allow_more_cids,
                         ));
@@ -2889,15 +2888,15 @@ impl Connection {
     }
 
     /// Issue an initial set of connection IDs to the peer upon connection
-    fn issue_first_cids(&mut self, now: Instant) {
+    fn issue_first_cids(&mut self) {
         if self.local_cid_state.cid_len() == 0 {
             return;
         }
 
         // Subtract 1 to account for the CID we supplied while handshaking
         let n = self.peer_params.issue_cids_limit() - 1;
         self.endpoint_events
-            .push_back(EndpointEventInner::NeedIdentifiers(now, n));
+            .push_back(EndpointEventInner::NeedIdentifiers(n));
     }
 
     fn populate_packet(
@@ -3372,10 +3371,10 @@ impl Connection {
     /// Instruct the peer to replace previously issued CIDs by sending a NEW_CONNECTION_ID frame
     /// with updated `retire_prior_to` field set to `v`
     #[cfg(test)]
-    pub(crate) fn rotate_local_cid(&mut self, v: u64, now: Instant) {
+    pub(crate) fn rotate_local_cid(&mut self, v: u64) {
         let n = self.local_cid_state.assign_retire_seq(v);
         self.endpoint_events
-            .push_back(EndpointEventInner::NeedIdentifiers(now, n));
+            .push_back(EndpointEventInner::NeedIdentifiers(n));
     }
 
     /// Check the current active remote CID sequence

quinn-proto/src/endpoint.rs

@@ -85,8 +85,8 @@ impl Endpoint {
     ) -> Option<ConnectionEvent> {
         use EndpointEventInner::*;
         match event.0 {
-            NeedIdentifiers(now, n) => {
-                return Some(self.send_new_identifiers(now, ch, n));
+            NeedIdentifiers(n) => {
+                return Some(self.send_new_identifiers(ch, n));
             }
             ResetToken(remote, token) => {
                 if let Some(old) = self.connections[ch].reset_token.replace((remote, token)) {
@@ -96,12 +96,12 @@ impl Endpoint {
                     warn!("duplicate reset token");
                 }
             }
-            RetireConnectionId(now, seq, allow_more_cids) => {
+            RetireConnectionId(seq, allow_more_cids) => {
                 if let Some(cid) = self.connections[ch].loc_cids.remove(&seq) {
                     trace!("peer retired CID {}: {}", seq, cid);
                     self.index.retire(&cid);
                     if allow_more_cids {
-                        return Some(self.send_new_identifiers(now, ch, 1));
+                        return Some(self.send_new_identifiers(ch, 1));
                     }
                 }
             }
@@ -362,12 +362,7 @@ impl Endpoint {
         Ok((ch, conn))
     }
 
-    fn send_new_identifiers(
-        &mut self,
-        now: Instant,
-        ch: ConnectionHandle,
-        num: u64,
-    ) -> ConnectionEvent {
+    fn send_new_identifiers(&mut self, ch: ConnectionHandle, num: u64) -> ConnectionEvent {
         let mut ids = vec![];
         for _ in 0..num {
             let id = self.new_cid(ch);
@@ -381,7 +376,7 @@ impl Endpoint {
                 reset_token: ResetToken::new(&*self.config.reset_key, &id),
             });
         }
-        ConnectionEvent(ConnectionEventInner::NewIdentifiers(ids, now))
+        ConnectionEvent(ConnectionEventInner::NewIdentifiers(ids))
     }
 
     /// Generate a connection ID for `ch`

quinn-proto/src/shared.rs

@@ -19,7 +19,7 @@ pub(crate) enum ConnectionEventInner {
         remaining: Option<BytesMut>,
     },
     /// New connection identifiers have been issued for the Connection
-    NewIdentifiers(Vec<IssuedCid>, Instant),
+    NewIdentifiers(Vec<IssuedCid>),
 }
 
 /// Events sent from a Connection to an Endpoint
@@ -50,10 +50,10 @@ pub(crate) enum EndpointEventInner {
     /// The reset token and/or address eligible for generating resets has been updated
     ResetToken(SocketAddr, ResetToken),
     /// The connection needs connection identifiers
-    NeedIdentifiers(Instant, u64),
+    NeedIdentifiers(u64),
     /// Stop routing connection ID for this sequence number to the connection
     /// When `bool == true`, a new connection ID will be issued to peer
-    RetireConnectionId(Instant, u64, bool),
+    RetireConnectionId(u64, bool),
 }
 
 /// Protocol-level identifier for a connection.

quinn-proto/src/tests/mod.rs

@@ -91,7 +91,7 @@ fn version_negotiate_client() {
         &mut buf,
     );
     if let Some(DatagramEvent::ConnectionEvent(_, event)) = opt_event {
-        client_ch.handle_event(event);
+        client_ch.handle_event(event, now);
     }
     assert_matches!(
         client_ch.poll(),
@@ -1406,8 +1406,7 @@ fn cid_retirement() {
     let (client_ch, server_ch) = pair.connect();
 
     // Server retires current active remote CIDs
-    pair.server_conn_mut(server_ch)
-        .rotate_local_cid(1, Instant::now());
+    pair.server_conn_mut(server_ch).rotate_local_cid(1);
     pair.drive();
     // Any unexpected behavior may trigger TransportError::CONNECTION_ID_LIMIT_ERROR
     assert!(!pair.client_conn_mut(client_ch).is_closed());
@@ -1423,7 +1422,7 @@ fn cid_retirement() {
     pair.client_conn_mut(client_ch).ping();
     // Server retires all valid remote CIDs
     pair.server_conn_mut(server_ch)
-        .rotate_local_cid(next_retire_prior_to, Instant::now());
+        .rotate_local_cid(next_retire_prior_to);
     pair.drive();
     assert!(!pair.client_conn_mut(client_ch).is_closed());
     assert!(!pair.server_conn_mut(server_ch).is_closed());

quinn-proto/src/tests/util.rs

@@ -371,7 +371,7 @@ impl TestEndpoint {
 
                 for (_, mut events) in self.conn_events.drain() {
                     for event in events.drain(..) {
-                        conn.handle_event(event);
+                        conn.handle_event(event, now);
                     }
                 }
 
@@ -393,7 +393,7 @@ impl TestEndpoint {
             for (ch, event) in endpoint_events {
                 if let Some(event) = self.handle_event(ch, event) {
                     if let Some(conn) = self.connections.get_mut(&ch) {
-                        conn.handle_event(event);
+                        conn.handle_event(event, now);
                     }
                 }
             }

quinn/src/connection.rs

@@ -913,13 +913,14 @@ impl State {
         shared: &Shared,
         cx: &mut Context,
     ) -> Result<(), ConnectionError> {
+        let now = Instant::now();
         loop {
             match self.conn_events.poll_recv(cx) {
                 Poll::Ready(Some(ConnectionEvent::Ping)) => {
                     self.inner.ping();
                 }
                 Poll::Ready(Some(ConnectionEvent::Proto(event))) => {
-                    self.inner.handle_event(event);
+                    self.inner.handle_event(event, now);
                 }
                 Poll::Ready(Some(ConnectionEvent::Close { reason, error_code })) => {
                     self.close(error_code, reason, shared);