From ee6633e0385526a8c6fe31e6ce80d4bc946ce5fd Mon Sep 17 00:00:00 2001 From: Demi Obenour Date: Thu, 21 May 2020 11:57:29 +0000 Subject: [PATCH] Add notes about safe uses of twox (#6082) * Add notes about safe uses of twox * Update frame/grandpa/src/lib.rs Co-authored-by: Nikolay Volf * Update frame/elections/src/lib.rs * Apply suggestions from code review Co-authored-by: Gavin Wood Co-authored-by: Nikolay Volf --- substrate/frame/assets/src/lib.rs | 2 ++ substrate/frame/babe/src/lib.rs | 2 ++ substrate/frame/contracts/src/lib.rs | 2 ++ substrate/frame/democracy/src/lib.rs | 10 ++++++++++ substrate/frame/elections-phragmen/src/lib.rs | 2 ++ substrate/frame/elections/src/lib.rs | 9 +++++++++ substrate/frame/generic-asset/src/lib.rs | 8 ++++++++ substrate/frame/grandpa/src/lib.rs | 2 ++ substrate/frame/identity/src/lib.rs | 4 ++++ 9 files changed, 41 insertions(+) diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index 48806e30cd..2c67a320c1 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/substrate/frame/assets/src/lib.rs @@ -257,6 +257,8 @@ decl_storage! { /// The next asset identifier up for grabs. NextAssetId get(fn next_asset_id): T::AssetId; /// The total unit supply of an asset. + /// + /// TWOX-NOTE: `AssetId` is trusted, so this is safe. TotalSupply: map hasher(twox_64_concat) T::AssetId => T::Balance; } } diff --git a/substrate/frame/babe/src/lib.rs b/substrate/frame/babe/src/lib.rs index 153ff0e992..9142173932 100644 --- a/substrate/frame/babe/src/lib.rs +++ b/substrate/frame/babe/src/lib.rs @@ -152,6 +152,8 @@ decl_storage! { /// We reset all segments and return to `0` at the beginning of every /// epoch. SegmentIndex build(|_| 0): u32; + + /// TWOX-NOTE: `SegmentIndex` is an increasing integer, so this is okay. UnderConstruction: map hasher(twox_64_concat) u32 => Vec; /// Temporary value (cleared at block finalization) which is `Some` diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index df53cf0a0e..509229cd96 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -836,6 +836,8 @@ decl_storage! { /// The subtrie counter. pub AccountCounter: u64 = 0; /// The code associated with a given account. + /// + /// TWOX-NOTE: SAFE since `AccountId` is a secure hash. pub ContractInfoOf: map hasher(twox_64_concat) T::AccountId => Option>; } } diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs index 039d48d75c..ee9417ce0c 100644 --- a/substrate/frame/democracy/src/lib.rs +++ b/substrate/frame/democracy/src/lib.rs @@ -344,6 +344,8 @@ decl_storage! { /// The public proposals. Unsorted. The second item is the proposal's hash. pub PublicProps get(fn public_props): Vec<(PropIndex, T::Hash, T::AccountId)>; /// Those who have locked a deposit. + /// + /// TWOX-NOTE: Safe, as increasing integer keys are safe. pub DepositOf get(fn deposit_of): map hasher(twox_64_concat) PropIndex => Option<(Vec, BalanceOf)>; @@ -362,22 +364,30 @@ decl_storage! { pub LowestUnbaked get(fn lowest_unbaked) build(|_| 0 as ReferendumIndex): ReferendumIndex; /// Information concerning any given referendum. + /// + /// TWOX-NOTE: SAFE as indexes are not under an attacker’s control. pub ReferendumInfoOf get(fn referendum_info): map hasher(twox_64_concat) ReferendumIndex => Option>>; /// All votes for a particular voter. We store the balance for the number of votes that we /// have recorded. The second item is the total amount of delegations, that will be added. + /// + /// TWOX-NOTE: SAFE as `AccountId`s are crypto hashes anyway. pub VotingOf: map hasher(twox_64_concat) T::AccountId => Voting, T::AccountId, T::BlockNumber>; /// Who is able to vote for whom. Value is the fund-holding account, key is the /// vote-transaction-sending account. + /// + /// TWOX-NOTE: OK ― `AccountId` is a secure hash. // TODO: Refactor proxy into its own pallet. // https://github.com/paritytech/substrate/issues/5322 pub Proxy get(fn proxy): map hasher(twox_64_concat) T::AccountId => Option>; /// Accounts for which there are locks in action which may be removed at some point in the /// future. The value is the block number at which the lock expires and may be removed. + /// + /// TWOX-NOTE: OK ― `AccountId` is a secure hash. pub Locks get(fn locks): map hasher(twox_64_concat) T::AccountId => Option; /// True if the last referendum tabled was submitted externally. False if it was a public diff --git a/substrate/frame/elections-phragmen/src/lib.rs b/substrate/frame/elections-phragmen/src/lib.rs index 0c35283e1a..5d7d2bf503 100644 --- a/substrate/frame/elections-phragmen/src/lib.rs +++ b/substrate/frame/elections-phragmen/src/lib.rs @@ -197,6 +197,8 @@ decl_storage! { pub ElectionRounds get(fn election_rounds): u32 = Zero::zero(); /// Votes and locked stake of a particular voter. + /// + /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash pub Voting get(fn voting): map hasher(twox_64_concat) T::AccountId => (BalanceOf, Vec); /// The present candidate list. Sorted based on account-id. A current member or runner-up diff --git a/substrate/frame/elections/src/lib.rs b/substrate/frame/elections/src/lib.rs index b87db45909..1085831373 100644 --- a/substrate/frame/elections/src/lib.rs +++ b/substrate/frame/elections/src/lib.rs @@ -237,16 +237,25 @@ decl_storage! { // bit-wise manner. In order to get a human-readable representation (`Vec`), use // [`all_approvals_of`]. Furthermore, each vector of scalars is chunked with the cap of // `APPROVAL_SET_SIZE`. + /// + /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash and `SetIndex` is not + /// attacker-controlled. pub ApprovalsOf get(fn approvals_of): map hasher(twox_64_concat) (T::AccountId, SetIndex) => Vec; /// The vote index and list slot that the candidate `who` was registered or `None` if they /// are not currently registered. + /// + /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash. pub RegisterInfoOf get(fn candidate_reg_info): map hasher(twox_64_concat) T::AccountId => Option<(VoteIndex, u32)>; /// Basic information about a voter. + /// + /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash. pub VoterInfoOf get(fn voter_info): map hasher(twox_64_concat) T::AccountId => Option>>; /// The present voter list (chunked and capped at [`VOTER_SET_SIZE`]). + /// + /// TWOX-NOTE: OKAY ― `SetIndex` is not user-controlled data. pub Voters get(fn voters): map hasher(twox_64_concat) SetIndex => Vec>; /// the next free set to store a voter in. This will keep growing. pub NextVoterSet get(fn next_nonfull_voter_set): SetIndex = 0; diff --git a/substrate/frame/generic-asset/src/lib.rs b/substrate/frame/generic-asset/src/lib.rs index 646e217366..f94c83b5ed 100644 --- a/substrate/frame/generic-asset/src/lib.rs +++ b/substrate/frame/generic-asset/src/lib.rs @@ -442,16 +442,22 @@ pub struct BalanceLock { decl_storage! { trait Store for Module as GenericAsset { /// Total issuance of a given asset. + /// + /// TWOX-NOTE: `AssetId` is trusted. pub TotalIssuance get(fn total_issuance) build(|config: &GenesisConfig| { let issuance = config.initial_balance * (config.endowed_accounts.len() as u32).into(); config.assets.iter().map(|id| (id.clone(), issuance)).collect::>() }): map hasher(twox_64_concat) T::AssetId => T::Balance; /// The free balance of a given asset under an account. + /// + /// TWOX-NOTE: `AssetId` is trusted. pub FreeBalance: double_map hasher(twox_64_concat) T::AssetId, hasher(blake2_128_concat) T::AccountId => T::Balance; /// The reserved balance of a given asset under an account. + /// + /// TWOX-NOTE: `AssetId` is trusted. pub ReservedBalance: double_map hasher(twox_64_concat) T::AssetId, hasher(blake2_128_concat) T::AccountId => T::Balance; @@ -459,6 +465,8 @@ decl_storage! { pub NextAssetId get(fn next_asset_id) config(): T::AssetId; /// Permission options for a given asset. + /// + /// TWOX-NOTE: `AssetId` is trusted. pub Permissions get(fn get_permission): map hasher(twox_64_concat) T::AssetId => PermissionVersions; diff --git a/substrate/frame/grandpa/src/lib.rs b/substrate/frame/grandpa/src/lib.rs index 43b6ba0b2f..3432c11020 100644 --- a/substrate/frame/grandpa/src/lib.rs +++ b/substrate/frame/grandpa/src/lib.rs @@ -212,6 +212,8 @@ decl_storage! { /// A mapping from grandpa set ID to the index of the *most recent* session for which its /// members were responsible. + /// + /// TWOX-NOTE: `SetId` is not under user control. SetIdSession get(fn session_for_set): map hasher(twox_64_concat) SetId => Option; } add_extra_genesis { diff --git a/substrate/frame/identity/src/lib.rs b/substrate/frame/identity/src/lib.rs index 6a69797c90..2b58437685 100644 --- a/substrate/frame/identity/src/lib.rs +++ b/substrate/frame/identity/src/lib.rs @@ -389,6 +389,8 @@ pub struct RegistrarInfo< decl_storage! { trait Store for Module as Identity { /// Information that is pertinent to identify the entity behind an account. + /// + /// TWOX-NOTE: OK ― `AccountId` is a secure hash. pub IdentityOf get(fn identity): map hasher(twox_64_concat) T::AccountId => Option>>; @@ -400,6 +402,8 @@ decl_storage! { /// Alternative "sub" identities of this account. /// /// The first item is the deposit, the second is a vector of the accounts. + /// + /// TWOX-NOTE: OK ― `AccountId` is a secure hash. pub SubsOf get(fn subs_of): map hasher(twox_64_concat) T::AccountId => (BalanceOf, Vec);