Start testing

Cargo.lock

@@ -2400,21 +2400,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 name = "lmd_ghost"
 version = "0.1.0"
 dependencies = [
- "beacon_chain 0.1.0",
- "bls 0.1.0",
- "criterion 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "eth2_ssz 0.1.2",
  "eth2_ssz_derive 0.1.0",
- "hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "rand 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "slot_clock 0.1.0",
  "store 0.1.0",
  "types 0.1.0",
- "yaml-rust 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]

eth2/lmd_ghost/Cargo.toml

@@ -11,14 +11,3 @@ types = { path = "../types" }
 itertools = "0.8.1"
 eth2_ssz = "0.1.2"
 eth2_ssz_derive = "0.1.0"
-
-[dev-dependencies]
-criterion = "0.3.0"
-hex = "0.3"
-yaml-rust = "0.4.3"
-bls = { path = "../utils/bls" }
-slot_clock = { path = "../utils/slot_clock" }
-beacon_chain = { path = "../../beacon_node/beacon_chain" }
-env_logger = "0.7.1"
-lazy_static = "1.4.0"
-rand = "0.7.2"

eth2/lmd_ghost/src/proto_array.rs

@@ -192,28 +192,35 @@ fn compute_deltas(
             // If the current vote is for the zero-hash there's no need to subtract the balance
             // from it. Additionally, the zero-hash will most likley not exist in `indices`.
             if vote.current_root != Hash256::zero() {
-                let current_delta_index = *indices
-                    .get(&vote.current_root)
-                    .ok_or_else(|| Error::NodeUnknown(vote.current_root))?;
+                // We ignore the vote if it is not known in `indices`. We assume that it is outside
+                // of our tree (i.e., pre-finalization) and therefore not interesting.
+                if let Some(current_delta_index) = indices.get(&vote.current_root).copied() {
+                    let delta = deltas
+                        .get_mut(current_delta_index)
+                        .ok_or_else(|| Error::InvalidNodeDelta(current_delta_index))?
+                        .checked_sub(old_balance as i64)
+                        .ok_or_else(|| Error::DeltaOverflow(current_delta_index))?;
 
-                deltas
-                    .get_mut(current_delta_index)
-                    .ok_or_else(|| Error::InvalidNodeDelta(current_delta_index))?
-                    .checked_sub(old_balance as i64)
-                    .ok_or_else(|| Error::DeltaOverflow(current_delta_index))?;
+                    // Array access safe due to check on previous line.
+                    deltas[current_delta_index] = delta;
+                }
             }
 
             // If the next vote is for the zero-hash there's no need to add the balance
             // to it. Additionally, the zero-hash will most likley not exist in `indices`.
             if vote.next_root != Hash256::zero() {
-                let next_delta_index = *indices
-                    .get(&vote.next_root)
-                    .ok_or_else(|| Error::NodeUnknown(vote.next_root))?;
-                deltas
-                    .get_mut(next_delta_index)
-                    .ok_or_else(|| Error::InvalidNodeDelta(next_delta_index))?
-                    .checked_add(new_balance as i64)
-                    .ok_or_else(|| Error::DeltaOverflow(next_delta_index))?;
+                // We ignore the vote if it is not known in `indices`. We assume that it is outside
+                // of our tree (i.e., pre-finalization) and therefore not interesting.
+                if let Some(next_delta_index) = indices.get(&vote.next_root).copied() {
+                    let delta = deltas
+                        .get(next_delta_index)
+                        .ok_or_else(|| Error::InvalidNodeDelta(next_delta_index))?
+                        .checked_add(new_balance as i64)
+                        .ok_or_else(|| Error::DeltaOverflow(next_delta_index))?;
+
+                    // Array access safe due to check on previous line.
+                    deltas[next_delta_index] = delta;
+                }
             }
 
             vote.current_root = vote.next_root;
@@ -538,6 +545,10 @@ where
         }
     }
 
+    pub fn push(&mut self, val: T) {
+        self.0.push(val)
+    }
+
     pub fn get(&mut self, i: usize) -> &T {
         self.ensure(i);
         &self.0[i]
@@ -552,3 +563,131 @@ where
         self.0.iter_mut()
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    /// Gives a hash that is not the zero hash (unless i is `usize::max_value)`.
+    fn hash_from_index(i: usize) -> Hash256 {
+        Hash256::from_low_u64_be(i as u64 + 1)
+    }
+
+    #[test]
+    fn compute_deltas_zero_hash() {
+        let validator_count: usize = 16;
+
+        let mut indices = HashMap::new();
+        let mut votes = ElasticList::default();
+        let mut old_balances = vec![];
+        let mut new_balances = vec![];
+
+        for i in 0..validator_count {
+            indices.insert(hash_from_index(i), i);
+            votes.push(VoteTracker {
+                current_root: Hash256::zero(),
+                next_root: Hash256::zero(),
+                next_epoch: Epoch::new(0),
+            });
+            old_balances.push(0);
+            new_balances.push(0);
+        }
+
+        let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances)
+            .expect("should compute deltas");
+
+        assert_eq!(
+            deltas.len(),
+            validator_count,
+            "deltas should have expected length"
+        );
+        assert_eq!(
+            deltas,
+            vec![0; validator_count],
+            "deltas should all be zero"
+        );
+    }
+
+    #[test]
+    fn compute_deltas_all_voted_the_same() {
+        const BALANCE: u64 = 42;
+
+        let validator_count: usize = 16;
+
+        let mut indices = HashMap::new();
+        let mut votes = ElasticList::default();
+        let mut old_balances = vec![];
+        let mut new_balances = vec![];
+
+        for i in 0..validator_count {
+            indices.insert(hash_from_index(i), i);
+            votes.push(VoteTracker {
+                current_root: Hash256::zero(),
+                next_root: hash_from_index(0),
+                next_epoch: Epoch::new(0),
+            });
+            old_balances.push(BALANCE);
+            new_balances.push(BALANCE);
+        }
+
+        let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances)
+            .expect("should compute deltas");
+
+        assert_eq!(
+            deltas.len(),
+            validator_count,
+            "deltas should have expected length"
+        );
+
+        for (i, delta) in deltas.into_iter().enumerate() {
+            if i == 0 {
+                assert_eq!(
+                    delta,
+                    BALANCE as i64 * validator_count as i64,
+                    "zero'th root should have a delta"
+                );
+            } else {
+                assert_eq!(delta, 0, "all other deltas should be zero");
+            }
+        }
+    }
+
+    #[test]
+    fn compute_deltas_different_votes() {
+        const BALANCE: u64 = 42;
+
+        let validator_count: usize = 16;
+
+        let mut indices = HashMap::new();
+        let mut votes = ElasticList::default();
+        let mut old_balances = vec![];
+        let mut new_balances = vec![];
+
+        for i in 0..validator_count {
+            indices.insert(hash_from_index(i), i);
+            votes.push(VoteTracker {
+                current_root: Hash256::zero(),
+                next_root: hash_from_index(i),
+                next_epoch: Epoch::new(0),
+            });
+            old_balances.push(BALANCE);
+            new_balances.push(BALANCE);
+        }
+
+        let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances)
+            .expect("should compute deltas");
+
+        assert_eq!(
+            deltas.len(),
+            validator_count,
+            "deltas should have expected length"
+        );
+
+        for delta in deltas.into_iter() {
+            assert_eq!(
+                delta, BALANCE as i64,
+                "each root should have the same delta"
+            );
+        }
+    }
+}

eth2/lmd_ghost/tests/test.rs

@@ -1,3 +1,6 @@
+// use lmd_ghost::ProtoArrayForkChoice;
+
+/*
 #![cfg(not(debug_assertions))]
 
 #[macro_use]
@@ -397,3 +400,4 @@ fn update_finalized_root_honest() {
 
     test_update_finalized_root(&harness.honest_roots)
 }
+*/