From 2dbffa1208f513701e7c413ed859162c5cd63f02 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 1 Feb 2023 19:14:07 +0000 Subject: [PATCH] tor-netdir: Use typed-index-collections for router status index --- Cargo.lock | 7 ++++++ crates/tor-netdir/Cargo.toml | 1 + crates/tor-netdir/src/lib.rs | 45 +++++++++++++++++++++++------------- 3 files changed, 37 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 022a58dd1..edb49a87b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4061,6 +4061,7 @@ dependencies = [ "tor-protover", "tor-units", "tracing", + "typed-index-collections", ] [[package]] @@ -4397,6 +4398,12 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3528ecfd12c466c6f163363caf2d02a71161dd5e1cc6ae7b34207ea2d42d81ed" +[[package]] +name = "typed-index-collections" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "183496e014253d15abbe6235677b1392dba2d40524c88938991226baa38ac7c4" + [[package]] name = "typenum" version = "1.16.0" diff --git a/crates/tor-netdir/Cargo.toml b/crates/tor-netdir/Cargo.toml index 0ccc687b3..a700c6096 100644 --- a/crates/tor-netdir/Cargo.toml +++ b/crates/tor-netdir/Cargo.toml @@ -55,6 +55,7 @@ tor-netdoc = { path = "../tor-netdoc", version = "0.6.1" } tor-protover = { path = "../tor-protover", version = "0.4.0" } tor-units = { path = "../tor-units", version = "0.4.1" } tracing = "0.1.18" +typed-index-collections = "3.1" [dev-dependencies] float_eq = "1.0.0" diff --git a/crates/tor-netdir/src/lib.rs b/crates/tor-netdir/src/lib.rs index dd6824d85..a3c271493 100644 --- a/crates/tor-netdir/src/lib.rs +++ b/crates/tor-netdir/src/lib.rs @@ -63,6 +63,7 @@ use tor_netdoc::doc::microdesc::{MdDigest, Microdesc}; use tor_netdoc::doc::netstatus::{self, MdConsensus, MdConsensusRouterStatus, RouterStatus}; use tor_netdoc::types::policy::PortPolicy; +use derive_more::{From, Into}; use futures::stream::BoxStream; use num_enum::{IntoPrimitive, TryFromPrimitive}; use serde::Deserialize; @@ -72,6 +73,7 @@ use std::ops::Deref; use std::sync::Arc; use strum::{EnumCount, EnumIter}; use tracing::warn; +use typed_index_collections::{TiSlice, TiVec}; #[cfg(feature = "onion-common")] use tor_hscrypto::{pk::BlindedOnionId, time::TimePeriod}; @@ -86,6 +88,20 @@ pub use err::OnionDirLookupError; use params::NetParameters; +/// Index into the consensus relays +/// +/// This is an index into the list of relays returned by +/// [`.c_relays()`](ConsensusRelays::c_relays) +/// (on the corresponding consensus or netdir). +/// +/// This is just a `usize` inside, but using a newtype prevents getting a relay index +/// confused with other kinds of slice indices or counts. +/// +/// If you are in a part of the code which needs to work with multiple consensuses, +/// the typechecking cannot tell if you try to index into the wrong consensus. +#[derive(Debug, From, Into, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] +pub(crate) struct RouterStatusIdx(usize); + /// Extension trait to provide index-type-safe `.c_relays()` method // // TODO: Really it would be better to have MdConsensns::relays() return TiSlice, @@ -93,16 +109,16 @@ use params::NetParameters; pub(crate) trait ConsensusRelays { /// Obtain the list of relays in the consensus // + fn c_relays(&self) -> &TiSlice; // TODO hs: This will return a TiSlice, shortly - fn c_relays(&self) -> &[MdConsensusRouterStatus]; } impl ConsensusRelays for MdConsensus { - fn c_relays(&self) -> &[MdConsensusRouterStatus] { - MdConsensus::relays(self) + fn c_relays(&self) -> &TiSlice { + TiSlice::from_ref(MdConsensus::relays(self)) } } impl ConsensusRelays for NetDir { - fn c_relays(&self) -> &[MdConsensusRouterStatus] { + fn c_relays(&self) -> &TiSlice { self.consensus.c_relays() } } @@ -278,10 +294,10 @@ pub struct NetDir { params: NetParameters, /// Map from routerstatus index (the position of a routerstatus within the /// consensus), to that routerstatus's microdescriptor (if we have one.) - mds: Vec>>, + mds: TiVec>>, /// Map from SHA256 of _missing_ microdescriptors to the position of their /// corresponding routerstatus indices within `consensus`. - rs_idx_by_missing: HashMap, + rs_idx_by_missing: HashMap, /// Map from ed25519 identity to index of the routerstatus within /// `self.consensus.relays()`. /// @@ -295,13 +311,13 @@ pub struct NetDir { /// `MdEntry::*::rsa_idx`, it might be cool to have references instead. /// But that would make this into a self-referential structure, /// which isn't possible in safe rust. - rs_idx_by_ed: HashMap, + rs_idx_by_ed: HashMap, /// Map from RSA identity to index of the routerstatus within /// `self.consensus.relays()`. /// /// This is constructed at the same time as the NetDir object, so it /// can be immutable. - rs_idx_by_rsa: Arc>, + rs_idx_by_rsa: Arc>, /// A hash ring describing the onion service directory. /// @@ -561,15 +577,13 @@ impl PartialNetDir { let rs_idx_by_missing = consensus .c_relays() - .iter() - .enumerate() + .iter_enumerated() .map(|(rs_idx, rs)| (*rs.md_digest(), rs_idx)) .collect(); let rs_idx_by_rsa = consensus .c_relays() - .iter() - .enumerate() + .iter_enumerated() .map(|(rs_idx, rs)| (*rs.rsa_identity(), rs_idx)) .collect(); @@ -598,7 +612,7 @@ impl PartialNetDir { let netdir = NetDir { consensus: Arc::new(consensus), params, - mds: vec![None; n_relays], + mds: vec![None; n_relays].into(), rs_idx_by_missing, rs_idx_by_rsa: Arc::new(rs_idx_by_rsa), rs_idx_by_ed: HashMap::with_capacity(n_relays), @@ -720,7 +734,7 @@ impl NetDir { fn relay_from_rs_and_idx<'a>( &'a self, rs: &'a netstatus::MdConsensusRouterStatus, - rs_idx: usize, + rs_idx: RouterStatusIdx, ) -> UncheckedRelay<'a> { debug_assert_eq!(self.c_relays()[rs_idx].rsa_identity(), rs.rsa_identity()); let md = self.mds[rs_idx].as_deref(); @@ -753,8 +767,7 @@ impl NetDir { // TODO: I'd like if we could memoize this so we don't have to // do so many hashtable lookups. self.c_relays() - .iter() - .enumerate() + .iter_enumerated() .map(move |(idx, rs)| self.relay_from_rs_and_idx(rs, idx)) } /// Return an iterator over all usable Relays.