better error handling

This commit is contained in:
John Smith
2023-06-15 20:22:54 -04:00
parent 615e0ca1d0
commit d6f442d431
29 changed files with 368 additions and 234 deletions
+11 -10
View File
@@ -628,14 +628,14 @@ impl RoutingTable {
}
/// Resolve an existing routing table entry using any crypto kind and return a reference to it
pub fn lookup_any_node_ref(&self, node_id_key: PublicKey) -> Option<NodeRef> {
pub fn lookup_any_node_ref(&self, node_id_key: PublicKey) -> EyreResult<Option<NodeRef>> {
self.inner
.read()
.lookup_any_node_ref(self.clone(), node_id_key)
}
/// Resolve an existing routing table entry and return a reference to it
pub fn lookup_node_ref(&self, node_id: TypedKey) -> Option<NodeRef> {
pub fn lookup_node_ref(&self, node_id: TypedKey) -> EyreResult<Option<NodeRef>> {
self.inner.read().lookup_node_ref(self.clone(), node_id)
}
@@ -645,7 +645,7 @@ impl RoutingTable {
node_id: TypedKey,
routing_domain_set: RoutingDomainSet,
dial_info_filter: DialInfoFilter,
) -> Option<NodeRef> {
) -> EyreResult<Option<NodeRef>> {
self.inner.read().lookup_and_filter_noderef(
self.clone(),
node_id,
@@ -662,7 +662,7 @@ impl RoutingTable {
routing_domain: RoutingDomain,
peer_info: PeerInfo,
allow_invalid: bool,
) -> Option<NodeRef> {
) -> EyreResult<NodeRef> {
self.inner.write().register_node_with_peer_info(
self.clone(),
routing_domain,
@@ -678,7 +678,7 @@ impl RoutingTable {
node_id: TypedKey,
descriptor: ConnectionDescriptor,
timestamp: Timestamp,
) -> Option<NodeRef> {
) -> EyreResult<NodeRef> {
self.inner.write().register_node_with_existing_connection(
self.clone(),
node_id,
@@ -711,7 +711,7 @@ impl RoutingTable {
// (uses same logic as send_data, ensuring last_connection works for UDP)
for e in &recent_peers {
let mut dead = true;
if let Some(nr) = self.lookup_node_ref(*e) {
if let Ok(Some(nr)) = self.lookup_node_ref(*e) {
if let Some(last_connection) = nr.last_connection() {
out.push((*e, RecentPeersEntry { last_connection }));
dead = false;
@@ -1017,10 +1017,11 @@ impl RoutingTable {
}
// Register the node if it's new
if let Some(nr) =
self.register_node_with_peer_info(RoutingDomain::PublicInternet, p, false)
{
out.push(nr);
match self.register_node_with_peer_info(RoutingDomain::PublicInternet, p, false) {
Ok(nr) => out.push(nr),
Err(e) => {
log_rtab!(debug "failed to register node with peer info from find node answer: {}", e);
}
}
}
out
+16 -17
View File
@@ -192,25 +192,24 @@ pub trait NodeRefBase: Sized {
}
dif
}
fn relay(&self, routing_domain: RoutingDomain) -> Option<NodeRef> {
fn relay(&self, routing_domain: RoutingDomain) -> EyreResult<Option<NodeRef>> {
self.operate_mut(|rti, e| {
e.signed_node_info(routing_domain)
.and_then(|n| n.relay_peer_info())
.and_then(|rpi| {
// If relay is ourselves, then return None, because we can't relay through ourselves
// and to contact this node we should have had an existing inbound connection
if rti.unlocked_inner.matches_own_node_id(rpi.node_ids()) {
return None;
}
let Some(sni) = e.signed_node_info(routing_domain) else {
return Ok(None);
};
let Some(rpi) = sni.relay_peer_info() else {
return Ok(None);
};
// If relay is ourselves, then return None, because we can't relay through ourselves
// and to contact this node we should have had an existing inbound connection
if rti.unlocked_inner.matches_own_node_id(rpi.node_ids()) {
bail!("Can't relay though ourselves");
}
// Register relay node and return noderef
rti.register_node_with_peer_info(
self.routing_table(),
routing_domain,
rpi,
false,
)
})
// Register relay node and return noderef
let nr =
rti.register_node_with_peer_info(self.routing_table(), routing_domain, rpi, false)?;
Ok(Some(nr))
})
}
+15 -3
View File
@@ -37,15 +37,27 @@ impl RouteNode {
match self {
RouteNode::NodeId(id) => {
//
routing_table.lookup_node_ref(TypedKey::new(crypto_kind, *id))
match routing_table.lookup_node_ref(TypedKey::new(crypto_kind, *id)) {
Ok(nr) => nr,
Err(e) => {
log_rtab!(debug "failed to look up route node: {}", e);
return None;
}
}
}
RouteNode::PeerInfo(pi) => {
//
routing_table.register_node_with_peer_info(
match routing_table.register_node_with_peer_info(
RoutingDomain::PublicInternet,
pi.clone(),
false,
)
) {
Ok(nr) => Some(nr),
Err(e) => {
log_rtab!(debug "failed to register route node: {}", e);
return None;
}
}
}
}
}
@@ -378,7 +378,14 @@ impl RouteSpecStore {
// Already seen this node, should not be in the route twice
return None;
}
if let Some(relay) = node.locked_mut(rti).relay(RoutingDomain::PublicInternet) {
let opt_relay = match node.locked_mut(rti).relay(RoutingDomain::PublicInternet) {
Ok(r) => r,
Err(e) => {
log_rtab!(error "failed to get relay for route node: {}", e);
return None;
}
};
if let Some(relay) = opt_relay {
let relay_id = relay.locked(rti).best_node_id();
if !seen_nodes.insert(relay_id) {
// Already seen this node, should not be in the route twice
@@ -869,13 +876,15 @@ impl RouteSpecStore {
};
let opt_first_hop = match pr_first_hop_node {
RouteNode::NodeId(id) => rti.lookup_node_ref(routing_table.clone(), TypedKey::new(crypto_kind, id)),
RouteNode::PeerInfo(pi) => rti.register_node_with_peer_info(
routing_table.clone(),
RoutingDomain::PublicInternet,
pi,
false,
),
RouteNode::NodeId(id) => rti.lookup_node_ref(routing_table.clone(), TypedKey::new(crypto_kind, id))?,
RouteNode::PeerInfo(pi) => {
Some(rti.register_node_with_peer_info(
routing_table.clone(),
RoutingDomain::PublicInternet,
pi,
false,
)?)
}
};
if opt_first_hop.is_none() {
// Can't reach this private route any more
@@ -40,7 +40,7 @@ impl RouteSpecStoreContent {
// Go through best route and resolve noderefs
let mut hop_node_refs = Vec::with_capacity(rsd.hops.len());
for h in &rsd.hops {
let Some(nr) = routing_table.lookup_node_ref(TypedKey::new(rsd.crypto_kind, *h)) else {
let Ok(Some(nr)) = routing_table.lookup_node_ref(TypedKey::new(rsd.crypto_kind, *h)) else {
dead_ids.push(rsid.clone());
break;
};
@@ -651,14 +651,13 @@ impl RoutingTableInner {
outer_self: RoutingTable,
node_ids: &TypedKeySet,
update_func: F,
) -> Option<NodeRef>
) -> EyreResult<NodeRef>
where
F: FnOnce(&mut RoutingTableInner, &mut BucketEntryInner),
{
// Ensure someone isn't trying register this node itself
if self.unlocked_inner.matches_own_node_id(node_ids) {
log_rtab!(debug "can't register own node");
return None;
bail!("can't register own node");
}
// Look up all bucket entries and make sure we only have zero or one
@@ -688,8 +687,7 @@ impl RoutingTableInner {
if let Some(best_entry) = best_entry {
// Update the entry with all of the node ids
if let Err(e) = self.update_bucket_entries(best_entry.clone(), node_ids) {
log_rtab!(debug "Not registering new ids for existing node: {}", e);
return None;
bail!("Not registering new ids for existing node: {}", e);
}
// Make a noderef to return
@@ -699,7 +697,7 @@ impl RoutingTableInner {
best_entry.with_mut_inner(|e| update_func(self, e));
// Return the noderef
return Some(nr);
return Ok(nr);
}
// If no entry exists yet, add the first entry to a bucket, possibly evicting a bucket member
@@ -712,8 +710,7 @@ impl RoutingTableInner {
// Update the other bucket entries with the remaining node ids
if let Err(e) = self.update_bucket_entries(new_entry.clone(), node_ids) {
log_rtab!(debug "Not registering new node: {}", e);
return None;
bail!("Not registering new node: {}", e);
}
// Make node ref to return
@@ -725,7 +722,7 @@ impl RoutingTableInner {
// Kick the bucket
log_rtab!(debug "Routing table now has {} nodes, {} live", self.bucket_entry_count(), self.get_entry_count(RoutingDomainSet::all(), BucketEntryState::Unreliable, &VALID_CRYPTO_KINDS));
Some(nr)
Ok(nr)
}
/// Resolve an existing routing table entry using any crypto kind and return a reference to it
@@ -733,28 +730,35 @@ impl RoutingTableInner {
&self,
outer_self: RoutingTable,
node_id_key: PublicKey,
) -> Option<NodeRef> {
VALID_CRYPTO_KINDS.iter().find_map(|ck| {
self.lookup_node_ref(outer_self.clone(), TypedKey::new(*ck, node_id_key))
})
) -> EyreResult<Option<NodeRef>> {
for ck in VALID_CRYPTO_KINDS {
if let Some(nr) =
self.lookup_node_ref(outer_self.clone(), TypedKey::new(ck, node_id_key))?
{
return Ok(Some(nr));
}
}
Ok(None)
}
/// Resolve an existing routing table entry and return a reference to it
pub fn lookup_node_ref(&self, outer_self: RoutingTable, node_id: TypedKey) -> Option<NodeRef> {
pub fn lookup_node_ref(
&self,
outer_self: RoutingTable,
node_id: TypedKey,
) -> EyreResult<Option<NodeRef>> {
if self.unlocked_inner.matches_own_node_id(&[node_id]) {
log_rtab!(error "can't look up own node id in routing table");
return None;
bail!("can't look up own node id in routing table");
}
if !VALID_CRYPTO_KINDS.contains(&node_id.kind) {
log_rtab!(error "can't look up node id with invalid crypto kind");
return None;
bail!("can't look up node id with invalid crypto kind");
}
let bucket_index = self.unlocked_inner.calculate_bucket_index(&node_id);
let bucket = self.get_bucket(bucket_index);
bucket
Ok(bucket
.entry(&node_id.value)
.map(|e| NodeRef::new(outer_self, e, None))
.map(|e| NodeRef::new(outer_self, e, None)))
}
/// Resolve an existing routing table entry and return a filtered reference to it
@@ -764,15 +768,15 @@ impl RoutingTableInner {
node_id: TypedKey,
routing_domain_set: RoutingDomainSet,
dial_info_filter: DialInfoFilter,
) -> Option<NodeRef> {
) -> EyreResult<Option<NodeRef>> {
let nr = self.lookup_node_ref(outer_self, node_id)?;
Some(
Ok(nr.map(|nr| {
nr.filtered_clone(
NodeRefFilter::new()
.with_dial_info_filter(dial_info_filter)
.with_routing_domain_set(routing_domain_set),
),
)
)
}))
}
/// Resolve an existing routing table entry and call a function on its entry without using a noderef
@@ -802,50 +806,53 @@ impl RoutingTableInner {
routing_domain: RoutingDomain,
peer_info: PeerInfo,
allow_invalid: bool,
) -> Option<NodeRef> {
) -> EyreResult<NodeRef> {
// if our own node is in the list, then ignore it as we don't add ourselves to our own routing table
if self
.unlocked_inner
.matches_own_node_id(peer_info.node_ids())
{
log_rtab!(debug "can't register own node id in routing table");
return None;
bail!("can't register own node id in routing table");
}
// node can not be its own relay
let rids = peer_info.signed_node_info().relay_ids();
let nids = peer_info.node_ids();
if nids.contains_any(&rids) {
log_rtab!(debug "node can not be its own relay");
return None;
bail!("node can not be its own relay");
}
if !allow_invalid {
// verify signature
if !peer_info.signed_node_info().has_any_signature() {
log_rtab!(debug "signed node info for {:?} has no valid signature", peer_info.node_ids());
return None;
bail!(
"signed node info for {:?} has no valid signature",
peer_info.node_ids()
);
}
// verify signed node info is valid in this routing domain
if !self.signed_node_info_is_valid_in_routing_domain(
routing_domain,
peer_info.signed_node_info(),
) {
log_rtab!(debug "signed node info for {:?} not valid in the {:?} routing domain", peer_info.node_ids(), routing_domain);
return None;
bail!(
"signed node info for {:?} not valid in the {:?} routing domain",
peer_info.node_ids(),
routing_domain
);
}
}
let (node_ids, signed_node_info) = peer_info.destructure();
self.create_node_ref(outer_self, &node_ids, |_rti, e| {
let mut nr = self.create_node_ref(outer_self, &node_ids, |_rti, e| {
e.update_signed_node_info(routing_domain, signed_node_info);
})
.map(|mut nr| {
nr.set_filter(Some(
NodeRefFilter::new().with_routing_domain(routing_domain),
));
nr
})
})?;
nr.set_filter(Some(
NodeRefFilter::new().with_routing_domain(routing_domain),
));
Ok(nr)
}
/// Shortcut function to add a node to our routing table if it doesn't exist
@@ -856,17 +863,15 @@ impl RoutingTableInner {
node_id: TypedKey,
descriptor: ConnectionDescriptor,
timestamp: Timestamp,
) -> Option<NodeRef> {
let out = self.create_node_ref(outer_self, &TypedKeySet::from(node_id), |_rti, e| {
) -> EyreResult<NodeRef> {
let nr = self.create_node_ref(outer_self, &TypedKeySet::from(node_id), |_rti, e| {
// this node is live because it literally just connected to us
e.touch_last_seen(timestamp);
});
if let Some(nr) = &out {
// set the most recent node address for connection finding and udp replies
nr.locked_mut(self)
.set_last_connection(descriptor, timestamp);
}
out
})?;
// set the most recent node address for connection finding and udp replies
nr.locked_mut(self)
.set_last_connection(descriptor, timestamp);
Ok(nr)
}
//////////////////////////////////////////////////////////////////////
@@ -259,19 +259,27 @@ impl RoutingTable {
// Got peer info, let's add it to the routing table
for pi in peer_info {
// Register the node
if let Some(nr) =
self.register_node_with_peer_info(RoutingDomain::PublicInternet, pi, false)
{
// Add this our futures to process in parallel
for crypto_kind in VALID_CRYPTO_KINDS {
let routing_table = self.clone();
let nr = nr.clone();
unord.push(
// lets ask bootstrap to find ourselves now
async move { routing_table.reverse_find_node(crypto_kind, nr, true).await }
.instrument(Span::current()),
);
let nr = match self.register_node_with_peer_info(
RoutingDomain::PublicInternet,
pi,
false,
) {
Ok(nr) => nr,
Err(e) => {
log_rtab!(error "failed to register direct bootstrap peer info: {}", e);
continue;
}
};
// Add this our futures to process in parallel
for crypto_kind in VALID_CRYPTO_KINDS {
let routing_table = self.clone();
let nr = nr.clone();
unord.push(
// lets ask bootstrap to find ourselves now
async move { routing_table.reverse_find_node(crypto_kind, nr, true).await }
.instrument(Span::current()),
);
}
}
}
@@ -341,44 +349,46 @@ impl RoutingTable {
let pi = PeerInfo::new(bsrec.node_ids, sni);
if let Some(nr) =
self.register_node_with_peer_info(RoutingDomain::PublicInternet, pi, true)
{
// Add this our futures to process in parallel
for crypto_kind in VALID_CRYPTO_KINDS {
// Do we need to bootstrap this crypto kind?
let eckey = (RoutingDomain::PublicInternet, crypto_kind);
let cnt = entry_count.get(&eckey).copied().unwrap_or_default();
if cnt != 0 {
let nr =
match self.register_node_with_peer_info(RoutingDomain::PublicInternet, pi, true) {
Ok(nr) => nr,
Err(e) => {
log_rtab!(error "failed to register bootstrap peer info: {}", e);
continue;
}
// Bootstrap this crypto kind
let nr = nr.clone();
let routing_table = self.clone();
unord.push(
async move {
// Need VALID signed peer info, so ask bootstrap to find_node of itself
// which will ensure it has the bootstrap's signed peer info as part of the response
let _ = routing_table.find_target(crypto_kind, nr.clone()).await;
// Ensure we got the signed peer info
if !nr
.signed_node_info_has_valid_signature(RoutingDomain::PublicInternet)
{
log_rtab!(warn
"bootstrap at {:?} did not return valid signed node info",
nr
);
// If this node info is invalid, it will time out after being unpingable
} else {
// otherwise this bootstrap is valid, lets ask it to find ourselves now
routing_table.reverse_find_node(crypto_kind, nr, true).await
}
}
.instrument(Span::current()),
);
};
// Add this our futures to process in parallel
for crypto_kind in VALID_CRYPTO_KINDS {
// Do we need to bootstrap this crypto kind?
let eckey = (RoutingDomain::PublicInternet, crypto_kind);
let cnt = entry_count.get(&eckey).copied().unwrap_or_default();
if cnt != 0 {
continue;
}
// Bootstrap this crypto kind
let nr = nr.clone();
let routing_table = self.clone();
unord.push(
async move {
// Need VALID signed peer info, so ask bootstrap to find_node of itself
// which will ensure it has the bootstrap's signed peer info as part of the response
let _ = routing_table.find_target(crypto_kind, nr.clone()).await;
// Ensure we got the signed peer info
if !nr.signed_node_info_has_valid_signature(RoutingDomain::PublicInternet) {
log_rtab!(warn
"bootstrap at {:?} did not return valid signed node info",
nr
);
// If this node info is invalid, it will time out after being unpingable
} else {
// otherwise this bootstrap is valid, lets ask it to find ourselves now
routing_table.reverse_find_node(crypto_kind, nr, true).await
}
}
.instrument(Span::current()),
);
}
}
@@ -51,14 +51,19 @@ impl RoutingTable {
// The outbound relay is the host of the PWA
if let Some(outbound_relay_peerinfo) = intf::get_outbound_relay_peer().await {
// Register new outbound relay
if let Some(nr) = self.register_node_with_peer_info(
match self.register_node_with_peer_info(
RoutingDomain::PublicInternet,
outbound_relay_peerinfo,
false,
) {
info!("Outbound relay node selected: {}", nr);
editor.set_relay_node(nr);
got_outbound_relay = true;
Ok(nr) => {
log_rtab!("Outbound relay node selected: {}", nr);
editor.set_relay_node(nr);
got_outbound_relay = true;
}
Err(e) => {
log_rtab!(error "failed to register node with peer info: {}", e);
}
}
}
}