diff --git a/Cargo.lock b/Cargo.lock index 2370e0c8c7..a5aef17c56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4530,6 +4530,7 @@ dependencies = [ "frame-system", "hash-db", "hex-literal 0.3.4", + "log", "pallet-dip-consumer", "pallet-dip-provider", "pallet-relay-store", diff --git a/crates/kilt-dip-support/Cargo.toml b/crates/kilt-dip-support/Cargo.toml index 49893b6fd6..8941e0e46d 100644 --- a/crates/kilt-dip-support/Cargo.toml +++ b/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 diff --git a/crates/kilt-dip-support/src/did.rs b/crates/kilt-dip-support/src/did.rs index 65e7ff81b4..7b16bcbb8e 100644 --- a/crates/kilt-dip-support/src/did.rs +++ b/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 { pub block_number: BlockNumber, } +pub enum RevealedDidKeysSignatureAndCallVerifierError { + SignatureNotFresh, + SignatureUnverifiable, + OriginCheckFailed, + Internal, +} + +impl From 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, merkle_revealed_did_signature: RevealedDidKeysAndSignature, - ) -> Result<(DidVerificationKey, DidVerificationKeyRelationship), ()> { + ) -> Result< + (DidVerificationKey, 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, 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::>()?; + 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)) } } diff --git a/crates/kilt-dip-support/src/lib.rs b/crates/kilt-dip-support/src/lib.rs index 9bfc305b1f..0d0e80425e 100644 --- a/crates/kilt-dip-support/src/lib.rs +++ b/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 { signature: TimeBoundDidSignature, } +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, + IdentityCommitmentMerkleProofVerificationError: Into, + DipProofVerificationError: Into, + DidSignatureVerificationError: Into, +{ + 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,7 +282,8 @@ 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 = @@ -219,7 +291,8 @@ impl< 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, } +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, + IdentityCommitmentMerkleProofVerificationError: Into, + DipProofVerificationError: Into, + DidSignatureVerificationError: Into, +{ + 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< ::BlockNumber, @@ -415,12 +555,12 @@ impl< proof: Self::Proof, ) -> Result { // 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,7 +572,8 @@ 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 = @@ -440,7 +581,8 @@ impl< 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) } } diff --git a/crates/kilt-dip-support/src/merkle.rs b/crates/kilt-dip-support/src/merkle.rs index a346659444..2f23c952e5 100644 --- a/crates/kilt-dip-support/src/merkle.rs +++ b/crates/kilt-dip-support/src/merkle.rs @@ -44,7 +44,6 @@ impl From for DidKeyRelationship { } impl TryFrom for DidVerificationKeyRelationship { - // TODO: Error handling type Error = (); fn try_from(value: DidKeyRelationship) -> Result { @@ -179,32 +178,6 @@ pub struct RevealedDidMerkleProofLeaves< pub linked_accounts: BoundedVec>, } -// impl< -// KeyId, -// AccountId, -// BlockNumber, -// Web3Name, -// LinkedAccountId, -// const MAX_REVEALED_KEYS_COUNT: u32, -// const MAX_REVEALED_ACCOUNTS_COUNT: u32, -// > Default for RevealedDidMerkleProofLeaves< -// KeyId, -// AccountId, -// BlockNumber, -// Web3Name, -// LinkedAccountId, -// MAX_REVEALED_KEYS_COUNT, -// MAX_REVEALED_ACCOUNTS_COUNT, -// > { -// fn default() -> Self { -// Self { -// did_keys: BoundedVec::default(), -// linked_accounts: BoundedVec::default(), -// web3_name: None, -// } -// } -// } - impl< KeyId, AccountId, @@ -229,6 +202,22 @@ impl< } } +pub enum DidMerkleProofVerifierError { + InvalidMerkleProof, + TooManyRevealedKeys, + TooManyRevealedAccounts, +} + +impl From for u8 { + fn from(value: DidMerkleProofVerifierError) -> Self { + match value { + DidMerkleProofVerifierError::InvalidMerkleProof => 0, + DidMerkleProofVerifierError::TooManyRevealedKeys => 1, + DidMerkleProofVerifierError::TooManyRevealedAccounts => 2, + } + } +} + /// A type that verifies a Merkle proof that reveals some leaves representing /// keys in a DID Document. /// Can also be used on its own, without any DID signature verification. @@ -288,7 +277,7 @@ impl< MAX_REVEALED_KEYS_COUNT, MAX_REVEALED_ACCOUNTS_COUNT, >, - (), + DidMerkleProofVerifierError, > { // TODO: more efficient by removing cloning and/or collecting. // Did not find another way of mapping a Vec<(Vec, Vec)> to a @@ -299,7 +288,7 @@ impl< .map(|leaf| (leaf.encoded_key(), Some(leaf.encoded_value()))) .collect::, Option>)>>(); verify_trie_proof::, _, _, _>(identity_commitment, &proof.blinded, &proof_leaves) - .map_err(|_| ())?; + .map_err(|_| DidMerkleProofVerifierError::InvalidMerkleProof)?; // At this point, we know the proof is valid. We just need to map the revealed // leaves to something the consumer can easily operate on. @@ -322,8 +311,8 @@ impl< relationship: key_id.1, details: key_value.0.clone(), }) - .map_err(|_| ())?; - Ok::<_, ()>((keys, web3_name, linked_accounts)) + .map_err(|_| DidMerkleProofVerifierError::TooManyRevealedKeys)?; + Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) } // TODO: Avoid cloning if possible RevealedDidMerkleProofLeaf::Web3Name(revealed_web3_name, details) => Ok(( @@ -335,8 +324,10 @@ impl< linked_accounts, )), RevealedDidMerkleProofLeaf::LinkedAccount(account_id, _) => { - linked_accounts.try_push(account_id.0.clone()).map_err(|_| ())?; - Ok::<_, ()>((keys, web3_name, linked_accounts)) + linked_accounts + .try_push(account_id.0.clone()) + .map_err(|_| DidMerkleProofVerifierError::TooManyRevealedAccounts)?; + Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) } }, )?; diff --git a/crates/kilt-dip-support/src/state_proofs.rs b/crates/kilt-dip-support/src/state_proofs.rs index 94a9a63421..53099e6adb 100644 --- a/crates/kilt-dip-support/src/state_proofs.rs +++ b/crates/kilt-dip-support/src/state_proofs.rs @@ -17,7 +17,7 @@ // If you feel like getting in touch with us, you can do so at info@botlabs.org use parity_scale_codec::{Decode, Encode, HasCompact}; -use sp_core::{storage::StorageKey, U256}; +use sp_core::{storage::StorageKey, RuntimeDebug, U256}; use sp_runtime::generic::Header; use sp_std::{marker::PhantomData, vec::Vec}; use sp_trie::StorageProof; @@ -92,6 +92,25 @@ pub(super) mod relay_chain { use crate::traits::{RelayChainStateInfo, RelayChainStorageInfo}; + #[derive(RuntimeDebug)] + pub enum ParachainHeadProofVerifierError { + InvalidMerkleProof, + RequiredLeafNotRevealed, + HeaderDecode, + RelaychainStateRootNotFound, + } + + impl From for u8 { + fn from(value: ParachainHeadProofVerifierError) -> Self { + match value { + ParachainHeadProofVerifierError::InvalidMerkleProof => 0, + ParachainHeadProofVerifierError::RequiredLeafNotRevealed => 1, + ParachainHeadProofVerifierError::HeaderDecode => 2, + ParachainHeadProofVerifierError::RelaychainStateRootNotFound => 3, + } + } + } + pub struct ParachainHeadProofVerifier(PhantomData); // Uses the provided `root` to verify the proof. @@ -106,23 +125,23 @@ pub(super) mod relay_chain { para_id: &RelayChainState::ParaId, root: &OutputOf<::Hasher>, proof: impl IntoIterator>, - ) -> Result, ()> { + ) -> Result, ParachainHeadProofVerifierError> { let parachain_storage_key = RelayChainState::parachain_head_storage_key(para_id); let storage_proof = StorageProof::new(proof); let revealed_leaves = read_proof_check::(*root, storage_proof, [¶chain_storage_key].iter()) - .map_err(|_| ())?; + .map_err(|_| ParachainHeadProofVerifierError::InvalidMerkleProof)?; // TODO: Remove at some point { debug_assert!(revealed_leaves.len() == 1usize); debug_assert!(revealed_leaves.contains_key(parachain_storage_key.as_ref())); } let Some(Some(encoded_head)) = revealed_leaves.get(parachain_storage_key.as_ref()) else { - return Err(()); + return Err(ParachainHeadProofVerifierError::RequiredLeafNotRevealed); }; // TODO: Figure out why RPC call returns 2 bytes in front which we don't need let mut unwrapped_head = &encoded_head[2..]; - Header::decode(&mut unwrapped_head).map_err(|_| ()) + Header::decode(&mut unwrapped_head).map_err(|_| ParachainHeadProofVerifierError::HeaderDecode) } } @@ -140,8 +159,9 @@ pub(super) mod relay_chain { para_id: &RelayChainState::ParaId, relay_height: &RelayChainState::BlockNumber, proof: impl IntoIterator>, - ) -> Result, ()> { - let relay_state_root = RelayChainState::state_root_for_block(relay_height).ok_or(())?; + ) -> Result, ParachainHeadProofVerifierError> { + let relay_state_root = RelayChainState::state_root_for_block(relay_height) + .ok_or(ParachainHeadProofVerifierError::RelaychainStateRootNotFound)?; Self::verify_proof_for_parachain_with_root(para_id, &relay_state_root, proof) } } @@ -250,6 +270,23 @@ pub(super) mod parachain { use crate::traits::ProviderParachainStateInfo; + #[derive(RuntimeDebug)] + pub enum DipIdentityCommitmentProofVerifierError { + InvalidMerkleProof, + RequiredLeafNotRevealed, + CommitmentDecode, + } + + impl From for u8 { + fn from(value: DipIdentityCommitmentProofVerifierError) -> Self { + match value { + DipIdentityCommitmentProofVerifierError::InvalidMerkleProof => 0, + DipIdentityCommitmentProofVerifierError::RequiredLeafNotRevealed => 1, + DipIdentityCommitmentProofVerifierError::CommitmentDecode => 2, + } + } + } + pub struct DipIdentityCommitmentProofVerifier(PhantomData); impl DipIdentityCommitmentProofVerifier @@ -264,7 +301,7 @@ pub(super) mod parachain { identifier: &ParaInfo::Identifier, state_root: OutputOf, proof: impl IntoIterator>, - ) -> Result { + ) -> Result { let dip_commitment_storage_key = ParaInfo::dip_subject_storage_key(identifier); let storage_proof = StorageProof::new(proof); let revealed_leaves = read_proof_check::( @@ -272,16 +309,17 @@ pub(super) mod parachain { storage_proof, [&dip_commitment_storage_key].iter(), ) - .map_err(|_| ())?; + .map_err(|_| DipIdentityCommitmentProofVerifierError::InvalidMerkleProof)?; // TODO: Remove at some point { debug_assert!(revealed_leaves.len() == 1usize); debug_assert!(revealed_leaves.contains_key(dip_commitment_storage_key.as_ref())); } let Some(Some(encoded_commitment)) = revealed_leaves.get(dip_commitment_storage_key.as_ref()) else { - return Err(()); + return Err(DipIdentityCommitmentProofVerifierError::RequiredLeafNotRevealed); }; - ParaInfo::Commitment::decode(&mut &encoded_commitment[..]).map_err(|_| ()) + ParaInfo::Commitment::decode(&mut &encoded_commitment[..]) + .map_err(|_| DipIdentityCommitmentProofVerifierError::CommitmentDecode) } } diff --git a/crates/kilt-dip-support/src/utils.rs b/crates/kilt-dip-support/src/utils.rs index 85fa1334f9..30d2d01d50 100644 --- a/crates/kilt-dip-support/src/utils.rs +++ b/crates/kilt-dip-support/src/utils.rs @@ -17,6 +17,8 @@ // If you feel like getting in touch with us, you can do so at info@botlabs.org use pallet_dip_provider::traits::IdentityProvider; +use parity_scale_codec::{Decode, Encode}; +use scale_info::TypeInfo; use sp_std::marker::PhantomData; pub struct CombinedIdentityResult { @@ -81,14 +83,35 @@ where pub struct CombineIdentityFrom(PhantomData<(A, B, C)>); +#[derive(Encode, Decode, TypeInfo)] +pub enum CombineError { + A(ErrorA), + B(ErrorB), + C(ErrorC), +} + +impl From> for u16 +where + ErrorA: Into, + ErrorB: Into, + ErrorC: Into, +{ + fn from(value: CombineError) -> Self { + match value { + CombineError::A(error) => error.into(), + CombineError::B(error) => error.into(), + CombineError::C(error) => error.into(), + } + } +} + impl IdentityProvider for CombineIdentityFrom where A: IdentityProvider, B: IdentityProvider, C: IdentityProvider, { - // TODO: Proper error handling - type Error = (); + type Error = CombineError; type Success = CombinedIdentityResult, Option, Option>; fn retrieve(identifier: &Identifier) -> Result, Self::Error> { @@ -105,8 +128,9 @@ where b: ok_b, c: ok_c, })), - // If any of them returns an `Err`, return an `Err` - _ => Err(()), + (Err(e), _, _) => Err(CombineError::A(e)), + (_, Err(e), _) => Err(CombineError::B(e)), + (_, _, Err(e)) => Err(CombineError::C(e)), } } } diff --git a/dip-template/runtimes/dip-consumer/src/dip.rs b/dip-template/runtimes/dip-consumer/src/dip.rs index b9ac581d0a..40eb0bf100 100644 --- a/dip-template/runtimes/dip-consumer/src/dip.rs +++ b/dip-template/runtimes/dip-consumer/src/dip.rs @@ -55,6 +55,7 @@ impl pallet_dip_consumer::Config for Runtime { type Identifier = DidIdentifier; type IdentityProof = >::Proof; type LocalIdentityInfo = u128; + type ProofVerificationError = >::Error; type ProofVerifier = ProofVerifier; type RuntimeCall = RuntimeCall; type RuntimeOrigin = RuntimeOrigin; @@ -106,20 +107,26 @@ fn single_key_relationship<'a>( }) } +pub enum DipCallFilterError { + BadOrigin, + WrongVerificationRelationship, +} + pub struct DipCallFilter; impl DipCallOriginFilter for DipCallFilter { - type Error = (); + type Error = DipCallFilterError; type OriginInfo = (DidVerificationKey, DidVerificationKeyRelationship); type Success = (); // Accepts only a DipOrigin for the DidLookup pallet calls. fn check_call_origin_info(call: &RuntimeCall, info: &Self::OriginInfo) -> Result { - let key_relationship = single_key_relationship([call].into_iter())?; + let key_relationship = + single_key_relationship([call].into_iter()).map_err(|_| DipCallFilterError::BadOrigin)?; if info.1 == key_relationship { Ok(()) } else { - Err(()) + Err(DipCallFilterError::WrongVerificationRelationship) } } } diff --git a/dip-template/runtimes/dip-provider/src/dip.rs b/dip-template/runtimes/dip-provider/src/dip.rs index 108bcbd7f2..bd86831553 100644 --- a/dip-template/runtimes/dip-provider/src/dip.rs +++ b/dip-template/runtimes/dip-provider/src/dip.rs @@ -16,17 +16,42 @@ // If you feel like getting in touch with us, you can do so at info@botlabs.org -use did::{DidRawOrigin, EnsureDidOrigin}; -use runtime_common::dip::{did::LinkedDidInfoProviderOf, merkle::DidMerkleRootGenerator}; +use did::{DidRawOrigin, EnsureDidOrigin, KeyIdOf}; +use pallet_did_lookup::linkable_account::LinkableAccountId; +use pallet_dip_provider::traits::IdentityProvider; +use parity_scale_codec::{Decode, Encode}; +use runtime_common::dip::{ + did::LinkedDidInfoProviderOf, + merkle::{DidMerkleProofError, DidMerkleRootGenerator}, +}; +use scale_info::TypeInfo; +use sp_std::vec::Vec; use crate::{AccountId, DidIdentifier, Hash, Runtime, RuntimeEvent}; +#[derive(Encode, Decode, TypeInfo)] +pub struct RuntimeApiDipProofRequest { + pub(crate) identifier: DidIdentifier, + pub(crate) keys: Vec>, + pub(crate) accounts: Vec, + pub(crate) should_include_web3_name: bool, +} + +#[derive(Encode, Decode, TypeInfo)] +pub enum RuntimeApiDipProofError { + IdentityNotFound, + IdentityProviderError( as IdentityProvider>::Error), + MerkleProofError(DidMerkleProofError), +} + impl pallet_dip_provider::Config for Runtime { type CommitOriginCheck = EnsureDidOrigin; type CommitOrigin = DidRawOrigin; type Identifier = DidIdentifier; type IdentityCommitment = Hash; type IdentityCommitmentGenerator = DidMerkleRootGenerator; + type IdentityCommitmentGeneratorError = DidMerkleProofError; type IdentityProvider = LinkedDidInfoProviderOf; + type IdentityProviderError = as IdentityProvider>::Error; type RuntimeEvent = RuntimeEvent; } diff --git a/dip-template/runtimes/dip-provider/src/lib.rs b/dip-template/runtimes/dip-provider/src/lib.rs index 784a881bf3..578fb1939b 100644 --- a/dip-template/runtimes/dip-provider/src/lib.rs +++ b/dip-template/runtimes/dip-provider/src/lib.rs @@ -22,7 +22,6 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); -use pallet_did_lookup::linkable_account::LinkableAccountId; use pallet_web3_names::web3_name::AsciiWeb3Name; use parity_scale_codec::{Decode, Encode}; use scale_info::TypeInfo; @@ -34,7 +33,7 @@ use cumulus_pallet_parachain_system::{ }; use cumulus_primitives_core::CollationInfo; use cumulus_primitives_timestamp::InherentDataProvider; -use did::{DidRawOrigin, EnsureDidOrigin, KeyIdOf}; +use did::{DidRawOrigin, EnsureDidOrigin}; use frame_support::{ construct_runtime, dispatch::DispatchClass, @@ -425,14 +424,6 @@ impl pallet_web3_names::Config for Runtime { type WeightInfo = (); } -#[derive(Encode, Decode, TypeInfo)] -pub struct DipProofRequest { - identifier: DidIdentifier, - keys: Vec>, - accounts: Vec, - should_include_web3_name: bool, -} - impl_runtime_apis! { impl sp_consensus_aura::AuraApi for Runtime { fn slot_duration() -> SlotDuration { @@ -578,13 +569,14 @@ impl_runtime_apis! { } // TODO: Support generating different versions of the proof, based on the provided parameter - impl kilt_runtime_api_dip_provider::DipProvider>, ()> for Runtime { - fn generate_proof(request: DipProofRequest) -> Result>, ()> { - let Some(linked_did_info) = ::IdentityProvider::retrieve(&request.identifier)? else { return Err(()) }; - if linked_did_info.a.is_none() { - return Err(()); - } - DidMerkleRootGenerator::::generate_proof(&linked_did_info, request.keys.iter(), request.should_include_web3_name, request.accounts.iter()) + impl kilt_runtime_api_dip_provider::DipProvider>, RuntimeApiDipProofError> for Runtime { + fn generate_proof(request: RuntimeApiDipProofRequest) -> Result>, RuntimeApiDipProofError> { + let linked_did_info = match ::IdentityProvider::retrieve(&request.identifier) { + Ok(Some(linked_did_info)) => Ok(linked_did_info), + Ok(None) => Err(RuntimeApiDipProofError::IdentityNotFound), + Err(e) => Err(RuntimeApiDipProofError::IdentityProviderError(e)) + }?; + DidMerkleRootGenerator::::generate_proof(&linked_did_info, request.keys.iter(), request.should_include_web3_name, request.accounts.iter()).map_err(RuntimeApiDipProofError::MerkleProofError) } } } diff --git a/nodes/standalone/src/command.rs b/nodes/standalone/src/command.rs index 1c813ecc08..378cc50fec 100644 --- a/nodes/standalone/src/command.rs +++ b/nodes/standalone/src/command.rs @@ -16,12 +16,12 @@ // If you feel like getting in touch with us, you can do so at info@botlabs.org -use crate::service::ExecutorDispatch; use crate::{ benchmarking::{inherent_benchmark_data, RemarkBuilder, TransferKeepAliveBuilder}, chain_spec, cli::{Cli, Subcommand}, service, + service::ExecutorDispatch, }; use frame_benchmarking_cli::{BenchmarkCmd, ExtrinsicFactory, SUBSTRATE_REFERENCE_HARDWARE}; diff --git a/nodes/standalone/src/rpc.rs b/nodes/standalone/src/rpc.rs index 6a164d9ce0..44baefbe65 100644 --- a/nodes/standalone/src/rpc.rs +++ b/nodes/standalone/src/rpc.rs @@ -72,7 +72,8 @@ where // Extend this RPC with a custom API by using the following syntax. // `YourRpcStruct` should have a reference to a client, which is needed // to call into the runtime. - // `module.merge(YourRpcTrait::into_rpc(YourRpcStruct::new(ReferenceToClient, ...)))?;` + // `module.merge(YourRpcTrait::into_rpc(YourRpcStruct::new(ReferenceToClient, + // ...)))?;` Ok(module) } diff --git a/pallets/attestation/src/benchmarking.rs b/pallets/attestation/src/benchmarking.rs index 930c5f89c8..821f005a53 100644 --- a/pallets/attestation/src/benchmarking.rs +++ b/pallets/attestation/src/benchmarking.rs @@ -18,8 +18,7 @@ use frame_benchmarking::{account, benchmarks}; use frame_support::traits::{fungible::Mutate, Get}; -use frame_system::pallet_prelude::BlockNumberFor; -use frame_system::RawOrigin; +use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use sp_runtime::traits::Hash; use ctype::CtypeEntryOf; diff --git a/pallets/delegation/src/benchmarking.rs b/pallets/delegation/src/benchmarking.rs index 19a72acb6a..f5c8902533 100644 --- a/pallets/delegation/src/benchmarking.rs +++ b/pallets/delegation/src/benchmarking.rs @@ -27,8 +27,7 @@ use frame_support::{ Get, }, }; -use frame_system::pallet_prelude::BlockNumberFor; -use frame_system::RawOrigin; +use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use parity_scale_codec::Encode; use sp_core::{offchain::KeyTypeId, sr25519}; use sp_io::crypto::sr25519_generate; diff --git a/pallets/did/src/benchmarking.rs b/pallets/did/src/benchmarking.rs index a1fa993728..c11f98a8d6 100644 --- a/pallets/did/src/benchmarking.rs +++ b/pallets/did/src/benchmarking.rs @@ -22,8 +22,7 @@ use frame_support::{ assert_ok, traits::fungible::{Inspect, Mutate, MutateHold}, }; -use frame_system::pallet_prelude::BlockNumberFor; -use frame_system::RawOrigin; +use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use parity_scale_codec::Encode; use sp_core::{crypto::KeyTypeId, ecdsa, ed25519, sr25519}; use sp_io::crypto::{ecdsa_generate, ecdsa_sign, ed25519_generate, ed25519_sign, sr25519_generate, sr25519_sign}; diff --git a/pallets/pallet-did-lookup/src/benchmarking.rs b/pallets/pallet-did-lookup/src/benchmarking.rs index 688c0215d2..292f3ad7a6 100644 --- a/pallets/pallet-did-lookup/src/benchmarking.rs +++ b/pallets/pallet-did-lookup/src/benchmarking.rs @@ -27,8 +27,7 @@ use frame_support::{ Get, }, }; -use frame_system::pallet_prelude::BlockNumberFor; -use frame_system::RawOrigin; +use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use sha3::{Digest, Keccak256}; use sp_io::crypto::{ecdsa_generate, ed25519_generate, sr25519_generate}; use sp_runtime::{ diff --git a/pallets/pallet-dip-consumer/src/lib.rs b/pallets/pallet-dip-consumer/src/lib.rs index cb0e49f801..c513bbc130 100644 --- a/pallets/pallet-dip-consumer/src/lib.rs +++ b/pallets/pallet-dip-consumer/src/lib.rs @@ -66,11 +66,13 @@ pub mod pallet { type IdentityProof: Parameter; /// The details stored in this pallet associated with any given subject. type LocalIdentityInfo: FullCodec + TypeInfo + MaxEncodedLen; + type ProofVerificationError: Into; /// The logic of the proof verifier, called upon each execution of the /// `dispatch_as` extrinsic. type ProofVerifier: IdentityProofVerifier< ::RuntimeCall, Self::Identifier, + Error = Self::ProofVerificationError, Proof = Self::IdentityProof, IdentityDetails = Self::LocalIdentityInfo, Submitter = ::AccountId, @@ -90,9 +92,9 @@ pub mod pallet { /// An identity with the provided identifier could not be found. IdentityNotFound, /// The identity proof provided could not be successfully verified. - InvalidProof, - /// The specified call could not be dispatched. - Dispatch, + InvalidProof(u16), + /// The specified call is filtered by the DIP call origin filter. + Filtered, } /// The origin this pallet creates after a user has provided a valid @@ -116,8 +118,7 @@ pub mod pallet { // TODO: Make origin check configurable, and require that it at least returns // the submitter's account. let submitter = ensure_signed(origin)?; - // TODO: Proper error handling - ensure!(T::DipCallOriginFilter::contains(&*call), Error::::Dispatch); + ensure!(T::DipCallOriginFilter::contains(&*call), Error::::Filtered); let mut identity_entry = IdentityEntries::::get(&identifier); let proof_verification_result = T::ProofVerifier::verify_proof_for_call_against_details( &*call, @@ -125,20 +126,16 @@ pub mod pallet { &submitter, &mut identity_entry, proof, - ); - // Write the identity info to storage after it has optionally been updated by - // the `ProofVerifier`, regardless of whether the proof has been verified or - // not. + ) + .map_err(|e| Error::::InvalidProof(e.into()))?; IdentityEntries::::mutate(&identifier, |entry| *entry = identity_entry); - // Unwrap the result if `ok`. - let proof_verification_result = proof_verification_result.map_err(|_| Error::::InvalidProof)?; let did_origin = DipOrigin { identifier, account_address: submitter, details: proof_verification_result, }; // TODO: Use dispatch info for weight calculation - let _ = call.dispatch(did_origin.into()).map_err(|_| Error::::Dispatch)?; + let _ = call.dispatch(did_origin.into()).map_err(|e| e.error)?; Ok(()) } } diff --git a/pallets/pallet-dip-provider/src/lib.rs b/pallets/pallet-dip-provider/src/lib.rs index 5171084147..ad17b78e4e 100644 --- a/pallets/pallet-dip-provider/src/lib.rs +++ b/pallets/pallet-dip-provider/src/lib.rs @@ -48,9 +48,12 @@ pub mod pallet { type IdentityCommitmentGenerator: IdentityCommitmentGenerator< Self::Identifier, IdentityOf, + Error = Self::IdentityCommitmentGeneratorError, Output = Self::IdentityCommitment, >; - type IdentityProvider: IdentityProvider; + type IdentityCommitmentGeneratorError: Into; + type IdentityProvider: IdentityProvider; + type IdentityProviderError: Into; type RuntimeEvent: From> + IsType<::RuntimeEvent>; } @@ -77,8 +80,8 @@ pub mod pallet { #[pallet::error] pub enum Error { - IdentityNotFound, - IdentityCommitmentGeneration, + IdentityProvider(u16), + IdentityCommitmentGenerator(u16), } #[pallet::call] @@ -94,9 +97,9 @@ pub mod pallet { let identity_commitment: Option = match T::IdentityProvider::retrieve(&identifier) { Ok(Some(identity)) => T::IdentityCommitmentGenerator::generate_commitment(&identifier, &identity) .map(Some) - .map_err(|_| Error::::IdentityCommitmentGeneration), + .map_err(|error| Error::::IdentityCommitmentGenerator(error.into())), Ok(None) => Ok(None), - Err(_) => Err(Error::::IdentityNotFound), + Err(error) => Err(Error::::IdentityProvider(error.into())), }?; if let Some(commitment) = identity_commitment { @@ -111,5 +114,7 @@ pub mod pallet { Ok(()) } + // TODO: Add extrinsic to remove commitment without requiring the identity to be + // deleted. } } diff --git a/pallets/pallet-migration/src/lib.rs b/pallets/pallet-migration/src/lib.rs index abc074ad65..90ac062660 100644 --- a/pallets/pallet-migration/src/lib.rs +++ b/pallets/pallet-migration/src/lib.rs @@ -39,8 +39,7 @@ pub mod pallet { traits::{fungible::Inspect, Currency, ReservableCurrency}, }; use frame_system::pallet_prelude::*; - use sp_runtime::traits::Hash; - use sp_runtime::SaturatedConversion; + use sp_runtime::{traits::Hash, SaturatedConversion}; use attestation::{Attestations, ClaimHashOf}; use delegation::{DelegationNodeIdOf, DelegationNodes}; diff --git a/pallets/pallet-migration/src/test.rs b/pallets/pallet-migration/src/test.rs index 5cc1374f7e..7b70e8e6a1 100644 --- a/pallets/pallet-migration/src/test.rs +++ b/pallets/pallet-migration/src/test.rs @@ -292,7 +292,8 @@ fn check_attempt_to_migrate_already_migrated_keys() { requested_migrations.clone() )); - // Since the keys are already migrated, a second attempt should have not affect to the storage. + // Since the keys are already migrated, a second attempt should have not affect + // to the storage. assert_storage_noop!( Migration::update_balance(RuntimeOrigin::signed(ACCOUNT_00), requested_migrations) .expect("Update balance should not panic") diff --git a/pallets/pallet-web3-names/src/benchmarking.rs b/pallets/pallet-web3-names/src/benchmarking.rs index 3954a3d637..608775860a 100644 --- a/pallets/pallet-web3-names/src/benchmarking.rs +++ b/pallets/pallet-web3-names/src/benchmarking.rs @@ -27,8 +27,7 @@ use frame_support::{ }, BoundedVec, }; -use frame_system::pallet_prelude::BlockNumberFor; -use frame_system::RawOrigin; +use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use sp_runtime::app_crypto::sr25519; use kilt_support::{traits::GenerateBenchmarkOrigin, Deposit}; diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index c36def60c0..095bed7e4e 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -143,7 +143,6 @@ pub mod pallet { pub use crate::inflation::{InflationInfo, RewardRate, StakingInfo}; use core::cmp::Ordering; - use frame_support::traits::BuildGenesisConfig; use frame_support::{ pallet_prelude::*, storage::bounded_btree_map::BoundedBTreeMap, @@ -153,7 +152,7 @@ pub mod pallet { fungible::{Inspect, MutateFreeze, Unbalanced}, Fortitude, Precision, Preservation, }, - EstimateNextSessionRotation, Get, OnUnbalanced, StorageVersion, + BuildGenesisConfig, EstimateNextSessionRotation, Get, OnUnbalanced, StorageVersion, }, BoundedVec, }; @@ -2513,8 +2512,8 @@ pub mod pallet { /// 2. In hook new_session: Read the current top n candidates from the /// TopCandidates and assign this set to author blocks for the next /// session. - /// 3. AuRa queries the authorities from the session pallet for - /// this session and picks authors on round-robin-basis from list of + /// 3. AuRa queries the authorities from the session pallet for this + /// session and picks authors on round-robin-basis from list of /// authorities. fn new_session(new_index: SessionIndex) -> Option> { log::debug!( diff --git a/runtimes/common/src/dip/did.rs b/runtimes/common/src/dip/did.rs index 1bd6194e17..f4255c438b 100644 --- a/runtimes/common/src/dip/did.rs +++ b/runtimes/common/src/dip/did.rs @@ -24,16 +24,32 @@ use kilt_dip_support::{ }; use pallet_did_lookup::linkable_account::LinkableAccountId; use pallet_dip_provider::traits::IdentityProvider; +use parity_scale_codec::{Decode, Encode}; +use scale_info::TypeInfo; use sp_std::{marker::PhantomData, vec::Vec}; +#[derive(Encode, Decode, TypeInfo)] +pub enum DidIdentityProviderError { + DidNotFound, + Internal, +} + +impl From for u16 { + fn from(value: DidIdentityProviderError) -> Self { + match value { + DidIdentityProviderError::DidNotFound => 0, + DidIdentityProviderError::Internal => u16::MAX, + } + } +} + pub struct DidIdentityProvider(PhantomData); impl IdentityProvider for DidIdentityProvider where T: did::Config, { - // TODO: Proper error handling - type Error = (); + type Error = DidIdentityProviderError; type Success = DidDetails; fn retrieve(identifier: &T::DidIdentifier) -> Result, Self::Error> { @@ -43,7 +59,7 @@ where ) { (Some(details), _) => Ok(Some(details)), (_, Some(_)) => Ok(None), - _ => Err(()), + _ => Err(DidIdentityProviderError::DidNotFound), } } } @@ -56,8 +72,7 @@ impl IdentityProvider for DidWeb3NameProvider where T: pallet_web3_names::Config, { - // TODO: Proper error handling - type Error = (); + type Error = DidIdentityProviderError; type Success = Web3OwnershipOf; fn retrieve(identifier: &T::Web3NameOwner) -> Result, Self::Error> { @@ -65,7 +80,11 @@ where return Ok(None); }; let Some(details) = pallet_web3_names::Pallet::::owner(&web3_name) else { - return Err(()); + log::error!( + "Inconsistent reverse map pallet_web3_names::owner(web3_name). Cannot find owner for web3name {:#?}", + web3_name + ); + return Err(DidIdentityProviderError::Internal); }; Ok(Some(Web3OwnershipOf:: { web3_name, @@ -80,8 +99,7 @@ impl IdentityProvider for DidLinkedAccountsProvider where T: pallet_did_lookup::Config, { - // TODO: Proper error handling - type Error = (); + type Error = DidIdentityProviderError; type Success = Vec; fn retrieve(identifier: &T::DidIdentifier) -> Result, Self::Error> { diff --git a/runtimes/common/src/dip/merkle.rs b/runtimes/common/src/dip/merkle.rs index 3f485f3be2..0059d267ae 100644 --- a/runtimes/common/src/dip/merkle.rs +++ b/runtimes/common/src/dip/merkle.rs @@ -15,7 +15,6 @@ // along with this program. If not, see . // If you feel like getting in touch with us, you can do so at info@botlabs.org - use did::{DidVerificationKeyRelationship, KeyIdOf}; use frame_support::RuntimeDebug; use frame_system::pallet_prelude::BlockNumberFor; @@ -49,6 +48,27 @@ pub struct CompleteMerkleProof { pub proof: Proof, } +#[derive(Clone, RuntimeDebug, Encode, Decode, TypeInfo, PartialEq)] +pub enum DidMerkleProofError { + DidNotFound, + KeyNotFound, + LinkedAccountNotFound, + Web3NameNotFound, + Internal, +} + +impl From for u16 { + fn from(value: DidMerkleProofError) -> Self { + match value { + DidMerkleProofError::DidNotFound => 0, + DidMerkleProofError::KeyNotFound => 1, + DidMerkleProofError::LinkedAccountNotFound => 2, + DidMerkleProofError::Web3NameNotFound => 3, + DidMerkleProofError::Internal => u16::MAX, + } + } +} + pub struct DidMerkleRootGenerator(PhantomData); type ProofLeafOf = RevealedDidMerkleProofLeaf< @@ -74,20 +94,25 @@ where // A valid proof will contain a leaf with the key details for each reference // leaf, with multiple reference leaves potentially referring to the same // details leaf, as we already do with out `DidDetails` type. - fn calculate_root_with_db(identity: &LinkedDidInfoOf, db: &mut MemoryDB) -> Result { + fn calculate_root_with_db( + identity: &LinkedDidInfoOf, + db: &mut MemoryDB, + ) -> Result { // Fails if the DID details do not exist. let (Some(did_details), web3_name, linked_accounts) = (&identity.a, &identity.b, &identity.c) else { - return Err(()); + return Err(DidMerkleProofError::DidNotFound); }; let mut trie = TrieHash::>::default(); let mut trie_builder = TrieDBMutBuilder::>::new(db, &mut trie).build(); // Authentication key - // TODO: No panic let auth_key_details = did_details .public_keys .get(&did_details.authentication_key) - .expect("Authentication key should be part of the public keys."); + .ok_or_else(|| { + log::error!("Authentication key should be part of the public keys."); + DidMerkleProofError::Internal + })?; let auth_leaf = ProofLeafOf::::DidKey( DidKeyMerkleKey( did_details.authentication_key, @@ -97,51 +122,75 @@ where ); trie_builder .insert(auth_leaf.encoded_key().as_slice(), auth_leaf.encoded_value().as_slice()) - .map_err(|_| ())?; + .map_err(|_| { + log::error!( + "Failed to insert authentication key in the trie builder. Authentication leaf: {:#?}", + auth_leaf + ); + DidMerkleProofError::Internal + })?; // Attestation key, if present if let Some(att_key_id) = did_details.attestation_key { - let att_key_details = did_details - .public_keys - .get(&att_key_id) - .expect("Attestation key should be part of the public keys."); + let att_key_details = did_details.public_keys.get(&att_key_id).ok_or_else(|| { + log::error!("Attestation key should be part of the public keys."); + DidMerkleProofError::Internal + })?; let att_leaf = ProofLeafOf::::DidKey( (att_key_id, DidVerificationKeyRelationship::AssertionMethod.into()).into(), att_key_details.clone().into(), ); trie_builder .insert(att_leaf.encoded_key().as_slice(), att_leaf.encoded_value().as_slice()) - .map_err(|_| ())?; + .map_err(|_| { + log::error!( + "Failed to insert attestation key in the trie builder. Attestation leaf: {:#?}", + att_leaf + ); + DidMerkleProofError::Internal + })?; }; // Delegation key, if present if let Some(del_key_id) = did_details.delegation_key { - let del_key_details = did_details - .public_keys - .get(&del_key_id) - .expect("Delegation key should be part of the public keys."); + let del_key_details = did_details.public_keys.get(&del_key_id).ok_or_else(|| { + log::error!("Delegation key should be part of the public keys."); + DidMerkleProofError::Internal + })?; let del_leaf = ProofLeafOf::::DidKey( (del_key_id, DidVerificationKeyRelationship::CapabilityDelegation.into()).into(), del_key_details.clone().into(), ); trie_builder .insert(del_leaf.encoded_key().as_slice(), del_leaf.encoded_value().as_slice()) - .map_err(|_| ())?; + .map_err(|_| { + log::error!( + "Failed to insert delegation key in the trie builder. Delegation leaf: {:#?}", + del_leaf + ); + DidMerkleProofError::Internal + })?; }; // Key agreement keys did_details .key_agreement_keys .iter() - .try_for_each(|id| -> Result<(), ()> { - let key_agreement_details = did_details - .public_keys - .get(id) - .expect("Key agreement key should be part of the public keys."); + .try_for_each(|id| -> Result<(), DidMerkleProofError> { + let key_agreement_details = did_details.public_keys.get(id).ok_or_else(|| { + log::error!("Key agreement key should be part of the public keys."); + DidMerkleProofError::Internal + })?; let enc_leaf = ProofLeafOf::::DidKey( (*id, DidKeyRelationship::Encryption).into(), key_agreement_details.clone().into(), ); trie_builder .insert(enc_leaf.encoded_key().as_slice(), enc_leaf.encoded_value().as_slice()) - .map_err(|_| ())?; + .map_err(|_| { + log::error!( + "Failed to insert key agreement key in the trie builder. Key agreement leaf: {:#?}", + enc_leaf + ); + DidMerkleProofError::Internal + })?; Ok(()) })?; @@ -149,14 +198,20 @@ where if let Some(linked_accounts) = linked_accounts { linked_accounts .iter() - .try_for_each(|linked_account| -> Result<(), ()> { + .try_for_each(|linked_account| -> Result<(), DidMerkleProofError> { let linked_account_leaf = ProofLeafOf::::LinkedAccount(linked_account.clone().into(), ().into()); trie_builder .insert( linked_account_leaf.encoded_key().as_slice(), linked_account_leaf.encoded_value().as_slice(), ) - .map_err(|_| ())?; + .map_err(|_| { + log::error!( + "Failed to insert linked account in the trie builder. Linked account leaf: {:#?}", + linked_account_leaf + ); + DidMerkleProofError::Internal + })?; Ok(()) })?; } @@ -172,14 +227,19 @@ where web3_name_leaf.encoded_key().as_slice(), web3_name_leaf.encoded_value().as_slice(), ) - .map_err(|_| ())?; + .map_err(|_| { + log::error!( + "Failed to insert web3name in the trie builder. Web3name leaf: {:#?}", + web3_name_leaf + ); + DidMerkleProofError::Internal + })?; } trie_builder.commit(); Ok(trie_builder.root().to_owned()) } - // TODO: Better error handling // Only used for testing and as part of the features exposed by the runtime API // of the provider. Given the provided `DidDetails` and a list of key IDs, it // generates a merkle proof which only reveals the details of the provided key @@ -190,22 +250,25 @@ where key_ids: K, should_include_web3_name: bool, account_ids: A, - ) -> Result>, ()> + ) -> Result>, DidMerkleProofError> where K: Iterator>, A: Iterator, { // Fails if the DID details do not exist. let (Some(did_details), linked_web3_name, linked_accounts) = (&identity.a, &identity.b, &identity.c) else { - return Err(()); + return Err(DidMerkleProofError::DidNotFound); }; let mut db = MemoryDB::default(); let root = Self::calculate_root_with_db(identity, &mut db)?; let mut leaves = key_ids - .map(|key_id| -> Result, ()> { - let key_details = did_details.public_keys.get(key_id).ok_or(())?; + .map(|key_id| -> Result<_, DidMerkleProofError> { + let key_details = did_details + .public_keys + .get(key_id) + .ok_or(DidMerkleProofError::KeyNotFound)?; // Create the merkle leaf key depending on the relationship of the key to the // DID document. let did_key_merkle_key: DidKeyMerkleKey> = if *key_id == did_details.authentication_key { @@ -217,16 +280,19 @@ where } else if did_details.key_agreement_keys.contains(key_id) { Ok((*key_id, DidKeyRelationship::Encryption).into()) } else { - Err(()) + log::error!("Unknown key ID {:#?} retrieved from DID details.", key_id); + Err(DidMerkleProofError::Internal) }?; Ok(RevealedDidMerkleProofLeaf::DidKey( did_key_merkle_key, key_details.clone().into(), )) }) - .chain(account_ids.map(|account_id| -> Result, ()> { + .chain(account_ids.map(|account_id| -> Result<_, DidMerkleProofError> { let Some(linked_accounts) = linked_accounts else { - return Err(()); + // Directly LinkedAccountNotFound since there's no linked accounts to check + // against. + return Err(DidMerkleProofError::LinkedAccountNotFound); }; if linked_accounts.contains(account_id) { Ok(RevealedDidMerkleProofLeaf::LinkedAccount( @@ -234,7 +300,7 @@ where ().into(), )) } else { - Err(()) + Err(DidMerkleProofError::LinkedAccountNotFound) } })) .collect::, _>>()?; @@ -249,13 +315,19 @@ where Ok(()) } // ...else if web3name should be included and it DOES NOT exist... - (true, None) => Err(()), - // ...else if web3name should NOT be included. + (true, None) => Err(DidMerkleProofError::Web3NameNotFound), + // ...else (if web3name should NOT be included). (false, _) => Ok(()), }?; let encoded_keys: Vec> = leaves.iter().map(|l| l.encoded_key()).collect(); - let proof = generate_trie_proof::, _, _, _>(&db, root, &encoded_keys).map_err(|_| ())?; + let proof = generate_trie_proof::, _, _, _>(&db, root, &encoded_keys).map_err(|_| { + log::error!( + "Failed to generate a merkle proof for the encoded keys: {:#?}", + encoded_keys + ); + DidMerkleProofError::Internal + })?; Ok(CompleteMerkleProof { root, proof: DidMerkleProofOf:: { @@ -270,8 +342,7 @@ impl IdentityCommitmentGenerator> for DidMe where T: did::Config + pallet_did_lookup::Config + pallet_web3_names::Config, { - // TODO: Proper error handling - type Error = (); + type Error = DidMerkleProofError; type Output = T::Hash; fn generate_commitment(_identifier: &DidIdentifier, identity: &LinkedDidInfoOf) -> Result {