From 7432565b7e91cbe41f47be57ee0a344429724cc8 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 15 Nov 2021 11:59:25 +0100 Subject: [PATCH] Simple Mapping Storage Primitive (#946) * Add `Mapping` storage collection * Implement `insert` and `get` for `Mapping` * Implement `SpreadLayout` for `Mapping` * Fix typo * Add some basic tests * Fix some documentation formatting * Use `PackedLayout` as trait bound instead of `Encode/Decode` * Avoid using low level `ink_env` functions when interacting with storage * RustFmt * Appease Clippy * Only use single `PhantomData` field * Change `get` API to take reference to `key` * Implement `TypeInfo` and `StorageLayout` for `Mapping` * Properly gate `TypeInfo` and `StorageLayout` impls behind `std` * Replace `HashMap` with `Mapping` in ERC-20 example * Return `Option` from `Mapping::get` * Update ERC-20 to handle `Option` returns * Change `get` and `key` to use `Borrow`-ed values * Add `Debug` and `Default` implementations * Proper spelling * Change `insert` to only accept borrowed K,V pairs * Update ERC-20 example accordingly * Make more explicit what each `key` is referring to * Try using a `RefCell` instead of passing `Key` around * Try using `UnsafeCell` instead * Revert "Try using a `RefCell` instead of passing `Key` around" This reverts commit cede0330f1f60645f19a825801a806fe39d2b0be. Using `RefCell`/`UnsafeCell` doesn't reduce the contract size more than what we have now, and it introduced `unsafe` code. We believe the limiting factor here is the `Key` type definition anyways. * Clean up some of the documentation * Simple Mapping type improvements (#979) * Add `Mapping` storage collection * Implement `insert` and `get` for `Mapping` * Implement `SpreadLayout` for `Mapping` * Fix typo * Add some basic tests * Fix some documentation formatting * Use `PackedLayout` as trait bound instead of `Encode/Decode` * Avoid using low level `ink_env` functions when interacting with storage * RustFmt * Appease Clippy * Only use single `PhantomData` field * Change `get` API to take reference to `key` * Implement `TypeInfo` and `StorageLayout` for `Mapping` * Properly gate `TypeInfo` and `StorageLayout` impls behind `std` * Replace `HashMap` with `Mapping` in ERC-20 example * Return `Option` from `Mapping::get` * Update ERC-20 to handle `Option` returns * Change `get` and `key` to use `Borrow`-ed values * Add `Debug` and `Default` implementations * Proper spelling * Change `insert` to only accept borrowed K,V pairs * Update ERC-20 example accordingly * Make more explicit what each `key` is referring to * Try using a `RefCell` instead of passing `Key` around * Try using `UnsafeCell` instead * Revert "Try using a `RefCell` instead of passing `Key` around" This reverts commit cede0330f1f60645f19a825801a806fe39d2b0be. Using `RefCell`/`UnsafeCell` doesn't reduce the contract size more than what we have now, and it introduced `unsafe` code. We believe the limiting factor here is the `Key` type definition anyways. * Clean up some of the documentation * adjust the Mapping type for the new SpreadAllocate trait * adjust ERC-20 example for changes in Mapping type * remove commented out code * add doc comment to new_init * make it possible to use references in more cases with Mapping * use references in more cases for ERC-20 example contract * remove unnecessary references in Mapping methods * refactor/improve pull_packed_root_opt utility method slightly * fix ERC-20 example contract The problem with *self.total_supply is that it may implicitly read from storage in case it has not yet read a value from storage whereas Lazy::set just writes the value to the Lazy instance. Co-authored-by: Hernando Castano Co-authored-by: Hernando Castano * Use new `initialize_contract()` function * Derive `SpreadAllocate` for `ink(storage)` structs * Stop manually implementing SpreadAllocate for ERC-20 * Stop implementing `SpreadAllocate` in the storage codegen * Derive `SpreadAllocate` manually for ERC-20 * RustFmt example * Move `Mapping` from `collections` to `lazy` * Remove extra `0` in docs Co-authored-by: Robin Freyler --- crates/storage/src/lazy/mapping.rs | 189 +++++++++++++++++++++++++++ crates/storage/src/lazy/mod.rs | 2 + crates/storage/src/traits/optspec.rs | 24 ++-- crates/storage/src/traits/spread.rs | 4 +- examples/erc20/lib.rs | 89 +++++++++---- 5 files changed, 267 insertions(+), 41 deletions(-) create mode 100644 crates/storage/src/lazy/mapping.rs diff --git a/crates/storage/src/lazy/mapping.rs b/crates/storage/src/lazy/mapping.rs new file mode 100644 index 0000000000..f5b9fb8aab --- /dev/null +++ b/crates/storage/src/lazy/mapping.rs @@ -0,0 +1,189 @@ +// Copyright 2018-2021 Parity Technologies (UK) Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! A simple mapping to contract storage. +//! +//! # Note +//! +//! This mapping doesn't actually "own" any data. +//! Instead it is just a simple wrapper around the contract storage facilities. + +use crate::traits::{ + pull_packed_root_opt, + push_packed_root, + ExtKeyPtr, + KeyPtr, + PackedLayout, + SpreadAllocate, + SpreadLayout, +}; +use core::marker::PhantomData; + +use ink_env::hash::{ + Blake2x256, + HashOutput, +}; +use ink_primitives::Key; + +/// A mapping of key-value pairs directly into contract storage. +#[cfg_attr(feature = "std", derive(scale_info::TypeInfo))] +#[derive(Default)] +pub struct Mapping { + offset_key: Key, + _marker: PhantomData (K, V)>, +} + +impl core::fmt::Debug for Mapping { + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + f.debug_struct("Mapping") + .field("offset_key", &self.offset_key) + .finish() + } +} + +impl Mapping { + /// Creates a new empty `Mapping`. + fn new(offset_key: Key) -> Self { + Self { + offset_key, + _marker: Default::default(), + } + } +} + +impl Mapping +where + K: PackedLayout, + V: PackedLayout, +{ + /// Insert the given `value` to the contract storage. + #[inline] + pub fn insert(&mut self, key: Q, value: &R) + where + Q: scale::EncodeLike, + R: scale::EncodeLike + PackedLayout, + { + push_packed_root(value, &self.storage_key(key)); + } + + /// Get the `value` at `key` from the contract storage. + /// + /// Returns `None` if no `value` exists at the given `key`. + #[inline] + pub fn get(&self, key: Q) -> Option + where + Q: scale::EncodeLike, + { + pull_packed_root_opt(&self.storage_key(key)) + } + + /// Returns a `Key` pointer used internally by the storage API. + /// + /// This key is a combination of the `Mapping`'s internal `offset_key` + /// and the user provided `key`. + fn storage_key(&self, key: Q) -> Key + where + Q: scale::EncodeLike, + { + let encodedable_key = (&self.offset_key, key); + let mut output = ::Type::default(); + ink_env::hash_encoded::(&encodedable_key, &mut output); + output.into() + } +} + +impl SpreadLayout for Mapping { + const FOOTPRINT: u64 = 1; + const REQUIRES_DEEP_CLEAN_UP: bool = false; + + #[inline] + fn pull_spread(ptr: &mut KeyPtr) -> Self { + // Note: There is no need to pull anything from the storage for the + // mapping type since it initializes itself entirely by the + // given key pointer. + Self::new(*ExtKeyPtr::next_for::(ptr)) + } + + #[inline] + fn push_spread(&self, ptr: &mut KeyPtr) { + // Note: The mapping type does not store any state in its associated + // storage region, therefore only the pointer has to be incremented. + ptr.advance_by(Self::FOOTPRINT); + } + + #[inline] + fn clear_spread(&self, ptr: &mut KeyPtr) { + // Note: The mapping type is not aware of its elements, therefore + // it is not possible to clean up after itself. + ptr.advance_by(Self::FOOTPRINT); + } +} + +impl SpreadAllocate for Mapping { + #[inline] + fn allocate_spread(ptr: &mut KeyPtr) -> Self { + // Note: The mapping type initializes itself entirely by the key pointer. + Self::new(*ExtKeyPtr::next_for::(ptr)) + } +} + +#[cfg(feature = "std")] +const _: () = { + use crate::traits::StorageLayout; + use ink_metadata::layout::{ + CellLayout, + Layout, + LayoutKey, + }; + + impl StorageLayout for Mapping + where + K: scale_info::TypeInfo + 'static, + V: scale_info::TypeInfo + 'static, + { + fn layout(key_ptr: &mut KeyPtr) -> Layout { + Layout::Cell(CellLayout::new::(LayoutKey::from( + key_ptr.advance_by(1), + ))) + } + } +}; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn insert_and_get_work() { + ink_env::test::run_test::(|_| { + let mut mapping: Mapping = Mapping::new([0u8; 32].into()); + mapping.insert(&1, &2); + assert_eq!(mapping.get(&1), Some(2)); + + Ok(()) + }) + .unwrap() + } + + #[test] + fn gets_default_if_no_key_set() { + ink_env::test::run_test::(|_| { + let mapping: Mapping = Mapping::new([0u8; 32].into()); + assert_eq!(mapping.get(&1), None); + + Ok(()) + }) + .unwrap() + } +} diff --git a/crates/storage/src/lazy/mod.rs b/crates/storage/src/lazy/mod.rs index c9726a5716..b2076ccb25 100644 --- a/crates/storage/src/lazy/mod.rs +++ b/crates/storage/src/lazy/mod.rs @@ -25,6 +25,7 @@ //! extra care has to be taken when operating directly on them. pub mod lazy_hmap; +pub mod mapping; mod cache_cell; mod entry; @@ -46,6 +47,7 @@ pub use self::{ lazy_cell::LazyCell, lazy_hmap::LazyHashMap, lazy_imap::LazyIndexMap, + mapping::Mapping, }; use crate::traits::{ KeyPtr, diff --git a/crates/storage/src/traits/optspec.rs b/crates/storage/src/traits/optspec.rs index 3e28a9d86c..6d8697f057 100644 --- a/crates/storage/src/traits/optspec.rs +++ b/crates/storage/src/traits/optspec.rs @@ -102,17 +102,19 @@ pub fn pull_packed_root_opt(root_key: &Key) -> Option where T: PackedLayout, { - match ink_env::get_contract_storage::(root_key) - .expect("decoding does not match expected type") - { - Some(mut value) => { - // In case the contract storage is occupied we handle - // the Option as if it was a T. + ink_env::get_contract_storage::(root_key) + .unwrap_or_else(|error| { + panic!( + "failed to pull packed from root key {}: {:?}", + root_key, error + ) + }) + .map(|mut value| { + // In case the contract storage is occupied at the root key + // we handle the Option as if it was a T. ::pull_packed(&mut value, root_key); - Some(value) - } - None => None, - } + value + }) } pub fn push_packed_root_opt(entity: Option<&T>, root_key: &Key) @@ -128,7 +130,7 @@ where super::push_packed_root(value, root_key) } None => { - // Clear the associated storage cell. + // Clear the associated storage cell since the entity is `None`. ink_env::clear_contract_storage(root_key); } } diff --git a/crates/storage/src/traits/spread.rs b/crates/storage/src/traits/spread.rs index 651aae3084..393149e1d7 100644 --- a/crates/storage/src/traits/spread.rs +++ b/crates/storage/src/traits/spread.rs @@ -43,8 +43,8 @@ pub trait SpreadLayout { /// /// # Examples /// - /// An instance of type `i32` requires one storage cell, so its footprint is - /// 1. An instance of type `(i32, i32)` requires 2 storage cells since a + /// An instance of type `i32` requires one storage cell, so its footprint is 1. + /// An instance of type `(i32, i32)` requires 2 storage cells since a /// tuple or any other combined data structure always associates disjunctive /// cells for its sub types. The same applies to arrays, e.g. `[i32; 5]` /// has a footprint of 5. diff --git a/examples/erc20/lib.rs b/examples/erc20/lib.rs index 4f333981d8..5fca7d52be 100644 --- a/examples/erc20/lib.rs +++ b/examples/erc20/lib.rs @@ -5,20 +5,24 @@ use ink_lang as ink; #[ink::contract] mod erc20 { use ink_storage::{ - collections::HashMap as StorageHashMap, - lazy::Lazy, + lazy::{ + Lazy, + Mapping, + }, + traits::SpreadAllocate, }; /// A simple ERC-20 contract. #[ink(storage)] + #[derive(SpreadAllocate)] pub struct Erc20 { /// Total token supply. total_supply: Lazy, /// Mapping from owner to number of owned token. - balances: StorageHashMap, + balances: Mapping, /// Mapping of the token amount which an account is allowed to withdraw /// from another account. - allowances: StorageHashMap<(AccountId, AccountId), Balance>, + allowances: Mapping<(AccountId, AccountId), Balance>, } /// Event emitted when a token transfer occurs. @@ -59,20 +63,21 @@ mod erc20 { /// Creates a new ERC-20 contract with the specified initial supply. #[ink(constructor)] pub fn new(initial_supply: Balance) -> Self { + ink_lang::codegen::initialize_contract(|contract| { + Self::new_init(contract, initial_supply) + }) + } + + /// Default initializes the ERC-20 contract with the specified initial supply. + fn new_init(&mut self, initial_supply: Balance) { let caller = Self::env().caller(); - let mut balances = StorageHashMap::new(); - balances.insert(caller, initial_supply); - let instance = Self { - total_supply: Lazy::new(initial_supply), - balances, - allowances: StorageHashMap::new(), - }; + self.balances.insert(&caller, &initial_supply); + Lazy::set(&mut self.total_supply, initial_supply); Self::env().emit_event(Transfer { from: None, to: Some(caller), value: initial_supply, }); - instance } /// Returns the total token supply. @@ -86,15 +91,41 @@ mod erc20 { /// Returns `0` if the account is non-existent. #[ink(message)] pub fn balance_of(&self, owner: AccountId) -> Balance { - self.balances.get(&owner).copied().unwrap_or(0) + self.balance_of_impl(&owner) + } + + /// Returns the account balance for the specified `owner`. + /// + /// Returns `0` if the account is non-existent. + /// + /// # Note + /// + /// Prefer to call this method over `balance_of` since this + /// works using references which are more efficient in Wasm. + #[inline] + fn balance_of_impl(&self, owner: &AccountId) -> Balance { + self.balances.get(owner).unwrap_or_default() } /// Returns the amount which `spender` is still allowed to withdraw from `owner`. /// - /// Returns `0` if no allowance has been set `0`. + /// Returns `0` if no allowance has been set. #[ink(message)] pub fn allowance(&self, owner: AccountId, spender: AccountId) -> Balance { - self.allowances.get(&(owner, spender)).copied().unwrap_or(0) + self.allowance_impl(&owner, &spender) + } + + /// Returns the amount which `spender` is still allowed to withdraw from `owner`. + /// + /// Returns `0` if no allowance has been set. + /// + /// # Note + /// + /// Prefer to call this method over `allowance` since this + /// works using references which are more efficient in Wasm. + #[inline] + fn allowance_impl(&self, owner: &AccountId, spender: &AccountId) -> Balance { + self.allowances.get((owner, spender)).unwrap_or_default() } /// Transfers `value` amount of tokens from the caller's account to account `to`. @@ -108,7 +139,7 @@ mod erc20 { #[ink(message)] pub fn transfer(&mut self, to: AccountId, value: Balance) -> Result<()> { let from = self.env().caller(); - self.transfer_from_to(from, to, value) + self.transfer_from_to(&from, &to, value) } /// Allows `spender` to withdraw from the caller's account multiple times, up to @@ -120,7 +151,7 @@ mod erc20 { #[ink(message)] pub fn approve(&mut self, spender: AccountId, value: Balance) -> Result<()> { let owner = self.env().caller(); - self.allowances.insert((owner, spender), value); + self.allowances.insert((&owner, &spender), &value); self.env().emit_event(Approval { owner, spender, @@ -151,12 +182,13 @@ mod erc20 { value: Balance, ) -> Result<()> { let caller = self.env().caller(); - let allowance = self.allowance(from, caller); + let allowance = self.allowance_impl(&from, &caller); if allowance < value { return Err(Error::InsufficientAllowance) } - self.transfer_from_to(from, to, value)?; - self.allowances.insert((from, caller), allowance - value); + self.transfer_from_to(&from, &to, value)?; + self.allowances + .insert((&from, &caller), &(allowance - value)); Ok(()) } @@ -170,20 +202,21 @@ mod erc20 { /// the caller's account balance. fn transfer_from_to( &mut self, - from: AccountId, - to: AccountId, + from: &AccountId, + to: &AccountId, value: Balance, ) -> Result<()> { - let from_balance = self.balance_of(from); + let from_balance = self.balance_of_impl(from); if from_balance < value { return Err(Error::InsufficientBalance) } - self.balances.insert(from, from_balance - value); - let to_balance = self.balance_of(to); - self.balances.insert(to, to_balance + value); + + self.balances.insert(from, &(from_balance - value)); + let to_balance = self.balance_of_impl(to); + self.balances.insert(to, &(to_balance + value)); self.env().emit_event(Transfer { - from: Some(from), - to: Some(to), + from: Some(*from), + to: Some(*to), value, }); Ok(())