Storage refactoring new hope for review (#1331)

* Import new unstable functions with transparent hashing.
Updated the API to work with the generic key `K: Encode` instead of the old `Key`.
Also, the change contains optimization to reduce the size of contracts. In most cases, it is `#[inline(always)]`; but `return_value` also got small optimization; removed usage of `extract_from_slice` where it is not needed.

* primitives crate:
Removed old 32 bytes `Key`. Replaced it with `u32`. Added `KeyComposer`, it is a helper struct that does all manipulation with the storage key. It allows concat two storage keys into one, compute storage key for fields based on the filed, struct, enum, variants names.
Removed all tests and benches. Didn't add it for new primitives because the key is standard `u32` and all keys are calculated during compilation.

storage crate:
Removed `SpreadLayout`, `PackedLayout`, `SpreadAllocate`, `PackedAllocate`, and all related helper functions.
Removed `Packed` struct cause it is the default behavior for storage right now. Added `Lazy` struct that allows `get`/`set` value from/into the storage. It is similar to `Mapping` but works with one storage key and one value.
Introduced new main traits to work with storage in `storage/src/traits/storage.rs`.
Also added a new `OnCallInitializer` trait to improve the flow with upgradable contracts and support initialization on demand by default. Added `pull_or_init` macro that inits the storage struct if it is impossible to load it. It also can be used as optimization for contracts without an explicit constructor.
Replaced implementation of old traits for main primitives with a new one. Added blanket implementation of new traits for structures that are `Packed` by default. It reduces the amount of code and adds support of generic structures but adds problems with tuples(now tuples implement new traits only if inner items are `Packed`).
Introduced `AutoKey` and `ManualKey` that allows specifying which key the user wants to use. Added support of it into all traits and structures.
Refactored `Mapping` to follow new rules.

* metadata crate:
Updated storage layout in the metadata. Introduces the concept of roots and leafs. Root defines the storage key for all sub-tree until there will be another Root. Leafs are common types that are part of the sub-tree and serialized/deserialized together into one storage key.
Replaced 32 bytes storage key with `u32`.
Added validation that all root storage keys don't overlap. Maybe better to add that error or reuse that validator on the `cargo-contract` side to show a more user-friendly error.
`RootLayout` and validator are used in codegen(next commit). The contract is wrapped into `RootLayout`, and we do validation for that tree.
Metadata now contains name for each struct/enum/variant/field. It is useful information because now the storage key is calculated based on the name of struct/enum/variant/field.

storage crate:
Added a new helper crate `storage/codegen`. It contains some useful functional that is used in `ink_storage_derive` and in `ink_lang_codegen` crates. It has a method that returns all types of the struct/enum/unit and a method that finds "salt" in the generics of the struct/enum. It uses magic constant "KeyHolder"(about that you can read in issue) to find salt, so I tried to have only one place where we are using that constant.

Replaced derive implementation of old trait with new one. You can check the tests to see how the implementation looks like. `Storable` recursively call `encode` and `decode` for all fields. `KeyHolder` return key of the salt. `Item` uses `AutoKey` or key specified by the user. I want to highlight that `PreferredKey` only is used with the `AutoItem` trait. If `PreferredKey` is `AutoKey`, then `AutoItem<AutoGenerated>` select auto-generated key, otherwise preferred. So `AutoItem` trait decides that key to use. It is why derive macro only set `PreferredKey`.
Updated drive for `StorageLayout`, now it uses `u32` and pass name of the struct into metadata.

* Removed `initialize_contract` and related to initialization stuff. Now the codegen uses `pull_or_init` in the `call`. Updated `execute_constructor` to work with new storage methods.
Allowed usage of generics during the declaration of the primary contract storage. Users can specify the default storage key with `KeyHolder` via generic.
Added parser for `storage_item` macro with its config.

* Removed the old codegen related to spread and packed layout. If some object implements `Decode` and `Encode`, it is `Packed`, and it uses the blanket implementation of new traits.
In dispatch, codegen started to use a new method to work with storage.
In metadata codegen added usage of new `RootLayout`. We wrap the contract into that layout because the contract has its storage key for all inner fields by default. Also added a run of validation logic during metadata generation.
Added `storage_item` macro. It transforms all types from autokey into manual key(if types support it). It calculates the storage key based on the name(it uses the `KeyComposer::compute_key` function from the primitives crate). Also, macro generates an additional `Check` structure that includes all raw fields. It helps show correct errors to the user in case of typos, wrong types, etc.

* Updated all examples to use a new API.
Simplified delegate call example very well causes of new `OnCallInitializer` trait and support of manual specifying key.

* UI tests for a new codegen.
Can't highlight something unusual here=)

* Apply all suggestion from the review

* Make CI happy

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Apply suggestions:
- In most cases it is comments=)
- Moved `pull_or_init` on one level upper.
- Put the tests into the `impls/mod.rs`

* Fix doc

* Add comment to autoref specialisation

* Suggestion from the review

* Revert back u8

* Remove unwrap

* Collapse if let

* Fixed overflow for enums

* Fixing comments

* Renamed `Item` to `StorableHint` and `AutoItem` to `AutoStorableHint`

* Fix test

* Renamed key_holder.
Add UI test for double storage_item.
Applied suggestion from the review.

* Nightly fmt

* Remove `Packed` path

* Fix doc test

* Apply suggestions from hte review

* Fixed build

* Fix build

* Removed `initialize_contract` from linting and deleted all tests

* Fix doc link

* Fix mapping example

* Applied suggestion.
Removed `delegate-call` example with `OnCallInitializer`

* Removed `delegate-calls` from the CI. Replaced it with `set-code-hash`

* fix test

* fix test

* Fix CI to use stable for contract build

* Fix CI to use stable for examples

.config/cargo_spellcheck.dic

@@ -38,6 +38,7 @@ defragmentation
 deploy
 dereferencing
 deserialize/S
+deserialization
 dispatchable/S
 encodable
 evaluable
@@ -106,3 +107,4 @@ natively
 payability
 unpayable
 initializer
+storable

.github/workflows/examples.yml

@@ -26,15 +26,20 @@ jobs:
 # Once this is resolved we should re-enable building the examples on windows
 #          - windows-latest
         toolchain:
-          - nightly
+          - stable
         job:
           - build
           - test
     runs-on: ${{ matrix.platform }}
     env:
-      UPGRADEABLE_CONTRACTS:    "forward-calls delegate-calls"
+      UPGRADEABLE_CONTRACTS:    "forward-calls set-code-hash"
       DELEGATOR_SUBCONTRACTS:   "accumulator adder subber"
       RUST_BACKTRACE:           full
+      # We need to enable `RUSTC_BOOTSTRAP` so that the nightly ink!
+      # features still work on stable. This is done automatically by
+      # `cargo-contract`, but in our CI here we also use e.g.
+      # `cargo check` directly.
+      RUSTC_BOOTSTRAP:          "1"
     steps:
 
       - uses: actions/setup-node@v3
@@ -90,12 +95,12 @@ jobs:
                echo "Processing delegator contract: $contract";
                cargo contract ${{ matrix.job }} --verbose --manifest-path examples/delegator/${contract}/Cargo.toml;
            }
-           $upgradeable_contracts = "forward-calls","delegate-calls"
+           $upgradeable_contracts = "forward-calls","set-code-hash"
            foreach ($contract in $upgradeable_contracts) {
                echo "Processing upgradeable contract: $contract";
                cargo contract ${{ matrix.job }} --verbose --manifest-path examples/upgradeable-contracts/${contract}/Cargo.toml;
            }
-           cargo contract ${{ matrix.job }} --verbose --manifest-path examples/upgradeable-contracts/delegate-calls/upgradeable-flipper/Cargo.toml;
+           cargo contract ${{ matrix.job }} --verbose --manifest-path examples/upgradeable-contracts/set-code-hash/updated-incrementer/Cargo.toml;
            foreach ($example in Get-ChildItem  -Directory "examples\*") {
                if ($example -Match 'upgradeable-contracts') { continue }
                echo "Processing example: $example";
@@ -114,7 +119,7 @@ jobs:
                 echo "Processing upgradeable contract: $contract";
                 cargo contract ${{ matrix.job }} --verbose --manifest-path=examples/upgradeable-contracts/$contract/Cargo.toml;
            done
-           cargo contract ${{ matrix.job }} --verbose --manifest-path=examples/upgradeable-contracts/delegate-calls/upgradeable-flipper/Cargo.toml;
+           cargo contract ${{ matrix.job }} --verbose --manifest-path=examples/upgradeable-contracts/set-code-hash/updated-incrementer/Cargo.toml;
            for example in examples/*/; do
                if [ "$example" = "examples/upgradeable-contracts/" ]; then continue; fi;
                echo "Processing example: $example";

.gitlab-ci.yml

@@ -33,7 +33,7 @@ variables:
   ALSO_WASM_CRATES:                "env storage storage/derive allocator prelude primitives lang lang/macro lang/ir"
   ALL_CRATES:                      "${PURELY_STD_CRATES} ${ALSO_WASM_CRATES}"
   DELEGATOR_SUBCONTRACTS:          "accumulator adder subber"
-  UPGRADEABLE_CONTRACTS:           "forward-calls delegate-calls"
+  UPGRADEABLE_CONTRACTS:           "forward-calls set-code-hash"
   # We need to enable `RUSTC_BOOTSTRAP` so that the nightly ink!
   # features still work on stable. This is done automatically by
   # `cargo-contract`, but in our CI here we also use e.g.
@@ -127,7 +127,7 @@ examples-fmt:
     - for contract in ${UPGRADEABLE_CONTRACTS}; do
         cargo +nightly fmt --verbose --manifest-path ./examples/upgradeable-contracts/${contract}/Cargo.toml -- --check;
       done
-    - cargo +nightly fmt --verbose --manifest-path ./examples/upgradeable-contracts/delegate-calls/upgradeable-flipper/Cargo.toml -- --check
+    - cargo +nightly fmt --verbose --manifest-path ./examples/upgradeable-contracts/set-code-hash/updated-incrementer/Cargo.toml -- --check
   allow_failure:                   true
 
 clippy-std:
@@ -164,7 +164,7 @@ examples-clippy-std:
     - for contract in ${UPGRADEABLE_CONTRACTS}; do
         cargo +nightly clippy --verbose --all-targets --manifest-path ./examples/upgradeable-contracts/${contract}/Cargo.toml -- -D warnings;
       done
-    - cargo +nightly clippy --verbose --all-targets --manifest-path ./examples/upgradeable-contracts/delegate-calls/upgradeable-flipper/Cargo.toml -- -D warnings;
+    - cargo +nightly clippy --verbose --all-targets --manifest-path ./examples/upgradeable-contracts/set-code-hash/updated-incrementer/Cargo.toml -- -D warnings;
   allow_failure:                   true
 
 examples-clippy-wasm:
@@ -182,7 +182,7 @@ examples-clippy-wasm:
     - for contract in ${UPGRADEABLE_CONTRACTS}; do
         cargo +nightly clippy --verbose --manifest-path ./examples/upgradeable-contracts/${contract}/Cargo.toml --no-default-features --target wasm32-unknown-unknown -- -D warnings;
       done
-    - cargo +nightly clippy --verbose --manifest-path ./examples/upgradeable-contracts/delegate-calls/upgradeable-flipper/Cargo.toml --no-default-features --target wasm32-unknown-unknown -- -D warnings;
+    - cargo +nightly clippy --verbose --manifest-path ./examples/upgradeable-contracts/set-code-hash/updated-incrementer/Cargo.toml --no-default-features --target wasm32-unknown-unknown -- -D warnings;
   allow_failure:                   true
 
 
@@ -281,7 +281,7 @@ docs:
     - cargo doc --no-deps --all-features
         -p scale-info -p ink_metadata
         -p ink_env -p ink_storage -p ink_storage_derive
-        -p ink_primitives -p ink_prelude
+        -p ink_primitives -p ink_prelude -p ink_primitives_derive
         -p ink_lang -p ink_lang_macro -p ink_lang_ir -p ink_lang_codegen
     - mv ${CARGO_TARGET_DIR}/doc ./crate-docs
     # FIXME: remove me after CI image gets nonroot
@@ -352,7 +352,7 @@ examples-test:
     - for contract in ${UPGRADEABLE_CONTRACTS}; do
         cargo test --verbose --manifest-path ./examples/upgradeable-contracts/${contract}/Cargo.toml;
       done
-    - cargo test --verbose --manifest-path ./examples/upgradeable-contracts/delegate-calls/upgradeable-flipper/Cargo.toml;
+    - cargo test --verbose --manifest-path ./examples/upgradeable-contracts/set-code-hash/updated-incrementer/Cargo.toml;
 
 examples-contract-build:
   stage:                           examples
@@ -372,7 +372,7 @@ examples-contract-build:
     - for contract in ${UPGRADEABLE_CONTRACTS}; do
         cargo +stable contract build --manifest-path ./examples/upgradeable-contracts/${contract}/Cargo.toml;
       done
-    - cargo +stable contract build --manifest-path ./examples/upgradeable-contracts/delegate-calls/upgradeable-flipper/Cargo.toml
+    - cargo +stable contract build --manifest-path ./examples/upgradeable-contracts/set-code-hash/updated-incrementer/Cargo.toml
 
 examples-docs:
   stage:                           examples
@@ -395,7 +395,7 @@ examples-docs:
     - for contract in ${UPGRADEABLE_CONTRACTS}; do
         cargo doc --manifest-path ./examples/upgradeable-contracts/${contract}/Cargo.toml --document-private-items --verbose --no-deps;
       done
-    - cargo doc --manifest-path ./examples/upgradeable-contracts/delegate-calls/upgradeable-flipper/Cargo.toml --document-private-items --verbose --no-deps
+    - cargo doc --manifest-path ./examples/upgradeable-contracts/set-code-hash/updated-incrementer/Cargo.toml --document-private-items --verbose --no-deps
 
 
 #### stage:                        ink-waterfall

crates/engine/src/ext.rs

@@ -229,7 +229,7 @@ impl Engine {
 
     /// Writes the encoded value into the storage at the given key.
     /// Returns the size of the previously stored value at the key if any.
-    pub fn set_storage(&mut self, key: &[u8; 32], encoded_value: &[u8]) -> Option<u32> {
+    pub fn set_storage(&mut self, key: &[u8], encoded_value: &[u8]) -> Option<u32> {
         let callee = self.get_callee();
         let account_id = AccountId::from_bytes(&callee[..]);
 
@@ -243,7 +243,7 @@ impl Engine {
     }
 
     /// Returns the decoded contract storage at the key if any.
-    pub fn get_storage(&mut self, key: &[u8; 32], output: &mut &mut [u8]) -> Result {
+    pub fn get_storage(&mut self, key: &[u8], output: &mut &mut [u8]) -> Result {
         let callee = self.get_callee();
         let account_id = AccountId::from_bytes(&callee[..]);
 
@@ -258,7 +258,7 @@ impl Engine {
     }
 
     /// Returns the size of the value stored in the contract storage at the key if any.
-    pub fn contains_storage(&mut self, key: &[u8; 32]) -> Option<u32> {
+    pub fn contains_storage(&mut self, key: &[u8]) -> Option<u32> {
         let callee = self.get_callee();
         let account_id = AccountId::from_bytes(&callee[..]);
 
@@ -269,14 +269,17 @@ impl Engine {
     }
 
     /// Removes the storage entries at the given key.
-    pub fn clear_storage(&mut self, key: &[u8; 32]) {
+    /// Returns the size of the previously stored value at the key if any.
+    pub fn clear_storage(&mut self, key: &[u8]) -> Option<u32> {
         let callee = self.get_callee();
         let account_id = AccountId::from_bytes(&callee[..]);
         self.debug_info.inc_writes(account_id.clone());
         let _ = self
             .debug_info
             .remove_cell_for_account(account_id, key.to_vec());
-        let _ = self.database.remove_contract_storage(&callee, key);
+        self.database
+            .remove_contract_storage(&callee, key)
+            .map(|val| val.len() as u32)
     }
 
     /// Remove the calling account and transfer remaining balance.

crates/env/src/api.rs

@@ -39,7 +39,7 @@ use crate::{
     Environment,
     Result,
 };
-use ink_primitives::Key;
+use ink_primitives::traits::Storable;
 
 /// Returns the address of the caller of the executed contract.
 ///
@@ -183,49 +183,58 @@ where
     })
 }
 
-/// Writes the value to the contract storage under the given key and returns
-/// the size of pre-existing value at the specified key if any.
+/// Writes the value to the contract storage under the given storage key and returns the size
+/// of pre-existing value if any.
 ///
 /// # Panics
 ///
 /// - If the encode length of value exceeds the configured maximum value length of a storage entry.
-pub fn set_contract_storage<V>(key: &Key, value: &V) -> Option<u32>
+pub fn set_contract_storage<K, V>(key: &K, value: &V) -> Option<u32>
 where
-    V: scale::Encode,
+    K: scale::Encode,
+    V: Storable,
 {
     <EnvInstance as OnInstance>::on_instance(|instance| {
-        EnvBackend::set_contract_storage::<V>(instance, key, value)
+        EnvBackend::set_contract_storage::<K, V>(instance, key, value)
     })
 }
 
-/// Returns the value stored under the given key in the contract's storage if any.
+/// Returns the value stored under the given storage key in the contract's storage if any.
 ///
 /// # Errors
 ///
 /// - If the decoding of the typed value failed (`KeyNotFound`)
-pub fn get_contract_storage<R>(key: &Key) -> Result<Option<R>>
+pub fn get_contract_storage<K, R>(key: &K) -> Result<Option<R>>
 where
-    R: scale::Decode,
+    K: scale::Encode,
+    R: Storable,
 {
     <EnvInstance as OnInstance>::on_instance(|instance| {
-        EnvBackend::get_contract_storage::<R>(instance, key)
+        EnvBackend::get_contract_storage::<K, R>(instance, key)
     })
 }
 
-/// Checks whether there is a value stored under the given key in
-/// the contract's storage.
+/// Checks whether there is a value stored under the given storage key in the contract's storage.
 ///
 /// If a value is stored under the specified key, the size of the value is returned.
-pub fn contract_storage_contains(key: &Key) -> Option<u32> {
+pub fn contains_contract_storage<K>(key: &K) -> Option<u32>
+where
+    K: scale::Encode,
+{
     <EnvInstance as OnInstance>::on_instance(|instance| {
-        EnvBackend::contract_storage_contains(instance, key)
+        EnvBackend::contains_contract_storage::<K>(instance, key)
     })
 }
 
-/// Clears the contract's storage key entry.
-pub fn clear_contract_storage(key: &Key) {
+/// Clears the contract's storage entry under the given storage key.
+///
+/// If a value was stored under the specified storage key, the size of the value is returned.
+pub fn clear_contract_storage<K>(key: &K) -> Option<u32>
+where
+    K: scale::Encode,
+{
     <EnvInstance as OnInstance>::on_instance(|instance| {
-        EnvBackend::clear_contract_storage(instance, key)
+        EnvBackend::clear_contract_storage::<K>(instance, key)
     })
 }
 

crates/env/src/backend.rs

@@ -27,7 +27,7 @@ use crate::{
     Environment,
     Result,
 };
-use ink_primitives::Key;
+use ink_primitives::traits::Storable;
 
 /// The flags to indicate further information about the end of a contract execution.
 #[derive(Default)]
@@ -162,26 +162,35 @@ impl CallFlags {
 
 /// Environmental contract functionality that does not require `Environment`.
 pub trait EnvBackend {
-    /// Writes the value to the contract storage under the given key and returns
-    /// the size of the pre-existing value at the specified key if any.
-    fn set_contract_storage<V>(&mut self, key: &Key, value: &V) -> Option<u32>
+    /// Writes the value to the contract storage under the given storage key.
+    ///
+    /// Returns the size of the pre-existing value at the specified key if any.
+    fn set_contract_storage<K, V>(&mut self, key: &K, value: &V) -> Option<u32>
     where
-        V: scale::Encode;
+        K: scale::Encode,
+        V: Storable;
 
-    /// Returns the value stored under the given key in the contract's storage if any.
+    /// Returns the value stored under the given storage key in the contract's storage if any.
     ///
     /// # Errors
     ///
     /// - If the decoding of the typed value failed
-    fn get_contract_storage<R>(&mut self, key: &Key) -> Result<Option<R>>
+    fn get_contract_storage<K, R>(&mut self, key: &K) -> Result<Option<R>>
     where
-        R: scale::Decode;
+        K: scale::Encode,
+        R: Storable;
 
-    /// Returns the size of a value stored under the specified key is returned if any.
-    fn contract_storage_contains(&mut self, key: &Key) -> Option<u32>;
+    /// 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>
+    where
+        K: scale::Encode;
 
-    /// Clears the contract's storage key entry.
-    fn clear_contract_storage(&mut self, key: &Key);
+    /// Clears the contract's storage key entry under the given storage key.
+    ///
+    /// Returns the size of the previously stored value at the specified key if any.
+    fn clear_contract_storage<K>(&mut self, key: &K) -> Option<u32>
+    where
+        K: scale::Encode;
 
     /// Returns the execution input to the executed contract and decodes it as `T`.
     ///

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

@@ -44,7 +44,7 @@ use ink_engine::{
     ext,
     ext::Engine,
 };
-use ink_primitives::Key;
+use ink_primitives::traits::Storable;
 
 /// The capacity of the static buffer.
 /// This is the same size as the ink! on-chain environment. We chose to use the same size
@@ -184,34 +184,43 @@ impl EnvInstance {
 }
 
 impl EnvBackend for EnvInstance {
-    fn set_contract_storage<V>(&mut self, key: &Key, value: &V) -> Option<u32>
+    fn set_contract_storage<K, V>(&mut self, key: &K, value: &V) -> Option<u32>
     where
-        V: scale::Encode,
+        K: scale::Encode,
+        V: Storable,
     {
-        let v = scale::Encode::encode(value);
-        self.engine.set_storage(key.as_ref(), &v[..])
+        let mut v = vec![];
+        Storable::encode(value, &mut v);
+        self.engine.set_storage(&key.encode(), &v[..])
     }
 
-    fn get_contract_storage<R>(&mut self, key: &Key) -> Result<Option<R>>
+    fn get_contract_storage<K, R>(&mut self, key: &K) -> Result<Option<R>>
     where
-        R: scale::Decode,
+        K: scale::Encode,
+        R: Storable,
     {
         let mut output: [u8; 9600] = [0; 9600];
-        match self.engine.get_storage(key.as_ref(), &mut &mut output[..]) {
+        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 = scale::Decode::decode(&mut &output[..])?;
+        let decoded = Storable::decode(&mut &output[..])?;
         Ok(Some(decoded))
     }
 
-    fn contract_storage_contains(&mut self, key: &Key) -> Option<u32> {
-        self.engine.contains_storage(key.as_ref())
+    fn contains_contract_storage<K>(&mut self, key: &K) -> Option<u32>
+    where
+        K: scale::Encode,
+    {
+        self.engine.contains_storage(&key.encode())
     }
 
-    fn clear_contract_storage(&mut self, key: &Key) {
-        self.engine.clear_storage(key.as_ref())
+    fn clear_contract_storage<K>(&mut self, key: &K) -> Option<u32>
+    where
+        K: scale::Encode,
+    {
+        self.engine.clear_storage(&key.encode())
     }
 
     fn decode_input<T>(&mut self) -> Result<T>