From a9decd3aede58716ea55ea3816aa17427e10e052 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Wed, 24 Oct 2018 12:21:51 +0200 Subject: [PATCH 01/11] Simplifies the boolean-bitfield implementation to use `bit-vec` crate --- beacon_chain/types/src/attestation_record.rs | 2 +- .../utils/boolean-bitfield/Cargo.toml | 4 + beacon_chain/utils/boolean-bitfield/README.md | 6 +- .../utils/boolean-bitfield/src/lib.rs | 384 ++++++------------ 4 files changed, 119 insertions(+), 277 deletions(-) diff --git a/beacon_chain/types/src/attestation_record.rs b/beacon_chain/types/src/attestation_record.rs index 15b1a2e71..569cfa2dc 100644 --- a/beacon_chain/types/src/attestation_record.rs +++ b/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_vec(&self.attester_bitfield.to_bytes()); s.append(&self.justified_slot); s.append(&self.justified_block_hash); s.append_vec(&self.aggregate_sig.as_bytes()); diff --git a/beacon_chain/utils/boolean-bitfield/Cargo.toml b/beacon_chain/utils/boolean-bitfield/Cargo.toml index feacb844a..b93e88f23 100644 --- a/beacon_chain/utils/boolean-bitfield/Cargo.toml +++ b/beacon_chain/utils/boolean-bitfield/Cargo.toml @@ -5,3 +5,7 @@ authors = ["Paul Hauner "] [dependencies] ssz = { path = "../ssz" } +bit-vec = "0.5.0" + +[dev-dependencies] +rand = "0.5.5" \ No newline at end of file diff --git a/beacon_chain/utils/boolean-bitfield/README.md b/beacon_chain/utils/boolean-bitfield/README.md index 749ccdb51..adf83f6f8 100644 --- a/beacon_chain/utils/boolean-bitfield/README.md +++ b/beacon_chain/utils/boolean-bitfield/README.md @@ -1,7 +1,3 @@ # Boolean Bitfield -A work-in-progress implementation of an unbounded boolean bitfield. - -Based upon a `Vec` - -Documentation TBC... +Implements a set of boolean as a tightly-packed vector of bits. diff --git a/beacon_chain/utils/boolean-bitfield/src/lib.rs b/beacon_chain/utils/boolean-bitfield/src/lib.rs index db3d4f99f..4d562d95b 100644 --- a/beacon_chain/utils/boolean-bitfield/src/lib.rs +++ b/beacon_chain/utils/boolean-bitfield/src/lib.rs @@ -1,161 +1,74 @@ -/* - * Implemenation of a bitfield as a vec. Only - * supports bytes (Vec) as the underlying - * storage. - * - * A future implementation should be more efficient, - * this is just to get the job done for now. - */ +extern crate bit_vec; extern crate ssz; -use std::cmp::max; +#[cfg(test)] +extern crate rand; -#[derive(Eq, Clone, Default, Debug)] -pub struct BooleanBitfield { - len: usize, - vec: Vec, +use bit_vec::BitVec; + +/// A BooleanBitfield represents a set of booleans compactly stored as a vector of bits. +#[derive(Debug, Clone, PartialEq)] +pub struct BooleanBitfield(BitVec); + +/// Error represents some reason a request against a bitfield was not satisfied +#[derive(Debug)] +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. pub fn new() -> Self { - Self { - len: 0, - vec: vec![0], - } + Self { 0: BitVec::new() } } - /// Create a new bitfield of a certain capacity - pub fn with_capacity(capacity: usize) -> Self { - let mut vec = Vec::with_capacity(capacity / 8 + 1); - vec.push(0); - Self { len: 0, vec } + /// Create a new bitfield using the supplied `bytes` as input + pub fn from_bytes(bytes: &[u8]) -> Self { + Self { + 0: BitVec::from_bytes(bytes), + } } /// Read the value of a bit. /// - /// Will return `true` if the bit has been set to `true` - /// without then being set to `False`. - pub fn get_bit(&self, i: usize) -> bool { - let bit = |i: usize| i % 8; - let byte = |i: usize| i / 8; - - if byte(i) >= self.vec.len() { - false - } else { - self.vec[byte(i)] & (1 << (bit(i) as u8)) != 0 + /// 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. + /// If the index is out of bounds, we return an error to that extent. + pub fn get(&self, i: usize) -> Result { + match self.0.get(i) { + Some(value) => Ok(value), + None => Err(Error::OutOfBounds(i, self.0.len())), } } /// Set the value of a bit. /// - /// If this bit is larger than the length of the underlying byte - /// array it will be extended. - pub fn set_bit(&mut self, i: usize, to: bool) { - let bit = |i: usize| i % 8; - let byte = |i: usize| i / 8; - - self.len = max(self.len, i + 1); - - if byte(i) >= self.vec.len() { - 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)) - } + /// Returns the previous value if successful. + /// If the index is out of bounds, we return an error to that extent. + pub fn set(&mut self, i: usize, value: bool) -> Result { + let previous = self.get(i)?; + self.0.set(i, value); + Ok(previous) } - /// Return the "length" of this bitfield. Length is defined as - /// the highest bit that has been set. - /// - /// Note: this is distinct from the length of the underlying - /// vector. + /// Returns the index of the highest set bit. Some(n) if some bit is set, None otherwise. + pub fn highest_set_bit(&self) -> Option { + self.0.iter().rposition(|bit| bit) + } + + /// 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 - /// 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 + /// Returns the number of `1` bits in the bitfield + pub fn num_set_bits(&self) -> usize { + self.0.iter().filter(|&bit| bit).count() } - /// The number of bytes required to represent the bitfield. - pub fn num_bytes(&self) -> usize { - self.vec.len() - } - - /// Iterate through the underlying vector and count the number of - /// true bits. - pub fn num_true_bits(&self) -> u64 { - 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 - /// set bit. Useful for instantiating a new instance from - /// some set of bytes. - pub fn compute_length(bytes: &[u8]) -> usize { - 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`). - pub fn to_vec(&self) -> Vec { - self.vec.clone() - } - - /// Clone and return the underlying byte array (`Vec`) in big-endinan format. - pub fn to_be_vec(&self) -> Vec { - let mut o = self.vec.clone(); - o.reverse(); - o - } -} - -impl<'a> From<&'a [u8]> for BooleanBitfield { - fn from(input: &[u8]) -> Self { - let mut vec = input.to_vec(); - vec.reverse(); - BooleanBitfield { - vec, - len: BooleanBitfield::compute_length(input), - } - } -} - -impl PartialEq for BooleanBitfield { - fn eq(&self, other: &BooleanBitfield) -> bool { - (self.vec == other.vec) & (self.len == other.len) - } -} - -impl ssz::Encodable for BooleanBitfield { - fn ssz_append(&self, s: &mut ssz::SszStream) { - s.append_vec(&self.to_vec()); + /// Returns a vector of bytes representing the bitfield + pub fn to_bytes(&self) -> Vec { + self.0.to_bytes() } } @@ -165,12 +78,13 @@ 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 field = BooleanBitfield::from_bytes(&bytes[(index + 4)..(index + len + 4)]); let index = index + ssz::LENGTH_BYTES + len; - Ok((b, index)) + Ok((field, index)) } } } @@ -178,149 +92,77 @@ impl ssz::Decodable for BooleanBitfield { #[cfg(test)] mod tests { use super::*; - use ssz::Decodable; #[test] - fn test_new_from_slice() { - let s = [0]; - let b = BooleanBitfield::from(&s[..]); - assert_eq!(b.len, 0); + fn test_empty_bitfield() { + let mut field = BooleanBitfield::new(); - let s = [255]; - let b = BooleanBitfield::from(&s[..]); - assert_eq!(b.len, 8); - - let s = [0, 1]; - let b = BooleanBitfield::from(&s[..]); - assert_eq!(b.len, 9); - - let s = [31]; - let b = BooleanBitfield::from(&s[..]); - assert_eq!(b.len, 5); - } - - #[test] - fn test_ssz_encoding() { - let mut b = BooleanBitfield::new(); - b.set_bit(8, true); - - 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); + for _ in 0..100 { + let index: usize = rand::random(); + assert!(field.get(index).is_err()); + assert!(field.set(index, rand::random()).is_err()) } } + const INPUT: &[u8] = &[0b0000_0010, 0b0000_0010]; + #[test] - fn test_bitfield_num_true_bits() { - let mut b = BooleanBitfield::new(); - assert_eq!(b.num_true_bits(), 0); - b.set_bit(15, true); - assert_eq!(b.num_true_bits(), 1); - b.set_bit(15, false); - assert_eq!(b.num_true_bits(), 0); - b.set_bit(0, true); - b.set_bit(7, true); - b.set_bit(8, true); - b.set_bit(1337, true); - assert_eq!(b.num_true_bits(), 4); + 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::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(), 0); + } + + #[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![]); } } From 37b05e1a5bdf02d21a32e809d8248f5df51f33f7 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Wed, 7 Nov 2018 14:32:33 -0800 Subject: [PATCH 02/11] get tests passing (except one) --- beacon_chain/types/src/lib.rs | 9 +++++-- .../utils/boolean-bitfield/src/lib.rs | 16 ++++++------ .../validation/src/attestation_validation.rs | 7 +++-- .../validation/src/signature_verification.rs | 15 ++++++++--- .../tests/attestation_validation/helpers.rs | 2 +- .../tests/attestation_validation/tests.rs | 26 +++++-------------- 6 files changed, 36 insertions(+), 39 deletions(-) diff --git a/beacon_chain/types/src/lib.rs b/beacon_chain/types/src/lib.rs index e8e42e8e9..c531da299 100644 --- a/beacon_chain/types/src/lib.rs +++ b/beacon_chain/types/src/lib.rs @@ -15,7 +15,11 @@ pub mod validator_record; pub mod validator_registration; use self::boolean_bitfield::BooleanBitfield; -use self::ethereum_types::{H160, H256, U256}; +use self::ethereum_types::{ + H256, + H160, + U256 +}; use std::collections::HashMap; pub use active_state::ActiveState; @@ -32,7 +36,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>; diff --git a/beacon_chain/utils/boolean-bitfield/src/lib.rs b/beacon_chain/utils/boolean-bitfield/src/lib.rs index 4d562d95b..1f96a9afd 100644 --- a/beacon_chain/utils/boolean-bitfield/src/lib.rs +++ b/beacon_chain/utils/boolean-bitfield/src/lib.rs @@ -24,7 +24,7 @@ impl BooleanBitfield { } /// Create a new bitfield using the supplied `bytes` as input - pub fn from_bytes(bytes: &[u8]) -> Self { + pub fn from(bytes: &[u8]) -> Self { Self { 0: BitVec::from_bytes(bytes), } @@ -82,7 +82,7 @@ impl ssz::Decodable for BooleanBitfield { if len == 0 { Ok((BooleanBitfield::new(), index + ssz::LENGTH_BYTES)) } else { - let field = BooleanBitfield::from_bytes(&bytes[(index + 4)..(index + len + 4)]); + let field = BooleanBitfield::from(&bytes[(index + 4)..(index + len + 4)]); let index = index + ssz::LENGTH_BYTES + len; Ok((field, index)) } @@ -108,7 +108,7 @@ mod tests { #[test] fn test_get_from_bitfield() { - let field = BooleanBitfield::from_bytes(INPUT); + let field = BooleanBitfield::from(INPUT); let unset = field.get(0).unwrap(); assert!(!unset); let set = field.get(6).unwrap(); @@ -119,7 +119,7 @@ mod tests { #[test] fn test_set_for_bitfield() { - let mut field = BooleanBitfield::from_bytes(INPUT); + let mut field = BooleanBitfield::from(INPUT); let previous = field.set(10, true).unwrap(); assert!(!previous); let previous = field.get(10).unwrap(); @@ -132,7 +132,7 @@ mod tests { #[test] fn test_highest_set_bit() { - let field = BooleanBitfield::from_bytes(INPUT); + let field = BooleanBitfield::from(INPUT); assert_eq!(field.highest_set_bit().unwrap(), 14); let field = BooleanBitfield::new(); @@ -141,7 +141,7 @@ mod tests { #[test] fn test_len() { - let field = BooleanBitfield::from_bytes(INPUT); + let field = BooleanBitfield::from(INPUT); assert_eq!(field.len(), 16); let field = BooleanBitfield::new(); @@ -150,7 +150,7 @@ mod tests { #[test] fn test_num_set_bits() { - let field = BooleanBitfield::from_bytes(INPUT); + let field = BooleanBitfield::from(INPUT); assert_eq!(field.num_set_bits(), 2); let field = BooleanBitfield::new(); @@ -159,7 +159,7 @@ mod tests { #[test] fn test_to_bytes() { - let field = BooleanBitfield::from_bytes(INPUT); + let field = BooleanBitfield::from(INPUT); assert_eq!(field.to_bytes(), INPUT); let field = BooleanBitfield::new(); diff --git a/beacon_chain/validation/src/attestation_validation.rs b/beacon_chain/validation/src/attestation_validation.rs index d4e451d38..d63d74483 100644 --- a/beacon_chain/validation/src/attestation_validation.rs +++ b/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. @@ -198,10 +199,6 @@ where } } -fn bytes_for_bits(bits: usize) -> usize { - (bits.saturating_sub(1) / 8) + 1 -} - impl From for AttestationValidationError { fn from(e: ParentHashesError) -> Self { match e { @@ -242,6 +239,8 @@ impl From for AttestationValidationError { AttestationValidationError::NoPublicKeyForValidator } SignatureVerificationError::DBError(s) => AttestationValidationError::DBError(s), + SignatureVerificationError::OutOfBoundsBitfieldIndex + => AttestationValidationError::OutOfBoundsBitfieldIndex, } } } diff --git a/beacon_chain/validation/src/signature_verification.rs b/beacon_chain/validation/src/signature_verification.rs index fddaabb99..18a447267 100644 --- a/beacon_chain/validation/src/signature_verification.rs +++ b/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 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. @@ -123,7 +130,7 @@ mod tests { let attestation_indices: Vec = (0..all_keypairs.len()).collect(); let mut bitfield = Bitfield::new(); 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, diff --git a/beacon_chain/validation/tests/attestation_validation/helpers.rs b/beacon_chain/validation/tests/attestation_validation/helpers.rs index a148f9a69..a9c92ca5a 100644 --- a/beacon_chain/validation/tests/attestation_validation/helpers.rs +++ b/beacon_chain/validation/tests/attestation_validation/helpers.rs @@ -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); } diff --git a/beacon_chain/validation/tests/attestation_validation/tests.rs b/beacon_chain/validation/tests/attestation_validation/tests.rs index 7c9617070..4ea954c89 100644 --- a/beacon_chain/validation/tests/attestation_validation/tests.rs +++ b/beacon_chain/validation/tests/attestation_validation/tests.rs @@ -133,12 +133,8 @@ fn test_attestation_validation_invalid_bad_bitfield_length() { * of the bitfield. */ let one_byte_higher = rig.attester_count + 8; - rig.attestation - .attester_bitfield - .set_bit(one_byte_higher, true); - rig.attestation - .attester_bitfield - .set_bit(one_byte_higher, false); + rig.attestation.attester_bitfield.set(one_byte_higher, true).unwrap(); + rig.attestation.attester_bitfield.set(one_byte_higher, false).unwrap(); 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 - .attester_bitfield - .set_bit(one_bit_high, true); + rig.attestation.attester_bitfield.set(one_bit_high, true).unwrap(); let result = rig.context.validate_attestation(&rig.attestation); assert_eq!( @@ -174,19 +168,11 @@ fn test_attestation_validation_invalid_invalid_bitfield_end_bit_with_irreguar_bi * bit in a bitfield and the byte length of that bitfield */ let one_bit_high = rig.attester_count + 1; - assert!( - one_bit_high % 8 != 0, - "the test is ineffective in this case." - ); - rig.attestation - .attester_bitfield - .set_bit(one_bit_high, true); + assert!(one_bit_high % 8 != 0, "the test is ineffective in this case."); + rig.attestation.attester_bitfield.set(one_bit_high, true).unwrap(); let result = rig.context.validate_attestation(&rig.attestation); - assert_eq!( - result, - Err(AttestationValidationError::InvalidBitfieldEndBits) - ); + assert_eq!(result, Err(AttestationValidationError::InvalidBitfieldEndBits)); } #[test] From 832d1bd295cbded813af1ff5b3eaa2e7ec9b4b08 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 15 Nov 2018 09:19:59 -0800 Subject: [PATCH 03/11] Update bitfield to expand size when writing out-of-bounds --- .../utils/boolean-bitfield/Cargo.toml | 5 +- .../utils/boolean-bitfield/src/lib.rs | 96 ++++++++++++++----- 2 files changed, 73 insertions(+), 28 deletions(-) diff --git a/beacon_chain/utils/boolean-bitfield/Cargo.toml b/beacon_chain/utils/boolean-bitfield/Cargo.toml index b93e88f23..1633401e2 100644 --- a/beacon_chain/utils/boolean-bitfield/Cargo.toml +++ b/beacon_chain/utils/boolean-bitfield/Cargo.toml @@ -5,7 +5,4 @@ authors = ["Paul Hauner "] [dependencies] ssz = { path = "../ssz" } -bit-vec = "0.5.0" - -[dev-dependencies] -rand = "0.5.5" \ No newline at end of file +bit-vec = "0.5.0" \ No newline at end of file diff --git a/beacon_chain/utils/boolean-bitfield/src/lib.rs b/beacon_chain/utils/boolean-bitfield/src/lib.rs index 1f96a9afd..ceff3bbcf 100644 --- a/beacon_chain/utils/boolean-bitfield/src/lib.rs +++ b/beacon_chain/utils/boolean-bitfield/src/lib.rs @@ -1,12 +1,12 @@ extern crate bit_vec; extern crate ssz; -#[cfg(test)] -extern crate rand; - use bit_vec::BitVec; +use std::default; + /// A BooleanBitfield represents a set of booleans compactly stored as a vector of bits. +/// 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); @@ -18,13 +18,20 @@ pub enum Error { } impl BooleanBitfield { - /// Create a new bitfield with a length of zero. + /// Create a new bitfield. pub fn new() -> Self { - Self { 0: BitVec::new() } + 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 { + 0: BitVec::from_elem(inital_len, bit), + } } /// Create a new bitfield using the supplied `bytes` as input - pub fn from(bytes: &[u8]) -> Self { + pub fn from_bytes(bytes: &[u8]) -> Self { Self { 0: BitVec::from_bytes(bytes), } @@ -43,12 +50,19 @@ impl BooleanBitfield { /// Set the value of a bit. /// - /// Returns the previous value if successful. - /// If the index is out of bounds, we return an error to that extent. - pub fn set(&mut self, i: usize, value: bool) -> Result { - let previous = self.get(i)?; + /// If the index is out of bounds, we expand the size of the underlying set to include the new index. + /// Returns the previous value if there was one. + pub fn set(&mut self, i: usize, value: bool) -> Option { + let previous = match self.get(i) { + Ok(previous) => Some(previous), + Err(Error::OutOfBounds(_, len)) => { + let new_len = i - len + 1; + self.0.grow(new_len, false); + None + } + }; self.0.set(i, value); - Ok(previous) + previous } /// Returns the index of the highest set bit. Some(n) if some bit is set, None otherwise. @@ -72,6 +86,14 @@ impl BooleanBitfield { } } +impl default::Default for BooleanBitfield { + /// default provides the "empty" bitfield + /// Note: the empty bitfield is set to the `0` byte. + fn default() -> Self { + Self::from_elem(8, false) + } +} + impl ssz::Decodable for BooleanBitfield { fn ssz_decode(bytes: &[u8], index: usize) -> Result<(Self, usize), ssz::DecodeError> { let len = ssz::decode::decode_length(bytes, index, ssz::LENGTH_BYTES)?; @@ -82,7 +104,7 @@ impl ssz::Decodable for BooleanBitfield { if len == 0 { Ok((BooleanBitfield::new(), index + ssz::LENGTH_BYTES)) } else { - let field = BooleanBitfield::from(&bytes[(index + 4)..(index + len + 4)]); + let field = BooleanBitfield::from_bytes(&bytes[(index + 4)..(index + len + 4)]); let index = index + ssz::LENGTH_BYTES + len; Ok((field, index)) } @@ -96,11 +118,20 @@ mod tests { #[test] fn test_empty_bitfield() { let mut field = BooleanBitfield::new(); + let original_len = field.len(); - for _ in 0..100 { - let index: usize = rand::random(); - assert!(field.get(index).is_err()); - assert!(field.set(index, rand::random()).is_err()) + for i in 0..100 { + if i < original_len { + assert!(!field.get(i).unwrap()); + } else { + assert!(field.get(i).is_err()); + } + let previous = field.set(i, true); + if i < original_len { + assert!(!previous.unwrap()); + } else { + assert!(previous.is_none()); + } } } @@ -108,7 +139,7 @@ mod tests { #[test] fn test_get_from_bitfield() { - let field = BooleanBitfield::from(INPUT); + let field = BooleanBitfield::from_bytes(INPUT); let unset = field.get(0).unwrap(); assert!(!unset); let set = field.get(6).unwrap(); @@ -119,7 +150,7 @@ mod tests { #[test] fn test_set_for_bitfield() { - let mut field = BooleanBitfield::from(INPUT); + let mut field = BooleanBitfield::from_bytes(INPUT); let previous = field.set(10, true).unwrap(); assert!(!previous); let previous = field.get(10).unwrap(); @@ -132,7 +163,7 @@ mod tests { #[test] fn test_highest_set_bit() { - let field = BooleanBitfield::from(INPUT); + let field = BooleanBitfield::from_bytes(INPUT); assert_eq!(field.highest_set_bit().unwrap(), 14); let field = BooleanBitfield::new(); @@ -141,16 +172,16 @@ mod tests { #[test] fn test_len() { - let field = BooleanBitfield::from(INPUT); + let field = BooleanBitfield::from_bytes(INPUT); assert_eq!(field.len(), 16); let field = BooleanBitfield::new(); - assert_eq!(field.len(), 0); + assert_eq!(field.len(), 8); } #[test] fn test_num_set_bits() { - let field = BooleanBitfield::from(INPUT); + let field = BooleanBitfield::from_bytes(INPUT); assert_eq!(field.num_set_bits(), 2); let field = BooleanBitfield::new(); @@ -159,10 +190,27 @@ mod tests { #[test] fn test_to_bytes() { - let field = BooleanBitfield::from(INPUT); + let field = BooleanBitfield::from_bytes(INPUT); assert_eq!(field.to_bytes(), INPUT); let field = BooleanBitfield::new(); - assert_eq!(field.to_bytes(), vec![]); + 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.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()); + } + } } } From 72cf7ad1bda43b374c4e068926bdb13d158b0df9 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 15 Nov 2018 09:20:46 -0800 Subject: [PATCH 04/11] Begin updating tests to reflect changes to bitfield --- beacon_chain/types/src/attestation_record.rs | 2 +- .../utils/ssz_helpers/src/attestation_ssz_splitter.rs | 4 ++-- beacon_chain/validation/src/signature_verification.rs | 2 +- .../validation/tests/attestation_validation/helpers.rs | 2 +- .../validation/tests/attestation_validation/tests.rs | 10 ++++------ 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/beacon_chain/types/src/attestation_record.rs b/beacon_chain/types/src/attestation_record.rs index 569cfa2dc..ed38664a9 100644 --- a/beacon_chain/types/src/attestation_record.rs +++ b/beacon_chain/types/src/attestation_record.rs @@ -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(), diff --git a/beacon_chain/utils/ssz_helpers/src/attestation_ssz_splitter.rs b/beacon_chain/utils/ssz_helpers/src/attestation_ssz_splitter.rs index 2c5b1d6aa..a2f05ccb7 100644 --- a/beacon_chain/utils/ssz_helpers/src/attestation_ssz_splitter.rs +++ b/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(), diff --git a/beacon_chain/validation/src/signature_verification.rs b/beacon_chain/validation/src/signature_verification.rs index 18a447267..59fa59dcb 100644 --- a/beacon_chain/validation/src/signature_verification.rs +++ b/beacon_chain/validation/src/signature_verification.rs @@ -128,7 +128,7 @@ mod tests { all_keypairs.append(&mut non_signing_keypairs.clone()); let attestation_indices: Vec = (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(i, true).unwrap(); } diff --git a/beacon_chain/validation/tests/attestation_validation/helpers.rs b/beacon_chain/validation/tests/attestation_validation/helpers.rs index a9c92ca5a..680f979da 100644 --- a/beacon_chain/validation/tests/attestation_validation/helpers.rs +++ b/beacon_chain/validation/tests/attestation_validation/helpers.rs @@ -63,7 +63,7 @@ pub fn generate_attestation( signing_keys: &[Option], block_store: &BeaconBlockStore, ) -> 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 = { diff --git a/beacon_chain/validation/tests/attestation_validation/tests.rs b/beacon_chain/validation/tests/attestation_validation/tests.rs index 4ea954c89..171d983f5 100644 --- a/beacon_chain/validation/tests/attestation_validation/tests.rs +++ b/beacon_chain/validation/tests/attestation_validation/tests.rs @@ -129,12 +129,10 @@ 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 - * of the bitfield. + * We take advantage of the fact that setting a bit outside the current bounds will grow the bitvector. */ let one_byte_higher = rig.attester_count + 8; - rig.attestation.attester_bitfield.set(one_byte_higher, true).unwrap(); - rig.attestation.attester_bitfield.set(one_byte_higher, false).unwrap(); + rig.attestation.attester_bitfield.set(one_byte_higher, false); let result = rig.context.validate_attestation(&rig.attestation); assert_eq!(result, Err(AttestationValidationError::BadBitfieldLength)); @@ -145,7 +143,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.attester_bitfield.set(one_bit_high, true).unwrap(); + rig.attestation.attester_bitfield.set(one_bit_high, true); let result = rig.context.validate_attestation(&rig.attestation); assert_eq!( @@ -169,7 +167,7 @@ fn test_attestation_validation_invalid_invalid_bitfield_end_bit_with_irreguar_bi */ let one_bit_high = rig.attester_count + 1; assert!(one_bit_high % 8 != 0, "the test is ineffective in this case."); - rig.attestation.attester_bitfield.set(one_bit_high, true).unwrap(); + rig.attestation.attester_bitfield.set(one_bit_high, true); let result = rig.context.validate_attestation(&rig.attestation); assert_eq!(result, Err(AttestationValidationError::InvalidBitfieldEndBits)); From 031b7bf225742ddf29b805f1da3fb6d317df579d Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 20 Nov 2018 10:12:49 -0800 Subject: [PATCH 05/11] Add method to calculate the underlying number of bytes Required for part of attestation validation logic --- beacon_chain/utils/boolean-bitfield/src/lib.rs | 18 ++++++++++++++++++ .../validation/src/attestation_validation.rs | 6 +++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/beacon_chain/utils/boolean-bitfield/src/lib.rs b/beacon_chain/utils/boolean-bitfield/src/lib.rs index ceff3bbcf..2a08930fe 100644 --- a/beacon_chain/utils/boolean-bitfield/src/lib.rs +++ b/beacon_chain/utils/boolean-bitfield/src/lib.rs @@ -75,6 +75,11 @@ impl BooleanBitfield { self.0.len() } + /// Returns the number of bytes required to represent this bitfield. + pub fn num_bytes(&self) -> usize { + self.to_bytes().len() + } + /// Returns the number of `1` bits in the bitfield pub fn num_set_bits(&self) -> usize { self.0.iter().filter(|&bit| bit).count() @@ -203,6 +208,7 @@ mod tests { 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 { @@ -213,4 +219,16 @@ mod tests { } } } + + #[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); + } } diff --git a/beacon_chain/validation/src/attestation_validation.rs b/beacon_chain/validation/src/attestation_validation.rs index d63d74483..da761432e 100644 --- a/beacon_chain/validation/src/attestation_validation.rs +++ b/beacon_chain/validation/src/attestation_validation.rs @@ -199,6 +199,10 @@ where } } +fn bytes_for_bits(bits: usize) -> usize { + (bits.saturating_sub(1) / 8) + 1 +} + impl From for AttestationValidationError { fn from(e: ParentHashesError) -> Self { match e { @@ -243,4 +247,4 @@ impl From for AttestationValidationError { => AttestationValidationError::OutOfBoundsBitfieldIndex, } } -} +} \ No newline at end of file From 57dcad149feca2f00786384e70580be58dd82c7c Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 20 Nov 2018 12:27:44 -0800 Subject: [PATCH 06/11] Fixes bug with `ssz` encoding of `BooleanBitfield` --- beacon_chain/types/src/attestation_record.rs | 2 +- .../utils/boolean-bitfield/src/lib.rs | 64 ++++++++++++++++++- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/beacon_chain/types/src/attestation_record.rs b/beacon_chain/types/src/attestation_record.rs index ed38664a9..e4ddffd62 100644 --- a/beacon_chain/types/src/attestation_record.rs +++ b/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_bytes()); + s.append(&self.attester_bitfield); s.append(&self.justified_slot); s.append(&self.justified_block_hash); s.append_vec(&self.aggregate_sig.as_bytes()); diff --git a/beacon_chain/utils/boolean-bitfield/src/lib.rs b/beacon_chain/utils/boolean-bitfield/src/lib.rs index 2a08930fe..b90e4949e 100644 --- a/beacon_chain/utils/boolean-bitfield/src/lib.rs +++ b/beacon_chain/utils/boolean-bitfield/src/lib.rs @@ -86,6 +86,7 @@ impl BooleanBitfield { } /// Returns a vector of bytes representing the bitfield + /// Note that this returns the bit layout of the underlying implementation in the `bit-vec` crate. pub fn to_bytes(&self) -> Vec { self.0.to_bytes() } @@ -99,6 +100,28 @@ impl default::Default for BooleanBitfield { } } +// borrowed from bit_vec crate +fn reverse_bits(byte: u8) -> u8 { + 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) { + let bytes: Vec = self + .to_bytes() + .iter() + .map(|&byte| reverse_bits(byte)) + .collect(); + s.append_vec(&bytes); + } +} + impl ssz::Decodable for BooleanBitfield { fn ssz_decode(bytes: &[u8], index: usize) -> Result<(Self, usize), ssz::DecodeError> { let len = ssz::decode::decode_length(bytes, index, ssz::LENGTH_BYTES)?; @@ -109,7 +132,18 @@ impl ssz::Decodable for BooleanBitfield { if len == 0 { Ok((BooleanBitfield::new(), index + ssz::LENGTH_BYTES)) } else { - let field = BooleanBitfield::from_bytes(&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((field, index)) } @@ -119,6 +153,7 @@ impl ssz::Decodable for BooleanBitfield { #[cfg(test)] mod tests { use super::*; + use ssz::SszStream; #[test] fn test_empty_bitfield() { @@ -231,4 +266,31 @@ mod tests { 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); + } } From 11cdf6607951d8e1de71486e807d2d891b58975f Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 20 Nov 2018 12:51:51 -0800 Subject: [PATCH 07/11] Remove warning about unused import --- beacon_chain/types/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/beacon_chain/types/src/lib.rs b/beacon_chain/types/src/lib.rs index c531da299..65c8b7bce 100644 --- a/beacon_chain/types/src/lib.rs +++ b/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::{ H256, H160, From a6953822609c0fed1a02ef5dc8b5450c4c0ca8f4 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 20 Nov 2018 12:54:35 -0800 Subject: [PATCH 08/11] Run `cargo fmt` that got clobbered in merge --- beacon_chain/types/src/lib.rs | 6 +----- .../validation/src/attestation_validation.rs | 7 ++++--- .../tests/attestation_validation/tests.rs | 14 +++++++++++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/beacon_chain/types/src/lib.rs b/beacon_chain/types/src/lib.rs index 65c8b7bce..c70d62555 100644 --- a/beacon_chain/types/src/lib.rs +++ b/beacon_chain/types/src/lib.rs @@ -14,11 +14,7 @@ pub mod special_record; pub mod validator_record; pub mod validator_registration; -use self::ethereum_types::{ - H256, - H160, - U256 -}; +use self::ethereum_types::{H160, H256, U256}; use std::collections::HashMap; pub use active_state::ActiveState; diff --git a/beacon_chain/validation/src/attestation_validation.rs b/beacon_chain/validation/src/attestation_validation.rs index da761432e..e31f3ae52 100644 --- a/beacon_chain/validation/src/attestation_validation.rs +++ b/beacon_chain/validation/src/attestation_validation.rs @@ -243,8 +243,9 @@ impl From for AttestationValidationError { AttestationValidationError::NoPublicKeyForValidator } SignatureVerificationError::DBError(s) => AttestationValidationError::DBError(s), - SignatureVerificationError::OutOfBoundsBitfieldIndex - => AttestationValidationError::OutOfBoundsBitfieldIndex, + SignatureVerificationError::OutOfBoundsBitfieldIndex => { + AttestationValidationError::OutOfBoundsBitfieldIndex + } } } -} \ No newline at end of file +} diff --git a/beacon_chain/validation/tests/attestation_validation/tests.rs b/beacon_chain/validation/tests/attestation_validation/tests.rs index 171d983f5..e4a86b3e0 100644 --- a/beacon_chain/validation/tests/attestation_validation/tests.rs +++ b/beacon_chain/validation/tests/attestation_validation/tests.rs @@ -132,7 +132,9 @@ fn test_attestation_validation_invalid_bad_bitfield_length() { * We take advantage of the fact that setting a bit outside the current bounds will grow the bitvector. */ let one_byte_higher = rig.attester_count + 8; - rig.attestation.attester_bitfield.set(one_byte_higher, false); + rig.attestation + .attester_bitfield + .set(one_byte_higher, false); let result = rig.context.validate_attestation(&rig.attestation); assert_eq!(result, Err(AttestationValidationError::BadBitfieldLength)); @@ -166,11 +168,17 @@ fn test_attestation_validation_invalid_invalid_bitfield_end_bit_with_irreguar_bi * bit in a bitfield and the byte length of that bitfield */ let one_bit_high = rig.attester_count + 1; - assert!(one_bit_high % 8 != 0, "the test is ineffective in this case."); + assert!( + one_bit_high % 8 != 0, + "the test is ineffective in this case." + ); rig.attestation.attester_bitfield.set(one_bit_high, true); let result = rig.context.validate_attestation(&rig.attestation); - assert_eq!(result, Err(AttestationValidationError::InvalidBitfieldEndBits)); + assert_eq!( + result, + Err(AttestationValidationError::InvalidBitfieldEndBits) + ); } #[test] From f65888226abfb58975fb5802cbf6dac8dd9f23b0 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 23 Nov 2018 08:57:28 +1100 Subject: [PATCH 09/11] Add some extra tests for boolean-bitfield --- .../utils/boolean-bitfield/src/lib.rs | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/beacon_chain/utils/boolean-bitfield/src/lib.rs b/beacon_chain/utils/boolean-bitfield/src/lib.rs index b90e4949e..e0adc64dd 100644 --- a/beacon_chain/utils/boolean-bitfield/src/lib.rs +++ b/beacon_chain/utils/boolean-bitfield/src/lib.rs @@ -11,7 +11,7 @@ use std::default; pub struct BooleanBitfield(BitVec); /// Error represents some reason a request against a bitfield was not satisfied -#[derive(Debug)] +#[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), @@ -156,7 +156,7 @@ mod tests { use ssz::SszStream; #[test] - fn test_empty_bitfield() { + fn test_new_bitfield() { let mut field = BooleanBitfield::new(); let original_len = field.len(); @@ -175,6 +175,31 @@ mod tests { } } + #[test] + fn test_empty_bitfield() { + let mut field = BooleanBitfield::from_elem(0, false); + let original_len = field.len(); + + assert_eq!(original_len, 0); + + for i in 0..100 { + if i < original_len { + assert!(!field.get(i).unwrap()); + } else { + assert!(field.get(i).is_err()); + } + 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] @@ -206,6 +231,9 @@ mod tests { 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); } @@ -255,6 +283,26 @@ mod tests { } } + #[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); From d6bf1a611997ad09c6ad74c25791e49d1f52517c Mon Sep 17 00:00:00 2001 From: mjkeating Date: Fri, 23 Nov 2018 19:29:03 -0800 Subject: [PATCH 10/11] removed unnecessary call to saturated_sub() in vec_shuffle --- beacon_chain/utils/vec_shuffle/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/utils/vec_shuffle/src/lib.rs b/beacon_chain/utils/vec_shuffle/src/lib.rs index 184c5e165..72cbd430c 100644 --- a/beacon_chain/utils/vec_shuffle/src/lib.rs +++ b/beacon_chain/utils/vec_shuffle/src/lib.rs @@ -29,7 +29,7 @@ pub fn shuffle(seed: &[u8], mut list: Vec) -> Result, ShuffleErr> { return Ok(list); } - for i in 0..(list.len().saturating_sub(1)) { + for i in 0..(list.len() - 1) { let n = list.len() - i; let j = rng.rand_range(n as u32) as usize + i; list.swap(i, j); From 3ed4de65d2ccdaebcc1ae5ec414ef26139b651c9 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sun, 25 Nov 2018 15:39:50 +1100 Subject: [PATCH 11/11] Add new `State` type and dependant types --- .../src/candidate_pow_receipt_root_record.rs | 7 +++ beacon_chain/types/src/lib.rs | 3 ++ .../types/src/shard_reassignment_record.rs | 6 +++ beacon_chain/types/src/state.rs | 54 +++++++++++++++++++ 4 files changed, 70 insertions(+) create mode 100644 beacon_chain/types/src/candidate_pow_receipt_root_record.rs create mode 100644 beacon_chain/types/src/shard_reassignment_record.rs create mode 100644 beacon_chain/types/src/state.rs diff --git a/beacon_chain/types/src/candidate_pow_receipt_root_record.rs b/beacon_chain/types/src/candidate_pow_receipt_root_record.rs new file mode 100644 index 000000000..9a1e3e5c8 --- /dev/null +++ b/beacon_chain/types/src/candidate_pow_receipt_root_record.rs @@ -0,0 +1,7 @@ +use super::Hash256; + +#[derive(Debug, PartialEq)] +pub struct CandidatePoWReceiptRootRecord { + pub candidate_pow_receipt_root: Hash256, + pub votes: u64, +} diff --git a/beacon_chain/types/src/lib.rs b/beacon_chain/types/src/lib.rs index c70d62555..0cb9b8de0 100644 --- a/beacon_chain/types/src/lib.rs +++ b/beacon_chain/types/src/lib.rs @@ -6,11 +6,14 @@ extern crate ssz; pub mod active_state; pub mod attestation_record; pub mod beacon_block; +pub mod candidate_pow_receipt_root_record; pub mod chain_config; pub mod crosslink_record; pub mod crystallized_state; pub mod shard_and_committee; +pub mod shard_reassignment_record; pub mod special_record; +pub mod state; pub mod validator_record; pub mod validator_registration; diff --git a/beacon_chain/types/src/shard_reassignment_record.rs b/beacon_chain/types/src/shard_reassignment_record.rs new file mode 100644 index 000000000..2f2c8a8d9 --- /dev/null +++ b/beacon_chain/types/src/shard_reassignment_record.rs @@ -0,0 +1,6 @@ +#[derive(Debug, PartialEq)] +pub struct ShardReassignmentRecord { + pub validator_index: u64, + pub shard: u64, + pub slot: u64, +} diff --git a/beacon_chain/types/src/state.rs b/beacon_chain/types/src/state.rs new file mode 100644 index 000000000..1d65c3dae --- /dev/null +++ b/beacon_chain/types/src/state.rs @@ -0,0 +1,54 @@ +use super::attestation_record::AttestationRecord; +use super::candidate_pow_receipt_root_record::CandidatePoWReceiptRootRecord; +use super::crosslink_record::CrosslinkRecord; +use super::shard_and_committee::ShardAndCommittee; +use super::shard_reassignment_record::ShardReassignmentRecord; +use super::validator_record::ValidatorRecord; +use super::Hash256; + +#[derive(Debug, PartialEq)] +pub struct BeaconState { + // Slot of last validator set change + validator_set_change_slot: u64, + // List of validators + validators: Vec, + // Most recent crosslink for each shard + crosslinks: Vec, + // Last cycle-boundary state recalculation + last_state_recalculation_slot: u64, + // Last finalized slot + last_finalized_slot: u64, + // Last justified slot + last_justified_slot: u64, + // Number of consecutive justified slots + justified_streak: u64, + // Committee members and their assigned shard, per slot + shard_and_committee_for_slots: Vec>, + // Persistent shard committees + persistent_committees: Vec>, + persistent_committee_reassignments: Vec, + // Randao seed used for next shuffling + next_shuffling_seed: Hash256, + // Total deposits penalized in the given withdrawal period + deposits_penalized_in_period: Vec, + // Hash chain of validator set changes (for light clients to easily track deltas) + validator_set_delta_hash_chain: Hash256, + // Current sequence number for withdrawals + current_exit_seq: u64, + // Genesis time + genesis_time: u64, + // PoW receipt root + processed_pow_receipt_root: Hash256, + candidate_pow_receipt_roots: Vec, + // Parameters relevant to hard forks / versioning. + // Should be updated only by hard forks. + pre_fork_version: u64, + post_fork_version: u64, + fork_slot_number: u64, + // Attestations not yet processed + pending_attestations: Vec, + // recent beacon block hashes needed to process attestations, older to newer + recent_block_hashes: Vec, + // RANDAO state + randao_mix: Hash256, +}