Merge pull request #62 from ralexstokes/master

Simplifies the boolean-bitfield implementation to use `bit-vec` crate

beacon_chain/types/src/attestation_record.rs

@@ -31,7 +31,7 @@ impl Encodable for AttestationRecord {
         s.append(&self.shard_id);
         s.append_vec(&self.oblique_parent_hashes);
         s.append(&self.shard_block_hash);
-        s.append_vec(&self.attester_bitfield.to_be_vec());
+        s.append(&self.attester_bitfield);
         s.append(&self.justified_slot);
         s.append(&self.justified_block_hash);
         s.append_vec(&self.aggregate_sig.as_bytes());
@@ -103,7 +103,7 @@ mod tests {
             shard_id: 9,
             oblique_parent_hashes: vec![Hash256::from(&vec![14; 32][..])],
             shard_block_hash: Hash256::from(&vec![15; 32][..]),
-            attester_bitfield: Bitfield::from(&vec![17; 42][..]),
+            attester_bitfield: Bitfield::from_bytes(&vec![17; 42][..]),
             justified_slot: 19,
             justified_block_hash: Hash256::from(&vec![15; 32][..]),
             aggregate_sig: AggregateSignature::new(),

beacon_chain/types/src/lib.rs

@@ -14,7 +14,6 @@ pub mod special_record;
 pub mod validator_record;
 pub mod validator_registration;
 
-use self::boolean_bitfield::BooleanBitfield;
 use self::ethereum_types::{H160, H256, U256};
 use std::collections::HashMap;
 
@@ -32,7 +31,8 @@ pub use validator_registration::ValidatorRegistration;
 pub type Hash256 = H256;
 pub type Address = H160;
 pub type EthBalance = U256;
-pub type Bitfield = BooleanBitfield;
+pub type Bitfield = boolean_bitfield::BooleanBitfield;
+pub type BitfieldError = boolean_bitfield::Error;
 
 /// Maps a (slot, shard_id) to attestation_indices.
 pub type AttesterMap = HashMap<(u64, u16), Vec<usize>>;

beacon_chain/utils/boolean-bitfield/Cargo.toml

@@ -5,3 +5,4 @@ authors = ["Paul Hauner <paul@paulhauner.com>"]
 
 [dependencies]
 ssz = { path = "../ssz" }
+bit-vec = "0.5.0"

beacon_chain/utils/boolean-bitfield/README.md

@@ -1,7 +1,3 @@
 # Boolean Bitfield
 
-A work-in-progress implementation of an unbounded boolean bitfield.
+Implements a set of boolean as a tightly-packed vector of bits.
-
-Based upon a `Vec<u8>`
-
-Documentation TBC...

beacon_chain/utils/boolean-bitfield/src/lib.rs

@@ -1,161 +1,124 @@
-/*
+extern crate bit_vec;
- * Implemenation of a bitfield as a vec. Only
- * supports bytes (Vec<u8>) as the underlying
- * storage.
- *
- * A future implementation should be more efficient,
- * this is just to get the job done for now.
- */
 extern crate ssz;
 
-use std::cmp::max;
+use bit_vec::BitVec;
 
-#[derive(Eq, Clone, Default, Debug)]
+use std::default;
-pub struct BooleanBitfield {
+
-    len: usize,
+/// A BooleanBitfield represents a set of booleans compactly stored as a vector of bits.
-    vec: Vec<u8>,
+/// The BooleanBitfield is given a fixed size during construction. Reads outside of the current size return an out-of-bounds error. Writes outside of the current size expand the size of the set.
+#[derive(Debug, Clone, PartialEq)]
+pub struct BooleanBitfield(BitVec);
+
+/// Error represents some reason a request against a bitfield was not satisfied
+#[derive(Debug, PartialEq)]
+pub enum Error {
+    /// OutOfBounds refers to indexing into a bitfield where no bits exist; returns the illegal index and the current size of the bitfield, respectively
+    OutOfBounds(usize, usize),
 }
 
 impl BooleanBitfield {
-    /// Create a new bitfield with a length of zero.
+    /// Create a new bitfield.
     pub fn new() -> Self {
+        Default::default()
+    }
+
+    /// Create a new bitfield with the given length `initial_len` and all values set to `bit`.
+    pub fn from_elem(inital_len: usize, bit: bool) -> Self {
         Self {
-            len: 0,
+            0: BitVec::from_elem(inital_len, bit),
-            vec: vec![0],
         }
     }
 
-    /// Create a new bitfield of a certain capacity
+    /// Create a new bitfield using the supplied `bytes` as input
-    pub fn with_capacity(capacity: usize) -> Self {
+    pub fn from_bytes(bytes: &[u8]) -> Self {
-        let mut vec = Vec::with_capacity(capacity / 8 + 1);
+        Self {
-        vec.push(0);
+            0: BitVec::from_bytes(bytes),
-        Self { len: 0, vec }
+        }
     }
 
     /// Read the value of a bit.
     ///
-    /// Will return `true` if the bit has been set to `true`
+    /// If the index is in bounds, then result is Ok(value) where value is `true` if the bit is 1 and `false` if the bit is 0.
-    /// without then being set to `False`.
+    /// If the index is out of bounds, we return an error to that extent.
-    pub fn get_bit(&self, i: usize) -> bool {
+    pub fn get(&self, i: usize) -> Result<bool, Error> {
-        let bit = |i: usize| i % 8;
+        match self.0.get(i) {
-        let byte = |i: usize| i / 8;
+            Some(value) => Ok(value),
-
+            None => Err(Error::OutOfBounds(i, self.0.len())),
-        if byte(i) >= self.vec.len() {
-            false
-        } else {
-            self.vec[byte(i)] & (1 << (bit(i) as u8)) != 0
         }
     }
 
     /// Set the value of a bit.
     ///
-    /// If this bit is larger than the length of the underlying byte
+    /// If the index is out of bounds, we expand the size of the underlying set to include the new index.
-    /// array it will be extended.
+    /// Returns the previous value if there was one.
-    pub fn set_bit(&mut self, i: usize, to: bool) {
+    pub fn set(&mut self, i: usize, value: bool) -> Option<bool> {
-        let bit = |i: usize| i % 8;
+        let previous = match self.get(i) {
-        let byte = |i: usize| i / 8;
+            Ok(previous) => Some(previous),
-
+            Err(Error::OutOfBounds(_, len)) => {
-        self.len = max(self.len, i + 1);
+                let new_len = i - len + 1;
-
+                self.0.grow(new_len, false);
-        if byte(i) >= self.vec.len() {
+                None
-            self.vec.resize(byte(i) + 1, 0);
-        }
-        if to {
-            self.vec[byte(i)] = self.vec[byte(i)] | (1 << (bit(i) as u8))
-        } else {
-            self.vec[byte(i)] = self.vec[byte(i)] & !(1 << (bit(i) as u8))
             }
+        };
+        self.0.set(i, value);
+        previous
     }
 
-    /// Return the "length" of this bitfield. Length is defined as
+    /// Returns the index of the highest set bit. Some(n) if some bit is set, None otherwise.
-    /// the highest bit that has been set.
+    pub fn highest_set_bit(&self) -> Option<usize> {
-    ///
+        self.0.iter().rposition(|bit| bit)
-    /// Note: this is distinct from the length of the underlying
+    }
-    /// vector.
+
+    /// Returns the number of bits in this bitfield.
     pub fn len(&self) -> usize {
-        self.len
+        self.0.len()
     }
 
-    /// True if no bits have ever been set. A bit that is set and then
+    /// Returns the number of bytes required to represent this bitfield.
-    /// unset will still count to the length of the bitfield.
-    ///
-    /// Note: this is distinct from the length of the underlying
-    /// vector.
-    pub fn is_empty(&self) -> bool {
-        self.len == 0
-    }
-
-    /// The number of bytes required to represent the bitfield.
     pub fn num_bytes(&self) -> usize {
-        self.vec.len()
+        self.to_bytes().len()
     }
 
-    /// Iterate through the underlying vector and count the number of
+    /// Returns the number of `1` bits in the bitfield
-    /// true bits.
+    pub fn num_set_bits(&self) -> usize {
-    pub fn num_true_bits(&self) -> u64 {
+        self.0.iter().filter(|&bit| bit).count()
-        let mut count: u64 = 0;
-        for byte in &self.vec {
-            for bit in 0..8 {
-                if byte & (1 << (bit as u8)) != 0 {
-                    count += 1;
-                }
-            }
-        }
-        count
     }
 
-    /// Iterate through the underlying vector and find the highest
+    /// Returns a vector of bytes representing the bitfield
-    /// set bit. Useful for instantiating a new instance from
+    /// Note that this returns the bit layout of the underlying implementation in the `bit-vec` crate.
-    /// some set of bytes.
+    pub fn to_bytes(&self) -> Vec<u8> {
-    pub fn compute_length(bytes: &[u8]) -> usize {
+        self.0.to_bytes()
-        for byte in (0..bytes.len()).rev() {
-            for bit in (0..8).rev() {
-                if bytes[byte] & (1 << (bit as u8)) != 0 {
-                    return (byte * 8) + bit + 1;
-                }
-            }
-        }
-        0
-    }
-
-    /// Get the byte at a position, assuming big-endian encoding.
-    pub fn get_byte(&self, n: usize) -> Option<&u8> {
-        self.vec.get(n)
-    }
-
-    /// Clone and return the underlying byte array (`Vec<u8>`).
-    pub fn to_vec(&self) -> Vec<u8> {
-        self.vec.clone()
-    }
-
-    /// Clone and return the underlying byte array (`Vec<u8>`) in big-endinan format.
-    pub fn to_be_vec(&self) -> Vec<u8> {
-        let mut o = self.vec.clone();
-        o.reverse();
-        o
     }
 }
 
-impl<'a> From<&'a [u8]> for BooleanBitfield {
+impl default::Default for BooleanBitfield {
-    fn from(input: &[u8]) -> Self {
+    /// default provides the "empty" bitfield
-        let mut vec = input.to_vec();
+    /// Note: the empty bitfield is set to the `0` byte.
-        vec.reverse();
+    fn default() -> Self {
-        BooleanBitfield {
+        Self::from_elem(8, false)
-            vec,
-            len: BooleanBitfield::compute_length(input),
-        }
     }
 }
 
-impl PartialEq for BooleanBitfield {
+// borrowed from bit_vec crate
-    fn eq(&self, other: &BooleanBitfield) -> bool {
+fn reverse_bits(byte: u8) -> u8 {
-        (self.vec == other.vec) & (self.len == other.len)
+    let mut result = 0;
+    for i in 0..8 {
+        result = result | ((byte >> i) & 1) << (7 - i);
     }
+    result
 }
 
 impl ssz::Encodable for BooleanBitfield {
+    // ssz_append encodes Self according to the `ssz` spec.
+    // Note that we have to flip the endianness of the encoding with `reverse_bits` to account for an implementation detail of `bit-vec` crate.
     fn ssz_append(&self, s: &mut ssz::SszStream) {
-        s.append_vec(&self.to_vec());
+        let bytes: Vec<u8> = self
+            .to_bytes()
+            .iter()
+            .map(|&byte| reverse_bits(byte))
+            .collect();
+        s.append_vec(&bytes);
     }
 }
 
@@ -165,12 +128,24 @@ impl ssz::Decodable for BooleanBitfield {
         if (ssz::LENGTH_BYTES + len) > bytes.len() {
             return Err(ssz::DecodeError::TooShort);
         }
+
         if len == 0 {
             Ok((BooleanBitfield::new(), index + ssz::LENGTH_BYTES))
         } else {
-            let b = BooleanBitfield::from(&bytes[(index + 4)..(index + len + 4)]);
+            let bytes = &bytes[(index + 4)..(index + len + 4)];
+
+            let mut field = BooleanBitfield::from_elem(0, false);
+            for (byte_index, byte) in bytes.iter().enumerate() {
+                for i in 0..8 {
+                    let bit = byte & (1 << i);
+                    if bit != 0 {
+                        field.set(8 * byte_index + i, true);
+                    }
+                }
+            }
+
             let index = index + ssz::LENGTH_BYTES + len;
-            Ok((b, index))
+            Ok((field, index))
         }
     }
 }
@@ -178,149 +153,192 @@ impl ssz::Decodable for BooleanBitfield {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use ssz::Decodable;
+    use ssz::SszStream;
 
     #[test]
-    fn test_new_from_slice() {
+    fn test_new_bitfield() {
-        let s = [0];
+        let mut field = BooleanBitfield::new();
-        let b = BooleanBitfield::from(&s[..]);
+        let original_len = field.len();
-        assert_eq!(b.len, 0);
 
-        let s = [255];
+        for i in 0..100 {
-        let b = BooleanBitfield::from(&s[..]);
+            if i < original_len {
-        assert_eq!(b.len, 8);
+                assert!(!field.get(i).unwrap());
-
+            } else {
-        let s = [0, 1];
+                assert!(field.get(i).is_err());
-        let b = BooleanBitfield::from(&s[..]);
-        assert_eq!(b.len, 9);
-
-        let s = [31];
-        let b = BooleanBitfield::from(&s[..]);
-        assert_eq!(b.len, 5);
             }
-
+            let previous = field.set(i, true);
-    #[test]
+            if i < original_len {
-    fn test_ssz_encoding() {
+                assert!(!previous.unwrap());
-        let mut b = BooleanBitfield::new();
+            } else {
-        b.set_bit(8, true);
+                assert!(previous.is_none());
-
-        let mut stream = ssz::SszStream::new();
-        stream.append(&b);
-
-        assert_eq!(stream.drain(), vec![0, 0, 0, 2, 0, 1]);
             }
-
-    /*
-    #[test]
-    fn test_ssz_decoding() {
-        /*
-         * Correct input
-         */
-        let input = vec![0, 0, 0, 2, 0, 1];
-        let (b, i) = BooleanBitfield::ssz_decode(&input, 0).unwrap();
-        assert_eq!(i, 6);
-        assert_eq!(b.num_true_bits(), 1);
-        assert_eq!(b.get_bit(8), true);
-
-        /*
-         * Input too long
-         */
-        let mut input = vec![0, 0, 0, 2, 0, 1];
-        input.push(42);
-        let (b, i) = BooleanBitfield::ssz_decode(&input, 0).unwrap();
-        assert_eq!(i, 6);
-        assert_eq!(b.num_true_bits(), 1);
-        assert_eq!(b.get_bit(8), true);
-
-        /*
-         * Input too short
-         */
-        let input = vec![0, 0, 0, 2, 1];
-        let res = BooleanBitfield::ssz_decode(&input, 0);
-        assert_eq!(res, Err(ssz::DecodeError::TooShort));
-    }
-    */
-
-    #[test]
-    fn test_new_bitfield_len() {
-        let b = BooleanBitfield::new();
-        assert_eq!(b.len(), 0);
-        assert_eq!(b.to_be_vec(), vec![0]);
-
-        let b = BooleanBitfield::with_capacity(100);
-        assert_eq!(b.len(), 0);
-        assert_eq!(b.to_be_vec(), vec![0]);
-    }
-
-    #[test]
-    fn test_bitfield_set() {
-        let mut b = BooleanBitfield::new();
-        b.set_bit(0, false);
-        assert_eq!(b.to_be_vec(), [0]);
-
-        b = BooleanBitfield::new();
-        b.set_bit(7, true);
-        assert_eq!(b.to_be_vec(), [128]);
-        b.set_bit(7, false);
-        assert_eq!(b.to_be_vec(), [0]);
-        assert_eq!(b.len(), 8);
-
-        b = BooleanBitfield::new();
-        b.set_bit(7, true);
-        b.set_bit(0, true);
-        assert_eq!(b.to_be_vec(), [129]);
-        b.set_bit(7, false);
-        assert_eq!(b.to_be_vec(), [1]);
-        assert_eq!(b.len(), 8);
-
-        b = BooleanBitfield::new();
-        b.set_bit(8, true);
-        assert_eq!(b.to_be_vec(), [1, 0]);
-        assert_eq!(b.len(), 9);
-        b.set_bit(8, false);
-        assert_eq!(b.to_be_vec(), [0, 0]);
-        assert_eq!(b.len(), 9);
-
-        b = BooleanBitfield::new();
-        b.set_bit(15, true);
-        assert_eq!(b.to_be_vec(), [128, 0]);
-        b.set_bit(15, false);
-        assert_eq!(b.to_be_vec(), [0, 0]);
-        assert_eq!(b.len(), 16);
-
-        b = BooleanBitfield::new();
-        b.set_bit(8, true);
-        b.set_bit(15, true);
-        assert_eq!(b.to_be_vec(), [129, 0]);
-        b.set_bit(15, false);
-        assert_eq!(b.to_be_vec(), [1, 0]);
-        assert_eq!(b.len(), 16);
-    }
-
-    #[test]
-    fn test_bitfield_get() {
-        let test_nums = vec![0, 8, 15, 42, 1337];
-        for i in test_nums {
-            let mut b = BooleanBitfield::new();
-            assert_eq!(b.get_bit(i), false);
-            b.set_bit(i, true);
-            assert_eq!(b.get_bit(i), true);
-            b.set_bit(i, true);
         }
     }
 
     #[test]
-    fn test_bitfield_num_true_bits() {
+    fn test_empty_bitfield() {
-        let mut b = BooleanBitfield::new();
+        let mut field = BooleanBitfield::from_elem(0, false);
-        assert_eq!(b.num_true_bits(), 0);
+        let original_len = field.len();
-        b.set_bit(15, true);
+
-        assert_eq!(b.num_true_bits(), 1);
+        assert_eq!(original_len, 0);
-        b.set_bit(15, false);
+
-        assert_eq!(b.num_true_bits(), 0);
+        for i in 0..100 {
-        b.set_bit(0, true);
+            if i < original_len {
-        b.set_bit(7, true);
+                assert!(!field.get(i).unwrap());
-        b.set_bit(8, true);
+            } else {
-        b.set_bit(1337, true);
+                assert!(field.get(i).is_err());
-        assert_eq!(b.num_true_bits(), 4);
+            }
+            let previous = field.set(i, true);
+            if i < original_len {
+                assert!(!previous.unwrap());
+            } else {
+                assert!(previous.is_none());
+            }
+        }
+
+        assert_eq!(field.len(), 100);
+        assert_eq!(field.num_set_bits(), 100);
+    }
+
+    const INPUT: &[u8] = &[0b0000_0010, 0b0000_0010];
+
+    #[test]
+    fn test_get_from_bitfield() {
+        let field = BooleanBitfield::from_bytes(INPUT);
+        let unset = field.get(0).unwrap();
+        assert!(!unset);
+        let set = field.get(6).unwrap();
+        assert!(set);
+        let set = field.get(14).unwrap();
+        assert!(set);
+    }
+
+    #[test]
+    fn test_set_for_bitfield() {
+        let mut field = BooleanBitfield::from_bytes(INPUT);
+        let previous = field.set(10, true).unwrap();
+        assert!(!previous);
+        let previous = field.get(10).unwrap();
+        assert!(previous);
+        let previous = field.set(6, false).unwrap();
+        assert!(previous);
+        let previous = field.get(6).unwrap();
+        assert!(!previous);
+    }
+
+    #[test]
+    fn test_highest_set_bit() {
+        let field = BooleanBitfield::from_bytes(INPUT);
+        assert_eq!(field.highest_set_bit().unwrap(), 14);
+
+        let field = BooleanBitfield::from_bytes(&[0b0000_0011]);
+        assert_eq!(field.highest_set_bit().unwrap(), 7);
+
+        let field = BooleanBitfield::new();
+        assert_eq!(field.highest_set_bit(), None);
+    }
+
+    #[test]
+    fn test_len() {
+        let field = BooleanBitfield::from_bytes(INPUT);
+        assert_eq!(field.len(), 16);
+
+        let field = BooleanBitfield::new();
+        assert_eq!(field.len(), 8);
+    }
+
+    #[test]
+    fn test_num_set_bits() {
+        let field = BooleanBitfield::from_bytes(INPUT);
+        assert_eq!(field.num_set_bits(), 2);
+
+        let field = BooleanBitfield::new();
+        assert_eq!(field.num_set_bits(), 0);
+    }
+
+    #[test]
+    fn test_to_bytes() {
+        let field = BooleanBitfield::from_bytes(INPUT);
+        assert_eq!(field.to_bytes(), INPUT);
+
+        let field = BooleanBitfield::new();
+        assert_eq!(field.to_bytes(), vec![0]);
+    }
+
+    #[test]
+    fn test_out_of_bounds() {
+        let mut field = BooleanBitfield::from_bytes(INPUT);
+
+        let out_of_bounds_index = field.len();
+        assert!(field.set(out_of_bounds_index, true).is_none());
+        assert!(field.len() == out_of_bounds_index + 1);
+        assert!(field.get(out_of_bounds_index).unwrap());
+
+        for i in 0..100 {
+            if i <= out_of_bounds_index {
+                assert!(field.set(i, true).is_some());
+            } else {
+                assert!(field.set(i, true).is_none());
+            }
+        }
+    }
+
+    #[test]
+    fn test_grows_with_false() {
+        let input_all_set: &[u8] = &[0b1111_1111, 0b1111_1111];
+        let mut field = BooleanBitfield::from_bytes(input_all_set);
+
+        // Define `a` and `b`, where both are out of bounds and `b` is greater than `a`.
+        let a = field.len();
+        let b = a + 1;
+
+        // Ensure `a` is out-of-bounds for test integrity.
+        assert!(field.get(a).is_err());
+
+        // Set `b` to `true`. Also, for test integrity, ensure it was previously out-of-bounds.
+        assert!(field.set(b, true).is_none());
+
+        // Ensure that `a` wasn't also set to `true` during the grow.
+        assert_eq!(field.get(a), Ok(false));
+        assert_eq!(field.get(b), Ok(true));
+    }
+
+    #[test]
+    fn test_num_bytes() {
+        let field = BooleanBitfield::from_bytes(INPUT);
+        assert_eq!(field.num_bytes(), 2);
+
+        let field = BooleanBitfield::from_elem(2, true);
+        assert_eq!(field.num_bytes(), 1);
+
+        let field = BooleanBitfield::from_elem(13, true);
+        assert_eq!(field.num_bytes(), 2);
+    }
+
+    #[test]
+    fn test_ssz_encode() {
+        let field = BooleanBitfield::from_elem(5, true);
+
+        let mut stream = SszStream::new();
+        stream.append(&field);
+        assert_eq!(stream.drain(), vec![0, 0, 0, 1, 31]);
+
+        let field = BooleanBitfield::from_elem(18, true);
+        let mut stream = SszStream::new();
+        stream.append(&field);
+        assert_eq!(stream.drain(), vec![0, 0, 0, 3, 255, 255, 3]);
+    }
+
+    #[test]
+    fn test_ssz_decode() {
+        let encoded = vec![0, 0, 0, 1, 31];
+        let (field, _): (BooleanBitfield, usize) = ssz::decode_ssz(&encoded, 0).unwrap();
+        let expected = BooleanBitfield::from_elem(5, true);
+        assert_eq!(field, expected);
+
+        let encoded = vec![0, 0, 0, 3, 255, 255, 3];
+        let (field, _): (BooleanBitfield, usize) = ssz::decode_ssz(&encoded, 0).unwrap();
+        let expected = BooleanBitfield::from_elem(18, true);
+        assert_eq!(field, expected);
     }
 }

beacon_chain/utils/ssz_helpers/src/attestation_ssz_splitter.rs

@@ -62,7 +62,7 @@ mod tests {
             shard_id: 9,
             oblique_parent_hashes: vec![Hash256::from(&vec![14; 32][..])],
             shard_block_hash: Hash256::from(&vec![15; 32][..]),
-            attester_bitfield: Bitfield::from(&vec![17; 42][..]),
+            attester_bitfield: Bitfield::from_bytes(&vec![17; 42][..]),
             justified_slot: 19,
             justified_block_hash: Hash256::from(&vec![15; 32][..]),
             aggregate_sig: AggregateSignature::new(),
@@ -72,7 +72,7 @@ mod tests {
             shard_id: 7,
             oblique_parent_hashes: vec![Hash256::from(&vec![15; 32][..])],
             shard_block_hash: Hash256::from(&vec![14; 32][..]),
-            attester_bitfield: Bitfield::from(&vec![19; 42][..]),
+            attester_bitfield: Bitfield::from_bytes(&vec![19; 42][..]),
             justified_slot: 15,
             justified_block_hash: Hash256::from(&vec![17; 32][..]),
             aggregate_sig: AggregateSignature::new(),

beacon_chain/validation/src/attestation_validation.rs

@@ -32,6 +32,7 @@ pub enum AttestationValidationError {
     NonZeroTrailingBits,
     BadAggregateSignature,
     DBError(String),
+    OutOfBoundsBitfieldIndex,
 }
 
 /// The context against which some attestation should be validated.
@@ -242,6 +243,9 @@ impl From<SignatureVerificationError> for AttestationValidationError {
                 AttestationValidationError::NoPublicKeyForValidator
             }
             SignatureVerificationError::DBError(s) => AttestationValidationError::DBError(s),
+            SignatureVerificationError::OutOfBoundsBitfieldIndex => {
+                AttestationValidationError::OutOfBoundsBitfieldIndex
+            }
         }
     }
 }

beacon_chain/validation/src/signature_verification.rs

@@ -1,7 +1,7 @@
 use super::bls::{AggregatePublicKey, AggregateSignature};
 use super::db::stores::{ValidatorStore, ValidatorStoreError};
 use super::db::ClientDB;
-use super::types::Bitfield;
+use super::types::{Bitfield, BitfieldError};
 use std::collections::HashSet;
 
 #[derive(Debug, PartialEq)]
@@ -10,6 +10,13 @@ pub enum SignatureVerificationError {
     PublicKeyCorrupt,
     NoPublicKeyForValidator,
     DBError(String),
+    OutOfBoundsBitfieldIndex,
+}
+
+impl From<BitfieldError> for SignatureVerificationError {
+    fn from(_error: BitfieldError) -> Self {
+        SignatureVerificationError::OutOfBoundsBitfieldIndex
+    }
 }
 
 /// Verify an aggregate signature across the supplied message.
@@ -33,7 +40,7 @@ where
     let mut agg_pub_key = AggregatePublicKey::new();
 
     for i in 0..attestation_indices.len() {
-        let voted = bitfield.get_bit(i);
+        let voted = bitfield.get(i)?;
         if voted {
             /*
              * De-reference the attestation index into a canonical ValidatorRecord index.
@@ -121,9 +128,9 @@ mod tests {
         all_keypairs.append(&mut non_signing_keypairs.clone());
 
         let attestation_indices: Vec<usize> = (0..all_keypairs.len()).collect();
-        let mut bitfield = Bitfield::new();
+        let mut bitfield = Bitfield::from_elem(all_keypairs.len(), false);
         for i in 0..signing_keypairs.len() {
-            bitfield.set_bit(i, true);
+            bitfield.set(i, true).unwrap();
         }
 
         let db = Arc::new(MemoryDB::open());
@@ -159,7 +166,7 @@ mod tests {
          * Add another validator to the bitfield, run validation will all other
          * parameters the same and assert that it fails.
          */
-        bitfield.set_bit(signing_keypairs.len() + 1, true);
+        bitfield.set(signing_keypairs.len() + 1, true).unwrap();
         let voters = verify_aggregate_signature_for_indices(
             &message,
             &agg_sig,

beacon_chain/validation/tests/attestation_validation/helpers.rs

@@ -63,7 +63,7 @@ pub fn generate_attestation(
     signing_keys: &[Option<SecretKey>],
     block_store: &BeaconBlockStore<MemoryDB>,
 ) -> AttestationRecord {
-    let mut attester_bitfield = Bitfield::new();
+    let mut attester_bitfield = Bitfield::from_elem(signing_keys.len(), false);
     let mut aggregate_sig = AggregateSignature::new();
 
     let parent_hashes_slice = {
@@ -95,7 +95,7 @@ pub fn generate_attestation(
          * and sign the aggregate sig.
          */
         if let Some(sk) = secret_key {
-            attester_bitfield.set_bit(i, true);
+            attester_bitfield.set(i, true).unwrap();
             let sig = Signature::new(&attestation_message, sk);
             aggregate_sig.add(&sig);
         }

beacon_chain/validation/tests/attestation_validation/tests.rs

@@ -129,16 +129,12 @@ fn test_attestation_validation_invalid_bad_bitfield_length() {
     /*
      * Extend the bitfield by one byte
      *
-     * This is a little hacky and makes assumptions about the internals
+     * We take advantage of the fact that setting a bit outside the current bounds will grow the bitvector.
-     * of the bitfield.
      */
     let one_byte_higher = rig.attester_count + 8;
     rig.attestation
         .attester_bitfield
-        .set_bit(one_byte_higher, true);
+        .set(one_byte_higher, false);
-    rig.attestation
-        .attester_bitfield
-        .set_bit(one_byte_higher, false);
 
     let result = rig.context.validate_attestation(&rig.attestation);
     assert_eq!(result, Err(AttestationValidationError::BadBitfieldLength));
@@ -149,9 +145,7 @@ fn test_attestation_validation_invalid_invalid_bitfield_end_bit() {
     let mut rig = generic_rig();
 
     let one_bit_high = rig.attester_count + 1;
-    rig.attestation
+    rig.attestation.attester_bitfield.set(one_bit_high, true);
-        .attester_bitfield
-        .set_bit(one_bit_high, true);
 
     let result = rig.context.validate_attestation(&rig.attestation);
     assert_eq!(
@@ -178,9 +172,7 @@ fn test_attestation_validation_invalid_invalid_bitfield_end_bit_with_irreguar_bi
         one_bit_high % 8 != 0,
         "the test is ineffective in this case."
     );
-    rig.attestation
+    rig.attestation.attester_bitfield.set(one_bit_high, true);
-        .attester_bitfield
-        .set_bit(one_bit_high, true);
 
     let result = rig.context.validate_attestation(&rig.attestation);
     assert_eq!(