fixes on feedback applied + make it build

frame/contracts/src/exec.rs

@@ -24,7 +24,6 @@ use crate::{
 use frame_support::{
 	crypto::ecdsa::ECDSAExt,
 	dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable},
-	pallet_prelude::ConstU32,
 	storage::{with_transaction, TransactionOutcome},
 	traits::{Contains, Currency, ExistenceRequirement, OriginTrait, Randomness, Time},
 	weights::Weight,
@@ -50,17 +49,23 @@ pub type TopicOf<T> = <T as frame_system::Config>::Hash;
 pub type FixSizedKey = [u8; 32];
 pub type VarSizedKey<T> = BoundedVec<u8, <T as Config>::MaxStorageKeyLen>;
 
-pub trait StorageHash {
+pub trait StorageHash<T>
+where
+	T: Config,
+{
 	fn hash(self) -> Vec<u8>;
 }
 
-impl StorageHash for FixSizedKey {
+impl<T: Config> StorageHash<T> for FixSizedKey {
 	fn hash(self) -> Vec<u8> {
 		blake2_256(self.as_slice()).to_vec()
 	}
 }
 
-impl StorageHash for VarSizedKey {
+impl<T> StorageHash<T> for VarSizedKey<T>
+where
+	T: Config,
+{
 	fn hash(self) -> Vec<u8> {
 		Blake2_128Concat::hash(self.as_slice())
 	}
@@ -168,7 +173,7 @@ pub trait Ext: sealing::Sealed {
 	///
 	/// Returns `None` if the `key` wasn't previously set by `set_storage` or
 	/// was deleted.
-	fn get_storage_transparent(&mut self, key: VarSizedKey) -> Option<Vec<u8>>;
+	fn get_storage_transparent(&mut self, key: VarSizedKey<Self::T>) -> Option<Vec<u8>>;
 
 	/// Returns `Some(len)` (in bytes) if a storage item exists at `key`.
 	///
@@ -182,7 +187,7 @@ pub trait Ext: sealing::Sealed {
 	///
 	/// Returns `None` if the `key` wasn't previously set by `set_storage` or
 	/// was deleted.
-	fn get_storage_size_transparent(&mut self, key: VarSizedKey) -> Option<u32>;
+	fn get_storage_size_transparent(&mut self, key: VarSizedKey<Self::T>) -> Option<u32>;
 
 	/// Sets the storage entry by the given key to the specified value. If `value` is `None` then
 	/// the storage entry is deleted.
@@ -196,7 +201,7 @@ pub trait Ext: sealing::Sealed {
 	///
 	fn set_storage_transparent(
 		&mut self,
-		key: VarSizedKey,
+		key: VarSizedKey<Self::T>,
 		value: Option<Vec<u8>>,
 		take_old: bool,
 	) -> Result<WriteOutcome, DispatchError>;
@@ -1133,15 +1138,15 @@ where
 		Storage::<T>::read(&self.top_frame_mut().contract_info().trie_id, key)
 	}
 
-	fn get_storage_transparent(&mut self, key: VarSizedKey) -> Option<Vec<u8>> {
+	fn get_storage_transparent(&mut self, key: VarSizedKey<T>) -> Option<Vec<u8>> {
 		Storage::<T>::read(&self.top_frame_mut().contract_info().trie_id, key)
 	}
 
 	fn get_storage_size(&mut self, key: FixSizedKey) -> Option<u32> {
 		Storage::<T>::size(&self.top_frame_mut().contract_info().trie_id, key)
 	}
 
-	fn get_storage_size_transparent(&mut self, key: VarSizedKey) -> Option<u32> {
+	fn get_storage_size_transparent(&mut self, key: VarSizedKey<T>) -> Option<u32> {
 		Storage::<T>::size(&self.top_frame_mut().contract_info().trie_id, key)
 	}
 
@@ -1163,7 +1168,7 @@ where
 
 	fn set_storage_transparent(
 		&mut self,
-		key: VarSizedKey,
+		key: VarSizedKey<T>,
 		value: Option<Vec<u8>>,
 		take_old: bool,
 	) -> Result<WriteOutcome, DispatchError> {

frame/contracts/src/lib.rs

@@ -932,7 +932,7 @@ where
 	}
 
 	/// Query storage of a specified contract under a specified key.
-	pub fn get_storage<K: StorageHash>(address: T::AccountId, key: K) -> GetStorageResult {
+	pub fn get_storage<K: StorageHash<T>>(address: T::AccountId, key: K) -> GetStorageResult {
 		let contract_info =
 			ContractInfoOf::<T>::get(&address).ok_or(ContractAccessError::DoesntExist)?;
 

frame/contracts/src/storage.rs

@@ -123,15 +123,15 @@ where
 	///
 	/// The read is performed from the `trie_id` only. The `address` is not necessary. If the
 	/// contract doesn't store under the given `key` `None` is returned.
-	pub fn read<K: StorageHash>(trie_id: &TrieId, key: K) -> Option<Vec<u8>> {
+	pub fn read<K: StorageHash<T>>(trie_id: &TrieId, key: K) -> Option<Vec<u8>> {
 		child::get_raw(&child_trie_info(trie_id), key.hash().as_slice())
 	}
 
 	/// Returns `Some(len)` (in bytes) if a storage item exists at `key`.
 	///
 	/// Returns `None` if the `key` wasn't previously set by `set_storage` or
 	/// was deleted.
-	pub fn size<K: StorageHash>(trie_id: &TrieId, key: K) -> Option<u32> {
+	pub fn size<K: StorageHash<T>>(trie_id: &TrieId, key: K) -> Option<u32> {
 		child::len(&child_trie_info(trie_id), key.hash().as_slice())
 	}
 
@@ -142,7 +142,7 @@ where
 	///
 	/// This function also records how much storage was created or removed if a `storage_meter`
 	/// is supplied. It should only be absent for testing or benchmarking code.
-	pub fn write<K: StorageHash>(
+	pub fn write<K: StorageHash<T>>(
 		trie_id: &TrieId,
 		key: K,
 		new_value: Option<Vec<u8>>,

frame/contracts/src/wasm/runtime.rs

@@ -27,7 +27,7 @@ use crate::{
 
 use bitflags::bitflags;
 use codec::{Decode, DecodeAll, Encode, MaxEncodedLen};
-use frame_support::{dispatch::DispatchError, ensure, weights::Weight};
+use frame_support::{dispatch::DispatchError, ensure, traits::Get, weights::Weight};
 use pallet_contracts_primitives::{ExecReturnValue, ReturnFlags};
 use sp_core::{crypto::UncheckedFrom, Bytes};
 use sp_io::hashing::{blake2_128, blake2_256, keccak_256, sha2_256};
@@ -41,7 +41,7 @@ use wasm_instrument::parity_wasm::elements::ValueType;
 /// VarSized is a key used for transparent hashing, which is a u8 vector maximum length of
 /// MaxStorageKeyLen.
 enum KeyType {
-	Fixed,
+	Fix,
 	Variable(u32),
 }
 
@@ -707,7 +707,6 @@ where
 		&mut self,
 		key_type: KeyType,
 		key_ptr: u32,
-		key_len: u32,
 		value_ptr: u32,
 		value_len: u32,
 	) -> Result<u32, TrapReason> {
@@ -717,19 +716,26 @@ where
 		if value_len > max_size {
 			return Err(Error::<E::T>::ValueTooLarge.into())
 		}
-
-                ensure!(key_len <= <E::T>::MaxStorageKeyLen, Error::<E::T>::DecodingFailed);
-		let key = self.read_sandbox_memory(key_ptr, key_len)?
-
+		let key_len = match key_type {
+			KeyType::Fix => 32u32,
+			KeyType::Variable(len) => {
+				ensure!(
+					len <= <E::T as Config>::MaxStorageKeyLen::get(),
+					Error::<E::T>::DecodingFailed
+				);
+				len
+			},
+		};
+		let key = self.read_sandbox_memory(key_ptr, key_len)?;
 		let value = Some(self.read_sandbox_memory(value_ptr, value_len)?);
 		let write_outcome = match key_type {
-			KeyType::FixSized => self.ext.set_storage(
+			KeyType::Fix => self.ext.set_storage(
 				FixSizedKey::try_from(key).map_err(|_| Error::<E::T>::DecodingFailed)?,
 				value,
 				false,
 			)?,
-			KeyType::VarSized => self.ext.set_storage_transparent(
-				VarSizedKey::try_from(key).map_err(|_| Error::<E::T>::DecodingFailed)?,
+			KeyType::Variable(_) => self.ext.set_storage_transparent(
+				VarSizedKey::<E::T>::try_from(key).map_err(|_| Error::<E::T>::DecodingFailed)?,
 				value,
 				false,
 			)?,
@@ -742,25 +748,27 @@ where
 		Ok(write_outcome.old_len_with_sentinel())
 	}
 
-	fn clear_storage(
-		&mut self,
-		key_type: KeyType,
-		key_ptr: u32,
-		key_len: u32,
-	) -> Result<u32, TrapReason> {
+	fn clear_storage(&mut self, key_type: KeyType, key_ptr: u32) -> Result<u32, TrapReason> {
 		let charged = self.charge_gas(RuntimeCosts::ClearStorage(self.ext.max_value_size()))?;
-
-		let mut key = vec![0; key_len.try_into().map_err(|_| Error::<E::T>::ValueTooLarge)?];
-		self.read_sandbox_memory_into_buf(key_ptr, &mut key)?;
-
+		let key_len = match key_type {
+			KeyType::Fix => 32u32,
+			KeyType::Variable(len) => {
+				ensure!(
+					len <= <E::T as Config>::MaxStorageKeyLen::get(),
+					Error::<E::T>::DecodingFailed
+				);
+				len
+			},
+		};
+		let key = self.read_sandbox_memory(key_ptr, key_len)?;
 		let outcome = match key_type {
-			KeyType::FixSized => self.ext.set_storage(
+			KeyType::Fix => self.ext.set_storage(
 				FixSizedKey::try_from(key).map_err(|_| Error::<E::T>::DecodingFailed)?,
 				None,
 				false,
 			)?,
-			KeyType::VarSized => self.ext.set_storage_transparent(
-				VarSizedKey::try_from(key).map_err(|_| Error::<E::T>::DecodingFailed)?,
+			KeyType::Variable(_) => self.ext.set_storage_transparent(
+				VarSizedKey::<E::T>::try_from(key).map_err(|_| Error::<E::T>::DecodingFailed)?,
 				None,
 				false,
 			)?,
@@ -908,7 +916,7 @@ define_env!(Env, <E: Ext>,
 	// Equivalent to the newer version of `seal_set_storage` with the exception of the return
 	// type. Still a valid thing to call when not interested in the return value.
 	[seal0] seal_set_storage(ctx, key_ptr: u32, value_ptr: u32, value_len: u32) => {
-		ctx.set_storage(KeyType::FixSized, key_ptr, 32u32, value_ptr, value_len).map(|_| ())
+		ctx.set_storage(KeyType::Fix, key_ptr, value_ptr, value_len).map(|_| ())
 	},
 
 	// Set the value at the given key in the contract storage.
@@ -927,29 +935,30 @@ define_env!(Env, <E: Ext>,
 	// Returns the size of the pre-existing value at the specified key if any. Otherwise
 	// `SENTINEL` is returned as a sentinel value.
 	[seal1] seal_set_storage(ctx, key_ptr: u32, value_ptr: u32, value_len: u32) -> u32 => {
-		ctx.set_storage(KeyType::FixSized, key_ptr, 32u32, value_ptr, value_len)
+		ctx.set_storage(KeyType::Fix, key_ptr, value_ptr, value_len)
 	},
 
 	// Clear the value at the given key in the contract storage.
 	//
 	// Equivalent to the newer version of `seal_clear_storage` with the exception of the return
 	// type. Still a valid thing to call when not interested in the return value.
 	[seal0] seal_clear_storage(ctx, key_ptr: u32) => {
-		ctx.clear_storage(KeyType::FixSized, key_ptr, 32u32).map(|_| ()).map_err(Into::into)
+		ctx.clear_storage(KeyType::Fix, key_ptr).map(|_| ()).map_err(Into::into)
 	},
 
 	// Clear the value at the given key in the contract storage.
 	//
 	// # Parameters
 	//
-	// - `key_ptr`: pointer into the linear memory where the location to clear the value is placed.
+	// - `key_ptr`: pointer into the linear memory where the key is placed.
+	// - `key_len`: the length of the key in bytes.
 	//
 	// # Return Value
 	//
 	// Returns the size of the pre-existing value at the specified key if any. Otherwise
 	// `SENTINEL` is returned as a sentinel value.
-	[__unstable__] seal_clear_storage(ctx, key_ptr: u32) -> u32 => {
-		ctx.clear_storage(KeyType::FixSized, key_ptr, 32u32).map_err(Into::into)
+	[__unstable__] seal_clear_storage(ctx, key_ptr: u32, key_len: u32) -> u32 => {
+		ctx.clear_storage(KeyType::Variable(key_len), key_ptr).map_err(Into::into)
 	},
 
 	// Retrieve the value under the given key from storage.
@@ -1006,17 +1015,17 @@ define_env!(Env, <E: Ext>,
 	// # Parameters
 	//
 	// - `key_ptr`: pointer into the linear memory where the key of the requested value is placed.
+	// - `key_len`: the length of the key in bytes.
 	// - `out_ptr`: pointer to the linear memory where the value is written to.
 	// - `out_len_ptr`: in-out pointer into linear memory where the buffer length
 	//   is read from and the value length is written to.
 	//
 	// # Errors
 	//
 	// `ReturnCode::KeyNotFound`
-	[__unstable__] seal_take_storage(ctx, key_ptr: u32, out_ptr: u32, out_len_ptr: u32) -> ReturnCode => {
+	[__unstable__] seal_take_storage(ctx, key_ptr: u32, key_len: u32, out_ptr: u32, out_len_ptr: u32) -> ReturnCode => {
 		let charged = ctx.charge_gas(RuntimeCosts::TakeStorage(ctx.ext.max_value_size()))?;
-		let mut key = FixSizedKey::default();
-		ctx.read_sandbox_memory_into_buf(key_ptr, &mut key)?;
+		let key = ctx.read_sandbox_memory(key_ptr, key_len)?;
 		if let crate::storage::WriteOutcome::Taken(value) = ctx.ext.set_storage(key, None, true)? {
 			ctx.adjust_gas(charged, RuntimeCosts::TakeStorage(value.len() as u32));
 			ctx.write_sandbox_output(out_ptr, out_len_ptr, &value, false, already_charged)?;