fix: add did lookup pallet to DID authorization logic + reverse looku…

…p index (#343)

* fix: add did lookup pallet to DID authorization logic

* test: add unit tests for spiritnet and peregrine runtimes for correct DID verification key derivation

* chore: clippy

* feat: add additional map for reverse index

* test: add unit tests

* chore: update benchmarks checks

* wip: runtime upgrade scripts

* wip: lookup pallet migration

* chore: update toolchain version to nightly 1.59 (#339)

* chore: bump up toolchain to nightly 1.59

* chore: address Clippy warnings

* wip: benchmarks

* bench: benchmark compiling

* chore: try-runtime complete

* chore: update deps

* chore: add migrations to Spiritnet runtime

* chore: add comment in lookup migration

* chore: move migration into lookup pallet

* fix: add try-runtime feature to did lookup crate

* chore: fixes after rebase

* chore: fmt

* chore: ConnectedAccounts map comment

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet-did-lookup --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=pallets/pallet-did-lookup/src/default_weights.rs --template=.maintain/weight-template.hbs

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet-did-lookup --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/peregrine/src/weights/pallet_did-lookup.rs --template=.maintain/runtime-weight-template.hbs

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=pallet-did-lookup --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/pallet_did-lookup.rs --template=.maintain/runtime-weight-template.hbs

* bench: update benchmarks to include account replacement as worst case

* bench: fix InsufficientFunds error in benchmarks

* chore: fmt

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet-did-lookup --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=pallets/pallet-did-lookup/src/default_weights.rs --template=.maintain/weight-template.hbs

* chore: update comments

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet-did-lookup --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/peregrine/src/weights/pallet_did-lookup.rs --template=.maintain/runtime-weight-template.hbs

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=pallet-did-lookup --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/pallet_did-lookup.rs --template=.maintain/runtime-weight-template.hbs

Co-authored-by: kiltbot <>
(cherry picked from commit ff34f86)

pallets/pallet-did-lookup/Cargo.toml

@@ -19,6 +19,7 @@ sp-keystore = {branch = "polkadot-v0.9.17", default-features = false, git = "htt
 
 [dependencies]
 codec = {default-features = false, features = ["derive"], package = "parity-scale-codec", version = "2.3.1"}
+log = "0.4"
 scale-info = {version = "1.0", default-features = false, features = ["derive"]}
 
 # KILT
@@ -49,7 +50,13 @@ std = [
   "frame-benchmarking/std",
   "frame-support/std",
   "frame-system/std",
+  "log/std",
   "scale-info/std",
   "sp-runtime/std",
   "sp-std/std",
 ]
+
+try-runtime = [
+  "frame-support/try-runtime",
+  "kilt-support/try-runtime",
+]

pallets/pallet-did-lookup/src/benchmarking.rs

@@ -19,7 +19,7 @@
 
 //! Benchmarking
 
-use crate::{AccountIdOf, Call, Config, ConnectedDids, CurrencyOf, Pallet};
+use crate::{AccountIdOf, Call, Config, ConnectedAccounts, ConnectedDids, CurrencyOf, Pallet};
 
 use codec::Encode;
 use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite};
@@ -31,8 +31,12 @@ use sp_runtime::{app_crypto::sr25519, KeyTypeId};
 
 const SEED: u32 = 0;
 
+// Free 2x deposit amount + existential deposit so that we can use this function
+// to link an account two times to two different DIDs.
 fn make_free_for_did<T: Config>(account: &AccountIdOf<T>) {
-	let balance = <CurrencyOf<T> as Currency<AccountIdOf<T>>>::minimum_balance() + <T as Config>::Deposit::get();
+	let balance = <CurrencyOf<T> as Currency<AccountIdOf<T>>>::minimum_balance()
+		+ <T as Config>::Deposit::get()
+		+ <T as Config>::Deposit::get();
 	<CurrencyOf<T> as Currency<AccountIdOf<T>>>::make_free_balance_be(account, balance);
 }
 
@@ -48,6 +52,7 @@ benchmarks! {
 	associate_account {
 		let caller: T::AccountId = account("caller", 0, SEED);
 		let did: T::DidIdentifier = account("did", 0, SEED);
+		let previous_did: T::DidIdentifier = account("prev", 0, SEED + 1);
 		let connected_acc = sr25519_generate(KeyTypeId(*b"aura"), None);
 		let connected_acc_id: T::AccountId = connected_acc.into();
 		let bn: <T as frame_system::Config>::BlockNumber = 500_u32.into();
@@ -57,34 +62,48 @@ benchmarks! {
 			.into();
 
 		make_free_for_did::<T>(&caller);
-		let origin = T::EnsureOrigin::generate_origin(caller, did);
+
+		// Add existing connected_acc -> previous_did connection that will be replaced
+		Pallet::<T>::add_association(caller.clone(), previous_did.clone(), connected_acc_id.clone()).expect("should create previous association");
+		assert!(ConnectedAccounts::<T>::get(&previous_did, T::AccountId::from(connected_acc)).is_some());
+		let origin = T::EnsureOrigin::generate_origin(caller, did.clone());
 	}: _<T::Origin>(origin, connected_acc_id, bn, sig)
 	verify {
 		assert!(ConnectedDids::<T>::get(T::AccountId::from(connected_acc)).is_some());
+		assert!(ConnectedAccounts::<T>::get(&previous_did, T::AccountId::from(connected_acc)).is_none());
+		assert!(ConnectedAccounts::<T>::get(did, T::AccountId::from(connected_acc)).is_some());
 	}
 
 	associate_sender {
 		let caller: T::AccountId = account("caller", 0, SEED);
 		let did: T::DidIdentifier = account("did", 0, SEED);
+		let previous_did: T::DidIdentifier = account("prev", 0, SEED + 1);
 
 		make_free_for_did::<T>(&caller);
-		let origin = T::EnsureOrigin::generate_origin(caller.clone(), did);
+
+		// Add existing sender -> previous_did connection that will be replaced
+		Pallet::<T>::add_association(caller.clone(), previous_did.clone(), caller.clone()).expect("should create previous association");
+		assert!(ConnectedAccounts::<T>::get(&previous_did, &caller).is_some());
+		let origin = T::EnsureOrigin::generate_origin(caller.clone(), did.clone());
 	}: _<T::Origin>(origin)
 	verify {
-		assert!(ConnectedDids::<T>::get(caller).is_some());
+		assert!(ConnectedDids::<T>::get(&caller).is_some());
+		assert!(ConnectedAccounts::<T>::get(previous_did, &caller).is_none());
+		assert!(ConnectedAccounts::<T>::get(did, caller).is_some());
 	}
 
 	remove_sender_association {
 		let caller: T::AccountId = account("caller", 0, SEED);
 		let did: T::DidIdentifier = account("did", 0, SEED);
 
 		make_free_for_did::<T>(&caller);
-		Pallet::<T>::add_association(caller.clone(), did, caller.clone()).expect("should create association");
+		Pallet::<T>::add_association(caller.clone(), did.clone(), caller.clone()).expect("should create association");
 
 		let origin = RawOrigin::Signed(caller.clone());
 	}: _(origin)
 	verify {
-		assert!(ConnectedDids::<T>::get(caller).is_none());
+		assert!(ConnectedDids::<T>::get(&caller).is_none());
+		assert!(ConnectedAccounts::<T>::get(did, caller).is_none());
 	}
 
 	remove_account_association {
@@ -94,10 +113,12 @@ benchmarks! {
 
 		Pallet::<T>::add_association(caller.clone(), did.clone(), caller.clone()).expect("should create association");
 
-		let origin = T::EnsureOrigin::generate_origin(caller.clone(), did);
-	}: _<T::Origin>(origin, caller.clone())
+		let origin = T::EnsureOrigin::generate_origin(caller.clone(), did.clone());
+		let caller_clone = caller.clone();
+	}: _<T::Origin>(origin, caller_clone)
 	verify {
-		assert!(ConnectedDids::<T>::get(caller).is_none());
+		assert!(ConnectedDids::<T>::get(&caller).is_none());
+		assert!(ConnectedAccounts::<T>::get(did, caller).is_none());
 	}
 }
 

pallets/pallet-did-lookup/src/default_weights.rs

@@ -19,8 +19,8 @@
 //! Autogenerated weights for pallet_did_lookup
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2021-11-19, STEPS: {{cmd.steps}}\, REPEAT: {{cmd.repeat}}\, LOW RANGE: {{cmd.lowest_range_values}}\, HIGH RANGE: {{cmd.highest_range_values}}\
-//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128
+//! DATE: 2022-03-24, STEPS: {{cmd.steps}}\, REPEAT: {{cmd.repeat}}\, LOW RANGE: {{cmd.lowest_range_values}}\, HIGH RANGE: {{cmd.highest_range_values}}\
+//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
 
 // Executed Command:
 // target/release/kilt-parachain
@@ -36,7 +36,6 @@
 // --output=pallets/pallet-did-lookup/src/default_weights.rs
 // --template=.maintain/weight-template.hbs
 
-
 #![cfg_attr(rustfmt, rustfmt_skip)]
 #![allow(unused_parens)]
 #![allow(unused_imports)]
@@ -56,52 +55,72 @@ pub trait WeightInfo {
 /// Weights for pallet_did_lookup using the Substrate node and recommended hardware.
 pub struct SubstrateWeight<T>(PhantomData<T>);
 impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
+	// Storage: System Account (r:1 w:1)
 	// Storage: DidLookup ConnectedDids (r:1 w:1)
+	// Storage: DidLookup ConnectedAccounts (r:0 w:2)
 	fn associate_account() -> Weight {
-		(74_292_000_u64)
-			.saturating_add(T::DbWeight::get().reads(1_u64))
-			.saturating_add(T::DbWeight::get().writes(1_u64))
+		(111_846_000 as Weight)
+			.saturating_add(T::DbWeight::get().reads(2 as Weight))
+			.saturating_add(T::DbWeight::get().writes(4 as Weight))
 	}
+	// Storage: System Account (r:1 w:1)
 	// Storage: DidLookup ConnectedDids (r:1 w:1)
+	// Storage: DidLookup ConnectedAccounts (r:0 w:2)
 	fn associate_sender() -> Weight {
-		(17_673_000_u64)
-			.saturating_add(T::DbWeight::get().reads(1_u64))
-			.saturating_add(T::DbWeight::get().writes(1_u64))
+		(52_637_000 as Weight)
+			.saturating_add(T::DbWeight::get().reads(2 as Weight))
+			.saturating_add(T::DbWeight::get().writes(4 as Weight))
 	}
 	// Storage: DidLookup ConnectedDids (r:1 w:1)
+	// Storage: System Account (r:1 w:1)
+	// Storage: DidLookup ConnectedAccounts (r:0 w:1)
 	fn remove_sender_association() -> Weight {
-		(19_086_000_u64)
-			.saturating_add(T::DbWeight::get().reads(1_u64))
-			.saturating_add(T::DbWeight::get().writes(1_u64))
+		(35_370_000 as Weight)
+			.saturating_add(T::DbWeight::get().reads(2 as Weight))
+			.saturating_add(T::DbWeight::get().writes(3 as Weight))
 	}
 	// Storage: DidLookup ConnectedDids (r:1 w:1)
+	// Storage: System Account (r:1 w:1)
+	// Storage: DidLookup ConnectedAccounts (r:0 w:1)
 	fn remove_account_association() -> Weight {
-		(21_926_000_u64)
-			.saturating_add(T::DbWeight::get().reads(1_u64))
-			.saturating_add(T::DbWeight::get().writes(1_u64))
+		(36_181_000 as Weight)
+			.saturating_add(T::DbWeight::get().reads(2 as Weight))
+			.saturating_add(T::DbWeight::get().writes(3 as Weight))
 	}
 }
 
 // For backwards compatibility and tests
 impl WeightInfo for () {
+	// Storage: System Account (r:1 w:1)
+	// Storage: DidLookup ConnectedDids (r:1 w:1)
+	// Storage: DidLookup ConnectedAccounts (r:0 w:2)
 	fn associate_account() -> Weight {
-		(74_292_000_u64)
-			.saturating_add(RocksDbWeight::get().reads(1_u64))
-			.saturating_add(RocksDbWeight::get().writes(1_u64))
+		(111_846_000 as Weight)
+			.saturating_add(RocksDbWeight::get().reads(2 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(4 as Weight))
 	}
+	// Storage: System Account (r:1 w:1)
+	// Storage: DidLookup ConnectedDids (r:1 w:1)
+	// Storage: DidLookup ConnectedAccounts (r:0 w:2)
 	fn associate_sender() -> Weight {
-		(17_673_000_u64)
-			.saturating_add(RocksDbWeight::get().reads(1_u64))
-			.saturating_add(RocksDbWeight::get().writes(1_u64))
+		(52_637_000 as Weight)
+			.saturating_add(RocksDbWeight::get().reads(2 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(4 as Weight))
 	}
+	// Storage: DidLookup ConnectedDids (r:1 w:1)
+	// Storage: System Account (r:1 w:1)
+	// Storage: DidLookup ConnectedAccounts (r:0 w:1)
 	fn remove_sender_association() -> Weight {
-		(19_086_000_u64)
-			.saturating_add(RocksDbWeight::get().reads(1_u64))
-			.saturating_add(RocksDbWeight::get().writes(1_u64))
+		(35_370_000 as Weight)
+			.saturating_add(RocksDbWeight::get().reads(2 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(3 as Weight))
 	}
+	// Storage: DidLookup ConnectedDids (r:1 w:1)
+	// Storage: System Account (r:1 w:1)
+	// Storage: DidLookup ConnectedAccounts (r:0 w:1)
 	fn remove_account_association() -> Weight {
-		(21_926_000_u64)
-			.saturating_add(RocksDbWeight::get().reads(1_u64))
-			.saturating_add(RocksDbWeight::get().writes(1_u64))
+		(36_181_000 as Weight)
+			.saturating_add(RocksDbWeight::get().reads(2 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(3 as Weight))
 	}
 }

pallets/pallet-did-lookup/src/lib.rs

@@ -26,6 +26,7 @@
 
 mod connection_record;
 pub mod default_weights;
+pub mod migrations;
 
 #[cfg(test)]
 mod tests;
@@ -70,7 +71,7 @@ pub mod pallet {
 	/// The connection record type.
 	pub(crate) type ConnectionRecordOf<T> = ConnectionRecord<DidIdentifierOf<T>, AccountIdOf<T>, BalanceOf<T>>;
 
-	pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+	pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
 
 	#[pallet::config]
 	pub trait Config: frame_system::Config {
@@ -110,6 +111,14 @@ pub mod pallet {
 	#[pallet::getter(fn connected_dids)]
 	pub type ConnectedDids<T> = StorageMap<_, Blake2_128Concat, AccountIdOf<T>, ConnectionRecordOf<T>>;
 
+	/// Mapping from (DID + account identifier) -> ().
+	/// The empty tuple is used as a sentinel value to simply indicate the
+	/// presence of a given tuple in the map.
+	#[pallet::storage]
+	#[pallet::getter(fn connected_accounts)]
+	pub type ConnectedAccounts<T> =
+		StorageDoubleMap<_, Blake2_128Concat, DidIdentifierOf<T>, Blake2_128Concat, AccountIdOf<T>, ()>;
+
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
@@ -129,7 +138,7 @@ pub mod pallet {
 		/// and the account ID.
 		NotAuthorized,
 
-		/// The supplied proof of ownership was outdated and will not
+		/// The supplied proof of ownership was outdated.
 		OutdatedProof,
 
 		/// The account has insufficient funds and can't pay the fees or reserve
@@ -153,8 +162,8 @@ pub mod pallet {
 		///
 		/// # <weight>
 		/// Weight: O(1)
-		/// - Reads: ConnectedDids + DID Origin Check
-		/// - Writes: ConnectedDids
+		/// - Reads: ConnectedDids + ConnectedAccounts + DID Origin Check
+		/// - Writes: ConnectedDids + ConnectedAccounts
 		/// # </weight>
 		#[pallet::weight(<T as Config>::WeightInfo::associate_account())]
 		pub fn associate_account(
@@ -196,8 +205,8 @@ pub mod pallet {
 		///
 		/// # <weight>
 		/// Weight: O(1)
-		/// - Reads: ConnectedDids + DID Origin Check
-		/// - Writes: ConnectedDids
+		/// - Reads: ConnectedDids + ConnectedAccounts + DID Origin Check
+		/// - Writes: ConnectedDids + ConnectedAccounts
 		/// # </weight>
 		#[pallet::weight(<T as Config>::WeightInfo::associate_sender())]
 		pub fn associate_sender(origin: OriginFor<T>) -> DispatchResult {
@@ -222,8 +231,8 @@ pub mod pallet {
 		///
 		/// # <weight>
 		/// Weight: O(1)
-		/// - Reads: ConnectedDids
-		/// - Writes: ConnectedDids
+		/// - Reads: ConnectedDids + ConnectedAccounts + DID Origin Check
+		/// - Writes: ConnectedDids + ConnectedAccounts
 		/// # </weight>
 		#[pallet::weight(<T as Config>::WeightInfo::remove_sender_association())]
 		pub fn remove_sender_association(origin: OriginFor<T>) -> DispatchResult {
@@ -240,8 +249,8 @@ pub mod pallet {
 		///
 		/// # <weight>
 		/// Weight: O(1)
-		/// - Reads: ConnectedDids + DID Origin Check
-		/// - Writes: ConnectedDids
+		/// - Reads: ConnectedDids + ConnectedAccounts + DID Origin Check
+		/// - Writes: ConnectedDids + ConnectedAccounts
 		/// # </weight>
 		#[pallet::weight(<T as Config>::WeightInfo::remove_account_association())]
 		pub fn remove_account_association(origin: OriginFor<T>, account: AccountIdOf<T>) -> DispatchResult {
@@ -293,18 +302,21 @@ pub mod pallet {
 			CurrencyOf::<T>::reserve(&record.deposit.owner, record.deposit.amount)?;
 
 			ConnectedDids::<T>::mutate(&account, |did_entry| {
-				if let Some(old_did) = did_entry.replace(record) {
-					Self::deposit_event(Event::<T>::AssociationRemoved(account.clone(), old_did.did));
-					kilt_support::free_deposit::<AccountIdOf<T>, CurrencyOf<T>>(&old_did.deposit);
+				if let Some(old_connection) = did_entry.replace(record) {
+					ConnectedAccounts::<T>::remove(&old_connection.did, &account);
+					Self::deposit_event(Event::<T>::AssociationRemoved(account.clone(), old_connection.did));
+					kilt_support::free_deposit::<AccountIdOf<T>, CurrencyOf<T>>(&old_connection.deposit);
 				}
 			});
+			ConnectedAccounts::<T>::insert(&did_identifier, &account, ());
 			Self::deposit_event(Event::AssociationEstablished(account, did_identifier));
 
 			Ok(())
 		}
 
 		pub(crate) fn remove_association(account: AccountIdOf<T>) -> DispatchResult {
 			if let Some(connection) = ConnectedDids::<T>::take(&account) {
+				ConnectedAccounts::<T>::remove(&connection.did, &account);
 				kilt_support::free_deposit::<AccountIdOf<T>, CurrencyOf<T>>(&connection.deposit);
 				Self::deposit_event(Event::AssociationRemoved(account, connection.did));