feat: support for unique linking (#782)

Part of KILTprotocol/ticket#3650. Built on top
of #781.

## Trade-off

I chose to go this way instead of providing an optional counter, because
providing a counter would require one of the following two approaches:

1. Transform the storage double map into a counted one, requiring a
migration also for our currently-deployed pallet, which I wanted to
avoid
2. Not use a counter, but iterate every time to make sure there are
still "spots" left for the current DID. This would require changing the
benchmarking logic as now we have a potentially unbounded iteration
happening. I also wanted to avoid that.

Hence, the solution was to provide a somehow more limited feature of
simply specifying whether the links are expected to be unique per DID or
not. This, as long as we set `false` for our deployed pallets would not
require any storage migration, and does not require any changes in the
benchmarks, so I found it a good compromise.

Author: ntn-x2

dip-template/runtimes/dip-provider/src/lib.rs

@@ -433,6 +433,7 @@ impl pallet_did_lookup::Config for Runtime {
 	type OriginSuccess = DidRawOrigin<AccountId, DidIdentifier>;
 	type RuntimeEvent = RuntimeEvent;
 	type RuntimeHoldReason = RuntimeHoldReason;
+	type UniqueLinkingEnabled = ConstBool<false>;
 	type WeightInfo = weights::pallet_did_lookup::WeightInfo<Runtime>;
 }
 

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

@@ -131,6 +131,11 @@ pub mod pallet {
 
 		/// Migration manager to handle new created entries
 		type BalanceMigrationManager: BalanceMigrationManager<AccountIdOf<Self>, BalanceOf<Self, I>>;
+
+		// Flag specifying whether there should only ever be a single account <-> DID
+		// link, or multiple.
+		#[pallet::constant]
+		type UniqueLinkingEnabled: Get<bool>;
 	}
 
 	#[pallet::pallet]
@@ -196,6 +201,9 @@ pub mod pallet {
 		///
 		/// NOTE: this will only be returned if the storage has inconsistencies.
 		Migration,
+		/// The deployed pallet supports a single account <-> link, which has
+		/// already been previously created for the provided DID.
+		LinkExisting,
 	}
 
 	#[pallet::genesis_config]
@@ -446,6 +454,14 @@ pub mod pallet {
 				did: did_identifier.clone(),
 			};
 
+			let is_unique_flag_enabled = <T as Config<I>>::UniqueLinkingEnabled::get();
+			if is_unique_flag_enabled {
+				let is_did_already_linked = ConnectedAccounts::<T, I>::iter_key_prefix(&did_identifier)
+					.next()
+					.is_some();
+				ensure!(!is_did_already_linked, Error::<T, I>::LinkExisting);
+			}
+
 			LinkableAccountDepositCollector::<T, I>::create_deposit(
 				record.clone().deposit.owner,
 				record.deposit.amount,

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

@@ -16,13 +16,14 @@
 
 // If you feel like getting in touch with us, you can do so at [email protected]
 
-use frame_support::parameter_types;
+use frame_support::{pallet_prelude::ValueQuery, parameter_types, storage_alias};
 use frame_system::pallet_prelude::BlockNumberFor;
 use kilt_support::{
 	mock::{mock_origin, SubjectId},
 	traits::StorageDepositCollector,
 };
 
+use sp_core::Get;
 use sp_runtime::{
 	traits::{BlakeTwo256, IdentifyAccount, IdentityLookup, Verify},
 	BuildStorage, MultiSignature,
@@ -109,6 +110,23 @@ parameter_types! {
 	pub const DidLookupDeposit: Balance = 10;
 }
 
+pub struct UniqueLinkEnabledFlag;
+
+#[storage_alias]
+type FlagStorage = StorageValue<DidLookup, bool, ValueQuery>;
+
+impl UniqueLinkEnabledFlag {
+	fn set(flag: bool) {
+		FlagStorage::set(flag)
+	}
+}
+
+impl Get<bool> for UniqueLinkEnabledFlag {
+	fn get() -> bool {
+		FlagStorage::get()
+	}
+}
+
 impl pallet_did_lookup::Config for Test {
 	type BalanceMigrationManager = ();
 	type RuntimeEvent = RuntimeEvent;
@@ -119,6 +137,7 @@ impl pallet_did_lookup::Config for Test {
 	type OriginSuccess = mock_origin::DoubleOrigin<AccountId, SubjectId>;
 	type DidIdentifier = SubjectId;
 	type WeightInfo = ();
+	type UniqueLinkingEnabled = UniqueLinkEnabledFlag;
 }
 
 impl mock_origin::Config for Test {
@@ -163,6 +182,7 @@ pub struct ExtBuilder {
 	balances: Vec<(AccountId, Balance)>,
 	/// list of connection (sender, did, connected address)
 	connections: Vec<(AccountId, SubjectId, LinkableAccountId)>,
+	unique_flag: bool,
 }
 
 impl ExtBuilder {
@@ -179,6 +199,11 @@ impl ExtBuilder {
 		self
 	}
 
+	pub fn with_unique_connections(mut self) -> Self {
+		self.unique_flag = true;
+		self
+	}
+
 	pub fn build(self) -> sp_io::TestExternalities {
 		let mut storage = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
 		pallet_balances::GenesisConfig::<Test> {
@@ -197,6 +222,8 @@ impl ExtBuilder {
 				pallet_did_lookup::Pallet::<Test>::add_association(sender, did, account)
 					.expect("Should create connection");
 			}
+
+			UniqueLinkEnabledFlag::set(self.unique_flag);
 		});
 		ext
 	}

pallets/pallet-did-lookup/src/tests/associate.rs

@@ -372,3 +372,44 @@ fn test_remove_association_account_not_authorized() {
 			);
 		});
 }
+
+#[test]
+fn test_add_association_with_unique_linking_enabled() {
+	ExtBuilder::default()
+		.with_balances(vec![
+			(ACCOUNT_00, <Test as crate::Config>::Deposit::get() * 50),
+			(ACCOUNT_01, <Test as crate::Config>::Deposit::get() * 50),
+		])
+		.with_unique_connections()
+		.build_and_execute_with_sanity_tests(|| {
+			// First time linking works.
+			assert_ok!(DidLookup::associate_sender(
+				mock_origin::DoubleOrigin(ACCOUNT_00, DID_00).into()
+			));
+			assert!(ConnectedDids::<Test>::contains_key(LinkableAccountId::from(ACCOUNT_00)));
+			assert!(ConnectedAccounts::<Test>::contains_key(
+				DID_00,
+				LinkableAccountId::from(ACCOUNT_00)
+			));
+
+			// Changing the DID linked to an account (overriding the previous DID) works.
+			assert_ok!(DidLookup::associate_sender(
+				mock_origin::DoubleOrigin(ACCOUNT_00, DID_01).into()
+			));
+			assert!(ConnectedDids::<Test>::contains_key(LinkableAccountId::from(ACCOUNT_00)));
+			assert!(ConnectedAccounts::<Test>::contains_key(
+				DID_01,
+				LinkableAccountId::from(ACCOUNT_00)
+			));
+			assert!(!ConnectedAccounts::<Test>::contains_key(
+				DID_00,
+				LinkableAccountId::from(ACCOUNT_00)
+			));
+
+			// Linking a second account to the same DID fails.
+			assert_noop!(
+				DidLookup::associate_sender(mock_origin::DoubleOrigin(ACCOUNT_01, DID_01).into()),
+				Error::<Test>::LinkExisting
+			);
+		})
+}

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

@@ -19,11 +19,15 @@
 use frame_support::ensure;
 use kilt_support::test_utils::log_and_return_error_message;
 use scale_info::prelude::format;
-use sp_runtime::TryRuntimeError;
+use sp_runtime::{
+	traits::{Get, One},
+	TryRuntimeError,
+};
 
-use crate::{Config, ConnectedAccounts, ConnectedDids};
+use crate::{Config, ConnectedAccounts, ConnectedDids, ConnectionRecord};
 
 pub(crate) fn do_try_state<T: Config<I>, I: 'static>() -> Result<(), TryRuntimeError> {
+	// Verify DID -> account link integrity.
 	ConnectedDids::<T, I>::iter().try_for_each(|(account, record)| -> Result<(), TryRuntimeError> {
 		ensure!(
 			ConnectedAccounts::<T, I>::contains_key(&record.did, &account),
@@ -32,6 +36,7 @@ pub(crate) fn do_try_state<T: Config<I>, I: 'static>() -> Result<(), TryRuntimeE
 		Ok(())
 	})?;
 
+	// Verify account -> DID link integrity.
 	ConnectedAccounts::<T, I>::iter().try_for_each(
 		|(did_identifier, linked_account_id, _)| -> Result<(), TryRuntimeError> {
 			ensure!(
@@ -43,5 +48,24 @@ pub(crate) fn do_try_state<T: Config<I>, I: 'static>() -> Result<(), TryRuntimeE
 			);
 			Ok(())
 		},
-	)
+	)?;
+
+	// Verify account <-> DID link unicity.
+	if <T as Config<I>>::UniqueLinkingEnabled::get() {
+		let mut did_linked_to_accounts = ConnectedDids::<T, I>::iter_values().map(|ConnectionRecord { did, .. }| did);
+
+		did_linked_to_accounts.try_for_each(|did_identifier| -> Result<(), TryRuntimeError> {
+			let linked_accounts = ConnectedAccounts::<T, I>::iter_key_prefix(&did_identifier).count();
+			ensure!(
+				linked_accounts.is_one(),
+				log_and_return_error_message(format!(
+					"DID {:?} has more than a single account linked: {:?}.",
+					did_identifier, linked_accounts
+				))
+			);
+			Ok(())
+		})?;
+	}
+
+	Ok(())
 }

pallets/pallet-dip-consumer/src/mock.rs

@@ -23,7 +23,7 @@ use frame_support::{
 		traits::{BlakeTwo256, IdentityLookup},
 		AccountId32,
 	},
-	traits::{ConstU16, ConstU32, ConstU64, Contains, Currency, Everything},
+	traits::{ConstBool, ConstU16, ConstU32, ConstU64, Contains, Currency, Everything},
 };
 use frame_system::{mocking::MockBlock, EnsureSigned};
 
@@ -94,6 +94,7 @@ impl pallet_did_lookup::Config for TestRuntime {
 	type OriginSuccess = DipOrigin<AccountId32, AccountId32, ()>;
 	type RuntimeEvent = RuntimeEvent;
 	type RuntimeHoldReason = RuntimeHoldReason;
+	type UniqueLinkingEnabled = ConstBool<false>;
 	type WeightInfo = ();
 }
 

pallets/pallet-migration/src/mock.rs

@@ -57,7 +57,7 @@ pub mod runtime {
 	use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
 	use public_credentials::InputSubjectIdOf;
 	use scale_info::TypeInfo;
-	use sp_core::{ed25519, ConstU128, ConstU32};
+	use sp_core::{ed25519, ConstBool, ConstU128, ConstU32};
 	use sp_runtime::{
 		traits::{BlakeTwo256, IdentifyAccount, IdentityLookup, Verify},
 		AccountId32, BuildStorage, MultiSignature, MultiSigner, Perquintill, RuntimeDebug,
@@ -309,6 +309,7 @@ pub mod runtime {
 		type DidIdentifier = SubjectId;
 		type WeightInfo = ();
 		type BalanceMigrationManager = Migration;
+		type UniqueLinkingEnabled = ConstBool<false>;
 	}
 
 	pub(crate) type TestWeb3Name = AsciiWeb3Name<Test>;

runtimes/common/src/dip/mock.rs

@@ -30,7 +30,7 @@ use frame_system::{mocking::MockBlock, pallet_prelude::BlockNumberFor, EnsureRoo
 use kilt_dip_primitives::RevealedWeb3Name;
 use pallet_did_lookup::{account::AccountId20, linkable_account::LinkableAccountId};
 use pallet_web3_names::{web3_name::AsciiWeb3Name, Web3NameOf};
-use sp_core::{sr25519, ConstU128, ConstU16, ConstU32, ConstU64};
+use sp_core::{sr25519, ConstBool, ConstU128, ConstU16, ConstU32, ConstU64};
 use sp_runtime::{traits::IdentityLookup, AccountId32, BoundedVec};
 
 use crate::{
@@ -169,6 +169,7 @@ impl pallet_did_lookup::Config for TestRuntime {
 	type OriginSuccess = AccountId;
 	type RuntimeEvent = RuntimeEvent;
 	type RuntimeHoldReason = RuntimeHoldReason;
+	type UniqueLinkingEnabled = ConstBool<false>;
 	type WeightInfo = ();
 }
 

runtimes/kestrel/src/lib.rs

@@ -437,6 +437,9 @@ impl pallet_did_lookup::Config for Runtime {
 	type OriginSuccess = did::DidRawOrigin<AccountId, DidIdentifier>;
 	type BalanceMigrationManager = ();
 	type WeightInfo = ();
+	// Do not change the below flag to `true` without also deploying a runtime
+	// migration which removes any links that point to the same DID!
+	type UniqueLinkingEnabled = ConstBool<false>;
 }
 
 impl pallet_web3_names::Config for Runtime {

runtimes/peregrine/src/lib.rs

@@ -702,6 +702,9 @@ impl pallet_did_lookup::Config for Runtime {
 
 	type WeightInfo = weights::pallet_did_lookup::WeightInfo<Runtime>;
 	type BalanceMigrationManager = Migration;
+	// Do not change the below flag to `true` without also deploying a runtime
+	// migration which removes any links that point to the same DID!
+	type UniqueLinkingEnabled = ConstBool<false>;
 }
 
 impl pallet_web3_names::Config for Runtime {