Storage rework

[xgreenx]

Implements #1134

Overview

It is an implementation of new storage of smart contracts. Previously each field was stored in its storage cell under its storage key. The storage key was calculated in runtime over fields iteration. In a new version, all atomic fields are stored in one storage cell under one storage key. All non-atomic fields know their storage key during compilation time.

• Removed old traits: SpreadLayout, PacketLayout, SpreadAllocate, PacketAllocate, and all related code.
• Removed ink_storage::collection and ink_storage::lazy. A new collections/lazy should be added in follow-up PRs(Removed collection and lazy from storage #1271).

Introduced new traits: StorageType, StorageKeyHolder, AtomicGuard. All those traits are implemented for primitives and base collections.

• StorageType and StorageKeyHolder is used by a new #[ink_lang::storage_item] macro to automate storage key calculation during compilation.
• AtomicGuard prevents the usage of non-atomic structs/enums in collections or mappings. It is a guard for the user to not corrupt the storage. AtomicGuard can be derived for the structure or enum if all fields are atomic(it can be implemented manually. You can do that if you sure that all types are atomic).

The scale::Encode and scale::Decode is used to pull and push storage.

The StorageKey is u32. Due to some limitations of the host functions, we still use the [u8; 32] storage key in some places but it should be fixed with paritytech/substrate#11029.

Atomic vs Non-atomic

The field is atomic if it doesn't require its storage cell.

New ink_storage::Mapping and ink_storage::StorageValue allow to occupy additional storage cells. The storage key is calculated for those types during the compilation time. The rules about automatic storage key calculation you can find below. By default, those types use the ink_storage::traits::AutoKey policy, but anyone can specify that a storage key should be used via ink_storage::traits::ManualKey like:

#[ink(storage)]
#[derive(Default)]
pub struct Erc20 {
    /// Total token supply.
    total_supply: Balance,
    /// Mapping from owner to number of owned token.
    balances: Mapping<AccountId, Balance, ManualKey<123>>,
    /// Mapping of the token amount which an account is allowed to withdraw
    /// from another account.
    allowances: Mapping<(AccountId, AccountId), Balance, ManualKey<456>>,
}

The specified storage key is more prior than autogenerated. Any struct or enum, that uses those types, becomes non-atomic(more non-atomic types in follow-up PRs).

#[ink_lang::storage_item]

#[ink_lang::storage_item] should be used during the declaration of a new struct or enum if it should be part of the storage. The macro derives all required traits by default:

#[derive(
    ::ink_storage::traits::AtomicGuard,
    ::ink_storage::traits::StorageType,
    ::ink_storage::traits::StorageKeyHolder,
    ::scale::Encode,
    ::scale::Decode,
)]
#[cfg_attr(feature = "std", derive(
    ::scale_info::TypeInfo,
    ::ink_storage::traits::StorageLayout,
))]

But it can be disabled via the derive argument to implement them manually.

#[ink_lang::storage_item(derive = false)]
#[derive(Default)]
struct Contract {
    a: u16,
    b: u64,
    c: u128,
}

// Disabling of deriving allow to implement the trait manually
impl<KEY: StorageKeyHolder> AtomicGuard<true> for Contract<KEY> {}

If you declared and planned to use a non-atomic type twice or more, you need to declare it with a generic storage key KEY: StorageKeyHolder. It will propagate the storage key from parent to child to prevent the same storage key.

#[derive(Default)]
struct NonAtomic<KEY: StorageKeyHolder = AutoKey> {
    field: StorageType<u128>,
}

#[ink_lang::storage_item]
#[derive(Default)]
struct Contract {
    // Storage key - Hash(Hash("Contract::a"), Hash("NonAtomic::field"))
    a: StorageType<NonAtomic>,
    // Storage key - Hash(Hash("Contract::b"), Hash("NonAtomic::field"))
    b: StorageType<NonAtomic>,
}
// Without `NonAtomic<KEY: StorageKeyHolder = AutoKey>` the storage key in both cases is 
// Hash("NonAtomic::field") -> conflicting storage keys

The macro expects StorageKeyHolder naming. It is hardcoded and you can't use aliases.

Storage key rules

Right now hashing is used SHA2 because it allows to do it during compilation time.
Maybe in the future, it will be changed if Blake2b will support const calculation.

The rules to calculate the storage key of the field:

1. Compute the ASCII byte representation of struct_name and call it S.
2. If variant_name is Some then computes the ASCII byte representation and call it V.
3. Compute the ASCII byte representation of field_name and call it F.
4. Concatenate (S and F) or (S, V and F) using :: as separator and call it C.
5. Apply the SHA2 256-bit hash H of C.
6. The first 4 bytes of H make up the storage key.

• variant_name is None for structures and unions.
• if the field is unnamed then field_name is "{}" where {} is a number of the field("0", "1" ...).

If the parent's storage key is passed then use the next rules: If one of the keys is zero, then return another
without hashing. If both keys are non-zero, return the hash of both keys.

If the type is a tuple, then it uses additional rules to calculate the inner storage key:

• The storage key is generated based on the tuple name and field number concatenated with ::.
• The name of the tuple looks like: "(A)", "(A, B)" ... "(A, B, ..., J)" where A, B, ..., J are hardcoded.
• Fields number looks like: "0", "1", ..., "9"

The final:
For (A) it is (A)::0
For (A, B) it is (A, B)::0, (A, B)::1
...
For (A, B, ..., J) it is (A, B, ..., J)::0, (A, B, ..., J)::1, ..., (A, B, ..., J)::9

Contract customizability

• Remove ContractRootKey trait.
• Added support of ManualKey for the contract's storage. Now anyone can specify the default storage key of the contract.

/// A simple ERC-20 contract.
#[ink(storage)]
#[derive(Default)]
pub struct Erc20<K: StorageKeyHolder = ManualKey<123>> {
    /// Total token supply.
    total_supply: Balance,
    /// Mapping from owner to number of owned token.
    balances: Mapping<AccountId, Balance>,
    /// Mapping of the token amount which an account is allowed to withdraw
    /// from another account.
    allowances: Mapping<(AccountId, AccountId), Balance>,
}

• Added a new trait OnCallInitializer. The contract can implement that trait to support initialization on the runtime if it is unable to pull from the storage.
  It can be in several cases:
  • The contract doesn't have constructor. That initializer can be alternative for the constructor.
  • The constructor was not called due to upgrade ability, Proxy or Diamond pattern.
  • The storage was moved or corrupted.
    If the trait is not implemented the behavior of the storage is the default - It should be first initialized by the constructor.

Metadata

The metadata was reworked to be fully compatible with storage key generation rules. The storage layout now uses the u32 layout key instead of [u8; 32]. Each struct or enum layout has a name and all unnamed fields have a enumerate name like "0", "1" etc. Now everyone can calculate the storage key based on the naming that was used. The metadata still is version 3, it should be updated to version 4 in #1265.

Before the generation of the metadata, it checks that the metadata is valid. Right now it can be invalid if some storage keys intersect(maybe more errors in the future).

The conflict storage key error looks like:

The storage layout example (it's pretty large, so it's hidden by default):

{
  "storage": {
    "root": {
      "layout": {
        "struct": {
          "fields": [
            {
              "layout": {
                "leaf": {
                  "key": "0x00000000",
                  "ty": 0
                }
              },
              "name": "balance"
            },
            {
              "layout": {
                "root": {
                  "layout": {
                    "struct": {
                      "fields": [
                        {
                          "layout": {
                            "leaf": {
                              "key": "0x9cbeb3b2",
                              "ty": 0
                            }
                          },
                          "name": "a"
                        },
                        {
                          "layout": {
                            "leaf": {
                              "key": "0x9cbeb3b2",
                              "ty": 1
                            }
                          },
                          "name": "b"
                        },
                        {
                          "layout": {
                            "struct": {
                              "fields": [
                                {
                                  "layout": {
                                    "leaf": {
                                      "key": "0x9cbeb3b2",
                                      "ty": 0
                                    }
                                  },
                                  "name": "0"
                                },
                                {
                                  "layout": {
                                    "leaf": {
                                      "key": "0x9cbeb3b2",
                                      "ty": 2
                                    }
                                  },
                                  "name": "1"
                                }
                              ],
                              "name": "(A, B)"
                            }
                          },
                          "name": "c"
                        }
                      ],
                      "name": "Atomic"
                    }
                  },
                  "root_key": "0x9cbeb3b2"
                }
              },
              "name": "atomics"
            },
            {
              "layout": {
                "root": {
                  "layout": {
                    "struct": {
                      "fields": [
                        {
                          "layout": {
                            "root": {
                              "layout": {
                                "leaf": {
                                  "key": "0x70d94bd5",
                                  "ty": 0
                                }
                              },
                              "root_key": "0x70d94bd5"
                            }
                          },
                          "name": "a"
                        },
                        {
                          "layout": {
                            "root": {
                              "layout": {
                                "struct": {
                                  "fields": [
                                    {
                                      "layout": {
                                        "leaf": {
                                          "key": "0x506b3e18",
                                          "ty": 0
                                        }
                                      },
                                      "name": "0"
                                    },
                                    {
                                      "layout": {
                                        "leaf": {
                                          "key": "0x506b3e18",
                                          "ty": 2
                                        }
                                      },
                                      "name": "1"
                                    }
                                  ],
                                  "name": "(A, B)"
                                }
                              },
                              "root_key": "0x506b3e18"
                            }
                          },
                          "name": "b"
                        },
                        {
                          "layout": {
                            "root": {
                              "layout": {
                                "struct": {
                                  "fields": [
                                    {
                                      "layout": {
                                        "leaf": {
                                          "key": "0x84a13f7d",
                                          "ty": 0
                                        }
                                      },
                                      "name": "0"
                                    },
                                    {
                                      "layout": {
                                        "leaf": {
                                          "key": "0x84a13f7d",
                                          "ty": 2
                                        }
                                      },
                                      "name": "1"
                                    }
                                  ],
                                  "name": "(A, B)"
                                }
                              },
                              "root_key": "0x84a13f7d"
                            }
                          },
                          "name": "c"
                        }
                      ],
                      "name": "NonAtomic"
                    }
                  },
                  "root_key": "0x0000007c"
                }
              },
              "name": "non_atomic"
            }
          ],
          "name": "Erc20"
        }
      },
      "root_key": "0x00000000"
    }
  }
}

Follow-ups

• If GATs are stabilized we can simplify the StorageType and use generics salt in the associated type.
• #[ink_lang::storage_item] may have problems with bounds for inner types. We can implement the approach from that comment.
• Implement a Lazy non-atomic type, that allows loading of the storage type in a lazy manner.
• Fully remove usage of [u8; 32]
• Update metadata to version 4 in polkadot-js

[xgreenx]

The change already works for Erc20 and Erc1155 examples. So you can check what it looks like. If you are okay with it, we can start moving in direction of removing old traits, adding comments, tests and so on.

The size reduction:
Erc20 from 8535 -> 7972 = -563
Erc1155 from 17447 -> 16820 = -627

Example of storage layout of Erc20 with custom root key:

"storage": {
  "struct": {
    "fields": [
      {
        "layout": {
          "cell": {
            "key": "0x00000123",
            "ty": 0
          }
        },
        "name": "total_supply"
      },
      {
        "layout": {
          "cell": {
            "key": "0xd44dbd33",
            "ty": 1
          }
        },
        "name": "balances"
      },
      {
        "layout": {
          "cell": {
            "key": "0x429121b3",
            "ty": 9
          }
        },
        "name": "allowances"
      }
    ]
  }
},

The change has several differences from what we discussed. We added AutoKey and ManualKey as possible keys. If the user specifies the manual key, it overrides the auto key.

If the user wants to use the non-atomic struct twice or more he should specify a generic with StorageKeyHolder instead of const with StorageKey. It is done to provide an ability to the user to specify the manual key for the struct. That feature is used to specify the root storage key for the contract. An example is the Erc20 definition:

/// A simple ERC-20 contract.
#[ink(storage)]
#[derive(Default)]
pub struct Erc20<KEY: StorageKeyHolder = ManualKey<0x123>> {
    /// Total token supply.
    total_supply: Balance,
    /// Mapping from owner to number of owned token.
    balances: StorageMapping<AccountId, Balance>,
    /// Mapping of the token amount which an account is allowed to withdraw
    /// from another account.
    allowances: StorageMapping<(AccountId, AccountId), Balance>,
}

That feature also is useful for upgradable contracts t specify the storage key of the upgradable struct.

Also in lib.rs of the Erc20 example, you can see definitions of atomic and non-atomic structures.

#[ink_lang::storage_item]
struct Atomic {
    s1: u128,
    s2: Vec<u128>,
    // Fails because `StorageType` implemented only for `Vec` where T: AtomicGuard<true>
    // s3: Vec<NonAtomic>,
}

#[ink_lang::storage_item]
struct NonAtomic {
    s1: StorageMapping<u128, u128>,
    s2: StorageValue<u128>,
}

#[ink_lang::storage_item]
struct Jora<KEY: StorageKeyHolder> {
    s1: StorageMapping<u128, u128>,
    s2: StorageValue<u128>,
    s3: StorageMapping<u128, Atomic>,
    s4: StorageValue<NonAtomic>,
    // Fails because: the trait `AtomicGuard<true>` is not implemented for `NonAtomic`
    // s5: StorageMapping<u128, NonAtomic>,
}

In the comment, you can see how it is expanded.

We need to get a feedback from you to work in direction of finilization of the change.

[xgreenx]

We've also commented out on a lot of stuff in lazy module. The question: Do we need to support it and update according to new storage? Or can we remove it, or leave it commented out?

[ascjones]

We've also commented out on a lot of stuff in lazy module. The question: Do we need to support it and update according to new storage? Or can we remove it, or leave it commented out?

IMO we can remove these (and in turn collections) since they are all based on the legacy storage abstractions.

Also they are currently internal and not exposed as part of the public API.

As a future task we can revisit these legacy lazy/collection types to see whether it is worth adapting them to the new storage abstractions in terms of code size/gas costs.

[cmichi]

The StorageKey is u32 but at the end, we convert it into [u8; 32] because the host function expects 32 bytes=(

Could you quickly recap why u32 is fine?

In a next iteration we can introduce a new seal_… host function that takes the primitive directly.

I agree with @ascjones that we can remove Lazy.

From our pov it looks like the right direction, so it's fine if you go ahead with adding docs and tests.

[xgreenx]

Could you quickly recap why u32 is fine?

We agreed on that in the issue. The chance of collision is low and it is still optimal for us. We don't need [u8; 32] because contract-pallet hashes it too.

In a next iteration we can introduce a new seal_… host function that takes the primitive directly.

It will be resolved by paritytech/substrate#11029. I meant that the size will be reduced more in the future, but right now it is [u8; 32]=)

I agree with @ascjones that we can remove Lazy.
From our pov it looks like the right direction, so it's fine if you go ahead with adding docs and tests.

Okay, cool, thanks for review=)

[athei]

In a next iteration we can introduce a new seal_… host function that takes the primitive directly.

The next version will take a ptr/len pair. So arbitrary length. ink! will use u32 for now. Since those keys are stored in static data we can save more space by just using the smallest possible type. We detect collisions by scanning the metadata.

[xgreenx]

The current implementation of the trait that returns the type with the right storage key looks like this:

/// Returns the type that should be used for storing the value
pub trait StorageType<Salt: StorageKeyHolder> {
    /// Type with storage key inside
    type Type;
}

It works well if we don't want to support generics in structures that can be part of the storage. Because we can't define that kind of struct.

// Compilation error because `Salt` is not part of the struct + we will use another `ManualKey` 
struct Generic<T: StorageType<Salt>, Salt: StorageKeyHolder> {
    s1: u128,
    s2: u32,
    s3: T,
}

I created a new StorageType2 trait with upcoming stabilization of GATs. And it helped to simplify the code with auto storage key selection, simplify codegen, and now the usage of the trait is very user-friendly. That allows us to define the structs and enums with generic now. We still have a problem with the auto-generated AtomicGuard trait, but it can be resolved by a manual implementation.

I played with it, and it is better than before, but still, I have issues with #[derive(Default, Debug, ...]. Because it requires bounds for StorageType2::Type<???>: Default + Debug + .... And with GATs I came up with the next solution.
Here is an example of the code.

We can create a fully generic private structure, that structure can have a lot of bounds from the derive. After that, we create an alias to that structure with the same name and same generics. It uses already actual types there.

The user defines the next struct:

#[ink_lang::storage_item]
#[derive(Default, Debug)]
struct Struct<T: StorageType + core::any::Any> {
     s1: u32,
     s2: u64,
     s3: u128,
     s4: T,
}

#[ink_lang::storage_item] generates the next code:

#[allow(non_snake_case)]
mod private_Struct {
    use super::*;
    
    #[derive(Default, Debug)]
    pub struct Struct<A: StorageType, B: StorageType, C: StorageType, D: StorageType> {
        pub s1: A::Type<ManualKey>,
        pub s2: B::Type<ManualKey>,
        pub s3: C::Type<ManualKey>,
        pub s4: D::Type<ManualKey>,
    }
    
    impl<A: StorageType, B: StorageType, C: StorageType, D: StorageType> IsAtomic for Struct<A, B, C, D>
    where
        A: IsAtomic,
        B: IsAtomic,
        C: IsAtomic,
        D: IsAtomic,
    {}
}

#[allow(type_alias_bounds)]
type Struct<T: StorageType + core::any::Any> = private_Struct::Struct<u32, u64, u128, T>;

For the user, it is the same type, but for us, we applied all bounds that we need. It also allows us to improve the implementation of the AtomicGuard<true>. Instead of evaluating it via the is_atomic! macro, we can create a generic implementation (if all types are IsAtomic then the struct is too IsAtomic).

If we believe that GATs would be stabilized, I will use the implementation with GATs and remove the old one.

What do you think about the idea of a private struct?
Pros:

• Support of all derives and unlimited bounds. -> We can implement IsAtomic in a natural way.
• With GATs can support generics.

Cons:

• More complex codegen.
• Maybe, not clear errors for the user with aliases.
• It can break other macro that are used by the user.
• It is dark magic=)

The idea with a private struct can be implemented without GATs, but that means that we will not support generics=) If you are okay with that idea I will implement it.

[ascjones]

It works well if we don't want to support generics in structures that can be part of the storage

Do we really need to support generics for this first iteration? The current storage struct does not allow generics so just to replicate that would be good enough, and we can add that as future enhancement?

If we believe that GATs would be stabilized, I will use the implementation with GATs and remove the old one.

I don't know whether we can count on that, there still seems to be some contention in the stabilization PR, and it has been "coming soon" for a long time. I personally think it is a really nice feature but I'm wary of depending upon unstable features, there should be a very strong justification.

What do you think about the idea of a private struct?

Legacy versions of ink! heavily depended on private sub modules in codegen but we (more specifically @Robbepop, IIRC starting in #470) had a major effort to remove all of them from the codegen, moving to traits + associated types for addressing concrete implementions, in order to use the Rust type system rather than the more brittle name resolution. I'm sure you are very familiar with this technique from working in the ink! codegen.

At the very least we should consider if it is possible to use the same technique in this situation.

I still need to spend more time looking into your WIP here to get my head fully around it, in order to provide some more in depth feedback.

[xgreenx]

Do we really need to support generics for this first iteration? The current storage struct does not allow generics so just to replicate that would be good enough, and we can add that as a future enhancement?

It is not only about the storage of the contract it is about the #[ink_lang::storage_item] macro. All types used in the storage should be defined via that macro to automate key calculation. And the problem is that users can normally declare their generic types.

The workaround: implement all traits manually. So it is not critical, and we can do that in follow-up PR. If something is implemented wrongly, the user can still have bugs, but we can handle it by documentation.

I don't know whether we can count on that, there still seems to be some contention in the stabilization PR, and it has been "coming soon" for a long time. I personally think it is a really nice feature but I'm wary of depending upon unstable features, there should be a very strong justification.

Okay, then let's integrate GATs in follow-up PR when it will be stabilized=)

[ascjones]

The workaround: implement all traits manually. So it is not critical, and we can do that in follow-up PR. If something is implemented wrongly, the user can still have bugs, but we can handle it by documentation.

I'm okay with doing this for the first iteration. My preference is to get something as simple as possible working from this PR, then iterate upon that. Then it will be easier to review and there will be faster feedback.

[xgreenx]

I'm okay with doing this for the first iteration. My preference is to get something as simple as possible working from this PR, then iterate upon that. Then it will be easier to review and there will be faster feedback.

Yep, I got it=) I've already updated everything to use StorageType. All old tests are green now. Now I need to fix the latest comments and add some new tests and the change is ready for review=)

[HCastano]

Alright, so imo this PR is too thorny to review in its current state. It touches too many
different (but related) parts of the code.

If we ever want to get this merged we need to make it easily reviewable. I suggest we set
up of series of stacked PRs whose individual reviews should be quick.

By stacking I mean something like:

master
    |- working-base
        |- feature-a [1/2]
            |- feature-b [2/2]

Some blog posts about stacked PRs

• https://jg.gg/2018/09/29/stacked-diffs-versus-pull-requests/
• https://timothya.com/blog/git-stack/
• https://blog.logrocket.com/using-stacked-pull-requests-in-github/

You obviously know the PR better than we do, but here's the order of the stack I would
suggest:

• Primitives
• Traits
• Derive Macros for Traits (may need to be split into PRs per macro + tests)
• IR + Generators
• #[ink::storage_item] Macro
• Codegen
• ink_env API
• Data Structures (Lazy, Mapping)
• Metadata (with associated codegen)
• Examples
• Old code removal (may also be done first, or along the way if it's not too distruptive)

If some of these PRs are 50 lines long, that's fine. If they're like 2500, please find a
way to further split them.

Thanks again for you work on this 🙇

[xgreenx]

Moved to #1331