better route selection

external/keyring-manager

@@ -1 +1 @@
-Subproject commit c153eb3015d6d118e5d467865510d053ddd84533
+Subproject commit b127b2d3c653fea163a776dd58b3798f28aeeee3

veilid-core/src/network_manager/mod.rs

@@ -1379,9 +1379,13 @@ impl NetworkManager {
 
             let some_relay_nr = if self.check_client_whitelist(sender_id) {
                 // Full relay allowed, do a full resolve_node
-                rpc.resolve_node(recipient_id).await.wrap_err(
+                match rpc.resolve_node(recipient_id).await {
-                    "failed to resolve recipient node for relay, dropping outbound relayed packet",
+                    Ok(v) => v,
-                )?
+                    Err(e) => {
+                        log_net!(debug "failed to resolve recipient node for relay, dropping outbound relayed packet: {}" ,e);
+                        return Ok(false);
+                    }
+                }
             } else {
                 // If this is not a node in the client whitelist, only allow inbound relay
                 // which only performs a lightweight lookup before passing the packet back out
@@ -1396,9 +1400,14 @@ impl NetworkManager {
             if let Some(relay_nr) = some_relay_nr {
                 // Relay the packet to the desired destination
                 log_net!("relaying {} bytes to {}", data.len(), relay_nr);
-                network_result_value_or_log!(self.send_data(relay_nr, data.to_vec())
+                network_result_value_or_log!(match self.send_data(relay_nr, data.to_vec())
-                    .await
+                    .await {
-                    .wrap_err("failed to forward envelope")? => {
+                        Ok(v) => v,
+                        Err(e) => {
+                            log_net!(debug "failed to forward envelope: {}" ,e);
+                            return Ok(false);    
+                        }
+                    } => {
                         return Ok(false);
                     }
                 );
@@ -1411,10 +1420,15 @@ impl NetworkManager {
         let node_id_secret = routing_table.node_id_secret();
 
         // Decrypt the envelope body
-        // xxx: punish nodes that send messages that fail to decrypt eventually
+        let body = match envelope
-        let body = envelope
+            .decrypt_body(self.crypto(), data, &node_id_secret) {
-            .decrypt_body(self.crypto(), data, &node_id_secret)
+                Ok(v) => v,
-            .wrap_err("failed to decrypt envelope body")?;
+                Err(e) => {
+                    log_net!(debug "failed to decrypt envelope body: {}",e);
+                    // xxx: punish nodes that send messages that fail to decrypt eventually
+                    return Ok(false);
+                }
+            };
 
         // Cache the envelope information in the routing table
         let source_noderef = match routing_table.register_node_with_existing_connection(

veilid-core/src/routing_table/route_spec_store.rs

@@ -646,22 +646,43 @@ impl RouteSpecStore {
                 let v = v.unwrap();
 
                 // Exclude our relay if we have one
-                if let Some(relay_id) = opt_relay_id {
+                if let Some(own_relay_id) = opt_relay_id {
-                    if k == relay_id {
+                    if k == own_relay_id {
                         return false;
                     }
                 }
 
-                // Exclude nodes on our local network
+                // Exclude nodes we have specifically chosen to avoid
-                let on_local_network = v.with(rti, |_rti, e| {
+                if avoid_node_ids.contains(&k) {
-                    e.node_info(RoutingDomain::LocalNetwork).is_some()
-                });
-                if on_local_network {
                     return false;
                 }
 
-                // Exclude nodes we have specifically chosen to avoid
+                // Process node info exclusions
-                if avoid_node_ids.contains(&k) {
+                let keep = v.with(rti, |_rti, e| {
+                    // Exclude nodes on our local network
+                    if e.node_info(RoutingDomain::LocalNetwork).is_some() {
+                        return false;
+                    }
+                    // Exclude nodes that have no publicinternet signednodeinfo
+                    let Some(sni) = e.signed_node_info(RoutingDomain::PublicInternet) else {    
+                        return false;
+                    };
+                    // Relay check
+                    if let Some(relay_id) = sni.relay_id() {
+                        // Exclude nodes whose relays we have chosen to avoid
+                        if avoid_node_ids.contains(&relay_id.key) {
+                            return false;
+                        }
+                        // Exclude nodes whose relay is our own relay if we have one
+                        if let Some(own_relay_id) = opt_relay_id {
+                            if own_relay_id == relay_id.key {
+                                return false;
+                            }
+                        }
+                    }
+                    return true;
+                });
+                if !keep {
                     return false;
                 }
 
@@ -774,6 +795,22 @@ impl RouteSpecStore {
                 return None;
             }
 
+            // Ensure the route doesn't contain both a node and its relay
+            let mut seen_nodes: HashSet<DHTKey> = HashSet::new();
+            for n in permutation {
+                let node = nodes.get(*n).unwrap();
+                if !seen_nodes.insert(node.node_id.key) {
+                    // Already seen this node, should not be in the route twice
+                    return None;
+                }
+                if let Some(relay_id) = node.signed_node_info.relay_id() {
+                    if !seen_nodes.insert(relay_id.key) {
+                        // Already seen this node, should not be in the route twice
+                        return None;
+                    }
+                }
+            }
+
             // Ensure this route is viable by checking that each node can contact the next one
             if directions.contains(Direction::Outbound) {
                 let mut previous_node = &our_peer_info;

veilid-core/src/routing_table/routing_domains.rs

@@ -294,8 +294,10 @@ impl RoutingDomainDetail for PublicInternetRoutingDomainDetail {
             // Get the target's inbound relay, it must have one or it is not reachable
             if let Some(node_b_relay) = peer_b.signed_node_info.relay_info() {
                 let node_b_relay_id = peer_b.signed_node_info.relay_id().unwrap();
+
                 // Note that relay_peer_info could be node_a, in which case a connection already exists
-                // and we shouldn't have even gotten here
+                // and we only get here if the connection had dropped, in which case node_a is unreachable until
+                // it gets a new relay connection up
                 if node_b_relay_id.key == peer_a.node_id.key {
                     return ContactMethod::Existing;
                 }
@@ -376,6 +378,13 @@ impl RoutingDomainDetail for PublicInternetRoutingDomainDetail {
         else if let Some(node_b_relay) = peer_b.signed_node_info.relay_info() {
             let node_b_relay_id = peer_b.signed_node_info.relay_id().unwrap();
             
+            // Note that relay_peer_info could be node_a, in which case a connection already exists
+            // and we only get here if the connection had dropped, in which case node_a is unreachable until
+            // it gets a new relay connection up
+            if node_b_relay_id.key == peer_a.node_id.key {
+                return ContactMethod::Existing;
+            }
+
             // Can we reach the full relay?
             if first_filtered_dial_info_detail(
                 node_a,

veilid-core/src/veilid_api/types.rs

@@ -1988,7 +1988,6 @@ impl SignedNodeInfo {
             SignedNodeInfo::Relayed(r) => r.timestamp,
         }
     }
-
     pub fn node_info(&self) -> &NodeInfo {
         match self {
             SignedNodeInfo::Direct(d) => &d.node_info,