Use Mandatory Indices for FRAME Storage API (and others)

[shawntabrizi]

The FRAME storage APIs use a combination of prefixes to describe the location of storage items. Something like:

hash(pallet_name) + hash(storage_name) + hash(key_value) + ...

We have seen multiple times where this has come to bite us in the butt:

• Recent migrations from frame v1 -> v2 where we changed the string used for the hasher
• Any changes to the name of a storage item (typos, better clarification, etc....)

Similarly, we ran into issues with Construct Runtime and the ordering of pallets, and the call enum and ordering of extrinsics.

My suggestion is that we should use indexes, not human strings / variable names, wherever there could be some breaking change caused by reorganization, renaming, refactoring, etc...

Protobuf follows a similar philosophy: https://developers.google.com/protocol-buffers/docs/overview

I believe the following FRAME objects should use explicitly defined indexes:

1. Pallets in Construct Runtime
2. Calls in Pallet
3. Storage in Pallet

Items which are not breaking need not have explicit ordering, but should allow optional explicit ordering:

• Errors in Pallet
• Events in Pallet

When defining a storage item with the FRAME v2 macros, users should be required(?) to include an explicit index:

#[pallet::storage]
#[pallet::storage::index(0)]  <-- this
#[pallet::getter(fn auction_counter)]
pub type AuctionCounter<T> = StorageValue<_, AuctionIndex, ValueQuery>;

We already support and require indices on Pallet ordering in construct runtime. This is needed for transaction versioning.

Combining these two, the new storage pattern should be:

hash(pallet_index) + hash(storage_index) + hash(key_value) + ...

In this case, we can update the name of the storage or move the storage definition to anywhere in the source code, and not break your pallet.

We still want to hash these indices so that pallet storage is in its own trie branch.

We have also wanted to put all pallet storage into its own child trie, so maybe we should do that here too.

[kianenigma]

Pallets in Construct Runtime
Calls in Pallet
Storage in Pallet

How would the index look in a Call? This might be related to: paritytech/polkadot#1823 (comment)

To recap, I think it is useful to be able to pin certain calls variants to particular indices in the call enum, so that we can guarantee that they will remain backwards compatible. For example, we could freeze the index of balances::transfer and even when transaction version changes, a hardware wallet vendor can continue to operate on this call (unless, if, the change explicitly changed the semantic of transfer).

Although, this is more like a patch for the lack of resources on readily parsing of metadata on the tools side. Perhaps it is more worthwhile to spend more effort metadata parsing. If we have great tooling for metadata in different languages, it won't matter at all how frequent we break storage/calls.

[shawntabrizi]

I thought it would just be another tag on top of each of the extrinsic functions?

#[pallet::call::index(0)]
fn transfer (...) -> DispatchResult { ... }

[shawntabrizi]

Feedback in chat determined the following:

1. We should not make this mandatory.
2. We can simply introduce a more general attribute which controls the prefix used:

#[storge_name="something-ther-than-StructName-or-index"]
type StructName = StorageValue<_, ...>

Then anything which impls encode can be used here, whether it be an old name (for migration compatibility), or a number (for index), or any other custom static type.