From 55e5b2147827b4256ff6e6fead6349071b281c21 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 26 Sep 2019 11:44:52 +0200 Subject: [PATCH] Improve storage doc (#3691) * doc * fix * Apply suggestions from code review Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * complete suggestion --- substrate/srml/support/procedural/src/lib.rs | 54 +++++++++---------- .../src/storage/generator/double_map.rs | 14 +++-- .../src/storage/generator/linked_map.rs | 22 +++++++- .../srml/support/src/storage/generator/map.rs | 10 +++- .../support/src/storage/generator/value.rs | 5 +- 5 files changed, 70 insertions(+), 35 deletions(-) diff --git a/substrate/srml/support/procedural/src/lib.rs b/substrate/srml/support/procedural/src/lib.rs index ea6a8a7995..a9cfa8d30d 100644 --- a/substrate/srml/support/procedural/src/lib.rs +++ b/substrate/srml/support/procedural/src/lib.rs @@ -44,12 +44,18 @@ use proc_macro::TokenStream; /// with `Store` a (pub) trait generated associating each storage item to the `Module` and /// `as Example` setting the prefix used for storage items of this module. `Example` must be unique: /// another module with the same name and the same inner storage item name will conflict. +/// `Example` is called the module prefix. +/// +/// note: For instantiable modules the module prefix is prepended with instance +/// prefix. Instance prefix is "" for default instance and "Instance$n" for instance number $n. +/// Thus, instance 3 of module Example has a module prefix of `Instance3Example` /// /// Basic storage consists of a name and a type; supported types are: /// /// * Value: `Foo: type`: Implements the /// [`StorageValue`](../srml_support/storage/trait.StorageValue.html) trait using the /// [`StorageValue generator`](../srml_support/storage/generator/trait.StorageValue.html). +/// The generator `unhashed_key` is `$module_prefix ++ " " ++ $storage_name` /// /// * Map: `Foo: map hasher($hash) type => type`: Implements the /// [`StorageMap`](../srml_support/storage/trait.StorageMap.html) trait using the @@ -58,12 +64,12 @@ use proc_macro::TokenStream; /// `$hash` representing a choice of hashing algorithms available in the /// [`Hashable`](../srml_support/trait.Hashable.html) trait. /// -/// `hasher($hash)` is optional and its default is `blake2_256`. +/// `hasher($hash)` is optional and its default is `blake2_256`. One should use another hasher +/// with care, see generator documentation. /// -/// /!\ Be careful with each key in the map that is inserted in the trie -/// `$hash(module_name ++ " " ++ storage_name ++ encoding(key))`. -/// If the keys are not trusted (e.g. can be set by a user), a cryptographic `hasher` such as -/// `blake2_256` must be used. Otherwise, other values in storage can be compromised. +/// The generator is implemented with: +/// * `prefix`: `$module_prefix ++ " " ++ $storage_name` +/// * `Hasher`: $hash /// /// * Linked map: `Foo: linked_map hasher($hash) type => type`: Implements the /// [`StorageLinkedMap`](../srml_support/storage/trait.StorageLinkedMap.html) trait using the @@ -72,39 +78,26 @@ use proc_macro::TokenStream; /// `$hash` representing a choice of hashing algorithms available in the /// [`Hashable`](../srml_support/trait.Hashable.html) trait. /// -/// `hasher($hash)` is optional and its default is `blake2_256`. +/// `hasher($hash)` is optional and its default is `blake2_256`. One should use another hasher +/// with care, see generator documentation. /// -/// /!\ Be careful with each key in the map that is inserted in the trie -/// `$hash(module_name ++ " " ++ storage_name ++ encoding(key))`. -/// If the keys are not trusted (e.g. can be set by a user), a cryptographic `hasher` such as -/// `blake2_256` must be used. Otherwise, other values in storage can be compromised. -/// -/// Once read from the trie (e.g. through an RPC call), a tuple of `(T, Linkage)` is returned, -/// where `T` is the value and `K` is the key type of the mapping. -/// [`Linkage`](../srml_support/storage/generator/struct.Linkage.html) is a pointer to the -/// previous and the next element. -/// -/// Each linked map has a special key in the trie that can be queried and it can be particularly -/// useful to query all items: `"head of " ++ module_name ++ " " ++ storage_name`). This will -/// return a key `K` that can be later on used with the aforementioned `$hash(module_name ++ " " -/// ++ storage_name ++ encoding(K))` to fetch the head. For consequent elements, the -/// [`Linkage`](../srml_support/storage/generator/struct.Linkage.html) can be used. +/// The generator is implemented with: +/// * `prefix`: `$module_prefix ++ " " ++ $storage_name` +/// * `head_key`: `"head of " ++ $module_prefix ++ " " ++ $storage_name` +/// * `Hasher`: $hash /// /// * Double map: `Foo: double_map hasher($hash1) u32, $hash2(u32) => u32`: Implements the /// [`StorageDoubleMap`](../srml_support/storage/trait.StorageDoubleMap.html) trait using the /// [`StorageDoubleMap generator`](../srml_support/storage/generator/trait.StorageDoubleMap.html). /// /// `$hash1` and `$hash2` representing choices of hashing algorithms available in the -/// [`Hashable`](../srml_support/trait.Hashable.html) trait. +/// [`Hashable`](../srml_support/trait.Hashable.html) trait. They must be choosen with care, see +/// generator documentation. /// /// `hasher($hash)` is optional and its default is `blake2_256`. /// -/// /!\ Be careful with each key pair in the double map that is inserted in the trie. -/// The final key is calculated as follows: -/// -/// ```nocompile -/// $hash(module_name ++ " " ++ storage_name ++ encoding(first_key)) ++ $hash2(encoding(second_key)) -/// ``` +/// `hasher($hash)` is optional and its default is `blake2_256`. One should use another hasher +/// with care, see generator documentation. /// /// If the first key is untrusted, a cryptographic `hasher` such as `blake2_256` must be used. /// Otherwise, other values of all storage items can be compromised. @@ -112,6 +105,11 @@ use proc_macro::TokenStream; /// If the second key is untrusted, a cryptographic `hasher` such as `blake2_256` must be used. /// Otherwise, other items in storage with the same first key can be compromised. /// +/// The generator is implemented with: +/// * `key1_prefix`: `$module_prefix ++ " " ++ $storage_name` +/// * `Hasher1`: $hash1 +/// * `Hasher2`: $hash2 +/// /// Supported hashers (ordered from least to best security): /// /// * `twox_64_concat` - TwoX with 64bit + key concatenated. diff --git a/substrate/srml/support/src/storage/generator/double_map.rs b/substrate/srml/support/src/storage/generator/double_map.rs index 46361340ac..b41fe47422 100644 --- a/substrate/srml/support/src/storage/generator/double_map.rs +++ b/substrate/srml/support/src/storage/generator/double_map.rs @@ -26,10 +26,18 @@ use crate::{storage::{self, unhashed, hashed::StorageHasher}, rstd::borrow::Borr /// The first part is a hash of a concatenation of the `key1_prefix` and `Key1`. And the second part /// is a hash of a `Key2`. /// -/// Thus value for (key1, key2) is stored at `Hasher1(key1_prefix ++ key1) ++ Hasher2(key2)`. +/// Thus value for (key1, key2) is stored at: +/// ```nocompile +/// Hasher1(key1_prefix ++ key1) ++ Hasher2(key2) +/// ``` /// -/// /!\ be careful while choosing the Hash, indeed malicious could craft second keys to lower the -/// trie. +/// # Warning +/// +/// If the key1s are not trusted (e.g. can be set by a user), a cryptographic `hasher` such as +/// `blake2_256` must be used for Hasher1. Otherwise, other values in storage can be compromised. +/// If the key2s are not trusted (e.g. can be set by a user), a cryptographic `hasher` such as +/// `blake2_256` must be used for Hasher2. Otherwise, other items in storage with the same first +/// key can be compromised. pub trait StorageDoubleMap { /// The type that get/take returns. type Query; diff --git a/substrate/srml/support/src/storage/generator/linked_map.rs b/substrate/srml/support/src/storage/generator/linked_map.rs index 2c69df4364..d11c025afb 100644 --- a/substrate/srml/support/src/storage/generator/linked_map.rs +++ b/substrate/srml/support/src/storage/generator/linked_map.rs @@ -23,8 +23,26 @@ use rstd::{ /// Generator for `StorageLinkedMap` used by `decl_storage`. /// -/// For each key value is stored at `Hasher(prefix ++ key)` along with a linkage used for -/// enumeration. +/// # Mapping of keys to a storage path +/// +/// The key for the head of the map is stored at one fixed path: +/// ```nocompile +/// Hasher(head_key) +/// ``` +/// +/// For each key, the value stored under that key is appended with a +/// [`Linkage`](struct.Linkage.html) (which hold previous and next key) at the path: +/// ```nocompile +/// Hasher(prefix ++ key) +/// ``` +/// +/// Enumeration is done by getting the head of the linked map and then iterating getting the +/// value and linkage stored at the key until the found linkage has no next key. +/// +/// # Warning +/// +/// If the keys are not trusted (e.g. can be set by a user), a cryptographic `hasher` such as +/// `blake2_256` must be used. Otherwise, other values in storage can be compromised. pub trait StorageLinkedMap { /// The type that get/take returns. type Query; diff --git a/substrate/srml/support/src/storage/generator/map.rs b/substrate/srml/support/src/storage/generator/map.rs index 48b8cc097f..f5b22fc19a 100644 --- a/substrate/srml/support/src/storage/generator/map.rs +++ b/substrate/srml/support/src/storage/generator/map.rs @@ -22,7 +22,15 @@ use crate::{storage::{self, unhashed, hashed::StorageHasher}, traits::Len}; /// Generator for `StorageMap` used by `decl_storage`. /// -/// For each key value is stored at `Hasher(prefix ++ key)`. +/// For each key value is stored at: +/// ```nocompile +/// Hasher(prefix ++ key) +/// ``` +/// +/// # Warning +/// +/// If the keys are not trusted (e.g. can be set by a user), a cryptographic `hasher` such as +/// `blake2_256` must be used. Otherwise, other values in storage can be compromised. pub trait StorageMap { /// The type that get/take returns. type Query; diff --git a/substrate/srml/support/src/storage/generator/value.rs b/substrate/srml/support/src/storage/generator/value.rs index 69851db1a0..2f632d94d3 100644 --- a/substrate/srml/support/src/storage/generator/value.rs +++ b/substrate/srml/support/src/storage/generator/value.rs @@ -22,7 +22,10 @@ use crate::{storage::{self, unhashed, hashed::{Twox128, StorageHasher}}, traits: /// Generator for `StorageValue` used by `decl_storage`. /// -/// Value is stored at `Twox128(unhashed_key)`. +/// Value is stored at: +/// ```nocompile +/// Twox128(unhashed_key) +/// ``` pub trait StorageValue { /// The type that get/take returns. type Query;