feat: add meaningful errors

crates/kilt-dip-support/Cargo.toml

@@ -13,6 +13,7 @@ version.workspace = true
 [dependencies]
 # External dependencies
 hash-db.workspace = true
+log.workspace = true
 
 # Internal dependencies
 did.workspace = true

crates/kilt-dip-support/src/did.rs

@@ -25,7 +25,7 @@ use parity_scale_codec::{Decode, Encode};
 use scale_info::TypeInfo;
 use sp_core::RuntimeDebug;
 use sp_runtime::traits::CheckedSub;
-use sp_std::marker::PhantomData;
+use sp_std::{marker::PhantomData, vec::Vec};
 
 use crate::{
 	merkle::RevealedDidKey,
@@ -44,6 +44,24 @@ pub struct TimeBoundDidSignature<BlockNumber> {
 	pub block_number: BlockNumber,
 }
 
+pub enum RevealedDidKeysSignatureAndCallVerifierError {
+	SignatureNotFresh,
+	SignatureUnverifiable,
+	OriginCheckFailed,
+	Internal,
+}
+
+impl From<RevealedDidKeysSignatureAndCallVerifierError> for u8 {
+	fn from(value: RevealedDidKeysSignatureAndCallVerifierError) -> Self {
+		match value {
+			RevealedDidKeysSignatureAndCallVerifierError::SignatureNotFresh => 0,
+			RevealedDidKeysSignatureAndCallVerifierError::SignatureUnverifiable => 1,
+			RevealedDidKeysSignatureAndCallVerifierError::OriginCheckFailed => 2,
+			RevealedDidKeysSignatureAndCallVerifierError::Internal => u8::MAX,
+		}
+	}
+}
+
 pub(crate) struct RevealedDidKeysSignatureAndCallVerifier<
 	Call,
 	Submitter,
@@ -109,7 +127,10 @@ impl<
 		submitter: &Submitter,
 		local_details: &mut Option<DidLocalDetails>,
 		merkle_revealed_did_signature: RevealedDidKeysAndSignature<MerkleProofEntries, ContextProvider::BlockNumber>,
-	) -> Result<(DidVerificationKey<RemoteAccountId>, DidVerificationKeyRelationship), ()> {
+	) -> Result<
+		(DidVerificationKey<RemoteAccountId>, DidVerificationKeyRelationship),
+		RevealedDidKeysSignatureAndCallVerifierError,
+	> {
 		let block_number = ContextProvider::block_number();
 		let is_signature_fresh = if let Some(blocks_ago_from_now) =
 			block_number.checked_sub(&merkle_revealed_did_signature.did_signature.block_number)
@@ -120,7 +141,10 @@ impl<
 			// Signature generated at a future time, not possible to verify.
 			false
 		};
-		ensure!(is_signature_fresh, ());
+		ensure!(
+			is_signature_fresh,
+			RevealedDidKeysSignatureAndCallVerifierError::SignatureNotFresh,
+		);
 		let encoded_payload = (
 			call,
 			&local_details,
@@ -131,24 +155,32 @@ impl<
 		)
 			.encode();
 		// Only consider verification keys from the set of revealed keys.
-		let mut proof_verification_keys = merkle_revealed_did_signature.merkle_leaves.borrow().iter().filter_map(|RevealedDidKey {
+		let proof_verification_keys: Vec<(DidVerificationKey<RemoteAccountId>, DidVerificationKeyRelationship)> = merkle_revealed_did_signature.merkle_leaves.borrow().iter().filter_map(|RevealedDidKey {
 			relationship, details: DidPublicKeyDetails { key, .. }, .. } | {
 				let DidPublicKey::PublicVerificationKey(key) = key else { return None };
-					Some((key, DidVerificationKeyRelationship::try_from(*relationship).expect("Should never fail to build a VerificationRelationship from the given DidKeyRelationship because we have already made sure the conditions hold."))) 		});
-		let valid_signing_key = proof_verification_keys.find(|(verification_key, _)| {
+				if let Ok(vr) = DidVerificationKeyRelationship::try_from(*relationship) {
+					// TODO: Fix this logic to avoid cloning
+					Some(Ok((key.clone(), vr)))
+				} else {
+					log::error!("Should never fail to build a VerificationRelationship from the given DidKeyRelationship because we have already made sure the conditions hold.");
+					Some(Err(RevealedDidKeysSignatureAndCallVerifierError::Internal))
+				}
+			}).collect::<Result<_, _>>()?;
+		let valid_signing_key = proof_verification_keys.iter().find(|(verification_key, _)| {
 			verification_key
 				.verify_signature(&encoded_payload, &merkle_revealed_did_signature.did_signature.signature)
 				.is_ok()
 		});
 		let Some((key, relationship)) = valid_signing_key else {
-			return Err(());
+			return Err(RevealedDidKeysSignatureAndCallVerifierError::SignatureUnverifiable);
 		};
 		if let Some(details) = local_details {
 			details.bump();
 		} else {
 			*local_details = Some(DidLocalDetails::default());
 		};
-		CallVerifier::check_call_origin_info(call, &(key.clone(), relationship)).map_err(|_| ())?;
-		Ok((key.clone(), relationship))
+		CallVerifier::check_call_origin_info(call, &(key.clone(), *relationship))
+			.map_err(|_| RevealedDidKeysSignatureAndCallVerifierError::OriginCheckFailed)?;
+		Ok((key.clone(), *relationship))
 	}
 }

crates/kilt-dip-support/src/lib.rs

@@ -33,9 +33,18 @@ use ::did::{did_details::DidVerificationKey, DidVerificationKeyRelationship};
 use pallet_dip_consumer::traits::IdentityProofVerifier;
 
 use crate::{
-	did::{RevealedDidKeysAndSignature, RevealedDidKeysSignatureAndCallVerifier, TimeBoundDidSignature},
-	merkle::{DidMerkleProof, DidMerkleProofVerifier, RevealedDidMerkleProofLeaf, RevealedDidMerkleProofLeaves},
-	state_proofs::{parachain::DipIdentityCommitmentProofVerifier, relay_chain::ParachainHeadProofVerifier},
+	did::{
+		RevealedDidKeysAndSignature, RevealedDidKeysSignatureAndCallVerifier,
+		RevealedDidKeysSignatureAndCallVerifierError, TimeBoundDidSignature,
+	},
+	merkle::{
+		DidMerkleProof, DidMerkleProofVerifier, DidMerkleProofVerifierError, RevealedDidMerkleProofLeaf,
+		RevealedDidMerkleProofLeaves,
+	},
+	state_proofs::{
+		parachain::{DipIdentityCommitmentProofVerifier, DipIdentityCommitmentProofVerifierError},
+		relay_chain::{ParachainHeadProofVerifier, ParachainHeadProofVerifierError},
+	},
 	traits::{
 		Bump, DidSignatureVerifierContext, DipCallOriginFilter, HistoricalBlockRegistry, ProviderParachainStateInfo,
 		RelayChainStorageInfo,
@@ -75,6 +84,63 @@ pub struct DipMerkleProofAndDidSignature<BlindedValues, Leaf, BlockNumber> {
 	signature: TimeBoundDidSignature<BlockNumber>,
 }
 
+pub enum DipSiblingProviderStateProofVerifierError<
+	ParachainHeadMerkleProofVerificationError,
+	IdentityCommitmentMerkleProofVerificationError,
+	DipProofVerificationError,
+	DidSignatureVerificationError,
+> {
+	ParachainHeadMerkleProofVerificationError(ParachainHeadMerkleProofVerificationError),
+	IdentityCommitmentMerkleProofVerificationError(IdentityCommitmentMerkleProofVerificationError),
+	DipProofVerificationError(DipProofVerificationError),
+	DidSignatureVerificationError(DidSignatureVerificationError),
+}
+
+impl<
+		ParachainHeadMerkleProofVerificationError,
+		IdentityCommitmentMerkleProofVerificationError,
+		DipProofVerificationError,
+		DidSignatureVerificationError,
+	>
+	From<
+		DipSiblingProviderStateProofVerifierError<
+			ParachainHeadMerkleProofVerificationError,
+			IdentityCommitmentMerkleProofVerificationError,
+			DipProofVerificationError,
+			DidSignatureVerificationError,
+		>,
+	> for u16
+where
+	ParachainHeadMerkleProofVerificationError: Into<u8>,
+	IdentityCommitmentMerkleProofVerificationError: Into<u8>,
+	DipProofVerificationError: Into<u8>,
+	DidSignatureVerificationError: Into<u8>,
+{
+	fn from(
+		value: DipSiblingProviderStateProofVerifierError<
+			ParachainHeadMerkleProofVerificationError,
+			IdentityCommitmentMerkleProofVerificationError,
+			DipProofVerificationError,
+			DidSignatureVerificationError,
+		>,
+	) -> Self {
+		match value {
+			DipSiblingProviderStateProofVerifierError::ParachainHeadMerkleProofVerificationError(error) => {
+				error.into() as u16
+			}
+			DipSiblingProviderStateProofVerifierError::IdentityCommitmentMerkleProofVerificationError(error) => {
+				u8::MAX as u16 + error.into() as u16
+			}
+			DipSiblingProviderStateProofVerifierError::DipProofVerificationError(error) => {
+				u8::MAX as u16 * 2 + error.into() as u16
+			}
+			DipSiblingProviderStateProofVerifierError::DidSignatureVerificationError(error) => {
+				u8::MAX as u16 * 3 + error.into() as u16
+			}
+		}
+	}
+}
+
 #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)]
 pub struct DipSiblingProviderStateProofVerifier<
 	RelayChainStateInfo,
@@ -174,7 +240,12 @@ impl<
 	ProviderLinkedAccountId: Encode + Clone,
 	ProviderWeb3Name: Encode + Clone,
 {
-	type Error = ();
+	type Error = DipSiblingProviderStateProofVerifierError<
+		ParachainHeadProofVerifierError,
+		DipIdentityCommitmentProofVerifierError,
+		DidMerkleProofVerifierError,
+		RevealedDidKeysSignatureAndCallVerifierError,
+	>;
 	type IdentityDetails = LocalDidDetails;
 	type Proof = SiblingParachainDipStateProof<
 		RelayChainStateInfo::BlockNumber,
@@ -211,15 +282,17 @@ impl<
 			&SiblingProviderParachainId::get(),
 			&proof.para_state_root.relay_block_height,
 			proof.para_state_root.proof,
-		)?;
+		)
+		.map_err(DipSiblingProviderStateProofVerifierError::ParachainHeadMerkleProofVerificationError)?;
 
 		// 2. Verify parachain state proof.
 		let subject_identity_commitment =
 			DipIdentityCommitmentProofVerifier::<SiblingProviderStateInfo>::verify_proof_for_identifier(
 				subject,
 				provider_parachain_header.state_root.into(),
 				proof.dip_identity_commitment,
-			)?;
+			)
+			.map_err(DipSiblingProviderStateProofVerifierError::IdentityCommitmentMerkleProofVerificationError)?;
 
 		// 3. Verify DIP merkle proof.
 		let proof_leaves = DidMerkleProofVerifier::<
@@ -231,7 +304,8 @@ impl<
 			_,
 			MAX_REVEALED_KEYS_COUNT,
 			MAX_REVEALED_ACCOUNTS_COUNT,
-		>::verify_dip_merkle_proof(&subject_identity_commitment, proof.did.leaves)?;
+		>::verify_dip_merkle_proof(&subject_identity_commitment, proof.did.leaves)
+		.map_err(DipSiblingProviderStateProofVerifierError::DipProofVerificationError)?;
 
 		// 4. Verify DID signature.
 		RevealedDidKeysSignatureAndCallVerifier::<
@@ -252,7 +326,7 @@ impl<
 				merkle_leaves: proof_leaves.borrow(),
 				did_signature: proof.did.signature,
 			},
-		)?;
+		).map_err(DipSiblingProviderStateProofVerifierError::DidSignatureVerificationError)?;
 
 		Ok(proof_leaves)
 	}
@@ -271,6 +345,67 @@ pub struct ChildParachainDipStateProof<
 	did: DipMerkleProofAndDidSignature<DipMerkleProofBlindedValues, DipMerkleProofRevealedLeaf, ParentBlockHeight>,
 }
 
+pub enum DipChildProviderStateProofVerifierError<
+	ParachainHeadMerkleProofVerificationError,
+	IdentityCommitmentMerkleProofVerificationError,
+	DipProofVerificationError,
+	DidSignatureVerificationError,
+> {
+	InvalidBlockHeight,
+	InvalidBlockHash,
+	ParachainHeadMerkleProofVerificationError(ParachainHeadMerkleProofVerificationError),
+	IdentityCommitmentMerkleProofVerificationError(IdentityCommitmentMerkleProofVerificationError),
+	DipProofVerificationError(DipProofVerificationError),
+	DidSignatureVerificationError(DidSignatureVerificationError),
+}
+
+impl<
+		ParachainHeadMerkleProofVerificationError,
+		IdentityCommitmentMerkleProofVerificationError,
+		DipProofVerificationError,
+		DidSignatureVerificationError,
+	>
+	From<
+		DipChildProviderStateProofVerifierError<
+			ParachainHeadMerkleProofVerificationError,
+			IdentityCommitmentMerkleProofVerificationError,
+			DipProofVerificationError,
+			DidSignatureVerificationError,
+		>,
+	> for u16
+where
+	ParachainHeadMerkleProofVerificationError: Into<u8>,
+	IdentityCommitmentMerkleProofVerificationError: Into<u8>,
+	DipProofVerificationError: Into<u8>,
+	DidSignatureVerificationError: Into<u8>,
+{
+	fn from(
+		value: DipChildProviderStateProofVerifierError<
+			ParachainHeadMerkleProofVerificationError,
+			IdentityCommitmentMerkleProofVerificationError,
+			DipProofVerificationError,
+			DidSignatureVerificationError,
+		>,
+	) -> Self {
+		match value {
+			DipChildProviderStateProofVerifierError::InvalidBlockHeight => 0,
+			DipChildProviderStateProofVerifierError::InvalidBlockHash => 1,
+			DipChildProviderStateProofVerifierError::ParachainHeadMerkleProofVerificationError(error) => {
+				u8::MAX as u16 + error.into() as u16
+			}
+			DipChildProviderStateProofVerifierError::IdentityCommitmentMerkleProofVerificationError(error) => {
+				u8::MAX as u16 * 2 + error.into() as u16
+			}
+			DipChildProviderStateProofVerifierError::DipProofVerificationError(error) => {
+				u8::MAX as u16 * 3 + error.into() as u16
+			}
+			DipChildProviderStateProofVerifierError::DidSignatureVerificationError(error) => {
+				u8::MAX as u16 * 4 + error.into() as u16
+			}
+		}
+	}
+}
+
 pub struct DipChildProviderStateProofVerifier<
 	RelayChainInfo,
 	ChildProviderParachainId,
@@ -382,7 +517,12 @@ impl<
 	ProviderLinkedAccountId: Encode + Clone,
 	ProviderWeb3Name: Encode + Clone,
 {
-	type Error = ();
+	type Error = DipChildProviderStateProofVerifierError<
+		ParachainHeadProofVerifierError,
+		DipIdentityCommitmentProofVerifierError,
+		DidMerkleProofVerifierError,
+		RevealedDidKeysSignatureAndCallVerifierError,
+	>;
 	type IdentityDetails = LocalDidDetails;
 	type Proof = ChildParachainDipStateProof<
 		<RelayChainInfo as RelayChainStorageInfo>::BlockNumber,
@@ -415,12 +555,12 @@ impl<
 		proof: Self::Proof,
 	) -> Result<Self::VerificationResult, Self::Error> {
 		// 1. Retrieve block hash from provider at the proof height
-		let block_hash_at_height =
-			RelayChainInfo::block_hash_for(&proof.para_state_root.relay_block_height).ok_or(())?;
+		let block_hash_at_height = RelayChainInfo::block_hash_for(&proof.para_state_root.relay_block_height)
+			.ok_or(DipChildProviderStateProofVerifierError::InvalidBlockHeight)?;
 
 		// 1.1 Verify that the provided header hashes to the same block has retrieved
 		if block_hash_at_height != proof.relay_header.hash() {
-			return Err(());
+			return Err(DipChildProviderStateProofVerifierError::InvalidBlockHash);
 		}
 		// 1.2 If so, extract the state root from the header
 		let state_root_at_height = proof.relay_header.state_root;
@@ -432,15 +572,17 @@ impl<
 				&ChildProviderParachainId::get(),
 				&state_root_at_height,
 				proof.para_state_root.proof,
-			)?;
+			)
+			.map_err(DipChildProviderStateProofVerifierError::ParachainHeadMerkleProofVerificationError)?;
 
 		// 3. Verify parachain state proof.
 		let subject_identity_commitment =
 			DipIdentityCommitmentProofVerifier::<ChildProviderStateInfo>::verify_proof_for_identifier(
 				subject,
 				provider_parachain_header.state_root.into(),
 				proof.dip_identity_commitment,
-			)?;
+			)
+			.map_err(DipChildProviderStateProofVerifierError::IdentityCommitmentMerkleProofVerificationError)?;
 
 		// 4. Verify DIP merkle proof.
 		let proof_leaves = DidMerkleProofVerifier::<
@@ -452,7 +594,8 @@ impl<
 			_,
 			MAX_REVEALED_KEYS_COUNT,
 			MAX_REVEALED_ACCOUNTS_COUNT,
-		>::verify_dip_merkle_proof(&subject_identity_commitment, proof.did.leaves)?;
+		>::verify_dip_merkle_proof(&subject_identity_commitment, proof.did.leaves)
+		.map_err(DipChildProviderStateProofVerifierError::DipProofVerificationError)?;
 
 		// 5. Verify DID signature.
 		RevealedDidKeysSignatureAndCallVerifier::<
@@ -473,7 +616,7 @@ impl<
 				merkle_leaves: proof_leaves.borrow(),
 				did_signature: proof.did.signature,
 			},
-		)?;
+		).map_err(DipChildProviderStateProofVerifierError::DidSignatureVerificationError)?;
 		Ok(proof_leaves)
 	}
 }