Fail when decoding from storage and not all bytes consumed

crates/env/src/api.rs

@@ -41,7 +41,10 @@ use crate::{
     Environment,
     Result,
 };
-use ink_storage_traits::Storable;
+use ink_storage_traits::{
+    Storable,
+    StorableDecodeAll,
+};
 
 /// Returns the address of the caller of the executed contract.
 ///
@@ -210,7 +213,7 @@ where
 pub fn get_contract_storage<K, R>(key: &K) -> Result<Option<R>>
 where
     K: scale::Encode,
-    R: Storable,
+    R: StorableDecodeAll,
 {
     <EnvInstance as OnInstance>::on_instance(|instance| {
         EnvBackend::get_contract_storage::<K, R>(instance, key)
@@ -225,7 +228,7 @@ where
 pub fn take_contract_storage<K, R>(key: &K) -> Result<Option<R>>
 where
     K: scale::Encode,
-    R: Storable,
+    R: StorableDecodeAll,
 {
     <EnvInstance as OnInstance>::on_instance(|instance| {
         EnvBackend::take_contract_storage::<K, R>(instance, key)

crates/env/src/backend.rs

@@ -29,7 +29,10 @@ use crate::{
     Environment,
     Result,
 };
-use ink_storage_traits::Storable;
+use ink_storage_traits::{
+    Storable,
+    StorableDecodeAll,
+};
 
 /// The flags to indicate further information about the end of a contract execution.
 #[derive(Default)]
@@ -186,7 +189,7 @@ pub trait EnvBackend {
     fn get_contract_storage<K, R>(&mut self, key: &K) -> Result<Option<R>>
     where
         K: scale::Encode,
-        R: Storable;
+        R: StorableDecodeAll;
 
     /// Removes the `value` at `key`, returning the previous `value` at `key` from storage
     /// if any.
@@ -197,7 +200,7 @@ pub trait EnvBackend {
     fn take_contract_storage<K, R>(&mut self, key: &K) -> Result<Option<R>>
     where
         K: scale::Encode,
-        R: Storable;
+        R: StorableDecodeAll;
 
     /// Returns the size of a value stored under the given storage key is returned if any.
     fn contains_contract_storage<K>(&mut self, key: &K) -> Option<u32>

crates/env/src/engine/off_chain/impls.rs

@@ -46,7 +46,10 @@ use ink_engine::{
     ext,
     ext::Engine,
 };
-use ink_storage_traits::Storable;
+use ink_storage_traits::{
+    Storable,
+    StorableDecodeAll,
+};
 use schnorrkel::{
     PublicKey,
     Signature,
@@ -200,22 +203,22 @@ impl EnvBackend for EnvInstance {
     fn get_contract_storage<K, R>(&mut self, key: &K) -> Result<Option<R>>
     where
         K: scale::Encode,
-        R: Storable,
+        R: StorableDecodeAll,
     {
         let mut output: [u8; 9600] = [0; 9600];
         match self.engine.get_storage(&key.encode(), &mut &mut output[..]) {
             Ok(_) => (),
             Err(ext::Error::KeyNotFound) => return Ok(None),
             Err(_) => panic!("encountered unexpected error"),
         }
-        let decoded = Storable::decode(&mut &output[..])?;
+        let decoded = StorableDecodeAll::decode_all(&mut &output[..])?;
         Ok(Some(decoded))
     }
 
     fn take_contract_storage<K, R>(&mut self, key: &K) -> Result<Option<R>>
     where
         K: scale::Encode,
-        R: Storable,
+        R: StorableDecodeAll,
     {
         let mut output: [u8; 9600] = [0; 9600];
         match self
@@ -226,7 +229,7 @@ impl EnvBackend for EnvInstance {
             Err(ext::Error::KeyNotFound) => return Ok(None),
             Err(_) => panic!("encountered unexpected error"),
         }
-        let decoded = Storable::decode(&mut &output[..])?;
+        let decoded = StorableDecodeAll::decode_all(&mut &output[..])?;
         Ok(Some(decoded))
     }
 

crates/env/src/engine/on_chain/impls.rs

@@ -48,7 +48,10 @@ use crate::{
     ReturnFlags,
     TypedEnvBackend,
 };
-use ink_storage_traits::Storable;
+use ink_storage_traits::{
+    Storable,
+    StorableDecodeAll,
+};
 
 impl CryptoHash for Blake2x128 {
     fn hash(input: &[u8], output: &mut <Self as HashOutput>::Type) {
@@ -226,7 +229,7 @@ impl EnvBackend for EnvInstance {
     fn get_contract_storage<K, R>(&mut self, key: &K) -> Result<Option<R>>
     where
         K: scale::Encode,
-        R: Storable,
+        R: StorableDecodeAll,
     {
         let mut buffer = self.scoped_buffer();
         let key = buffer.take_encoded(key);
@@ -236,14 +239,14 @@ impl EnvBackend for EnvInstance {
             Err(ExtError::KeyNotFound) => return Ok(None),
             Err(_) => panic!("encountered unexpected error"),
         }
-        let decoded = Storable::decode(&mut &output[..])?;
+        let decoded = StorableDecodeAll::decode_all(&mut &output[..])?;
         Ok(Some(decoded))
     }
 
     fn take_contract_storage<K, R>(&mut self, key: &K) -> Result<Option<R>>
     where
         K: scale::Encode,
-        R: Storable,
+        R: StorableDecodeAll,
     {
         let mut buffer = self.scoped_buffer();
         let key = buffer.take_encoded(key);
@@ -253,7 +256,7 @@ impl EnvBackend for EnvInstance {
             Err(ExtError::KeyNotFound) => return Ok(None),
             Err(_) => panic!("encountered unexpected error"),
         }
-        let decoded = Storable::decode(&mut &output[..])?;
+        let decoded = StorableDecodeAll::decode_all(&mut &output[..])?; // todo: [AJ] test.
         Ok(Some(decoded))
     }
 

crates/storage/src/lazy/mod.rs

@@ -30,7 +30,10 @@ use crate::traits::{
 };
 use core::marker::PhantomData;
 use ink_primitives::Key;
-use ink_storage_traits::Storable;
+use ink_storage_traits::{
+    Storable,
+    StorableDecodeAll,
+};
 use scale::{
     Error,
     Input,
@@ -131,7 +134,7 @@ where
 
 impl<V, KeyType> Lazy<V, KeyType>
 where
-    V: Storable,
+    V: Storable + StorableDecodeAll,
     KeyType: StorageKey,
 {
     /// Reads the `value` from the contract storage, if it exists.
@@ -150,7 +153,7 @@ where
 
 impl<V, KeyType> Lazy<V, KeyType>
 where
-    V: Storable + Default,
+    V: StorableDecodeAll + Default,
     KeyType: StorageKey,
 {
     /// Reads the `value` from the contract storage.

crates/storage/traits/src/lib.rs

@@ -50,6 +50,7 @@ pub use self::{
         AutoStorableHint,
         Packed,
         Storable,
+        StorableDecodeAll,
         StorableHint,
         StorageKey,
     },

crates/storage/traits/src/storage.rs

@@ -44,6 +44,30 @@ where
     }
 }
 
+/// Extension trait to [`Storable`] that ensures that the given input data is consumed
+/// completely.
+pub trait StorableDecodeAll: Sized {
+    /// Decode `Self` and consume all of the given input data.
+    ///
+    /// If not all data is consumed, an error is returned.
+    fn decode_all(input: &mut &[u8]) -> Result<Self, scale::Error>;
+}
+
+impl<P> StorableDecodeAll for P
+where
+    P: Storable,
+{
+    fn decode_all(input: &mut &[u8]) -> Result<Self, scale::Error> {
+        let res = Storable::decode(input)?;
+
+        if input.is_empty() {
+            Ok(res)
+        } else {
+            Err("Input buffer has still data left after decoding!".into())
+        }
+    }
+}
+
 pub(crate) mod private {
     /// Seals the implementation of `Packed`.
     pub trait Sealed {}

integration-tests/contract-storage/.gitignore

@@ -0,0 +1,9 @@
+# Ignore build artifacts from the local tests sub-crate.
+/target/
+
+# Ignore backup files creates by cargo fmt.
+**/*.rs.bk
+
+# Remove Cargo.lock when creating an executable, leave it for libraries
+# More information here http://doc.crates.io/guide.html#cargotoml-vs-cargolock
+Cargo.lock

integration-tests/contract-storage/Cargo.toml

@@ -0,0 +1,23 @@
+[package]
+name = "contract-storage"
+version = "4.2.0"
+authors = ["Parity Technologies <[email protected]>"]
+edition = "2021"
+publish = false
+
+[dependencies]
+ink = { path = "../../crates/ink", default-features = false }
+
+[dev-dependencies]
+ink_e2e = { path = "../../crates/e2e" }
+
+[lib]
+path = "lib.rs"
+
+[features]
+default = ["std"]
+std = [
+    "ink/std",
+]
+ink-as-dependency = []
+e2e-tests = []

integration-tests/contract-storage/e2e_tests.rs

@@ -0,0 +1,63 @@
+use super::contract_storage::*;
+use ink_e2e::ContractsBackend;
+
+type E2EResult<T> = std::result::Result<T, Box<dyn std::error::Error>>;
+
+#[ink_e2e::test]
+async fn get_contract_storage_consumes_entire_buffer<Client: E2EBackend>(
+    mut client: Client,
+) -> E2EResult<()> {
+    // given
+    let constructor = ContractStorageRef::new();
+    let contract = client
+        .instantiate("contract-storage", &ink_e2e::alice(), constructor, 0, None)
+        .await
+        .expect("instantiate failed");
+    let call = contract.call::<ContractStorage>();
+
+    // when
+    let result = client
+        .call(
+            &ink_e2e::alice(),
+            &call.set_and_get_storage_all_data_consumed(),
+            0,
+            None,
+        )
+        .await
+        .expect("Calling `insert_balance` failed")
+        .return_value();
+
+    assert!(result.is_ok());
+
+    Ok(())
+}
+
+#[ink_e2e::test]
+async fn get_contract_storage_fails_when_extra_data<Client: E2EBackend>(
+    mut client: Client,
+) -> E2EResult<()> {
+    // given
+    let constructor = ContractStorageRef::new();
+    let contract = client
+        .instantiate("contract-storage", &ink_e2e::alice(), constructor, 0, None)
+        .await
+        .expect("instantiate failed");
+    let call = contract.call::<ContractStorage>();
+
+    // when
+    let result = client
+        .call(
+            &ink_e2e::alice(),
+            &call.set_and_get_storage_partial_data_consumed(),
+            0,
+            None,
+        )
+        .await;
+
+    assert!(
+        result.is_err(),
+        "Expected the contract to revert when only partially consuming the buffer"
+    );
+
+    Ok(())
+}

integration-tests/contract-storage/lib.rs

@@ -0,0 +1,51 @@
+//! A smart contract to test reading and writing contract storage
+
+#![cfg_attr(not(feature = "std"), no_std, no_main)]
+
+#[ink::contract]
+mod contract_storage {
+    use ink::prelude::{
+        format,
+        string::String,
+    };
+
+    /// A contract for testing reading and writing contract storage.
+    #[ink(storage)]
+    #[derive(Default)]
+    pub struct ContractStorage;
+
+    impl ContractStorage {
+        #[ink(constructor)]
+        pub fn new() -> Self {
+            Self {}
+        }
+
+        /// Read from the contract storage slot, consuming all the data from the buffer.
+        #[ink(message)]
+        pub fn set_and_get_storage_all_data_consumed(&self) -> Result<(), String> {
+            let key = 0u32;
+            let value = [0x42; 32];
+            ink::env::set_contract_storage(&key, &value);
+            let loaded_value = ink::env::get_contract_storage(&key)
+                .map_err(|e| format!("get_contract_storage failed: {:?}", e))?;
+            assert_eq!(loaded_value, Some(value));
+            Ok(())
+        }
+
+        /// Read from the contract storage slot, only partially consuming data from the
+        /// buffer.
+        #[ink(message)]
+        pub fn set_and_get_storage_partial_data_consumed(&self) -> Result<(), String> {
+            let key = 0u32;
+            let value = [0x42; 32];
+            ink::env::set_contract_storage(&key, &value);
+            // Only attempt to read the first byte (the `u8`) of the storage value data
+            let _loaded_value: Option<u8> = ink::env::get_contract_storage(&key)
+                .map_err(|e| format!("get_contract_storage failed: {:?}", e))?;
+            Ok(())
+        }
+    }
+}
+
+#[cfg(all(test, feature = "e2e-tests"))]
+mod e2e_tests;