Merge pull request #86 from ralexstokes/magic

Reduce use of "magic numbers"

ssz-rs/src/bitlist.rs

@@ -2,15 +2,19 @@ use crate::{
     de::{Deserialize, DeserializeError},
     error::{Error, InstanceError},
     lib::*,
-    merkleization::{merkleize, mix_in_length, pack_bytes, MerkleizationError, Merkleized, Node},
+    merkleization::{
+        merkleize, mix_in_length, pack_bytes, MerkleizationError, Merkleized, Node, BITS_PER_CHUNK,
+    },
     ser::{Serialize, SerializeError},
     SimpleSerialize, Sized,
 };
 use bitvec::prelude::{BitVec, Lsb0};
 
+const BITS_PER_BYTE: usize = crate::BITS_PER_BYTE as usize;
+
 // +1 for length bit
 fn byte_length(bound: usize) -> usize {
-    (bound + 7 + 1) / 8
+    (bound + BITS_PER_BYTE - 1 + 1) / BITS_PER_BYTE
 }
 
 type BitlistInner = BitVec<u8, Lsb0>;
@@ -103,7 +107,7 @@ impl<const N: usize> Bitlist<N> {
 
         if with_length_bit {
             let element_count = self.len();
-            let marker_index = element_count % 8;
+            let marker_index = element_count % BITS_PER_BYTE;
             if marker_index == 0 {
                 buffer.push(1u8);
             } else {
@@ -114,6 +118,10 @@ impl<const N: usize> Bitlist<N> {
         // SAFETY: checked subtraction is unnecessary, as buffer.len() > start_len; qed
         Ok(buffer.len() - start_len)
     }
+
+    fn chunk_count() -> usize {
+        (N + BITS_PER_CHUNK - 1) / BITS_PER_CHUNK
+    }
 }
 
 impl<const N: usize> Deref for Bitlist<N> {
@@ -174,7 +182,7 @@ impl<const N: usize> Deserialize for Bitlist<N> {
         // SAFETY: checked subtraction is unnecessary,
         // as last_byte != 0, so last.trailing_zeros <= 7; qed
         // therefore: bit_length >= 1
-        let bit_length = 8 - last.trailing_zeros();
+        let bit_length = BITS_PER_BYTE - last.trailing_zeros();
         let additional_members = bit_length - 1; // skip marker bit
         let total_members = result.len() + additional_members;
         if total_members > N {
@@ -192,7 +200,7 @@ impl<const N: usize> Deserialize for Bitlist<N> {
 impl<const N: usize> Merkleized for Bitlist<N> {
     fn hash_tree_root(&mut self) -> Result<Node, MerkleizationError> {
         let chunks = self.pack_bits()?;
-        let data_root = merkleize(&chunks, Some((N + 255) / 256))?;
+        let data_root = merkleize(&chunks, Some(Self::chunk_count()))?;
         Ok(mix_in_length(&data_root, self.len()))
     }
 }

ssz-rs/src/bitvector.rs

@@ -2,7 +2,7 @@ use crate::{
     de::{Deserialize, DeserializeError},
     error::{Error, InstanceError, TypeError},
     lib::*,
-    merkleization::{merkleize, pack_bytes, MerkleizationError, Merkleized, Node},
+    merkleization::{merkleize, pack_bytes, MerkleizationError, Merkleized, Node, BITS_PER_CHUNK},
     ser::{Serialize, SerializeError},
     SimpleSerialize, Sized,
 };
@@ -11,8 +11,10 @@ use bitvec::{
     prelude::{BitVec, Lsb0},
 };
 
+const BITS_PER_BYTE: usize = crate::BITS_PER_BYTE as usize;
+
 fn byte_length(bound: usize) -> usize {
-    (bound + 7) / 8
+    (bound + BITS_PER_BYTE - 1) / BITS_PER_BYTE
 }
 
 type BitvectorInner = BitVec<u8, Lsb0>;
@@ -102,6 +104,10 @@ impl<const N: usize> Bitvector<N> {
         pack_bytes(&mut data);
         Ok(data)
     }
+
+    fn chunk_count() -> usize {
+        (N + BITS_PER_CHUNK - 1) / BITS_PER_CHUNK
+    }
 }
 
 impl<const N: usize> Deref for Bitvector<N> {
@@ -124,7 +130,7 @@ impl<const N: usize> Sized for Bitvector<N> {
     }
 
     fn size_hint() -> usize {
-        (N + 7) / 8
+        byte_length(N)
     }
 }
 
@@ -135,7 +141,7 @@ impl<const N: usize> Serialize for Bitvector<N> {
         }
         let bytes_to_write = Self::size_hint();
         buffer.reserve(bytes_to_write);
-        for byte in self.chunks(8) {
+        for byte in self.chunks(BITS_PER_BYTE) {
             buffer.push(byte.load());
         }
         Ok(bytes_to_write)
@@ -163,10 +169,10 @@ impl<const N: usize> Deserialize for Bitvector<N> {
         }
 
         let mut result = Self::default();
-        for (slot, byte) in result.chunks_mut(8).zip(encoding.iter().copied()) {
+        for (slot, byte) in result.chunks_mut(BITS_PER_BYTE).zip(encoding.iter().copied()) {
             slot.store_le(byte);
         }
-        let remainder_count = N % 8;
+        let remainder_count = N % BITS_PER_BYTE;
         if remainder_count != 0 {
             let last_byte = encoding.last().unwrap();
             let remainder_bits = last_byte >> remainder_count;
@@ -181,7 +187,7 @@ impl<const N: usize> Deserialize for Bitvector<N> {
 impl<const N: usize> Merkleized for Bitvector<N> {
     fn hash_tree_root(&mut self) -> Result<Node, MerkleizationError> {
         let chunks = self.pack_bits()?;
-        merkleize(&chunks, Some((N + 255) / 256))
+        merkleize(&chunks, Some(Self::chunk_count()))
     }
 }
 

ssz-rs/src/lib.rs

@@ -83,12 +83,16 @@ mod lib {
     pub use self::core::marker::PhantomData;
 }
 
+pub(crate) const BITS_PER_BYTE: u32 = 8;
+
 /// `Sized` is a trait for types that can
 /// provide sizing information relevant for the SSZ spec.
 pub trait Sized {
     // is this type variable or fixed size?
     fn is_variable_size() -> bool;
 
+    // expected number of bytes for the serialization of this type
+    // or 0 if unknown ahead of time
     fn size_hint() -> usize;
 }
 

ssz-rs/src/list.rs

@@ -4,6 +4,7 @@ use crate::{
     lib::*,
     merkleization::{
         elements_to_chunks, merkleize, mix_in_length, pack, MerkleizationError, Merkleized, Node,
+        BYTES_PER_CHUNK,
     },
     ser::{Serialize, SerializeError, Serializer},
     SimpleSerialize, Sized,
@@ -218,6 +219,11 @@ impl<T, const N: usize> List<T, N>
 where
     T: SimpleSerialize,
 {
+    // Number of chunks for this type, rounded up to a complete number of chunks
+    fn chunk_count() -> usize {
+        (N * T::size_hint() + BYTES_PER_CHUNK - 1) / BYTES_PER_CHUNK
+    }
+
     fn compute_hash_tree_root(&mut self) -> Result<Node, MerkleizationError> {
         if T::is_composite_type() {
             let count = self.len();
@@ -226,8 +232,7 @@ where
             Ok(mix_in_length(&data_root, self.len()))
         } else {
             let chunks = pack(self)?;
-            let chunk_count = (N * T::size_hint() + 31) / 32;
-            let data_root = merkleize(&chunks, Some(chunk_count))?;
+            let data_root = merkleize(&chunks, Some(Self::chunk_count()))?;
             Ok(mix_in_length(&data_root, self.len()))
         }
     }

ssz-rs/src/merkleization/mod.rs

@@ -11,6 +11,7 @@ pub use node::Node;
 pub use proofs::is_valid_merkle_branch;
 
 pub(crate) const BYTES_PER_CHUNK: usize = 32;
+pub(crate) const BITS_PER_CHUNK: usize = BYTES_PER_CHUNK * (crate::BITS_PER_BYTE as usize);
 
 /// A `Merkleized` type provides a "hash tree root" following the SSZ spec.
 pub trait Merkleized {

ssz-rs/src/ser.rs

@@ -5,7 +5,7 @@ use crate::{
 };
 
 // NOTE: if this is changed, go change in `ssz_derive` as well!
-pub const BYTES_PER_LENGTH_OFFSET: usize = 4;
+pub(crate) const BYTES_PER_LENGTH_OFFSET: usize = 4;
 const MAXIMUM_LENGTH: u64 = 2u64.pow((8 * BYTES_PER_LENGTH_OFFSET) as u32);
 
 /// Serialization errors.

ssz-rs/src/uint.rs

@@ -3,10 +3,15 @@ use crate::{
     lib::*,
     merkleization::{pack_bytes, MerkleizationError, Merkleized, Node},
     ser::{Serialize, SerializeError},
-    SimpleSerialize, Sized,
+    SimpleSerialize, Sized, BITS_PER_BYTE,
 };
 use num_bigint::BigUint;
 
+#[inline]
+fn bits_to_bytes(count: u32) -> usize {
+    (count / BITS_PER_BYTE) as usize
+}
+
 macro_rules! define_uint {
     ($uint:ty) => {
         impl Sized for $uint {
@@ -15,20 +20,20 @@ macro_rules! define_uint {
             }
 
             fn size_hint() -> usize {
-                (<$uint>::BITS / 8) as usize
+                bits_to_bytes(<$uint>::BITS)
             }
         }
 
         impl Serialize for $uint {
             fn serialize(&self, buffer: &mut Vec<u8>) -> Result<usize, SerializeError> {
                 buffer.extend_from_slice(&self.to_le_bytes());
-                Ok((<$uint>::BITS / 8) as usize)
+                Ok(bits_to_bytes(<$uint>::BITS))
             }
         }
 
         impl Deserialize for $uint {
             fn deserialize(encoding: &[u8]) -> Result<Self, DeserializeError> {
-                let byte_size = (<$uint>::BITS / 8) as usize;
+                let byte_size = bits_to_bytes(<$uint>::BITS);
                 if encoding.len() < byte_size {
                     return Err(DeserializeError::ExpectedFurtherInput {
                         provided: encoding.len(),