From e9ea7515bc6d6fd8029c23d7cb5ed4969c08bd09 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 12 Nov 2021 12:12:21 -0500
Subject: [PATCH] tor-dirmgr: put routerdesc storage behind a feature.

(We keep routerdescs in the schema, since we don't want _that_ to
fragment.)

Part of #125.
---
 crates/tor-dirmgr/Cargo.toml            |  2 +
 crates/tor-dirmgr/src/docid.rs          | 56 +++++++++++++++++--------
 crates/tor-dirmgr/src/lib.rs            | 21 +++++++---
 crates/tor-dirmgr/src/storage/sqlite.rs |  8 ++++
 doc/semver_status.md                    |  1 +
 5 files changed, 65 insertions(+), 23 deletions(-)

diff --git a/crates/tor-dirmgr/Cargo.toml b/crates/tor-dirmgr/Cargo.toml
index b623acee41..a671ce0e81 100644
--- a/crates/tor-dirmgr/Cargo.toml
+++ b/crates/tor-dirmgr/Cargo.toml
@@ -14,6 +14,8 @@ repository="https://gitlab.torproject.org/tpo/core/arti.git/"
 default = [ "mmap" ]
 mmap = [ "memmap2" ]
 static = [ "rusqlite/bundled" ]
+# (Incomplete) support for downloading and storing router descriptors
+routerdesc = [ ]
 
 [dependencies]
 retry-error = { path="../retry-error", version = "0.0.1"}
diff --git a/crates/tor-dirmgr/src/docid.rs b/crates/tor-dirmgr/src/docid.rs
index dcc187ee1e..c2fb4e1729 100644
--- a/crates/tor-dirmgr/src/docid.rs
+++ b/crates/tor-dirmgr/src/docid.rs
@@ -4,9 +4,9 @@
 use std::{borrow::Borrow, collections::HashMap};
 
 use tor_dirclient::request;
-use tor_netdoc::doc::{
-    authcert::AuthCertKeyIds, microdesc::MdDigest, netstatus::ConsensusFlavor, routerdesc::RdDigest,
-};
+#[cfg(feature = "routerdesc")]
+use tor_netdoc::doc::routerdesc::RdDigest;
+use tor_netdoc::doc::{authcert::AuthCertKeyIds, microdesc::MdDigest, netstatus::ConsensusFlavor};
 
 /// The identity of a single document, in enough detail to load it
 /// from storage.
@@ -27,6 +27,7 @@ pub enum DocId {
     Microdesc(MdDigest),
     /// A request for the router descriptor of a public relay, by SHA1
     /// digest.
+    #[cfg(feature = "routerdesc")]
     RouterDesc(RdDigest),
 }
 
@@ -44,6 +45,7 @@ pub(crate) enum DocType {
     /// A microdescriptor
     Microdesc,
     /// A router descriptor.
+    #[cfg(feature = "routerdesc")]
     RouterDesc,
 }
 
@@ -56,6 +58,7 @@ impl DocId {
             LatestConsensus { flavor: f, .. } => T::Consensus(*f),
             AuthCert(_) => T::AuthCert,
             Microdesc(_) => T::Microdesc,
+            #[cfg(feature = "routerdesc")]
             RouterDesc(_) => T::RouterDesc,
         }
     }
@@ -72,6 +75,7 @@ pub(crate) enum ClientRequest {
     /// Request for one or more microdescriptors
     Microdescs(request::MicrodescRequest),
     /// Request for one or more router descriptors
+    #[cfg(feature = "routerdesc")]
     RouterDescs(request::RouterDescRequest),
 }
 
@@ -83,6 +87,7 @@ impl ClientRequest {
             Consensus(a) => a,
             AuthCert(a) => a,
             Microdescs(a) => a,
+            #[cfg(feature = "routerdesc")]
             RouterDescs(a) => a,
         }
     }
@@ -133,6 +138,7 @@ pub(crate) enum DocQuery {
     /// A request for microdescriptors
     Microdesc(Vec<MdDigest>),
     /// A request for router descriptors
+    #[cfg(feature = "routerdesc")]
     RouterDesc(Vec<RdDigest>),
 }
 
@@ -149,6 +155,7 @@ impl DocQuery {
             },
             DocId::AuthCert(_) => Self::AuthCert(Vec::new()),
             DocId::Microdesc(_) => Self::Microdesc(Vec::new()),
+            #[cfg(feature = "routerdesc")]
             DocId::RouterDesc(_) => Self::RouterDesc(Vec::new()),
         }
     }
@@ -159,6 +166,7 @@ impl DocQuery {
             (Self::LatestConsensus { .. }, DocId::LatestConsensus { .. }) => {}
             (Self::AuthCert(ids), DocId::AuthCert(id)) => ids.push(id),
             (Self::Microdesc(ids), DocId::Microdesc(id)) => ids.push(id),
+            #[cfg(feature = "routerdesc")]
             (Self::RouterDesc(ids), DocId::RouterDesc(id)) => ids.push(id),
             (_, _) => panic!(),
         }
@@ -180,6 +188,7 @@ impl DocQuery {
                 v.sort_unstable();
                 v[..].chunks(N).map(|s| Microdesc(s.to_vec())).collect()
             }
+            #[cfg(feature = "routerdesc")]
             RouterDesc(mut v) => {
                 v.sort_unstable();
                 v[..].chunks(N).map(|s| RouterDesc(s.to_vec())).collect()
@@ -236,6 +245,7 @@ mod test {
         assert_eq!(DocId::AuthCert(auth_id).doctype(), DocType::AuthCert);
 
         assert_eq!(DocId::Microdesc([22; 32]).doctype(), DocType::Microdesc);
+        #[cfg(feature = "routerdesc")]
         assert_eq!(DocId::RouterDesc([42; 20]).doctype(), DocType::RouterDesc);
     }
 
@@ -244,6 +254,7 @@ mod test {
         let mut ids = Vec::new();
         for byte in 0..=255 {
             ids.push(DocId::Microdesc([byte; 32]));
+            #[cfg(feature = "routerdesc")]
             ids.push(DocId::RouterDesc([byte; 20]));
             ids.push(DocId::AuthCert(AuthCertKeyIds {
                 id_fingerprint: [byte; 20].into(),
@@ -257,7 +268,10 @@ mod test {
         ids.push(consensus_q);
 
         let split = partition_by_type(ids);
+        #[cfg(feature = "routerdesc")]
         assert_eq!(split.len(), 4); // 4 distinct types.
+        #[cfg(not(feature = "routerdesc"))]
+        assert_eq!(split.len(), 3); // 3 distinct types.
 
         let q = split
             .get(&DocType::Consensus(ConsensusFlavor::Microdesc))
@@ -267,9 +281,11 @@ mod test {
         let q = split.get(&DocType::Microdesc).unwrap();
         assert!(matches!(q, DocQuery::Microdesc(v) if v.len() == 256));
 
-        let q = split.get(&DocType::RouterDesc).unwrap();
-        assert!(matches!(q, DocQuery::RouterDesc(v) if v.len() == 256));
-
+        #[cfg(feature = "routerdesc")]
+        {
+            let q = split.get(&DocType::RouterDesc).unwrap();
+            assert!(matches!(q, DocQuery::RouterDesc(v) if v.len() == 256));
+        }
         let q = split.get(&DocType::AuthCert).unwrap();
         assert!(matches!(q, DocQuery::AuthCert(v) if v.len() == 256));
     }
@@ -300,20 +316,24 @@ mod test {
         assert_eq!(found_ids, ids);
 
         // Test routerdescs.
-        let ids: HashSet<RdDigest> = (0..1001).into_iter().map(|_| rng.gen()).collect();
-        let split = DocQuery::RouterDesc(ids.clone().into_iter().collect()).split_for_download();
-        assert_eq!(split.len(), 3);
-        let mut found_ids = HashSet::new();
-        for q in split {
-            match q {
-                DocQuery::RouterDesc(ids) => ids.into_iter().for_each(|id| {
-                    found_ids.insert(id);
-                }),
-                _ => panic!("Wrong type."),
+        #[cfg(feature = "routerdesc")]
+        {
+            let ids: HashSet<RdDigest> = (0..1001).into_iter().map(|_| rng.gen()).collect();
+            let split =
+                DocQuery::RouterDesc(ids.clone().into_iter().collect()).split_for_download();
+            assert_eq!(split.len(), 3);
+            let mut found_ids = HashSet::new();
+            for q in split {
+                match q {
+                    DocQuery::RouterDesc(ids) => ids.into_iter().for_each(|id| {
+                        found_ids.insert(id);
+                    }),
+                    _ => panic!("Wrong type."),
+                }
             }
+            assert_eq!(found_ids.len(), 1001);
+            assert_eq!(&found_ids, &ids);
         }
-        assert_eq!(found_ids.len(), 1001);
-        assert_eq!(&found_ids, &ids);
 
         // Test authcerts.
         let ids: HashSet<AuthCertKeyIds> = (0..2500)
diff --git a/crates/tor-dirmgr/src/lib.rs b/crates/tor-dirmgr/src/lib.rs
index 38b7918a55..2b3a94b938 100644
--- a/crates/tor-dirmgr/src/lib.rs
+++ b/crates/tor-dirmgr/src/lib.rs
@@ -20,6 +20,9 @@
 //! reading large directory objects from disk.
 //!
 //! `static` -- Try to link with a static copy of sqlite3.
+//!
+//! `routerdesc` -- (Incomplete) support for downloading and storing
+//!      router descriptors.
 
 #![deny(missing_docs)]
 #![warn(noop_method_call)]
@@ -574,6 +577,7 @@ impl<R: Runtime> DirMgr<R> {
                         .map(|(id, md)| (DocId::Microdesc(id), DocumentText::from_string(md))),
                 );
             }
+            #[cfg(feature = "routerdesc")]
             RouterDesc(digests) => result.extend(
                 store
                     .routerdescs(digests)?
@@ -602,6 +606,7 @@ impl<R: Runtime> DirMgr<R> {
                 DocQuery::Microdesc(ids) => {
                     res.push(ClientRequest::Microdescs(ids.into_iter().collect()));
                 }
+                #[cfg(feature = "routerdesc")]
                 DocQuery::RouterDesc(ids) => {
                     res.push(ClientRequest::RouterDescs(ids.into_iter().collect()));
                 }
@@ -823,6 +828,7 @@ mod test {
                     )
                     .unwrap();
 
+                #[cfg(feature = "routerdesc")]
                 store
                     .store_routerdescs(vec![("Fake rd1", now, &d4), ("Fake rd2", now, &d5)])
                     .unwrap();
@@ -874,10 +880,10 @@ mod test {
                     DocId::Microdesc(d3),
                     d_bogus,
                     DocId::AuthCert(certid2),
+                    #[cfg(feature = "routerdesc")]
                     DocId::RouterDesc(d5),
                 ])
                 .unwrap();
-            assert_eq!(res.len(), 4);
             assert_eq!(
                 res.get(&DocId::Microdesc(d2)).unwrap().as_str(),
                 Ok("Fake micro 2")
@@ -891,6 +897,7 @@ mod test {
                 res.get(&DocId::AuthCert(certid2)).unwrap().as_str(),
                 Ok("Fake certificate two")
             );
+            #[cfg(feature = "routerdesc")]
             assert_eq!(
                 res.get(&DocId::RouterDesc(d5)).unwrap().as_str(),
                 Ok("Fake rd2")
@@ -961,6 +968,7 @@ mod test {
                 sk_fingerprint: [100; 20].into(),
             };
             let mut rng = rand::thread_rng();
+            #[cfg(feature = "routerdesc")]
             let rd_ids: Vec<[u8; 20]> = (0..1000).map(|_| rng.gen()).collect();
             let md_ids: Vec<[u8; 32]> = (0..1000).map(|_| rng.gen()).collect();
 
@@ -982,10 +990,13 @@ mod test {
             assert!(matches!(reqs[0], ClientRequest::Microdescs(_)));
 
             // Try a bunch of rds.
-            let query = DocQuery::RouterDesc(rd_ids);
-            let reqs = mgr.query_into_requests(query).unwrap();
-            assert_eq!(reqs.len(), 2);
-            assert!(matches!(reqs[0], ClientRequest::RouterDescs(_)));
+            #[cfg(feature = "routerdesc")]
+            {
+                let query = DocQuery::RouterDesc(rd_ids);
+                let reqs = mgr.query_into_requests(query).unwrap();
+                assert_eq!(reqs.len(), 2);
+                assert!(matches!(reqs[0], ClientRequest::RouterDescs(_)));
+            }
         })
     }
 
diff --git a/crates/tor-dirmgr/src/storage/sqlite.rs b/crates/tor-dirmgr/src/storage/sqlite.rs
index 3e52c9efb1..4fa5754ec5 100644
--- a/crates/tor-dirmgr/src/storage/sqlite.rs
+++ b/crates/tor-dirmgr/src/storage/sqlite.rs
@@ -10,6 +10,7 @@ use crate::{Error, Result};
 use tor_netdoc::doc::authcert::AuthCertKeyIds;
 use tor_netdoc::doc::microdesc::MdDigest;
 use tor_netdoc::doc::netstatus::{ConsensusFlavor, Lifetime};
+#[cfg(feature = "routerdesc")]
 use tor_netdoc::doc::routerdesc::RdDigest;
 
 use std::collections::HashMap;
@@ -540,6 +541,9 @@ impl SqliteStore {
     }
 
     /// Read all the microdescriptors listed in `input` from the cache.
+    ///
+    /// Only available when the `routerdesc` feature is present.
+    #[cfg(feature = "routerdesc")]
     pub(crate) fn routerdescs<'a, I>(&self, input: I) -> Result<HashMap<RdDigest, String>>
     where
         I: IntoIterator<Item = &'a RdDigest>,
@@ -607,6 +611,7 @@ impl SqliteStore {
     }
 
     /// Store every router descriptors in `input` into the cache.
+    #[cfg(feature = "routerdesc")]
     #[allow(unused)]
     pub(crate) fn store_routerdescs<'a, I>(&mut self, input: I) -> Result<()>
     where
@@ -847,6 +852,7 @@ const FIND_MD: &str = "
 ";
 
 /// Query: find the router descriptors with a given hex-encoded sha1 digest
+#[cfg(feature = "routerdesc")]
 const FIND_RD: &str = "
   SELECT contents
   FROM RouterDescs
@@ -886,6 +892,7 @@ const INSERT_MD: &str = "
 
 /// Query: Add a new router descriptor
 #[allow(unused)]
+#[cfg(feature = "routerdesc")]
 const INSERT_RD: &str = "
   INSERT OR REPLACE INTO RouterDescs ( sha1_digest, published, contents )
   VALUES ( ?, ?, ? );
@@ -1214,6 +1221,7 @@ mod test {
     }
 
     #[test]
+    #[cfg(feature = "routerdesc")]
     fn routerdescs() -> Result<()> {
         let (_tmp_dir, mut store) = new_empty()?;
 
diff --git a/doc/semver_status.md b/doc/semver_status.md
index 0e03873345..4b892e0906 100644
--- a/doc/semver_status.md
+++ b/doc/semver_status.md
@@ -21,6 +21,7 @@ We can delete older sections here after we bump the releases.
 
 tor-client: MODIFIED
 tor-dirclient: MODIFIED
+tor-dirmgr: BREAKING
 tor-llcrypto: BREAKING
 tor-netdoc: BREAKING
 tor-persist: BREAKING if `testing` feature is enabled.
-- 
GitLab