From 27f4657ff1a6f302228894e95c2d7cd7d45940e8 Mon Sep 17 00:00:00 2001
From: eta <tor@eta.st>
Date: Tue, 10 May 2022 11:29:25 +0100
Subject: [PATCH] tor-dirmgr: small fixups for the bootstrapping refactor
- Some FIXMEs got removed or amended.
- AddMicrodescs now yields a mutable reference, so we can use .drain()
and reuse the allocation.
- Some panics were downgraded to debug_asserts.
---
crates/tor-dirmgr/src/bootstrap.rs | 8 +++-----
crates/tor-dirmgr/src/state.rs | 17 ++++++-----------
crates/tor-dirmgr/src/storage.rs | 1 +
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/crates/tor-dirmgr/src/bootstrap.rs b/crates/tor-dirmgr/src/bootstrap.rs
index 3a80327f04..f669bc8889 100644
--- a/crates/tor-dirmgr/src/bootstrap.rs
+++ b/crates/tor-dirmgr/src/bootstrap.rs
@@ -113,7 +113,6 @@ fn load_documents_from_store(
/// Construct an appropriate ClientRequest to download a consensus
/// of the given flavor.
-// FIXME(eta): remove pub
pub(crate) fn make_consensus_request(
now: SystemTime,
flavor: ConsensusFlavor,
@@ -144,7 +143,6 @@ pub(crate) fn make_consensus_request(
}
/// Construct a set of `ClientRequest`s in order to fetch the documents in `docs`.
-// FIXME(eta): remove pub
pub(crate) fn make_requests_for_documents<R: Runtime>(
rt: &R,
docs: &[DocId],
@@ -390,7 +388,7 @@ fn apply_netdir_changes<R: Runtime>(
}
NetDirChange::AddMicrodescs(mds) => {
dirmgr.netdir.mutate(|netdir| {
- for md in mds {
+ for md in mds.drain(..) {
netdir.add_microdesc(md);
}
Ok(())
@@ -506,7 +504,7 @@ pub(crate) async fn download<R: Runtime>(
continue 'next_state;
}
// Apply any netdir changes that the state gives us.
- // FIXME(eta): Don't throw away the return value (once we actually use this API).
+ // TODO(eta): Consider deprecating state.is_ready().
{
let dirmgr = upgrade_weak_ref(&dirmgr)?;
let mut store = dirmgr.store.lock().expect("store lock poisoned");
@@ -572,7 +570,7 @@ pub(crate) async fn download<R: Runtime>(
};
// Apply any netdir changes that the state gives us.
- // FIXME(eta): Don't throw away the return value (once we actually use this API).
+ // TODO(eta): Consider deprecating state.is_ready().
{
let dirmgr = upgrade_weak_ref(&dirmgr)?;
let mut store = dirmgr.store.lock().expect("store lock poisoned");
diff --git a/crates/tor-dirmgr/src/state.rs b/crates/tor-dirmgr/src/state.rs
index f009f5c2f2..ebe461ed65 100644
--- a/crates/tor-dirmgr/src/state.rs
+++ b/crates/tor-dirmgr/src/state.rs
@@ -65,7 +65,7 @@ pub(crate) enum NetDirChange<'a> {
consensus_meta: &'a ConsensusMeta,
},
/// Add the provided microdescriptors to the current `NetDir`.
- AddMicrodescs(Vec<Microdesc>),
+ AddMicrodescs(&'a mut Vec<Microdesc>),
}
/// A "state" object used to represent our progress in downloading a
@@ -663,12 +663,9 @@ impl MdReceiver for PendingNetDir {
} => {
let wanted = missing_microdescs.remove(md.digest());
if let Some(nd) = netdir.as_mut() {
- let digest = *md.digest();
let nd_wanted = nd.add_microdesc(md);
- if wanted != nd_wanted {
- // This shouldn't ever happen; if it does, our invariants are violated.
- panic!("NetDir changed its mind about wanting microdesc {:?} (pending {}, nd {})", digest, wanted, nd_wanted);
- }
+ // This shouldn't ever happen; if it does, our invariants are violated.
+ debug_assert_eq!(wanted, nd_wanted);
nd_wanted
} else {
collected_microdescs.push(md);
@@ -688,10 +685,8 @@ impl MdReceiver for PendingNetDir {
..
} => {
if let Some(nd) = netdir.as_ref() {
- if nd.n_missing() != missing_microdescs.len() {
- // This shouldn't ever happen; if it does, our invariants are violated.
- panic!("PendingNetDir lost track of wanted microdescs (netdir wants {}, us {})", nd.n_missing(), missing_microdescs.len());
- }
+ // This shouldn't ever happen; if it does, our invariants are violated.
+ debug_assert_eq!(nd.n_missing(), missing_microdescs.len());
nd.n_missing()
} else {
missing_microdescs.len()
@@ -817,7 +812,7 @@ impl<R: Runtime> DirState for GetMicrodescsState<R> {
} else {
collected_microdescs
.is_empty()
- .then(|| NetDirChange::AddMicrodescs(mem::take(collected_microdescs)))
+ .then(move || NetDirChange::AddMicrodescs(collected_microdescs))
}
}
_ => None,
diff --git a/crates/tor-dirmgr/src/storage.rs b/crates/tor-dirmgr/src/storage.rs
index 5f598285b3..e451f36d6e 100644
--- a/crates/tor-dirmgr/src/storage.rs
+++ b/crates/tor-dirmgr/src/storage.rs
@@ -279,6 +279,7 @@ pub(crate) trait Store {
fn store_routerdescs(&mut self, digests: &[(&str, SystemTime, &RdDigest)]) -> Result<()>;
}
+// TODO(eta): maybe use something like the `delegate` crate to autogenerate this?
impl Store for DynStore {
fn is_readonly(&self) -> bool {
self.deref().is_readonly()
--
GitLab