diff --git a/dip-template/pallets/pallet-postit/src/lib.rs b/dip-template/pallets/pallet-postit/src/lib.rs index e3c9c38b9b..72443fff7e 100644 --- a/dip-template/pallets/pallet-postit/src/lib.rs +++ b/dip-template/pallets/pallet-postit/src/lib.rs @@ -57,10 +57,14 @@ pub mod pallet { type Username: Encode + Decode + TypeInfo + MaxEncodedLen + Clone + PartialEq + Debug + Default; } + // Safety: The hash is calculated on chain. The hashing algorithm provided by + // frame_system::Config must be cryptographically secure. #[pallet::storage] #[pallet::getter(fn posts)] pub type Posts = StorageMap<_, Twox64Concat, ::Hash, PostOf>; + // Safety: The hash is calculated on chain. The hashing algorithm provided by + // frame_system::Config must be cryptographically secure. #[pallet::storage] #[pallet::getter(fn comments)] pub type Comments = StorageMap<_, Twox64Concat, ::Hash, CommentOf>; diff --git a/nodes/parachain/src/chain_spec/clone.rs b/nodes/parachain/src/chain_spec/clone.rs index 099a75f36c..f002be4a5c 100644 --- a/nodes/parachain/src/chain_spec/clone.rs +++ b/nodes/parachain/src/chain_spec/clone.rs @@ -22,14 +22,12 @@ use cumulus_primitives_core::ParaId; use hex_literal::hex; use runtime_common::constants::KILT; use sc_chain_spec::ChainType; -use sp_core::crypto::UncheckedInto; -use sp_core::sr25519; +use sp_core::{crypto::UncheckedInto, sr25519}; use clone_runtime::{ - BalancesConfig, ParachainInfoConfig, PolkadotXcmConfig, RuntimeGenesisConfig, SessionConfig, SudoConfig, - SystemConfig, + BalancesConfig, CollatorSelectionConfig, ParachainInfoConfig, PolkadotXcmConfig, RuntimeGenesisConfig, + SessionConfig, SudoConfig, SystemConfig, WASM_BINARY, }; -use clone_runtime::{CollatorSelectionConfig, WASM_BINARY}; use runtime_common::{AccountId, AuthorityId, Balance}; use super::{get_account_id_from_seed, get_from_seed, get_properties, DEFAULT_PARA_ID}; diff --git a/pallets/attestation/src/lib.rs b/pallets/attestation/src/lib.rs index 230db30308..f95d2e17d0 100644 --- a/pallets/attestation/src/lib.rs +++ b/pallets/attestation/src/lib.rs @@ -105,7 +105,7 @@ pub mod pallet { }; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(2); /// Type of a claim hash. pub type ClaimHashOf = ::Hash; @@ -194,9 +194,9 @@ pub mod pallet { /// /// It maps from a delegation ID to a vector of claim hashes. #[pallet::storage] - #[pallet::getter(fn external_attestations)] - pub type ExternalAttestations = - StorageDoubleMap<_, Twox64Concat, AuthorizationIdOf, Blake2_128Concat, ClaimHashOf, bool, ValueQuery>; + // #[pallet::getter(fn external_attestations)] + pub(crate) type ExternalAttestations = + StorageDoubleMap<_, Blake2_128Concat, AuthorizationIdOf, Blake2_128Concat, ClaimHashOf, bool, ValueQuery>; #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -501,6 +501,13 @@ pub mod pallet { } impl Pallet { + // TODO: Delete this and add `#[pallet::getter(fn external_attestations)]` once + // the migration is over. + pub fn external_attestations(authorization_id: AuthorizationIdOf, claim_hash: ClaimHashOf) -> bool { + ExternalAttestations::::get(&authorization_id, claim_hash) + || migrations::v1::ExternalAttestations::::get(&authorization_id, claim_hash) + } + fn remove_attestation( authorized_by: AuthorizedByOf, attestation: AttestationDetailsOf, @@ -520,6 +527,7 @@ pub mod pallet { Attestations::::remove(claim_hash); if let Some(authorization_id) = &attestation.authorization_id { ExternalAttestations::::remove(authorization_id, claim_hash); + migrations::v1::ExternalAttestations::::remove(authorization_id, claim_hash); } if !attestation.revoked { Self::deposit_event(Event::AttestationRevoked { diff --git a/pallets/attestation/src/migrations.rs b/pallets/attestation/src/migrations.rs index e0dbe7d5ab..d67b1b269f 100644 --- a/pallets/attestation/src/migrations.rs +++ b/pallets/attestation/src/migrations.rs @@ -37,6 +37,23 @@ where ) } +pub mod v1 { + use frame_support::{pallet_prelude::*, storage_alias, Blake2_128Concat}; + + use crate::{AuthorizationIdOf, ClaimHashOf, Config, Pallet}; + + #[storage_alias] + pub(crate) type ExternalAttestations = StorageDoubleMap< + Pallet, + Twox64Concat, + AuthorizationIdOf, + Blake2_128Concat, + ClaimHashOf, + bool, + ValueQuery, + >; +} + #[cfg(test)] pub mod test { use ctype::mock::get_ctype_hash; diff --git a/pallets/attestation/src/mock.rs b/pallets/attestation/src/mock.rs index e8c9a213b7..dd26bd5321 100644 --- a/pallets/attestation/src/mock.rs +++ b/pallets/attestation/src/mock.rs @@ -167,6 +167,19 @@ pub fn insert_attestation(claim_hash: ClaimHashOf, details: Attest } } +pub fn insert_attestation_v1(claim_hash: ClaimHashOf, details: AttestationDetailsOf) { + crate::AttestationStorageDepositCollector::::create_deposit( + details.deposit.owner.clone(), + details.deposit.amount, + ) + .expect("Should have balance"); + + crate::Attestations::::insert(claim_hash, details.clone()); + if let Some(delegation_id) = details.authorization_id.as_ref() { + crate::migrations::v1::ExternalAttestations::::insert(delegation_id, claim_hash, true) + } +} + pub fn sr25519_did_from_public_key(public_key: &[u8; 32]) -> SubjectId { MultiSigner::from(sr25519::Public(*public_key)).into_account().into() } @@ -352,6 +365,7 @@ pub(crate) mod runtime { /// endowed accounts with balances balances: Vec<(AccountIdOf, BalanceOf)>, attestations: Vec<(ClaimHashOf, AttestationDetailsOf)>, + attestations_v1: Vec<(ClaimHashOf, AttestationDetailsOf)>, } impl ExtBuilder { @@ -373,6 +387,15 @@ pub(crate) mod runtime { self } + #[must_use] + pub fn with_attestations_v1( + mut self, + attestations: Vec<(ClaimHashOf, AttestationDetailsOf)>, + ) -> Self { + self.attestations_v1 = attestations; + self + } + pub fn build(self) -> sp_io::TestExternalities { let mut storage = frame_system::GenesisConfig::::default().build_storage().unwrap(); pallet_balances::GenesisConfig:: { @@ -401,6 +424,10 @@ pub(crate) mod runtime { for (claim_hash, details) in self.attestations { insert_attestation::(claim_hash, details); } + + for (claim_hash, details) in self.attestations_v1 { + insert_attestation_v1::(claim_hash, details); + } }); ext diff --git a/pallets/attestation/src/tests/delete.rs b/pallets/attestation/src/tests/delete.rs index e61cc73fdb..d9b5450ca0 100644 --- a/pallets/attestation/src/tests/delete.rs +++ b/pallets/attestation/src/tests/delete.rs @@ -20,7 +20,7 @@ use frame_support::{assert_noop, assert_ok, traits::fungible::InspectHold}; use kilt_support::mock::mock_origin::DoubleOrigin; use sp_runtime::traits::Zero; -use crate::{self as attestation, mock::*, AttesterOf, Config, Event, HoldReason}; +use crate::{self as attestation, migrations, mock::*, AttesterOf, Config, Event, ExternalAttestations, HoldReason}; #[test] fn test_remove() { @@ -106,6 +106,9 @@ fn test_remove_authorized() { .with_ctypes(vec![(attestation.ctype_hash, revoker.clone())]) .with_attestations(vec![(claim_hash, attestation)]) .build_and_execute_with_sanity_tests(|| { + assert!(Attestation::attestations(claim_hash).is_some()); + assert!(Attestation::external_attestations(revoker.clone(), claim_hash)); + assert_ok!(Attestation::remove( DoubleOrigin(ACCOUNT_00, revoker.clone()).into(), claim_hash, @@ -174,3 +177,55 @@ fn test_remove_not_found() { ); }); } + +#[test] +fn test_remove_v1() { + let attester: AttesterOf = sr25519_did_from_public_key(&ALICE_SEED); + let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); + let mut attestation = generate_base_attestation::(attester.clone(), ACCOUNT_00); + attestation.authorization_id = Some(attester.clone()); + let ctype_hash = attestation.ctype_hash; + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, ::Deposit::get() * 100)]) + .with_ctypes(vec![(attestation.ctype_hash, attester.clone())]) + .with_attestations_v1(vec![(claim_hash, attestation)]) + .build_and_execute_with_sanity_tests(|| { + assert!(Attestation::attestations(claim_hash).is_some()); + assert!(!ExternalAttestations::::get(attester.clone(), claim_hash)); + assert!(migrations::v1::ExternalAttestations::::get( + attester.clone(), + claim_hash + )); + + assert_ok!(Attestation::remove( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + claim_hash, + None + )); + assert!(Attestation::attestations(claim_hash).is_none()); + assert!(!ExternalAttestations::::get(attester.clone(), claim_hash)); + assert!(!migrations::v1::ExternalAttestations::::get( + attester.clone(), + claim_hash + )); + + assert_eq!( + events(), + vec![ + Event::AttestationRevoked { + attester: attester.clone(), + claim_hash, + authorized_by: attestation::authorized_by::AuthorizedBy::Attester(attester.clone()), + ctype_hash, + }, + Event::AttestationRemoved { + attester: attester.clone(), + claim_hash, + authorized_by: attestation::authorized_by::AuthorizedBy::Attester(attester.clone()), + ctype_hash, + } + ] + ); + }); +} diff --git a/pallets/did/src/lib.rs b/pallets/did/src/lib.rs index c84c8f62c1..e1818e2964 100644 --- a/pallets/did/src/lib.rs +++ b/pallets/did/src/lib.rs @@ -334,6 +334,10 @@ pub mod pallet { /// Service endpoints associated with DIDs. /// /// It maps from (DID identifier, service ID) to the service details. + // SAFETY of Twox64Concat: + // The DID Identifier must be registered on chain first. To register a DID + // Identifier, you must provide a valid signature for the identifier or + // control the origin that corresponds to the identifier. #[pallet::storage] #[pallet::getter(fn get_service_endpoints)] pub type ServiceEndpoints = diff --git a/pallets/pallet-deposit-storage/src/lib.rs b/pallets/pallet-deposit-storage/src/lib.rs index fe9dbc6d87..5e4d5fac2e 100644 --- a/pallets/pallet-deposit-storage/src/lib.rs +++ b/pallets/pallet-deposit-storage/src/lib.rs @@ -143,10 +143,19 @@ pub mod pallet { /// Storage of all deposits. Its first key is a namespace, and the second /// one the deposit key. Its value includes the details associated to a /// deposit instance. + // SAFETY of Twox64Concat: + // The Namespace is not chosen by the user but by the pallet and therefore + // doesn't allow for arbitrary data. #[pallet::storage] #[pallet::getter(fn deposits)] - pub(crate) type Deposits = - StorageDoubleMap<_, Twox64Concat, ::Namespace, Twox64Concat, DepositKeyOf, DepositEntryOf>; + pub(crate) type Deposits = StorageDoubleMap< + _, + Twox64Concat, + ::Namespace, + Blake2_128Concat, + DepositKeyOf, + DepositEntryOf, + >; #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] diff --git a/pallets/pallet-dip-consumer/src/lib.rs b/pallets/pallet-dip-consumer/src/lib.rs index bc633025bb..283079892e 100644 --- a/pallets/pallet-dip-consumer/src/lib.rs +++ b/pallets/pallet-dip-consumer/src/lib.rs @@ -41,7 +41,6 @@ pub mod pallet { dispatch::{Dispatchable, GetDispatchInfo, PostDispatchInfo}, pallet_prelude::*, traits::{Contains, EnsureOriginWithArg}, - Twox64Concat, }; use frame_system::pallet_prelude::*; use parity_scale_codec::{FullCodec, MaxEncodedLen}; @@ -105,6 +104,9 @@ pub mod pallet { /// The pallet contains a single storage element, the `IdentityEntries` map. /// It maps from a subject `Identifier` to an instance of /// `LocalIdentityInfo`. + // SAFETY of Twox64Concat: + // The Identifier cannot be chosen freely it is therefore not efficient to + // craft specific hashes. #[pallet::storage] #[pallet::getter(fn identity_proofs)] pub(crate) type IdentityEntries = diff --git a/pallets/pallet-dip-provider/src/lib.rs b/pallets/pallet-dip-provider/src/lib.rs index 856d272d54..8a87d4509b 100644 --- a/pallets/pallet-dip-provider/src/lib.rs +++ b/pallets/pallet-dip-provider/src/lib.rs @@ -85,6 +85,11 @@ pub mod pallet { /// double map. Its first key is the `Identifier` of subjects, while the /// second key is the commitment version. The values are identity /// commitments. + // SAFETY of Twox64Concat: + // The identifier MUST not be freely choosable. This must be ensured by the + // Config::IdentityProvider. + // The commitment version is a u16 and has only very limited value space + // which doesn't to exploit any collisions. #[pallet::storage] #[pallet::getter(fn identity_commitments)] pub type IdentityCommitments = StorageDoubleMap< diff --git a/pallets/pallet-relay-store/src/lib.rs b/pallets/pallet-relay-store/src/lib.rs index 27ad314cf4..aba31dc19a 100644 --- a/pallets/pallet-relay-store/src/lib.rs +++ b/pallets/pallet-relay-store/src/lib.rs @@ -48,6 +48,8 @@ pub mod pallet { /// Maps from a relaychain block height to its related information, /// including the state root. + // SAFETY of Twox64Concat: + // The key is the relaychain blocknumber. It cannot be chosen or set arbitrarily. #[pallet::storage] #[pallet::getter(fn latest_relay_head_for_block)] pub(crate) type LatestRelayHeads = StorageMap<_, Twox64Concat, u32, RelayParentInfo>; diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index 75f7f30c35..f11e445e79 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -552,6 +552,9 @@ pub mod pallet { /// /// It maps from an account to the number of delegations in the last /// session in which they (re-)delegated. + // SAFETY of Twox64Concat: + // The AccountId cannot be chosen freely since it's the signed origin of + // `join_delegators`. #[pallet::storage] #[pallet::getter(fn last_delegation)] pub(crate) type LastDelegation = @@ -560,6 +563,9 @@ pub mod pallet { /// Delegation staking information. /// /// It maps from an account to its delegation details. + // SAFETY of Twox64Concat: + // The AccountId cannot be chosen freely since it's the signed origin of + // `join_delegators`. #[pallet::storage] #[pallet::getter(fn delegator_state)] pub(crate) type DelegatorState = @@ -569,6 +575,9 @@ pub mod pallet { /// /// It maps from an account to its information. /// Moreover, it counts the number of candidates. + // SAFETY of Twox64Concat: + // The AccountId cannot be chosen freely since it's the signed origin of + // `join_candidates`. #[pallet::storage] #[pallet::getter(fn candidate_pool)] pub(crate) type CandidatePool = CountedStorageMap< @@ -612,6 +621,9 @@ pub mod pallet { /// /// It maps from accounts to all the funds addressed to them in the future /// blocks. + // SAFETY of Twox64Concat: + // The AccountId is either the Collator or Delegator account and therefore + // needs to be a valid signed origin. The key cannot be chosen arbitrarily. #[pallet::storage] #[pallet::getter(fn unstaking)] pub(crate) type Unstaking = StorageMap< @@ -638,6 +650,9 @@ pub mod pallet { /// The number of authored blocks for collators. It is updated via the /// `note_author` hook when authoring a block . + // SAFETY of Twox64Concat: + // The Account Id is the Id of a collator. It can only be set to a valid + // signed origin and cannot be chosen arbitrarily. #[pallet::storage] #[pallet::getter(fn blocks_authored)] pub(crate) type BlocksAuthored = @@ -652,6 +667,9 @@ pub mod pallet { /// For delegators, this can be at most BlocksAuthored of the collator.It is /// updated when incrementing delegator rewards, either when calling /// `inc_delegator_rewards` or updating the `InflationInfo`. + // SAFETY of Twox64Concat: + // The Account Id is the Id of a collator. It can only be set to a valid + // signed origin and cannot be chosen arbitrarily. #[pallet::storage] #[pallet::getter(fn blocks_rewarded)] pub(crate) type BlocksRewarded = @@ -660,6 +678,9 @@ pub mod pallet { /// The accumulated rewards for collator candidates and delegators. /// /// It maps from accounts to their total rewards since the last payout. + // SAFETY of Twox64Concat: + // The Account Id is the Id of a collator or delegator. It can only be set + // to a valid signed origin and cannot be chosen arbitrarily. #[pallet::storage] #[pallet::getter(fn rewards)] pub(crate) type Rewards = StorageMap<_, Twox64Concat, T::AccountId, BalanceOf, ValueQuery>; diff --git a/runtimes/clone/src/lib.rs b/runtimes/clone/src/lib.rs index b4da182d5f..cd89eeecb6 100644 --- a/runtimes/clone/src/lib.rs +++ b/runtimes/clone/src/lib.rs @@ -26,11 +26,11 @@ include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); use cumulus_pallet_parachain_system::RelayNumberStrictlyIncreases; -use frame_support::PalletId; use frame_support::{ construct_runtime, parameter_types, traits::Everything, weights::{ConstantMultiplier, Weight}, + PalletId, }; use frame_system::EnsureRoot; use pallet_transaction_payment::CurrencyAdapter;