feat: refactor complete

pallets/did/src/benchmarking.rs

@@ -125,21 +125,15 @@ fn generate_base_did_creation_operation<T: Config>() -> DidCreationOperation {
 
 fn generate_base_did_update_operation<T: Config>(did: DidIdentifierOf<T>) -> DidUpdateOperation<T> {
 	DidUpdateOperation {
-		did,
 		new_authentication_key: None,
 		new_key_agreement_keys: BTreeSet::new(),
 		attestation_key_update: DidVerificationKeyUpdateAction::default(),
 		delegation_key_update: DidVerificationKeyUpdateAction::default(),
 		new_endpoint_url: None,
 		public_keys_to_remove: BTreeSet::new(),
-		tx_counter: 1u64,
 	}
 }
 
-fn generate_base_did_deletion_operation<T: Config>(did: DidIdentifierOf<T>) -> DidDeletionOperation<T> {
-	DidDeletionOperation { did, tx_counter: 1u64 }
-}
-
 // Must always be dispatched with the DID authentication key
 fn generate_base_did_call_operation<T: Config>(did: DidIdentifierOf<T>) -> DidAuthorizedCallOperation<T> {
 	let test_call = <T as Config>::Call::get_call_for_did_call_benchmark();
@@ -335,7 +329,7 @@ benchmarks! {
 		did_update_op.new_endpoint_url = Some(new_url);
 
 		let did_update_signature = ed25519_sign(AUTHENTICATION_KEY_ID, &did_public_auth_key, did_update_op.encode().as_ref()).expect("Failed to create DID signature from raw ed25519 signature.");
-	}: submit_did_update_operation(RawOrigin::Signed(submitter), did_update_op.clone(), DidSignature::from(did_update_signature))
+	}: submit_did_update_operation(RawOrigin::Signed(submitter), did_update_op.clone())
 	verify {
 		let stored_did = Did::<T>::get(&did_subject).expect("DID should be stored on chain.");
 		let stored_key_agreement_keys_ids = stored_did.get_key_agreement_keys_ids();
@@ -361,7 +355,6 @@ benchmarks! {
 			&Some(expected_attestation_key_id)
 		);
 		assert_eq!(stored_did.endpoint_url, did_update_op.new_endpoint_url);
-		assert_eq!(stored_did.last_tx_counter, did_update_op.tx_counter);
 	}
 
 	submit_did_update_operation_sr25519_keys {
@@ -397,7 +390,7 @@ benchmarks! {
 		did_update_op.new_endpoint_url = Some(new_url);
 
 		let did_update_signature = sr25519_sign(AUTHENTICATION_KEY_ID, &did_public_auth_key, did_update_op.encode().as_ref()).expect("Failed to create DID signature from raw sr25519 signature.");
-	}: submit_did_update_operation(RawOrigin::Signed(submitter), did_update_op.clone(), DidSignature::from(did_update_signature))
+	}: submit_did_update_operation(RawOrigin::Signed(submitter), did_update_op.clone())
 	verify {
 		let stored_did = Did::<T>::get(&did_subject).expect("DID should be stored on chain.");
 		let stored_key_agreement_keys_ids = stored_did.get_key_agreement_keys_ids();
@@ -423,7 +416,6 @@ benchmarks! {
 			&Some(expected_attestation_key_id)
 		);
 		assert_eq!(stored_did.endpoint_url, did_update_op.new_endpoint_url);
-		assert_eq!(stored_did.last_tx_counter, did_update_op.tx_counter);
 	}
 
 	submit_did_update_operation_ecdsa_keys {
@@ -459,7 +451,7 @@ benchmarks! {
 		did_update_op.new_endpoint_url = Some(new_url);
 
 		let did_update_signature = ecdsa_sign(AUTHENTICATION_KEY_ID, &did_public_auth_key, did_update_op.encode().as_ref()).expect("Failed to create DID signature from raw ecdsa signature.");
-	}: submit_did_update_operation(RawOrigin::Signed(submitter), did_update_op.clone(), DidSignature::from(did_update_signature))
+	}: submit_did_update_operation(RawOrigin::Signed(submitter), did_update_op.clone())
 	verify {
 		let stored_did = Did::<T>::get(&did_subject).expect("DID should be stored on chain.");
 		let stored_key_agreement_keys_ids = stored_did.get_key_agreement_keys_ids();
@@ -485,7 +477,6 @@ benchmarks! {
 			&Some(expected_attestation_key_id)
 		);
 		assert_eq!(stored_did.endpoint_url, did_update_op.new_endpoint_url);
-		assert_eq!(stored_did.last_tx_counter, did_update_op.tx_counter);
 	}
 
 	submit_did_delete_operation {
@@ -496,11 +487,7 @@ benchmarks! {
 
 		let did_details = get_did_base_details(DidVerificationKey::from(did_public_auth_key));
 		Did::<T>::insert(&did_subject, did_details);
-
-		let did_deletion_op = generate_base_did_deletion_operation::<T>(did_subject.clone());
-
-		let did_deletion_signature = ed25519_sign(AUTHENTICATION_KEY_ID, &did_public_auth_key, did_deletion_op.encode().as_ref()).expect("Failed to create DID signature from raw ed25519 signature.");
-	}: _(RawOrigin::Signed(submitter), did_deletion_op.clone(), DidSignature::from(did_deletion_signature))
+	}: _(RawOrigin::Signed(submitter))
 	verify {
 		assert_eq!(
 			Did::<T>::get(&did_subject),
@@ -520,7 +507,7 @@ benchmarks! {
 		let did_call_op = generate_base_did_call_operation::<T>(did_subject);
 
 		let did_call_signature = ed25519_sign(AUTHENTICATION_KEY_ID, &did_public_auth_key, did_call_op.encode().as_ref()).expect("Failed to create DID signature from raw ed25519 signature.");
-	}: submit_did_call(RawOrigin::Signed(submitter), Box::new(did_call_op.clone()), DidSignature::from(did_call_signature))
+	}: submit_did_call(RawOrigin::Signed(submitter), Box::new(did_call_op.clone()), Some(DidSignature::from(did_call_signature)))
 
 	submit_did_call_sr25519_key {
 		let submitter: AccountIdentifierOf<T> = account(DEFAULT_ACCOUNT_ID, 0, DEFAULT_ACCOUNT_SEED);
@@ -534,7 +521,7 @@ benchmarks! {
 		let did_call_op = generate_base_did_call_operation::<T>(did_subject);
 
 		let did_call_signature = sr25519_sign(AUTHENTICATION_KEY_ID, &did_public_auth_key, did_call_op.encode().as_ref()).expect("Failed to create DID signature from raw sr25519 signature.");
-	}: submit_did_call(RawOrigin::Signed(submitter), Box::new(did_call_op.clone()), DidSignature::from(did_call_signature))
+	}: submit_did_call(RawOrigin::Signed(submitter), Box::new(did_call_op.clone()), Some(DidSignature::from(did_call_signature)))
 
 	submit_did_call_ecdsa_key {
 		let submitter: AccountIdentifierOf<T> = account(DEFAULT_ACCOUNT_ID, 0, DEFAULT_ACCOUNT_SEED);
@@ -548,7 +535,7 @@ benchmarks! {
 		let did_call_op = generate_base_did_call_operation::<T>(did_subject);
 
 		let did_call_signature = ecdsa_sign(AUTHENTICATION_KEY_ID, &did_public_auth_key, did_call_op.encode().as_ref()).expect("Failed to create DID signature from raw ecdsa signature.");
-	}: submit_did_call(RawOrigin::Signed(submitter), Box::new(did_call_op.clone()), DidSignature::from(did_call_signature))
+	}: submit_did_call(RawOrigin::Signed(submitter), Box::new(did_call_op.clone()), Some(DidSignature::from(did_call_signature)))
 }
 
 // impl_benchmark_test_suite! {

pallets/did/src/errors.rs

@@ -64,6 +64,8 @@ pub enum SignatureError {
 	InvalidSignature,
 	/// The operation nonce is not equal to the current DID nonce + 1.
 	InvalidNonce,
+	/// The operation required a signature which has not been provided.
+	SignatureRequired,
 }
 
 /// Error generated when validating a byte-encoded endpoint URL.

pallets/did/src/lib.rs

@@ -230,6 +230,8 @@ pub mod pallet {
 		/// The DID operation signature is invalid for the payload and the
 		/// verification key provided.
 		InvalidSignature,
+		/// The operation required a signature which has not been provided.
+		SignatureRequired,
 		/// The DID with the given identifier is already present on chain.
 		DidAlreadyPresent,
 		/// No DID with the given identifier is present on chain.
@@ -296,6 +298,7 @@ pub mod pallet {
 				SignatureError::InvalidSignature => Self::InvalidSignature,
 				SignatureError::InvalidSignatureFormat => Self::InvalidSignatureFormat,
 				SignatureError::InvalidNonce => Self::InvalidNonce,
+				SignatureError::SignatureRequired => Self::SignatureRequired,
 			}
 		}
 	}
@@ -579,7 +582,7 @@ pub mod pallet {
 		pub fn submit_did_call(
 			origin: OriginFor<T>,
 			did_call: Box<DidAuthorizedCallOperation<T>>,
-			signature: DidSignature,
+			signature: Option<DidSignature>,
 		) -> DispatchResultWithPostInfo {
 			ensure_signed(origin)?;
 
@@ -632,7 +635,7 @@ impl<T: Config> Pallet<T> {
 	/// # </weight>
 	pub fn verify_did_operation_signature_and_increase_nonce(
 		operation: &DidAuthorizedCallOperationWithVerificationRelationship<T>,
-		signature: &DidSignature,
+		signature: &Option<DidSignature>,
 	) -> Result<(), DidError> {
 		let mut did_details =
 			<Did<T>>::get(&operation.did).ok_or(DidError::StorageError(StorageError::DidNotPresent))?;
@@ -641,6 +644,7 @@ impl<T: Config> Pallet<T> {
 			// Do nothing if no key is required.
 			DidOperationAuthorizationKey::NoKey => (),
 			DidOperationAuthorizationKey::DidKey(verification_key_type) => {
+				let signature = signature.as_ref().ok_or(DidError::SignatureError(SignatureError::SignatureRequired))?;
 				Self::validate_counter_value(operation.tx_counter, &did_details)?;
 				// Increase the tx counter as soon as it is considered valid, no matter if the signature is valid or not.
 				did_details.increase_tx_counter().map_err(DidError::StorageError)?;

pallets/did/src/mock.rs

@@ -22,8 +22,6 @@
 use codec::{Decode, Encode};
 use frame_support::{parameter_types, weights::constants::RocksDbWeight};
 use frame_system::EnsureSigned;
-#[cfg(feature = "runtime-benchmarks")]
-use frame_system::EnsureSigned;
 use sp_core::{ecdsa, ed25519, sr25519, Pair};
 use sp_keystore::{testing::KeyStore, KeystoreExt};
 use sp_runtime::{
@@ -334,7 +332,7 @@ impl did::DeriveDidCallAuthorizationVerificationKeyRelationship for Call {
 			#[cfg(feature = "runtime-benchmarks")]
 			if *self == Self::get_call_for_did_call_benchmark() {
 				// Always require an authentication key to dispatch calls during benchmarking
-				return Some(did::DidVerificationKeyRelationship::Authentication);
+				return Some(did::DidOperationAuthorizationKey::DidKey(did::DidVerificationKeyRelationship::Authentication));
 			}
 			None
 		}
@@ -348,31 +346,26 @@ impl did::DeriveDidCallAuthorizationVerificationKeyRelationship for Call {
 }
 
 pub fn generate_test_did_call(
-	verification_key_required: did::DidVerificationKeyRelationship,
+	verification_key_required: did::DidOperationAuthorizationKey,
 	caller: TestDidIdentifier,
-) -> did::DidAuthorizedCallOperation<Test> {
+) -> did::DidAuthorizedCallOperationWithVerificationRelationship<Test> {
 	let call = match verification_key_required {
-		DidVerificationKeyRelationship::AssertionMethod => get_attestation_key_call(),
-		DidVerificationKeyRelationship::Authentication => get_authentication_key_call(),
-		DidVerificationKeyRelationship::CapabilityDelegation => get_delegation_key_call(),
-		_ => get_none_key_call(),
+		DidOperationAuthorizationKey::DidKey(DidVerificationKeyRelationship::AssertionMethod) => get_attestation_key_call(),
+		DidOperationAuthorizationKey::DidKey(DidVerificationKeyRelationship::Authentication) => get_authentication_key_call(),
+		DidOperationAuthorizationKey::DidKey(DidVerificationKeyRelationship::CapabilityDelegation) => get_delegation_key_call(),
+		DidOperationAuthorizationKey::DidKey(_) => get_none_key_call(),
+		DidOperationAuthorizationKey::NoKey => get_no_key_call(),
 	};
-	did::DidAuthorizedCallOperation {
-		did: caller,
-		call,
-		tx_counter: 1u64,
+	did::DidAuthorizedCallOperationWithVerificationRelationship {
+		operation: did::DidAuthorizedCallOperation {
+			did: caller,
+			call,
+			tx_counter: 1u64
+		},
+		operation_authorization_key_type: verification_key_required
 	}
 }
 
-// A test DID operation which can be crated to require any DID verification key
-// type.
-#[derive(Clone, Decode, Debug, Encode, PartialEq)]
-pub struct TestDidOperation {
-	pub did: TestDidIdentifier,
-	pub verification_key_type: DidVerificationKeyRelationship,
-	pub tx_counter: u64,
-}
-
 #[allow(dead_code)]
 pub fn initialize_logger() {
 	env_logger::builder().is_test(true).try_init();