docs: Document safe use of Twox64Concat

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<T> = StorageMap<_, Twox64Concat, <T as frame_system::Config>::Hash, PostOf<T>>;
 
+	// 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<T> = StorageMap<_, Twox64Concat, <T as frame_system::Config>::Hash, CommentOf<T>>;

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};

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<T> = <T as frame_system::Config>::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<T> =
-		StorageDoubleMap<_, Twox64Concat, AuthorizationIdOf<T>, Blake2_128Concat, ClaimHashOf<T>, bool, ValueQuery>;
+	// #[pallet::getter(fn external_attestations)]
+	pub(crate) type ExternalAttestations<T> =
+		StorageDoubleMap<_, Blake2_128Concat, AuthorizationIdOf<T>, Blake2_128Concat, ClaimHashOf<T>, bool, ValueQuery>;
 
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
@@ -501,6 +501,13 @@ pub mod pallet {
 	}
 
 	impl<T: Config> Pallet<T> {
+		// TODO: Delete this and add `#[pallet::getter(fn external_attestations)]` once
+		// the migration is over.
+		pub fn external_attestations(authorization_id: AuthorizationIdOf<T>, claim_hash: ClaimHashOf<T>) -> bool {
+			ExternalAttestations::<T>::get(&authorization_id, claim_hash)
+				|| migrations::v1::ExternalAttestations::<T>::get(&authorization_id, claim_hash)
+		}
+
 		fn remove_attestation(
 			authorized_by: AuthorizedByOf<T>,
 			attestation: AttestationDetailsOf<T>,
@@ -520,6 +527,7 @@ pub mod pallet {
 			Attestations::<T>::remove(claim_hash);
 			if let Some(authorization_id) = &attestation.authorization_id {
 				ExternalAttestations::<T>::remove(authorization_id, claim_hash);
+				migrations::v1::ExternalAttestations::<T>::remove(authorization_id, claim_hash);
 			}
 			if !attestation.revoked {
 				Self::deposit_event(Event::AttestationRevoked {

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<T: Config> = StorageDoubleMap<
+		Pallet<T>,
+		Twox64Concat,
+		AuthorizationIdOf<T>,
+		Blake2_128Concat,
+		ClaimHashOf<T>,
+		bool,
+		ValueQuery,
+	>;
+}
+
 #[cfg(test)]
 pub mod test {
 	use ctype::mock::get_ctype_hash;

pallets/attestation/src/mock.rs

@@ -167,6 +167,19 @@ pub fn insert_attestation<T: Config>(claim_hash: ClaimHashOf<T>, details: Attest
 	}
 }
 
+pub fn insert_attestation_v1<T: Config>(claim_hash: ClaimHashOf<T>, details: AttestationDetailsOf<T>) {
+	crate::AttestationStorageDepositCollector::<T>::create_deposit(
+		details.deposit.owner.clone(),
+		details.deposit.amount,
+	)
+	.expect("Should have balance");
+
+	crate::Attestations::<T>::insert(claim_hash, details.clone());
+	if let Some(delegation_id) = details.authorization_id.as_ref() {
+		crate::migrations::v1::ExternalAttestations::<T>::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<Test>, BalanceOf<Test>)>,
 		attestations: Vec<(ClaimHashOf<Test>, AttestationDetailsOf<Test>)>,
+		attestations_v1: Vec<(ClaimHashOf<Test>, AttestationDetailsOf<Test>)>,
 	}
 
 	impl ExtBuilder {
@@ -373,6 +387,15 @@ pub(crate) mod runtime {
 			self
 		}
 
+		#[must_use]
+		pub fn with_attestations_v1(
+			mut self,
+			attestations: Vec<(ClaimHashOf<Test>, AttestationDetailsOf<Test>)>,
+		) -> Self {
+			self.attestations_v1 = attestations;
+			self
+		}
+
 		pub fn build(self) -> sp_io::TestExternalities {
 			let mut storage = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
 			pallet_balances::GenesisConfig::<Test> {
@@ -401,6 +424,10 @@ pub(crate) mod runtime {
 				for (claim_hash, details) in self.attestations {
 					insert_attestation::<Test>(claim_hash, details);
 				}
+
+				for (claim_hash, details) in self.attestations_v1 {
+					insert_attestation_v1::<Test>(claim_hash, details);
+				}
 			});
 
 			ext

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<Test> = sr25519_did_from_public_key(&ALICE_SEED);
+	let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01);
+	let mut attestation = generate_base_attestation::<Test>(attester.clone(), ACCOUNT_00);
+	attestation.authorization_id = Some(attester.clone());
+	let ctype_hash = attestation.ctype_hash;
+
+	ExtBuilder::default()
+		.with_balances(vec![(ACCOUNT_00, <Test as Config>::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::<Test>::get(attester.clone(), claim_hash));
+			assert!(migrations::v1::ExternalAttestations::<Test>::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::<Test>::get(attester.clone(), claim_hash));
+			assert!(!migrations::v1::ExternalAttestations::<Test>::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,
+					}
+				]
+			);
+		});
+}

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<T> =

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<T> =
-		StorageDoubleMap<_, Twox64Concat, <T as Config>::Namespace, Twox64Concat, DepositKeyOf<T>, DepositEntryOf<T>>;
+	pub(crate) type Deposits<T> = StorageDoubleMap<
+		_,
+		Twox64Concat,
+		<T as Config>::Namespace,
+		Blake2_128Concat,
+		DepositKeyOf<T>,
+		DepositEntryOf<T>,
+	>;
 
 	#[pallet::pallet]
 	#[pallet::storage_version(STORAGE_VERSION)]

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<T> =

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<T> = StorageDoubleMap<

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<T: Config> = StorageMap<_, Twox64Concat, u32, RelayParentInfo<H256>>;

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<T: Config> =
@@ -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<T: Config> =
@@ -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<T: Config> = 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<T: Config> = 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<T: Config> =
@@ -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<T: Config> =
@@ -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<T: Config> = StorageMap<_, Twox64Concat, T::AccountId, BalanceOf<T>, ValueQuery>;

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;