From f7a50c50e3de228defe8244f87cdbb4a8f68bc9d Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Tue, 12 Sep 2023 21:54:14 +0200 Subject: [PATCH 01/22] wip Signed-off-by: Cyrill Leutwiler --- crates/storage/src/lazy/mapping.rs | 25 +++++----------- crates/storage/src/lazy/mod.rs | 45 ++++++++++++++++++---------- crates/storage/traits/src/storage.rs | 8 +++++ 3 files changed, 46 insertions(+), 32 deletions(-) diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index 4a9b1db7eb..3a7857abcc 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -19,21 +19,11 @@ //! This mapping doesn't actually "own" any data. //! Instead it is just a simple wrapper around the contract storage facilities. -use crate::traits::{ - AutoKey, - Packed, - StorableHint, - StorageKey, -}; +use crate::traits::{AutoKey, Packed, StorableHint, StorageKey}; use core::marker::PhantomData; use ink_primitives::Key; use ink_storage_traits::Storable; -use scale::{ - Encode, - Error, - Input, - Output, -}; +use scale::{Encode, Error, Input, Output}; /// A mapping of key-value pairs directly into contract storage. /// @@ -219,6 +209,11 @@ where fn decode(_input: &mut I) -> Result { Ok(Default::default()) } + + #[inline] + fn size_hint(&self) -> usize { + 0 + } } impl StorableHint for Mapping @@ -242,11 +237,7 @@ where #[cfg(feature = "std")] const _: () = { use crate::traits::StorageLayout; - use ink_metadata::layout::{ - Layout, - LayoutKey, - RootLayout, - }; + use ink_metadata::layout::{Layout, LayoutKey, RootLayout}; impl StorageLayout for Mapping where diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index d730f541f3..6cd677e898 100644 --- a/crates/storage/src/lazy/mod.rs +++ b/crates/storage/src/lazy/mod.rs @@ -23,19 +23,11 @@ mod mapping; #[doc(inline)] pub use self::mapping::Mapping; -use crate::traits::{ - AutoKey, - StorableHint, - StorageKey, -}; +use crate::traits::{AutoKey, StorableHint, StorageKey}; use core::marker::PhantomData; use ink_primitives::Key; use ink_storage_traits::Storable; -use scale::{ - Error, - Input, - Output, -}; +use scale::{Error, Input, Output}; /// A simple wrapper around a type to store it in a separate storage cell under its own /// storage key. If you want to update the value, first you need to @@ -142,10 +134,32 @@ where } } + /// Try to read the `value` from the contract storage, if it exists. + /// + /// Fails if `value` exceeds the static buffer size. + pub fn try_get(&self) -> ink_env::Result> { + match ink_env::get_contract_storage::(&KeyType::KEY) { + Ok(Some(value)) => Some(value), + Err() + _ => None, + } + } + /// Writes the given `value` to the contract storage. pub fn set(&mut self, value: &V) { ink_env::set_contract_storage::(&KeyType::KEY, value); } + + /// Try to set the given `value` to the contract storage. + /// + /// Fails if `value` exceeds the static buffer size. + pub fn try_set(&mut self, value: &V) -> ink_env::Result<()> { + if value.size_hint() > ink_env::BUFFER_SIZE { + return Err(ink_env::Error::Decode(scale::Error::from("buffer size"))); + }; + ink_env::set_contract_storage::(&KeyType::KEY, value); + Ok(()) + } } impl Lazy @@ -175,6 +189,11 @@ where fn decode(_input: &mut I) -> Result { Ok(Default::default()) } + + #[inline(always)] + fn size_hint(&self) -> usize { + 0 + } } impl StorableHint for Lazy @@ -197,11 +216,7 @@ where #[cfg(feature = "std")] const _: () = { use crate::traits::StorageLayout; - use ink_metadata::layout::{ - Layout, - LayoutKey, - RootLayout, - }; + use ink_metadata::layout::{Layout, LayoutKey, RootLayout}; impl StorageLayout for Lazy where diff --git a/crates/storage/traits/src/storage.rs b/crates/storage/traits/src/storage.rs index a0e3c4951d..67a06db271 100644 --- a/crates/storage/traits/src/storage.rs +++ b/crates/storage/traits/src/storage.rs @@ -25,6 +25,8 @@ pub trait Storable: Sized { /// Attempt to deserialize the value from input. fn decode(input: &mut I) -> Result; + + fn size_hint(&self) -> usize; } /// Types which implement `scale::Encode` and `scale::Decode` are `Storable` by default @@ -42,6 +44,12 @@ where fn decode(input: &mut I) -> Result { scale::Decode::decode(input) } + + #[inline] + fn size_hint(&self) -> usize { +

::encoded_size(&self) +

::encoded_size(&self) + } } pub(crate) mod private { From d1bbb7ec63c635472dd06a45ac45ab1df9306d99 Mon Sep 17 00:00:00 2001 From: xermicus Date: Wed, 13 Sep 2023 14:37:33 +0200 Subject: [PATCH 02/22] draft fallibe getter Signed-off-by: xermicus --- crates/storage/src/lazy/mapping.rs | 2 +- crates/storage/src/lazy/mod.rs | 31 ++++++++++++++++++---------- crates/storage/traits/src/storage.rs | 5 ++--- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index 3a7857abcc..fc8524eada 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -211,7 +211,7 @@ where } #[inline] - fn size_hint(&self) -> usize { + fn encoded_size(&self) -> usize { 0 } } diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index 6cd677e898..496844298a 100644 --- a/crates/storage/src/lazy/mod.rs +++ b/crates/storage/src/lazy/mod.rs @@ -134,15 +134,22 @@ where } } - /// Try to read the `value` from the contract storage, if it exists. + /// Try to read the `value` from the contract storage. /// - /// Fails if `value` exceeds the static buffer size. - pub fn try_get(&self) -> ink_env::Result> { - match ink_env::get_contract_storage::(&KeyType::KEY) { - Ok(Some(value)) => Some(value), - Err() - _ => None, + /// Returns: + /// - `Some(Ok(_))` if `value` was received from storage and could be decoded. + /// - `Some(Err(_))` if the encoded length of `value` exceeds the static buffer size. + /// - `None` if there was no value under this storage key. + pub fn try_get(&self) -> Option> { + let encoded_length: usize = ink_env::contains_contract_storage(&KeyType::KEY)? + .try_into() + .expect("targets of less than 32bit pointer size are not supported; qed"); + + if encoded_length > ink_env::BUFFER_SIZE.try_into().expect("BufferTooSmall") { + return Err(ink_env::Error::Decode(scale::Error::from("buffer size"))).into(); } + + self.get().map(Ok) } /// Writes the given `value` to the contract storage. @@ -154,10 +161,12 @@ where /// /// Fails if `value` exceeds the static buffer size. pub fn try_set(&mut self, value: &V) -> ink_env::Result<()> { - if value.size_hint() > ink_env::BUFFER_SIZE { - return Err(ink_env::Error::Decode(scale::Error::from("buffer size"))); + if value.encoded_size() > ink_env::BUFFER_SIZE { + return Err(ink_env::Error::Decode(scale::Error::from("BufferTooSmall"))); }; - ink_env::set_contract_storage::(&KeyType::KEY, value); + + self.set(value); + Ok(()) } } @@ -191,7 +200,7 @@ where } #[inline(always)] - fn size_hint(&self) -> usize { + fn encoded_size(&self) -> usize { 0 } } diff --git a/crates/storage/traits/src/storage.rs b/crates/storage/traits/src/storage.rs index 67a06db271..a92036a66e 100644 --- a/crates/storage/traits/src/storage.rs +++ b/crates/storage/traits/src/storage.rs @@ -26,7 +26,7 @@ pub trait Storable: Sized { /// Attempt to deserialize the value from input. fn decode(input: &mut I) -> Result; - fn size_hint(&self) -> usize; + fn encoded_size(&self) -> usize; } /// Types which implement `scale::Encode` and `scale::Decode` are `Storable` by default @@ -46,8 +46,7 @@ where } #[inline] - fn size_hint(&self) -> usize { -

::encoded_size(&self) + fn encoded_size(&self) -> usize {

::encoded_size(&self) } } From 377c1a7d41defc626ff1f5b74fe57be4e3308250 Mon Sep 17 00:00:00 2001 From: xermicus Date: Wed, 13 Sep 2023 16:49:08 +0200 Subject: [PATCH 03/22] implement it in macros Signed-off-by: xermicus --- crates/ink/macro/src/storage/storable.rs | 55 +++++++++- crates/ink/macro/src/tests/storable.rs | 134 +++++++++++++++++++++++ 2 files changed, 184 insertions(+), 5 deletions(-) diff --git a/crates/ink/macro/src/storage/storable.rs b/crates/ink/macro/src/storage/storable.rs index 728077e8f6..5274861e91 100644 --- a/crates/ink/macro/src/storage/storable.rs +++ b/crates/ink/macro/src/storage/storable.rs @@ -13,10 +13,7 @@ // limitations under the License. use proc_macro2::TokenStream as TokenStream2; -use quote::{ - quote, - quote_spanned, -}; +use quote::{quote, quote_spanned}; use syn::spanned::Spanned; /// `Storable` derive implementation for `struct` types. @@ -36,6 +33,12 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { ::ink::storage::traits::Storable::encode(#binding, __dest); ) }); + let encoded_size_body = variant.each(|binding| { + let span = binding.ast().ty.span(); + quote_spanned!(span => + encoded_size + ::ink::storage::traits::Storable::encoded_size(#binding); + ) + }); s.gen_impl(quote! { gen impl ::ink::storage::traits::Storable for @Self { @@ -50,6 +53,15 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { fn encode<__ink_O: ::ink::scale::Output + ?::core::marker::Sized>(&self, __dest: &mut __ink_O) { match self { #encode_body } } + + #[inline(always)] + #[allow(unused_mut)] + #[allow(non_camel_case_types)] + fn encoded_size(&self) -> ::core::primitive::usize { + let mut encoded_size = 0; + match self { #encoded_size_body } + encoded_size + } } }) } @@ -66,7 +78,7 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { s.ast().span(), "Currently only enums with at most 256 variants are supported.", ) - .to_compile_error() + .to_compile_error(); } let decode_body = s @@ -108,6 +120,26 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { } } }); + + let encoded_size_body = s.variants().iter().enumerate().map(|(index, variant)| { + let pat = variant.pat(); + let index = index as u8; + let fields = variant.bindings().iter().map(|field| { + let span = field.ast().ty.span(); + quote_spanned!(span => + encoded_size += ::ink::storage::traits::Storable::encoded_size(#field); + ) + }); + quote! { + #pat => { + { encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&#index); } + #( + { #fields } + )* + } + } + }); + s.gen_impl(quote! { gen impl ::ink::storage::traits::Storable for @Self { #[inline(always)] @@ -130,6 +162,19 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { )* } } + + #[inline(always)] + #[allow(unused_mut)] + #[allow(non_camel_case_types)] + fn encoded_size(&self) -> ::core::primitive::usize { + let mut encoded_size = 0; + match self { + #( + #encoded_size_body + )* + } + encoded_size + } } }) } diff --git a/crates/ink/macro/src/tests/storable.rs b/crates/ink/macro/src/tests/storable.rs index 37ba2bf9bb..0ebd5cb12b 100644 --- a/crates/ink/macro/src/tests/storable.rs +++ b/crates/ink/macro/src/tests/storable.rs @@ -44,6 +44,17 @@ fn unit_struct_works() { UnitStruct => { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + #[allow(unused_mut)] + fn encoded_size(&self) -> ::core::primitive::usize { + let mut encoded_size = 0; + match self { + UnitStruct => {} + } + encoded_size + } } }; } @@ -106,6 +117,27 @@ fn struct_works() { } } } + + #[inline (always)] + #[allow (non_camel_case_types)] + #[allow(unused_mut)] + fn encoded_size(&self) -> ::core::primitive::usize { + let mut encoded_size = 0; + match self { + NamedFields { a : __binding_0 , b : __binding_1 , d : __binding_2 , } => { + { + encoded_size += ::ink::storage:: traits:: Storable:: encoded_size(__binding_0); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_2); + } + } + } + encoded_size + } } }; } @@ -149,6 +181,21 @@ fn one_variant_enum_works() { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + #[allow(unused_mut)] + fn encoded_size (&self) -> ::core::primitive::usize { + let mut encoded_size = 0; + match self { + OneVariantEnum::A => { + { + encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8); + } + } + } + encoded_size + } } }; } @@ -244,6 +291,43 @@ fn enum_works() { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + #[allow(unused_mut)] + fn encoded_size (&self) -> ::core::primitive::usize{ + let mut encoded_size = 0; + match self { + MixedEnum::A => { + { + encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8); + } + } + MixedEnum::B(__binding_0, __binding_1,) => { + { + encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&1u8); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + } + } + MixedEnum::C { a: __binding_0, b: __binding_1, } => { + { + encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&2u8); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + } + } + } + encoded_size + } } }; } @@ -310,6 +394,24 @@ fn generic_struct_works() { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + #[allow(unused_mut)] + fn encoded_size(&self) -> ::core::primitive::usize { + let mut encoded_size = 0; + match self { + GenericStruct { a : __binding_0 , b : __binding_1 , } => { + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + } + } + } + encoded_size + } } }; } @@ -394,6 +496,38 @@ fn generic_enum_works() { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + #[allow(unused_mut)] + fn encoded_size(&self) -> ::core::primitive::usize { + let mut encoded_size = 0; + match self { + GenericEnum::Tuple (__binding_0, __binding_1,) => { + { + encoded_size += <::core::primitive:: u8 as::ink::storage::traits::Storable>::encoded_size(&0u8); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + } + } + GenericEnum::Named { a: __binding_0, b: __binding_1,} => { + { + encoded_size += <::core::primitive::u8 as::ink::storage::traits::Storable>::encoded_size(&1u8); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + } + { + encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + } + } + } + encoded_size + } } }; } From 2af27482ed011fef5ad6f0a6e0ab3575499a79b1 Mon Sep 17 00:00:00 2001 From: xermicus Date: Wed, 13 Sep 2023 16:55:06 +0200 Subject: [PATCH 04/22] typos Signed-off-by: xermicus --- crates/ink/macro/src/storage/storable.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ink/macro/src/storage/storable.rs b/crates/ink/macro/src/storage/storable.rs index 5274861e91..91f0ea5732 100644 --- a/crates/ink/macro/src/storage/storable.rs +++ b/crates/ink/macro/src/storage/storable.rs @@ -36,7 +36,7 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { let encoded_size_body = variant.each(|binding| { let span = binding.ast().ty.span(); quote_spanned!(span => - encoded_size + ::ink::storage::traits::Storable::encoded_size(#binding); + encoded_size += ::ink::storage::traits::Storable::encoded_size(#binding); ) }); @@ -55,8 +55,8 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { } #[inline(always)] - #[allow(unused_mut)] #[allow(non_camel_case_types)] + #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { let mut encoded_size = 0; match self { #encoded_size_body } @@ -164,8 +164,8 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { } #[inline(always)] - #[allow(unused_mut)] #[allow(non_camel_case_types)] + #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { let mut encoded_size = 0; match self { From b93ccec934ffab38ace7d6e93116110ef210d782 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Wed, 27 Sep 2023 10:51:32 +0200 Subject: [PATCH 05/22] add tests for lazy Signed-off-by: Cyrill Leutwiler --- crates/env/src/engine/off_chain/impls.rs | 2 +- crates/storage/src/lazy/mod.rs | 58 ++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/crates/env/src/engine/off_chain/impls.rs b/crates/env/src/engine/off_chain/impls.rs index 04b792df6f..3d7feee6b3 100644 --- a/crates/env/src/engine/off_chain/impls.rs +++ b/crates/env/src/engine/off_chain/impls.rs @@ -202,7 +202,7 @@ impl EnvBackend for EnvInstance { K: scale::Encode, R: Storable, { - let mut output: [u8; 9600] = [0; 9600]; + let mut output: [u8; BUFFER_SIZE] = [0; BUFFER_SIZE]; match self.engine.get_storage(&key.encode(), &mut &mut output[..]) { Ok(_) => (), Err(ext::Error::KeyNotFound) => return Ok(None), diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index 496844298a..e8ba587bd0 100644 --- a/crates/storage/src/lazy/mod.rs +++ b/crates/storage/src/lazy/mod.rs @@ -23,11 +23,19 @@ mod mapping; #[doc(inline)] pub use self::mapping::Mapping; -use crate::traits::{AutoKey, StorableHint, StorageKey}; +use crate::traits::{ + AutoKey, + StorableHint, + StorageKey, +}; use core::marker::PhantomData; use ink_primitives::Key; use ink_storage_traits::Storable; -use scale::{Error, Input, Output}; +use scale::{ + Error, + Input, + Output, +}; /// A simple wrapper around a type to store it in a separate storage cell under its own /// storage key. If you want to update the value, first you need to @@ -146,7 +154,7 @@ where .expect("targets of less than 32bit pointer size are not supported; qed"); if encoded_length > ink_env::BUFFER_SIZE.try_into().expect("BufferTooSmall") { - return Err(ink_env::Error::Decode(scale::Error::from("buffer size"))).into(); + return Err(ink_env::Error::Decode(scale::Error::from("buffer size"))).into() } self.get().map(Ok) @@ -162,7 +170,7 @@ where /// Fails if `value` exceeds the static buffer size. pub fn try_set(&mut self, value: &V) -> ink_env::Result<()> { if value.encoded_size() > ink_env::BUFFER_SIZE { - return Err(ink_env::Error::Decode(scale::Error::from("BufferTooSmall"))); + return Err(ink_env::Error::Decode(scale::Error::from("BufferTooSmall"))) }; self.set(value); @@ -225,7 +233,11 @@ where #[cfg(feature = "std")] const _: () = { use crate::traits::StorageLayout; - use ink_metadata::layout::{Layout, LayoutKey, RootLayout}; + use ink_metadata::layout::{ + Layout, + LayoutKey, + RootLayout, + }; impl StorageLayout for Lazy where @@ -293,4 +305,40 @@ mod tests { }) .unwrap() } + + #[test] + fn fallible_storage_works_for_fitting_data() { + ink_env::test::run_test::(|_| { + let mut storage: Lazy<[u8; ink_env::BUFFER_SIZE]> = Lazy::new(); + + let value = [0u8; ink_env::BUFFER_SIZE]; + assert_eq!(storage.try_set(&value), Ok(())); + assert_eq!(storage.try_get(), Some(Ok(value))); + + Ok(()) + }) + .unwrap() + } + + #[test] + fn fallible_storage_fails_gracefully_for_overgrown_data() { + ink_env::test::run_test::(|_| { + let mut storage: Lazy<[u8; ink_env::BUFFER_SIZE + 1]> = Lazy::new(); + + let value = [0u8; ink_env::BUFFER_SIZE + 1]; + assert_eq!(storage.try_get(), None); + assert!(storage.try_set(&value).is_err()); + + // The off-chain impl conveniently uses a Vec for encoding, + // allowing writing values exceeding the static buffer size. + ink_env::set_contract_storage(&storage.key(), &value); + assert!(matches!( + storage.try_get(), + Some(Err(ink_env::Error::Decode(scale::Error { .. }))) + )); + + Ok(()) + }) + .unwrap() + } } From a461884f9788da30854965a98071755fc0c42f5f Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Wed, 27 Sep 2023 14:17:43 +0200 Subject: [PATCH 06/22] dedicated error variant Signed-off-by: Cyrill Leutwiler --- crates/env/src/error.rs | 2 ++ crates/storage/src/lazy/mod.rs | 13 +++++-------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/env/src/error.rs b/crates/env/src/error.rs index 55a71926f4..977a9116a8 100644 --- a/crates/env/src/error.rs +++ b/crates/env/src/error.rs @@ -22,6 +22,8 @@ use crate::engine::off_chain::OffChainError; pub enum Error { /// Error upon decoding an encoded value. Decode(scale::Error), + /// The static buffer used during ABI encoding or ABI decoding is too small. + BufferTooSmall, /// An error that can only occur in the off-chain environment. #[cfg(any(feature = "std", test, doc))] OffChain(OffChainError), diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index e8ba587bd0..5e27f56965 100644 --- a/crates/storage/src/lazy/mod.rs +++ b/crates/storage/src/lazy/mod.rs @@ -153,8 +153,8 @@ where .try_into() .expect("targets of less than 32bit pointer size are not supported; qed"); - if encoded_length > ink_env::BUFFER_SIZE.try_into().expect("BufferTooSmall") { - return Err(ink_env::Error::Decode(scale::Error::from("buffer size"))).into() + if encoded_length > ink_env::BUFFER_SIZE { + return Some(Err(ink_env::Error::BufferTooSmall)) } self.get().map(Ok) @@ -170,7 +170,7 @@ where /// Fails if `value` exceeds the static buffer size. pub fn try_set(&mut self, value: &V) -> ink_env::Result<()> { if value.encoded_size() > ink_env::BUFFER_SIZE { - return Err(ink_env::Error::Decode(scale::Error::from("BufferTooSmall"))) + return Err(ink_env::Error::BufferTooSmall) }; self.set(value); @@ -327,15 +327,12 @@ mod tests { let value = [0u8; ink_env::BUFFER_SIZE + 1]; assert_eq!(storage.try_get(), None); - assert!(storage.try_set(&value).is_err()); + assert_eq!(storage.try_set(&value), Err(ink_env::Error::BufferTooSmall)); // The off-chain impl conveniently uses a Vec for encoding, // allowing writing values exceeding the static buffer size. ink_env::set_contract_storage(&storage.key(), &value); - assert!(matches!( - storage.try_get(), - Some(Err(ink_env::Error::Decode(scale::Error { .. }))) - )); + assert_eq!(storage.try_get(), Some(Err(ink_env::Error::BufferTooSmall))); Ok(()) }) From 8c9c3aadf9dfa41df15f4ff659c76b4f194f0b44 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Wed, 27 Sep 2023 15:07:07 +0200 Subject: [PATCH 07/22] implement try API for mapping Signed-off-by: Cyrill Leutwiler --- crates/env/src/engine/off_chain/impls.rs | 2 +- crates/storage/src/lazy/mapping.rs | 146 ++++++++++++++++++++++- crates/storage/src/lazy/mod.rs | 4 +- crates/storage/traits/src/storage.rs | 3 +- 4 files changed, 147 insertions(+), 8 deletions(-) diff --git a/crates/env/src/engine/off_chain/impls.rs b/crates/env/src/engine/off_chain/impls.rs index 3d7feee6b3..5a79a75af4 100644 --- a/crates/env/src/engine/off_chain/impls.rs +++ b/crates/env/src/engine/off_chain/impls.rs @@ -217,7 +217,7 @@ impl EnvBackend for EnvInstance { K: scale::Encode, R: Storable, { - let mut output: [u8; 9600] = [0; 9600]; + let mut output: [u8; BUFFER_SIZE] = [0; BUFFER_SIZE]; match self .engine .take_storage(&key.encode(), &mut &mut output[..]) diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index fc8524eada..ccb2e47f8f 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -19,11 +19,21 @@ //! This mapping doesn't actually "own" any data. //! Instead it is just a simple wrapper around the contract storage facilities. -use crate::traits::{AutoKey, Packed, StorableHint, StorageKey}; +use crate::traits::{ + AutoKey, + Packed, + StorableHint, + StorageKey, +}; use core::marker::PhantomData; use ink_primitives::Key; use ink_storage_traits::Storable; -use scale::{Encode, Error, Input, Output}; +use scale::{ + Encode, + Error, + Input, + Output, +}; /// A mapping of key-value pairs directly into contract storage. /// @@ -134,6 +144,28 @@ where ink_env::set_contract_storage(&(&KeyType::KEY, key), value) } + /// Try to insert the given `value` into the mapping under given `key`. + /// + /// Fails if `value` exceeds the static buffer size. + /// + /// Returns: + /// - `Ok(Some(_))` if the value was inserted successfully, containing the size in + /// bytes of the pre-existing value at the specified key if any. + /// - `Ok(None)` if the insert was succesfull but there was no pre-existing value. + /// - `Err(_)` if the encoded value exceeds the static buffer size. + #[inline] + pub fn try_insert(&mut self, key: Q, value: &R) -> ink_env::Result> + where + Q: scale::EncodeLike, + R: Storable + scale::EncodeLike, + { + if ::encoded_size(value) > ink_env::BUFFER_SIZE { + return Err(ink_env::Error::BufferTooSmall) + }; + + Ok(self.insert(key, value)) + } + /// Get the `value` at `key` from the contract storage. /// /// Returns `None` if no `value` exists at the given `key`. @@ -146,6 +178,30 @@ where .unwrap_or_else(|error| panic!("Failed to get value in Mapping: {error:?}")) } + /// Try to get the `value` at the given `key`. + /// + /// Returns: + /// - `Some(Ok(_))` containing the value if it existed and was decoded successfully. + /// - `Some(Err(_))` if the value existed but its length exceeds the static buffer + /// size. + /// - `None` if there was no value under this mapping key. + #[inline] + pub fn try_get(&self, key: Q) -> Option> + where + Q: scale::EncodeLike, + { + let encoded_length: usize = + ink_env::contains_contract_storage(&(&KeyType::KEY, &key))? + .try_into() + .expect("targets of less than 32bit pointer size are not supported; qed"); + + if encoded_length > ink_env::BUFFER_SIZE { + return Some(Err(ink_env::Error::BufferTooSmall)) + } + + self.get(key).map(Ok) + } + /// Removes the `value` at `key`, returning the previous `value` at `key` from /// storage. /// @@ -165,6 +221,37 @@ where .unwrap_or_else(|error| panic!("Failed to take value in Mapping: {error:?}")) } + /// Try to take the `value` at the given `key`. + /// On success, this operation will remove the value from the mapping + /// + /// Returns: + /// - `Some(Ok(_))` containing the value if it existed and was decoded successfully. + /// - `Some(Err(_))` if the value existed but its length exceeds the static buffer + /// size. + /// - `None` if there was no value under this mapping key. + //// + /// # Warning + /// + /// This method uses the + /// [unstable interface](https://github.com/paritytech/substrate/tree/master/frame/contracts#unstable-interfaces), + /// which is unsafe and normally is not available on production chains. + #[inline] + pub fn try_take(&self, key: Q) -> Option> + where + Q: scale::EncodeLike, + { + let encoded_length: usize = + ink_env::contains_contract_storage(&(&KeyType::KEY, &key))? + .try_into() + .expect("targets of less than 32bit pointer size are not supported; qed"); + + if encoded_length > ink_env::BUFFER_SIZE { + return Some(Err(ink_env::Error::BufferTooSmall)) + } + + self.take(key).map(Ok) + } + /// Get the size in bytes of a value stored at `key` in the contract storage. /// /// Returns `None` if no `value` exists at the given `key`. @@ -237,7 +324,11 @@ where #[cfg(feature = "std")] const _: () = { use crate::traits::StorageLayout; - use ink_metadata::layout::{Layout, LayoutKey, RootLayout}; + use ink_metadata::layout::{ + Layout, + LayoutKey, + RootLayout, + }; impl StorageLayout for Mapping where @@ -356,4 +447,53 @@ mod tests { }) .unwrap() } + + #[test] + fn fallible_storage_works_for_fitting_data() { + ink_env::test::run_test::(|_| { + let mut mapping: Mapping = Mapping::new(); + + let key = 0; + let value = [0u8; ink_env::BUFFER_SIZE]; + + assert_eq!(mapping.try_insert(key, &value), Ok(None)); + assert_eq!(mapping.try_get(key), Some(Ok(value))); + assert_eq!(mapping.try_take(key), Some(Ok(value))); + assert_eq!(mapping.try_get(key), None); + + Ok(()) + }) + .unwrap() + } + + #[test] + fn fallible_storage_fails_gracefully_for_overgrown_data() { + ink_env::test::run_test::(|_| { + let mut mapping: Mapping = Mapping::new(); + + let key = 0; + let value = [0u8; ink_env::BUFFER_SIZE + 1]; + + assert_eq!(mapping.try_get(0), None); + assert_eq!( + mapping.try_insert(key, &value), + Err(ink_env::Error::BufferTooSmall) + ); + + // The off-chain impl conveniently uses a Vec for encoding, + // allowing writing values exceeding the static buffer size. + ink_env::set_contract_storage(&(&mapping.key(), key), &value); + assert_eq!( + mapping.try_get(key), + Some(Err(ink_env::Error::BufferTooSmall)) + ); + assert_eq!( + mapping.try_take(key), + Some(Err(ink_env::Error::BufferTooSmall)) + ); + + Ok(()) + }) + .unwrap() + } } diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index 5e27f56965..a5a9d82719 100644 --- a/crates/storage/src/lazy/mod.rs +++ b/crates/storage/src/lazy/mod.rs @@ -173,9 +173,7 @@ where return Err(ink_env::Error::BufferTooSmall) }; - self.set(value); - - Ok(()) + Ok(self.set(value)) } } diff --git a/crates/storage/traits/src/storage.rs b/crates/storage/traits/src/storage.rs index a92036a66e..54053b44f8 100644 --- a/crates/storage/traits/src/storage.rs +++ b/crates/storage/traits/src/storage.rs @@ -26,6 +26,7 @@ pub trait Storable: Sized { /// Attempt to deserialize the value from input. fn decode(input: &mut I) -> Result; + /// The exact number of bytes this type consumes in the encoded form. fn encoded_size(&self) -> usize; } @@ -47,7 +48,7 @@ where #[inline] fn encoded_size(&self) -> usize { -

::encoded_size(&self) +

::encoded_size(self) } } From 8f064c9312b0482d0cc5d880365d483a67809e93 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Wed, 27 Sep 2023 15:12:23 +0200 Subject: [PATCH 08/22] clippy, fmt, spellcheck Signed-off-by: Cyrill Leutwiler --- crates/ink/macro/src/storage/storable.rs | 7 ++++-- crates/storage/src/lazy/mapping.rs | 28 ++++++------------------ crates/storage/src/lazy/mod.rs | 4 +++- 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/crates/ink/macro/src/storage/storable.rs b/crates/ink/macro/src/storage/storable.rs index 91f0ea5732..79370dad03 100644 --- a/crates/ink/macro/src/storage/storable.rs +++ b/crates/ink/macro/src/storage/storable.rs @@ -13,7 +13,10 @@ // limitations under the License. use proc_macro2::TokenStream as TokenStream2; -use quote::{quote, quote_spanned}; +use quote::{ + quote, + quote_spanned, +}; use syn::spanned::Spanned; /// `Storable` derive implementation for `struct` types. @@ -78,7 +81,7 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { s.ast().span(), "Currently only enums with at most 256 variants are supported.", ) - .to_compile_error(); + .to_compile_error() } let decode_body = s diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index ccb2e47f8f..88145e9e60 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -19,21 +19,11 @@ //! This mapping doesn't actually "own" any data. //! Instead it is just a simple wrapper around the contract storage facilities. -use crate::traits::{ - AutoKey, - Packed, - StorableHint, - StorageKey, -}; +use crate::traits::{AutoKey, Packed, StorableHint, StorageKey}; use core::marker::PhantomData; use ink_primitives::Key; use ink_storage_traits::Storable; -use scale::{ - Encode, - Error, - Input, - Output, -}; +use scale::{Encode, Error, Input, Output}; /// A mapping of key-value pairs directly into contract storage. /// @@ -151,7 +141,7 @@ where /// Returns: /// - `Ok(Some(_))` if the value was inserted successfully, containing the size in /// bytes of the pre-existing value at the specified key if any. - /// - `Ok(None)` if the insert was succesfull but there was no pre-existing value. + /// - `Ok(None)` if the insert was successful but there was no pre-existing value. /// - `Err(_)` if the encoded value exceeds the static buffer size. #[inline] pub fn try_insert(&mut self, key: Q, value: &R) -> ink_env::Result> @@ -160,7 +150,7 @@ where R: Storable + scale::EncodeLike, { if ::encoded_size(value) > ink_env::BUFFER_SIZE { - return Err(ink_env::Error::BufferTooSmall) + return Err(ink_env::Error::BufferTooSmall); }; Ok(self.insert(key, value)) @@ -196,7 +186,7 @@ where .expect("targets of less than 32bit pointer size are not supported; qed"); if encoded_length > ink_env::BUFFER_SIZE { - return Some(Err(ink_env::Error::BufferTooSmall)) + return Some(Err(ink_env::Error::BufferTooSmall)); } self.get(key).map(Ok) @@ -246,7 +236,7 @@ where .expect("targets of less than 32bit pointer size are not supported; qed"); if encoded_length > ink_env::BUFFER_SIZE { - return Some(Err(ink_env::Error::BufferTooSmall)) + return Some(Err(ink_env::Error::BufferTooSmall)); } self.take(key).map(Ok) @@ -324,11 +314,7 @@ where #[cfg(feature = "std")] const _: () = { use crate::traits::StorageLayout; - use ink_metadata::layout::{ - Layout, - LayoutKey, - RootLayout, - }; + use ink_metadata::layout::{Layout, LayoutKey, RootLayout}; impl StorageLayout for Mapping where diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index a5a9d82719..5e27f56965 100644 --- a/crates/storage/src/lazy/mod.rs +++ b/crates/storage/src/lazy/mod.rs @@ -173,7 +173,9 @@ where return Err(ink_env::Error::BufferTooSmall) }; - Ok(self.set(value)) + self.set(value); + + Ok(()) } } From 2f524f8b390b6738ceb1f7127e3a81f6184fea74 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Wed, 27 Sep 2023 17:03:06 +0200 Subject: [PATCH 09/22] fmt Signed-off-by: Cyrill Leutwiler --- crates/storage/src/lazy/mapping.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index 88145e9e60..0a1a3c1d2a 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -19,11 +19,21 @@ //! This mapping doesn't actually "own" any data. //! Instead it is just a simple wrapper around the contract storage facilities. -use crate::traits::{AutoKey, Packed, StorableHint, StorageKey}; +use crate::traits::{ + AutoKey, + Packed, + StorableHint, + StorageKey, +}; use core::marker::PhantomData; use ink_primitives::Key; use ink_storage_traits::Storable; -use scale::{Encode, Error, Input, Output}; +use scale::{ + Encode, + Error, + Input, + Output, +}; /// A mapping of key-value pairs directly into contract storage. /// @@ -150,7 +160,7 @@ where R: Storable + scale::EncodeLike, { if ::encoded_size(value) > ink_env::BUFFER_SIZE { - return Err(ink_env::Error::BufferTooSmall); + return Err(ink_env::Error::BufferTooSmall) }; Ok(self.insert(key, value)) @@ -186,7 +196,7 @@ where .expect("targets of less than 32bit pointer size are not supported; qed"); if encoded_length > ink_env::BUFFER_SIZE { - return Some(Err(ink_env::Error::BufferTooSmall)); + return Some(Err(ink_env::Error::BufferTooSmall)) } self.get(key).map(Ok) @@ -236,7 +246,7 @@ where .expect("targets of less than 32bit pointer size are not supported; qed"); if encoded_length > ink_env::BUFFER_SIZE { - return Some(Err(ink_env::Error::BufferTooSmall)); + return Some(Err(ink_env::Error::BufferTooSmall)) } self.take(key).map(Ok) @@ -314,7 +324,11 @@ where #[cfg(feature = "std")] const _: () = { use crate::traits::StorageLayout; - use ink_metadata::layout::{Layout, LayoutKey, RootLayout}; + use ink_metadata::layout::{ + Layout, + LayoutKey, + RootLayout, + }; impl StorageLayout for Mapping where From fdfe5e255fe50166e4524f6b593a2cb8477a406f Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 29 Sep 2023 16:07:14 +0200 Subject: [PATCH 10/22] checked_add Signed-off-by: Cyrill Leutwiler --- crates/ink/macro/src/storage/storable.rs | 25 ++++--- crates/ink/macro/src/tests/storable.rs | 88 +++++++++++++++++------- 2 files changed, 78 insertions(+), 35 deletions(-) diff --git a/crates/ink/macro/src/storage/storable.rs b/crates/ink/macro/src/storage/storable.rs index 79370dad03..adff8650c4 100644 --- a/crates/ink/macro/src/storage/storable.rs +++ b/crates/ink/macro/src/storage/storable.rs @@ -13,10 +13,7 @@ // limitations under the License. use proc_macro2::TokenStream as TokenStream2; -use quote::{ - quote, - quote_spanned, -}; +use quote::{quote, quote_spanned}; use syn::spanned::Spanned; /// `Storable` derive implementation for `struct` types. @@ -39,7 +36,9 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { let encoded_size_body = variant.each(|binding| { let span = binding.ast().ty.span(); quote_spanned!(span => - encoded_size += ::ink::storage::traits::Storable::encoded_size(#binding); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(#binding)) + .unwrap(); ) }); @@ -61,7 +60,7 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { #[allow(non_camel_case_types)] #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size = 0; + let mut encoded_size: ::core::primitive::usize = 0; match self { #encoded_size_body } encoded_size } @@ -81,7 +80,7 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { s.ast().span(), "Currently only enums with at most 256 variants are supported.", ) - .to_compile_error() + .to_compile_error(); } let decode_body = s @@ -130,12 +129,18 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { let fields = variant.bindings().iter().map(|field| { let span = field.ast().ty.span(); quote_spanned!(span => - encoded_size += ::ink::storage::traits::Storable::encoded_size(#field); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(#field)) + .unwrap(); ) }); quote! { #pat => { - { encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&#index); } + { + encoded_size = encoded_size + .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&#index)) + .unwrap(); + } #( { #fields } )* @@ -170,7 +175,7 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { #[allow(non_camel_case_types)] #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size = 0; + let mut encoded_size: ::core::primitive::usize = 0; match self { #( #encoded_size_body diff --git a/crates/ink/macro/src/tests/storable.rs b/crates/ink/macro/src/tests/storable.rs index 0ebd5cb12b..0ab2811fb9 100644 --- a/crates/ink/macro/src/tests/storable.rs +++ b/crates/ink/macro/src/tests/storable.rs @@ -49,7 +49,7 @@ fn unit_struct_works() { #[allow(non_camel_case_types)] #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size = 0; + let mut encoded_size: ::core::primitive::usize = 0; match self { UnitStruct => {} } @@ -122,17 +122,23 @@ fn struct_works() { #[allow (non_camel_case_types)] #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size = 0; + let mut encoded_size: ::core::primitive::usize = 0; match self { NamedFields { a : __binding_0 , b : __binding_1 , d : __binding_2 , } => { { - encoded_size += ::ink::storage:: traits:: Storable:: encoded_size(__binding_0); + encoded_size = encoded_size + .checked_add(::ink::storage:: traits:: Storable:: encoded_size(__binding_0)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_2); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_2)) + .unwrap(); } } } @@ -186,11 +192,13 @@ fn one_variant_enum_works() { #[allow(non_camel_case_types)] #[allow(unused_mut)] fn encoded_size (&self) -> ::core::primitive::usize { - let mut encoded_size = 0; + let mut encoded_size: ::core::primitive::usize = 0; match self { OneVariantEnum::A => { { - encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8); + encoded_size = encoded_size + .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8)) + .unwrap(); } } } @@ -296,33 +304,47 @@ fn enum_works() { #[allow(non_camel_case_types)] #[allow(unused_mut)] fn encoded_size (&self) -> ::core::primitive::usize{ - let mut encoded_size = 0; + let mut encoded_size: ::core::primitive::usize = 0; match self { MixedEnum::A => { { - encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8); + encoded_size = encoded_size + .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8)) + .unwrap(); } } MixedEnum::B(__binding_0, __binding_1,) => { { - encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&1u8); + encoded_size = encoded_size + .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&1u8)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + .unwrap(); } } MixedEnum::C { a: __binding_0, b: __binding_1, } => { { - encoded_size += <::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&2u8); + encoded_size = encoded_size + .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&2u8)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + .unwrap(); } } } @@ -399,14 +421,18 @@ fn generic_struct_works() { #[allow(non_camel_case_types)] #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size = 0; + let mut encoded_size: ::core::primitive::usize = 0; match self { GenericStruct { a : __binding_0 , b : __binding_1 , } => { { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + .unwrap(); } } } @@ -501,28 +527,40 @@ fn generic_enum_works() { #[allow(non_camel_case_types)] #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size = 0; + let mut encoded_size: ::core::primitive::usize = 0; match self { GenericEnum::Tuple (__binding_0, __binding_1,) => { { - encoded_size += <::core::primitive:: u8 as::ink::storage::traits::Storable>::encoded_size(&0u8); + encoded_size = encoded_size + .checked_add(<::core::primitive:: u8 as::ink::storage::traits::Storable>::encoded_size(&0u8)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + .unwrap(); } } GenericEnum::Named { a: __binding_0, b: __binding_1,} => { { - encoded_size += <::core::primitive::u8 as::ink::storage::traits::Storable>::encoded_size(&1u8); + encoded_size = encoded_size + .checked_add(<::core::primitive::u8 as::ink::storage::traits::Storable>::encoded_size(&1u8)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_0); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .unwrap(); } { - encoded_size += ::ink::storage::traits::Storable::encoded_size(__binding_1); + encoded_size = encoded_size + .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + .unwrap(); } } } From 265660943a8738c170c9aaf1ca7c12b8c8b7f4b5 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 29 Sep 2023 16:41:31 +0200 Subject: [PATCH 11/22] fmrt Signed-off-by: Cyrill Leutwiler --- crates/ink/macro/src/storage/storable.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/ink/macro/src/storage/storable.rs b/crates/ink/macro/src/storage/storable.rs index adff8650c4..8bd77e4c01 100644 --- a/crates/ink/macro/src/storage/storable.rs +++ b/crates/ink/macro/src/storage/storable.rs @@ -13,7 +13,10 @@ // limitations under the License. use proc_macro2::TokenStream as TokenStream2; -use quote::{quote, quote_spanned}; +use quote::{ + quote, + quote_spanned, +}; use syn::spanned::Spanned; /// `Storable` derive implementation for `struct` types. @@ -80,7 +83,7 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { s.ast().span(), "Currently only enums with at most 256 variants are supported.", ) - .to_compile_error(); + .to_compile_error() } let decode_body = s From a2c991b24c879a7aface6f7b382be7a9092585f7 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Mon, 23 Oct 2023 14:27:04 +0200 Subject: [PATCH 12/22] saturating_add prevents panicking Signed-off-by: Cyrill Leutwiler --- crates/ink/macro/src/storage/storable.rs | 9 ++-- crates/ink/macro/src/tests/storable.rs | 57 ++++++++---------------- crates/storage/src/lazy/mapping.rs | 12 +++++ crates/storage/src/lazy/mod.rs | 8 ++++ 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/crates/ink/macro/src/storage/storable.rs b/crates/ink/macro/src/storage/storable.rs index 8bd77e4c01..b2d18a3f33 100644 --- a/crates/ink/macro/src/storage/storable.rs +++ b/crates/ink/macro/src/storage/storable.rs @@ -40,8 +40,7 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { let span = binding.ast().ty.span(); quote_spanned!(span => encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(#binding)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(#binding)); ) }); @@ -133,16 +132,14 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { let span = field.ast().ty.span(); quote_spanned!(span => encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(#field)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(#field)); ) }); quote! { #pat => { { encoded_size = encoded_size - .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&#index)) - .unwrap(); + .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&#index)); } #( { #fields } diff --git a/crates/ink/macro/src/tests/storable.rs b/crates/ink/macro/src/tests/storable.rs index 0ab2811fb9..55713f86cb 100644 --- a/crates/ink/macro/src/tests/storable.rs +++ b/crates/ink/macro/src/tests/storable.rs @@ -127,18 +127,15 @@ fn struct_works() { NamedFields { a : __binding_0 , b : __binding_1 , d : __binding_2 , } => { { encoded_size = encoded_size - .checked_add(::ink::storage:: traits:: Storable:: encoded_size(__binding_0)) - .unwrap(); + .saturating_add(::ink::storage:: traits:: Storable:: encoded_size(__binding_0)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_2)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_2)); } } } @@ -197,8 +194,7 @@ fn one_variant_enum_works() { OneVariantEnum::A => { { encoded_size = encoded_size - .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8)) - .unwrap(); + .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8)); } } } @@ -309,42 +305,35 @@ fn enum_works() { MixedEnum::A => { { encoded_size = encoded_size - .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8)) - .unwrap(); + .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8)); } } MixedEnum::B(__binding_0, __binding_1,) => { { encoded_size = encoded_size - .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&1u8)) - .unwrap(); + .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&1u8)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); } } MixedEnum::C { a: __binding_0, b: __binding_1, } => { { encoded_size = encoded_size - .checked_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&2u8)) - .unwrap(); + .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&2u8)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); } } } @@ -426,13 +415,11 @@ fn generic_struct_works() { GenericStruct { a : __binding_0 , b : __binding_1 , } => { { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); } } } @@ -532,35 +519,29 @@ fn generic_enum_works() { GenericEnum::Tuple (__binding_0, __binding_1,) => { { encoded_size = encoded_size - .checked_add(<::core::primitive:: u8 as::ink::storage::traits::Storable>::encoded_size(&0u8)) - .unwrap(); + .saturating_add(<::core::primitive:: u8 as::ink::storage::traits::Storable>::encoded_size(&0u8)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); } } GenericEnum::Named { a: __binding_0, b: __binding_1,} => { { encoded_size = encoded_size - .checked_add(<::core::primitive::u8 as::ink::storage::traits::Storable>::encoded_size(&1u8)) - .unwrap(); + .saturating_add(<::core::primitive::u8 as::ink::storage::traits::Storable>::encoded_size(&1u8)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); } { encoded_size = encoded_size - .checked_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) - .unwrap(); + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); } } } diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index 0a1a3c1d2a..72fd9e2cf7 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -135,6 +135,10 @@ where /// Insert the given `value` to the contract storage. /// /// Returns the size in bytes of the pre-existing value at the specified key if any. + /// + /// # Panics + /// + /// Traps if the encoded `value` doesn't fit into the static buffer. #[inline] pub fn insert(&mut self, key: Q, value: &R) -> Option where @@ -169,6 +173,10 @@ where /// Get the `value` at `key` from the contract storage. /// /// Returns `None` if no `value` exists at the given `key`. + /// + /// # Panics + /// + /// Traps if the encoded `value` doesn't fit into the static buffer. #[inline] pub fn get(&self, key: Q) -> Option where @@ -207,6 +215,10 @@ where /// /// Returns `None` if no `value` exists at the given `key`. /// + /// # Panics + /// + /// Traps if the encoded `value` doesn't fit into the static buffer. + /// /// # Warning /// /// This method uses the diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index 5e27f56965..058c975c0e 100644 --- a/crates/storage/src/lazy/mod.rs +++ b/crates/storage/src/lazy/mod.rs @@ -135,6 +135,10 @@ where KeyType: StorageKey, { /// Reads the `value` from the contract storage, if it exists. + /// + /// # Panics + /// + /// Traps if the encoded `value` doesn't fit into the static buffer. pub fn get(&self) -> Option { match ink_env::get_contract_storage::(&KeyType::KEY) { Ok(Some(value)) => Some(value), @@ -161,6 +165,10 @@ where } /// Writes the given `value` to the contract storage. + /// + /// # Panics + /// + /// Traps if the encoded `value` doesn't fit into the static buffer. pub fn set(&mut self, value: &V) { ink_env::set_contract_storage::(&KeyType::KEY, value); } From ecb504c8ac4f55e803453e986e2ae6fc4413b7b1 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Wed, 25 Oct 2023 11:39:52 +0200 Subject: [PATCH 13/22] fix ui test Signed-off-by: Cyrill Leutwiler --- .../ink/tests/ui/storage_item/pass/argument_derive_false.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ink/tests/ui/storage_item/pass/argument_derive_false.rs b/crates/ink/tests/ui/storage_item/pass/argument_derive_false.rs index 6e4e83caa5..336ad249f5 100644 --- a/crates/ink/tests/ui/storage_item/pass/argument_derive_false.rs +++ b/crates/ink/tests/ui/storage_item/pass/argument_derive_false.rs @@ -23,6 +23,10 @@ impl Storable for Contract { c: Default::default(), }) } + + fn encoded_size(&self) -> usize { + 2 + 8 + 16 + } } fn main() { From c6565cc4f636af3ab176815dbb023b87785377cd Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Thu, 26 Oct 2023 09:39:07 +0200 Subject: [PATCH 14/22] use chained expressions to calculate the encoded size Signed-off-by: Cyrill Leutwiler --- crates/ink/macro/src/storage/storable.rs | 38 +++---- crates/ink/macro/src/tests/storable.rs | 121 +++++------------------ 2 files changed, 33 insertions(+), 126 deletions(-) diff --git a/crates/ink/macro/src/storage/storable.rs b/crates/ink/macro/src/storage/storable.rs index b2d18a3f33..e94d68ec1e 100644 --- a/crates/ink/macro/src/storage/storable.rs +++ b/crates/ink/macro/src/storage/storable.rs @@ -36,13 +36,13 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { ::ink::storage::traits::Storable::encode(#binding, __dest); ) }); - let encoded_size_body = variant.each(|binding| { - let span = binding.ast().ty.span(); - quote_spanned!(span => - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(#binding)); - ) - }); + let encoded_size_body = + variant.fold(quote!(::core::primitive::usize::MIN), |acc, binding| { + let span = binding.ast().ty.span(); + quote_spanned!(span => + #acc.saturating_add(::ink::storage::traits::Storable::encoded_size(#binding)) + ) + }); s.gen_impl(quote! { gen impl ::ink::storage::traits::Storable for @Self { @@ -60,11 +60,8 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { #[inline(always)] #[allow(non_camel_case_types)] - #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size: ::core::primitive::usize = 0; match self { #encoded_size_body } - encoded_size } } }) @@ -125,26 +122,16 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { } }); - let encoded_size_body = s.variants().iter().enumerate().map(|(index, variant)| { + let encoded_size_body = s.variants().iter().map(|variant| { let pat = variant.pat(); - let index = index as u8; - let fields = variant.bindings().iter().map(|field| { + let field = variant.bindings().iter().fold(quote!(1usize), |acc, field| { let span = field.ast().ty.span(); quote_spanned!(span => - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(#field)); + #acc.saturating_add(::ink::storage::traits::Storable::encoded_size(#field)) ) }); quote! { - #pat => { - { - encoded_size = encoded_size - .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&#index)); - } - #( - { #fields } - )* - } + #pat => { #field } } }); @@ -173,15 +160,12 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { #[inline(always)] #[allow(non_camel_case_types)] - #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size: ::core::primitive::usize = 0; match self { #( #encoded_size_body )* } - encoded_size } } }) diff --git a/crates/ink/macro/src/tests/storable.rs b/crates/ink/macro/src/tests/storable.rs index 55713f86cb..50a96ecba2 100644 --- a/crates/ink/macro/src/tests/storable.rs +++ b/crates/ink/macro/src/tests/storable.rs @@ -47,13 +47,10 @@ fn unit_struct_works() { #[inline(always)] #[allow(non_camel_case_types)] - #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size: ::core::primitive::usize = 0; match self { - UnitStruct => {} + UnitStruct => { ::core::primitive::usize::MIN } } - encoded_size } } }; @@ -120,26 +117,15 @@ fn struct_works() { #[inline (always)] #[allow (non_camel_case_types)] - #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size: ::core::primitive::usize = 0; match self { NamedFields { a : __binding_0 , b : __binding_1 , d : __binding_2 , } => { - { - encoded_size = encoded_size - .saturating_add(::ink::storage:: traits:: Storable:: encoded_size(__binding_0)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_2)); - } + ::core::primitive::usize::MIN + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_2)) } } - encoded_size } } }; @@ -187,18 +173,10 @@ fn one_variant_enum_works() { #[inline(always)] #[allow(non_camel_case_types)] - #[allow(unused_mut)] fn encoded_size (&self) -> ::core::primitive::usize { - let mut encoded_size: ::core::primitive::usize = 0; match self { - OneVariantEnum::A => { - { - encoded_size = encoded_size - .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8)); - } - } + OneVariantEnum::A => { 1usize } } - encoded_size } } }; @@ -298,46 +276,20 @@ fn enum_works() { #[inline(always)] #[allow(non_camel_case_types)] - #[allow(unused_mut)] fn encoded_size (&self) -> ::core::primitive::usize{ - let mut encoded_size: ::core::primitive::usize = 0; match self { - MixedEnum::A => { - { - encoded_size = encoded_size - .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&0u8)); - } - } + MixedEnum::A => { 1usize } MixedEnum::B(__binding_0, __binding_1,) => { - { - encoded_size = encoded_size - .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&1u8)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); - } + 1usize + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) } MixedEnum::C { a: __binding_0, b: __binding_1, } => { - { - encoded_size = encoded_size - .saturating_add(<::core::primitive::u8 as ::ink::storage::traits::Storable>::encoded_size(&2u8)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); - } + 1usize + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) } } - encoded_size } } }; @@ -408,22 +360,14 @@ fn generic_struct_works() { #[inline(always)] #[allow(non_camel_case_types)] - #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size: ::core::primitive::usize = 0; match self { GenericStruct { a : __binding_0 , b : __binding_1 , } => { - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); - } + ::core::primitive::usize::MIN + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) } } - encoded_size } } }; @@ -512,40 +456,19 @@ fn generic_enum_works() { #[inline(always)] #[allow(non_camel_case_types)] - #[allow(unused_mut)] fn encoded_size(&self) -> ::core::primitive::usize { - let mut encoded_size: ::core::primitive::usize = 0; match self { GenericEnum::Tuple (__binding_0, __binding_1,) => { - { - encoded_size = encoded_size - .saturating_add(<::core::primitive:: u8 as::ink::storage::traits::Storable>::encoded_size(&0u8)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); - } + 1usize + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) } GenericEnum::Named { a: __binding_0, b: __binding_1,} => { - { - encoded_size = encoded_size - .saturating_add(<::core::primitive::u8 as::ink::storage::traits::Storable>::encoded_size(&1u8)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)); - } - { - encoded_size = encoded_size - .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)); - } + 1usize + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) } } - encoded_size } } }; From 44de2cffff60f2710460924f39831e4803903c37 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 27 Oct 2023 10:09:29 +0200 Subject: [PATCH 15/22] the key size adds to the encoded size too for getters Signed-off-by: Cyrill Leutwiler --- crates/storage/src/lazy/mapping.rs | 35 +++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index 72fd9e2cf7..ac8061f896 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -163,9 +163,12 @@ where Q: scale::EncodeLike, R: Storable + scale::EncodeLike, { - if ::encoded_size(value) > ink_env::BUFFER_SIZE { + let size_key = ::encoded_size(&key); + let size_value = ::encoded_size(value); + + if size_key.saturating_add(size_value) > ink_env::BUFFER_SIZE { return Err(ink_env::Error::BufferTooSmall) - }; + } Ok(self.insert(key, value)) } @@ -198,12 +201,18 @@ where where Q: scale::EncodeLike, { - let encoded_length: usize = + let key_size = ::encoded_size(&key); + + if key_size > ink_env::BUFFER_SIZE { + return Some(Err(ink_env::Error::BufferTooSmall)) + } + + let value_size: usize = ink_env::contains_contract_storage(&(&KeyType::KEY, &key))? .try_into() .expect("targets of less than 32bit pointer size are not supported; qed"); - if encoded_length > ink_env::BUFFER_SIZE { + if key_size.saturating_add(value_size) > ink_env::BUFFER_SIZE { return Some(Err(ink_env::Error::BufferTooSmall)) } @@ -252,12 +261,18 @@ where where Q: scale::EncodeLike, { - let encoded_length: usize = + let key_size = ::encoded_size(&key); + + if key_size > ink_env::BUFFER_SIZE { + return Some(Err(ink_env::Error::BufferTooSmall)) + } + + let value_size: usize = ink_env::contains_contract_storage(&(&KeyType::KEY, &key))? .try_into() .expect("targets of less than 32bit pointer size are not supported; qed"); - if encoded_length > ink_env::BUFFER_SIZE { + if key_size.saturating_add(value_size) > ink_env::BUFFER_SIZE { return Some(Err(ink_env::Error::BufferTooSmall)) } @@ -463,10 +478,10 @@ mod tests { #[test] fn fallible_storage_works_for_fitting_data() { ink_env::test::run_test::(|_| { - let mut mapping: Mapping = Mapping::new(); + let mut mapping: Mapping = Mapping::new(); let key = 0; - let value = [0u8; ink_env::BUFFER_SIZE]; + let value = [0u8; ink_env::BUFFER_SIZE - 1]; assert_eq!(mapping.try_insert(key, &value), Ok(None)); assert_eq!(mapping.try_get(key), Some(Ok(value))); @@ -481,10 +496,10 @@ mod tests { #[test] fn fallible_storage_fails_gracefully_for_overgrown_data() { ink_env::test::run_test::(|_| { - let mut mapping: Mapping = Mapping::new(); + let mut mapping: Mapping = Mapping::new(); let key = 0; - let value = [0u8; ink_env::BUFFER_SIZE + 1]; + let value = [0u8; ink_env::BUFFER_SIZE]; assert_eq!(mapping.try_get(0), None); assert_eq!( From da9681d69955a134c6f46458411873a66c81d6fb Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 27 Oct 2023 11:16:53 +0200 Subject: [PATCH 16/22] exercise the fallible storage methods in the mapping integration test Signed-off-by: Cyrill Leutwiler --- .../.cargo/config.toml | 3 + .../mapping-integration-tests/Cargo.toml | 2 +- .../mapping-integration-tests/lib.rs | 103 +++++++++++++++++- 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 integration-tests/mapping-integration-tests/.cargo/config.toml diff --git a/integration-tests/mapping-integration-tests/.cargo/config.toml b/integration-tests/mapping-integration-tests/.cargo/config.toml new file mode 100644 index 0000000000..856df7a1e7 --- /dev/null +++ b/integration-tests/mapping-integration-tests/.cargo/config.toml @@ -0,0 +1,3 @@ +[env] +# Makes testing the fallible storage methods more efficient +INK_STATIC_BUFFER_SIZE = "256" \ No newline at end of file diff --git a/integration-tests/mapping-integration-tests/Cargo.toml b/integration-tests/mapping-integration-tests/Cargo.toml index 48bfc1392f..f266e6b3c2 100755 --- a/integration-tests/mapping-integration-tests/Cargo.toml +++ b/integration-tests/mapping-integration-tests/Cargo.toml @@ -20,4 +20,4 @@ std = [ "ink/std", ] ink-as-dependency = [] -e2e-tests = [] +e2e-tests = [] \ No newline at end of file diff --git a/integration-tests/mapping-integration-tests/lib.rs b/integration-tests/mapping-integration-tests/lib.rs index d8bad79de7..6a73c89469 100755 --- a/integration-tests/mapping-integration-tests/lib.rs +++ b/integration-tests/mapping-integration-tests/lib.rs @@ -4,14 +4,23 @@ #[ink::contract] mod mapping_integration_tests { + use ink::prelude::{string::String, vec::Vec}; use ink::storage::Mapping; + #[derive(Debug, PartialEq)] + #[ink::scale_derive(Encode, Decode, TypeInfo)] + pub enum ContractError { + ValueTooLarge, + } + /// A contract for testing `Mapping` functionality. #[ink(storage)] #[derive(Default)] pub struct Mappings { /// Mapping from owner to number of owned token. balances: Mapping, + /// Mapping from owner to aliases. + names: Mapping>, } impl Mappings { @@ -21,7 +30,8 @@ mod mapping_integration_tests { #[ink(constructor)] pub fn new() -> Self { let balances = Mapping::default(); - Self { balances } + let names = Mapping::default(); + Self { balances, names } } /// Demonstrates the usage of `Mapping::get()`. @@ -84,6 +94,53 @@ mod mapping_integration_tests { let caller = Self::env().caller(); self.balances.take(caller) } + + /// Demonstrates the usage of `Mappings::try_take()` and `Mappings::try_insert()`. + /// + /// Adds a name of a given account. + /// + /// Returns `Ok(None)` if the account is not in the `Mapping`. + /// Returns `Ok(Some(_))` if the account was already in the `Mapping` + /// Returns `Err(_)` if the mapping value couldn't be encoded. + #[ink(message)] + pub fn try_insert_name(&mut self, name: String) -> Result<(), ContractError> { + let caller = Self::env().caller(); + let mut names = match self.names.try_take(caller) { + None => Vec::new(), + Some(value) => value.map_err(|_| ContractError::ValueTooLarge)?, + }; + ink::env::debug_println!("names: {:?}", &names); + + ink::env::debug_println!("name len: {}", name.len()); + ink::env::debug_println!("names len: {}", names.len()); + names.push(name); + use ink::storage::traits::Storable; + ink::env::debug_println!( + "encoded_size: {}", + as Storable>::encoded_size(&names) + ); + + self.names + .try_insert(caller, &names) + .map_err(|_| ContractError::ValueTooLarge)?; + + Ok(()) + } + + /// Demonstrates the usage of `Mappings::try_get()`. + /// + /// Returns the name of a given account. + /// + /// Returns `Ok(None)` if the account is not in the `Mapping`. + /// Returns `Ok(Some(_))` if the account was already in the `Mapping` + /// Returns `Err(_)` if the mapping value couldn't be encoded. + #[ink(message)] + pub fn try_get_names(&mut self) -> Option, ContractError>> { + let caller = Self::env().caller(); + self.names + .try_get(caller) + .map(|result| result.map_err(|_| ContractError::ValueTooLarge)) + } } #[cfg(all(test, feature = "e2e-tests"))] @@ -313,5 +370,49 @@ mod mapping_integration_tests { Ok(()) } + + #[ink_e2e::test] + async fn fallible_storage_methods_work( + mut client: Client, + ) -> E2EResult<()> { + // given + let mut constructor = MappingsRef::new(); + let contract = client + .instantiate( + "mapping-integration-tests", + &ink_e2e::ferdie(), + &mut constructor, + ) + .submit() + .await + .expect("instantiate failed"); + let mut call = contract.call::(); + + // when the mapping value overgrows the buffer + let name = ink_e2e::ferdie().public_key().to_account_id().to_string(); + let insert = call.try_insert_name(name.clone()); + while let Ok(_) = client.call(&ink_e2e::ferdie(), &insert).submit().await {} + + // then adding another one should fail gracefully + let expected_insert_result = client + .call(&ink_e2e::ferdie(), &insert) + .dry_run() + .await + .return_value(); + let received_insert_result = + Err(crate::mapping_integration_tests::ContractError::ValueTooLarge); + assert_eq!(received_insert_result, expected_insert_result); + + // then there should be 4 entries (that's what fits into the 256kb buffer) + let received_mapping_value = client + .call(&ink_e2e::ferdie(), &call.try_get_names()) + .dry_run() + .await + .return_value(); + let expected_mapping_value = Some(Ok(vec![name.clone(); 4])); + assert_eq!(received_mapping_value, expected_mapping_value); + + Ok(()) + } } } From 8f8f7eb0064a4b65fe9995860d097c76378f8f4b Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 27 Oct 2023 11:21:34 +0200 Subject: [PATCH 17/22] cleanup integration test Signed-off-by: Cyrill Leutwiler --- .../mapping-integration-tests/.cargo/config.toml | 2 +- integration-tests/mapping-integration-tests/Cargo.toml | 2 +- integration-tests/mapping-integration-tests/lib.rs | 8 -------- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/integration-tests/mapping-integration-tests/.cargo/config.toml b/integration-tests/mapping-integration-tests/.cargo/config.toml index 856df7a1e7..fcfc9c8888 100644 --- a/integration-tests/mapping-integration-tests/.cargo/config.toml +++ b/integration-tests/mapping-integration-tests/.cargo/config.toml @@ -1,3 +1,3 @@ [env] # Makes testing the fallible storage methods more efficient -INK_STATIC_BUFFER_SIZE = "256" \ No newline at end of file +INK_STATIC_BUFFER_SIZE = "256" diff --git a/integration-tests/mapping-integration-tests/Cargo.toml b/integration-tests/mapping-integration-tests/Cargo.toml index f266e6b3c2..48bfc1392f 100755 --- a/integration-tests/mapping-integration-tests/Cargo.toml +++ b/integration-tests/mapping-integration-tests/Cargo.toml @@ -20,4 +20,4 @@ std = [ "ink/std", ] ink-as-dependency = [] -e2e-tests = [] \ No newline at end of file +e2e-tests = [] diff --git a/integration-tests/mapping-integration-tests/lib.rs b/integration-tests/mapping-integration-tests/lib.rs index 6a73c89469..b37e94d159 100755 --- a/integration-tests/mapping-integration-tests/lib.rs +++ b/integration-tests/mapping-integration-tests/lib.rs @@ -109,16 +109,8 @@ mod mapping_integration_tests { None => Vec::new(), Some(value) => value.map_err(|_| ContractError::ValueTooLarge)?, }; - ink::env::debug_println!("names: {:?}", &names); - ink::env::debug_println!("name len: {}", name.len()); - ink::env::debug_println!("names len: {}", names.len()); names.push(name); - use ink::storage::traits::Storable; - ink::env::debug_println!( - "encoded_size: {}", - as Storable>::encoded_size(&names) - ); self.names .try_insert(caller, &names) From 94c8c7b8dd49ac71c423326794a78130fe4000e5 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 27 Oct 2023 11:28:10 +0200 Subject: [PATCH 18/22] add lazy test for key sizes Signed-off-by: Cyrill Leutwiler --- crates/storage/src/lazy/mapping.rs | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index ac8061f896..5ae762ef05 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -523,4 +523,35 @@ mod tests { }) .unwrap() } + + #[test] + fn fallible_storage_considers_key_size() { + ink_env::test::run_test::(|_| { + let mut mapping: Mapping<[u8; ink_env::BUFFER_SIZE + 1], u8> = Mapping::new(); + + let key = [0u8; ink_env::BUFFER_SIZE + 1]; + let value = 0; + + // Key is already too large, so this should fail anyways. + assert_eq!( + mapping.try_insert(key, &value), + Err(ink_env::Error::BufferTooSmall) + ); + + // The off-chain impl conveniently uses a Vec for encoding, + // allowing writing values exceeding the static buffer size. + ink_env::set_contract_storage(&(&mapping.key(), key), &value); + assert_eq!( + mapping.try_get(key), + Some(Err(ink_env::Error::BufferTooSmall)) + ); + assert_eq!( + mapping.try_take(key), + Some(Err(ink_env::Error::BufferTooSmall)) + ); + + Ok(()) + }) + .unwrap() + } } From 35b045bdbf5d4174c908fcaa66ea48f9d01083c7 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 27 Oct 2023 11:31:42 +0200 Subject: [PATCH 19/22] consistent naming Signed-off-by: Cyrill Leutwiler --- crates/storage/src/lazy/mapping.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index 5ae762ef05..fa92a0fb01 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -163,10 +163,10 @@ where Q: scale::EncodeLike, R: Storable + scale::EncodeLike, { - let size_key = ::encoded_size(&key); - let size_value = ::encoded_size(value); + let key_size = ::encoded_size(&key); + let value_size = ::encoded_size(value); - if size_key.saturating_add(size_value) > ink_env::BUFFER_SIZE { + if key_size.saturating_add(value_size) > ink_env::BUFFER_SIZE { return Err(ink_env::Error::BufferTooSmall) } From 570f3ab6b576eb6904211c6b93b521e891ca0607 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 27 Oct 2023 11:36:45 +0200 Subject: [PATCH 20/22] consider key size in try_insert too Signed-off-by: Cyrill Leutwiler --- crates/storage/src/lazy/mapping.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index fa92a0fb01..22fd15bed8 100644 --- a/crates/storage/src/lazy/mapping.rs +++ b/crates/storage/src/lazy/mapping.rs @@ -164,6 +164,11 @@ where R: Storable + scale::EncodeLike, { let key_size = ::encoded_size(&key); + + if key_size > ink_env::BUFFER_SIZE { + return Err(ink_env::Error::BufferTooSmall) + } + let value_size = ::encoded_size(value); if key_size.saturating_add(value_size) > ink_env::BUFFER_SIZE { From 1ead741d9ee3a20e54a0fa0985df4464c10b3f44 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 27 Oct 2023 11:49:56 +0200 Subject: [PATCH 21/22] fmt Signed-off-by: Cyrill Leutwiler --- integration-tests/mapping-integration-tests/lib.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/integration-tests/mapping-integration-tests/lib.rs b/integration-tests/mapping-integration-tests/lib.rs index b37e94d159..45822a4906 100755 --- a/integration-tests/mapping-integration-tests/lib.rs +++ b/integration-tests/mapping-integration-tests/lib.rs @@ -4,8 +4,13 @@ #[ink::contract] mod mapping_integration_tests { - use ink::prelude::{string::String, vec::Vec}; - use ink::storage::Mapping; + use ink::{ + prelude::{ + string::String, + vec::Vec, + }, + storage::Mapping, + }; #[derive(Debug, PartialEq)] #[ink::scale_derive(Encode, Decode, TypeInfo)] From 080fec01fbb3fbc3bb59614a055e451154f39b66 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Fri, 27 Oct 2023 12:25:30 +0200 Subject: [PATCH 22/22] do not depend on a particular static buffer size in the mapping test Signed-off-by: Cyrill Leutwiler --- integration-tests/mapping-integration-tests/lib.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/integration-tests/mapping-integration-tests/lib.rs b/integration-tests/mapping-integration-tests/lib.rs index 45822a4906..0836e5f245 100755 --- a/integration-tests/mapping-integration-tests/lib.rs +++ b/integration-tests/mapping-integration-tests/lib.rs @@ -388,7 +388,10 @@ mod mapping_integration_tests { // when the mapping value overgrows the buffer let name = ink_e2e::ferdie().public_key().to_account_id().to_string(); let insert = call.try_insert_name(name.clone()); - while let Ok(_) = client.call(&ink_e2e::ferdie(), &insert).submit().await {} + let mut names = Vec::new(); + while let Ok(_) = client.call(&ink_e2e::ferdie(), &insert).submit().await { + names.push(name.clone()) + } // then adding another one should fail gracefully let expected_insert_result = client @@ -406,7 +409,7 @@ mod mapping_integration_tests { .dry_run() .await .return_value(); - let expected_mapping_value = Some(Ok(vec![name.clone(); 4])); + let expected_mapping_value = Some(Ok(names)); assert_eq!(received_mapping_value, expected_mapping_value); Ok(())