From 561cec0bf69f9f7914b2a3435d3da062916b1165 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 11 Jul 2019 13:19:38 +1000 Subject: [PATCH] Move many bitfield Options to Results --- eth2/utils/ssz_types/src/bitfield.rs | 208 ++++++++++++---------- eth2/utils/ssz_types/src/fixed_vector.rs | 2 +- eth2/utils/ssz_types/src/lib.rs | 15 +- eth2/utils/ssz_types/src/variable_list.rs | 4 +- 4 files changed, 127 insertions(+), 102 deletions(-) diff --git a/eth2/utils/ssz_types/src/bitfield.rs b/eth2/utils/ssz_types/src/bitfield.rs index 6af82b48d..de9a198f3 100644 --- a/eth2/utils/ssz_types/src/bitfield.rs +++ b/eth2/utils/ssz_types/src/bitfield.rs @@ -1,3 +1,4 @@ +use crate::Error; use core::marker::PhantomData; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; @@ -62,11 +63,11 @@ pub type BitVector = Bitfield>; /// type BitList8 = BitList; /// /// // Creating a `BitList` with a larger-than-`N` capacity returns `None`. -/// assert!(BitList8::with_capacity(9).is_none()); +/// assert!(BitList8::with_capacity(9).is_err()); /// /// let mut bitlist = BitList8::with_capacity(4).unwrap(); // `BitList` permits a capacity of less than the maximum. -/// assert!(bitlist.set(3, true).is_some()); // Setting inside the instantiation capacity is permitted. -/// assert!(bitlist.set(5, true).is_none()); // Setting outside that capacity is not. +/// assert!(bitlist.set(3, true).is_ok()); // Setting inside the instantiation capacity is permitted. +/// assert!(bitlist.set(5, true).is_err()); // Setting outside that capacity is not. /// /// // `BitVector` has a type-level fixed length. Unlike `BitList`, it cannot be instantiated with a custom length /// // or grow/shrink. @@ -74,8 +75,8 @@ pub type BitVector = Bitfield>; /// /// let mut bitvector = BitVector8::new(); /// assert_eq!(bitvector.len(), 8); // `BitVector` length is fixed at the type-level. -/// assert!(bitvector.set(7, true).is_some()); // Setting inside the capacity is permitted. -/// assert!(bitvector.set(9, true).is_none()); // Setting outside the capacity is not. +/// assert!(bitvector.set(7, true).is_ok()); // Setting inside the capacity is permitted. +/// assert!(bitvector.set(9, true).is_err()); // Setting outside the capacity is not. /// /// ``` /// @@ -98,15 +99,18 @@ impl Bitfield> { /// All bits are initialized to `false`. /// /// Returns `None` if `num_bits > N`. - pub fn with_capacity(num_bits: usize) -> Option { + pub fn with_capacity(num_bits: usize) -> Result { if num_bits <= N::to_usize() { - Some(Self { + Ok(Self { bytes: vec![0; bytes_for_bit_len(num_bits)], len: num_bits, _phantom: PhantomData, }) } else { - None + Err(Error::OutOfBounds { + i: Self::max_len(), + len: Self::max_len(), + }) } } @@ -149,14 +153,16 @@ impl Bitfield> { /// produces (SSZ). /// /// Returns `None` if `bytes` are not a valid encoding. - pub fn from_bytes(bytes: Vec) -> Option { + pub fn from_bytes(bytes: Vec) -> Result { let mut initial_bitfield: Bitfield> = { let num_bits = bytes.len() * 8; Bitfield::from_raw_bytes(bytes, num_bits) .expect("Must have adequate bytes for bit count.") }; - let len = initial_bitfield.highest_set_bit()?; + let len = initial_bitfield + .highest_set_bit() + .ok_or_else(|| Error::MissingLengthInformation)?; if len <= Self::max_len() { initial_bitfield @@ -171,7 +177,10 @@ impl Bitfield> { Self::from_raw_bytes(bytes, len) } else { - None + Err(Error::OutOfBounds { + i: Self::max_len(), + len: Self::max_len(), + }) } } } @@ -213,7 +222,7 @@ impl Bitfield> { /// produces (SSZ). /// /// Returns `None` if `bytes` are not a valid encoding. - pub fn from_bytes(bytes: Vec) -> Option { + pub fn from_bytes(bytes: Vec) -> Result { Self::from_raw_bytes(bytes, Self::capacity()) } } @@ -228,7 +237,7 @@ impl Bitfield { /// Sets the `i`'th bit to `value`. /// /// Returns `None` if `i` is out-of-bounds of `self`. - pub fn set(&mut self, i: usize, value: bool) -> Option<()> { + pub fn set(&mut self, i: usize, value: bool) -> Result<(), Error> { if i < self.len { let byte = { let num_bytes = self.bytes.len(); @@ -244,16 +253,16 @@ impl Bitfield { *byte &= !(1 << (i % 8)) } - Some(()) + Ok(()) } else { - None + Err(Error::OutOfBounds { i, len: self.len }) } } /// Returns the value of the `i`'th bit. /// /// Returns `None` if `i` is out-of-bounds of `self`. - pub fn get(&self, i: usize) -> Option { + pub fn get(&self, i: usize) -> Result { if i < self.len { let byte = { let num_bytes = self.bytes.len(); @@ -263,9 +272,9 @@ impl Bitfield { .expect("Cannot be OOB if less than self.len") }; - Some(*byte & 1 << (i % 8) > 0) + Ok(*byte & 1 << (i % 8) > 0) } else { - None + Err(Error::OutOfBounds { i, len: self.len }) } } @@ -297,33 +306,36 @@ impl Bitfield { /// - `bytes` is not the minimal required bytes to represent a bitfield of `bit_len` bits. /// - `bit_len` is not a multiple of 8 and `bytes` contains set bits that are higher than, or /// equal to `bit_len`. - fn from_raw_bytes(bytes: Vec, bit_len: usize) -> Option { + fn from_raw_bytes(bytes: Vec, bit_len: usize) -> Result { if bit_len == 0 { if bytes.len() == 1 && bytes == [0] { // A bitfield with `bit_len` 0 can only be represented by a single zero byte. - Some(Self { + Ok(Self { bytes, len: 0, _phantom: PhantomData, }) } else { - None + Err(Error::ExcessBits) } - } else if bytes.len() != bytes_for_bit_len(bit_len) || bytes.is_empty() { + } else if bytes.len() != bytes_for_bit_len(bit_len) { // The number of bytes must be the minimum required to represent `bit_len`. - None + Err(Error::InvalidByteCount { + given: bytes.len(), + expected: bytes_for_bit_len(bit_len), + }) } else { // Ensure there are no bits higher than `bit_len` that are set to true. let (mask, _) = u8::max_value().overflowing_shr(8 - (bit_len as u32 % 8)); if (bytes.first().expect("Guarded against empty bytes") & !mask) == 0 { - Some(Self { + Ok(Self { bytes, len: bit_len, _phantom: PhantomData, }) } else { - None + Err(Error::ExcessBits) } } } @@ -449,9 +461,9 @@ impl<'a, T: BitfieldBehaviour> Iterator for BitIter<'a, T> { type Item = bool; fn next(&mut self) -> Option { - let res = self.bitfield.get(self.i); + let res = self.bitfield.get(self.i).ok()?; self.i += 1; - res + Some(res) } } @@ -471,8 +483,9 @@ impl Decode for Bitfield> { } fn from_ssz_bytes(bytes: &[u8]) -> Result { - Self::from_bytes(bytes.to_vec()) - .ok_or_else(|| ssz::DecodeError::BytesInvalid("Variable failed to decode".to_string())) + Self::from_bytes(bytes.to_vec()).map_err(|e| { + ssz::DecodeError::BytesInvalid(format!("BitList failed to decode: {:?}", e)) + }) } } @@ -496,8 +509,9 @@ impl Decode for Bitfield> { } fn from_ssz_bytes(bytes: &[u8]) -> Result { - Self::from_bytes(bytes.to_vec()) - .ok_or_else(|| ssz::DecodeError::BytesInvalid("Fixed failed to decode".to_string())) + Self::from_bytes(bytes.to_vec()).map_err(|e| { + ssz::DecodeError::BytesInvalid(format!("BitVector failed to decode: {:?}", e)) + }) } } @@ -725,34 +739,34 @@ mod bitvector { assert_round_trip(BitVector0::new()); let mut b = BitVector1::new(); - b.set(0, true); + b.set(0, true).unwrap(); assert_round_trip(b); let mut b = BitVector8::new(); for j in 0..8 { if j % 2 == 0 { - b.set(j, true); + b.set(j, true).unwrap(); } } assert_round_trip(b); let mut b = BitVector8::new(); for j in 0..8 { - b.set(j, true); + b.set(j, true).unwrap(); } assert_round_trip(b); let mut b = BitVector16::new(); for j in 0..16 { if j % 2 == 0 { - b.set(j, true); + b.set(j, true).unwrap(); } } assert_round_trip(b); let mut b = BitVector16::new(); for j in 0..16 { - b.set(j, true); + b.set(j, true).unwrap(); } assert_round_trip(b); } @@ -853,21 +867,21 @@ mod bitlist { } let mut b = BitList1::with_capacity(1).unwrap(); - b.set(0, true); + b.set(0, true).unwrap(); assert_round_trip(b); for i in 0..8 { let mut b = BitList8::with_capacity(i).unwrap(); for j in 0..i { if j % 2 == 0 { - b.set(j, true); + b.set(j, true).unwrap(); } } assert_round_trip(b); let mut b = BitList8::with_capacity(i).unwrap(); for j in 0..i { - b.set(j, true); + b.set(j, true).unwrap(); } assert_round_trip(b); } @@ -876,14 +890,14 @@ mod bitlist { let mut b = BitList16::with_capacity(i).unwrap(); for j in 0..i { if j % 2 == 0 { - b.set(j, true); + b.set(j, true).unwrap(); } } assert_round_trip(b); let mut b = BitList16::with_capacity(i).unwrap(); for j in 0..i { - b.set(j, true); + b.set(j, true).unwrap(); } assert_round_trip(b); } @@ -895,50 +909,50 @@ mod bitlist { #[test] fn from_raw_bytes() { - assert!(BitList1024::from_raw_bytes(vec![0b0000_0000], 0).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0001], 1).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0011], 2).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0111], 3).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_1111], 4).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0001_1111], 5).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0011_1111], 6).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0111_1111], 7).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b1111_1111], 8).is_some()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0000], 0).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0001], 1).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0011], 2).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0111], 3).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_1111], 4).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0001_1111], 5).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0011_1111], 6).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0111_1111], 7).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b1111_1111], 8).is_ok()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0001, 0b1111_1111], 9).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0011, 0b1111_1111], 10).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0111, 0b1111_1111], 11).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_1111, 0b1111_1111], 12).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0001_1111, 0b1111_1111], 13).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0011_1111, 0b1111_1111], 14).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b0111_1111, 0b1111_1111], 15).is_some()); - assert!(BitList1024::from_raw_bytes(vec![0b1111_1111, 0b1111_1111], 16).is_some()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0001, 0b1111_1111], 9).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0011, 0b1111_1111], 10).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0111, 0b1111_1111], 11).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_1111, 0b1111_1111], 12).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0001_1111, 0b1111_1111], 13).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0011_1111, 0b1111_1111], 14).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b0111_1111, 0b1111_1111], 15).is_ok()); + assert!(BitList1024::from_raw_bytes(vec![0b1111_1111, 0b1111_1111], 16).is_ok()); for i in 0..8 { - assert!(BitList1024::from_raw_bytes(vec![], i).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b1111_1111], i).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b1111_1110, 0b0000_0000], i).is_none()); + assert!(BitList1024::from_raw_bytes(vec![], i).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b1111_1111], i).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b1111_1110, 0b0000_0000], i).is_err()); } - assert!(BitList1024::from_raw_bytes(vec![0b0000_0001], 0).is_none()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0001], 0).is_err()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0001], 0).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0011], 1).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0111], 2).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_1111], 3).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0001_1111], 4).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0011_1111], 5).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0111_1111], 6).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b1111_1111], 7).is_none()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0001], 0).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0011], 1).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0111], 2).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_1111], 3).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0001_1111], 4).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0011_1111], 5).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0111_1111], 6).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b1111_1111], 7).is_err()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0001, 0b1111_1111], 8).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0011, 0b1111_1111], 9).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_0111, 0b1111_1111], 10).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0000_1111, 0b1111_1111], 11).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0001_1111, 0b1111_1111], 12).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0011_1111, 0b1111_1111], 13).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b0111_1111, 0b1111_1111], 14).is_none()); - assert!(BitList1024::from_raw_bytes(vec![0b1111_1111, 0b1111_1111], 15).is_none()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0001, 0b1111_1111], 8).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0011, 0b1111_1111], 9).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_0111, 0b1111_1111], 10).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0000_1111, 0b1111_1111], 11).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0001_1111, 0b1111_1111], 12).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0011_1111, 0b1111_1111], 13).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b0111_1111, 0b1111_1111], 14).is_err()); + assert!(BitList1024::from_raw_bytes(vec![0b1111_1111, 0b1111_1111], 15).is_err()); } fn test_set_unset(num_bits: usize) { @@ -947,17 +961,17 @@ mod bitlist { for i in 0..num_bits + 1 { if i < num_bits { // Starts as false - assert_eq!(bitfield.get(i), Some(false)); + assert_eq!(bitfield.get(i), Ok(false)); // Can be set true. - assert!(bitfield.set(i, true).is_some()); - assert_eq!(bitfield.get(i), Some(true)); + assert!(bitfield.set(i, true).is_ok()); + assert_eq!(bitfield.get(i), Ok(true)); // Can be set false - assert!(bitfield.set(i, false).is_some()); - assert_eq!(bitfield.get(i), Some(false)); + assert!(bitfield.set(i, false).is_ok()); + assert_eq!(bitfield.get(i), Ok(false)); } else { - assert_eq!(bitfield.get(i), None); - assert!(bitfield.set(i, true).is_none()); - assert_eq!(bitfield.get(i), None); + assert!(bitfield.get(i).is_err()); + assert!(bitfield.set(i, true).is_err()); + assert!(bitfield.get(i).is_err()); } } } @@ -989,47 +1003,47 @@ mod bitlist { #[test] fn into_raw_bytes() { let mut bitfield = BitList1024::with_capacity(9).unwrap(); - bitfield.set(0, true); + bitfield.set(0, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0000, 0b0000_0001] ); - bitfield.set(1, true); + bitfield.set(1, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0000, 0b0000_0011] ); - bitfield.set(2, true); + bitfield.set(2, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0000, 0b0000_0111] ); - bitfield.set(3, true); + bitfield.set(3, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0000, 0b0000_1111] ); - bitfield.set(4, true); + bitfield.set(4, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0000, 0b0001_1111] ); - bitfield.set(5, true); + bitfield.set(5, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0000, 0b0011_1111] ); - bitfield.set(6, true); + bitfield.set(6, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0000, 0b0111_1111] ); - bitfield.set(7, true); + bitfield.set(7, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0000, 0b1111_1111] ); - bitfield.set(8, true); + bitfield.set(8, true).unwrap(); assert_eq!( bitfield.clone().into_raw_bytes(), vec![0b0000_0001, 0b1111_1111] @@ -1115,8 +1129,8 @@ mod bitlist { #[test] fn iter() { let mut bitfield = BitList1024::with_capacity(9).unwrap(); - bitfield.set(2, true); - bitfield.set(8, true); + bitfield.set(2, true).unwrap(); + bitfield.set(8, true).unwrap(); assert_eq!( bitfield.iter().collect::>(), diff --git a/eth2/utils/ssz_types/src/fixed_vector.rs b/eth2/utils/ssz_types/src/fixed_vector.rs index 8d7c2264e..687d7d738 100644 --- a/eth2/utils/ssz_types/src/fixed_vector.rs +++ b/eth2/utils/ssz_types/src/fixed_vector.rs @@ -59,7 +59,7 @@ impl FixedVector { _phantom: PhantomData, }) } else { - Err(Error::InvalidLength { + Err(Error::OutOfBounds { i: vec.len(), len: Self::capacity(), }) diff --git a/eth2/utils/ssz_types/src/lib.rs b/eth2/utils/ssz_types/src/lib.rs index 8e7595d48..59869b7c0 100644 --- a/eth2/utils/ssz_types/src/lib.rs +++ b/eth2/utils/ssz_types/src/lib.rs @@ -50,6 +50,17 @@ pub mod length { /// Returned when an item encounters an error. #[derive(PartialEq, Debug)] pub enum Error { - InvalidLength { i: usize, len: usize }, - OutOfBounds { i: usize, len: usize }, + OutOfBounds { + i: usize, + len: usize, + }, + /// A `BitList` does not have a set bit, therefore it's length is unknowable. + MissingLengthInformation, + /// A `BitList` has excess bits set to true. + ExcessBits, + /// A `BitList` has an invalid number of bytes for a given bit length. + InvalidByteCount { + given: usize, + expected: usize, + }, } diff --git a/eth2/utils/ssz_types/src/variable_list.rs b/eth2/utils/ssz_types/src/variable_list.rs index 68f03deed..52872ada6 100644 --- a/eth2/utils/ssz_types/src/variable_list.rs +++ b/eth2/utils/ssz_types/src/variable_list.rs @@ -61,7 +61,7 @@ impl VariableList { _phantom: PhantomData, }) } else { - Err(Error::InvalidLength { + Err(Error::OutOfBounds { i: vec.len(), len: Self::max_len(), }) @@ -91,7 +91,7 @@ impl VariableList { self.vec.push(value); Ok(()) } else { - Err(Error::InvalidLength { + Err(Error::OutOfBounds { i: self.vec.len() + 1, len: Self::max_len(), })