guardmgr: Replace IdPair with RelayIds

I believe that this was the original motivation behind #428.

crates/tor-guardmgr/src/guard.rs

@@ -16,7 +16,7 @@ use crate::skew::SkewObservation;
 use crate::util::randomize_time;
 use crate::{ids::GuardId, GuardParams, GuardRestriction, GuardUsage};
 use crate::{ExternalActivity, FirstHopId, GuardUsageKind};
-use tor_linkspec::HasAddrs;
+use tor_linkspec::{HasAddrs, HasRelayIds};
 use tor_persist::{Futureproof, JsonValue};
 
 /// Tri-state to represent whether a guard is believed to be reachable or not.
@@ -393,8 +393,8 @@ impl Guard {
     /// Return true if this guard obeys a single restriction.
     fn obeys_restriction(&self, r: &GuardRestriction) -> bool {
         match r {
-            GuardRestriction::AvoidId(ed) => &self.id.0.ed25519 != ed,
-            GuardRestriction::AvoidAllIds(ids) => !ids.contains(&self.id.0.ed25519),
+            GuardRestriction::AvoidId(ed) => self.id.0.ed_identity() != ed,
+            GuardRestriction::AvoidAllIds(ids) => !ids.contains(self.id.0.ed_identity()),
         }
     }
 
@@ -424,7 +424,7 @@ impl Guard {
     /// download another microdescriptor before we can be certain whether this
     /// guard is listed or not.
     pub(crate) fn listed_in(&self, netdir: &NetDir) -> Option<bool> {
-        netdir.id_pair_listed(&self.id.0.ed25519, &self.id.0.rsa)
+        netdir.id_pair_listed(self.id.0.ed_identity(), self.id.0.rsa_identity())
     }
 
     /// Change this guard's status based on a newly received or newly
@@ -657,7 +657,7 @@ impl Guard {
     /// We use this information to decide whether we are about to sample
     /// too much of the network as guards.
     pub(crate) fn get_weight(&self, dir: &NetDir) -> Option<RelayWeight> {
-        dir.weight_by_rsa_id(&self.id.0.rsa, tor_netdir::WeightRole::Guard)
+        dir.weight_by_rsa_id(self.id.0.rsa_identity(), tor_netdir::WeightRole::Guard)
     }
 
     /// Return a [`FirstHop`](crate::FirstHop) object to represent this guard.
@@ -694,10 +694,10 @@ impl tor_linkspec::HasAddrs for Guard {
 
 impl tor_linkspec::HasRelayIds for Guard {
     fn ed_identity(&self) -> &Ed25519Identity {
-        &self.id.0.ed25519
+        self.id.0.ed_identity()
     }
     fn rsa_identity(&self) -> &RsaIdentity {
-        &self.id.0.rsa
+        self.id.0.rsa_identity()
     }
 }
 
@@ -813,8 +813,8 @@ mod test {
         let g = basic_guard();
 
         assert_eq!(g.guard_id(), &id);
-        assert_eq!(g.ed_identity(), &id.0.ed25519);
-        assert_eq!(g.rsa_identity(), &id.0.rsa);
+        assert_eq!(g.ed_identity(), id.0.ed_identity());
+        assert_eq!(g.rsa_identity(), id.0.rsa_identity());
         assert_eq!(g.addrs(), &["127.0.0.7:7777".parse().unwrap()]);
         assert_eq!(g.reachable(), Reachable::Unknown);
         assert_eq!(g.reachable(), Reachable::default());

crates/tor-guardmgr/src/ids.rs

@@ -2,28 +2,20 @@
 
 use derive_more::AsRef;
 use serde::{Deserialize, Serialize};
+use tor_linkspec::{HasRelayIds, RelayIds};
 use tor_llcrypto::pk;
 
-/// A pair of cryptographic identities used to distinguish a guard or fallback.
-#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub(crate) struct IdPair {
-    /// Ed25519 identity key for a guard
-    pub(crate) ed25519: pk::ed25519::Ed25519Identity,
-    /// RSA identity fingerprint for a guard
-    pub(crate) rsa: pk::rsa::RsaIdentity,
-}
-
 /// An identifier for a fallback directory cache.
 ///
 /// This is a separate type from GuardId and FirstHopId to avoid confusion
 /// about what kind of object we're identifying.
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, AsRef)]
-pub(crate) struct FallbackId(pub(crate) IdPair);
+pub(crate) struct FallbackId(pub(crate) RelayIds);
 
 impl FallbackId {
     /// Return a new, manually constructed `FallbackId`
     pub(crate) fn new(ed25519: pk::ed25519::Ed25519Identity, rsa: pk::rsa::RsaIdentity) -> Self {
-        Self(IdPair { ed25519, rsa })
+        Self(RelayIds::new(ed25519, rsa))
     }
     /// Extract a `FallbackId` from a ChanTarget object.
     pub(crate) fn from_chan_target<T: tor_linkspec::ChanTarget>(target: &T) -> Self {
@@ -37,12 +29,12 @@ impl FallbackId {
 /// about what kind of object we're identifying.
 #[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq, Hash, Ord, PartialOrd, AsRef)]
 #[serde(transparent)]
-pub(crate) struct GuardId(pub(crate) IdPair);
+pub(crate) struct GuardId(pub(crate) RelayIds);
 
 impl GuardId {
     /// Return a new, manually constructed `GuardId`
     pub(crate) fn new(ed25519: pk::ed25519::Ed25519Identity, rsa: pk::rsa::RsaIdentity) -> Self {
-        Self(IdPair { ed25519, rsa })
+        Self(RelayIds::new(ed25519, rsa))
     }
     /// Extract a `GuardId` from a ChanTarget object.
     pub(crate) fn from_chan_target<T: tor_linkspec::ChanTarget>(target: &T) -> Self {
@@ -79,18 +71,27 @@ impl From<FallbackId> for FirstHopId {
         Self(FirstHopIdInner::Fallback(id))
     }
 }
-impl AsRef<IdPair> for FirstHopId {
-    /// Return the inner IdPair for this object.
+impl AsRef<RelayIds> for FirstHopId {
+    /// Return the inner `RelayIds` for this object.
     ///
     /// Only use this when it's okay to erase the type information about
     /// whether this identifies a guard or a fallback.
-    fn as_ref(&self) -> &IdPair {
+    fn as_ref(&self) -> &RelayIds {
         match &self.0 {
             FirstHopIdInner::Guard(id) => id.as_ref(),
             FirstHopIdInner::Fallback(id) => id.as_ref(),
         }
     }
 }
+impl HasRelayIds for FirstHopId {
+    fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity {
+        self.as_ref().ed_identity()
+    }
+
+    fn rsa_identity(&self) -> &pk::rsa::RsaIdentity {
+        self.as_ref().rsa_identity()
+    }
+}
 
 impl FirstHopId {
     /// Return the relay in `netdir` that corresponds to this ID, if there
@@ -98,7 +99,6 @@ impl FirstHopId {
     //
     // We have to define this function so it'll be public.
     pub fn get_relay<'a>(&self, netdir: &'a tor_netdir::NetDir) -> Option<tor_netdir::Relay<'a>> {
-        let id = self.as_ref();
-        netdir.by_id_pair(&id.ed25519, &id.rsa)
+        netdir.by_ids(self)
     }
 }

crates/tor-guardmgr/src/lib.rs

@@ -1326,10 +1326,10 @@ impl tor_linkspec::HasAddrs for FirstHop {
 }
 impl tor_linkspec::HasRelayIds for FirstHop {
     fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity {
-        &self.id.as_ref().ed25519
+        self.id.ed_identity()
     }
     fn rsa_identity(&self) -> &pk::rsa::RsaIdentity {
-        &self.id.as_ref().rsa
+        self.id.rsa_identity()
     }
 }
 impl tor_linkspec::ChanTarget for FirstHop {}

crates/tor-guardmgr/src/sample.rs

@@ -883,6 +883,7 @@ impl<'a> From<GuardSample<'a>> for GuardSet {
 #[cfg(test)]
 mod test {
     #![allow(clippy::unwrap_used)]
+    use tor_linkspec::HasRelayIds;
     use tor_netdoc::doc::netstatus::{RelayFlags, RelayWeight};
 
     use super::*;
@@ -1316,7 +1317,7 @@ mod test {
 
         use tor_netdir::testnet;
         let netdir2 = testnet::construct_custom_netdir(|idx, bld| {
-            if idx == p_id1.0.ed25519.as_bytes()[0] as usize {
+            if idx == p_id1.0.ed_identity().as_bytes()[0] as usize {
                 bld.omit_md = true;
             }
         })