Replace Vec<u8> with [u8; 32] for contract storage key (#2184)

* Replace Vec<u8> with [u8; 32] for contract storage key

* Read storage keys from sandbox memory into fixed size buffer

* Increment `impl_version`

substrate/node/runtime/src/lib.rs

@@ -60,7 +60,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	impl_name: create_runtime_str!("substrate-node"),
 	authoring_version: 10,
 	spec_version: 57,
-	impl_version: 57,
+	impl_version: 58,
 	apis: RUNTIME_API_VERSIONS,
 };
 

substrate/srml/contract/src/account_db.rs

@@ -17,6 +17,7 @@
 //! Auxilliaries to help with managing partial changes to accounts state.
 
 use super::{CodeHash, CodeHashOf, Trait, TrieId, AccountInfoOf, BalanceOf, AccountInfo, TrieIdGenerator};
+use crate::exec::StorageKey;
 use system;
 use rstd::cell::RefCell;
 use rstd::collections::btree_map::{BTreeMap, Entry};
@@ -29,7 +30,7 @@ pub struct ChangeEntry<T: Trait> {
 	balance: Option<BalanceOf<T>>,
 	/// In the case the outer option is None, the code_hash remains untouched, while providing `Some(None)` signifies a removing of the code in question
 	code: Option<Option<CodeHash<T>>>,
-	storage: BTreeMap<Vec<u8>, Option<Vec<u8>>>,
+	storage: BTreeMap<StorageKey, Option<Vec<u8>>>,
 }
 
 // Cannot derive(Default) since it erroneously bounds T by Default.
@@ -51,7 +52,7 @@ pub trait AccountDb<T: Trait> {
 	///
 	/// Trie id can be None iff account doesn't have an associated trie id in <AccountInfoOf<T>>.
 	/// Because DirectAccountDb bypass the lookup for this association.
-	fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &[u8]) -> Option<Vec<u8>>;
+	fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option<Vec<u8>>;
 	fn get_code(&self, account: &T::AccountId) -> Option<CodeHash<T>>;
 	fn get_balance(&self, account: &T::AccountId) -> BalanceOf<T>;
 
@@ -60,7 +61,7 @@ pub trait AccountDb<T: Trait> {
 
 pub struct DirectAccountDb;
 impl<T: Trait> AccountDb<T> for DirectAccountDb {
-	fn get_storage(&self, _account: &T::AccountId, trie_id: Option<&TrieId>, location: &[u8]) -> Option<Vec<u8>> {
+	fn get_storage(&self, _account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option<Vec<u8>> {
 		trie_id.and_then(|id| child::get_raw(id, location))
 	}
 	fn get_code(&self, account: &T::AccountId) -> Option<CodeHash<T>> {
@@ -154,7 +155,7 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> {
 	pub fn set_storage(
 		&mut self,
 		account: &T::AccountId,
-		location: Vec<u8>,
+		location: StorageKey,
 		value: Option<Vec<u8>>,
 	) {
 		self.local.borrow_mut()
@@ -181,7 +182,7 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> {
 }
 
 impl<'a, T: Trait> AccountDb<T> for OverlayAccountDb<'a, T> {
-	fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &[u8]) -> Option<Vec<u8>> {
+	fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option<Vec<u8>> {
 		self.local
 			.borrow()
 			.get(account)

substrate/srml/contract/src/exec.rs

@@ -39,6 +39,8 @@ pub struct CallReceipt {
 	pub output_data: Vec<u8>,
 }
 
+pub type StorageKey = [u8; 32];
+
 /// An interface that provides access to the external environment in which the
 /// smart-contract is executed.
 ///
@@ -51,12 +53,12 @@ pub trait Ext {
 	///
 	/// Returns `None` if the `key` wasn't previously set by `set_storage` or
 	/// was deleted.
-	fn get_storage(&self, key: &[u8]) -> Option<Vec<u8>>;
+	fn get_storage(&self, key: &StorageKey) -> Option<Vec<u8>>;
 
 	/// Sets the storage entry by the given key to the specified value.
 	///
 	/// If `value` is `None` then the storage entry is deleted.
-	fn set_storage(&mut self, key: &[u8], value: Option<Vec<u8>>);
+	fn set_storage(&mut self, key: StorageKey, value: Option<Vec<u8>>);
 
 	/// Instantiate a contract from the given code.
 	///
@@ -557,14 +559,14 @@ where
 {
 	type T = T;
 
-	fn get_storage(&self, key: &[u8]) -> Option<Vec<u8>> {
+	fn get_storage(&self, key: &StorageKey) -> Option<Vec<u8>> {
 		self.ctx.overlay.get_storage(&self.ctx.self_account, self.ctx.self_trie_id.as_ref(), key)
 	}
 
-	fn set_storage(&mut self, key: &[u8], value: Option<Vec<u8>>) {
+	fn set_storage(&mut self, key: StorageKey, value: Option<Vec<u8>>) {
 		self.ctx
 			.overlay
-			.set_storage(&self.ctx.self_account, key.to_vec(), value)
+			.set_storage(&self.ctx.self_account, key, value)
 	}
 
 	fn instantiate(

substrate/srml/contract/src/wasm/mod.rs

@@ -174,7 +174,7 @@ mod tests {
 	use super::*;
 	use std::collections::HashMap;
 	use substrate_primitives::H256;
-	use crate::exec::{CallReceipt, Ext, InstantiateReceipt, EmptyOutputBuf};
+	use crate::exec::{CallReceipt, Ext, InstantiateReceipt, EmptyOutputBuf, StorageKey};
 	use crate::gas::GasMeter;
 	use crate::tests::{Test, Call};
 	use wabt;
@@ -199,7 +199,7 @@ mod tests {
 	}
 	#[derive(Default)]
 	pub struct MockExt {
-		storage: HashMap<Vec<u8>, Vec<u8>>,
+		storage: HashMap<StorageKey, Vec<u8>>,
 		creates: Vec<CreateEntry>,
 		transfers: Vec<TransferEntry>,
 		dispatches: Vec<DispatchEntry>,
@@ -210,11 +210,11 @@ mod tests {
 	impl Ext for MockExt {
 		type T = Test;
 
-		fn get_storage(&self, key: &[u8]) -> Option<Vec<u8>> {
+		fn get_storage(&self, key: &StorageKey) -> Option<Vec<u8>> {
 			self.storage.get(key).cloned()
 		}
-		fn set_storage(&mut self, key: &[u8], value: Option<Vec<u8>>) {
-			*self.storage.entry(key.to_vec()).or_insert(Vec::new()) = value.unwrap_or(Vec::new());
+		fn set_storage(&mut self, key: StorageKey, value: Option<Vec<u8>>) {
+			*self.storage.entry(key).or_insert(Vec::new()) = value.unwrap_or(Vec::new());
 		}
 		fn instantiate(
 			&mut self,
@@ -568,7 +568,7 @@ mod tests {
 		let mut mock_ext = MockExt::default();
 		mock_ext
 			.storage
-			.insert([0x11; 32].to_vec(), [0x22; 32].to_vec());
+			.insert([0x11; 32], [0x22; 32].to_vec());
 
 		let mut return_buf = Vec::new();
 		execute(

substrate/srml/contract/src/wasm/runtime.rs

@@ -177,6 +177,26 @@ fn read_sandbox_memory<E: Ext>(
 	Ok(buf)
 }
 
+/// Read designated chunk from the sandbox memory into the supplied buffer, consuming
+/// an appropriate amount of gas.
+///
+/// Returns `Err` if one of the following conditions occurs:
+///
+/// - calculating the gas cost resulted in overflow.
+/// - out of gas
+/// - requested buffer is not within the bounds of the sandbox memory.
+fn read_sandbox_memory_into_buf<E: Ext>(
+	ctx: &mut Runtime<E>,
+	ptr: u32,
+	buf: &mut [u8],
+) -> Result<(), sandbox::HostError> {
+	charge_gas(ctx.gas_meter, ctx.schedule, RuntimeToken::ReadMemory(buf.len() as u32))?;
+
+	ctx.memory().get(ptr, buf)?;
+
+	Ok(())
+}
+
 /// Write the given buffer to the designated location in the sandbox memory, consuming
 /// an appropriate amount of gas.
 ///
@@ -228,14 +248,15 @@ define_env!(Env, <E: Ext>,
 	//   where the value to set is placed. If `value_non_null` is set to 0, then this parameter is ignored.
 	// - value_len: the length of the value. If `value_non_null` is set to 0, then this parameter is ignored.
 	ext_set_storage(ctx, key_ptr: u32, value_non_null: u32, value_ptr: u32, value_len: u32) => {
-		let key = read_sandbox_memory(ctx, key_ptr, 32)?;
+		let mut key = [0; 32];
+		read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?;
 		let value =
 			if value_non_null != 0 {
 				Some(read_sandbox_memory(ctx, value_ptr, value_len)?)
 			} else {
 				None
 			};
-		ctx.ext.set_storage(&key, value);
+		ctx.ext.set_storage(key, value);
 
 		Ok(())
 	},
@@ -247,7 +268,8 @@ define_env!(Env, <E: Ext>,
 	// - key_ptr: pointer into the linear memory where the key
 	//   of the requested value is placed.
 	ext_get_storage(ctx, key_ptr: u32) -> u32 => {
-		let key = read_sandbox_memory(ctx, key_ptr, 32)?;
+		let mut key = [0; 32];
+		read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?;
 		if let Some(value) = ctx.ext.get_storage(&key) {
 			ctx.scratch_buf = value;
 			Ok(0)