From 0461b0d4d5c6d78f166ba02f3dc527b3b87b8e3b Mon Sep 17 00:00:00 2001 From: weichweich Date: Wed, 6 Dec 2023 15:31:29 +0100 Subject: [PATCH 01/10] chore: remove outdated clippy-allow --- crates/kilt-dip-support/src/did.rs | 1 - crates/kilt-dip-support/src/state_proofs.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/crates/kilt-dip-support/src/did.rs b/crates/kilt-dip-support/src/did.rs index dfdce46dba..b8ad51a551 100644 --- a/crates/kilt-dip-support/src/did.rs +++ b/crates/kilt-dip-support/src/did.rs @@ -235,7 +235,6 @@ impl< } #[cfg(feature = "runtime-benchmarks")] - #[allow(clippy::result_unit_err)] pub(crate) fn verify_did_signature_for_call( call: &Call, submitter: &Submitter, diff --git a/crates/kilt-dip-support/src/state_proofs.rs b/crates/kilt-dip-support/src/state_proofs.rs index 58619eb6bb..8b61bbe942 100644 --- a/crates/kilt-dip-support/src/state_proofs.rs +++ b/crates/kilt-dip-support/src/state_proofs.rs @@ -370,7 +370,6 @@ pub(super) mod parachain { /// Given a parachain state root, verify a state proof for the /// commitment of a given subject identifier. #[cfg(not(feature = "runtime-benchmarks"))] - #[allow(clippy::result_unit_err)] pub fn verify_proof_for_identifier( identifier: &ParaInfo::Identifier, state_root: OutputOf, From ed2d9d695bc45009df11a857c52d75dcd80d9a96 Mon Sep 17 00:00:00 2001 From: weichweich Date: Wed, 6 Dec 2023 15:37:13 +0100 Subject: [PATCH 02/10] merge impl blocks --- crates/kilt-dip-support/src/state_proofs.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/crates/kilt-dip-support/src/state_proofs.rs b/crates/kilt-dip-support/src/state_proofs.rs index 8b61bbe942..017470aa6e 100644 --- a/crates/kilt-dip-support/src/state_proofs.rs +++ b/crates/kilt-dip-support/src/state_proofs.rs @@ -187,17 +187,7 @@ pub(super) mod relay_chain { }); Ok(header) } - } - // Relies on the `RelayChainState::state_root_for_block` to retrieve the state - // root for the given block. - impl ParachainHeadProofVerifier - where - RelayChainState: RelayChainStateInfo, - OutputOf: Ord, - RelayChainState::BlockNumber: Copy + Into + TryFrom + HasCompact, - RelayChainState::Key: AsRef<[u8]>, - { /// Given a relaychain state root provided by the `RelayChainState` /// generic type, verify a state proof for the parachain with the /// provided ID. @@ -206,7 +196,10 @@ pub(super) mod relay_chain { para_id: &RelayChainState::ParaId, relay_height: &RelayChainState::BlockNumber, proof: impl IntoIterator>, - ) -> Result, ParachainHeadProofVerifierError> { + ) -> Result, ParachainHeadProofVerifierError> + where + RelayChainState: RelayChainStateInfo, + { 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) @@ -217,7 +210,10 @@ pub(super) mod relay_chain { para_id: &RelayChainState::ParaId, relay_height: &RelayChainState::BlockNumber, proof: impl IntoIterator>, - ) -> Result, ParachainHeadProofVerifierError> { + ) -> Result, ParachainHeadProofVerifierError> + where + RelayChainState: RelayChainStateInfo, + { let relay_state_root = RelayChainState::state_root_for_block(relay_height).unwrap_or_default(); Self::verify_proof_for_parachain_with_root(para_id, &relay_state_root, proof) } From 57af971e452262b5b087ee6f0f769eb4cf99eff9 Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 7 Dec 2023 14:40:53 +0100 Subject: [PATCH 03/10] refactor: don't duplicate code (verify_dip_merkle_proof) --- Cargo.lock | 1 + crates/kilt-dip-support/Cargo.toml | 1 + crates/kilt-dip-support/src/merkle.rs | 106 +++++++------------------- 3 files changed, 28 insertions(+), 80 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 741ea0e370..0d0eeaafa8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4558,6 +4558,7 @@ dependencies = [ name = "kilt-dip-support" version = "1.12.0-dev" dependencies = [ + "cfg-if", "cumulus-pallet-parachain-system", "cumulus-primitives-core", "did", diff --git a/crates/kilt-dip-support/Cargo.toml b/crates/kilt-dip-support/Cargo.toml index bbebdfa4f2..30262359b2 100644 --- a/crates/kilt-dip-support/Cargo.toml +++ b/crates/kilt-dip-support/Cargo.toml @@ -14,6 +14,7 @@ version.workspace = true # External dependencies hash-db.workspace = true log.workspace = true +cfg-if = "1.0" # Internal dependencies did.workspace = true diff --git a/crates/kilt-dip-support/src/merkle.rs b/crates/kilt-dip-support/src/merkle.rs index 3fecc5668e..75798e2588 100644 --- a/crates/kilt-dip-support/src/merkle.rs +++ b/crates/kilt-dip-support/src/merkle.rs @@ -341,7 +341,6 @@ impl< LinkedAccountId: Encode + Clone, Web3Name: Encode + Clone, { - #[cfg(not(feature = "runtime-benchmarks"))] pub(crate) fn verify_dip_merkle_proof( identity_commitment: &Hasher::Out, proof: DidMerkleProof< @@ -368,8 +367,14 @@ impl< .iter() .map(|leaf| (leaf.encoded_key(), Some(leaf.encoded_value()))) .collect::, Option>)>>(); - verify_trie_proof::, _, _, _>(identity_commitment, &proof.blinded, &proof_leaves) - .map_err(|_| DidMerkleProofVerifierError::InvalidMerkleProof)?; + let res = verify_trie_proof::, _, _, _>(identity_commitment, &proof.blinded, &proof_leaves); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + drop(res); + } else { + res.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. @@ -386,13 +391,20 @@ impl< ), |(mut keys, web3_name, mut linked_accounts), leaf| match leaf { RevealedDidMerkleProofLeaf::DidKey(key_id, key_value) => { - keys.try_push(RevealedDidKey { + let res = keys.try_push(RevealedDidKey { // TODO: Avoid cloning if possible id: key_id.0.clone(), relationship: key_id.1, details: key_value.0.clone(), - }) - .map_err(|_| DidMerkleProofVerifierError::TooManyRevealedKeys)?; + }); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + drop(res); + } else { + res.map_err(|_| DidMerkleProofVerifierError::TooManyRevealedKeys)?; + } + } + Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) } // TODO: Avoid cloning if possible @@ -405,81 +417,15 @@ impl< linked_accounts, )), RevealedDidMerkleProofLeaf::LinkedAccount(account_id, _) => { - linked_accounts - .try_push(account_id.0.clone()) - .map_err(|_| DidMerkleProofVerifierError::TooManyRevealedAccounts)?; - Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) - } - }, - )?; + let res = linked_accounts.try_push(account_id.0.clone()); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + drop(res); + } else { + res.map_err(|_| DidMerkleProofVerifierError::TooManyRevealedAccounts)?; + } + } - Ok(RevealedDidMerkleProofLeaves { - did_keys, - web3_name, - linked_accounts, - }) - } - - #[cfg(feature = "runtime-benchmarks")] - #[allow(clippy::type_complexity)] - pub(crate) fn verify_dip_merkle_proof( - identity_commitment: &Hasher::Out, - proof: DidMerkleProof< - crate::BoundedBlindedValue, - RevealedDidMerkleProofLeaf, - >, - ) -> Result< - RevealedDidMerkleProofLeaves< - KeyId, - AccountId, - BlockNumber, - Web3Name, - LinkedAccountId, - MAX_REVEALED_KEYS_COUNT, - MAX_REVEALED_ACCOUNTS_COUNT, - >, - DidMerkleProofVerifierError, - > { - let proof_leaves = proof - .revealed - .iter() - .map(|leaf| (leaf.encoded_key(), Some(leaf.encoded_value()))) - .collect::, Option>)>>(); - // Ignore result - let _ = verify_trie_proof::, _, _, _>(identity_commitment, &proof.blinded, &proof_leaves); - - #[allow(clippy::type_complexity)] - let (did_keys, web3_name, linked_accounts): ( - BoundedVec, ConstU32>, - Option>, - BoundedVec>, - ) = proof.revealed.iter().try_fold( - ( - BoundedVec::with_bounded_capacity(MAX_REVEALED_KEYS_COUNT.saturated_into()), - None, - BoundedVec::with_bounded_capacity(MAX_REVEALED_ACCOUNTS_COUNT.saturated_into()), - ), - |(mut keys, web3_name, mut linked_accounts), leaf| match leaf { - RevealedDidMerkleProofLeaf::DidKey(key_id, key_value) => { - // Ignore error, just discard anything in excess. - let _ = keys.try_push(RevealedDidKey { - id: key_id.0.clone(), - relationship: key_id.1, - details: key_value.0.clone(), - }); - Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) - } - RevealedDidMerkleProofLeaf::Web3Name(revealed_web3_name, details) => Ok(( - keys, - Some(RevealedWeb3Name { - web3_name: revealed_web3_name.0.clone(), - claimed_at: details.0.clone(), - }), - linked_accounts, - )), - RevealedDidMerkleProofLeaf::LinkedAccount(account_id, _) => { - // Ignore error, just discard anything in excess. - let _ = linked_accounts.try_push(account_id.0.clone()); Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) } }, From 8cdf225a9494d7e716e4a81a5a7aed80bd958442 Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 7 Dec 2023 14:41:27 +0100 Subject: [PATCH 04/10] perf: don't clone --- crates/kilt-dip-support/src/merkle.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/kilt-dip-support/src/merkle.rs b/crates/kilt-dip-support/src/merkle.rs index 75798e2588..30b39668bb 100644 --- a/crates/kilt-dip-support/src/merkle.rs +++ b/crates/kilt-dip-support/src/merkle.rs @@ -383,7 +383,7 @@ impl< BoundedVec, ConstU32>, Option>, BoundedVec>, - ) = proof.revealed.iter().try_fold( + ) = proof.revealed.into_iter().try_fold( ( BoundedVec::with_bounded_capacity(MAX_REVEALED_KEYS_COUNT.saturated_into()), None, @@ -392,10 +392,9 @@ impl< |(mut keys, web3_name, mut linked_accounts), leaf| match leaf { RevealedDidMerkleProofLeaf::DidKey(key_id, key_value) => { let res = keys.try_push(RevealedDidKey { - // TODO: Avoid cloning if possible - id: key_id.0.clone(), + id: key_id.0, relationship: key_id.1, - details: key_value.0.clone(), + details: key_value.0, }); cfg_if::cfg_if! { if #[cfg(feature = "runtime-benchmarks")] { @@ -407,17 +406,16 @@ impl< Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) } - // TODO: Avoid cloning if possible RevealedDidMerkleProofLeaf::Web3Name(revealed_web3_name, details) => Ok(( keys, Some(RevealedWeb3Name { - web3_name: revealed_web3_name.0.clone(), - claimed_at: details.0.clone(), + web3_name: revealed_web3_name.0, + claimed_at: details.0, }), linked_accounts, )), RevealedDidMerkleProofLeaf::LinkedAccount(account_id, _) => { - let res = linked_accounts.try_push(account_id.0.clone()); + let res = linked_accounts.try_push(account_id.0); cfg_if::cfg_if! { if #[cfg(feature = "runtime-benchmarks")] { drop(res); From f722e00eca783b232a347775585a58cba0f623ce Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 7 Dec 2023 14:57:05 +0100 Subject: [PATCH 05/10] refactor: don't match match is more complicated to read --- runtimes/common/src/dip/did.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/runtimes/common/src/dip/did.rs b/runtimes/common/src/dip/did.rs index 6490684fec..462e533c9b 100644 --- a/runtimes/common/src/dip/did.rs +++ b/runtimes/common/src/dip/did.rs @@ -84,14 +84,12 @@ where type Success = LinkedDidInfoOf; fn retrieve(identifier: &Runtime::Identifier) -> Result { - let did_details = match ( - did::Pallet::::get_did(identifier), - did::Pallet::::get_deleted_did(identifier), - ) { - (Some(details), _) => Ok(details), - (_, Some(_)) => Err(LinkedDidInfoProviderError::DidDeleted), - _ => Err(LinkedDidInfoProviderError::DidNotFound), - }?; + ensure!( + did::Pallet::::get_deleted_did(identifier).is_none(), + LinkedDidInfoProviderError::DidDeleted, + ); + let did_details = did::Pallet::::get_did(identifier).ok_or(LinkedDidInfoProviderError::DidNotFound)?; + let web3_name_details = if let Some(web3_name) = pallet_web3_names::Pallet::::names(identifier) { let Some(ownership) = pallet_web3_names::Pallet::::owner(&web3_name) else { log::error!( From 05b8121842568015acb59e88ff05820b0593dbb0 Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 7 Dec 2023 15:09:45 +0100 Subject: [PATCH 06/10] refactor: split into functions make the code more readable --- runtimes/common/src/dip/did.rs | 88 +++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 33 deletions(-) diff --git a/runtimes/common/src/dip/did.rs b/runtimes/common/src/dip/did.rs index 462e533c9b..fbbda7a3f1 100644 --- a/runtimes/common/src/dip/did.rs +++ b/runtimes/common/src/dip/did.rs @@ -90,40 +90,9 @@ where ); let did_details = did::Pallet::::get_did(identifier).ok_or(LinkedDidInfoProviderError::DidNotFound)?; - let web3_name_details = if let Some(web3_name) = pallet_web3_names::Pallet::::names(identifier) { - let Some(ownership) = pallet_web3_names::Pallet::::owner(&web3_name) else { - log::error!( - "Inconsistent reverse map pallet_web3_names::owner(web3_name). Cannot find owner for web3name {:#?}", - web3_name - ); - return Err(LinkedDidInfoProviderError::Internal); - }; - Ok(Some(Web3OwnershipOf:: { - web3_name, - claimed_at: ownership.claimed_at, - })) - } else { - Ok(None) - }?; - - // Check if the user has too many linked accounts. If they have more than - // [MAX_LINKED_ACCOUNTS], we throw an error. - let are_linked_accounts_within_limit = - pallet_did_lookup::ConnectedAccounts::::iter_key_prefix(identifier) - .nth(MAX_LINKED_ACCOUNTS.saturated_into()) - .is_none(); + let web3_name_details = retrieve_w3n::(identifier)?; - ensure!( - are_linked_accounts_within_limit, - LinkedDidInfoProviderError::TooManyLinkedAccounts - ); - - let linked_accounts = pallet_did_lookup::ConnectedAccounts::::iter_key_prefix(identifier) - .take(MAX_LINKED_ACCOUNTS.saturated_into()) - .collect::>() - .try_into() - // Should never happen since we checked above. - .map_err(|_| LinkedDidInfoProviderError::TooManyLinkedAccounts)?; + let linked_accounts = retrieve_linked_accounts::(identifier)?; Ok(LinkedDidInfoOf { did_details, @@ -133,6 +102,59 @@ where } } +fn retrieve_w3n( + identifier: &Runtime::Identifier, +) -> Result>, LinkedDidInfoProviderError> +where + Runtime: did::Config::Identifier> + + pallet_web3_names::Config::Identifier> + + pallet_dip_provider::Config, +{ + let Some(web3_name) = pallet_web3_names::Pallet::::names(identifier) else { + return Ok(None); + }; + + let ownership = pallet_web3_names::Pallet::::owner(&web3_name).ok_or_else(|| { + log::error!( + "Inconsistent reverse map pallet_web3_names::owner(web3_name). Cannot find owner for web3name {:#?}", + web3_name + ); + LinkedDidInfoProviderError::Internal + })?; + + Ok(Some(Web3OwnershipOf:: { + web3_name, + claimed_at: ownership.claimed_at, + })) +} + +fn retrieve_linked_accounts( + identifier: &Runtime::Identifier, +) -> Result>, LinkedDidInfoProviderError> +where + Runtime: did::Config::Identifier> + + pallet_did_lookup::Config::Identifier> + + pallet_dip_provider::Config, +{ + // Check if the user has too many linked accounts. If they have more than + // [MAX_LINKED_ACCOUNTS], we throw an error. + let are_linked_accounts_within_limit = pallet_did_lookup::ConnectedAccounts::::iter_key_prefix(identifier) + .nth(MAX_LINKED_ACCOUNTS.saturated_into()) + .is_none(); + + ensure!( + are_linked_accounts_within_limit, + LinkedDidInfoProviderError::TooManyLinkedAccounts + ); + + pallet_did_lookup::ConnectedAccounts::::iter_key_prefix(identifier) + .take(MAX_LINKED_ACCOUNTS.saturated_into()) + .collect::>() + .try_into() + // Should never happen since we checked above. + .map_err(|_| LinkedDidInfoProviderError::TooManyLinkedAccounts) +} + #[cfg(feature = "runtime-benchmarks")] impl GetWorstCase> for LinkedDidInfoOf From 42de1dc4878e109deec43f74404f3c3e596e2ad8 Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 7 Dec 2023 15:28:19 +0100 Subject: [PATCH 07/10] refactor: unnecessary struct DidMerkleProofVerifier --- crates/kilt-dip-support/src/export/child.rs | 6 +- crates/kilt-dip-support/src/export/sibling.rs | 6 +- crates/kilt-dip-support/src/merkle.rs | 188 ++++++++---------- 3 files changed, 89 insertions(+), 111 deletions(-) diff --git a/crates/kilt-dip-support/src/export/child.rs b/crates/kilt-dip-support/src/export/child.rs index 00a897b538..897f6ebf0a 100644 --- a/crates/kilt-dip-support/src/export/child.rs +++ b/crates/kilt-dip-support/src/export/child.rs @@ -499,7 +499,7 @@ pub mod v0 { }, export::common::v0::{DipMerkleProofAndDidSignature, ParachainRootStateProof}, merkle::{ - DidMerkleProofVerifier, DidMerkleProofVerifierError, RevealedDidMerkleProofLeaf, + verify_dip_merkle_proof, DidMerkleProofVerifierError, RevealedDidMerkleProofLeaf, RevealedDidMerkleProofLeaves, }, state_proofs::{ @@ -776,7 +776,7 @@ pub mod v0 { .map_err(DipChildProviderStateProofVerifierError::IdentityCommitmentMerkleProof)?; // 4. Verify DIP merkle proof. - let proof_leaves = DidMerkleProofVerifier::< + let proof_leaves = verify_dip_merkle_proof::< ProviderDipMerkleHasher, _, _, @@ -785,7 +785,7 @@ pub mod v0 { _, MAX_REVEALED_KEYS_COUNT, MAX_REVEALED_ACCOUNTS_COUNT, - >::verify_dip_merkle_proof(&subject_identity_commitment, proof.did.leaves) + >(&subject_identity_commitment, proof.did.leaves) .map_err(DipChildProviderStateProofVerifierError::DipProof)?; // 5. Verify DID signature. diff --git a/crates/kilt-dip-support/src/export/sibling.rs b/crates/kilt-dip-support/src/export/sibling.rs index daf3759881..0b2544e5e4 100644 --- a/crates/kilt-dip-support/src/export/sibling.rs +++ b/crates/kilt-dip-support/src/export/sibling.rs @@ -471,7 +471,7 @@ pub mod v0 { use crate::{ did::{RevealedDidKeysAndSignature, RevealedDidKeysSignatureAndCallVerifier}, export::common::v0::{DipMerkleProofAndDidSignature, ParachainRootStateProof}, - merkle::DidMerkleProofVerifier, + merkle::verify_dip_merkle_proof, state_proofs::{parachain::DipIdentityCommitmentProofVerifier, relay_chain::ParachainHeadProofVerifier}, traits::ProviderParachainStorageInfo, }; @@ -741,7 +741,7 @@ pub mod v0 { ProviderLinkedAccountId, MAX_REVEALED_KEYS_COUNT, MAX_REVEALED_ACCOUNTS_COUNT, - > = DidMerkleProofVerifier::< + > = verify_dip_merkle_proof::< ProviderDipMerkleHasher, _, _, @@ -750,7 +750,7 @@ pub mod v0 { _, MAX_REVEALED_KEYS_COUNT, MAX_REVEALED_ACCOUNTS_COUNT, - >::verify_dip_merkle_proof(&subject_identity_commitment, proof.did.leaves) + >(&subject_identity_commitment, proof.did.leaves) .map_err(DipSiblingProviderStateProofVerifierError::DipProof)?; // 4. Verify DID signature. diff --git a/crates/kilt-dip-support/src/merkle.rs b/crates/kilt-dip-support/src/merkle.rs index 30b39668bb..5952eb0a30 100644 --- a/crates/kilt-dip-support/src/merkle.rs +++ b/crates/kilt-dip-support/src/merkle.rs @@ -23,7 +23,7 @@ use frame_support::{traits::ConstU32, DefaultNoBound, RuntimeDebug}; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; use sp_runtime::{BoundedVec, SaturatedConversion}; -use sp_std::{fmt::Debug, marker::PhantomData, vec::Vec}; +use sp_std::{fmt::Debug, vec::Vec}; use sp_trie::{verify_trie_proof, LayoutV1}; /// Type of a Merkle proof containing DID-related information. @@ -303,7 +303,7 @@ impl From for u8 { /// supported when verifying the Merkle proof. /// * `MAX_REVEALED_ACCOUNTS_COUNT`: The maximum number of linked accounts that /// are supported when verifying the Merkle proof. -pub(crate) struct DidMerkleProofVerifier< +pub(crate) fn verify_dip_merkle_proof< Hasher, KeyId, AccountId, @@ -312,20 +312,14 @@ pub(crate) struct DidMerkleProofVerifier< LinkedAccountId, const MAX_REVEALED_KEYS_COUNT: u32, const MAX_REVEALED_ACCOUNTS_COUNT: u32, ->(#[allow(clippy::type_complexity)] PhantomData<(Hasher, KeyId, AccountId, BlockNumber, Web3Name, LinkedAccountId)>); - -impl< - Hasher, - KeyId, - AccountId, - BlockNumber, - Web3Name, - LinkedAccountId, - const MAX_REVEALED_KEYS_COUNT: u32, - const MAX_REVEALED_ACCOUNTS_COUNT: u32, - > - DidMerkleProofVerifier< - Hasher, +>( + identity_commitment: &Hasher::Out, + proof: DidMerkleProof< + crate::BoundedBlindedValue, + RevealedDidMerkleProofLeaf, + >, +) -> Result< + RevealedDidMerkleProofLeaves< KeyId, AccountId, BlockNumber, @@ -333,7 +327,10 @@ impl< LinkedAccountId, MAX_REVEALED_KEYS_COUNT, MAX_REVEALED_ACCOUNTS_COUNT, - > where + >, + DidMerkleProofVerifierError, +> +where BlockNumber: Encode + Clone, Hasher: sp_core::Hasher, KeyId: Encode + Clone, @@ -341,98 +338,79 @@ impl< LinkedAccountId: Encode + Clone, Web3Name: Encode + Clone, { - pub(crate) fn verify_dip_merkle_proof( - identity_commitment: &Hasher::Out, - proof: DidMerkleProof< - crate::BoundedBlindedValue, - RevealedDidMerkleProofLeaf, - >, - ) -> Result< - RevealedDidMerkleProofLeaves< - KeyId, - AccountId, - BlockNumber, - Web3Name, - LinkedAccountId, - 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 - // Vec<(Vec, Option>)>. - let proof_leaves = proof - .revealed - .iter() - .map(|leaf| (leaf.encoded_key(), Some(leaf.encoded_value()))) - .collect::, Option>)>>(); - let res = verify_trie_proof::, _, _, _>(identity_commitment, &proof.blinded, &proof_leaves); - cfg_if::cfg_if! { - if #[cfg(feature = "runtime-benchmarks")] { - drop(res); - } else { - res.map_err(|_| DidMerkleProofVerifierError::InvalidMerkleProof)?; - } + // TODO: more efficient by removing cloning and/or collecting. + // Did not find another way of mapping a Vec<(Vec, Vec)> to a + // Vec<(Vec, Option>)>. + let proof_leaves = proof + .revealed + .iter() + .map(|leaf| (leaf.encoded_key(), Some(leaf.encoded_value()))) + .collect::, Option>)>>(); + let res = verify_trie_proof::, _, _, _>(identity_commitment, &proof.blinded, &proof_leaves); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + drop(res); + } else { + res.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. - #[allow(clippy::type_complexity)] - let (did_keys, web3_name, linked_accounts): ( - BoundedVec, ConstU32>, - Option>, - BoundedVec>, - ) = proof.revealed.into_iter().try_fold( - ( - BoundedVec::with_bounded_capacity(MAX_REVEALED_KEYS_COUNT.saturated_into()), - None, - BoundedVec::with_bounded_capacity(MAX_REVEALED_ACCOUNTS_COUNT.saturated_into()), - ), - |(mut keys, web3_name, mut linked_accounts), leaf| match leaf { - RevealedDidMerkleProofLeaf::DidKey(key_id, key_value) => { - let res = keys.try_push(RevealedDidKey { - id: key_id.0, - relationship: key_id.1, - details: key_value.0, - }); - cfg_if::cfg_if! { - if #[cfg(feature = "runtime-benchmarks")] { - drop(res); - } else { - res.map_err(|_| DidMerkleProofVerifierError::TooManyRevealedKeys)?; - } + // 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. + #[allow(clippy::type_complexity)] + let (did_keys, web3_name, linked_accounts): ( + BoundedVec, ConstU32>, + Option>, + BoundedVec>, + ) = proof.revealed.into_iter().try_fold( + ( + BoundedVec::with_bounded_capacity(MAX_REVEALED_KEYS_COUNT.saturated_into()), + None, + BoundedVec::with_bounded_capacity(MAX_REVEALED_ACCOUNTS_COUNT.saturated_into()), + ), + |(mut keys, web3_name, mut linked_accounts), leaf| match leaf { + RevealedDidMerkleProofLeaf::DidKey(key_id, key_value) => { + let res = keys.try_push(RevealedDidKey { + id: key_id.0, + relationship: key_id.1, + details: key_value.0, + }); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + drop(res); + } else { + res.map_err(|_| DidMerkleProofVerifierError::TooManyRevealedKeys)?; } - - Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) } - RevealedDidMerkleProofLeaf::Web3Name(revealed_web3_name, details) => Ok(( - keys, - Some(RevealedWeb3Name { - web3_name: revealed_web3_name.0, - claimed_at: details.0, - }), - linked_accounts, - )), - RevealedDidMerkleProofLeaf::LinkedAccount(account_id, _) => { - let res = linked_accounts.try_push(account_id.0); - cfg_if::cfg_if! { - if #[cfg(feature = "runtime-benchmarks")] { - drop(res); - } else { - res.map_err(|_| DidMerkleProofVerifierError::TooManyRevealedAccounts)?; - } - } - Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) + Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) + } + RevealedDidMerkleProofLeaf::Web3Name(revealed_web3_name, details) => Ok(( + keys, + Some(RevealedWeb3Name { + web3_name: revealed_web3_name.0, + claimed_at: details.0, + }), + linked_accounts, + )), + RevealedDidMerkleProofLeaf::LinkedAccount(account_id, _) => { + let res = linked_accounts.try_push(account_id.0); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + drop(res); + } else { + res.map_err(|_| DidMerkleProofVerifierError::TooManyRevealedAccounts)?; + } } - }, - )?; - - Ok(RevealedDidMerkleProofLeaves { - did_keys, - web3_name, - linked_accounts, - }) - } + + Ok::<_, DidMerkleProofVerifierError>((keys, web3_name, linked_accounts)) + } + }, + )?; + + Ok(RevealedDidMerkleProofLeaves { + did_keys, + web3_name, + linked_accounts, + }) } From 04ac119fa2418a0314db463c05cda7f75d327fb4 Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 7 Dec 2023 15:46:53 +0100 Subject: [PATCH 08/10] refactor: don't duplicate function verify_did_signature_for_call --- crates/kilt-dip-support/src/did.rs | 109 +++++++---------------------- 1 file changed, 27 insertions(+), 82 deletions(-) diff --git a/crates/kilt-dip-support/src/did.rs b/crates/kilt-dip-support/src/did.rs index b8ad51a551..cfd8089da2 100644 --- a/crates/kilt-dip-support/src/did.rs +++ b/crates/kilt-dip-support/src/did.rs @@ -169,7 +169,6 @@ impl< CallVerifier: DipCallOriginFilter, DidVerificationKeyRelationship)>, { - #[cfg(not(feature = "runtime-benchmarks"))] pub(crate) fn verify_did_signature_for_call( call: &Call, submitter: &Submitter, @@ -212,8 +211,14 @@ impl< // 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)) + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + None + } 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, _)| { @@ -221,91 +226,31 @@ impl< .verify_signature(&encoded_payload, &merkle_revealed_did_signature.did_signature.signature) .is_ok() }); - let Some((key, relationship)) = valid_signing_key else { - 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(|_| RevealedDidKeysSignatureAndCallVerifierError::OriginCheckFailed)?; - Ok((key.clone(), *relationship)) - } - - #[cfg(feature = "runtime-benchmarks")] - pub(crate) fn verify_did_signature_for_call( - call: &Call, - submitter: &Submitter, - local_details: &mut Option, - merkle_revealed_did_signature: RevealedDidKeysAndSignature, - ) -> Result< - (DidVerificationKey, DidVerificationKeyRelationship), - RevealedDidKeysSignatureAndCallVerifierError, - > { - use sp_core::ed25519; + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + let default = ( + DidVerificationKey::Ed25519(sp_core::ed25519::Public::from_raw([0u8; 32])), + DidVerificationKeyRelationship::Authentication, + ); + let (key, relationship) = valid_signing_key.unwrap_or(&default); + } else { + let (key, relationship) = valid_signing_key.ok_or(RevealedDidKeysSignatureAndCallVerifierError::SignatureUnverifiable)?; + } + } - let block_number = ContextProvider::current_block_number(); - // Computed but ignored - if let Some(blocks_ago_from_now) = - block_number.checked_sub(&merkle_revealed_did_signature.did_signature.block_number) - { - // False if the signature is too old. - blocks_ago_from_now <= ContextProvider::SIGNATURE_VALIDITY.into() - } else { - // Signature generated at a future time, not possible to verify. - false - }; - let encoded_payload = ( - call, - &local_details, - submitter, - &merkle_revealed_did_signature.did_signature.block_number, - ContextProvider::genesis_hash(), - ContextProvider::signed_extra(), - ) - .encode(); - // Only consider verification keys from the set of revealed keys. - 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; - }; - if let Ok(vr) = DidVerificationKeyRelationship::try_from(*relationship) { - Some(Ok((key.clone(), vr))) - } else { - None - } - }, - ) - .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 default = ( - DidVerificationKey::Ed25519(ed25519::Public::from_raw([0u8; 32])), - DidVerificationKeyRelationship::Authentication, - ); - let (key, relationship) = valid_signing_key.unwrap_or(&default); if let Some(details) = local_details { details.bump(); } else { *local_details = Some(DidLocalDetails::default()); }; - // Ignore the result of this call - let _ = CallVerifier::check_call_origin_info(call, &(key.clone(), *relationship)); + let res = CallVerifier::check_call_origin_info(call, &(key.clone(), *relationship)); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + drop(res); + } else { + res.map_err(|_| RevealedDidKeysSignatureAndCallVerifierError::OriginCheckFailed)?; + } + } Ok((key.clone(), *relationship)) } } From 5005b9b75d8007338fe961842345ae3e37e3d543 Mon Sep 17 00:00:00 2001 From: weichweich Date: Thu, 7 Dec 2023 15:54:20 +0100 Subject: [PATCH 09/10] refactor: unnecessary struct RevealedDidKeysSignatureAndCallVerifier --- crates/kilt-dip-support/src/did.rs | 165 +++++++----------- crates/kilt-dip-support/src/export/child.rs | 33 ++-- crates/kilt-dip-support/src/export/sibling.rs | 32 ++-- 3 files changed, 86 insertions(+), 144 deletions(-) diff --git a/crates/kilt-dip-support/src/did.rs b/crates/kilt-dip-support/src/did.rs index cfd8089da2..58c39fe32c 100644 --- a/crates/kilt-dip-support/src/did.rs +++ b/crates/kilt-dip-support/src/did.rs @@ -26,7 +26,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, vec::Vec}; +use sp_std::vec::Vec; use crate::{ merkle::RevealedDidKey, @@ -110,7 +110,7 @@ impl From for u8 { /// * `RemoteBlockNumber`: Definition of a block number on the provider chain. /// * `CallVerifier`: A type specifying whether the provided `Call` can be /// dispatched with the information provided in the DIP proof. -pub(crate) struct RevealedDidKeysSignatureAndCallVerifier< +pub(crate) fn verify_did_signature_for_call< Call, Submitter, DidLocalDetails, @@ -121,42 +121,15 @@ pub(crate) struct RevealedDidKeysSignatureAndCallVerifier< RemoteBlockNumber, CallVerifier, >( - #[allow(clippy::type_complexity)] - PhantomData<( - Call, - Submitter, - DidLocalDetails, - MerkleProofEntries, - ContextProvider, - RemoteKeyId, - RemoteAccountId, - RemoteBlockNumber, - CallVerifier, - )>, -); - -impl< - Call, - Submitter, - DidLocalDetails, - MerkleProofEntries, - ContextProvider, - RemoteKeyId, - RemoteAccountId, - RemoteBlockNumber, - CallVerifier, - > - RevealedDidKeysSignatureAndCallVerifier< - Call, - Submitter, - DidLocalDetails, - MerkleProofEntries, - ContextProvider, - RemoteKeyId, - RemoteAccountId, - RemoteBlockNumber, - CallVerifier, - > where + call: &Call, + submitter: &Submitter, + local_details: &mut Option, + merkle_revealed_did_signature: RevealedDidKeysAndSignature, +) -> Result< + (DidVerificationKey, DidVerificationKeyRelationship), + RevealedDidKeysSignatureAndCallVerifierError, +> +where Call: Encode, Submitter: Encode, ContextProvider: DidSignatureVerifierContext, @@ -169,42 +142,33 @@ impl< CallVerifier: DipCallOriginFilter, DidVerificationKeyRelationship)>, { - pub(crate) fn verify_did_signature_for_call( - call: &Call, - submitter: &Submitter, - local_details: &mut Option, - merkle_revealed_did_signature: RevealedDidKeysAndSignature, - ) -> Result< - (DidVerificationKey, DidVerificationKeyRelationship), - RevealedDidKeysSignatureAndCallVerifierError, - > { - use frame_support::ensure; - - let block_number = ContextProvider::current_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) - { - // False if the signature is too old. - blocks_ago_from_now <= ContextProvider::SIGNATURE_VALIDITY.into() - } else { - // Signature generated at a future time, not possible to verify. - false - }; - ensure!( - is_signature_fresh, - RevealedDidKeysSignatureAndCallVerifierError::SignatureNotFresh, - ); - let encoded_payload = ( - call, - &local_details, - submitter, - &merkle_revealed_did_signature.did_signature.block_number, - ContextProvider::genesis_hash(), - ContextProvider::signed_extra(), - ) - .encode(); - // Only consider verification keys from the set of revealed keys. - let proof_verification_keys: Vec<(DidVerificationKey, DidVerificationKeyRelationship)> = merkle_revealed_did_signature.merkle_leaves.borrow().iter().filter_map(|RevealedDidKey { + use frame_support::ensure; + + let block_number = ContextProvider::current_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) + { + // False if the signature is too old. + blocks_ago_from_now <= ContextProvider::SIGNATURE_VALIDITY.into() + } else { + // Signature generated at a future time, not possible to verify. + false + }; + ensure!( + is_signature_fresh, + RevealedDidKeysSignatureAndCallVerifierError::SignatureNotFresh, + ); + let encoded_payload = ( + call, + &local_details, + submitter, + &merkle_revealed_did_signature.did_signature.block_number, + ContextProvider::genesis_hash(), + ContextProvider::signed_extra(), + ) + .encode(); + // Only consider verification keys from the set of revealed keys. + 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 }; if let Ok(vr) = DidVerificationKeyRelationship::try_from(*relationship) { @@ -221,36 +185,35 @@ impl< } } }).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() - }); - cfg_if::cfg_if! { - if #[cfg(feature = "runtime-benchmarks")] { - let default = ( - DidVerificationKey::Ed25519(sp_core::ed25519::Public::from_raw([0u8; 32])), - DidVerificationKeyRelationship::Authentication, - ); - let (key, relationship) = valid_signing_key.unwrap_or(&default); - } else { - let (key, relationship) = valid_signing_key.ok_or(RevealedDidKeysSignatureAndCallVerifierError::SignatureUnverifiable)?; - } + 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() + }); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + let default = ( + DidVerificationKey::Ed25519(sp_core::ed25519::Public::from_raw([0u8; 32])), + DidVerificationKeyRelationship::Authentication, + ); + let (key, relationship) = valid_signing_key.unwrap_or(&default); + } else { + let (key, relationship) = valid_signing_key.ok_or(RevealedDidKeysSignatureAndCallVerifierError::SignatureUnverifiable)?; } + } - if let Some(details) = local_details { - details.bump(); + if let Some(details) = local_details { + details.bump(); + } else { + *local_details = Some(DidLocalDetails::default()); + }; + let res = CallVerifier::check_call_origin_info(call, &(key.clone(), *relationship)); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + drop(res); } else { - *local_details = Some(DidLocalDetails::default()); - }; - let res = CallVerifier::check_call_origin_info(call, &(key.clone(), *relationship)); - cfg_if::cfg_if! { - if #[cfg(feature = "runtime-benchmarks")] { - drop(res); - } else { - res.map_err(|_| RevealedDidKeysSignatureAndCallVerifierError::OriginCheckFailed)?; - } + res.map_err(|_| RevealedDidKeysSignatureAndCallVerifierError::OriginCheckFailed)?; } - Ok((key.clone(), *relationship)) } + Ok((key.clone(), *relationship)) } diff --git a/crates/kilt-dip-support/src/export/child.rs b/crates/kilt-dip-support/src/export/child.rs index 897f6ebf0a..58321c11bb 100644 --- a/crates/kilt-dip-support/src/export/child.rs +++ b/crates/kilt-dip-support/src/export/child.rs @@ -494,8 +494,7 @@ pub mod v0 { use crate::{ did::{ - RevealedDidKeysAndSignature, RevealedDidKeysSignatureAndCallVerifier, - RevealedDidKeysSignatureAndCallVerifierError, + verify_did_signature_for_call, RevealedDidKeysAndSignature, RevealedDidKeysSignatureAndCallVerifierError, }, export::common::v0::{DipMerkleProofAndDidSignature, ParachainRootStateProof}, merkle::{ @@ -789,26 +788,16 @@ pub mod v0 { .map_err(DipChildProviderStateProofVerifierError::DipProof)?; // 5. Verify DID signature. - RevealedDidKeysSignatureAndCallVerifier::< - _, - _, - _, - _, - LocalContextProvider, - _, - _, - _, - LocalDidCallVerifier, - >::verify_did_signature_for_call( - call, - submitter, - identity_details, - RevealedDidKeysAndSignature { - merkle_leaves: proof_leaves.borrow(), - did_signature: proof.did.signature, - }, - ) - .map_err(DipChildProviderStateProofVerifierError::DidSignature)?; + verify_did_signature_for_call::<_, _, _, _, LocalContextProvider, _, _, _, LocalDidCallVerifier>( + call, + submitter, + identity_details, + RevealedDidKeysAndSignature { + merkle_leaves: proof_leaves.borrow(), + did_signature: proof.did.signature, + }, + ) + .map_err(DipChildProviderStateProofVerifierError::DidSignature)?; Ok(proof_leaves) } } diff --git a/crates/kilt-dip-support/src/export/sibling.rs b/crates/kilt-dip-support/src/export/sibling.rs index 0b2544e5e4..a67d9da56b 100644 --- a/crates/kilt-dip-support/src/export/sibling.rs +++ b/crates/kilt-dip-support/src/export/sibling.rs @@ -469,7 +469,7 @@ pub mod v0 { use sp_std::borrow::Borrow; use crate::{ - did::{RevealedDidKeysAndSignature, RevealedDidKeysSignatureAndCallVerifier}, + did::{verify_did_signature_for_call, RevealedDidKeysAndSignature}, export::common::v0::{DipMerkleProofAndDidSignature, ParachainRootStateProof}, merkle::verify_dip_merkle_proof, state_proofs::{parachain::DipIdentityCommitmentProofVerifier, relay_chain::ParachainHeadProofVerifier}, @@ -754,26 +754,16 @@ pub mod v0 { .map_err(DipSiblingProviderStateProofVerifierError::DipProof)?; // 4. Verify DID signature. - RevealedDidKeysSignatureAndCallVerifier::< - _, - _, - _, - _, - LocalContextProvider, - _, - _, - _, - LocalDidCallVerifier, - >::verify_did_signature_for_call( - call, - submitter, - identity_details, - RevealedDidKeysAndSignature { - merkle_leaves: proof_leaves.borrow(), - did_signature: proof.did.signature, - }, - ) - .map_err(DipSiblingProviderStateProofVerifierError::DidSignature)?; + verify_did_signature_for_call::<_, _, _, _, LocalContextProvider, _, _, _, LocalDidCallVerifier>( + call, + submitter, + identity_details, + RevealedDidKeysAndSignature { + merkle_leaves: proof_leaves.borrow(), + did_signature: proof.did.signature, + }, + ) + .map_err(DipSiblingProviderStateProofVerifierError::DidSignature)?; Ok(proof_leaves) } From fb8512c6d51d29240b7aca7f1ba8a1ad628dad16 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Mon, 11 Dec 2023 12:55:04 +0100 Subject: [PATCH 10/10] Correctly ignore block number in DID signature upon benchmarks --- crates/kilt-dip-support/src/did.rs | 38 +++++++++++++++------------ crates/kilt-dip-support/src/merkle.rs | 4 +-- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/crates/kilt-dip-support/src/did.rs b/crates/kilt-dip-support/src/did.rs index 58c39fe32c..88ec834d0e 100644 --- a/crates/kilt-dip-support/src/did.rs +++ b/crates/kilt-dip-support/src/did.rs @@ -86,7 +86,7 @@ impl From for u8 { } } -/// Proof verifier that tries to verify a DID signature over a given payload by +/// Function that tries to verify a DID signature over a given payload by /// using one of the DID keys revealed in the Merkle proof. This verifier is /// typically used in conjunction with a verifier that takes a user-provided /// input Merkle proof, verifies it, and transforms it into a struct that this @@ -142,22 +142,26 @@ where CallVerifier: DipCallOriginFilter, DidVerificationKeyRelationship)>, { - use frame_support::ensure; - - let block_number = ContextProvider::current_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) - { - // False if the signature is too old. - blocks_ago_from_now <= ContextProvider::SIGNATURE_VALIDITY.into() - } else { - // Signature generated at a future time, not possible to verify. - false - }; - ensure!( - is_signature_fresh, - RevealedDidKeysSignatureAndCallVerifierError::SignatureNotFresh, - ); + cfg_if::cfg_if! { + if #[cfg(feature = "runtime-benchmarks")] { + {} + } else { + let block_number = ContextProvider::current_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) + { + // False if the signature is too old. + blocks_ago_from_now <= ContextProvider::SIGNATURE_VALIDITY.into() + } else { + // Signature generated at a future time, not possible to verify. + false + }; + frame_support::ensure!( + is_signature_fresh, + RevealedDidKeysSignatureAndCallVerifierError::SignatureNotFresh, + ); + } + } let encoded_payload = ( call, &local_details, diff --git a/crates/kilt-dip-support/src/merkle.rs b/crates/kilt-dip-support/src/merkle.rs index 5952eb0a30..6d744777f8 100644 --- a/crates/kilt-dip-support/src/merkle.rs +++ b/crates/kilt-dip-support/src/merkle.rs @@ -279,8 +279,8 @@ impl From for u8 { } } -/// A type that verifies a DIP Merkle proof revealing some leaves representing -/// parts of a KILT DID identity stored on the KILT chain. +/// A function that verifies a DIP Merkle proof revealing some leaves +/// representing parts of a KILT DID identity stored on the KILT chain. /// If cross-chain DID signatures are not required for the specific use case, /// this verifier can also be used on its own, without any DID signature /// verification.