swarm: links by whom we reach

4 files changed, 75 insertions, 13 deletions

diff --git a/docs/dashboard.md b/docs/dashboard.md
index a9e4567..4092886 100644
--- a/docs/dashboard.md
+++ b/docs/dashboard.md
@@ -336,7 +336,8 @@ and the members are only asked every 30 s, for every page open on this node, unt
 asked otherwise or the node restarts.
 
 A member this node can't reach (on none of its transports: one on Tor alone, from a
-node on libp2p alone) is asked through a peer that can: an active member it reaches
+node on libp2p alone; or one that doesn't answer its HELLOs, as behind NAT) is asked
+through a peer that can: an active member it reaches
 that, by the member list, shares a transport with the member, else one on a transport
 this node isn't on (the lowest pair score of several). The LINKS names the members to
 ask (`of`): the peer asks them on this node's behalf, itself or again through a peer
diff --git a/src/simtest/links.rs b/src/simtest/links.rs
index 44836bd..17c7ced 100644
--- a/src/simtest/links.rs
+++ b/src/simtest/links.rs
@@ -103,3 +103,51 @@ async fn links_are_relayed_to_members_out_of_reach() {
         .await;
     }
 }
+
+/// a, b and c all on `sim`, but nothing passes between a and c (as behind NAT): once a
+/// judges c unreachable, it asks c's links through b, though it has c's address.
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+async fn links_are_relayed_to_members_that_dont_answer() {
+    let sim = Sim::with_identity(64);
+    let cut = crate::transport::sim::LinkParams {
+        loss: 1.0,
+        ..Default::default()
+    };
+    sim.net.set_link("a", "c", cut.clone());
+    sim.net.set_link("c", "a", cut);
+    let b = sim.start_on("b", &["sim"], &[], Start::Genesis);
+    let a = sim.start_on("a", &["sim"], &["b"], Start::Join);
+    let c = sim.start_on("c", &["sim"], &["b"], Start::Join);
+    sim.wait_ready(&[&a, &b, &c], SOON).await;
+    let (b_id, c_id) = (b.local_id().to_owned(), c.local_id().to_owned());
+    {
+        let (a, c_id) = (&a, c_id.clone());
+        sim.wait_until(
+            "a knows c, and that it doesn't answer",
+            SOON * 3,
+            move || {
+                let c_id = c_id.clone();
+                async move {
+                    let listed = members::members(&a.doc.read().await.doc).contains(&c_id);
+                    listed && a.transport.can_reach(&c_id) && !topology::reaches(a, &c_id).await
+                }
+            },
+        )
+        .await;
+    }
+    links::ask_all(
+        &a,
+        vec![b_id.clone(), c_id.clone()],
+        Duration::from_secs(10),
+    )
+    .await;
+    {
+        let (a, b_id, c_id) = (&a, b_id.clone(), c_id.clone());
+        sim.wait_until("a has c's links, relayed by b", SOON, move || {
+            let (b_id, c_id) = (b_id.clone(), c_id.clone());
+            async move { has_links(a, &c_id, &b_id) }
+        })
+        .await;
+    }
+    assert!(!c.links.subscribers().contains(&a.local_id().to_owned()));
+}
diff --git a/src/swarm/links.rs b/src/swarm/links.rs
index 7f8f0b0..14c5cf6 100644
--- a/src/swarm/links.rs
+++ b/src/swarm/links.rs
@@ -17,7 +17,9 @@
 //! renewing) or a LINKS with 0 ends it.
 //!
 //! A member the asker can't reach (on none of its transports: one on Tor alone, asked
-//! from one on libp2p alone) is asked through a peer that can (`relay_for`): a LINKS
+//! from one on libp2p alone; or one that doesn't answer it, as behind NAT: as choosing
+//! neighbours judges it, `topology::reaches`) is asked through a peer that can
+//! (`relay_for`): a LINKS
 //! naming the members (`of`) asks the receiver to ask them on the sender's behalf, and
 //! to pass their links on (`relayed`). The receiver asks each itself, or through a peer
 //! of its own while hops are left (`hops`), for as long as the sender asked, and holds
@@ -362,7 +364,7 @@ async fn split(
         if member == state.local_id() || Some(member.as_str()) == exclude {
             continue;
         }
-        if state.transport.can_reach(&member) {
+        if topology::reaches(state, &member).await {
             direct.push(member);
         } else if let Some(relay) = relay_for(state, &member, exclude).await {
             through.entry(relay).or_default().push(member);
@@ -372,7 +374,7 @@ async fn split(
 }
 
 /// The peer to ask `member` through, which we can't reach: an active member we reach
-/// that, by the member list, is on a transport `member` is on; else one on a transport
+/// (that answers us) that, by the member list, is on a transport `member` is on; else one on a transport
 /// we aren't on (it may know one that is). Of several, the one with the lowest pair score
 /// with `member` (`topology::pair_score`), so askers pick alike. Not `exclude`.
 async fn relay_for(state: &AppState, member: &str, exclude: Option<&str>) -> Option<String> {
@@ -385,7 +387,8 @@ async fn relay_for(state: &AppState, member: &str, exclude: Option<&str>) -> Opt
         }
         {
             let peer_state = peer_state.lock().await;
-            if peer_state.liveness.is_dormant()
+            if peer_state.is_unreachable()
+                || peer_state.liveness.is_dormant()
                 || !matches!(peer_state.standing, crate::state::Standing::Member { .. })
             {
                 continue;
diff --git a/src/swarm/topology/mod.rs b/src/swarm/topology/mod.rs
index e3479e3..992c448 100644
--- a/src/swarm/topology/mod.rs
+++ b/src/swarm/topology/mod.rs
@@ -142,6 +142,19 @@ pub(crate) fn intended_graph(state: &AppState) -> Option<BTreeMap<String, BTreeS
     Some(graph)
 }
 
+/// Whether we reach `peer` ourselves, as choosing neighbours judges it: on one of our
+/// transports, and not unanswered (three HELLOs, or dormant: `PeerState::is_unreachable`).
+/// One we have no word of yet counts as reached until it fails to answer.
+pub(crate) async fn reaches(state: &AppState, peer: &str) -> bool {
+    if !state.transport.can_reach(peer) {
+        return false;
+    }
+    match state.get_peer_state(peer).await {
+        Some(peer_state) => !peer_state.lock().await.is_unreachable(),
+        None => true,
+    }
+}
+
 /// The transports `member` is on, by the member list (the schemes of its addresses);
 /// ours as we are on them. None known: empty.
 pub(crate) fn transports_of(state: &AppState, member: &str) -> BTreeSet<String> {
@@ -251,18 +264,15 @@ pub(crate) async fn refresh(state: &AppState) -> Option<NeighbourChange> {
         if member == state.local_id() {
             continue;
         }
-        if !state.transport.can_reach(member) {
+        if !reaches(state, member).await {
             unreachable.insert(member.clone());
             continue;
         }
-        let (gone, heard) = match state.get_peer_state(member).await {
-            Some(peer_state) => {
-                let peer_state = peer_state.lock().await;
-                (peer_state.is_unreachable(), peer_state.last_seen.is_some())
-            }
-            None => (false, false),
+        let heard = match state.get_peer_state(member).await {
+            Some(peer_state) => peer_state.lock().await.last_seen.is_some(),
+            None => false,
         };
-        if gone || (!heard && !alive.contains(member)) {
+        if !heard && !alive.contains(member) {
             unreachable.insert(member.clone());
         }
     }