From 0a02567440f1e77dda40e0904b63077fb238d4e0 Mon Sep 17 00:00:00 2001
From: Michael Sproul <michael@sigmaprime.io>
Date: Tue, 2 Apr 2019 14:14:57 +1100
Subject: [PATCH] bitfield: fix bit ordering issue with YAML parsing

---
 .../src/common/verify_bitfield.rs             | 13 ++++++
 eth2/utils/boolean-bitfield/Cargo.toml        |  4 ++
 eth2/utils/boolean-bitfield/src/lib.rs        | 44 ++++++++++++++++---
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/eth2/state_processing/src/common/verify_bitfield.rs b/eth2/state_processing/src/common/verify_bitfield.rs
index 03fcdbb67..71c9f9c3e 100644
--- a/eth2/state_processing/src/common/verify_bitfield.rs
+++ b/eth2/state_processing/src/common/verify_bitfield.rs
@@ -18,3 +18,16 @@ pub fn verify_bitfield_length(bitfield: &Bitfield, committee_size: usize) -> boo
 
     true
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn bitfield_length() {
+        assert!(verify_bitfield_length(
+            &Bitfield::from_bytes(&[0b10000000]),
+            4
+        ));
+    }
+}
diff --git a/eth2/utils/boolean-bitfield/Cargo.toml b/eth2/utils/boolean-bitfield/Cargo.toml
index f08695bd1..61bbc60a8 100644
--- a/eth2/utils/boolean-bitfield/Cargo.toml
+++ b/eth2/utils/boolean-bitfield/Cargo.toml
@@ -8,6 +8,10 @@ edition = "2018"
 serde_hex = { path = "../serde_hex" }
 ssz = { path = "../ssz" }
 bit-vec = "0.5.0"
+bit_reverse = "0.1"
 serde = "1.0"
 serde_derive = "1.0"
 tree_hash = { path = "../tree_hash" }
+
+[dev-dependencies]
+serde_yaml = "0.8"
diff --git a/eth2/utils/boolean-bitfield/src/lib.rs b/eth2/utils/boolean-bitfield/src/lib.rs
index d35d87c5c..c19702ec9 100644
--- a/eth2/utils/boolean-bitfield/src/lib.rs
+++ b/eth2/utils/boolean-bitfield/src/lib.rs
@@ -1,8 +1,8 @@
 extern crate bit_vec;
 extern crate ssz;
 
+use bit_reverse::LookupReverse;
 use bit_vec::BitVec;
-
 use serde::de::{Deserialize, Deserializer};
 use serde::ser::{Serialize, Serializer};
 use serde_hex::{encode, PrefixedHexVisitor};
@@ -236,24 +236,36 @@ impl Decodable for BooleanBitfield {
     }
 }
 
+// Reverse the bit order of a whole byte vec, so that the ith bit
+// of the input vec is placed in the (N - i)th bit of the output vec.
+// This function is necessary for converting bitfields to and from YAML,
+// as the BitVec library and the hex-parser use opposing bit orders.
+fn reverse_bit_order(mut bytes: Vec<u8>) -> Vec<u8> {
+    bytes.reverse();
+    bytes.into_iter().map(|b| b.swap_bits()).collect()
+}
+
 impl Serialize for BooleanBitfield {
-    /// Serde serialization is compliant the Ethereum YAML test format.
+    /// Serde serialization is compliant with the Ethereum YAML test format.
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: Serializer,
     {
-        serializer.serialize_str(&encode(&self.to_bytes()))
+        serializer.serialize_str(&encode(&reverse_bit_order(self.to_bytes())))
     }
 }
 
 impl<'de> Deserialize<'de> for BooleanBitfield {
-    /// Serde serialization is compliant the Ethereum YAML test format.
+    /// Serde serialization is compliant with the Ethereum YAML test format.
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
+        // We reverse the bit-order so that the BitVec library can read its 0th
+        // bit from the end of the hex string, e.g.
+        // "0xef01" => [0xef, 0x01] => [0b1000_0000, 0b1111_1110]
         let bytes = deserializer.deserialize_str(PrefixedHexVisitor)?;
-        Ok(BooleanBitfield::from_bytes(&bytes))
+        Ok(BooleanBitfield::from_bytes(&reverse_bit_order(bytes)))
     }
 }
 
@@ -262,6 +274,7 @@ tree_hash_ssz_encoding_as_list!(BooleanBitfield);
 #[cfg(test)]
 mod tests {
     use super::*;
+    use serde_yaml;
     use ssz::{decode, ssz_encode, SszStream};
 
     #[test]
@@ -462,6 +475,27 @@ mod tests {
         assert_eq!(field, expected);
     }
 
+    #[test]
+    fn test_serialize_deserialize() {
+        use serde_yaml::Value;
+
+        let data: &[(_, &[_])] = &[
+            ("0x01", &[0b10000000]),
+            ("0xf301", &[0b10000000, 0b11001111]),
+        ];
+        for (hex_data, bytes) in data {
+            let bitfield = BooleanBitfield::from_bytes(bytes);
+            assert_eq!(
+                serde_yaml::from_str::<BooleanBitfield>(hex_data).unwrap(),
+                bitfield
+            );
+            assert_eq!(
+                serde_yaml::to_value(&bitfield).unwrap(),
+                Value::String(hex_data.to_string())
+            );
+        }
+    }
+
     #[test]
     fn test_ssz_round_trip() {
         let original = BooleanBitfield::from_bytes(&vec![18; 12][..]);