From cbbc1ef20176f40aa5821dd94ba4458e8f6871b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?dj8yf0=CE=BCl?= Date: Fri, 15 Sep 2023 16:03:11 +0300 Subject: [PATCH] feat!: add `Definition::Primitive` --- borsh/src/schema.rs | 115 ++++++++++----- borsh/src/schema/container_ext.rs | 6 +- borsh/src/schema/container_ext/max_size.rs | 136 +++++++++++------- borsh/src/schema/container_ext/validate.rs | 42 +++--- ...chema_enums__complex_enum_with_schema.snap | 20 +-- borsh/tests/test_schema_enums.rs | 5 +- borsh/tests/test_schema_nested.rs | 4 +- borsh/tests/test_schema_primitives.rs | 34 ++++- borsh/tests/test_schema_recursive.rs | 4 +- borsh/tests/test_schema_structs.rs | 29 ++-- borsh/tests/test_schema_tuple.rs | 3 +- borsh/tests/test_schema_validate.rs | 2 +- borsh/tests/test_schema_with.rs | 6 +- 13 files changed, 270 insertions(+), 136 deletions(-) diff --git a/borsh/src/schema.rs b/borsh/src/schema.rs index b7397fdc1..9ac1c6655 100644 --- a/borsh/src/schema.rs +++ b/borsh/src/schema.rs @@ -51,6 +51,8 @@ pub type FieldName = String; /// [varint]: https://en.wikipedia.org/wiki/Variable-length_quantity#Variants #[derive(Clone, PartialEq, Eq, Debug, BorshSerialize, BorshDeserialize, BorshSchemaMacro)] pub enum Definition { + /// A fixed-size type, which is considered undivisible + Primitive(u8), /// A fixed-size array with the length known at the compile time and the same-type elements. Array { length: u32, elements: Declaration }, /// A sequence of elements of length known at the run time and the same-type elements. @@ -261,11 +263,14 @@ where } macro_rules! impl_for_renamed_primitives { - ($($type: ty : $name: ident)+) => { + ($($ty: ty : $name: ident => $size: expr);+) => { $( - impl BorshSchema for $type { + impl BorshSchema for $ty { #[inline] - fn add_definitions_recursively(_definitions: &mut BTreeMap) {} + fn add_definitions_recursively(definitions: &mut BTreeMap) { + let definition = Definition::Primitive($size); + add_definition(Self::declaration(), definition, definitions); + } #[inline] fn declaration() -> Declaration { stringify!($name).into() } } @@ -274,31 +279,47 @@ macro_rules! impl_for_renamed_primitives { } macro_rules! impl_for_primitives { - ($($type: ident)+) => { - impl_for_renamed_primitives!{$($type : $type)+} + ($($ty: ident => $size: expr);+) => { + impl_for_renamed_primitives!{$($ty : $ty => $size);+} }; } -impl_for_primitives!(bool f32 f64 i8 i16 i32 i64 i128 u8 u16 u32 u64 u128); -impl_for_renamed_primitives!(String: string); -impl_for_renamed_primitives!(str: string); -impl_for_renamed_primitives!(isize: i64); -impl_for_renamed_primitives!(usize: u64); - -impl_for_renamed_primitives!(core::num::NonZeroI8: nonzero_i8); -impl_for_renamed_primitives!(core::num::NonZeroI16: nonzero_i16); -impl_for_renamed_primitives!(core::num::NonZeroI32: nonzero_i32); -impl_for_renamed_primitives!(core::num::NonZeroI64: nonzero_i64); -impl_for_renamed_primitives!(core::num::NonZeroI128: nonzero_i128); -impl_for_renamed_primitives!(core::num::NonZeroU8: nonzero_u8); -impl_for_renamed_primitives!(core::num::NonZeroU16: nonzero_u16); -impl_for_renamed_primitives!(core::num::NonZeroU32: nonzero_u32); -impl_for_renamed_primitives!(core::num::NonZeroU64: nonzero_u64); -impl_for_renamed_primitives!(core::num::NonZeroU128: nonzero_u128); +impl_for_primitives!(bool => 1; f32 => 4; f64 => 8; i8 => 1; i16 => 2; i32 => 4; i64 => 8; i128 => 16); +impl_for_primitives!(u8 => 1; u16 => 2; u32 => 4; u64 => 8; u128 => 16); +impl_for_renamed_primitives!(isize: i64 => 8); +impl_for_renamed_primitives!(usize: u64 => 8); + +impl_for_renamed_primitives!(core::num::NonZeroI8: nonzero_i8 => 1); +impl_for_renamed_primitives!(core::num::NonZeroI16: nonzero_i16 => 2); +impl_for_renamed_primitives!(core::num::NonZeroI32: nonzero_i32 => 4); +impl_for_renamed_primitives!(core::num::NonZeroI64: nonzero_i64 => 8); +impl_for_renamed_primitives!(core::num::NonZeroI128: nonzero_i128 => 16); +impl_for_renamed_primitives!(core::num::NonZeroU8: nonzero_u8 => 1); +impl_for_renamed_primitives!(core::num::NonZeroU16: nonzero_u16 => 2); +impl_for_renamed_primitives!(core::num::NonZeroU32: nonzero_u32 => 4); +impl_for_renamed_primitives!(core::num::NonZeroU64: nonzero_u64 => 8); +impl_for_renamed_primitives!(core::num::NonZeroU128: nonzero_u128 => 16); // see 12 lines above -impl_for_renamed_primitives!(core::num::NonZeroUsize: nonzero_u64); +impl_for_renamed_primitives!(core::num::NonZeroUsize: nonzero_u64 => 8); -impl_for_renamed_primitives!((): nil); +impl_for_renamed_primitives!((): nil => 0); + +impl BorshSchema for String { + #[inline] + fn add_definitions_recursively(_definitions: &mut BTreeMap) {} + #[inline] + fn declaration() -> Declaration { + "string".into() + } +} +impl BorshSchema for str { + #[inline] + fn add_definitions_recursively(_definitions: &mut BTreeMap) {} + #[inline] + fn declaration() -> Declaration { + "string".into() + } +} impl BorshSchema for core::ops::RangeFull { #[inline] @@ -369,6 +390,7 @@ where }; add_definition(Self::declaration(), definition, definitions); T::add_definitions_recursively(definitions); + <()>::add_definitions_recursively(definitions); } fn declaration() -> Declaration { @@ -391,6 +413,7 @@ where }; add_definition(Self::declaration(), definition, definitions); T::add_definitions_recursively(definitions); + E::add_definitions_recursively(definitions); } fn declaration() -> Declaration { @@ -520,7 +543,9 @@ where // Because it's a zero-sized marker, its type parameter doesn't need to be // included in the schema and so it's not bound to `BorshSchema` impl BorshSchema for PhantomData { - fn add_definitions_recursively(_definitions: &mut BTreeMap) {} + fn add_definitions_recursively(definitions: &mut BTreeMap) { + <()>::add_definitions_recursively(definitions); + } fn declaration() -> Declaration { <()>::declaration() @@ -610,7 +635,9 @@ mod tests { ("None".to_string(), "nil".to_string()), ("Some".to_string(), "u64".to_string()), ] - } + }, + "u64" => Definition::Primitive(8), + "nil" => Definition::Primitive(0) }, actual_defs ); @@ -637,7 +664,9 @@ mod tests { ("None".to_string(), "nil".to_string()), ("Some".to_string(), "Option".to_string()), ] - } + }, + "u64" => Definition::Primitive(8), + "nil" => Definition::Primitive(0) }, actual_defs ); @@ -651,7 +680,8 @@ mod tests { assert_eq!("Vec", actual_name); assert_eq!( map! { - "Vec" => Definition::Sequence { elements: "u64".to_string() } + "Vec" => Definition::Sequence { elements: "u64".to_string() }, + "u64" => Definition::Primitive(8) }, actual_defs ); @@ -666,7 +696,8 @@ mod tests { assert_eq!( map! { "Vec" => Definition::Sequence { elements: "u64".to_string() }, - "Vec>" => Definition::Sequence { elements: "Vec".to_string() } + "Vec>" => Definition::Sequence { elements: "Vec".to_string() }, + "u64" => Definition::Primitive(8) }, actual_defs ); @@ -686,7 +717,9 @@ mod tests { "nonzero_u16".to_string(), "string".to_string() ] - } + }, + "u64" => Definition::Primitive(8), + "nonzero_u16" => Definition::Primitive(2) }, actual_defs ); @@ -705,7 +738,10 @@ mod tests { "Tuple".to_string(), "string".to_string(), ]}, - "Tuple" => Definition::Tuple { elements: vec![ "u8".to_string(), "bool".to_string()]} + "Tuple" => Definition::Tuple { elements: vec![ "u8".to_string(), "bool".to_string()]}, + "u64" => Definition::Primitive(8), + "u8" => Definition::Primitive(1), + "bool" => Definition::Primitive(1) }, actual_defs ); @@ -721,7 +757,8 @@ mod tests { assert_eq!( map! { "HashMap" => Definition::Sequence { elements: "Tuple".to_string()} , - "Tuple" => Definition::Tuple { elements: vec![ "u64".to_string(), "string".to_string()]} + "Tuple" => Definition::Tuple { elements: vec![ "u64".to_string(), "string".to_string()]}, + "u64" => Definition::Primitive(8) }, actual_defs ); @@ -751,7 +788,8 @@ mod tests { assert_eq!( map! { "BTreeMap" => Definition::Sequence { elements: "Tuple".to_string()} , - "Tuple" => Definition::Tuple { elements: vec![ "u64".to_string(), "string".to_string()]} + "Tuple" => Definition::Tuple { elements: vec![ "u64".to_string(), "string".to_string()]}, + "u64" => Definition::Primitive(8) }, actual_defs ); @@ -778,7 +816,9 @@ mod tests { <[u64; 32]>::add_definitions_recursively(&mut actual_defs); assert_eq!("Array", actual_name); assert_eq!( - map! {"Array" => Definition::Array { length: 32, elements: "u64".to_string()}}, + map! {"Array" => Definition::Array { length: 32, elements: "u64".to_string()}, + "u64" => Definition::Primitive(8) + }, actual_defs ); } @@ -796,7 +836,8 @@ mod tests { "Array, 10>" => Definition::Array { length: 10, elements: "Array".to_string() }, "Array, 10>, 32>" => - Definition::Array { length: 32, elements: "Array, 10>".to_string() } + Definition::Array { length: 32, elements: "Array, 10>".to_string() }, + "u64" => Definition::Primitive(8) }, actual_defs ); @@ -854,7 +895,8 @@ mod tests { ("start".into(), "u64".into()), ("end".into(), "u64".into()), ]) - } + }, + "u64" => Definition::Primitive(8) }, actual_defs ); @@ -869,7 +911,8 @@ mod tests { fields: Fields::NamedFields(vec![ ("end".into(), "u64".into()), ]) - } + }, + "u64" => Definition::Primitive(8) }, actual_defs ); diff --git a/borsh/src/schema/container_ext.rs b/borsh/src/schema/container_ext.rs index ca6d4d085..ba1f95982 100644 --- a/borsh/src/schema/container_ext.rs +++ b/borsh/src/schema/container_ext.rs @@ -1,8 +1,8 @@ use super::{BorshSchemaContainer, Declaration, Definition, Fields}; -use max_size::is_zero_size; -pub use max_size::SchemaMaxSerializedSizeError; -pub use validate::SchemaContainerValidateError; +pub use max_size::Error as SchemaMaxSerializedSizeError; +use max_size::{is_zero_size, ZeroSizeError}; +pub use validate::Error as SchemaContainerValidateError; mod max_size; mod validate; diff --git a/borsh/src/schema/container_ext/max_size.rs b/borsh/src/schema/container_ext/max_size.rs index 040a0e24f..ec574b39c 100644 --- a/borsh/src/schema/container_ext/max_size.rs +++ b/borsh/src/schema/container_ext/max_size.rs @@ -10,9 +10,6 @@ const ONE: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(1) }; impl BorshSchemaContainer { /// Returns the largest possible size of a serialised object based solely on its type. /// - /// Zero-sized types should follow the convention of either providing a [Definition] or - /// specifying `"nil"` as their [Declaration] for this method to work correctly. - /// /// The function has limitations which may lead it to overestimate the size. /// For example, hypothetical `IPv4Packet` would be encoded as at most ~64 KiB. /// However, if it uses sequence schema, this function will claim that the @@ -43,7 +40,7 @@ impl BorshSchemaContainer { /// assert_eq!(Err(borsh::schema::SchemaMaxSerializedSizeError::Overflow), /// schema.max_serialized_size()); /// ``` - pub fn max_serialized_size(&self) -> Result { + pub fn max_serialized_size(&self) -> Result { let mut stack = Vec::new(); max_serialized_size_impl(ONE, self.declaration(), self, &mut stack) } @@ -51,7 +48,7 @@ impl BorshSchemaContainer { /// Possible error when calculating theoretical maximum size of encoded type `T`. #[derive(Clone, PartialEq, Eq, Debug)] -pub enum SchemaMaxSerializedSizeError { +pub enum Error { /// The theoretical maximum size of the encoded value overflows `usize`. /// /// This may happen for nested dynamically-sized types such as @@ -74,22 +71,19 @@ fn max_serialized_size_impl<'a>( declaration: &'a str, schema: &'a BorshSchemaContainer, stack: &mut Vec<&'a str>, -) -> Result { +) -> Result { use core::convert::TryFrom; /// Maximum number of elements in a vector or length of a string which can /// be serialised. const MAX_LEN: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(u32::MAX as usize) }; - fn add(x: usize, y: usize) -> Result { - x.checked_add(y) - .ok_or(SchemaMaxSerializedSizeError::Overflow) + fn add(x: usize, y: usize) -> Result { + x.checked_add(y).ok_or(Error::Overflow) } - fn mul(x: NonZeroUsize, y: usize) -> Result { - x.get() - .checked_mul(y) - .ok_or(SchemaMaxSerializedSizeError::Overflow) + fn mul(x: NonZeroUsize, y: usize) -> Result { + x.get().checked_mul(y).ok_or(Error::Overflow) } /// Calculates max serialised size of a tuple with given members. @@ -98,7 +92,7 @@ fn max_serialized_size_impl<'a>( elements: impl core::iter::IntoIterator, schema: &'a BorshSchemaContainer, stack: &mut Vec<&'a str>, - ) -> Result { + ) -> Result { let mut sum: usize = 0; for el in elements { sum = add(sum, max_serialized_size_impl(ONE, el, schema, stack)?)?; @@ -107,11 +101,20 @@ fn max_serialized_size_impl<'a>( } if stack.iter().any(|dec| *dec == declaration) { - return Err(SchemaMaxSerializedSizeError::Recursive); + return Err(Error::Recursive); } stack.push(declaration); let res = match schema.get_definition(declaration).ok_or(declaration) { + Ok(Definition::Primitive(size)) => match size { + 0 => Ok(0), + size => { + let count_sizes = usize::try_from(*size) + .ok() + .and_then(|size| size.checked_mul(count.get())); + count_sizes.ok_or(Error::Overflow) + } + }, Ok(Definition::Array { length, elements }) => { // Aggregate `count` and `length` to a single number. If this // overflows, check if array’s element is zero-sized. @@ -124,7 +127,7 @@ fn max_serialized_size_impl<'a>( return Ok(0); } None => { - return Err(SchemaMaxSerializedSizeError::Overflow); + return Err(Error::Overflow); } }; let count_lengths = NonZeroUsize::new(count_lengths); @@ -153,7 +156,7 @@ fn max_serialized_size_impl<'a>( max = max.max(sz); } max.checked_add(usize::from(*tag_width)) - .ok_or(SchemaMaxSerializedSizeError::Overflow) + .ok_or(Error::Overflow) } // Tuples and structs sum sizes of all the members. @@ -166,30 +169,19 @@ fn max_serialized_size_impl<'a>( Fields::Empty => Ok(0), }, - // Primitive types. - Err("nil") => Ok(0), - Err("bool" | "i8" | "u8" | "nonzero_i8" | "nonzero_u8") => Ok(count.get()), - Err("i16" | "u16" | "nonzero_i16" | "nonzero_u16") => mul(count, 2), - Err("i32" | "u32" | "f32" | "nonzero_i32" | "nonzero_u32") => mul(count, 4), - Err("i64" | "u64" | "f64" | "nonzero_i64" | "nonzero_u64") => mul(count, 8), - Err("i128" | "u128" | "nonzero_i128" | "nonzero_u128") => mul(count, 16), - // string is just Vec + // "string" definition is added in another pr Err("string") => mul(count, add(MAX_LEN.get(), 4)?), - Err(declaration) => Err(SchemaMaxSerializedSizeError::MissingDefinition( - declaration.to_string(), - )), + Err(declaration) => Err(Error::MissingDefinition(declaration.to_string())), }?; stack.pop(); Ok(res) } + /// Checks whether given declaration schema serialises to an empty string. /// -/// Zero-sized types should follow the convention of either providing a [Definition] or -/// specifying `"nil"` as their [Declaration] for this method to work correctly. -/// /// This is used by [`BorshSchemaContainer::max_serialized_size`] to handle weird types /// such as `[[[(); u32::MAX]; u32::MAX]; u32::MAX]` which serialises to an /// empty string even though its number of elements overflows `usize`. @@ -201,17 +193,23 @@ fn max_serialized_size_impl<'a>( pub(super) fn is_zero_size( declaration: &Declaration, schema: &BorshSchemaContainer, -) -> Result { +) -> Result { let mut stack = Vec::new(); is_zero_size_impl(declaration, schema, &mut stack) } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) struct Recursive; +#[derive(Debug, PartialEq, Eq)] +pub(super) enum ZeroSizeError { + Recursive, + MissingDefinition(Declaration), +} -impl From for SchemaMaxSerializedSizeError { - fn from(_: Recursive) -> Self { - Self::Recursive +impl From for Error { + fn from(value: ZeroSizeError) -> Self { + match value { + ZeroSizeError::Recursive => Self::Recursive, + ZeroSizeError::MissingDefinition(declaration) => Self::MissingDefinition(declaration), + } } } @@ -219,13 +217,13 @@ fn is_zero_size_impl<'a>( declaration: &'a str, schema: &'a BorshSchemaContainer, stack: &mut Vec<&'a str>, -) -> Result { +) -> Result { fn all<'a, T: 'a>( iter: impl Iterator, f_key: impl Fn(&T) -> &'a Declaration, schema: &'a BorshSchemaContainer, stack: &mut Vec<&'a str>, - ) -> Result { + ) -> Result { for element in iter { let declaration = f_key(&element); if !is_zero_size_impl(declaration.as_str(), schema, stack)? { @@ -236,11 +234,12 @@ fn is_zero_size_impl<'a>( } if stack.iter().any(|dec| *dec == declaration) { - return Err(Recursive); + return Err(ZeroSizeError::Recursive); } stack.push(declaration); let res = match schema.get_definition(declaration).ok_or(declaration) { + Ok(Definition::Primitive(size)) => matches!(size, 0), Ok(Definition::Array { length, elements }) => { *length == 0 || is_zero_size_impl(elements.as_str(), schema, stack)? } @@ -269,7 +268,12 @@ fn is_zero_size_impl<'a>( Fields::Empty => true, }, - Err(dec) => dec == "nil", + // another pr removes this exclusion rule + Err("string") => false, + + Err(declaration) => { + return Err(ZeroSizeError::MissingDefinition(declaration.into())); + } }; stack.pop(); Ok(res) @@ -303,7 +307,7 @@ mod tests { } #[track_caller] - fn test_err(err: SchemaMaxSerializedSizeError) { + fn test_err(err: Error) { let schema = BorshSchemaContainer::for_type::(); assert_eq!(Err(err), schema.max_serialized_size()); } @@ -329,17 +333,51 @@ mod tests { struct RecursiveNoExitStructUnnamed(Box); let schema = BorshSchemaContainer::for_type::(); - assert_eq!(Err(Recursive), is_zero_size(schema.declaration(), &schema)); + assert_eq!( + Err(ZeroSizeError::Recursive), + is_zero_size(schema.declaration(), &schema) + ); } #[test] - fn max_serialized_size_built_in_types() { + fn max_serialized_size_primitives() { + test_ok::<()>(0); + test_ok::(1); + + test_ok::(4); + test_ok::(8); + + test_ok::(1); + test_ok::(2); + test_ok::(4); + test_ok::(8); + test_ok::(16); + + test_ok::(1); test_ok::(2); - test_ok::(8); + test_ok::(4); + test_ok::(8); + test_ok::(16); + test_ok::(1); test_ok::(2); + test_ok::(4); + test_ok::(8); + test_ok::(16); + + test_ok::(1); + test_ok::(2); test_ok::(4); + test_ok::(8); + test_ok::(16); + + test_ok::(8); + test_ok::(8); + test_ok::(8); + } + #[test] + fn max_serialized_size_built_in_types() { test_ok::(0); test_ok::>(2); test_ok::>(8); @@ -347,6 +385,7 @@ mod tests { test_ok::>(1); test_ok::>(2); test_ok::>(9); + test_ok::>>(1 + 4 + MAX_LEN); test_ok::<()>(0); test_ok::<(u8,)>(1); @@ -359,7 +398,7 @@ mod tests { test_ok::>(4 + MAX_LEN); test_ok::(4 + MAX_LEN); - test_err::>>(SchemaMaxSerializedSizeError::Overflow); + test_err::>>(Error::Overflow); test_ok::>>(4 + MAX_LEN * 4); test_ok::<[[[(); MAX_LEN]; MAX_LEN]; MAX_LEN]>(0); } @@ -396,8 +435,8 @@ mod tests { test_ok::(23); test_ok::(23); test_ok::(3 * 8 + 2 * (4 + MAX_LEN * 8)); - test_err::(SchemaMaxSerializedSizeError::Overflow); - test_err::(SchemaMaxSerializedSizeError::Recursive); + test_err::(Error::Overflow); + test_err::(Error::Recursive); } #[test] @@ -423,6 +462,7 @@ mod tests { }; crate::schema::add_definition(Self::declaration(), definition, definitions); T::add_definitions_recursively(definitions); + <()>::add_definitions_recursively(definitions); } } diff --git a/borsh/src/schema/container_ext/validate.rs b/borsh/src/schema/container_ext/validate.rs index 0b2c16754..3b4f26991 100644 --- a/borsh/src/schema/container_ext/validate.rs +++ b/borsh/src/schema/container_ext/validate.rs @@ -1,4 +1,4 @@ -use super::is_zero_size; +use super::{is_zero_size, ZeroSizeError}; use super::{BorshSchemaContainer, Declaration, Definition, Fields}; use crate::__private::maybestd::{string::ToString, vec::Vec}; @@ -6,9 +6,6 @@ impl BorshSchemaContainer { /// Validates container for violation of any well-known rules with /// respect to `borsh` serialization. /// - /// Zero-sized types should follow the convention of either providing a [Definition] or - /// specifying `"nil"` as their [Declaration] for this method to work correctly. - /// /// # Example /// /// ``` @@ -17,7 +14,7 @@ impl BorshSchemaContainer { /// let schema = BorshSchemaContainer::for_type::(); /// assert_eq!(Ok(()), schema.validate()); /// ``` - pub fn validate(&self) -> core::result::Result<(), SchemaContainerValidateError> { + pub fn validate(&self) -> core::result::Result<(), Error> { let mut stack = Vec::new(); validate_impl(self.declaration(), self, &mut stack) } @@ -26,25 +23,26 @@ impl BorshSchemaContainer { /// Possible error when validating a [`BorshSchemaContainer`], generated for some type `T`, /// for violation of any well-known rules with respect to `borsh` serialization. #[derive(Clone, PartialEq, Eq, Debug)] -pub enum SchemaContainerValidateError { +pub enum Error { /// sequences of zero-sized types of dynamic length are forbidden by definition /// see and related ones ZSTSequence(Declaration), /// Declared tag width is too large. Tags may be at most eight bytes. TagTooWide(Declaration), + /// Some of the declared types were lacking definition, which is considered + /// a container's validation error + MissingDefinition(Declaration), } fn validate_impl<'a>( declaration: &'a Declaration, schema: &'a BorshSchemaContainer, stack: &mut Vec<&'a Declaration>, -) -> core::result::Result<(), SchemaContainerValidateError> { +) -> core::result::Result<(), Error> { let definition = match schema.get_definition(declaration) { Some(definition) => definition, - // it's not an error for a type to not contain any definition - // it's either a `borsh`'s lib type like `"string"` or type declared by user None => { - return Ok(()); + return Err(Error::MissingDefinition(declaration.to_string())); } }; if stack.iter().any(|dec| *dec == declaration) { @@ -52,15 +50,21 @@ fn validate_impl<'a>( } stack.push(declaration); match definition { + Definition::Primitive(_size) => {} Definition::Array { elements, .. } => validate_impl(elements, schema, stack)?, Definition::Sequence { elements } => { - // a recursive type either has no exit, so it cannot be instantiated - // or it uses `Definiotion::Enum` or `Definition::Sequence` to exit from recursion - // which make it non-zero size - if is_zero_size(elements, schema).unwrap_or(false) { - return Err(SchemaContainerValidateError::ZSTSequence( - declaration.to_string(), - )); + let is_zst = match is_zero_size(elements, schema) { + Ok(is_zst) => is_zst, + // a recursive type either has no exit, so it cannot be instantiated + // or it uses `Definiotion::Enum` or `Definition::Sequence` to exit from recursion + // which make it non-zero size + Err(ZeroSizeError::Recursive) => false, + Err(ZeroSizeError::MissingDefinition(declaration)) => { + return Err(Error::MissingDefinition(declaration)); + } + }; + if is_zst { + return Err(Error::ZSTSequence(declaration.to_string())); } validate_impl(elements, schema, stack)?; } @@ -69,9 +73,7 @@ fn validate_impl<'a>( variants, } => { if *tag_width > 8 { - return Err(SchemaContainerValidateError::TagTooWide( - declaration.to_string(), - )); + return Err(Error::TagTooWide(declaration.to_string())); } for (_, variant) in variants { validate_impl(variant, schema, stack)?; diff --git a/borsh/tests/snapshots/test_schema_enums__complex_enum_with_schema.snap b/borsh/tests/snapshots/test_schema_enums__complex_enum_with_schema.snap index a9c7f34d7..76c44907f 100644 --- a/borsh/tests/snapshots/test_schema_enums__complex_enum_with_schema.snap +++ b/borsh/tests/snapshots/test_schema_enums__complex_enum_with_schema.snap @@ -17,7 +17,7 @@ expression: data 0, 0, 65, - 3, + 4, 1, 4, 0, @@ -111,7 +111,7 @@ expression: data 99, 111, 110, - 4, + 5, 2, 5, 0, @@ -122,7 +122,7 @@ expression: data 103, 103, 115, - 4, + 5, 2, 6, 0, @@ -134,7 +134,7 @@ expression: data 108, 97, 100, - 4, + 5, 1, 3, 0, @@ -183,7 +183,7 @@ expression: data 97, 103, 101, - 4, + 5, 0, 2, 0, @@ -245,7 +245,7 @@ expression: data 98, 101, 114, - 4, + 5, 2, 7, 0, @@ -258,7 +258,7 @@ expression: data 105, 110, 103, - 4, + 5, 2, 3, 0, @@ -267,7 +267,7 @@ expression: data 79, 105, 108, - 4, + 5, 2, 8, 0, @@ -281,7 +281,7 @@ expression: data 111, 101, 115, - 4, + 5, 2, 7, 0, @@ -294,7 +294,7 @@ expression: data 112, 101, 114, - 4, + 5, 2, 3, ] diff --git a/borsh/tests/test_schema_enums.rs b/borsh/tests/test_schema_enums.rs index c393eb5f0..b926f7429 100644 --- a/borsh/tests/test_schema_enums.rs +++ b/borsh/tests/test_schema_enums.rs @@ -284,7 +284,10 @@ fn common_map() -> BTreeMap { ])}, "EnumParametrizedC" => Definition::Struct{ fields: Fields::UnnamedFields(vec!["string".to_string(), "u16".to_string()])}, "BTreeMap" => Definition::Sequence { elements: "Tuple".to_string()}, - "Tuple" => Definition::Tuple { elements: vec!["u32".to_string(), "u16".to_string()]} + "Tuple" => Definition::Tuple { elements: vec!["u32".to_string(), "u16".to_string()]}, + "u32" => Definition::Primitive(4), + "i8" => Definition::Primitive(1), + "u16" => Definition::Primitive(2) } } diff --git a/borsh/tests/test_schema_nested.rs b/borsh/tests/test_schema_nested.rs index 2bcdd5728..3bcd5fb72 100644 --- a/borsh/tests/test_schema_nested.rs +++ b/borsh/tests/test_schema_nested.rs @@ -112,7 +112,9 @@ pub fn duplicated_instantiations() { }, "Tomatoes" => Definition::Struct {fields: Fields::Empty}, "Tuple" => Definition::Tuple {elements: vec!["u64".to_string(), "string".to_string()]}, - "Wrapper" => Definition::Struct{ fields: Fields::NamedFields(vec![("foo".to_string(), "Option".to_string()), ("bar".to_string(), "A".to_string())])} + "Wrapper" => Definition::Struct{ fields: Fields::NamedFields(vec![("foo".to_string(), "Option".to_string()), ("bar".to_string(), "A".to_string())])}, + "u64" => Definition::Primitive(8), + "nil" => Definition::Primitive(0) }, defs ); diff --git a/borsh/tests/test_schema_primitives.rs b/borsh/tests/test_schema_primitives.rs index abc82c0d4..473e2bd6a 100644 --- a/borsh/tests/test_schema_primitives.rs +++ b/borsh/tests/test_schema_primitives.rs @@ -2,20 +2,42 @@ #![cfg(hash_collections)] #![cfg(feature = "unstable__schema")] +#[cfg(feature = "std")] +use std::collections::BTreeMap; + #[cfg(not(feature = "std"))] extern crate alloc; #[cfg(not(feature = "std"))] -use alloc::string::ToString; +use alloc::{collections::BTreeMap, string::ToString}; use borsh::{schema::*, schema_container_of}; +macro_rules! map( + () => { BTreeMap::new() }; + { $($key:expr => $value:expr),+ } => { + { + let mut m = BTreeMap::new(); + $( + m.insert($key.to_string(), $value); + )+ + m + } + }; +); + #[test] fn isize_schema() { let schema = schema_container_of::(); assert_eq!( schema, - BorshSchemaContainer::new("i64".to_string(), Default::default()) + BorshSchemaContainer::new( + "i64".to_string(), + map! { + "i64" => Definition::Primitive(8) + + } + ) ) } @@ -25,6 +47,12 @@ fn usize_schema() { assert_eq!( schema, - BorshSchemaContainer::new("u64".to_string(), Default::default()) + BorshSchemaContainer::new( + "u64".to_string(), + map! { + "u64" => Definition::Primitive(8) + + } + ) ) } diff --git a/borsh/tests/test_schema_recursive.rs b/borsh/tests/test_schema_recursive.rs index 9a1bef1d4..5bc37c4b4 100644 --- a/borsh/tests/test_schema_recursive.rs +++ b/borsh/tests/test_schema_recursive.rs @@ -99,7 +99,9 @@ pub fn recursive_enum_schema() { "Vec" => Definition::Sequence { elements: "ERecD".to_string(), - } + }, + "i32" => Definition::Primitive(4), + "u8" => Definition::Primitive(1) }, defs ); diff --git a/borsh/tests/test_schema_structs.rs b/borsh/tests/test_schema_structs.rs index 3c66ef2b6..e4255d6bc 100644 --- a/borsh/tests/test_schema_structs.rs +++ b/borsh/tests/test_schema_structs.rs @@ -64,7 +64,8 @@ pub fn simple_struct() { "A" => Definition::Struct{ fields: Fields::NamedFields(vec![ ("_f1".to_string(), "u64".to_string()), ("_f2".to_string(), "string".to_string()) - ])} + ])}, + "u64" => Definition::Primitive(8) }, defs ); @@ -88,7 +89,9 @@ pub fn boxed() { ("_f1".to_string(), "u64".to_string()), ("_f2".to_string(), "string".to_string()), ("_f3".to_string(), "Vec".to_string()) - ])} + ])}, + "u64" => Definition::Primitive(8), + "u8" => Definition::Primitive(1) }, defs ); @@ -103,7 +106,8 @@ pub fn wrapper_struct() { >::add_definitions_recursively(&mut defs); assert_eq!( map! { - "A" => Definition::Struct {fields: Fields::UnnamedFields(vec!["u64".to_string()])} + "A" => Definition::Struct {fields: Fields::UnnamedFields(vec!["u64".to_string()])}, + "u64" => Definition::Primitive(8) }, defs ); @@ -120,7 +124,8 @@ pub fn tuple_struct() { map! { "A" => Definition::Struct {fields: Fields::UnnamedFields(vec![ "u64".to_string(), "string".to_string() - ])} + ])}, + "u64" => Definition::Primitive(8) }, defs ); @@ -140,7 +145,8 @@ pub fn tuple_struct_params() { map! { "A" => Definition::Struct { fields: Fields::UnnamedFields(vec![ "u64".to_string(), "string".to_string() - ])} + ])}, + "u64" => Definition::Primitive(8) }, defs ); @@ -169,7 +175,8 @@ pub fn simple_generics() { ]) }, "HashMap" => Definition::Sequence {elements: "Tuple".to_string()}, - "Tuple" => Definition::Tuple{elements: vec!["u64".to_string(), "string".to_string()]} + "Tuple" => Definition::Tuple{elements: vec!["u64".to_string(), "string".to_string()]}, + "u64" => Definition::Primitive(8) }, defs ); @@ -183,7 +190,8 @@ fn common_map() -> BTreeMap { ("field".to_string(), "i8".to_string()), ("another".to_string(), "string".to_string()) ]) - } + }, + "i8" => Definition::Primitive(1) } } @@ -292,7 +300,9 @@ pub fn generic_associated_item3() { }, "Tuple" => Definition::Tuple { elements: vec!["i8".to_string(), "u32".to_string()] - } + }, + "i8" => Definition::Primitive(1), + "u32" => Definition::Primitive(4) }, defs ); @@ -321,7 +331,8 @@ pub fn with_phantom_data() { ("field".to_string(), "string".to_string()), ("another".to_string(), "nil".to_string()) ]) - } + }, + "nil" => Definition::Primitive(0) }, defs ); diff --git a/borsh/tests/test_schema_tuple.rs b/borsh/tests/test_schema_tuple.rs index 29ceb6768..761781d9b 100644 --- a/borsh/tests/test_schema_tuple.rs +++ b/borsh/tests/test_schema_tuple.rs @@ -31,7 +31,8 @@ fn test_unary_tuple_schema() { <(bool,)>::add_definitions_recursively(&mut defs); assert_eq!( map! { - "Tuple" => Definition::Tuple { elements: vec!["bool".to_string()] } + "Tuple" => Definition::Tuple { elements: vec!["bool".to_string()] }, + "bool" => Definition::Primitive(1) }, defs ); diff --git a/borsh/tests/test_schema_validate.rs b/borsh/tests/test_schema_validate.rs index 1c970b6fb..3d37b7324 100644 --- a/borsh/tests/test_schema_validate.rs +++ b/borsh/tests/test_schema_validate.rs @@ -48,7 +48,7 @@ fn validate_for_derived_types() { test_ok::(); test_ok::(); test_ok::(); - test_ok::(); + // test_ok::(); test_ok::(); test_ok::(); test_ok::(); diff --git a/borsh/tests/test_schema_with.rs b/borsh/tests/test_schema_with.rs index 9984757ae..28b07ffb4 100644 --- a/borsh/tests/test_schema_with.rs +++ b/borsh/tests/test_schema_with.rs @@ -114,7 +114,8 @@ pub fn struct_overriden() { "BTreeMap".to_string(), ]) }, "BTreeMap"=> Definition::Sequence { elements: "Tuple".to_string() }, - "Tuple" => Definition::Tuple { elements: vec!["u64".to_string(), "string".to_string()]} + "Tuple" => Definition::Tuple { elements: vec!["u64".to_string(), "string".to_string()]}, + "u64" => Definition::Primitive(8) }, defs ); @@ -145,7 +146,8 @@ pub fn enum_overriden() { "BTreeMap".to_string(), ]) }, "BTreeMap"=> Definition::Sequence { elements: "Tuple".to_string() }, - "Tuple" => Definition::Tuple { elements: vec!["u64".to_string(), "string".to_string()]} + "Tuple" => Definition::Tuple { elements: vec!["u64".to_string(), "string".to_string()]}, + "u64" => Definition::Primitive(8) }, defs );