From 27234865ec8d6a653c0a7eb5adfc02e139de92d0 Mon Sep 17 00:00:00 2001 From: Wolfgang Welz Date: Wed, 2 Aug 2023 20:10:22 +0200 Subject: [PATCH 1/4] fix bincode serialization --- Cargo.toml | 1 + crates/primitives/Cargo.toml | 1 + crates/primitives/src/bits/serde.rs | 107 ++++++++++++++++++--------- crates/primitives/src/bytes/serde.rs | 88 +++++++++++++++------- 4 files changed, 138 insertions(+), 59 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index de33bc15dc..ebc58ee4c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ alloy-rlp = { version = "0.3.0", default-features = false } alloy-rlp-derive = { version = "0.3.0", default-features = false } arbitrary = "1.3" arrayvec = { version = "0.7", default-features = false } +bincode = "1.3" bytes = { version = "1.4", default-features = false } criterion = "0.5" derive_arbitrary = "1.3" diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index cfda541ed9..190df6198b 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -45,6 +45,7 @@ proptest = { workspace = true, optional = true } proptest-derive = { workspace = true, optional = true } [dev-dependencies] +bincode.workspace = true serde_json.workspace = true serde = { workspace = true, features = ["derive"] } diff --git a/crates/primitives/src/bits/serde.rs b/crates/primitives/src/bits/serde.rs index 4094a79890..446870755c 100644 --- a/crates/primitives/src/bits/serde.rs +++ b/crates/primitives/src/bits/serde.rs @@ -18,58 +18,82 @@ impl Serialize for FixedBytes { impl<'de, const N: usize> Deserialize<'de> for FixedBytes { fn deserialize>(deserializer: D) -> Result { - struct FixedVisitor; + if deserializer.is_human_readable() { + struct FixedVisitor; - impl<'de, const N: usize> Visitor<'de> for FixedVisitor { - type Value = FixedBytes; + impl<'de, const N: usize> Visitor<'de> for FixedVisitor { + type Value = FixedBytes; - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - formatter, + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + formatter, "{} bytes, represented as a hex string of length {}, an array of u8, or raw bytes", - N, - N * 2 - ) - } - - fn visit_bytes(self, v: &[u8]) -> Result { - <[u8; N]>::try_from(v) - .map(FixedBytes) - .map_err(de::Error::custom) - } + N, + N * 2 + ) + } - fn visit_seq>(self, mut seq: A) -> Result { - let mut bytes = [0u8; N]; + fn visit_bytes(self, v: &[u8]) -> Result { + <[u8; N]>::try_from(v) + .map(FixedBytes) + .map_err(de::Error::custom) + } - bytes.iter_mut().enumerate().try_for_each(|(i, b)| { - *b = seq.next_element()?.ok_or_else(|| { - de::Error::invalid_length(i, &format!("exactly {} bytes", N).as_str()) + fn visit_seq>( + self, + mut seq: A, + ) -> Result { + let mut bytes = [0u8; N]; + + bytes.iter_mut().enumerate().try_for_each(|(i, b)| { + *b = seq.next_element()?.ok_or_else(|| { + de::Error::invalid_length(i, &format!("exactly {} bytes", N).as_str()) + })?; + Ok(()) })?; - Ok(()) - })?; - - if let Ok(Some(_)) = seq.next_element::() { - return Err(de::Error::invalid_length( - N + 1, - &format!("exactly {} bytes", N).as_str(), - )) + + if let Ok(Some(_)) = seq.next_element::() { + return Err(de::Error::invalid_length( + N + 1, + &format!("exactly {} bytes", N).as_str(), + )) + } + + Ok(FixedBytes(bytes)) } - Ok(FixedBytes(bytes)) + fn visit_str(self, v: &str) -> Result { + as hex::FromHex>::from_hex(v).map_err(de::Error::custom) + } } - fn visit_str(self, v: &str) -> Result { - as hex::FromHex>::from_hex(v).map_err(de::Error::custom) + deserializer.deserialize_any(FixedVisitor::) + } else { + struct FixedVisitor; + + impl<'de, const N: usize> Visitor<'de> for FixedVisitor { + type Value = FixedBytes; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("byte array") + } + + fn visit_bytes(self, v: &[u8]) -> Result { + <[u8; N]>::try_from(v) + .map(FixedBytes) + .map_err(de::Error::custom) + } } - } - deserializer.deserialize_any(FixedVisitor::) + deserializer.deserialize_bytes(FixedVisitor::) + } } } #[cfg(test)] mod tests { use super::*; + use bincode as _; use serde::Deserialize; #[derive(Debug, Deserialize)] struct TestCase { @@ -82,6 +106,13 @@ mod tests { let ser = serde_json::to_string(&bytes).unwrap(); assert_eq!(ser, "\"0x000000000123456789abcdef\""); assert_eq!(serde_json::from_str::>(&ser).unwrap(), bytes); + + let val = serde_json::to_value(&bytes).unwrap(); + assert_eq!(val, serde_json::json! {"0x000000000123456789abcdef"}); + assert_eq!( + serde_json::from_value::>(val).unwrap(), + bytes + ); } #[test] @@ -101,4 +132,12 @@ mod tests { ) .contains("invalid length 5, expected exactly 4 bytes"),); } + + #[test] + fn test_bincode_roundtrip() { + let bytes = FixedBytes([0, 0, 0, 0, 1, 35, 69, 103, 137, 171, 205, 239]); + + let bin = bincode::serialize(&bytes).unwrap(); + assert_eq!(bincode::deserialize::>(&bin).unwrap(), bytes); + } } diff --git a/crates/primitives/src/bytes/serde.rs b/crates/primitives/src/bytes/serde.rs index 33c510e9db..90834393f6 100644 --- a/crates/primitives/src/bytes/serde.rs +++ b/crates/primitives/src/bytes/serde.rs @@ -17,49 +17,75 @@ impl serde::Serialize for Bytes { impl<'de> serde::Deserialize<'de> for Bytes { #[inline] fn deserialize>(deserializer: D) -> Result { - struct BytesVisitor; + if deserializer.is_human_readable() { + struct BytesVisitor; - impl<'de> Visitor<'de> for BytesVisitor { - type Value = Bytes; + impl<'de> Visitor<'de> for BytesVisitor { + type Value = Bytes; - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("a variable number of bytes represented as a hex string, an array of u8, or raw bytes") - } + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a variable number of bytes represented as a hex string, an array of u8, or raw bytes") + } - fn visit_bytes(self, v: &[u8]) -> Result { - Ok(Bytes::from(v.to_vec())) - } + fn visit_bytes(self, v: &[u8]) -> Result { + Ok(Bytes::from(v.to_vec())) + } - fn visit_byte_buf(self, v: Vec) -> Result { - Ok(Bytes::from(v)) - } + fn visit_byte_buf(self, v: Vec) -> Result { + Ok(Bytes::from(v)) + } - fn visit_seq>(self, mut seq: A) -> Result { - let mut bytes = Vec::with_capacity(seq.size_hint().unwrap_or(0)); + fn visit_seq>( + self, + mut seq: A, + ) -> Result { + let mut bytes = Vec::with_capacity(seq.size_hint().unwrap_or(0)); - while let Some(byte) = seq.next_element()? { - bytes.push(byte); + while let Some(byte) = seq.next_element()? { + bytes.push(byte); + } + + Ok(Bytes::from(bytes)) } - Ok(Bytes::from(bytes)) + fn visit_str(self, v: &str) -> Result { + hex::decode(v) + .map_err(|_| { + de::Error::invalid_value(de::Unexpected::Str(v), &"a valid hex string") + }) + .map(From::from) + } } - fn visit_str(self, v: &str) -> Result { - hex::decode(v) - .map_err(|_| { - de::Error::invalid_value(de::Unexpected::Str(v), &"a valid hex string") - }) - .map(From::from) + deserializer.deserialize_any(BytesVisitor) + } else { + struct BytesVisitor; + + impl<'de> Visitor<'de> for BytesVisitor { + type Value = Bytes; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("byte array") + } + + fn visit_bytes(self, v: &[u8]) -> Result { + Ok(Bytes::from(v.to_vec())) + } + + fn visit_byte_buf(self, v: Vec) -> Result { + Ok(Bytes::from(v)) + } } - } - deserializer.deserialize_any(BytesVisitor) + deserializer.deserialize_byte_buf(BytesVisitor) + } } } #[cfg(test)] mod tests { use super::*; + use bincode as _; use serde::Deserialize; #[derive(Debug, Deserialize)] struct TestCase { @@ -72,6 +98,10 @@ mod tests { let ser = serde_json::to_string(&bytes).unwrap(); assert_eq!(ser, "\"0x0123456789abcdef\""); assert_eq!(serde_json::from_str::(&ser).unwrap(), bytes); + + let val = serde_json::to_value(&bytes).unwrap(); + assert_eq!(val, serde_json::json! {"0x0123456789abcdef"}); + assert_eq!(serde_json::from_value::(val).unwrap(), bytes); } #[test] @@ -85,4 +115,12 @@ mod tests { Bytes::from(Vec::from([0, 1, 2, 3, 4])) ); } + + #[test] + fn test_bincode_roundtrip() { + let bytes = Bytes::from_static(&[1, 35, 69, 103, 137, 171, 205, 239]); + + let bin = bincode::serialize(&bytes).unwrap(); + assert_eq!(bincode::deserialize::(&bin).unwrap(), bytes); + } } From d640422d726dd0c1fc965a241418fe1781cdfd21 Mon Sep 17 00:00:00 2001 From: Wolfgang Welz Date: Wed, 2 Aug 2023 20:33:30 +0200 Subject: [PATCH 2/4] fmt --- crates/primitives/src/bytes/serde.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives/src/bytes/serde.rs b/crates/primitives/src/bytes/serde.rs index 90834393f6..d88a566d76 100644 --- a/crates/primitives/src/bytes/serde.rs +++ b/crates/primitives/src/bytes/serde.rs @@ -60,7 +60,7 @@ impl<'de> serde::Deserialize<'de> for Bytes { deserializer.deserialize_any(BytesVisitor) } else { struct BytesVisitor; - + impl<'de> Visitor<'de> for BytesVisitor { type Value = Bytes; From 549b63208852053a6058a741c2c1c33707e27749 Mon Sep 17 00:00:00 2001 From: Wolfgang Welz Date: Wed, 2 Aug 2023 21:32:09 +0200 Subject: [PATCH 3/4] re-use visitor --- crates/primitives/src/bits/serde.rs | 83 +++++++++++----------------- crates/primitives/src/bytes/serde.rs | 75 +++++++++---------------- 2 files changed, 59 insertions(+), 99 deletions(-) diff --git a/crates/primitives/src/bits/serde.rs b/crates/primitives/src/bits/serde.rs index 446870755c..e6c4f7eec2 100644 --- a/crates/primitives/src/bits/serde.rs +++ b/crates/primitives/src/bits/serde.rs @@ -18,73 +18,54 @@ impl Serialize for FixedBytes { impl<'de, const N: usize> Deserialize<'de> for FixedBytes { fn deserialize>(deserializer: D) -> Result { - if deserializer.is_human_readable() { - struct FixedVisitor; + struct FixedVisitor; - impl<'de, const N: usize> Visitor<'de> for FixedVisitor { - type Value = FixedBytes; + impl<'de, const N: usize> Visitor<'de> for FixedVisitor { + type Value = FixedBytes; - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( formatter, "{} bytes, represented as a hex string of length {}, an array of u8, or raw bytes", N, N * 2 ) - } - - fn visit_bytes(self, v: &[u8]) -> Result { - <[u8; N]>::try_from(v) - .map(FixedBytes) - .map_err(de::Error::custom) - } + } - fn visit_seq>( - self, - mut seq: A, - ) -> Result { - let mut bytes = [0u8; N]; - - bytes.iter_mut().enumerate().try_for_each(|(i, b)| { - *b = seq.next_element()?.ok_or_else(|| { - de::Error::invalid_length(i, &format!("exactly {} bytes", N).as_str()) - })?; - Ok(()) - })?; + fn visit_bytes(self, v: &[u8]) -> Result { + <[u8; N]>::try_from(v) + .map(FixedBytes) + .map_err(de::Error::custom) + } - if let Ok(Some(_)) = seq.next_element::() { - return Err(de::Error::invalid_length( - N + 1, - &format!("exactly {} bytes", N).as_str(), - )) - } + fn visit_seq>(self, mut seq: A) -> Result { + let mut bytes = [0u8; N]; - Ok(FixedBytes(bytes)) + bytes.iter_mut().enumerate().try_for_each(|(i, b)| { + *b = seq.next_element()?.ok_or_else(|| { + de::Error::invalid_length(i, &format!("exactly {} bytes", N).as_str()) + })?; + Ok(()) + })?; + + if let Ok(Some(_)) = seq.next_element::() { + return Err(de::Error::invalid_length( + N + 1, + &format!("exactly {} bytes", N).as_str(), + )) } - fn visit_str(self, v: &str) -> Result { - as hex::FromHex>::from_hex(v).map_err(de::Error::custom) - } + Ok(FixedBytes(bytes)) } - deserializer.deserialize_any(FixedVisitor::) - } else { - struct FixedVisitor; - - impl<'de, const N: usize> Visitor<'de> for FixedVisitor { - type Value = FixedBytes; - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("byte array") - } - - fn visit_bytes(self, v: &[u8]) -> Result { - <[u8; N]>::try_from(v) - .map(FixedBytes) - .map_err(de::Error::custom) - } + fn visit_str(self, v: &str) -> Result { + as hex::FromHex>::from_hex(v).map_err(de::Error::custom) } + } + if deserializer.is_human_readable() { + deserializer.deserialize_any(FixedVisitor::) + } else { deserializer.deserialize_bytes(FixedVisitor::) } } diff --git a/crates/primitives/src/bytes/serde.rs b/crates/primitives/src/bytes/serde.rs index d88a566d76..0fbe8e14bd 100644 --- a/crates/primitives/src/bytes/serde.rs +++ b/crates/primitives/src/bytes/serde.rs @@ -17,66 +17,45 @@ impl serde::Serialize for Bytes { impl<'de> serde::Deserialize<'de> for Bytes { #[inline] fn deserialize>(deserializer: D) -> Result { - if deserializer.is_human_readable() { - struct BytesVisitor; - - impl<'de> Visitor<'de> for BytesVisitor { - type Value = Bytes; - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("a variable number of bytes represented as a hex string, an array of u8, or raw bytes") - } + struct BytesVisitor; - fn visit_bytes(self, v: &[u8]) -> Result { - Ok(Bytes::from(v.to_vec())) - } + impl<'de> Visitor<'de> for BytesVisitor { + type Value = Bytes; - fn visit_byte_buf(self, v: Vec) -> Result { - Ok(Bytes::from(v)) - } - - fn visit_seq>( - self, - mut seq: A, - ) -> Result { - let mut bytes = Vec::with_capacity(seq.size_hint().unwrap_or(0)); - - while let Some(byte) = seq.next_element()? { - bytes.push(byte); - } - - Ok(Bytes::from(bytes)) - } + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a variable number of bytes represented as a hex string, an array of u8, or raw bytes") + } - fn visit_str(self, v: &str) -> Result { - hex::decode(v) - .map_err(|_| { - de::Error::invalid_value(de::Unexpected::Str(v), &"a valid hex string") - }) - .map(From::from) - } + fn visit_bytes(self, v: &[u8]) -> Result { + Ok(Bytes::from(v.to_vec())) } - deserializer.deserialize_any(BytesVisitor) - } else { - struct BytesVisitor; + fn visit_byte_buf(self, v: Vec) -> Result { + Ok(Bytes::from(v)) + } - impl<'de> Visitor<'de> for BytesVisitor { - type Value = Bytes; + fn visit_seq>(self, mut seq: A) -> Result { + let mut bytes = Vec::with_capacity(seq.size_hint().unwrap_or(0)); - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("byte array") + while let Some(byte) = seq.next_element()? { + bytes.push(byte); } - fn visit_bytes(self, v: &[u8]) -> Result { - Ok(Bytes::from(v.to_vec())) - } + Ok(Bytes::from(bytes)) + } - fn visit_byte_buf(self, v: Vec) -> Result { - Ok(Bytes::from(v)) - } + fn visit_str(self, v: &str) -> Result { + hex::decode(v) + .map_err(|_| { + de::Error::invalid_value(de::Unexpected::Str(v), &"a valid hex string") + }) + .map(From::from) } + } + if deserializer.is_human_readable() { + deserializer.deserialize_any(BytesVisitor) + } else { deserializer.deserialize_byte_buf(BytesVisitor) } } From 53d2d88113065c0fd02783d7c02c6b99dee14aef Mon Sep 17 00:00:00 2001 From: Wolfgang Welz Date: Wed, 2 Aug 2023 21:35:11 +0200 Subject: [PATCH 4/4] fix macro indention --- crates/primitives/src/bits/serde.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/primitives/src/bits/serde.rs b/crates/primitives/src/bits/serde.rs index e6c4f7eec2..26af5c59e5 100644 --- a/crates/primitives/src/bits/serde.rs +++ b/crates/primitives/src/bits/serde.rs @@ -25,11 +25,11 @@ impl<'de, const N: usize> Deserialize<'de> for FixedBytes { fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { write!( - formatter, + formatter, "{} bytes, represented as a hex string of length {}, an array of u8, or raw bytes", - N, - N * 2 - ) + N, + N * 2 + ) } fn visit_bytes(self, v: &[u8]) -> Result {