Fix up replace function, remove recyclable

crates/lox-library/src/bridge_table.rs

@@ -213,7 +213,7 @@ impl BridgeLine {
 #[serde_as]
 #[derive(Debug, Default, Serialize, Deserialize)]
 pub struct BridgeTable {
-// All structures in the bridgetable are indexed by counter
+    // All structures in the bridgetable are indexed by counter
     pub counter: u32,
     pub keys: HashMap<u32, [u8; 16]>,
     pub buckets: HashMap<u32, [BridgeLine; MAX_BRIDGES_PER_BUCKET]>,
@@ -226,13 +226,6 @@ pub struct BridgeTable {
     /// them.  When a new Migration credential is needed, a bucket is
     /// removed from this set and used for that purpose.
     pub spares: HashSet<u32>,
-    /// bucket ids of "recyclable" buckets.  These buckets have not been handed out
-    /// to users, nor do they have any Migration credentials pointing to
-    /// them. When a single bridge is needed and there are no more readily available bridges,
-    /// bridges are taken from a bucket of hot spares, making the unallocated spare bucket empty
-    /// but still useable as it has not been handed out previously.
-    pub recyclable: HashSet<u32>,
-    /// In some instances a single bridge may need to be added to a bucket
     /// In some instances a single bridge may need to be added to a bucket
     /// In that case, a spare bucket will be removed from the set of spare bridges. One
     pub unallocated_bridges: Vec<BridgeLine>,
@@ -251,26 +244,6 @@ impl BridgeTable {
         self.buckets.len()
     }
 
-// TODO Don't need with hashmaps, but remove unused buckets
-    pub fn recycle_bucket(&mut self, bucket: &[BridgeLine; MAX_BRIDGES_PER_BUCKET], index: u32) {
-        // Pick a random key to encrypt this bucket
-        let mut rng = rand::thread_rng();
-        let mut key: [u8; 16] = [0; 16];
-        rng.fill_bytes(&mut key);
-        let key = self.keys.get(&index).unwrap();
-        let bucket = *self.buckets.get(&index).unwrap();
-        for (i, bridgeline) in bucket.iter().enumerate() {
-            if bridgeline.port > 0 {
-                if let Some(v) = self.reachable.get_mut(bridgeline) {
-                    v.push((index, i));
-                } else {
-                    let v = vec![(index, i)];
-                    self.reachable.insert(*bridgeline, v);
-                }
-            }
-        }
-    }
-
     /// Append a new bucket to the bridge table, returning its index
     pub fn new_bucket(&mut self, bucket: &[BridgeLine; MAX_BRIDGES_PER_BUCKET]) -> u32 {
         // Pick a random key to encrypt this bucket
@@ -308,7 +281,7 @@ impl BridgeTable {
         self.encbuckets.clear();
         // We want id to be a u32, so we use .zip(0u32..) instead of
         // enumerate()
-        for (uid, key) in self.keys.iter(){
+        for (uid, key) in self.keys.iter() {
             let bucket = self.buckets.get(uid).unwrap();
             let mut encbucket: [u8; ENC_BUCKET_BYTES] = [0; ENC_BUCKET_BYTES];
             let plainbucket: [u8; BUCKET_BYTES] = BridgeLine::bucket_encode(

crates/lox-library/src/lib.rs

@@ -301,13 +301,7 @@ impl BridgeAuth {
         bdb: &mut BridgeDb,
     ) {
         let bnum: u32;
-        if self.bridge_table.recyclable.is_empty() {
         bnum = self.bridge_table.new_bucket(&bridges);
-        } else {
-            bnum = *self.bridge_table.recyclable.iter().next().unwrap();
-            self.bridge_table.recyclable.remove(&bnum);
-            self.bridge_table.recycle_bucket(&bridges, bnum)
-        }
         let mut single = [BridgeLine::default(); MAX_BRIDGES_PER_BUCKET];
         for b in bridges.iter() {
             single[0] = *b;
@@ -320,13 +314,7 @@ impl BridgeAuth {
     /// Insert a hot spare bucket of bridges
     pub fn add_spare_bucket(&mut self, bucket: [BridgeLine; MAX_BRIDGES_PER_BUCKET]) {
         let bnum: u32;
-        if self.bridge_table.recyclable.is_empty() {
         bnum = self.bridge_table.new_bucket(&bucket);
-        } else {
-            bnum = *self.bridge_table.recyclable.iter().next().unwrap();
-            self.bridge_table.recyclable.remove(&bnum);
-            self.bridge_table.recycle_bucket(&bucket, bnum)
-        }
         self.bridge_table.spares.insert(bnum);
     }
 
@@ -382,22 +370,16 @@ impl BridgeAuth {
                         println!("Bucket num: {:?} and offset: {:?}", bucketnum, offset);
                         let bridgelines = match self.bridge_table.buckets.get(bucketnum) {
                             Some(bridgelines) => *bridgelines,
-                            None => return res
+                            None => return res,
                         };
-                        assert!(
+                        assert!(bridgelines[*offset] == reachable_bridge.0);
-                            bridgelines[*offset]
-                                == reachable_bridge.0
-                        );
                         bridgelines[*offset] = *bridge;
                         self.bridge_table.buckets.insert(*bucketnum, bridgelines);
                         let bridgelines = match self.bridge_table.buckets.get(bucketnum) {
                             Some(bridgelines) => *bridgelines,
-                            None => return res
+                            None => return res,
                         };
-                        assert!(
+                        assert!(bridgelines[*offset] != reachable_bridge.0);
-                            bridgelines[*offset]
-                                != reachable_bridge.0
-                        );
                     }
                     res = true;
                 } else {
@@ -448,7 +430,8 @@ impl BridgeAuth {
                     for (bucketnum, offset) in positions.iter() {
                         let bridgelines = match self.bridge_table.buckets.get(bucketnum) {
                             Some(bridgelines) => *bridgelines,
-                            None => return ReplaceSuccess::NotFound,
+                            // This should not happen if the rest of the function is correct, we can assume unwrap will succeed
+                            None => return ReplaceSuccess::NotReplaced,
                         };
                         assert!(bridgelines[*offset] == *bridge);
                         bridgelines[*offset] = *replacement;
@@ -465,7 +448,13 @@ impl BridgeAuth {
                     let spare = *self.bridge_table.spares.iter().next().unwrap();
                     self.bridge_table.spares.remove(&spare);
                     // Get the actual bridges from the spare bucket
-                    let spare_bucket = self.bridge_table.buckets[&spare];
+                    let spare_bucket = match self.bridge_table.buckets.remove(&spare) {
+                        Some(spare_bucket) => spare_bucket,
+                        // This should not happen if the rest of the functions are correct, we can assume unwrap will succeed
+                        None => return ReplaceSuccess::NotReplaced,
+                    };
+                    // Remove the spare bucket uid from the keys map
+                    self.bridge_table.keys.remove(&spare);
                     let mut replacement: &BridgeLine = &BridgeLine::default();
                     // Make the first spare the replacement bridge, add the others to the set of
                     // unallocated_bridges
@@ -479,7 +468,13 @@ impl BridgeAuth {
                         }
                     }
                     for (bucketnum, offset) in positions.iter() {
-                        self.bridge_table.buckets[*bucketnum as usize][*offset] = *replacement;
+                        let bridgelines = match self.bridge_table.buckets.get(&bucketnum) {
+                            Some(bridgelines) => *bridgelines,
+                            None => return ReplaceSuccess::NotReplaced,
+                        };
+                        assert!(bridgelines[*offset] == *bridge);
+                        bridgelines[*offset] = *replacement;
+                        self.bridge_table.buckets.insert(*bucketnum, bridgelines);
                         self.bridge_table
                             .reachable
                             .insert(*replacement, positions.clone());

crates/lox-library/src/tests.rs

@@ -738,8 +738,8 @@ fn test_bridge_replace() {
 
         // Randomly select a bridge to replace
         let table_size = th.ba.bridge_table.buckets.len();
-        let num = rand::thread_rng().gen_range(0, table_size - 1);
+        let num = rand::thread_rng().gen_range(0, table_size - 1) as u32;
-        let replaceable_bucket = th.ba.bridge_table.buckets.get(num).unwrap().clone();
+        let replaceable_bucket = th.ba.bridge_table.buckets.get(&num).unwrap().clone();
         let replacement_bridge = &replaceable_bucket[0];
         assert!(
             th.ba
@@ -918,7 +918,8 @@ fn test_mark_unreachable() {
     println!("openinv = {:?}\n", th.bdb.openinv_buckets);
 
     // Mark a bridge in an untrusted bucket as unreachable
-    let b6 = th.ba.bridge_table.buckets[6][0];
+    let bucket6 = th.ba.bridge_table.buckets.get(&6u32).unwrap();
+    let b6 = bucket6[0];
     th.ba.bridge_unreachable(&b6, &mut th.bdb);
 
     println!("spares = {:?}", th.ba.bridge_table.spares);
@@ -928,7 +929,8 @@ fn test_mark_unreachable() {
 
     // Mark another bridge grouped to the same trusted bucket as
     // unreachable
-    let b7 = th.ba.bridge_table.buckets[7][0];
+    let bucket7 = th.ba.bridge_table.buckets.get(&7u32).unwrap();
+    let b7 = bucket7[0];
     th.ba.bridge_unreachable(&b7, &mut th.bdb);
 
     println!("spares = {:?}", th.ba.bridge_table.spares);
@@ -947,8 +949,10 @@ fn test_mark_unreachable() {
         .1;
 
     // Block two of the bridges in that target bucket
-    let bt1 = th.ba.bridge_table.buckets[target as usize][1];
+    let bucket1 = th.ba.bridge_table.buckets.get(&target).unwrap();
-    let bt2 = th.ba.bridge_table.buckets[target as usize][2];
+    let bt1 = bucket1[1];
+    let bucket2 = th.ba.bridge_table.buckets.get(&target).unwrap();
+    let bt2 = bucket2[2];
     th.ba.bridge_unreachable(&bt1, &mut th.bdb);
     th.ba.bridge_unreachable(&bt2, &mut th.bdb);