diff --git a/crates/env/src/error.rs b/crates/env/src/error.rs index 55a71926f41..977a9116a8b 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/ink/macro/src/storage/storable.rs b/crates/ink/macro/src/storage/storable.rs index 728077e8f63..e94d68ec1ee 100644 --- a/crates/ink/macro/src/storage/storable.rs +++ b/crates/ink/macro/src/storage/storable.rs @@ -36,6 +36,13 @@ fn storable_struct_derive(s: &synstructure::Structure) -> TokenStream2 { ::ink::storage::traits::Storable::encode(#binding, __dest); ) }); + 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 { @@ -50,6 +57,12 @@ 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(non_camel_case_types)] + fn encoded_size(&self) -> ::core::primitive::usize { + match self { #encoded_size_body } + } } }) } @@ -108,6 +121,20 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { } } }); + + let encoded_size_body = s.variants().iter().map(|variant| { + let pat = variant.pat(); + let field = variant.bindings().iter().fold(quote!(1usize), |acc, field| { + let span = field.ast().ty.span(); + quote_spanned!(span => + #acc.saturating_add(::ink::storage::traits::Storable::encoded_size(#field)) + ) + }); + quote! { + #pat => { #field } + } + }); + s.gen_impl(quote! { gen impl ::ink::storage::traits::Storable for @Self { #[inline(always)] @@ -130,6 +157,16 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 { )* } } + + #[inline(always)] + #[allow(non_camel_case_types)] + fn encoded_size(&self) -> ::core::primitive::usize { + match self { + #( + #encoded_size_body + )* + } + } } }) } diff --git a/crates/ink/macro/src/tests/storable.rs b/crates/ink/macro/src/tests/storable.rs index 37ba2bf9bb3..50a96ecba23 100644 --- a/crates/ink/macro/src/tests/storable.rs +++ b/crates/ink/macro/src/tests/storable.rs @@ -44,6 +44,14 @@ fn unit_struct_works() { UnitStruct => { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + fn encoded_size(&self) -> ::core::primitive::usize { + match self { + UnitStruct => { ::core::primitive::usize::MIN } + } + } } }; } @@ -106,6 +114,19 @@ fn struct_works() { } } } + + #[inline (always)] + #[allow (non_camel_case_types)] + fn encoded_size(&self) -> ::core::primitive::usize { + match self { + NamedFields { a : __binding_0 , b : __binding_1 , d : __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)) + } + } + } } }; } @@ -149,6 +170,14 @@ fn one_variant_enum_works() { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + fn encoded_size (&self) -> ::core::primitive::usize { + match self { + OneVariantEnum::A => { 1usize } + } + } } }; } @@ -244,6 +273,24 @@ fn enum_works() { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + fn encoded_size (&self) -> ::core::primitive::usize{ + match self { + MixedEnum::A => { 1usize } + MixedEnum::B(__binding_0, __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, } => { + 1usize + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + } + } + } } }; } @@ -310,6 +357,18 @@ fn generic_struct_works() { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + fn encoded_size(&self) -> ::core::primitive::usize { + match self { + GenericStruct { a : __binding_0 , b : __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)) + } + } + } } }; } @@ -394,6 +453,23 @@ fn generic_enum_works() { } } } + + #[inline(always)] + #[allow(non_camel_case_types)] + fn encoded_size(&self) -> ::core::primitive::usize { + match self { + GenericEnum::Tuple (__binding_0, __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,} => { + 1usize + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_0)) + .saturating_add(::ink::storage::traits::Storable::encoded_size(__binding_1)) + } + } + } } }; } 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 6e4e83caa57..336ad249f52 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() { diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs index 4a9b1db7eb1..22fd15bed85 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 @@ -144,9 +148,43 @@ 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 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> + where + Q: scale::EncodeLike, + 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 { + 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`. + /// + /// # Panics + /// + /// Traps if the encoded `value` doesn't fit into the static buffer. #[inline] pub fn get(&self, key: Q) -> Option where @@ -156,11 +194,45 @@ 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 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 key_size.saturating_add(value_size) > 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. /// /// 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 @@ -175,6 +247,43 @@ 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 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 key_size.saturating_add(value_size) > 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`. @@ -219,6 +328,11 @@ where fn decode(_input: &mut I) -> Result { Ok(Default::default()) } + + #[inline] + fn encoded_size(&self) -> usize { + 0 + } } impl StorableHint for Mapping @@ -365,4 +479,84 @@ 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 - 1]; + + 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]; + + 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() + } + + #[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() + } } diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index d730f541f37..058c975c0ec 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), @@ -142,10 +146,45 @@ where } } + /// Try to read the `value` from the contract storage. + /// + /// 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 { + return Some(Err(ink_env::Error::BufferTooSmall)) + } + + self.get().map(Ok) + } + /// 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); } + + /// 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.encoded_size() > ink_env::BUFFER_SIZE { + return Err(ink_env::Error::BufferTooSmall) + }; + + self.set(value); + + Ok(()) + } } impl Lazy @@ -175,6 +214,11 @@ where fn decode(_input: &mut I) -> Result { Ok(Default::default()) } + + #[inline(always)] + fn encoded_size(&self) -> usize { + 0 + } } impl StorableHint for Lazy @@ -269,4 +313,37 @@ 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_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_eq!(storage.try_get(), Some(Err(ink_env::Error::BufferTooSmall))); + + Ok(()) + }) + .unwrap() + } } diff --git a/crates/storage/traits/src/storage.rs b/crates/storage/traits/src/storage.rs index 05d92373edd..42cd467b035 100644 --- a/crates/storage/traits/src/storage.rs +++ b/crates/storage/traits/src/storage.rs @@ -25,6 +25,9 @@ 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; } /// Types which implement `scale::Encode` and `scale::Decode` are `Storable` by default @@ -42,6 +45,11 @@ where fn decode(input: &mut I) -> Result { scale::Decode::decode(input) } + + #[inline] + fn encoded_size(&self) -> usize { +

::encoded_size(self) + } } /// Decode and consume all of the given input data. 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 00000000000..fcfc9c88880 --- /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" diff --git a/integration-tests/mapping-integration-tests/lib.rs b/integration-tests/mapping-integration-tests/lib.rs index d8bad79de78..0836e5f245a 100755 --- a/integration-tests/mapping-integration-tests/lib.rs +++ b/integration-tests/mapping-integration-tests/lib.rs @@ -4,7 +4,19 @@ #[ink::contract] mod mapping_integration_tests { - use ink::storage::Mapping; + use ink::{ + prelude::{ + string::String, + vec::Vec, + }, + storage::Mapping, + }; + + #[derive(Debug, PartialEq)] + #[ink::scale_derive(Encode, Decode, TypeInfo)] + pub enum ContractError { + ValueTooLarge, + } /// A contract for testing `Mapping` functionality. #[ink(storage)] @@ -12,6 +24,8 @@ mod mapping_integration_tests { pub struct Mappings { /// Mapping from owner to number of owned token. balances: Mapping, + /// Mapping from owner to aliases. + names: Mapping>, } impl Mappings { @@ -21,7 +35,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 +99,45 @@ 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)?, + }; + + names.push(name); + + 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 +367,52 @@ 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()); + 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 + .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(names)); + assert_eq!(received_mapping_value, expected_mapping_value); + + Ok(()) + } } }