Skip to content

Commit

Permalink
fix serialization of primitives with serde
Browse files Browse the repository at this point in the history
some tests in main.rs of Message Generator are not passing or are
commented. This is because serialization and/or deserialization of Sv2
primitives was broken.

into_static for serde_sv2 primitives
these functions were panicking if the seq field of sequences (Seq<T> or
Sv2Option) were Some(...).

most rust tests for MG are fixed, one is added
  • Loading branch information
lorbax committed Jul 9, 2024
1 parent 5a2b01b commit 89e4360
Show file tree
Hide file tree
Showing 9 changed files with 502 additions and 217 deletions.
2 changes: 1 addition & 1 deletion protocols/Cargo.lock

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

30 changes: 30 additions & 0 deletions protocols/v2/binary-sv2/serde-sv2/src/primitives/sequences/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::{ShortTxId, Signature, B016M, B0255, B064K, U24, U256};
use crate::Error;
use alloc::vec::Vec;
use core::convert::TryInto;
use serde::{de::Visitor, Serialize};

Expand All @@ -22,6 +23,23 @@ struct Seq<'s, T: Clone + Serialize + TryFromBSlice<'s>> {
_a: core::marker::PhantomData<T>,
}

impl<'s, T: Clone + Serialize + TryFromBSlice<'s>> Seq<'s, T> {
pub fn parse(self) -> Result<Vec<T>, ()> {
let mut ret = Vec::with_capacity(self.size as usize);
let elem_len = self.size as usize;
let mut first = 0;
let mut last = elem_len;
while last <= self.data.len() {
let slice = &self.data[first..last];
let elem = T::try_from_slice(slice).map_err(|_| ())?;
ret.push(elem);
first += elem_len;
last += elem_len;
}
Ok(ret)
}
}

struct SeqVisitor<T> {
inner_type_size: u8,
max_len: SeqMaxLen,
Expand Down Expand Up @@ -81,6 +99,18 @@ impl<'a> TryFromBSlice<'a> for bool {
}
}

impl<'a> TryFromBSlice<'a> for u8 {
type Error = Error;

#[inline]
fn try_from_slice(val: &'a [u8]) -> Result<Self, Error> {
if val.len() != 1 {
return Err(Error::InvalidU16Size(val.len()));
}
Ok(u8::from_le_bytes([val[0]]))
}
}

impl<'a> TryFromBSlice<'a> for u16 {
type Error = Error;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ use super::{
};
use crate::primitives::{FixedSize, GetSize};
use alloc::vec::Vec;
use serde::{ser, ser::SerializeTuple, Deserialize, Deserializer, Serialize};
use debug_assert;
use serde::{
ser::{self, SerializeSeq, SerializeTuple},
Deserialize, Deserializer, Serialize,
};

#[derive(Debug, Clone)]
pub struct Sv2Option<'s, T: Serialize + TryFromBSlice<'s> + Clone> {
Expand Down Expand Up @@ -65,13 +69,28 @@ impl<'s, T: Clone + Serialize + TryFromBSlice<'s>> Serialize for Sv2Option<'s, T
seq.end()
}
(None, Some(data)) => {
let tuple = (data.len() as u8, &data[..]);
let mut seq = serializer.serialize_tuple(2)?;
seq.serialize_element(&tuple.0)?;
seq.serialize_element(tuple.1)?;
seq.end()
if serializer.is_human_readable() {
let data_ = data.clone();
let mut seq = serializer.serialize_seq(Some(data_.len()))?;
for item in data_ {
seq.serialize_element(&item)?;
}
seq.end()
} else {
let tuple = (data.len() as u8, &data[..]);
let mut seq = serializer.serialize_tuple(2)?;
seq.serialize_element(&tuple.0)?;
seq.serialize_element(tuple.1)?;
seq.end()
}
}
_ => {
debug_assert!(
false,
"sv2option can never have boh fields of the same type"
);
panic!()
}
_ => panic!(),
}
}
}
Expand Down Expand Up @@ -219,15 +238,30 @@ impl<'a, T: Clone + FixedSize + Serialize + TryFromBSlice<'a>> GetSize for Sv2Op
}
impl<'s> Sv2Option<'s, U256<'s>> {
pub fn into_static(self) -> Sv2Option<'static, U256<'static>> {
if let Some(inner) = self.data {
let inner = inner.clone();
let data = inner.into_iter().map(|i| i.into_static()).collect();
Sv2Option {
seq: None,
data: Some(data),
match (self.data, self.seq) {
(None, Some(seq)) => {
let data = seq.parse().unwrap();
let data = data.into_iter().map(|i| i.into_static()).collect();
Sv2Option {
seq: None,
data: Some(data),
}
}
(Some(inner), None) => {
let inner = inner.clone();
let data = inner.into_iter().map(|i| i.into_static()).collect();
Sv2Option {
seq: None,
data: Some(data),
}
}
_ => {
debug_assert!(
false,
"sv2option can never have boh fields of the same type"
);
panic!()
}
} else {
panic!()
}
}
pub fn inner_as_ref(&self) -> &[&[u8]] {
Expand All @@ -236,13 +270,28 @@ impl<'s> Sv2Option<'s, U256<'s>> {
}
impl<'s> Sv2Option<'s, u32> {
pub fn into_static(self) -> Sv2Option<'static, u32> {
if let Some(inner) = self.data {
Sv2Option {
match (self.data, self.seq) {
(None, Some(seq)) => {
// this is an already valid seq should be safe to call the unwraps.
// also this library shouldn't be used for priduction envs so is ok do thigs like this
// one
let data = seq.parse().unwrap();
Sv2Option {
seq: None,
data: Some(data),
}
}
(Some(inner), None) => Sv2Option {
seq: None,
data: Some(inner),
},
_ => {
debug_assert!(
false,
"sv2option can never have boh fields of the same type"
);
panic!()
}
} else {
panic!()
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ use crate::{
Error,
};
use alloc::vec::Vec;
use serde::{ser, ser::SerializeTuple, Deserialize, Deserializer, Serialize};
use serde::{
ser,
ser::{SerializeSeq, SerializeTuple},
Deserialize, Deserializer, Serialize,
};

#[derive(Debug, Clone)]
pub struct Seq0255<'s, T: Serialize + TryFromBSlice<'s> + Clone> {
Expand Down Expand Up @@ -68,11 +72,20 @@ impl<'s, T: Clone + Serialize + TryFromBSlice<'s>> Serialize for Seq0255<'s, T>
seq.end()
}
(None, Some(data)) => {
let tuple = (data.len() as u8, &data[..]);
let mut seq = serializer.serialize_tuple(2)?;
seq.serialize_element(&tuple.0)?;
seq.serialize_element(tuple.1)?;
seq.end()
if serializer.is_human_readable() {
let data_ = data.clone();
let mut seq = serializer.serialize_seq(Some(data_.len()))?;
for item in data_ {
seq.serialize_element(&item)?;
}
seq.end()
} else {
let tuple = (data.len() as u8, &data[..]);
let mut seq = serializer.serialize_tuple(2)?;
seq.serialize_element(&tuple.0)?;
seq.serialize_element(tuple.1)?;
seq.end()
}
}
_ => panic!(),
}
Expand Down Expand Up @@ -222,15 +235,33 @@ impl<'a, T: Clone + FixedSize + Serialize + TryFromBSlice<'a>> GetSize for Seq02
}
impl<'s> Seq0255<'s, U256<'s>> {
pub fn into_static(self) -> Seq0255<'static, U256<'static>> {
if let Some(inner) = self.data {
let inner = inner.clone();
let data = inner.into_iter().map(|i| i.into_static()).collect();
Seq0255 {
seq: None,
data: Some(data),
match (self.data, self.seq) {
(None, Some(seq)) => {
// this is an already valid seq should be safe to call the unwraps.
// also this library shouldn't be used for priduction envs so is ok do thigs like this
// one
let data = seq.parse().unwrap();
let data = data.into_iter().map(|i| i.into_static()).collect();
Seq0255 {
seq: None,
data: Some(data),
}
}
(Some(inner), None) => {
let inner = inner.clone();
let data = inner.into_iter().map(|i| i.into_static()).collect();
Seq0255 {
seq: None,
data: Some(data),
}
}
_ => {
debug_assert!(
false,
"sequences can never have boh fields of the same type"
);
panic!()
}
} else {
panic!()
}
}
pub fn inner_as_ref(&self) -> &[&[u8]] {
Expand All @@ -239,13 +270,28 @@ impl<'s> Seq0255<'s, U256<'s>> {
}
impl<'s> Seq0255<'s, u32> {
pub fn into_static(self) -> Seq0255<'static, u32> {
if let Some(inner) = self.data {
Seq0255 {
match (self.data, self.seq) {
(None, Some(seq)) => {
// this is an already valid seq should be safe to call the unwraps.
// also this library shouldn't be used for priduction envs so is ok do thigs like this
// one
let data = seq.parse().unwrap();
Seq0255 {
seq: None,
data: Some(data),
}
}
(Some(data), None) => Seq0255 {
seq: None,
data: Some(inner),
data: Some(data),
},
_ => {
debug_assert!(
false,
"sequences can never have boh fields of the same type"
);
panic!()
}
} else {
panic!()
}
}
}
Expand Down
Loading

0 comments on commit 89e4360

Please sign in to comment.