net: stabilize HostState.unregister() calls

We previously deleted the call to `unregister()` after the refinery is
successful on a16dd562d95a89a9029fca6b2ad9931a56be1fde.

It was considered redundant since the `unregister()` call happens in
`subscribe_on_stop()`, which for refine session is called directly after
the refinery process finishes (after `channel.stop()`).

However, in a highly async environment the `unregister()` call in
`subscribe_on_stop()` can be called before the call to `move_host()`, meaning
that the host would then be stuck in the `Moving` state.

We have fixed this by specifying that `unregister` should only be called
in `subscribe_on_stop()` to peers that are not part of a refine session.
We seperately call `unregister` after `move_host` in the refinery.

This commit also fixes some documentation.

Note: the call to `unregister()` is highly fragile and can lead to race
conditions. We are working to replace this with something more robust
(like `tombstone()`).

src/net/hosts.rs

@@ -1233,6 +1233,14 @@ impl Hosts {
     /// * When the refinery passes successfully: move to white, remove from greylist.
     /// * When we connect to a peer, move to gold, remove from white or grey.
     /// * When we add a peer to the black list: move to black, remove from all other lists.
+    ///
+    /// Note that this method puts a given Url into the "Move" state but does not reset the
+    /// state afterwards. This is because the next state will differ depending on its usage.
+    /// The state transition from `Move` to `Connected` or `Suspend` are both valid operations.
+    /// In some cases, `unregister()` can be called after `move_host()` but it must be done with
+    /// care due to the potential for race conditions.
+    ///
+    /// TODO: replace unregister() with a tombstone() call.
     pub(in crate::net) fn move_host(
         &self,
         addr: &Url,

src/net/session/mod.rs

@@ -71,7 +71,7 @@ pub async fn remove_sub_on_stop(p2p: P2pPtr, channel: ChannelPtr, type_id: Sessi
         "Received stop event. Removing channel {}", addr,
     );
 
-    // Downgrade to greylist this is a outbound or manual session.
+    // Downgrade to greylist if this is a outbound session.
     if type_id & SESSION_OUTBOUND != 0 {
         debug!(
             target: "net::session::remove_sub_on_stop()",
@@ -82,8 +82,13 @@ pub async fn remove_sub_on_stop(p2p: P2pPtr, channel: ChannelPtr, type_id: Sessi
         hosts.move_host(addr, last_seen, HostColor::Grey).unwrap();
     }
 
-    // Remove channel from the HostRegistry. Free up this addr for any future operation.
-    hosts.unregister(channel.address());
+    // For all sessions that are not refine sessions, remove this channel
+    // from the HostRegistry. `unregister()` frees up this addr for any
+    // future operation. We don't call this on refine sessions since the
+    // unregister() call happens in the refinery directly.
+    if type_id & SESSION_REFINE == 0 {
+        hosts.unregister(channel.address());
+    }
 
     if !p2p.is_connected() {
         hosts.disconnect_publisher.notify(Error::NetworkNotConnected).await;

src/net/session/refine_session.rs

@@ -297,6 +297,7 @@ impl GreylistRefinery {
 
                     // Add to the whitelist and remove from the greylist.
                     hosts.move_host(url, last_seen, HostColor::White).unwrap();
+                    hosts.unregister(url);
 
                     debug!(target: "net::refinery", "GreylistRefinery complete!");