From 304793a6ab1c8790b7df0633dd4028dd05cbe6b9 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Thu, 29 Oct 2020 23:25:21 +0000 Subject: [PATCH] add quoted serialization util for `FixedVector` and `VariableList` (#1794) ## Issue Addressed This comment: https://github.com/sigp/lighthouse/issues/1776#issuecomment-712349841 ## Proposed Changes - Add quoted serde utils for `FixedVector` and `VariableList` - Had to remove the dependency that `ssz_types` has on `serde_utils` to avoid a circular dependency. ## Additional Info Co-authored-by: realbigsean --- Cargo.lock | 1 + consensus/ssz_types/Cargo.toml | 1 + consensus/ssz_types/src/lib.rs | 1 + consensus/ssz_types/src/serde_utils/mod.rs | 2 + .../src/serde_utils/quoted_u64_fixed_vec.rs | 113 ++++++++++++++ .../src/serde_utils/quoted_u64_var_list.rs | 139 ++++++++++++++++++ consensus/ssz_types/src/variable_list.rs | 2 +- consensus/types/src/beacon_state.rs | 2 + 8 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 consensus/ssz_types/src/serde_utils/mod.rs create mode 100644 consensus/ssz_types/src/serde_utils/quoted_u64_fixed_vec.rs create mode 100644 consensus/ssz_types/src/serde_utils/quoted_u64_var_list.rs diff --git a/Cargo.lock b/Cargo.lock index 7b4bb8a8e..bf633c19d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1797,6 +1797,7 @@ dependencies = [ "eth2_ssz", "serde", "serde_derive", + "serde_json", "serde_utils", "tree_hash", "tree_hash_derive", diff --git a/consensus/ssz_types/Cargo.toml b/consensus/ssz_types/Cargo.toml index a0ea0dbcd..39d085b66 100644 --- a/consensus/ssz_types/Cargo.toml +++ b/consensus/ssz_types/Cargo.toml @@ -17,4 +17,5 @@ typenum = "1.12.0" arbitrary = { version = "0.4.6", features = ["derive"], optional = true } [dev-dependencies] +serde_json = "1.0.58" tree_hash_derive = "0.2.0" diff --git a/consensus/ssz_types/src/lib.rs b/consensus/ssz_types/src/lib.rs index f343c4d48..3e181da8c 100644 --- a/consensus/ssz_types/src/lib.rs +++ b/consensus/ssz_types/src/lib.rs @@ -40,6 +40,7 @@ #[macro_use] mod bitfield; mod fixed_vector; +pub mod serde_utils; mod tree_hash; mod variable_list; diff --git a/consensus/ssz_types/src/serde_utils/mod.rs b/consensus/ssz_types/src/serde_utils/mod.rs new file mode 100644 index 000000000..2d315a050 --- /dev/null +++ b/consensus/ssz_types/src/serde_utils/mod.rs @@ -0,0 +1,2 @@ +pub mod quoted_u64_fixed_vec; +pub mod quoted_u64_var_list; diff --git a/consensus/ssz_types/src/serde_utils/quoted_u64_fixed_vec.rs b/consensus/ssz_types/src/serde_utils/quoted_u64_fixed_vec.rs new file mode 100644 index 000000000..8d6ee4f1b --- /dev/null +++ b/consensus/ssz_types/src/serde_utils/quoted_u64_fixed_vec.rs @@ -0,0 +1,113 @@ +//! Formats `FixedVector` using quotes. +//! +//! E.g., `FixedVector::from(vec![0, 1, 2])` serializes as `["0", "1", "2"]`. +//! +//! Quotes can be optional during decoding. If `N` does not equal the length deserialization will fail. + +use crate::serde_utils::quoted_u64_var_list::deserialize_max; +use crate::FixedVector; +use serde::ser::SerializeSeq; +use serde::{Deserializer, Serializer}; +use serde_utils::quoted_u64_vec::QuotedIntWrapper; +use std::marker::PhantomData; +use typenum::Unsigned; + +pub struct QuotedIntFixedVecVisitor { + _phantom: PhantomData, +} + +impl<'a, N> serde::de::Visitor<'a> for QuotedIntFixedVecVisitor +where + N: Unsigned, +{ + type Value = FixedVector; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(formatter, "a list of quoted or unquoted integers") + } + + fn visit_seq(self, seq: A) -> Result + where + A: serde::de::SeqAccess<'a>, + { + let vec = deserialize_max(seq, N::to_usize())?; + let fix: FixedVector = FixedVector::new(vec) + .map_err(|e| serde::de::Error::custom(format!("FixedVector: {:?}", e)))?; + Ok(fix) + } +} + +pub fn serialize(value: &[u64], serializer: S) -> Result +where + S: Serializer, +{ + let mut seq = serializer.serialize_seq(Some(value.len()))?; + for &int in value { + seq.serialize_element(&QuotedIntWrapper { int })?; + } + seq.end() +} + +pub fn deserialize<'de, D, N>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, + N: Unsigned, +{ + deserializer.deserialize_any(QuotedIntFixedVecVisitor { + _phantom: PhantomData, + }) +} + +#[cfg(test)] +mod test { + use super::*; + use serde_derive::{Deserialize, Serialize}; + use typenum::U4; + + #[derive(Debug, Serialize, Deserialize)] + struct Obj { + #[serde(with = "crate::serde_utils::quoted_u64_fixed_vec")] + values: FixedVector, + } + + #[test] + fn quoted_list_success() { + let obj: Obj = serde_json::from_str(r#"{ "values": ["1", "2", "3", "4"] }"#).unwrap(); + let expected: FixedVector = FixedVector::from(vec![1, 2, 3, 4]); + assert_eq!(obj.values, expected); + } + + #[test] + fn unquoted_list_success() { + let obj: Obj = serde_json::from_str(r#"{ "values": [1, 2, 3, 4] }"#).unwrap(); + let expected: FixedVector = FixedVector::from(vec![1, 2, 3, 4]); + assert_eq!(obj.values, expected); + } + + #[test] + fn mixed_list_success() { + let obj: Obj = serde_json::from_str(r#"{ "values": ["1", 2, "3", "4"] }"#).unwrap(); + let expected: FixedVector = FixedVector::from(vec![1, 2, 3, 4]); + assert_eq!(obj.values, expected); + } + + #[test] + fn empty_list_err() { + serde_json::from_str::(r#"{ "values": [] }"#).unwrap_err(); + } + + #[test] + fn short_list_err() { + serde_json::from_str::(r#"{ "values": [1, 2] }"#).unwrap_err(); + } + + #[test] + fn long_list_err() { + serde_json::from_str::(r#"{ "values": [1, 2, 3, 4, 5] }"#).unwrap_err(); + } + + #[test] + fn whole_list_quoted_err() { + serde_json::from_str::(r#"{ "values": "[1, 2, 3, 4]" }"#).unwrap_err(); + } +} diff --git a/consensus/ssz_types/src/serde_utils/quoted_u64_var_list.rs b/consensus/ssz_types/src/serde_utils/quoted_u64_var_list.rs new file mode 100644 index 000000000..b39bf62a9 --- /dev/null +++ b/consensus/ssz_types/src/serde_utils/quoted_u64_var_list.rs @@ -0,0 +1,139 @@ +//! Formats `VariableList` using quotes. +//! +//! E.g., `VariableList::from(vec![0, 1, 2])` serializes as `["0", "1", "2"]`. +//! +//! Quotes can be optional during decoding. If the length of the `Vec` is greater than `N`, deserialization fails. + +use crate::VariableList; +use serde::ser::SerializeSeq; +use serde::{Deserializer, Serializer}; +use serde_utils::quoted_u64_vec::QuotedIntWrapper; +use std::marker::PhantomData; +use typenum::Unsigned; + +pub struct QuotedIntVarListVisitor { + _phantom: PhantomData, +} + +impl<'a, N> serde::de::Visitor<'a> for QuotedIntVarListVisitor +where + N: Unsigned, +{ + type Value = VariableList; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(formatter, "a list of quoted or unquoted integers") + } + + fn visit_seq(self, seq: A) -> Result + where + A: serde::de::SeqAccess<'a>, + { + let vec = deserialize_max(seq, N::to_usize())?; + let list: VariableList = VariableList::new(vec) + .map_err(|e| serde::de::Error::custom(format!("VariableList: {:?}", e)))?; + Ok(list) + } +} + +pub fn serialize(value: &[u64], serializer: S) -> Result +where + S: Serializer, +{ + let mut seq = serializer.serialize_seq(Some(value.len()))?; + for &int in value { + seq.serialize_element(&QuotedIntWrapper { int })?; + } + seq.end() +} + +pub fn deserialize<'de, D, N>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, + N: Unsigned, +{ + deserializer.deserialize_any(QuotedIntVarListVisitor { + _phantom: PhantomData, + }) +} + +/// Returns a `Vec` of no more than `max_items` length. +pub(crate) fn deserialize_max<'a, A>(mut seq: A, max_items: usize) -> Result, A::Error> +where + A: serde::de::SeqAccess<'a>, +{ + let mut vec = vec![]; + let mut counter = 0; + + while let Some(val) = seq.next_element()? { + let val: QuotedIntWrapper = val; + counter += 1; + if counter > max_items { + return Err(serde::de::Error::custom(format!( + "Deserialization failed. Length cannot be greater than {}.", + max_items + ))); + } + + vec.push(val.int); + } + + Ok(vec) +} + +#[cfg(test)] +mod test { + use super::*; + use serde_derive::{Deserialize, Serialize}; + use typenum::U4; + + #[derive(Debug, Serialize, Deserialize)] + struct Obj { + #[serde(with = "crate::serde_utils::quoted_u64_var_list")] + values: VariableList, + } + + #[test] + fn quoted_list_success() { + let obj: Obj = serde_json::from_str(r#"{ "values": ["1", "2", "3", "4"] }"#).unwrap(); + let expected: VariableList = VariableList::from(vec![1, 2, 3, 4]); + assert_eq!(obj.values, expected); + } + + #[test] + fn unquoted_list_success() { + let obj: Obj = serde_json::from_str(r#"{ "values": [1, 2, 3, 4] }"#).unwrap(); + let expected: VariableList = VariableList::from(vec![1, 2, 3, 4]); + assert_eq!(obj.values, expected); + } + + #[test] + fn mixed_list_success() { + let obj: Obj = serde_json::from_str(r#"{ "values": ["1", 2, "3", "4"] }"#).unwrap(); + let expected: VariableList = VariableList::from(vec![1, 2, 3, 4]); + assert_eq!(obj.values, expected); + } + + #[test] + fn empty_list_success() { + let obj: Obj = serde_json::from_str(r#"{ "values": [] }"#).unwrap(); + assert!(obj.values.is_empty()); + } + + #[test] + fn short_list_success() { + let obj: Obj = serde_json::from_str(r#"{ "values": [1, 2] }"#).unwrap(); + let expected: VariableList = VariableList::from(vec![1, 2]); + assert_eq!(obj.values, expected); + } + + #[test] + fn long_list_err() { + serde_json::from_str::(r#"{ "values": [1, 2, 3, 4, 5] }"#).unwrap_err(); + } + + #[test] + fn whole_list_quoted_err() { + serde_json::from_str::(r#"{ "values": "[1, 2, 3, 4]" }"#).unwrap_err(); + } +} diff --git a/consensus/ssz_types/src/variable_list.rs b/consensus/ssz_types/src/variable_list.rs index 7d7ab48bd..774d4a358 100644 --- a/consensus/ssz_types/src/variable_list.rs +++ b/consensus/ssz_types/src/variable_list.rs @@ -320,7 +320,7 @@ mod test { let vec = vec![]; let fixed: VariableList = VariableList::from(vec); - assert_eq!(&fixed[..], &vec![][..]); + assert_eq!(&fixed[..], &[] as &[u64]); } #[test] diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 25cb85ce8..fd2781f1b 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -181,12 +181,14 @@ where #[compare_fields(as_slice)] pub validators: VariableList, #[compare_fields(as_slice)] + #[serde(with = "ssz_types::serde_utils::quoted_u64_var_list")] pub balances: VariableList, // Randomness pub randao_mixes: FixedVector, // Slashings + #[serde(with = "ssz_types::serde_utils::quoted_u64_fixed_vec")] pub slashings: FixedVector, // Attestations