Cleanup

Author: ImplOfAnImpl

p2p/src/net/default_backend/peer.rs

@@ -424,8 +424,6 @@ where
                     .await?
             }
             CategorizedMessage::BlockSyncMessage(msg) => {
-                block_sync_msg_sender.send(msg).await?;
-
                 if !self.sync_message_received {
                     self.peer_event_sender
                         .send(PeerEvent::MessageReceived(
@@ -434,10 +432,10 @@ where
                         .await?;
                     self.sync_message_received = true;
                 }
+
+                block_sync_msg_sender.send(msg).await?;
             }
             CategorizedMessage::TransactionSyncMessage(msg) => {
-                transaction_sync_msg_sender.send(msg).await?;
-
                 if !self.sync_message_received {
                     self.peer_event_sender
                         .send(PeerEvent::MessageReceived(
@@ -446,6 +444,8 @@ where
                         .await?;
                     self.sync_message_received = true;
                 }
+
+                transaction_sync_msg_sender.send(msg).await?;
             }
         }
 

p2p/src/peer_manager/mod.rs

@@ -1552,6 +1552,7 @@ where
             );
             if let Some(address) = address_opt {
                 log::debug!("Need to establish a feeler connection to address {address}");
+
                 self.connect(address, OutboundConnectType::Feeler);
                 self.next_feeler_connection_time = Self::choose_next_feeler_connection_time(
                     &self.p2p_config,
@@ -1581,7 +1582,7 @@ where
         // just in case, so that if things go horribly wrong, only the debug build will panic.
         if let Some(peer) = self.peers.get(&peer_id) {
             if !is_disconnection_message && peer.peer_role.is_outbound() {
-                self.peerdb.outbound_peer_has_activity(
+                self.peerdb.outbound_peer_had_activity(
                     peer.peer_address,
                     Self::lock_rng(&self.rng).deref_mut(),
                 );

p2p/src/peer_manager/peerdb/address_data/mod.rs

@@ -81,7 +81,7 @@ pub enum AddressState {
 #[strum_discriminants(name(AddressStateTransitionToTag), derive(strum::EnumIter))]
 pub enum AddressStateTransitionTo {
     Connected { peer_role: OutboundPeerRole },
-    HasActivity,
+    HadActivity,
     Disconnected,
     ConnectionFailed,
     SetReserved,
@@ -239,7 +239,7 @@ impl AddressData {
                 }
             },
 
-            AddressStateTransitionTo::HasActivity => match self.state {
+            AddressStateTransitionTo::HadActivity => match self.state {
                 AddressState::Connected {
                     had_activity: _,
                     peer_role,
@@ -253,13 +253,13 @@ impl AddressData {
                     was_reachable: _,
                 } => {
                     debug_panic_or_log!(
-                        "Unexpected address state transition: Disconnected -> HasActivity"
+                        "Unexpected address state transition: Disconnected -> HadActivity"
                     );
                     self.state.clone()
                 }
                 AddressState::Unreachable { erase_after: _ } => {
                     debug_panic_or_log!(
-                        "Unexpected address state transition: Unreachable -> HasActivity"
+                        "Unexpected address state transition: Unreachable -> HadActivity"
                     );
                     self.state.clone()
                 }
@@ -274,7 +274,7 @@ impl AddressData {
                         self.connections_without_activity_count = 0;
                     } else {
                         // We don't increase the counter for:
-                        // 1) feeler connections (because it gets disconnected immediately, so the peer may
+                        // 1) feeler connections (because they get disconnected immediately, so the peer may
                         // not have the chance to send us anything);
                         // 2) manual connections;
                         let is_regular_auto_connection = match peer_role {

p2p/src/peer_manager/peerdb/address_data/tests.rs

@@ -53,7 +53,7 @@ fn randomized(#[case] seed: Seed) {
                 AddressStateTransitionToTag::Connected => AddressStateTransitionTo::Connected {
                     peer_role: OutboundPeerRole::iter().choose(&mut rng).unwrap(),
                 },
-                AddressStateTransitionToTag::HasActivity => AddressStateTransitionTo::HasActivity,
+                AddressStateTransitionToTag::HadActivity => AddressStateTransitionTo::HadActivity,
                 AddressStateTransitionToTag::Disconnected => AddressStateTransitionTo::Disconnected,
                 AddressStateTransitionToTag::ConnectionFailed => {
                     AddressStateTransitionTo::ConnectionFailed
@@ -68,7 +68,7 @@ fn randomized(#[case] seed: Seed) {
                 AddressStateTransitionTo::Connected { peer_role: _ } => {
                     !address_data.is_connected()
                 }
-                AddressStateTransitionTo::HasActivity => address_data.is_connected(),
+                AddressStateTransitionTo::HadActivity => address_data.is_connected(),
                 AddressStateTransitionTo::Disconnected => address_data.is_connected(),
                 AddressStateTransitionTo::ConnectionFailed => !address_data.is_connected(),
                 AddressStateTransitionTo::SetReserved => true,

p2p/src/peer_manager/peerdb/mod.rs

@@ -425,8 +425,8 @@ impl<S: PeerDbStorage> PeerDb<S> {
     }
 
     /// Report that the peer has sent us a message other than the one indicating an intent to disconnect.
-    pub fn outbound_peer_has_activity(&mut self, address: SocketAddress, rng: &mut impl Rng) {
-        self.change_address_state(address, AddressStateTransitionTo::HasActivity, rng);
+    pub fn outbound_peer_had_activity(&mut self, address: SocketAddress, rng: &mut impl Rng) {
+        self.change_address_state(address, AddressStateTransitionTo::HadActivity, rng);
     }
 
     pub fn remove_address(&mut self, address: &SocketAddress) {

p2p/src/peer_manager/tests/unsuccessful_connection_counter_update.rs

@@ -286,7 +286,9 @@ async fn auto_connection_fails_peer_state_becomes_disconnected(
 //    Keep peer address's `was_reachable` field at false.
 // 3) Advance the time so that the corresponding connection would be attempted; make the
 //    connection fail; check that the peer address state is now `Unreachable`.
-// 4) Check that no further connection attempts are made.
+// 4) Optionally, accept an inbound connection from the peer address; expect that it doesn't
+//    affect the current address state.
+// 5) Check that no further connection attempts are made.
 #[tracing::instrument(skip(seed))]
 #[rstest]
 #[trace]
@@ -510,7 +512,6 @@ async fn manual_connection_fails(#[case] seed: Seed, #[values(false, true)] make
 
     if make_reachable {
         let peer_discovery_time = time_getter.get_time_getter().get_time();
-
         discover_peer(&peer_mgr_event_sender, peer_address, true).await;
 
         let peer_addr_state =
@@ -609,7 +610,7 @@ async fn manual_connection_fails(#[case] seed: Seed, #[values(false, true)] make
 //    Make the connection succeed and close it immediately; check that `connections_without_activity_count`
 //    has been incremented.
 // 3) Occasionally, make a successful incoming or manual outgoing connection without peer activity, or an unsuccessful
-//    manual outgoing one. Expect that this doesn't affect `connections_without_activity_count`.
+//    manual outgoing connection. Expect that this doesn't affect `connections_without_activity_count`.
 // 4) On the final iteration make the peer actually send a message. Check that `connections_without_activity_count` has
 //    been reset to zero.
 // Note that feeler connections are not checked in this test because once a feeler connection succeeds, it won't
@@ -765,7 +766,7 @@ async fn auto_connection_without_peer_activity(
 
         // An inbound or manual connection without peer activity shouldn't affect connections_without_activity_count.
         // Same for a failed outbound connection.
-        let extra_outboun_connection_failed = match rng.gen_range(0..4) {
+        let extra_outbound_connection_failed = match rng.gen_range(0..4) {
             0 => {
                 log::debug!("Accepting an extra inbound connection");
 
@@ -816,7 +817,7 @@ async fn auto_connection_without_peer_activity(
         };
 
         let expected_connections_without_activity_count = if is_last_iteration { 0 } else { i + 1 };
-        let expected_fail_count = if extra_outboun_connection_failed {
+        let expected_fail_count = if extra_outbound_connection_failed {
             1
         } else {
             0

p2p/src/peer_manager/tests/utils.rs

@@ -34,7 +34,6 @@ use crate::{
         types::{ConnectivityEvent, PeerInfo},
     },
     peer_manager::PeerManagerInterface,
-    peer_manager_event::PeerDisconnectionDbAction,
     test_helpers::TEST_PROTOCOL_VERSION,
     tests::helpers::PeerManagerNotification,
     utils::oneshot_nofail,
@@ -299,25 +298,6 @@ pub fn start_manually_connecting(
     result_receiver
 }
 
-pub fn disconnect_manually(
-    peer_mgr_event_sender: &mpsc::UnboundedSender<PeerManagerEvent>,
-    peer_id: PeerId,
-    peerdb_action: PeerDisconnectionDbAction,
-) -> oneshot_nofail::Receiver<crate::Result<()>> {
-    let (result_sender, result_receiver) = oneshot_nofail::channel();
-
-    peer_mgr_event_sender
-        .send(PeerManagerEvent::Disconnect(
-            peer_id,
-            peerdb_action,
-            None,
-            result_sender,
-        ))
-        .unwrap();
-
-    result_receiver
-}
-
 pub async fn adjust_peer_score(
     peer_mgr_event_sender: &mpsc::UnboundedSender<PeerManagerEvent>,
     peer_id: PeerId,

p2p/src/tests/peer_mgr_events.rs

@@ -65,7 +65,7 @@ enum TestParamForFirstSyncMessageReceivedMustBeSent {
 // is sent to the peer manager.
 // 1) Send a random message from the peer.
 // 2) If the sent message was a sync one, expect that `PeerManagerNotification::MessageReceived` is emitted with
-// message equal to `PeerManagerMessageExtTag::FirstSyncMessageReceived`. If the sent message was not a sync one,
+// the message equal to `PeerManagerMessageExtTag::FirstSyncMessageReceived`. If the sent message was not a sync one,
 // expect that no such notification is emitted.
 #[tracing::instrument(skip(seed))]
 #[rstest]
@@ -233,7 +233,7 @@ async fn first_sync_message_received_must_be_sent(
 
 // Check that `PeerManagerMessageExt::FirstSyncMessageReceived` will be sent only once even if
 // many sync messages are received from the peer.
-// 1) Send a few HeaderListRequest and/or NewTransaction (legit messages that a peer can send
+// 1) Send a few `HeaderListRequest` and/or `NewTransaction` messages (i.e. some legit messages that a peer can send
 // in any number).
 // 2) Expect that only one `PeerManagerNotification::MessageReceived` with `PeerManagerMessageExtTag::FirstSyncMessageReceived`
 // is produced.

test-utils/src/basic_test_time_getter.rs

@@ -48,7 +48,7 @@ impl BasicTestTimeGetter {
 
     // Same as advance_time, except that if the passed duration's precision is not representable by
     // the time getter (i.e. it's sub-millisecond), this function will round it up, ensuring that
-    // the resulting time is always bigger than the initial time plus duration.
+    // the resulting time is always greater-or-equal than the initial time plus the duration.
     pub fn advance_time_rounded_up(&self, duration: Duration) {
         let mut millis = duration.as_millis() as u64;
         if duration.subsec_nanos() % 1_000_000 != 0 {