Skip to content

Commit

Permalink
Add serde_utils module with quoted u64 support (#1588)
Browse files Browse the repository at this point in the history
## Proposed Changes

This is an extraction of the quoted int code from #1569, that I've come to rely on for #1544.

It allows us to parse integers from serde strings in YAML, JSON, etc. The main differences from the code in Paul's original PR are:

* Added a submodule that makes quoting mandatory (`require_quotes`).
* Decoding is generic over the type `T` being decoded. You can use `#[serde(with = "serde_utils::quoted_u64::require_quotes")]` on `Epoch` and `Slot` fields (this is what I do in my slashing protection PR).

I've turned on quoting for `Epoch` and `Slot` in this PR, but will leave the other `types` changes to you Paul.

I opted to put everything in the `conseus/serde_utils` module so that BLS can use it without a circular dependency. In future when we want to publish `types` I think we could publish `serde_utils` as `lighthouse_serde_utils` or something. Open to other ideas on this front too.
  • Loading branch information
michaelsproul committed Sep 7, 2020
1 parent 211109b commit 74fa87a
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 3 deletions.
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ members = [
"consensus/ssz_derive",
"consensus/ssz_types",
"consensus/serde_hex",
"consensus/serde_utils",
"consensus/state_processing",
"consensus/swap_or_not_shuffle",
"consensus/tree_hash",
Expand Down
12 changes: 12 additions & 0 deletions consensus/serde_utils/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "serde_utils"
version = "0.1.0"
authors = ["Paul Hauner <[email protected]", "Michael Sproul <[email protected]>"]
edition = "2018"

[dependencies]
serde = { version = "1.0.110", features = ["derive"] }
serde_derive = "1.0.110"

[dev-dependencies]
serde_json = "1.0.52"
2 changes: 2 additions & 0 deletions consensus/serde_utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub mod quoted_u64;
pub mod quoted_u64_vec;
115 changes: 115 additions & 0 deletions consensus/serde_utils/src/quoted_u64.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
use serde::{Deserializer, Serializer};
use serde_derive::{Deserialize, Serialize};
use std::marker::PhantomData;

/// Serde support for deserializing quoted integers.
///
/// Configurable so that quotes are either required or optional.
pub struct QuotedIntVisitor<T> {
require_quotes: bool,
_phantom: PhantomData<T>,
}

impl<'a, T> serde::de::Visitor<'a> for QuotedIntVisitor<T>
where
T: From<u64> + Into<u64> + Copy,
{
type Value = T;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
if self.require_quotes {
write!(formatter, "a quoted integer")
} else {
write!(formatter, "a quoted or unquoted integer")
}
}

fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
s.parse::<u64>()
.map(T::from)
.map_err(serde::de::Error::custom)
}

fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
if self.require_quotes {
Err(serde::de::Error::custom(
"received unquoted integer when quotes are required",
))
} else {
Ok(T::from(v))
}
}
}

/// Wrapper type for requiring quotes on a `u64`-like type.
///
/// Unlike using `serde(with = "quoted_u64::require_quotes")` this is composable, and can be nested
/// inside types like `Option`, `Result` and `Vec`.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Deserialize, Serialize)]
#[serde(transparent)]
pub struct Quoted<T>
where
T: From<u64> + Into<u64> + Copy,
{
#[serde(with = "require_quotes")]
pub value: T,
}

/// Serialize with quotes.
pub fn serialize<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
T: From<u64> + Into<u64> + Copy,
{
let v: u64 = (*value).into();
serializer.serialize_str(&format!("{}", v))
}

/// Deserialize with or without quotes.
pub fn deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
D: Deserializer<'de>,
T: From<u64> + Into<u64> + Copy,
{
deserializer.deserialize_any(QuotedIntVisitor {
require_quotes: false,
_phantom: PhantomData,
})
}

/// Requires quotes when deserializing.
///
/// Usage: `#[serde(with = "quoted_u64::require_quotes")]`.
pub mod require_quotes {
pub use super::serialize;
use super::*;

pub fn deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
D: Deserializer<'de>,
T: From<u64> + Into<u64> + Copy,
{
deserializer.deserialize_any(QuotedIntVisitor {
require_quotes: true,
_phantom: PhantomData,
})
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn require_quotes() {
let x = serde_json::from_str::<Quoted<u64>>("\"8\"").unwrap();
assert_eq!(x.value, 8);
serde_json::from_str::<Quoted<u64>>("8").unwrap_err();
}
}
91 changes: 91 additions & 0 deletions consensus/serde_utils/src/quoted_u64_vec.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
use serde::ser::SerializeSeq;
use serde::{Deserializer, Serializer};
use serde_derive::{Deserialize, Serialize};

#[derive(Serialize, Deserialize)]
#[serde(transparent)]
pub struct QuotedIntWrapper {
#[serde(with = "crate::quoted_u64")]
int: u64,
}

pub struct QuotedIntVecVisitor;
impl<'a> serde::de::Visitor<'a> for QuotedIntVecVisitor {
type Value = Vec<u64>;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(formatter, "a list of quoted or unquoted integers")
}

fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
where
A: serde::de::SeqAccess<'a>,
{
let mut vec = vec![];

while let Some(val) = seq.next_element()? {
let val: QuotedIntWrapper = val;
vec.push(val.int);
}

Ok(vec)
}
}

pub fn serialize<S>(value: &[u64], serializer: S) -> Result<S::Ok, S::Error>
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>(deserializer: D) -> Result<Vec<u64>, D::Error>
where
D: Deserializer<'de>,
{
deserializer.deserialize_any(QuotedIntVecVisitor)
}

#[cfg(test)]
mod test {
use super::*;

#[derive(Debug, Serialize, Deserialize)]
struct Obj {
#[serde(with = "crate::quoted_u64_vec")]
values: Vec<u64>,
}

#[test]
fn quoted_list_success() {
let obj: Obj = serde_json::from_str(r#"{ "values": ["1", "2", "3", "4"] }"#).unwrap();
assert_eq!(obj.values, vec![1, 2, 3, 4]);
}

#[test]
fn unquoted_list_success() {
let obj: Obj = serde_json::from_str(r#"{ "values": [1, 2, 3, 4] }"#).unwrap();
assert_eq!(obj.values, vec![1, 2, 3, 4]);
}

#[test]
fn mixed_list_success() {
let obj: Obj = serde_json::from_str(r#"{ "values": ["1", 2, "3", "4"] }"#).unwrap();
assert_eq!(obj.values, vec![1, 2, 3, 4]);
}

#[test]
fn empty_list_success() {
let obj: Obj = serde_json::from_str(r#"{ "values": [] }"#).unwrap();
assert!(obj.values.is_empty());
}

#[test]
fn whole_list_quoted_err() {
serde_json::from_str::<Obj>(r#"{ "values": "[1, 2, 3, 4]" }"#).unwrap_err();
}
}
1 change: 1 addition & 0 deletions consensus/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ rand = "0.7.3"
safe_arith = { path = "../safe_arith" }
serde = "1.0.110"
serde_derive = "1.0.110"
serde_utils = { path = "../serde_utils" }
slog = "2.5.2"
eth2_ssz = "0.1.2"
eth2_ssz_derive = "0.1.0"
Expand Down
5 changes: 3 additions & 2 deletions consensus/types/src/slot_epoch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Rem, Sub, SubAssi
#[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))]
#[derive(Eq, Clone, Copy, Default, Serialize, Deserialize)]
#[serde(transparent)]
pub struct Slot(u64);
pub struct Slot(#[serde(with = "serde_utils::quoted_u64")] u64);

#[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))]
#[derive(Eq, Clone, Copy, Default, Serialize, Deserialize)]
pub struct Epoch(u64);
#[serde(transparent)]
pub struct Epoch(#[serde(with = "serde_utils::quoted_u64")] u64);

impl_common!(Slot);
impl_common!(Epoch);
Expand Down
2 changes: 1 addition & 1 deletion consensus/types/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
mod serde_utils;

pub use serde_utils::*;
pub use self::serde_utils::*;

0 comments on commit 74fa87a

Please sign in to comment.