From d138faffe25101aafebade625272aab745b8389e Mon Sep 17 00:00:00 2001 From: weichweich Date: Wed, 3 Jan 2024 16:27:12 +0100 Subject: [PATCH 1/9] fix: switch to Blake2 hashing for ExternalAttestations --- pallets/attestation/src/lib.rs | 18 +++++-- pallets/attestation/src/migrations.rs | 18 +++++++ pallets/attestation/src/mock.rs | 27 +++++++++++ pallets/attestation/src/tests/delete.rs | 63 ++++++++++++++++++++++++- 4 files changed, 121 insertions(+), 5 deletions(-) diff --git a/pallets/attestation/src/lib.rs b/pallets/attestation/src/lib.rs index 7d57897f74..1f989d3d3e 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,15 @@ pub mod pallet { } impl Pallet { + // TODO: delete once + 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 +529,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 5ef8ae8250..937c1e5bb6 100644 --- a/pallets/attestation/src/migrations.rs +++ b/pallets/attestation/src/migrations.rs @@ -37,6 +37,24 @@ where ) } +pub mod v1 { + use frame_support::{pallet_prelude::*, storage_alias, Blake2_128Concat}; + + use crate::{AuthorizationIdOf, ClaimHashOf, Config, Pallet}; + + /// The old `PageIndex` storage item. + #[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 a6c65d56b8..9adf744c2b 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 44618989b7..fd9ecf3163 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, HoldReason, ExternalAttestations}; #[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,61 @@ 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.clone())]) + .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, + } + ] + ); + }); +} From 2cf3adff7f6dd004f1f3d895ac3f9d7318113a35 Mon Sep 17 00:00:00 2001 From: weichweich Date: Wed, 3 Jan 2024 16:27:42 +0100 Subject: [PATCH 2/9] docs(pallet-postit): add safety comment --- dip-template/pallets/pallet-postit/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dip-template/pallets/pallet-postit/src/lib.rs b/dip-template/pallets/pallet-postit/src/lib.rs index f0697f78df..573328a43c 100644 --- a/dip-template/pallets/pallet-postit/src/lib.rs +++ b/dip-template/pallets/pallet-postit/src/lib.rs @@ -57,10 +57,12 @@ 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>; From dcfc942d2d363b2a345a1d1cb0cab2f2eff54bac Mon Sep 17 00:00:00 2001 From: weichweich Date: Wed, 3 Jan 2024 16:33:33 +0100 Subject: [PATCH 3/9] fix(pallet-dip-consumer): use Blake2_128Concat hasher We can't ensure that the `Identifier` is not user choosable --- pallets/pallet-dip-consumer/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pallets/pallet-dip-consumer/src/lib.rs b/pallets/pallet-dip-consumer/src/lib.rs index ea6125d7b4..fd8d994deb 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}; @@ -108,7 +107,7 @@ pub mod pallet { #[pallet::storage] #[pallet::getter(fn identity_proofs)] pub(crate) type IdentityEntries = - StorageMap<_, Twox64Concat, ::Identifier, ::LocalIdentityInfo>; + StorageMap<_, Blake2_128Concat, ::Identifier, ::LocalIdentityInfo>; #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] From 5158f2572760d4e6ce10ac98a4d8ee597e5a5fb4 Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 4 Jan 2024 10:23:10 +0100 Subject: [PATCH 4/9] fix: copy paste error --- pallets/attestation/src/migrations.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/pallets/attestation/src/migrations.rs b/pallets/attestation/src/migrations.rs index 937c1e5bb6..c7653ca4ab 100644 --- a/pallets/attestation/src/migrations.rs +++ b/pallets/attestation/src/migrations.rs @@ -42,7 +42,6 @@ pub mod v1 { use crate::{AuthorizationIdOf, ClaimHashOf, Config, Pallet}; - /// The old `PageIndex` storage item. #[storage_alias] pub(crate) type ExternalAttestations = StorageDoubleMap< Pallet, From e15e9c593f5c94e547bd2fbe2da832f690664ab1 Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 4 Jan 2024 10:27:02 +0100 Subject: [PATCH 5/9] fix(pallet-deposit): safe use of Twox64Concat --- pallets/pallet-deposit-storage/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pallets/pallet-deposit-storage/src/lib.rs b/pallets/pallet-deposit-storage/src/lib.rs index 0043f9eedf..062adae98a 100644 --- a/pallets/pallet-deposit-storage/src/lib.rs +++ b/pallets/pallet-deposit-storage/src/lib.rs @@ -143,10 +143,11 @@ 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: 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>; + StorageDoubleMap<_, Twox64Concat, ::Namespace, Blake2_128Concat, DepositKeyOf, DepositEntryOf>; #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] From 16edb7581bd2d18d220b33d6ffaae324ff29fddd Mon Sep 17 00:00:00 2001 From: weichweich Date: Fri, 12 Jan 2024 10:15:14 +0100 Subject: [PATCH 6/9] chore: document safe use of Twox64 --- dip-template/pallets/pallet-postit/src/lib.rs | 6 ++++-- pallets/attestation/src/lib.rs | 5 +---- pallets/did/src/lib.rs | 4 ++++ pallets/pallet-deposit-storage/src/lib.rs | 14 ++++++++++--- pallets/pallet-dip-provider/src/lib.rs | 5 +++++ pallets/pallet-relay-store/src/lib.rs | 2 ++ pallets/parachain-staking/src/lib.rs | 21 +++++++++++++++++++ 7 files changed, 48 insertions(+), 9 deletions(-) diff --git a/dip-template/pallets/pallet-postit/src/lib.rs b/dip-template/pallets/pallet-postit/src/lib.rs index 573328a43c..1e7652ece9 100644 --- a/dip-template/pallets/pallet-postit/src/lib.rs +++ b/dip-template/pallets/pallet-postit/src/lib.rs @@ -57,12 +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. + // 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. + // 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/pallets/attestation/src/lib.rs b/pallets/attestation/src/lib.rs index 1f989d3d3e..60081d7987 100644 --- a/pallets/attestation/src/lib.rs +++ b/pallets/attestation/src/lib.rs @@ -502,10 +502,7 @@ pub mod pallet { impl Pallet { // TODO: delete once - pub fn external_attestations( - authorization_id: AuthorizationIdOf, - claim_hash: ClaimHashOf, - ) -> bool { + 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) } diff --git a/pallets/did/src/lib.rs b/pallets/did/src/lib.rs index b89e1c7754..3e005a83c6 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 062adae98a..836a1c1de2 100644 --- a/pallets/pallet-deposit-storage/src/lib.rs +++ b/pallets/pallet-deposit-storage/src/lib.rs @@ -143,11 +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: The Namespace is not chosen by the user but by the pallet and therefore doesn't allow for arbitrary data. + // 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, Blake2_128Concat, 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-provider/src/lib.rs b/pallets/pallet-dip-provider/src/lib.rs index 4f86e14ad6..33b27e8da7 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 084f88232a..9dc934d7f4 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 dcd29177f0..6b59602e8b 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>; From 0cd7ad3d6394c1e87e7d4cad0bf555f79b85caed Mon Sep 17 00:00:00 2001 From: weichweich Date: Fri, 12 Jan 2024 12:24:43 +0100 Subject: [PATCH 7/9] fix: clippy --- pallets/attestation/src/lib.rs | 4 ++-- pallets/attestation/src/tests/delete.rs | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pallets/attestation/src/lib.rs b/pallets/attestation/src/lib.rs index 60081d7987..c8828e5c85 100644 --- a/pallets/attestation/src/lib.rs +++ b/pallets/attestation/src/lib.rs @@ -503,8 +503,8 @@ pub mod pallet { impl Pallet { // TODO: delete once 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) + ExternalAttestations::::get(&authorization_id, claim_hash) + || migrations::v1::ExternalAttestations::::get(&authorization_id, claim_hash) } fn remove_attestation( diff --git a/pallets/attestation/src/tests/delete.rs b/pallets/attestation/src/tests/delete.rs index fd9ecf3163..2518e1ee22 100644 --- a/pallets/attestation/src/tests/delete.rs +++ b/pallets/attestation/src/tests/delete.rs @@ -189,16 +189,16 @@ fn test_remove_v1() { 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.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 + attester.clone(), + claim_hash )); assert!(migrations::v1::ExternalAttestations::::get( - &attester.clone(), - &claim_hash + attester.clone(), + claim_hash )); assert_ok!(Attestation::remove( @@ -208,12 +208,12 @@ fn test_remove_v1() { )); assert!(Attestation::attestations(claim_hash).is_none()); assert!(!ExternalAttestations::::get( - &attester.clone(), - &claim_hash + attester.clone(), + claim_hash )); assert!(!migrations::v1::ExternalAttestations::::get( - &attester.clone(), - &claim_hash + attester.clone(), + claim_hash )); assert_eq!( From add484856ccab16c65492c9d7414a5e68b9e8534 Mon Sep 17 00:00:00 2001 From: weichweich Date: Fri, 12 Jan 2024 12:35:06 +0100 Subject: [PATCH 8/9] Update docs & revert instroduction of Blake2_128Concat --- pallets/attestation/src/lib.rs | 2 +- pallets/pallet-dip-consumer/src/lib.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pallets/attestation/src/lib.rs b/pallets/attestation/src/lib.rs index c8828e5c85..416d10b83e 100644 --- a/pallets/attestation/src/lib.rs +++ b/pallets/attestation/src/lib.rs @@ -501,7 +501,7 @@ pub mod pallet { } impl Pallet { - // TODO: delete once + // 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) diff --git a/pallets/pallet-dip-consumer/src/lib.rs b/pallets/pallet-dip-consumer/src/lib.rs index fd8d994deb..c319a830f5 100644 --- a/pallets/pallet-dip-consumer/src/lib.rs +++ b/pallets/pallet-dip-consumer/src/lib.rs @@ -104,10 +104,13 @@ 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 = - StorageMap<_, Blake2_128Concat, ::Identifier, ::LocalIdentityInfo>; + StorageMap<_, Twox64Concat, ::Identifier, ::LocalIdentityInfo>; #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] From ef7c7f44b05126ed3c25b1d1cdad314271c28db5 Mon Sep 17 00:00:00 2001 From: weichweich Date: Fri, 12 Jan 2024 13:59:43 +0100 Subject: [PATCH 9/9] fmt --- nodes/parachain/src/chain_spec/clone.rs | 8 +++----- pallets/attestation/src/lib.rs | 3 ++- pallets/attestation/src/tests/delete.rs | 12 +++--------- runtimes/clone/src/lib.rs | 2 +- 4 files changed, 9 insertions(+), 16 deletions(-) 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 580b1e1627..f95d2e17d0 100644 --- a/pallets/attestation/src/lib.rs +++ b/pallets/attestation/src/lib.rs @@ -501,7 +501,8 @@ pub mod pallet { } impl Pallet { - // TODO: Delete this and add `#[pallet::getter(fn external_attestations)]` once the migration is over. + // 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) diff --git a/pallets/attestation/src/tests/delete.rs b/pallets/attestation/src/tests/delete.rs index 4d04c503d6..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, migrations, mock::*, AttesterOf, Config, Event, HoldReason, ExternalAttestations}; +use crate::{self as attestation, migrations, mock::*, AttesterOf, Config, Event, ExternalAttestations, HoldReason}; #[test] fn test_remove() { @@ -192,10 +192,7 @@ fn test_remove_v1() { .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!(!ExternalAttestations::::get(attester.clone(), claim_hash)); assert!(migrations::v1::ExternalAttestations::::get( attester.clone(), claim_hash @@ -207,10 +204,7 @@ fn test_remove_v1() { None )); assert!(Attestation::attestations(claim_hash).is_none()); - assert!(!ExternalAttestations::::get( - attester.clone(), - claim_hash - )); + assert!(!ExternalAttestations::::get(attester.clone(), claim_hash)); assert!(!migrations::v1::ExternalAttestations::::get( attester.clone(), claim_hash 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;