From 54b584252e423832e441f2b439f799060b4e5cd1 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Mon, 1 Aug 2022 15:57:40 +0200 Subject: [PATCH 01/21] wip: changing stuff based on meeting --- pallets/public-credentials/src/credentials.rs | 19 ---- pallets/public-credentials/src/lib.rs | 86 ++++++++----------- pallets/public-credentials/src/mock.rs | 17 +--- 3 files changed, 37 insertions(+), 85 deletions(-) diff --git a/pallets/public-credentials/src/credentials.rs b/pallets/public-credentials/src/credentials.rs index bfea88932c..2508ac875c 100644 --- a/pallets/public-credentials/src/credentials.rs +++ b/pallets/public-credentials/src/credentials.rs @@ -34,16 +34,6 @@ pub struct Claim { pub contents: Content, } -/// The type of a claimer's signature to prove the claimer's involvement in the -/// public credential issuance process. -#[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, PartialOrd, Ord, TypeInfo)] -pub struct ClaimerSignatureInfo { - /// The identifier of the claimer. - pub claimer_id: ClaimerIdentifier, - /// The signature of the claimer. - pub signature_payload: Signature, -} - /// The type of a credentials as incoming from the outside world. /// Some of its fields are parsed and/or transformed inside the `add` operation. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, PartialOrd, Ord, TypeInfo)] @@ -51,19 +41,10 @@ pub struct Credential< CtypeHash, SubjectIdentifier, ClaimContent, - ClaimHash, - Nonce, - ClaimerSignature, AuthorizationControl, > { /// The credential content. pub claim: Claim, - /// The nonce used to generate the root hash. - pub nonce: Nonce, - /// The root hash of the credential claims. - pub claim_hash: ClaimHash, - /// The claimer's signature information. - pub claimer_signature: Option, /// The authorization info to attest the credential. pub authorization_info: Option, } diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index d9aeb0de7f..bdb01826d1 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -61,19 +61,17 @@ pub mod pallet { use frame_support::{ pallet_prelude::*, - sp_runtime::traits::Saturating, traits::{Currency, IsType, ReservableCurrency, StorageVersion}, Parameter, }; use frame_system::pallet_prelude::*; - use sp_core::H256; - use sp_runtime::SaturatedConversion; + use sp_runtime::traits::{Hash, Saturating, SaturatedConversion}; use sp_std::{boxed::Box, vec::Vec}; use attestation::{AttestationAccessControl, AttesterOf, ClaimHashOf}; use ctype::CtypeHashOf; use kilt_support::{ - signature::{SignatureVerificationError, VerifySignature}, + signature::VerifySignature, traits::CallSources, }; @@ -94,22 +92,18 @@ pub mod pallet { pub type InputSubjectIdOf = BoundedVec::MaxSubjectIdLength>; /// The type of the credential subject input. It is bound in max length. pub type InputClaimsContentOf = BoundedVec::MaxEncodedClaimsLength>; - pub type ClaimerSignatureOf = - ClaimerSignatureInfo<::ClaimerIdentifier, ::ClaimerSignature>; pub type AccountIdOf = ::AccountId; pub type BlockNumberOf = ::BlockNumber; pub type CredentialEntryOf = CredentialEntry, AccountIdOf, BalanceOf>; /// The type of account's balances. pub type BalanceOf = as Currency>>::Balance; + pub(crate) type CredentialIdOf = <::CredentialHash as sp_runtime::traits::Hash>::Output; /// The type of a public credential as the pallet expects it. pub type InputCredentialOf = Credential< CtypeHashOf, InputSubjectIdOf, InputClaimsContentOf, - ClaimHashOf, - H256, - ClaimerSignatureOf, ::AccessControl, >; @@ -129,6 +123,8 @@ pub mod pallet { type EnsureOrigin: EnsureOrigin::OriginSuccess, Self::Origin>; /// The ubiquitous event type. type Event: From> + IsType<::Event>; + /// The hashing algorithm to derive a credential identifier from the credential content. + type CredentialHash: Hash>; /// The type of the origin when successfully converted from the outer /// origin. type OriginSuccess: CallSources>; @@ -192,14 +188,14 @@ pub mod pallet { /// The subject of the new credential. subject_id: T::SubjectId, /// The root hash of the new credential. - claim_hash: ClaimHashOf, + credential_id: ClaimHashOf, }, /// A public credentials has been removed. CredentialRemoved { /// The subject of the removed credential. subject_id: T::SubjectId, /// The root hash of the removed credential. - claim_hash: ClaimHashOf, + credential_id: ClaimHashOf, }, } @@ -247,9 +243,8 @@ pub mod pallet { /// Emits `CredentialStored`. #[allow(clippy::boxed_local)] #[pallet::weight({ - let signature_weight = credential.claimer_signature.as_ref().map(|_| ::ClaimerSignatureVerification::weight(credential.claim_hash.encoded_size())).unwrap_or(0); let ac_weight = credential.authorization_info.as_ref().map(|ac| ac.can_attest_weight()).unwrap_or(0); - ::WeightInfo::add(credential.claim.contents.len().saturated_into::()).saturating_add(signature_weight).saturating_add(ac_weight) + ::WeightInfo::add(credential.claim.contents.len().saturated_into::()).saturating_add(ac_weight) })] pub fn add(origin: OriginFor, credential: Box>) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; @@ -266,11 +261,10 @@ pub mod pallet { subject, contents: _, }, - claim_hash, - nonce: _, - claimer_signature, authorization_info, - } = *credential; + } = *credential.clone(); + + let credential_id = T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); // Try to decode subject ID to something structured let subject = T::SubjectId::try_from(subject.into_inner()).map_err(|_| Error::::InvalidInput)?; @@ -278,7 +272,7 @@ pub mod pallet { // Check that the same attestation has not already been issued previously // (potentially to a different subject) ensure!( - !CredentialsUnicityIndex::::contains_key(&claim_hash), + !CredentialsUnicityIndex::::contains_key(&credential_id), Error::::CredentialAlreadyIssued ); @@ -297,25 +291,11 @@ pub mod pallet { Error::::UnableToPayFees ); - // Check the validity of the claimer's signature, if present, over the - // credential root hash. - if let Some(ClaimerSignatureInfo { - claimer_id, - signature_payload, - }) = claimer_signature - { - T::ClaimerSignatureVerification::verify(&claimer_id, &claim_hash.encode(), &signature_payload) - .map_err(|err| match err { - SignatureVerificationError::SignerInformationNotPresent => Error::::ClaimerInfoNotFound, - SignatureVerificationError::SignatureInvalid => Error::::InvalidClaimerSignature, - })?; - } - // Delegate to the attestation pallet writing the attestation information and // reserve its part of the deposit attestation::Pallet::::write_attestation( ctype_hash, - claim_hash, + credential_id, attester, payer.clone(), authorization_info, @@ -330,12 +310,16 @@ pub mod pallet { let block_number = frame_system::Pallet::::block_number(); - Credentials::::insert(&subject, &claim_hash, CredentialEntryOf:: { deposit, block_number }); - CredentialsUnicityIndex::::insert(&claim_hash, subject.clone()); + Credentials::::insert( + &subject, + &credential_id, + CredentialEntryOf:: { deposit, block_number }, + ); + CredentialsUnicityIndex::::insert(&credential_id, subject.clone()); Self::deposit_event(Event::CredentialStored { subject_id: subject, - claim_hash, + credential_id, }); Ok(()) @@ -369,19 +353,19 @@ pub mod pallet { })] pub fn remove( origin: OriginFor, - claim_hash: ClaimHashOf, + credential_id: CredentialIdOf, authorization: Option, ) -> DispatchResultWithPostInfo { let source = ::EnsureOrigin::ensure_origin(origin)?; let attester = source.subject(); - let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&claim_hash)?; + let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; // Delegate to the attestation pallet the removal logic // This guarantees that the authorized attester is calling this function - let result = attestation::Pallet::::remove_attestation(attester, claim_hash, authorization)?; + let result = attestation::Pallet::::remove_attestation(attester, credential_id, authorization)?; - Self::remove_credential_entry(credential_subject, claim_hash, credential_entry); + Self::remove_credential_entry(credential_subject, credential_id, credential_entry); Ok(result) } @@ -396,16 +380,16 @@ pub mod pallet { /// attestation pallet still requires the deposit payer to also remove /// any traces of the corresponding public credential from this pallet. #[pallet::weight(::WeightInfo::reclaim_deposit())] - pub fn reclaim_deposit(origin: OriginFor, claim_hash: ClaimHashOf) -> DispatchResult { + pub fn reclaim_deposit(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { let who = ensure_signed(origin)?; - let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&claim_hash)?; + let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; // Delegate to the attestation pallet the removal logic. // This guarantees that the authorized payer is calling this function. - attestation::Pallet::::unlock_deposit(who, claim_hash)?; + attestation::Pallet::::unlock_deposit(who, credential_id)?; - Self::remove_credential_entry(credential_subject, claim_hash, credential_entry); + Self::remove_credential_entry(credential_subject, credential_id, credential_entry); Ok(()) } @@ -416,28 +400,28 @@ pub mod pallet { // credential. fn remove_credential_entry( credential_subject: T::SubjectId, - claim_hash: ClaimHashOf, + credential_id: CredentialIdOf, credential: CredentialEntryOf, ) { kilt_support::free_deposit::>(&credential.deposit); - Credentials::::remove(&credential_subject, &claim_hash); - CredentialsUnicityIndex::::remove(&claim_hash); + Credentials::::remove(&credential_subject, &credential_id); + CredentialsUnicityIndex::::remove(&credential_id); Self::deposit_event(Event::CredentialRemoved { subject_id: credential_subject, - claim_hash, + credential_id, }); } fn retrieve_credential_entry( - claim_hash: &ClaimHashOf, + credential_id: &CredentialIdOf, ) -> Result<(T::SubjectId, CredentialEntryOf), Error> { // Verify that the credential exists let credential_subject = - CredentialsUnicityIndex::::get(&claim_hash).ok_or(Error::::CredentialNotFound)?; + CredentialsUnicityIndex::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; // Should never happen if the line above succeeds - Credentials::::get(&credential_subject, &claim_hash) + Credentials::::get(&credential_subject, &credential_id) .map(|entry| (credential_subject, entry)) .ok_or(Error::::InternalError) } diff --git a/pallets/public-credentials/src/mock.rs b/pallets/public-credentials/src/mock.rs index d010870111..08472becef 100644 --- a/pallets/public-credentials/src/mock.rs +++ b/pallets/public-credentials/src/mock.rs @@ -16,21 +16,15 @@ // If you feel like getting in touch with us, you can do so at info@botlabs.org -use attestation::ClaimHashOf; use ctype::CtypeHashOf; -use crate::{Claim, ClaimerSignatureInfo, Config, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; - -pub(crate) type ClaimerSignatureInfoOf = - ClaimerSignatureInfo<::ClaimerIdentifier, ::ClaimerSignature>; +use crate::{Claim, Config, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; // Generate a public credential using a many Default::default() as possible. pub fn generate_base_public_credential_creation_op( subject_id: InputSubjectIdOf, - claim_hash: ClaimHashOf, ctype_hash: CtypeHashOf, contents: InputClaimsContentOf, - claimer_signature: Option>, ) -> InputCredentialOf { InputCredentialOf:: { claim: Claim { @@ -38,9 +32,6 @@ pub fn generate_base_public_credential_creation_op( subject: subject_id, contents, }, - claim_hash, - claimer_signature, - nonce: Default::default(), authorization_info: Default::default(), } } @@ -261,6 +252,7 @@ pub(crate) mod runtime { type ClaimerIdentifier = SubjectId; type ClaimerSignature = (Self::ClaimerIdentifier, Vec); type ClaimerSignatureVerification = EqualVerify>; + type CredentialHash = BlakeTwo256; type Deposit = ConstU128<{ 10 * MILLI_UNIT }>; type EnsureOrigin = ::EnsureOrigin; type Event = (); @@ -276,7 +268,6 @@ pub(crate) mod runtime { pub(crate) const ALICE_SEED: [u8; 32] = [0u8; 32]; pub(crate) const BOB_SEED: [u8; 32] = [1u8; 32]; - pub(crate) const CHARLIE_SEED: [u8; 32] = [2u8; 32]; pub(crate) const SUBJECT_ID_00: TestSubjectId = TestSubjectId([100u8; 32]); @@ -293,10 +284,6 @@ pub(crate) mod runtime { .into() } - pub(crate) fn hash_to_u8(hash: Hash) -> Vec { - hash.encode() - } - #[derive(Clone, Default)] pub(crate) struct ExtBuilder { /// initial ctypes & owners From 649e04a7181f74b892f8ede97129ae06cc606d5e Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Mon, 1 Aug 2022 16:25:21 +0200 Subject: [PATCH 02/21] feat: generate credential ID on chain --- pallets/public-credentials/src/mock.rs | 20 +-- pallets/public-credentials/src/tests.rs | 184 ++++++------------------ 2 files changed, 58 insertions(+), 146 deletions(-) diff --git a/pallets/public-credentials/src/mock.rs b/pallets/public-credentials/src/mock.rs index 08472becef..08a1b42e38 100644 --- a/pallets/public-credentials/src/mock.rs +++ b/pallets/public-credentials/src/mock.rs @@ -16,9 +16,13 @@ // If you feel like getting in touch with us, you can do so at info@botlabs.org +use codec::Encode; +use sp_runtime::traits::Hash; + use ctype::CtypeHashOf; +use attestation::AttesterOf; -use crate::{Claim, Config, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; +use crate::{Claim, Config, CredentialIdOf, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; // Generate a public credential using a many Default::default() as possible. pub fn generate_base_public_credential_creation_op( @@ -36,6 +40,13 @@ pub fn generate_base_public_credential_creation_op( } } +pub fn generate_credential_id( + input_credential: &InputCredentialOf, + attester: &AttesterOf +) -> CredentialIdOf { + T::CredentialHash::hash(&[&input_credential.encode()[..], &attester.encode()[..]].concat()[..]) +} + #[cfg(test)] pub use crate::mock::runtime::*; @@ -271,13 +282,6 @@ pub(crate) mod runtime { pub(crate) const SUBJECT_ID_00: TestSubjectId = TestSubjectId([100u8; 32]); - pub(crate) const CLAIM_HASH_SEED_01: u64 = 1u64; - pub(crate) const CLAIM_HASH_SEED_02: u64 = 2u64; - - pub(crate) fn claim_hash_from_seed(seed: u64) -> Hash { - Hash::from_low_u64_be(seed) - } - pub(crate) fn sr25519_did_from_seed(seed: &[u8; 32]) -> SubjectId { MultiSigner::from(sr25519::Pair::from_seed(seed).public()) .into_account() diff --git a/pallets/public-credentials/src/tests.rs b/pallets/public-credentials/src/tests.rs index 3abceafeea..f6edb334d0 100644 --- a/pallets/public-credentials/src/tests.rs +++ b/pallets/public-credentials/src/tests.rs @@ -23,31 +23,28 @@ use attestation::{attestations::AttestationDetails, mock::generate_base_attestat use ctype::mock::get_ctype_hash; use kilt_support::mock::mock_origin::DoubleOrigin; -use crate::{mock::*, Config, Credentials, CredentialsUnicityIndex, Error, InputClaimsContentOf}; +use crate::{mock::*, Config, Credentials, CredentialIdOf, CredentialsUnicityIndex, Error, InputClaimsContentOf}; // add #[test] -fn add_with_no_signature_successful() { +fn add_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); - let claim_hash_2 = claim_hash_from_seed(CLAIM_HASH_SEED_02); - let ctype_hash = get_ctype_hash::(true); + let ctype_hash_1 = get_ctype_hash::(true); + let ctype_hash_2 = get_ctype_hash::(false); let new_credential_1 = generate_base_public_credential_creation_op::( subject_id.into(), - claim_hash, - ctype_hash, + ctype_hash_1, InputClaimsContentOf::::default(), - None, ); + let credential_id_1: CredentialIdOf = generate_credential_id::(&new_credential_1, &attester); let new_credential_2 = generate_base_public_credential_creation_op::( subject_id.into(), - claim_hash_2, - ctype_hash, + ctype_hash_2, InputClaimsContentOf::::default(), - None, ); + let credential_id_2: CredentialIdOf = generate_credential_id::(&new_credential_2, &attester); let public_credential_deposit: Balance = ::Deposit::get(); let attestation_deposit: Balance = ::Deposit::get(); @@ -56,7 +53,7 @@ fn add_with_no_signature_successful() { ACCOUNT_00, (public_credential_deposit + attestation_deposit) * 2, )]) - .with_ctypes(vec![(ctype_hash, attester.clone())]) + .with_ctypes(vec![(ctype_hash_1, attester.clone()), (ctype_hash_2, attester.clone())]) .build() .execute_with(|| { // Check for 0 reserved deposit @@ -67,17 +64,17 @@ fn add_with_no_signature_successful() { Box::new(new_credential_1.clone()) )); let stored_attestation = - Attestations::::get(&claim_hash).expect("Attestation should be present on chain."); - let stored_public_credential_details = Credentials::::get(&subject_id, &claim_hash) + Attestations::::get(&credential_id_1).expect("Attestation should be present on chain."); + let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id_1) .expect("Public credential details should be present on chain."); // Test interactions with attestation pallet - assert_eq!(stored_attestation.ctype_hash, ctype_hash); + assert_eq!(stored_attestation.ctype_hash, ctype_hash_1); assert_eq!(stored_attestation.attester, attester); // Test this pallet logic assert_eq!(stored_public_credential_details.block_number, 0); - assert_eq!(CredentialsUnicityIndex::::get(&claim_hash), Some(subject_id)); + assert_eq!(CredentialsUnicityIndex::::get(&credential_id_1), Some(subject_id)); // Check deposit reservation logic assert_eq!( @@ -109,17 +106,17 @@ fn add_with_no_signature_successful() { )); let stored_attestation = - Attestations::::get(&claim_hash_2).expect("Attestation #2 should be present on chain."); - let stored_public_credential_details = Credentials::::get(&subject_id, &claim_hash_2) + Attestations::::get(&credential_id_2).expect("Attestation #2 should be present on chain."); + let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id_2) .expect("Public credential #2 details should be present on chain."); // Test interactions with attestation pallet - assert_eq!(stored_attestation.ctype_hash, ctype_hash); + assert_eq!(stored_attestation.ctype_hash, ctype_hash_2); assert_eq!(stored_attestation.attester, attester); // Test this pallet logic assert_eq!(stored_public_credential_details.block_number, 1); - assert_eq!(CredentialsUnicityIndex::::get(&claim_hash_2), Some(subject_id)); + assert_eq!(CredentialsUnicityIndex::::get(&credential_id_2), Some(subject_id)); // Deposit is 2x now assert_eq!( @@ -128,7 +125,7 @@ fn add_with_no_signature_successful() { ); // Deleting the attestation only from the attestation pallet will still fail - Attestation::reclaim_deposit(Origin::signed(ACCOUNT_00), claim_hash) + Attestation::reclaim_deposit(Origin::signed(ACCOUNT_00), credential_id_1) .expect("Attestation deposit reclaim should not fail"); assert_noop!( PublicCredentials::add( @@ -146,62 +143,15 @@ fn add_with_no_signature_successful() { }); } -#[test] -fn add_with_claimer_signature_successful() { - let attester = sr25519_did_from_seed(&ALICE_SEED); - let subject_id = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); - let ctype_hash = get_ctype_hash::(true); - let claimer_id = sr25519_did_from_seed(&BOB_SEED); - let new_credential = generate_base_public_credential_creation_op::( - subject_id.into(), - claim_hash, - ctype_hash, - InputClaimsContentOf::::default(), - Some(ClaimerSignatureInfoOf:: { - claimer_id: claimer_id.clone(), - signature_payload: (claimer_id, hash_to_u8(claim_hash)), - }), - ); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); - - ExtBuilder::default() - .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) - .with_ctypes(vec![(ctype_hash, attester.clone())]) - .build() - .execute_with(|| { - assert_ok!(PublicCredentials::add( - DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - Box::new(new_credential.clone()) - )); - let stored_attestation = - Attestations::::get(&claim_hash).expect("Attestation should be present on chain."); - let stored_public_credential_details = Credentials::::get(&subject_id, &claim_hash) - .expect("Public credential details should be present on chain."); - - // Test interactions with attestation pallet - assert_eq!(stored_attestation.ctype_hash, ctype_hash); - assert_eq!(stored_attestation.attester, attester); - - // Test this pallet logic - assert_eq!(stored_public_credential_details.block_number, System::block_number()); - assert_eq!(CredentialsUnicityIndex::::get(&claim_hash), Some(subject_id)); - }); -} - #[test] fn add_not_enough_balance() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); let ctype_hash = get_ctype_hash::(true); let new_credential = generate_base_public_credential_creation_op::( subject_id.into(), - claim_hash, ctype_hash, InputClaimsContentOf::::default(), - None, ); let public_credential_deposit: Balance = ::Deposit::get(); let attestation_deposit: Balance = ::Deposit::get(); @@ -222,57 +172,15 @@ fn add_not_enough_balance() { }); } -#[test] -fn add_invalid_signature() { - let attester = sr25519_did_from_seed(&ALICE_SEED); - let subject_id = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); - let ctype_hash = get_ctype_hash::(true); - let claimer_id = sr25519_did_from_seed(&CHARLIE_SEED); - let new_credential = generate_base_public_credential_creation_op::( - subject_id.into(), - claim_hash, - ctype_hash, - InputClaimsContentOf::::default(), - Some(ClaimerSignatureInfoOf:: { - claimer_id, - signature_payload: (sr25519_did_from_seed(&BOB_SEED), hash_to_u8(claim_hash)), - }), - ); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); - - ExtBuilder::default() - .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) - .with_ctypes(vec![(ctype_hash, attester.clone())]) - .build() - .execute_with(|| { - assert_noop!( - PublicCredentials::add( - DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - Box::new(new_credential.clone()) - ), - Error::::InvalidClaimerSignature - ); - }); -} - #[test] fn add_ctype_not_found() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); let ctype_hash = get_ctype_hash::(true); - let claimer_id = sr25519_did_from_seed(&CHARLIE_SEED); let new_credential = generate_base_public_credential_creation_op::( subject_id.into(), - claim_hash, ctype_hash, InputClaimsContentOf::::default(), - Some(ClaimerSignatureInfoOf:: { - claimer_id: claimer_id.clone(), - signature_payload: (claimer_id, hash_to_u8(claim_hash)), - }), ); let public_credential_deposit: Balance = ::Deposit::get(); let attestation_deposit: Balance = ::Deposit::get(); @@ -297,37 +205,37 @@ fn add_ctype_not_found() { fn remove_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); let attestation: AttestationDetails = generate_base_attestation(attester.clone(), ACCOUNT_00); let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); let public_credential_deposit: Balance = ::Deposit::get(); let attestation_deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) .with_ctypes(vec![(attestation.ctype_hash, attester.clone())]) - .with_attestations(vec![(claim_hash, attestation.clone())]) - .with_public_credentials(vec![(subject_id, claim_hash, new_credential)]) + .with_attestations(vec![(credential_id, attestation.clone())]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { assert_ok!(PublicCredentials::remove( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - claim_hash, + credential_id, None )); // Test interactions with attestation pallet - assert_eq!(Attestations::::get(&claim_hash), None); + assert_eq!(Attestations::::get(&credential_id), None); // Test this pallet logic - assert_eq!(Credentials::::get(&subject_id, &claim_hash), None); - assert_eq!(CredentialsUnicityIndex::::get(&claim_hash), None); + assert_eq!(Credentials::::get(&subject_id, &credential_id), None); + assert_eq!(CredentialsUnicityIndex::::get(&credential_id), None); // Check deposit release logic assert!(Balances::reserved_balance(ACCOUNT_00).is_zero()); // Removing the same credential again will fail assert_noop!( - PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), claim_hash, None), + PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, None), Error::::CredentialNotFound ); @@ -335,14 +243,14 @@ fn remove_successful() { // the credential. Attestation::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - claim_hash, + credential_id, attestation.ctype_hash, None, ) .expect("Adding the same attestation again should not fail"); assert_noop!( - PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), claim_hash, None), + PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, None), Error::::CredentialNotFound ); @@ -356,21 +264,21 @@ fn remove_unauthorized() { let attester = sr25519_did_from_seed(&ALICE_SEED); let wrong_attester = sr25519_did_from_seed(&BOB_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); let attestation: AttestationDetails = generate_base_attestation(attester.clone(), ACCOUNT_00); let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); let public_credential_deposit: Balance = ::Deposit::get(); let attestation_deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) .with_ctypes(vec![(attestation.ctype_hash, attester)]) - .with_attestations(vec![(claim_hash, attestation)]) - .with_public_credentials(vec![(subject_id, claim_hash, new_credential)]) + .with_attestations(vec![(credential_id, attestation)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { assert_noop!( - PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, wrong_attester).into(), claim_hash, None), + PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, wrong_attester).into(), credential_id, None), attestation::Error::::Unauthorized ); }); @@ -382,36 +290,36 @@ fn remove_unauthorized() { fn reclaim_deposit_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); let attestation: AttestationDetails = generate_base_attestation(attester.clone(), ACCOUNT_00); let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); let public_credential_deposit: Balance = ::Deposit::get(); let attestation_deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) .with_ctypes(vec![(attestation.ctype_hash, attester.clone())]) - .with_attestations(vec![(claim_hash, attestation.clone())]) - .with_public_credentials(vec![(subject_id, claim_hash, new_credential)]) + .with_attestations(vec![(credential_id, attestation.clone())]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { assert_ok!(PublicCredentials::reclaim_deposit( Origin::signed(ACCOUNT_00), - claim_hash + credential_id )); // Test interactions with attestation pallet - assert_eq!(Attestations::::get(&claim_hash), None); + assert_eq!(Attestations::::get(&credential_id), None); // Test this pallet logic - assert_eq!(Credentials::::get(&subject_id, &claim_hash), None); - assert_eq!(CredentialsUnicityIndex::::get(&claim_hash), None); + assert_eq!(Credentials::::get(&subject_id, &credential_id), None); + assert_eq!(CredentialsUnicityIndex::::get(&credential_id), None); // Check deposit release logic assert!(Balances::reserved_balance(ACCOUNT_00).is_zero()); // Reclaiming the deposit for the same credential again will fail assert_noop!( - PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_00), claim_hash), + PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_00), credential_id), Error::::CredentialNotFound ); @@ -419,14 +327,14 @@ fn reclaim_deposit_successful() { // the deposit for the credential. Attestation::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - claim_hash, + credential_id, attestation.ctype_hash, None, ) .expect("Adding the same attestation again should not fail"); assert_noop!( - PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_00), claim_hash), + PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_00), credential_id), Error::::CredentialNotFound ); @@ -439,21 +347,21 @@ fn reclaim_deposit_successful() { fn reclaim_deposit_unauthorized() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let claim_hash = claim_hash_from_seed(CLAIM_HASH_SEED_01); let attestation: AttestationDetails = generate_base_attestation(attester.clone(), ACCOUNT_00); let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); let public_credential_deposit: Balance = ::Deposit::get(); let attestation_deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) .with_ctypes(vec![(attestation.ctype_hash, attester)]) - .with_attestations(vec![(claim_hash, attestation)]) - .with_public_credentials(vec![(subject_id, claim_hash, new_credential)]) + .with_attestations(vec![(credential_id, attestation)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { assert_noop!( - PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_01), claim_hash), + PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_01), credential_id), attestation::Error::::Unauthorized ); }); From 8c932fc21e73c8de3507de24f7ed08cb85565578 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Tue, 2 Aug 2022 15:59:20 +0200 Subject: [PATCH 03/21] remove dependency on attestation pallet --- Cargo.lock | 1 - pallets/public-credentials/Cargo.toml | 3 - pallets/public-credentials/src/credentials.rs | 26 +-- .../public-credentials/src/default_weights.rs | 11 + pallets/public-credentials/src/lib.rs | 196 ++++++------------ pallets/public-credentials/src/mock.rs | 69 ++---- pallets/public-credentials/src/tests.rs | 175 +++------------- 7 files changed, 127 insertions(+), 354 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 56bf6f53dc..993dc3bcd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8472,7 +8472,6 @@ dependencies = [ name = "public-credentials" version = "1.7.0" dependencies = [ - "attestation", "base58", "ctype", "frame-benchmarking", diff --git a/pallets/public-credentials/Cargo.toml b/pallets/public-credentials/Cargo.toml index 2dd7bba950..f15efe086c 100644 --- a/pallets/public-credentials/Cargo.toml +++ b/pallets/public-credentials/Cargo.toml @@ -7,7 +7,6 @@ repository = "https://github.com/KILTprotocol/kilt-node" version = "1.7.0" [dev-dependencies] -attestation = {features = ["mock"], path = "../attestation"} ctype = {features = ["mock"], path = "../ctype"} kilt-support = {features = ["mock"], path = "../../support"} @@ -23,7 +22,6 @@ log = "0.4.17" scale-info = {version = "2.1.1", default-features = false, features = ["derive"]} # Internal dependencies -attestation = {default-features = false, path = "../attestation"} ctype = {default-features = false, path = "../ctype"} kilt-support = {default-features = false, path = "../../support"} @@ -51,7 +49,6 @@ std = [ "log/std", "scale-info/std", - "attestation/std", "ctype/std", "kilt-support/std", diff --git a/pallets/public-credentials/src/credentials.rs b/pallets/public-credentials/src/credentials.rs index 2508ac875c..86d11d3b2a 100644 --- a/pallets/public-credentials/src/credentials.rs +++ b/pallets/public-credentials/src/credentials.rs @@ -23,30 +23,16 @@ use frame_support::RuntimeDebug; use kilt_support::deposit::Deposit; -/// The bulk of the credential, i.e., its (encoded) claims, subject, and Ctype. +/// The type of a credentials as incoming from the outside world. +/// Some of its fields are parsed and/or transformed inside the `add` operation. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, PartialOrd, Ord, TypeInfo)] -pub struct Claim { +pub struct Credential { /// The Ctype of the credential. pub ctype_hash: CtypeHash, /// The credential subject ID as specified by the attester. pub subject: SubjectIdentifier, /// The credential claims. - pub contents: Content, -} - -/// The type of a credentials as incoming from the outside world. -/// Some of its fields are parsed and/or transformed inside the `add` operation. -#[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, PartialOrd, Ord, TypeInfo)] -pub struct Credential< - CtypeHash, - SubjectIdentifier, - ClaimContent, - AuthorizationControl, -> { - /// The credential content. - pub claim: Claim, - /// The authorization info to attest the credential. - pub authorization_info: Option, + pub claims: Claims, } /// The entry in the blockchain state corresponding to a successful public @@ -55,7 +41,9 @@ pub struct Credential< /// block. The block number is used to query the full content of the credential /// from archive nodes. #[derive(Encode, Decode, Clone, MaxEncodedLen, RuntimeDebug, PartialEq, Eq, PartialOrd, Ord, TypeInfo)] -pub struct CredentialEntry { +pub struct CredentialEntry { + /// The attester of the credential. + pub attester: Attester, /// The block number in which the credential tx was evaluated and included /// in the block. pub block_number: BlockNumber, diff --git a/pallets/public-credentials/src/default_weights.rs b/pallets/public-credentials/src/default_weights.rs index 2db42ac223..241991b7fb 100644 --- a/pallets/public-credentials/src/default_weights.rs +++ b/pallets/public-credentials/src/default_weights.rs @@ -50,6 +50,7 @@ use sp_std::marker::PhantomData; pub trait WeightInfo { fn add(c: u32, ) -> Weight; fn remove() -> Weight; + fn revoke() -> Weight; fn reclaim_deposit() -> Weight; } @@ -75,6 +76,11 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } + fn revoke() -> Weight { + (35_041_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) // Storage: Attestation Attestations (r:1 w:1) @@ -107,6 +113,11 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } + fn revoke() -> Weight { + (35_041_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + } // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) // Storage: Attestation Attestations (r:1 w:1) diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index bdb01826d1..51e55f1c72 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -65,13 +65,11 @@ pub mod pallet { Parameter, }; use frame_system::pallet_prelude::*; - use sp_runtime::traits::{Hash, Saturating, SaturatedConversion}; - use sp_std::{boxed::Box, vec::Vec}; + use sp_runtime::traits::{Hash, SaturatedConversion}; + use sp_std::vec::Vec; - use attestation::{AttestationAccessControl, AttesterOf, ClaimHashOf}; use ctype::CtypeHashOf; use kilt_support::{ - signature::VerifySignature, traits::CallSources, }; @@ -83,8 +81,7 @@ pub mod pallet { // and simply rollback if the two are the same and there is not enough // for both operations. /// The type of currency to use to reserve the required deposits. - /// Must match the definition of [::Currency]. - pub(crate) type CurrencyOf = ::Currency; + pub(crate) type CurrencyOf = ::Currency; /// The type of the credential subject input. It is bound in max length. /// It is transformed inside the `add` operation into a [ = BoundedVec::MaxEncodedClaimsLength>; pub type AccountIdOf = ::AccountId; pub type BlockNumberOf = ::BlockNumber; - pub type CredentialEntryOf = CredentialEntry, AccountIdOf, BalanceOf>; + pub type CredentialEntryOf = CredentialEntry, BlockNumberOf, AccountIdOf, BalanceOf>; + /// Type of an attester identifier. + pub type AttesterOf = ::AttesterId; /// The type of account's balances. pub type BalanceOf = as Currency>>::Balance; pub(crate) type CredentialIdOf = <::CredentialHash as sp_runtime::traits::Hash>::Output; @@ -104,27 +103,22 @@ pub mod pallet { CtypeHashOf, InputSubjectIdOf, InputClaimsContentOf, - ::AccessControl, >; #[pallet::config] - pub trait Config: frame_system::Config + ctype::Config + attestation::Config { - /// The identifier of the credential claimer. - type ClaimerIdentifier: Parameter; - /// The signature of the credential claimer. - type ClaimerSignature: Parameter; - /// The claimer's signature verification logic. - type ClaimerSignatureVerification: VerifySignature< - SignerId = Self::ClaimerIdentifier, - Payload = Vec, - Signature = Self::ClaimerSignature, - >; + pub trait Config: frame_system::Config + ctype::Config { + /// The identifier of the credential attester. + type AttesterId: Parameter + MaxEncodedLen; /// The origin allowed to issue/revoke/remove public credentials. type EnsureOrigin: EnsureOrigin::OriginSuccess, Self::Origin>; /// The ubiquitous event type. type Event: From> + IsType<::Event>; /// The hashing algorithm to derive a credential identifier from the credential content. - type CredentialHash: Hash>; + type CredentialHash: Hash; + /// The type of a credential identifier. + type CredentialId: Parameter + MaxEncodedLen; + /// The currency that is used to reserve funds for each attestation. + type Currency: ReservableCurrency>; /// The type of the origin when successfully converted from the outer /// origin. type OriginSuccess: CallSources>; @@ -155,7 +149,7 @@ pub mod pallet { pub struct Pallet(_); /// The map of public credentials already attested. - /// It maps from a (subject id + credential root hash) -> the creation + /// It maps from a (subject id + credential id) -> the creation /// details of the credential. #[pallet::storage] #[pallet::getter(fn get_credential_info)] @@ -164,20 +158,17 @@ pub mod pallet { Twox64Concat, ::SubjectId, Blake2_128Concat, - ClaimHashOf, + CredentialIdOf, CredentialEntryOf, >; - // Reverse map to make sure that the same claim hash cannot be issued to two - // different subjects by issuing it to subject #1, then removing it only from - // the attestation pallet and then issuing it to subject #2. - // This map ensures that at any time a claim hash is only linked (i.e., issued) - // to a single subject. + // This map ensures that at any time a credential is only linked (i.e., issued) + // to a single subject, as it maps from a credential ID to the subject it was issued to. // Not exposed to the outside world. #[pallet::storage] - #[pallet::getter(fn attested_claim_hashes)] - pub(crate) type CredentialsUnicityIndex = - StorageMap<_, Blake2_128Concat, ClaimHashOf, ::SubjectId>; + #[pallet::getter(fn get_credential_subject)] + pub(crate) type CredentialSubjects = + StorageMap<_, Blake2_128Concat, CredentialIdOf, ::SubjectId>; /// The events generated by this pallet. #[pallet::event] @@ -187,15 +178,15 @@ pub mod pallet { CredentialStored { /// The subject of the new credential. subject_id: T::SubjectId, - /// The root hash of the new credential. - credential_id: ClaimHashOf, + /// The id of the new credential. + credential_id: CredentialIdOf, }, /// A public credentials has been removed. CredentialRemoved { /// The subject of the removed credential. subject_id: T::SubjectId, - /// The root hash of the removed credential. - credential_id: ClaimHashOf, + /// The id of the removed credential. + credential_id: CredentialIdOf, }, } @@ -209,17 +200,10 @@ pub mod pallet { CredentialNotFound, /// Not enough tokens to pay for the fees or the deposit. UnableToPayFees, - /// The credential claimer's information cannot be found, hence the - /// signature cannot be verified. - ClaimerInfoNotFound, - /// The credential claimer's signature is invalid. - InvalidClaimerSignature, /// The credential input is invalid. InvalidInput, - /// The credential exceeds the maximum configured length in bytes. - CredentialTooLong, - /// The subject exceeds the maximum configured length in bytes. - SubjectTooLong, + /// The caller is not authorized to performed the operation. + Unauthorized, /// Catch-all for any other errors that should not happen, yet it /// happened. InternalError, @@ -229,93 +213,55 @@ pub mod pallet { impl Pallet { /// Register a new public credential on chain. /// - /// The preconditions of the `attestation::add()` function must all be - /// fulfilled, as this function calls `attestation:add()` internally. - /// - /// Furthermore, the input must meet the requirements as part of the - /// pallet's configuration, and the attester must be able to pay for the - /// deposit of both the underlying attestation and the public credential - /// info. - /// - /// This function fails if a credential already exists for the specified - /// subject, regardless of the identity of the attester. + /// This function fails if a credential with the same identifier already exists for the specified + /// subject. /// /// Emits `CredentialStored`. #[allow(clippy::boxed_local)] - #[pallet::weight({ - let ac_weight = credential.authorization_info.as_ref().map(|ac| ac.can_attest_weight()).unwrap_or(0); - ::WeightInfo::add(credential.claim.contents.len().saturated_into::()).saturating_add(ac_weight) - })] - pub fn add(origin: OriginFor, credential: Box>) -> DispatchResult { + #[pallet::weight(::WeightInfo::add(credential.claims.len().saturated_into::()))] + pub fn add(origin: OriginFor, credential: InputCredentialOf) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; let attester = source.subject(); let payer = source.sender(); let deposit_amount = ::Deposit::get(); - let attestation_deposit_amount = ::Deposit::get(); let Credential { - claim: Claim { - ctype_hash, - subject, - contents: _, - }, - authorization_info, - } = *credential.clone(); + ctype_hash: _, + subject, + claims: _, + } = credential.clone(); + // Credential ID = H( || ) let credential_id = T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); // Try to decode subject ID to something structured let subject = T::SubjectId::try_from(subject.into_inner()).map_err(|_| Error::::InvalidInput)?; - // Check that the same attestation has not already been issued previously - // (potentially to a different subject) ensure!( - !CredentialsUnicityIndex::::contains_key(&credential_id), + !Credentials::::contains_key(&subject, &credential_id), Error::::CredentialAlreadyIssued ); - // Check that enough funds can be reserved to pay for both attestation and - // public info deposits. - // It is harder to use two potentially different currencies while making sure - // that, if the same, the sum can be reserved, but if they are not, then each - // deposit could be reserved separately. We could switch to using - // `NamedReservableCurrency` to do that and check whether the name of the - // attestation currency matches the name of this pallet currency. - ensure!( - as ReservableCurrency>::can_reserve( - &payer, - deposit_amount.saturating_add(attestation_deposit_amount) - ), - Error::::UnableToPayFees - ); - - // Delegate to the attestation pallet writing the attestation information and - // reserve its part of the deposit - attestation::Pallet::::write_attestation( - ctype_hash, - credential_id, - attester, - payer.clone(), - authorization_info, - )?; - // *** No Fail beyond this point *** // Take the rest of the deposit. Should never fail since we made sure above that // enough funds can be reserved. let deposit = kilt_support::reserve_deposit::>(payer, deposit_amount) - .map_err(|_| Error::::InternalError)?; + .map_err(|_| Error::::UnableToPayFees)?; let block_number = frame_system::Pallet::::block_number(); Credentials::::insert( &subject, &credential_id, - CredentialEntryOf:: { deposit, block_number }, + CredentialEntryOf:: { attester, deposit, block_number }, + ); + CredentialSubjects::::insert( + &credential_id, + subject.clone() ); - CredentialsUnicityIndex::::insert(&credential_id, subject.clone()); Self::deposit_event(Event::CredentialStored { subject_id: subject, @@ -328,13 +274,6 @@ pub mod pallet { /// Removes the information pertaining a public credential from the /// chain. /// - /// The preconditions of the `attestation::remove()` function must all - /// be fulfilled, as this function calls `attestation::remove()` - /// internally. Nevertheless, the opposite is not true. - /// Removing an attestation from the attestation pallet still requires - /// the attester to also remove any traces of the corresponding public - /// credential from this pallet. - /// /// The removal of the credential does not delete it entirely from the /// blockchain history, but only its link *from* the blockchain state /// *to* the blockchain history is removed. @@ -344,30 +283,22 @@ pub mod pallet { /// removed by its attester some time in the past. /// /// This function fails if a credential already exists for the specified - /// subject, regardless of the identity of the attester. + /// subject. /// /// Emits `CredentialRemoved`. - #[pallet::weight({ - let ac_weight = authorization.as_ref().map(|ac| ac.can_attest_weight()).unwrap_or(0); - ::WeightInfo::remove().saturating_add(ac_weight) - })] + #[pallet::weight(::WeightInfo::remove())] pub fn remove( origin: OriginFor, credential_id: CredentialIdOf, - authorization: Option, - ) -> DispatchResultWithPostInfo { + ) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; let attester = source.subject(); let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; - // Delegate to the attestation pallet the removal logic - // This guarantees that the authorized attester is calling this function - let result = attestation::Pallet::::remove_attestation(attester, credential_id, authorization)?; - Self::remove_credential_entry(credential_subject, credential_id, credential_entry); - Ok(result) + Ok(()) } /// Performs the same function as the `remove` extrinsic, with the @@ -385,10 +316,6 @@ pub mod pallet { let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; - // Delegate to the attestation pallet the removal logic. - // This guarantees that the authorized payer is calling this function. - attestation::Pallet::::unlock_deposit(who, credential_id)?; - Self::remove_credential_entry(credential_subject, credential_id, credential_entry); Ok(()) @@ -396,6 +323,20 @@ pub mod pallet { } impl Pallet { + fn retrieve_credential_entry( + credential_id: &CredentialIdOf, + ) -> Result<(T::SubjectId, CredentialEntryOf), Error> { + // Verify that the credential exists + let credential_subject = + CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; + + // Should never happen if the line above succeeds + Credentials::::get(&credential_subject, &credential_id) + .map(|entry| (credential_subject, entry)) + .ok_or(Error::::InternalError) + } + + // Simple wrapper to remove entries from both storages when deleting a // credential. fn remove_credential_entry( @@ -405,25 +346,12 @@ pub mod pallet { ) { kilt_support::free_deposit::>(&credential.deposit); Credentials::::remove(&credential_subject, &credential_id); - CredentialsUnicityIndex::::remove(&credential_id); + CredentialSubjects::::remove(&credential_id); Self::deposit_event(Event::CredentialRemoved { subject_id: credential_subject, credential_id, }); } - - fn retrieve_credential_entry( - credential_id: &CredentialIdOf, - ) -> Result<(T::SubjectId, CredentialEntryOf), Error> { - // Verify that the credential exists - let credential_subject = - CredentialsUnicityIndex::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; - - // Should never happen if the line above succeeds - Credentials::::get(&credential_subject, &credential_id) - .map(|entry| (credential_subject, entry)) - .ok_or(Error::::InternalError) - } } } diff --git a/pallets/public-credentials/src/mock.rs b/pallets/public-credentials/src/mock.rs index 08a1b42e38..3d3b936fb9 100644 --- a/pallets/public-credentials/src/mock.rs +++ b/pallets/public-credentials/src/mock.rs @@ -20,23 +20,19 @@ use codec::Encode; use sp_runtime::traits::Hash; use ctype::CtypeHashOf; -use attestation::AttesterOf; -use crate::{Claim, Config, CredentialIdOf, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; +use crate::{AttesterOf, Config, CredentialIdOf, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; // Generate a public credential using a many Default::default() as possible. pub fn generate_base_public_credential_creation_op( subject_id: InputSubjectIdOf, ctype_hash: CtypeHashOf, - contents: InputClaimsContentOf, + claims: InputClaimsContentOf, ) -> InputCredentialOf { InputCredentialOf:: { - claim: Claim { - ctype_hash, - subject: subject_id, - contents, - }, - authorization_info: Default::default(), + ctype_hash, + subject: subject_id, + claims, } } @@ -74,11 +70,10 @@ pub(crate) mod runtime { signature::EqualVerify, }; - use attestation::{mock::MockAccessControl, AttestationDetails, ClaimHashOf}; use ctype::{CtypeCreatorOf, CtypeHashOf}; use crate::{ - BalanceOf, CredentialEntryOf, Credentials, CredentialsUnicityIndex, CurrencyOf, Error, InputSubjectIdOf, + BalanceOf, CredentialEntryOf, Credentials, CredentialSubjects, CurrencyOf, Error, InputSubjectIdOf, }; pub(crate) type BlockNumber = u64; @@ -145,8 +140,10 @@ pub(crate) mod runtime { pub(crate) fn generate_base_credential_entry( payer: T::AccountId, block_number: ::BlockNumber, + attester: T::AttesterId, ) -> CredentialEntryOf { CredentialEntryOf:: { + attester, block_number, deposit: Deposit::> { owner: payer, @@ -157,7 +154,7 @@ pub(crate) mod runtime { fn insert_public_credentials( subject_id: T::SubjectId, - claim_hash: ClaimHashOf, + credential_id: CredentialIdOf, credential_entry: CredentialEntryOf, ) { kilt_support::reserve_deposit::>( @@ -166,8 +163,8 @@ pub(crate) mod runtime { ) .expect("Attester should have enough balance"); - Credentials::::insert(&subject_id, &claim_hash, credential_entry); - CredentialsUnicityIndex::::insert(claim_hash, subject_id); + Credentials::::insert(&subject_id, &credential_id, credential_entry); + CredentialSubjects::::insert(credential_id, subject_id); } pub(crate) const MILLI_UNIT: Balance = 10u128.pow(12); @@ -179,7 +176,6 @@ pub(crate) mod runtime { UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event}, - Attestation: attestation::{Pallet, Call, Storage, Event}, Ctype: ctype::{Pallet, Call, Storage, Event}, Balances: pallet_balances::{Pallet, Call, Storage, Event}, MockOrigin: mock_origin::{Pallet, Origin}, @@ -245,31 +241,17 @@ pub(crate) mod runtime { type FeeCollector = (); } - impl attestation::Config for Test { - type EnsureOrigin = mock_origin::EnsureDoubleOrigin; - type OriginSuccess = mock_origin::DoubleOrigin; - type Event = (); - type WeightInfo = (); - - type Currency = Balances; - type Deposit = ConstU128<{ 100 * MILLI_UNIT }>; - type MaxDelegatedAttestations = ConstU32<0>; - type AttesterId = SubjectId; - type AuthorizationId = SubjectId; - type AccessControl = MockAccessControl; - } - impl Config for Test { - type ClaimerIdentifier = SubjectId; - type ClaimerSignature = (Self::ClaimerIdentifier, Vec); - type ClaimerSignatureVerification = EqualVerify>; + type AttesterId = SubjectId; + type CredentialId = Hash; type CredentialHash = BlakeTwo256; + type Currency = Balances; type Deposit = ConstU128<{ 10 * MILLI_UNIT }>; - type EnsureOrigin = ::EnsureOrigin; + type EnsureOrigin = mock_origin::EnsureDoubleOrigin; type Event = (); type MaxEncodedClaimsLength = ConstU32<500>; type MaxSubjectIdLength = ConstU32<100>; - type OriginSuccess = ::OriginSuccess; + type OriginSuccess = mock_origin::DoubleOrigin; type SubjectId = TestSubjectId; type WeightInfo = (); } @@ -294,8 +276,7 @@ pub(crate) mod runtime { ctypes: Vec<(CtypeHashOf, CtypeCreatorOf)>, /// endowed accounts with balances balances: Vec<(AccountId, Balance)>, - attestations: Vec<(ClaimHashOf, AttestationDetails)>, - public_credentials: Vec<(::SubjectId, ClaimHashOf, CredentialEntryOf)>, + public_credentials: Vec<(::SubjectId, CredentialIdOf, CredentialEntryOf)>, } impl ExtBuilder { @@ -311,16 +292,10 @@ pub(crate) mod runtime { self } - #[must_use] - pub fn with_attestations(mut self, attestations: Vec<(ClaimHashOf, AttestationDetails)>) -> Self { - self.attestations = attestations; - self - } - #[must_use] pub fn with_public_credentials( mut self, - credentials: Vec<(::SubjectId, ClaimHashOf, CredentialEntryOf)>, + credentials: Vec<(::SubjectId, CredentialIdOf, CredentialEntryOf)>, ) -> Self { self.public_credentials = credentials; self @@ -341,12 +316,8 @@ pub(crate) mod runtime { ctype::Ctypes::::insert(ctype.0, ctype.1.clone()); } - for (claim_hash, details) in self.attestations { - attestation::mock::insert_attestation(claim_hash, details); - } - - for (subject_id, claim_hash, credential_entry) in self.public_credentials { - insert_public_credentials::(subject_id, claim_hash, credential_entry); + for (subject_id, credential_id, credential_entry) in self.public_credentials { + insert_public_credentials::(subject_id, credential_id, credential_entry); } }); diff --git a/pallets/public-credentials/src/tests.rs b/pallets/public-credentials/src/tests.rs index f6edb334d0..2ec1121218 100644 --- a/pallets/public-credentials/src/tests.rs +++ b/pallets/public-credentials/src/tests.rs @@ -19,11 +19,10 @@ use frame_support::{assert_noop, assert_ok, traits::Get}; use sp_runtime::traits::Zero; -use attestation::{attestations::AttestationDetails, mock::generate_base_attestation, Attestations}; use ctype::mock::get_ctype_hash; use kilt_support::mock::mock_origin::DoubleOrigin; -use crate::{mock::*, Config, Credentials, CredentialIdOf, CredentialsUnicityIndex, Error, InputClaimsContentOf}; +use crate::{mock::*, Config, Credentials, CredentialIdOf, CredentialSubjects, Error, InputClaimsContentOf}; // add @@ -45,13 +44,12 @@ fn add_successful() { InputClaimsContentOf::::default(), ); let credential_id_2: CredentialIdOf = generate_credential_id::(&new_credential_2, &attester); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); + let deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![( ACCOUNT_00, - (public_credential_deposit + attestation_deposit) * 2, + (deposit) * 2, )]) .with_ctypes(vec![(ctype_hash_1, attester.clone()), (ctype_hash_2, attester.clone())]) .build() @@ -61,32 +59,26 @@ fn add_successful() { assert_ok!(PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - Box::new(new_credential_1.clone()) + new_credential_1.clone() )); - let stored_attestation = - Attestations::::get(&credential_id_1).expect("Attestation should be present on chain."); let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id_1) .expect("Public credential details should be present on chain."); - // Test interactions with attestation pallet - assert_eq!(stored_attestation.ctype_hash, ctype_hash_1); - assert_eq!(stored_attestation.attester, attester); - // Test this pallet logic assert_eq!(stored_public_credential_details.block_number, 0); - assert_eq!(CredentialsUnicityIndex::::get(&credential_id_1), Some(subject_id)); + assert_eq!(CredentialSubjects::::get(&credential_id_1), Some(subject_id)); // Check deposit reservation logic assert_eq!( Balances::reserved_balance(ACCOUNT_00), - public_credential_deposit + attestation_deposit + deposit ); // Re-issuing the same credential will fail assert_noop!( PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - Box::new(new_credential_1.clone()) + new_credential_1.clone() ), Error::::CredentialAlreadyIssued ); @@ -94,7 +86,7 @@ fn add_successful() { // Check deposit has not changed assert_eq!( Balances::reserved_balance(ACCOUNT_00), - public_credential_deposit + attestation_deposit + deposit ); System::set_block_number(1); @@ -102,43 +94,20 @@ fn add_successful() { // Issuing a completely new credential will work assert_ok!(PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - Box::new(new_credential_2.clone()) + new_credential_2.clone() )); - let stored_attestation = - Attestations::::get(&credential_id_2).expect("Attestation #2 should be present on chain."); let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id_2) .expect("Public credential #2 details should be present on chain."); - // Test interactions with attestation pallet - assert_eq!(stored_attestation.ctype_hash, ctype_hash_2); - assert_eq!(stored_attestation.attester, attester); - // Test this pallet logic assert_eq!(stored_public_credential_details.block_number, 1); - assert_eq!(CredentialsUnicityIndex::::get(&credential_id_2), Some(subject_id)); + assert_eq!(CredentialSubjects::::get(&credential_id_2), Some(subject_id)); // Deposit is 2x now assert_eq!( Balances::reserved_balance(ACCOUNT_00), - 2 * (public_credential_deposit + attestation_deposit) - ); - - // Deleting the attestation only from the attestation pallet will still fail - Attestation::reclaim_deposit(Origin::signed(ACCOUNT_00), credential_id_1) - .expect("Attestation deposit reclaim should not fail"); - assert_noop!( - PublicCredentials::add( - DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - Box::new(new_credential_1.clone()) - ), - Error::::CredentialAlreadyIssued - ); - - // Deposit should now be equal to 1 attestation + 2 public credentials - assert_eq!( - Balances::reserved_balance(ACCOUNT_00), - 2 * public_credential_deposit + attestation_deposit + 2 * deposit ); }); } @@ -153,19 +122,18 @@ fn add_not_enough_balance() { ctype_hash, InputClaimsContentOf::::default(), ); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); + let deposit: Balance = ::Deposit::get(); ExtBuilder::default() // One less than the minimum required - .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit - 1)]) + .with_balances(vec![(ACCOUNT_00, deposit - 1)]) .with_ctypes(vec![(ctype_hash, attester.clone())]) .build() .execute_with(|| { assert_noop!( PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - Box::new(new_credential.clone()) + new_credential ), Error::::UnableToPayFees ); @@ -182,17 +150,16 @@ fn add_ctype_not_found() { ctype_hash, InputClaimsContentOf::::default(), ); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); + let deposit: Balance = ::Deposit::get(); ExtBuilder::default() - .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) + .with_balances(vec![(ACCOUNT_00, deposit)]) .build() .execute_with(|| { assert_noop!( PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - Box::new(new_credential.clone()) + new_credential ), ctype::Error::::CTypeNotFound ); @@ -205,82 +172,37 @@ fn add_ctype_not_found() { fn remove_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let attestation: AttestationDetails = generate_base_attestation(attester.clone(), ACCOUNT_00); - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone()); let credential_id: CredentialIdOf = CredentialIdOf::::default(); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); + let deposit: Balance = ::Deposit::get(); ExtBuilder::default() - .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) - .with_ctypes(vec![(attestation.ctype_hash, attester.clone())]) - .with_attestations(vec![(credential_id, attestation.clone())]) + .with_balances(vec![(ACCOUNT_00, deposit)]) .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { assert_ok!(PublicCredentials::remove( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, - None )); - // Test interactions with attestation pallet - assert_eq!(Attestations::::get(&credential_id), None); // Test this pallet logic assert_eq!(Credentials::::get(&subject_id, &credential_id), None); - assert_eq!(CredentialsUnicityIndex::::get(&credential_id), None); + assert_eq!(CredentialSubjects::::get(&credential_id), None); // Check deposit release logic assert!(Balances::reserved_balance(ACCOUNT_00).is_zero()); // Removing the same credential again will fail assert_noop!( - PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, None), + PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id), Error::::CredentialNotFound ); - // Adding only the attestation without the credential will also fail to remove - // the credential. - Attestation::add( - DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - credential_id, - attestation.ctype_hash, - None, - ) - .expect("Adding the same attestation again should not fail"); - assert_noop!( - PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, None), + PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id), Error::::CredentialNotFound ); - - // Check that only the attestation deposit is reserved - assert_eq!(Balances::reserved_balance(ACCOUNT_00), attestation_deposit); - }); -} - -#[test] -fn remove_unauthorized() { - let attester = sr25519_did_from_seed(&ALICE_SEED); - let wrong_attester = sr25519_did_from_seed(&BOB_SEED); - let subject_id: ::SubjectId = SUBJECT_ID_00; - let attestation: AttestationDetails = generate_base_attestation(attester.clone(), ACCOUNT_00); - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0); - let credential_id: CredentialIdOf = CredentialIdOf::::default(); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); - - ExtBuilder::default() - .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) - .with_ctypes(vec![(attestation.ctype_hash, attester)]) - .with_attestations(vec![(credential_id, attestation)]) - .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) - .build() - .execute_with(|| { - assert_noop!( - PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, wrong_attester).into(), credential_id, None), - attestation::Error::::Unauthorized - ); }); } @@ -290,16 +212,12 @@ fn remove_unauthorized() { fn reclaim_deposit_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let attestation: AttestationDetails = generate_base_attestation(attester.clone(), ACCOUNT_00); - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); let credential_id: CredentialIdOf = CredentialIdOf::::default(); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); + let deposit: Balance = ::Deposit::get(); ExtBuilder::default() - .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) - .with_ctypes(vec![(attestation.ctype_hash, attester.clone())]) - .with_attestations(vec![(credential_id, attestation.clone())]) + .with_balances(vec![(ACCOUNT_00, deposit)]) .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { @@ -307,12 +225,10 @@ fn reclaim_deposit_successful() { Origin::signed(ACCOUNT_00), credential_id )); - // Test interactions with attestation pallet - assert_eq!(Attestations::::get(&credential_id), None); // Test this pallet logic assert_eq!(Credentials::::get(&subject_id, &credential_id), None); - assert_eq!(CredentialsUnicityIndex::::get(&credential_id), None); + assert_eq!(CredentialSubjects::::get(&credential_id), None); // Check deposit release logic assert!(Balances::reserved_balance(ACCOUNT_00).is_zero()); @@ -323,46 +239,9 @@ fn reclaim_deposit_successful() { Error::::CredentialNotFound ); - // Adding only the attestation without the credential will also fail to reclaim - // the deposit for the credential. - Attestation::add( - DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - credential_id, - attestation.ctype_hash, - None, - ) - .expect("Adding the same attestation again should not fail"); - assert_noop!( PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_00), credential_id), Error::::CredentialNotFound ); - - // Check that only the attestation deposit is reserved - assert_eq!(Balances::reserved_balance(ACCOUNT_00), attestation_deposit); - }); -} - -#[test] -fn reclaim_deposit_unauthorized() { - let attester = sr25519_did_from_seed(&ALICE_SEED); - let subject_id: ::SubjectId = SUBJECT_ID_00; - let attestation: AttestationDetails = generate_base_attestation(attester.clone(), ACCOUNT_00); - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0); - let credential_id: CredentialIdOf = CredentialIdOf::::default(); - let public_credential_deposit: Balance = ::Deposit::get(); - let attestation_deposit: Balance = ::Deposit::get(); - - ExtBuilder::default() - .with_balances(vec![(ACCOUNT_00, public_credential_deposit + attestation_deposit)]) - .with_ctypes(vec![(attestation.ctype_hash, attester)]) - .with_attestations(vec![(credential_id, attestation)]) - .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) - .build() - .execute_with(|| { - assert_noop!( - PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_01), credential_id), - attestation::Error::::Unauthorized - ); }); } From 974a16e36adfdb0adc3b7d925d56d0c3d9e1a6f5 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Tue, 2 Aug 2022 16:28:02 +0200 Subject: [PATCH 04/21] add revocation and unrevocation --- pallets/public-credentials/src/credentials.rs | 2 + pallets/public-credentials/src/lib.rs | 97 +++++++++++++++---- pallets/public-credentials/src/mock.rs | 20 ++-- pallets/public-credentials/src/tests.rs | 87 ++++++++++++++--- 4 files changed, 165 insertions(+), 41 deletions(-) diff --git a/pallets/public-credentials/src/credentials.rs b/pallets/public-credentials/src/credentials.rs index 86d11d3b2a..520b0bf01e 100644 --- a/pallets/public-credentials/src/credentials.rs +++ b/pallets/public-credentials/src/credentials.rs @@ -44,6 +44,8 @@ pub struct Credential { pub struct CredentialEntry { /// The attester of the credential. pub attester: Attester, + /// A flag indicating the revocation status of the credential + pub revoked: bool, /// The block number in which the credential tx was evaluated and included /// in the block. pub block_number: BlockNumber, diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index 51e55f1c72..a2bacf734f 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -69,9 +69,7 @@ pub mod pallet { use sp_std::vec::Vec; use ctype::CtypeHashOf; - use kilt_support::{ - traits::CallSources, - }; + use kilt_support::traits::CallSources; /// The current storage version. const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); @@ -99,11 +97,7 @@ pub mod pallet { pub(crate) type CredentialIdOf = <::CredentialHash as sp_runtime::traits::Hash>::Output; /// The type of a public credential as the pallet expects it. - pub type InputCredentialOf = Credential< - CtypeHashOf, - InputSubjectIdOf, - InputClaimsContentOf, - >; + pub type InputCredentialOf = Credential, InputSubjectIdOf, InputClaimsContentOf>; #[pallet::config] pub trait Config: frame_system::Config + ctype::Config { @@ -188,6 +182,16 @@ pub mod pallet { /// The id of the removed credential. credential_id: CredentialIdOf, }, + /// A public credentials has been revoked. + CredentialRevoked { + /// The id of the revoked credential. + credential_id: CredentialIdOf, + }, + /// A public credentials has been unrevoked. + CredentialUnrevoked { + /// The id of the unrevoked credential. + credential_id: CredentialIdOf, + }, } #[pallet::error] @@ -234,7 +238,8 @@ pub mod pallet { } = credential.clone(); // Credential ID = H( || ) - let credential_id = T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); + let credential_id = + T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); // Try to decode subject ID to something structured let subject = T::SubjectId::try_from(subject.into_inner()).map_err(|_| Error::::InvalidInput)?; @@ -256,12 +261,14 @@ pub mod pallet { Credentials::::insert( &subject, &credential_id, - CredentialEntryOf:: { attester, deposit, block_number }, - ); - CredentialSubjects::::insert( - &credential_id, - subject.clone() + CredentialEntryOf:: { + revoked: false, + attester, + deposit, + block_number, + }, ); + CredentialSubjects::::insert(&credential_id, subject.clone()); Self::deposit_event(Event::CredentialStored { subject_id: subject, @@ -287,10 +294,7 @@ pub mod pallet { /// /// Emits `CredentialRemoved`. #[pallet::weight(::WeightInfo::remove())] - pub fn remove( - origin: OriginFor, - credential_id: CredentialIdOf, - ) -> DispatchResult { + pub fn remove(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; let attester = source.subject(); @@ -301,6 +305,62 @@ pub mod pallet { Ok(()) } + /// Revokes a public credential. + /// + /// If a credential was already revoked, this function does not fail but simply results + /// in a noop. + /// + /// Emits `CredentialRevoked`. + #[pallet::weight(0)] + pub fn revoke(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { + let source = ::EnsureOrigin::ensure_origin(origin)?; + let attester = source.subject(); + + let credential_subject = + CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; + + Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { + if let Some(credential) = credential_entry { + credential.revoked = true; + Ok(()) + } else { + Err(DispatchError::from(Error::::CredentialNotFound)) + } + })?; + + Self::deposit_event(Event::CredentialRevoked { credential_id }); + + Ok(()) + } + + /// Unrevokes a public credential. + /// + /// If a credential was not revoked, this function does not fail but simply results + /// in a noop. + /// + /// Emits `CredentialUnrevoked`. + #[pallet::weight(0)] + pub fn unrevoke(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { + let source = ::EnsureOrigin::ensure_origin(origin)?; + let attester = source.subject(); + + let credential_subject = + CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; + + Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { + if let Some(credential) = credential_entry { + credential.revoked = false; + Ok(()) + } else { + Err(DispatchError::from(Error::::CredentialNotFound)) + } + })?; + + Self::deposit_event(Event::CredentialUnrevoked { credential_id }); + + Ok(()) + } + /// Performs the same function as the `remove` extrinsic, with the /// difference that the caller of this function must be the original /// payer of the deposit, and not the original attester of the @@ -336,7 +396,6 @@ pub mod pallet { .ok_or(Error::::InternalError) } - // Simple wrapper to remove entries from both storages when deleting a // credential. fn remove_credential_entry( diff --git a/pallets/public-credentials/src/mock.rs b/pallets/public-credentials/src/mock.rs index 3d3b936fb9..961de17d9c 100644 --- a/pallets/public-credentials/src/mock.rs +++ b/pallets/public-credentials/src/mock.rs @@ -38,7 +38,7 @@ pub fn generate_base_public_credential_creation_op( pub fn generate_credential_id( input_credential: &InputCredentialOf, - attester: &AttesterOf + attester: &AttesterOf, ) -> CredentialIdOf { T::CredentialHash::hash(&[&input_credential.encode()[..], &attester.encode()[..]].concat()[..]) } @@ -67,14 +67,11 @@ pub(crate) mod runtime { use kilt_support::{ deposit::Deposit, mock::{mock_origin, SubjectId}, - signature::EqualVerify, }; use ctype::{CtypeCreatorOf, CtypeHashOf}; - use crate::{ - BalanceOf, CredentialEntryOf, Credentials, CredentialSubjects, CurrencyOf, Error, InputSubjectIdOf, - }; + use crate::{BalanceOf, CredentialEntryOf, CredentialSubjects, Credentials, CurrencyOf, Error, InputSubjectIdOf}; pub(crate) type BlockNumber = u64; pub(crate) type Balance = u128; @@ -143,6 +140,7 @@ pub(crate) mod runtime { attester: T::AttesterId, ) -> CredentialEntryOf { CredentialEntryOf:: { + revoked: false, attester, block_number, deposit: Deposit::> { @@ -276,7 +274,11 @@ pub(crate) mod runtime { ctypes: Vec<(CtypeHashOf, CtypeCreatorOf)>, /// endowed accounts with balances balances: Vec<(AccountId, Balance)>, - public_credentials: Vec<(::SubjectId, CredentialIdOf, CredentialEntryOf)>, + public_credentials: Vec<( + ::SubjectId, + CredentialIdOf, + CredentialEntryOf, + )>, } impl ExtBuilder { @@ -295,7 +297,11 @@ pub(crate) mod runtime { #[must_use] pub fn with_public_credentials( mut self, - credentials: Vec<(::SubjectId, CredentialIdOf, CredentialEntryOf)>, + credentials: Vec<( + ::SubjectId, + CredentialIdOf, + CredentialEntryOf, + )>, ) -> Self { self.public_credentials = credentials; self diff --git a/pallets/public-credentials/src/tests.rs b/pallets/public-credentials/src/tests.rs index 2ec1121218..9e87380e48 100644 --- a/pallets/public-credentials/src/tests.rs +++ b/pallets/public-credentials/src/tests.rs @@ -65,6 +65,8 @@ fn add_successful() { .expect("Public credential details should be present on chain."); // Test this pallet logic + assert_eq!(stored_public_credential_details.attester, attester); + assert!(!stored_public_credential_details.revoked); assert_eq!(stored_public_credential_details.block_number, 0); assert_eq!(CredentialSubjects::::get(&credential_id_1), Some(subject_id)); @@ -101,6 +103,8 @@ fn add_successful() { .expect("Public credential #2 details should be present on chain."); // Test this pallet logic + assert_eq!(stored_public_credential_details.attester, attester); + assert!(!stored_public_credential_details.revoked); assert_eq!(stored_public_credential_details.block_number, 1); assert_eq!(CredentialSubjects::::get(&credential_id_2), Some(subject_id)); @@ -140,29 +144,82 @@ fn add_not_enough_balance() { }); } +// revoke + #[test] -fn add_ctype_not_found() { +fn revoke_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); - let subject_id = SUBJECT_ID_00; - let ctype_hash = get_ctype_hash::(true); - let new_credential = generate_base_public_credential_creation_op::( - subject_id.into(), - ctype_hash, - InputClaimsContentOf::::default(), - ); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone()); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { - assert_noop!( - PublicCredentials::add( - DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - new_credential - ), - ctype::Error::::CTypeNotFound - ); + assert_ok!(PublicCredentials::revoke( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + credential_id, + )); + + let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id) + .expect("Public credential details should be present on chain."); + + // Test this pallet logic + assert!(stored_public_credential_details.revoked); + + // Revoking the same credential does nothing + assert_ok!(PublicCredentials::revoke( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + credential_id, + )); + + let stored_public_credential_details_2 = Credentials::::get(&subject_id, &credential_id) + .expect("Public credential details should be present on chain."); + + assert_eq!(stored_public_credential_details, stored_public_credential_details_2); + }); +} + +// revoke + +#[test] +fn unrevoke_successful() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone()); + new_credential.revoked = true; + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) + .build() + .execute_with(|| { + assert_ok!(PublicCredentials::unrevoke( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + credential_id, + )); + + let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id) + .expect("Public credential details should be present on chain."); + + // Test this pallet logic + assert!(!stored_public_credential_details.revoked); + + // Unrevoking the same credential does nothing + assert_ok!(PublicCredentials::unrevoke( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + credential_id, + )); + + let stored_public_credential_details_2 = Credentials::::get(&subject_id, &credential_id) + .expect("Public credential details should be present on chain."); + + assert_eq!(stored_public_credential_details, stored_public_credential_details_2); }); } From eb25d721538606cc7d82544fcfeadaa092b3d6cc Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Tue, 2 Aug 2022 16:55:38 +0200 Subject: [PATCH 05/21] add checks that the attestation pallet also performs --- pallets/public-credentials/src/lib.rs | 108 ++++++++------ pallets/public-credentials/src/tests.rs | 179 +++++++++++++++++++++++- 2 files changed, 244 insertions(+), 43 deletions(-) diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index a2bacf734f..74d813d424 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -232,11 +232,16 @@ pub mod pallet { let deposit_amount = ::Deposit::get(); let Credential { - ctype_hash: _, + ctype_hash, subject, claims: _, } = credential.clone(); + ensure!( + ctype::Ctypes::::contains_key(&ctype_hash), + ctype::Error::::CTypeNotFound + ); + // Credential ID = H( || ) let credential_id = T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); @@ -249,13 +254,13 @@ pub mod pallet { Error::::CredentialAlreadyIssued ); - // *** No Fail beyond this point *** - // Take the rest of the deposit. Should never fail since we made sure above that // enough funds can be reserved. let deposit = kilt_support::reserve_deposit::>(payer, deposit_amount) .map_err(|_| Error::::UnableToPayFees)?; + // *** No Fail beyond this point *** + let block_number = frame_system::Pallet::::block_number(); Credentials::::insert( @@ -278,51 +283,31 @@ pub mod pallet { Ok(()) } - /// Removes the information pertaining a public credential from the - /// chain. - /// - /// The removal of the credential does not delete it entirely from the - /// blockchain history, but only its link *from* the blockchain state - /// *to* the blockchain history is removed. - /// - /// Clients parsing public credentials should interpret - /// the lack of such a link as the fact that the credential has been - /// removed by its attester some time in the past. - /// - /// This function fails if a credential already exists for the specified - /// subject. - /// - /// Emits `CredentialRemoved`. - #[pallet::weight(::WeightInfo::remove())] - pub fn remove(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { - let source = ::EnsureOrigin::ensure_origin(origin)?; - let attester = source.subject(); - - let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; - - Self::remove_credential_entry(credential_subject, credential_id, credential_entry); - - Ok(()) - } - /// Revokes a public credential. /// /// If a credential was already revoked, this function does not fail but simply results /// in a noop. /// + /// Only the original attester can revoke the credential. + /// /// Emits `CredentialRevoked`. #[pallet::weight(0)] pub fn revoke(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; - let attester = source.subject(); + let caller = source.subject(); let credential_subject = CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; + // Fails if the credential does not exist OR the caller is different than the original attester. Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { if let Some(credential) = credential_entry { - credential.revoked = true; - Ok(()) + if caller != credential.attester { + Err(DispatchError::from(Error::::Unauthorized)) + } else { + credential.revoked = true; + Ok(()) + } } else { Err(DispatchError::from(Error::::CredentialNotFound)) } @@ -338,19 +323,26 @@ pub mod pallet { /// If a credential was not revoked, this function does not fail but simply results /// in a noop. /// + /// Only the original attester can unrevoke the credential. + /// /// Emits `CredentialUnrevoked`. #[pallet::weight(0)] pub fn unrevoke(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; - let attester = source.subject(); + let caller = source.subject(); let credential_subject = CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; + // Fails if the credential does not exist OR the caller is different than the original attester. Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { if let Some(credential) = credential_entry { - credential.revoked = false; - Ok(()) + if caller != credential.attester { + Err(DispatchError::from(Error::::Unauthorized)) + } else { + credential.revoked = false; + Ok(()) + } } else { Err(DispatchError::from(Error::::CredentialNotFound)) } @@ -361,21 +353,53 @@ pub mod pallet { Ok(()) } + /// Removes the information pertaining a public credential from the + /// chain. + /// + /// The removal of the credential does not delete it entirely from the + /// blockchain history, but only its link *from* the blockchain state + /// *to* the blockchain history is removed. + /// + /// Clients parsing public credentials should interpret + /// the lack of such a link as the fact that the credential has been + /// removed by its attester some time in the past. + /// + /// This function fails if a credential already exists for the specified + /// subject. + /// + /// Emits `CredentialRemoved`. + #[pallet::weight(::WeightInfo::remove())] + pub fn remove(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { + let source = ::EnsureOrigin::ensure_origin(origin)?; + let caller = source.subject(); + + let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; + + ensure!( + caller == credential_entry.attester, + Error::::Unauthorized + ); + + Self::remove_credential_entry(credential_subject, credential_id, credential_entry); + + Ok(()) + } + /// Performs the same function as the `remove` extrinsic, with the /// difference that the caller of this function must be the original /// payer of the deposit, and not the original attester of the /// credential. - /// - /// It calls `attestation::reclaim_deposit()` internally, nevertheless - /// the opposite is not true. Removing an attestation from the - /// attestation pallet still requires the deposit payer to also remove - /// any traces of the corresponding public credential from this pallet. #[pallet::weight(::WeightInfo::reclaim_deposit())] pub fn reclaim_deposit(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { - let who = ensure_signed(origin)?; + let submitter = ensure_signed(origin)?; let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; + ensure!( + submitter == credential_entry.deposit.owner, + Error::::Unauthorized + ); + Self::remove_credential_entry(credential_subject, credential_id, credential_entry); Ok(()) diff --git a/pallets/public-credentials/src/tests.rs b/pallets/public-credentials/src/tests.rs index 9e87380e48..52848569d4 100644 --- a/pallets/public-credentials/src/tests.rs +++ b/pallets/public-credentials/src/tests.rs @@ -116,6 +116,33 @@ fn add_successful() { }); } +#[test] +fn add_ctype_not_existing() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let subject_id = SUBJECT_ID_00; + let ctype_hash = get_ctype_hash::(true); + let new_credential = generate_base_public_credential_creation_op::( + subject_id.into(), + ctype_hash, + InputClaimsContentOf::::default(), + ); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + // One less than the minimum required + .with_balances(vec![(ACCOUNT_00, deposit - 1)]) + .build() + .execute_with(|| { + assert_noop!( + PublicCredentials::add( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + new_credential + ), + ctype::Error::::CTypeNotFound + ); + }); +} + #[test] fn add_not_enough_balance() { let attester = sr25519_did_from_seed(&ALICE_SEED); @@ -183,7 +210,45 @@ fn revoke_successful() { }); } -// revoke +#[test] +fn revoke_credential_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .build() + .execute_with(|| { + assert_noop!(PublicCredentials::revoke( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + credential_id, + ), Error::::CredentialNotFound); + }); +} + +#[test] +fn revoke_unauthorised() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_attester = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) + .build() + .execute_with(|| { + assert_noop!(PublicCredentials::revoke( + DoubleOrigin(ACCOUNT_00, wrong_attester.clone()).into(), + credential_id, + ), Error::::Unauthorized); + }); +} + +// unrevoke #[test] fn unrevoke_successful() { @@ -223,6 +288,44 @@ fn unrevoke_successful() { }); } +#[test] +fn unrevoke_credential_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .build() + .execute_with(|| { + assert_noop!(PublicCredentials::unrevoke( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + credential_id, + ), Error::::CredentialNotFound); + }); +} + +#[test] +fn unrevoke_unauthorised() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_attester = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) + .build() + .execute_with(|| { + assert_noop!(PublicCredentials::unrevoke( + DoubleOrigin(ACCOUNT_00, wrong_attester.clone()).into(), + credential_id, + ), Error::::Unauthorized); + }); +} + // remove #[test] @@ -263,6 +366,44 @@ fn remove_successful() { }); } +#[test] +fn remove_credential_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .build() + .execute_with(|| { + assert_noop!(PublicCredentials::remove( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + credential_id, + ), Error::::CredentialNotFound); + }); +} + +#[test] +fn remove_unauthorized() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_attester = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) + .build() + .execute_with(|| { + assert_noop!(PublicCredentials::remove( + DoubleOrigin(ACCOUNT_00, wrong_attester).into(), + credential_id, + ), Error::::Unauthorized); + }); +} + // reclaim_deposit #[test] @@ -302,3 +443,39 @@ fn reclaim_deposit_successful() { ); }); } + +#[test] +fn reclaim_deposit_credential_not_found() { + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .build() + .execute_with(|| { + assert_noop!(PublicCredentials::reclaim_deposit( + Origin::signed(ACCOUNT_00), + credential_id + ), Error::::CredentialNotFound); + }); +} + +#[test] +fn reclaim_deposit_unauthorized() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) + .build() + .execute_with(|| { + assert_noop!(PublicCredentials::reclaim_deposit( + Origin::signed(ACCOUNT_01), + credential_id + ), Error::::Unauthorized); + }); +} From 6f5439d29e1a360bd38b3482ba0405c1e101d256 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Tue, 2 Aug 2022 17:04:43 +0200 Subject: [PATCH 06/21] add ctype info in the storage --- pallets/public-credentials/src/credentials.rs | 4 +++- pallets/public-credentials/src/lib.rs | 5 +++-- pallets/public-credentials/src/mock.rs | 6 +++--- pallets/public-credentials/src/tests.rs | 18 ++++++++++-------- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/pallets/public-credentials/src/credentials.rs b/pallets/public-credentials/src/credentials.rs index 520b0bf01e..7b82f9a38f 100644 --- a/pallets/public-credentials/src/credentials.rs +++ b/pallets/public-credentials/src/credentials.rs @@ -41,7 +41,9 @@ pub struct Credential { /// block. The block number is used to query the full content of the credential /// from archive nodes. #[derive(Encode, Decode, Clone, MaxEncodedLen, RuntimeDebug, PartialEq, Eq, PartialOrd, Ord, TypeInfo)] -pub struct CredentialEntry { +pub struct CredentialEntry { + /// The hash of the CType used for this attestation. + pub ctype_hash: CTypeHash, /// The attester of the credential. pub attester: Attester, /// A flag indicating the revocation status of the credential diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index 74d813d424..b6d12958a7 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -68,7 +68,7 @@ pub mod pallet { use sp_runtime::traits::{Hash, SaturatedConversion}; use sp_std::vec::Vec; - use ctype::CtypeHashOf; + pub use ctype::CtypeHashOf; use kilt_support::traits::CallSources; /// The current storage version. @@ -89,7 +89,7 @@ pub mod pallet { pub type InputClaimsContentOf = BoundedVec::MaxEncodedClaimsLength>; pub type AccountIdOf = ::AccountId; pub type BlockNumberOf = ::BlockNumber; - pub type CredentialEntryOf = CredentialEntry, BlockNumberOf, AccountIdOf, BalanceOf>; + pub type CredentialEntryOf = CredentialEntry, AttesterOf, BlockNumberOf, AccountIdOf, BalanceOf>; /// Type of an attester identifier. pub type AttesterOf = ::AttesterId; /// The type of account's balances. @@ -271,6 +271,7 @@ pub mod pallet { attester, deposit, block_number, + ctype_hash, }, ); CredentialSubjects::::insert(&credential_id, subject.clone()); diff --git a/pallets/public-credentials/src/mock.rs b/pallets/public-credentials/src/mock.rs index 961de17d9c..e904660cb4 100644 --- a/pallets/public-credentials/src/mock.rs +++ b/pallets/public-credentials/src/mock.rs @@ -19,9 +19,7 @@ use codec::Encode; use sp_runtime::traits::Hash; -use ctype::CtypeHashOf; - -use crate::{AttesterOf, Config, CredentialIdOf, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; +use crate::{AttesterOf, Config, CredentialIdOf, CtypeHashOf, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; // Generate a public credential using a many Default::default() as possible. pub fn generate_base_public_credential_creation_op( @@ -138,8 +136,10 @@ pub(crate) mod runtime { payer: T::AccountId, block_number: ::BlockNumber, attester: T::AttesterId, + ctype_hash: Option> ) -> CredentialEntryOf { CredentialEntryOf:: { + ctype_hash: ctype_hash.unwrap_or_default(), revoked: false, attester, block_number, diff --git a/pallets/public-credentials/src/tests.rs b/pallets/public-credentials/src/tests.rs index 52848569d4..f97f8f9050 100644 --- a/pallets/public-credentials/src/tests.rs +++ b/pallets/public-credentials/src/tests.rs @@ -68,6 +68,7 @@ fn add_successful() { assert_eq!(stored_public_credential_details.attester, attester); assert!(!stored_public_credential_details.revoked); assert_eq!(stored_public_credential_details.block_number, 0); + assert_eq!(stored_public_credential_details.ctype_hash, ctype_hash_1); assert_eq!(CredentialSubjects::::get(&credential_id_1), Some(subject_id)); // Check deposit reservation logic @@ -106,6 +107,7 @@ fn add_successful() { assert_eq!(stored_public_credential_details.attester, attester); assert!(!stored_public_credential_details.revoked); assert_eq!(stored_public_credential_details.block_number, 1); + assert_eq!(stored_public_credential_details.ctype_hash, ctype_hash_2); assert_eq!(CredentialSubjects::::get(&credential_id_2), Some(subject_id)); // Deposit is 2x now @@ -177,7 +179,7 @@ fn add_not_enough_balance() { fn revoke_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone()); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -232,7 +234,7 @@ fn revoke_unauthorised() { let attester = sr25519_did_from_seed(&ALICE_SEED); let wrong_attester = sr25519_did_from_seed(&BOB_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester, None); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -254,7 +256,7 @@ fn revoke_unauthorised() { fn unrevoke_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone()); + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); new_credential.revoked = true; let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -310,7 +312,7 @@ fn unrevoke_unauthorised() { let attester = sr25519_did_from_seed(&ALICE_SEED); let wrong_attester = sr25519_did_from_seed(&BOB_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester, None); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -332,7 +334,7 @@ fn unrevoke_unauthorised() { fn remove_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone()); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -388,7 +390,7 @@ fn remove_unauthorized() { let attester = sr25519_did_from_seed(&ALICE_SEED); let wrong_attester = sr25519_did_from_seed(&BOB_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester, None); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -410,7 +412,7 @@ fn remove_unauthorized() { fn reclaim_deposit_successful() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester, None); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -464,7 +466,7 @@ fn reclaim_deposit_credential_not_found() { fn reclaim_deposit_unauthorized() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester); + let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester, None); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); From dc1de22fd6159b539ca2d3113a92c4d99d355caa Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 09:31:44 +0200 Subject: [PATCH 07/21] Add support for access control --- .../public-credentials/src/access_control.rs | 130 +++++++ pallets/public-credentials/src/credentials.rs | 10 +- pallets/public-credentials/src/lib.rs | 135 +++++-- pallets/public-credentials/src/mock.rs | 93 ++++- pallets/public-credentials/src/tests.rs | 366 ++++++++++++++---- 5 files changed, 634 insertions(+), 100 deletions(-) create mode 100644 pallets/public-credentials/src/access_control.rs diff --git a/pallets/public-credentials/src/access_control.rs b/pallets/public-credentials/src/access_control.rs new file mode 100644 index 0000000000..5095049820 --- /dev/null +++ b/pallets/public-credentials/src/access_control.rs @@ -0,0 +1,130 @@ +// KILT Blockchain – https://botlabs.org +// Copyright (C) 2019-2022 BOTLabs GmbH + +// The KILT Blockchain is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// The KILT Blockchain is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// 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 frame_support::dispatch::Weight; +use sp_runtime::DispatchError; + +pub trait AccessControl { + /// Decides whether the account is allowed to issue a credential with the given + /// information provided by the sender (&self). + fn can_issue(&self, who: &AttesterId, ctype: &Ctype, credential_id: &CredentialId) -> Result; + + /// Decides whether the account is allowed to revoke the credential with + /// the `authorization_id` and the access information provided by the sender + /// (&self). + fn can_revoke( + &self, + who: &AttesterId, + ctype: &Ctype, + credential_id: &CredentialId, + authorization_id: &AuthorizationId, + ) -> Result; + + /// Decides whether the account is allowed to revoke the credential with + /// the `authorization_id` and the access information provided by the sender + /// (&self). + fn can_unrevoke( + &self, + who: &AttesterId, + ctype: &Ctype, + credential_id: &CredentialId, + authorization_id: &AuthorizationId, + ) -> Result; + + /// Decides whether the account is allowed to remove the credential with + /// the `authorization_id` and the access information provided by the sender + /// (&self). + fn can_remove( + &self, + who: &AttesterId, + ctype: &Ctype, + credential_id: &CredentialId, + authorization_id: &AuthorizationId, + ) -> Result; + + /// The authorization ID that the sender provided. This will be used for new + /// credentials. + /// + /// NOTE: This method must not read storage or do any heavy computation + /// since it's not covered by the weight returned by `self.weight()`. + fn authorization_id(&self) -> AuthorizationId; + + /// The worst-case weight of `can_issue`. + fn can_issue_weight(&self) -> Weight; + + /// The worst-case weight of `can_revoke`. + fn can_revoke_weight(&self) -> Weight; + + /// The worst-case weight of `can_unrevoke`. + fn can_unrevoke_weight(&self) -> Weight; + + /// The worst-case weight of `can_remove`. + fn can_remove_weight(&self) -> Weight; +} + +impl +AccessControl for () +where + AuthorizationId: Default, +{ + fn can_issue(&self, _who: &AttesterId, _ctype: &Ctype, _claim: &CredentialId) -> Result { + Err(DispatchError::Other("Unimplemented")) + } + fn can_revoke( + &self, + _who: &AttesterId, + _ctype: &Ctype, + _claim: &CredentialId, + _authorization_id: &AuthorizationId, + ) -> Result { + Err(DispatchError::Other("Unimplemented")) + } + fn can_unrevoke( + &self, + _who: &AttesterId, + _ctype: &Ctype, + _claim: &CredentialId, + _authorization_id: &AuthorizationId, + ) -> Result { + Err(DispatchError::Other("Unimplemented")) + } + fn can_remove( + &self, + _who: &AttesterId, + _ctype: &Ctype, + _claim: &CredentialId, + _authorization_id: &AuthorizationId, + ) -> Result { + Err(DispatchError::Other("Unimplemented")) + } + fn authorization_id(&self) -> AuthorizationId { + Default::default() + } + fn can_issue_weight(&self) -> Weight { + 0 + } + fn can_revoke_weight(&self) -> Weight { + 0 + } + fn can_unrevoke_weight(&self) -> Weight { + 0 + } + fn can_remove_weight(&self) -> Weight { + 0 + } +} diff --git a/pallets/public-credentials/src/credentials.rs b/pallets/public-credentials/src/credentials.rs index 7b82f9a38f..9b3313d291 100644 --- a/pallets/public-credentials/src/credentials.rs +++ b/pallets/public-credentials/src/credentials.rs @@ -22,17 +22,18 @@ use scale_info::TypeInfo; use frame_support::RuntimeDebug; use kilt_support::deposit::Deposit; - /// The type of a credentials as incoming from the outside world. /// Some of its fields are parsed and/or transformed inside the `add` operation. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, PartialOrd, Ord, TypeInfo)] -pub struct Credential { +pub struct Credential { /// The Ctype of the credential. pub ctype_hash: CtypeHash, /// The credential subject ID as specified by the attester. pub subject: SubjectIdentifier, /// The credential claims. pub claims: Claims, + /// The access control logic to authorize the creation operation. + pub authorization: Option, } /// The entry in the blockchain state corresponding to a successful public @@ -41,7 +42,7 @@ pub struct Credential { /// block. The block number is used to query the full content of the credential /// from archive nodes. #[derive(Encode, Decode, Clone, MaxEncodedLen, RuntimeDebug, PartialEq, Eq, PartialOrd, Ord, TypeInfo)] -pub struct CredentialEntry { +pub struct CredentialEntry { /// The hash of the CType used for this attestation. pub ctype_hash: CTypeHash, /// The attester of the credential. @@ -53,4 +54,7 @@ pub struct CredentialEntry, + /// The ID of the authorization information (e.g., a delegation node) used to authorize the + /// operation. + pub authorization_id: Option, } diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index b6d12958a7..13793a05af 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -42,6 +42,7 @@ //! functionalities are entirely delegated to the `attestation` pallet. #![cfg_attr(not(feature = "std"), no_std)] +mod access_control; pub mod credentials; pub mod default_weights; @@ -53,7 +54,10 @@ mod mock; #[cfg(test)] mod tests; -pub use crate::{credentials::*, default_weights::WeightInfo, pallet::*}; +pub use crate::{ + access_control::AccessControl as PublicCredentialsAccessControl, credentials::*, default_weights::WeightInfo, + pallet::*, +}; #[frame_support::pallet] pub mod pallet { @@ -89,20 +93,39 @@ pub mod pallet { pub type InputClaimsContentOf = BoundedVec::MaxEncodedClaimsLength>; pub type AccountIdOf = ::AccountId; pub type BlockNumberOf = ::BlockNumber; - pub type CredentialEntryOf = CredentialEntry, AttesterOf, BlockNumberOf, AccountIdOf, BalanceOf>; + pub type CredentialEntryOf = CredentialEntry< + CtypeHashOf, + AttesterOf, + BlockNumberOf, + AccountIdOf, + BalanceOf, + AuthorizationIdOf, + >; /// Type of an attester identifier. pub type AttesterOf = ::AttesterId; /// The type of account's balances. pub type BalanceOf = as Currency>>::Balance; + pub(crate) type AuthorizationIdOf = ::AuthorizationId; pub(crate) type CredentialIdOf = <::CredentialHash as sp_runtime::traits::Hash>::Output; /// The type of a public credential as the pallet expects it. - pub type InputCredentialOf = Credential, InputSubjectIdOf, InputClaimsContentOf>; + pub type InputCredentialOf = + Credential, InputSubjectIdOf, InputClaimsContentOf, ::AccessControl>; #[pallet::config] pub trait Config: frame_system::Config + ctype::Config { + /// The access control logic. + type AccessControl: Parameter + + PublicCredentialsAccessControl< + Self::AttesterId, + Self::AuthorizationId, + CtypeHashOf, + CredentialIdOf, + >; /// The identifier of the credential attester. type AttesterId: Parameter + MaxEncodedLen; + /// The identifier of the authorization info to perform access control for the different operations. + type AuthorizationId: Parameter + MaxEncodedLen; /// The origin allowed to issue/revoke/remove public credentials. type EnsureOrigin: EnsureOrigin::OriginSuccess, Self::Origin>; /// The ubiquitous event type. @@ -222,7 +245,11 @@ pub mod pallet { /// /// Emits `CredentialStored`. #[allow(clippy::boxed_local)] - #[pallet::weight(::WeightInfo::add(credential.claims.len().saturated_into::()))] + #[pallet::weight({ + let xt_weight = ::WeightInfo::add(credential.claims.len().saturated_into::()); + let ac_weight = credential.authorization.as_ref().map(|ac| ac.can_issue_weight()).unwrap_or(0); + xt_weight.saturating_add(ac_weight) + })] pub fn add(origin: OriginFor, credential: InputCredentialOf) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; @@ -235,6 +262,7 @@ pub mod pallet { ctype_hash, subject, claims: _, + authorization, } = credential.clone(); ensure!( @@ -246,6 +274,14 @@ pub mod pallet { let credential_id = T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); + // Check for validity of the authorization info if specified. + authorization + .as_ref() + .map(|ac| ac.can_issue(&attester, &ctype_hash, &credential_id)) + .transpose() + .map_err(|_| Error::::Unauthorized)?; + let authorization_id = authorization.as_ref().map(|ac| ac.authorization_id()); + // Try to decode subject ID to something structured let subject = T::SubjectId::try_from(subject.into_inner()).map_err(|_| Error::::InvalidInput)?; @@ -272,6 +308,7 @@ pub mod pallet { deposit, block_number, ctype_hash, + authorization_id, }, ); CredentialSubjects::::insert(&credential_id, subject.clone()); @@ -289,11 +326,19 @@ pub mod pallet { /// If a credential was already revoked, this function does not fail but simply results /// in a noop. /// - /// Only the original attester can revoke the credential. + /// Only authorized callers can revoke the credential. /// /// Emits `CredentialRevoked`. - #[pallet::weight(0)] - pub fn revoke(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { + #[pallet::weight({ + let xt_weight = ::WeightInfo::revoke(); + let ac_weight = credential.authorization.as_ref().map(|ac| ac.can_revoke_weight()).unwrap_or(0); + xt_weight.saturating_add(ac_weight) + })] + pub fn revoke( + origin: OriginFor, + credential_id: CredentialIdOf, + authorization: Option, + ) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; let caller = source.subject(); @@ -304,11 +349,15 @@ pub mod pallet { Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { if let Some(credential) = credential_entry { if caller != credential.attester { - Err(DispatchError::from(Error::::Unauthorized)) - } else { - credential.revoked = true; - Ok(()) + let credential_auth_id = + credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; + authorization + .ok_or(Error::::Unauthorized)? + .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) + .map_err(|_| Error::::Unauthorized)?; } + credential.revoked = true; + Ok(()) } else { Err(DispatchError::from(Error::::CredentialNotFound)) } @@ -324,11 +373,19 @@ pub mod pallet { /// If a credential was not revoked, this function does not fail but simply results /// in a noop. /// - /// Only the original attester can unrevoke the credential. + /// Only authorized callers can unrevoke the credential. /// /// Emits `CredentialUnrevoked`. - #[pallet::weight(0)] - pub fn unrevoke(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { + #[pallet::weight({ + let xt_weight = ::WeightInfo::unrevoke(); + let ac_weight = credential.authorization.as_ref().map(|ac| ac.can_unrevoke_weight()).unwrap_or(0); + xt_weight.saturating_add(ac_weight) + })] + pub fn unrevoke( + origin: OriginFor, + credential_id: CredentialIdOf, + authorization: Option, + ) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; let caller = source.subject(); @@ -339,11 +396,15 @@ pub mod pallet { Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { if let Some(credential) = credential_entry { if caller != credential.attester { - Err(DispatchError::from(Error::::Unauthorized)) - } else { - credential.revoked = false; - Ok(()) + let credential_auth_id = + credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; + authorization + .ok_or(Error::::Unauthorized)? + .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) + .map_err(|_| Error::::Unauthorized)?; } + credential.revoked = false; + Ok(()) } else { Err(DispatchError::from(Error::::CredentialNotFound)) } @@ -368,18 +429,39 @@ pub mod pallet { /// This function fails if a credential already exists for the specified /// subject. /// + /// Only authorized callers can remove the credential. + /// /// Emits `CredentialRemoved`. - #[pallet::weight(::WeightInfo::remove())] - pub fn remove(origin: OriginFor, credential_id: CredentialIdOf) -> DispatchResult { + #[pallet::weight({ + let xt_weight = ::WeightInfo::remove(); + let ac_weight = credential.authorization.as_ref().map(|ac| ac.can_remove_weight()).unwrap_or(0); + xt_weight.saturating_add(ac_weight) + })] + pub fn remove( + origin: OriginFor, + credential_id: CredentialIdOf, + authorization: Option, + ) -> DispatchResult { let source = ::EnsureOrigin::ensure_origin(origin)?; let caller = source.subject(); let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; - ensure!( - caller == credential_entry.attester, - Error::::Unauthorized - ); + if credential_entry.attester != caller { + let credential_auth_id = credential_entry + .authorization_id + .as_ref() + .ok_or(Error::::Unauthorized)?; + authorization + .ok_or(Error::::Unauthorized)? + .can_remove( + &caller, + &credential_entry.ctype_hash, + &credential_id, + credential_auth_id, + ) + .map_err(|_| Error::::Unauthorized)?; + } Self::remove_credential_entry(credential_subject, credential_id, credential_entry); @@ -396,10 +478,7 @@ pub mod pallet { let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; - ensure!( - submitter == credential_entry.deposit.owner, - Error::::Unauthorized - ); + ensure!(submitter == credential_entry.deposit.owner, Error::::Unauthorized); Self::remove_credential_entry(credential_subject, credential_id, credential_entry); diff --git a/pallets/public-credentials/src/mock.rs b/pallets/public-credentials/src/mock.rs index e904660cb4..94e7032dc6 100644 --- a/pallets/public-credentials/src/mock.rs +++ b/pallets/public-credentials/src/mock.rs @@ -31,6 +31,7 @@ pub fn generate_base_public_credential_creation_op( ctype_hash, subject: subject_id, claims, + authorization: None, } } @@ -51,12 +52,14 @@ pub(crate) mod runtime { use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ + dispatch::Weight, traits::{ConstU128, ConstU16, ConstU32, ConstU64, Get}, weights::constants::RocksDbWeight, }; use scale_info::TypeInfo; use sp_core::{sr25519, Pair}; use sp_runtime::{ + DispatchError, testing::Header, traits::{BlakeTwo256, IdentifyAccount, IdentityLookup, Verify}, MultiSignature, MultiSigner, @@ -69,7 +72,7 @@ pub(crate) mod runtime { use ctype::{CtypeCreatorOf, CtypeHashOf}; - use crate::{BalanceOf, CredentialEntryOf, CredentialSubjects, Credentials, CurrencyOf, Error, InputSubjectIdOf}; + use crate::{BalanceOf, Config, CredentialEntryOf, CredentialSubjects, Credentials, CurrencyOf, Error, InputSubjectIdOf, PublicCredentialsAccessControl}; pub(crate) type BlockNumber = u64; pub(crate) type Balance = u128; @@ -147,6 +150,7 @@ pub(crate) mod runtime { owner: payer, amount: ::Deposit::get(), }, + authorization_id: None, } } @@ -165,6 +169,91 @@ pub(crate) mod runtime { CredentialSubjects::::insert(credential_id, subject_id); } + /// Authorize iff the subject of the origin and the provided attester id match. + #[derive(Clone, Debug, Encode, Decode, TypeInfo, PartialEq, Eq)] + #[scale_info(skip_type_params(T))] + pub struct MockAccessControl(pub T::AttesterId); + + impl PublicCredentialsAccessControl, CredentialIdOf> + for MockAccessControl + where + T: Config::AttesterId>, + { + fn can_issue( + &self, + who: &T::AttesterId, + _ctype: &CtypeHashOf, + _credential_id: &CredentialIdOf, + ) -> Result { + if who == &self.0 { + Ok(0) + } else { + Err(DispatchError::Other("Unauthorized")) + } + } + + fn can_revoke( + &self, + who: &T::AttesterId, + _ctype: &CtypeHashOf, + _credential_id: &CredentialIdOf, + authorization_id: &T::AuthorizationId, + ) -> Result { + if authorization_id == who { + Ok(0) + } else { + Err(DispatchError::Other("Unauthorized")) + } + } + + fn can_unrevoke( + &self, + who: &T::AttesterId, + _ctype: &CtypeHashOf, + _credential_id: &CredentialIdOf, + authorization_id: &T::AuthorizationId, + ) -> Result { + if authorization_id == who { + Ok(0) + } else { + Err(DispatchError::Other("Unauthorized")) + } + } + + fn can_remove( + &self, + who: &T::AttesterId, + _ctype: &CtypeHashOf, + _credential_id: &CredentialIdOf, + authorization_id: &T::AuthorizationId, + ) -> Result { + println!("{:#?}", who); + println!("{:#?}", authorization_id); + if authorization_id == who { + Ok(0) + } else { + Err(DispatchError::Other("Unauthorized")) + } + } + + fn authorization_id(&self) -> T::AuthorizationId { + self.0.clone() + } + + fn can_issue_weight(&self) -> Weight { + 0 + } + fn can_revoke_weight(&self) -> Weight { + 0 + } + fn can_unrevoke_weight(&self) -> Weight { + 0 + } + fn can_remove_weight(&self) -> Weight { + 0 + } + } + pub(crate) const MILLI_UNIT: Balance = 10u128.pow(12); frame_support::construct_runtime!( @@ -240,7 +329,9 @@ pub(crate) mod runtime { } impl Config for Test { + type AccessControl = MockAccessControl; type AttesterId = SubjectId; + type AuthorizationId = SubjectId; type CredentialId = Hash; type CredentialHash = BlakeTwo256; type Currency = Balances; diff --git a/pallets/public-credentials/src/tests.rs b/pallets/public-credentials/src/tests.rs index f97f8f9050..1fbd71dfe9 100644 --- a/pallets/public-credentials/src/tests.rs +++ b/pallets/public-credentials/src/tests.rs @@ -22,12 +22,12 @@ use sp_runtime::traits::Zero; use ctype::mock::get_ctype_hash; use kilt_support::mock::mock_origin::DoubleOrigin; -use crate::{mock::*, Config, Credentials, CredentialIdOf, CredentialSubjects, Error, InputClaimsContentOf}; +use crate::{mock::*, Config, CredentialIdOf, CredentialSubjects, Credentials, Error, InputClaimsContentOf}; // add #[test] -fn add_successful() { +fn add_successful_without_authorization() { let attester = sr25519_did_from_seed(&ALICE_SEED); let subject_id = SUBJECT_ID_00; let ctype_hash_1 = get_ctype_hash::(true); @@ -47,10 +47,7 @@ fn add_successful() { let deposit: Balance = ::Deposit::get(); ExtBuilder::default() - .with_balances(vec![( - ACCOUNT_00, - (deposit) * 2, - )]) + .with_balances(vec![(ACCOUNT_00, (deposit) * 2)]) .with_ctypes(vec![(ctype_hash_1, attester.clone()), (ctype_hash_2, attester.clone())]) .build() .execute_with(|| { @@ -69,13 +66,11 @@ fn add_successful() { assert!(!stored_public_credential_details.revoked); assert_eq!(stored_public_credential_details.block_number, 0); assert_eq!(stored_public_credential_details.ctype_hash, ctype_hash_1); + assert_eq!(stored_public_credential_details.authorization_id, None); assert_eq!(CredentialSubjects::::get(&credential_id_1), Some(subject_id)); // Check deposit reservation logic - assert_eq!( - Balances::reserved_balance(ACCOUNT_00), - deposit - ); + assert_eq!(Balances::reserved_balance(ACCOUNT_00), deposit); // Re-issuing the same credential will fail assert_noop!( @@ -87,10 +82,7 @@ fn add_successful() { ); // Check deposit has not changed - assert_eq!( - Balances::reserved_balance(ACCOUNT_00), - deposit - ); + assert_eq!(Balances::reserved_balance(ACCOUNT_00), deposit); System::set_block_number(1); @@ -108,12 +100,75 @@ fn add_successful() { assert!(!stored_public_credential_details.revoked); assert_eq!(stored_public_credential_details.block_number, 1); assert_eq!(stored_public_credential_details.ctype_hash, ctype_hash_2); + assert_eq!(stored_public_credential_details.authorization_id, None); assert_eq!(CredentialSubjects::::get(&credential_id_2), Some(subject_id)); // Deposit is 2x now - assert_eq!( - Balances::reserved_balance(ACCOUNT_00), - 2 * deposit + assert_eq!(Balances::reserved_balance(ACCOUNT_00), 2 * deposit); + }); +} + +#[test] +fn add_successful_with_authorization() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let subject_id = SUBJECT_ID_00; + let ctype_hash = get_ctype_hash::(true); + let mut new_credential = generate_base_public_credential_creation_op::( + subject_id.into(), + ctype_hash, + InputClaimsContentOf::::default(), + ); + new_credential.authorization = Some(MockAccessControl(attester.clone())); + let credential_id: CredentialIdOf = generate_credential_id::(&new_credential, &attester); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_ctypes(vec![(ctype_hash, attester.clone())]) + .build() + .execute_with(|| { + assert_ok!(PublicCredentials::add( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + new_credential.clone() + )); + let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id) + .expect("Public credential details should be present on chain."); + + // Test this pallet logic + assert_eq!(stored_public_credential_details.attester, attester); + assert!(!stored_public_credential_details.revoked); + assert_eq!(stored_public_credential_details.block_number, 0); + assert_eq!(stored_public_credential_details.ctype_hash, ctype_hash); + assert_eq!(stored_public_credential_details.authorization_id, Some(attester)); + assert_eq!(CredentialSubjects::::get(&credential_id), Some(subject_id)); + }); +} + +#[test] +fn add_unauthorized() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_attester = sr25519_did_from_seed(&BOB_SEED); + let subject_id = SUBJECT_ID_00; + let ctype_hash = get_ctype_hash::(true); + let mut new_credential = generate_base_public_credential_creation_op::( + subject_id.into(), + ctype_hash, + InputClaimsContentOf::::default(), + ); + new_credential.authorization = Some(MockAccessControl(wrong_attester)); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_ctypes(vec![(ctype_hash, attester.clone())]) + .build() + .execute_with(|| { + assert_noop!( + PublicCredentials::add( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + new_credential.clone() + ), + Error::::Unauthorized ); }); } @@ -136,10 +191,7 @@ fn add_ctype_not_existing() { .build() .execute_with(|| { assert_noop!( - PublicCredentials::add( - DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - new_credential - ), + PublicCredentials::add(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), new_credential), ctype::Error::::CTypeNotFound ); }); @@ -164,10 +216,7 @@ fn add_not_enough_balance() { .build() .execute_with(|| { assert_noop!( - PublicCredentials::add( - DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - new_credential - ), + PublicCredentials::add(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), new_credential), Error::::UnableToPayFees ); }); @@ -191,6 +240,7 @@ fn revoke_successful() { assert_ok!(PublicCredentials::revoke( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, + None, )); let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id) @@ -203,6 +253,7 @@ fn revoke_successful() { assert_ok!(PublicCredentials::revoke( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, + None, )); let stored_public_credential_details_2 = Credentials::::get(&subject_id, &credential_id) @@ -213,28 +264,41 @@ fn revoke_successful() { } #[test] -fn revoke_credential_not_found() { +fn revoke_same_attester_wrong_ac() { let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.authorization_id = Some(attester.clone()); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { - assert_noop!(PublicCredentials::revoke( + assert_ok!(PublicCredentials::revoke( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, - ), Error::::CredentialNotFound); + Some(MockAccessControl(wrong_submitter)) + )); + + let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id) + .expect("Public credential details should be present on chain."); + + // Test this pallet logic + assert!(stored_public_credential_details.revoked); }); } #[test] -fn revoke_unauthorised() { +fn revoke_unauthorized() { let attester = sr25519_did_from_seed(&ALICE_SEED); - let wrong_attester = sr25519_did_from_seed(&BOB_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester, None); + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.authorization_id = Some(attester.clone()); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -243,10 +307,57 @@ fn revoke_unauthorised() { .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { - assert_noop!(PublicCredentials::revoke( - DoubleOrigin(ACCOUNT_00, wrong_attester.clone()).into(), - credential_id, - ), Error::::Unauthorized); + assert_noop!( + PublicCredentials::revoke( + DoubleOrigin(ACCOUNT_00, wrong_submitter).into(), + credential_id, + Some(MockAccessControl(attester)) + ), + Error::::Unauthorized + ); + }); +} + +#[test] +fn revoke_ac_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.authorization_id = Some(attester); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) + .build() + .execute_with(|| { + assert_noop!( + PublicCredentials::revoke( + DoubleOrigin(ACCOUNT_00, wrong_submitter.clone()).into(), + credential_id, + Some(MockAccessControl(wrong_submitter)) + ), + Error::::Unauthorized + ); + }); +} + +#[test] +fn revoke_credential_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .build() + .execute_with(|| { + assert_noop!( + PublicCredentials::revoke(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, None,), + Error::::CredentialNotFound + ); }); } @@ -269,6 +380,7 @@ fn unrevoke_successful() { assert_ok!(PublicCredentials::unrevoke( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, + None, )); let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id) @@ -281,6 +393,7 @@ fn unrevoke_successful() { assert_ok!(PublicCredentials::unrevoke( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, + None, )); let stored_public_credential_details_2 = Credentials::::get(&subject_id, &credential_id) @@ -291,28 +404,43 @@ fn unrevoke_successful() { } #[test] -fn unrevoke_credential_not_found() { +fn unrevoke_same_attester_wrong_ac() { let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.revoked = true; + new_credential.authorization_id = Some(attester.clone()); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { - assert_noop!(PublicCredentials::unrevoke( + assert_ok!(PublicCredentials::unrevoke( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, - ), Error::::CredentialNotFound); + Some(MockAccessControl(wrong_submitter)) + )); + + let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id) + .expect("Public credential details should be present on chain."); + + // Test this pallet logic + assert!(!stored_public_credential_details.revoked); }); } #[test] -fn unrevoke_unauthorised() { +fn unrevoke_unauthorized() { let attester = sr25519_did_from_seed(&ALICE_SEED); - let wrong_attester = sr25519_did_from_seed(&BOB_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester, None); + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.revoked = true; + new_credential.authorization_id = Some(attester.clone()); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -321,10 +449,58 @@ fn unrevoke_unauthorised() { .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { - assert_noop!(PublicCredentials::unrevoke( - DoubleOrigin(ACCOUNT_00, wrong_attester.clone()).into(), - credential_id, - ), Error::::Unauthorized); + assert_noop!( + PublicCredentials::unrevoke( + DoubleOrigin(ACCOUNT_00, wrong_submitter).into(), + credential_id, + Some(MockAccessControl(attester)) + ), + Error::::Unauthorized + ); + }); +} + +#[test] +fn unrevoke_ac_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.revoked = true; + new_credential.authorization_id = Some(attester); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) + .build() + .execute_with(|| { + assert_noop!( + PublicCredentials::unrevoke( + DoubleOrigin(ACCOUNT_00, wrong_submitter.clone()).into(), + credential_id, + Some(MockAccessControl(wrong_submitter)) + ), + Error::::Unauthorized + ); + }); +} + +#[test] +fn unrevoke_credential_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .build() + .execute_with(|| { + assert_noop!( + PublicCredentials::unrevoke(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, None,), + Error::::CredentialNotFound + ); }); } @@ -346,6 +522,7 @@ fn remove_successful() { assert_ok!(PublicCredentials::remove( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, + None, )); // Test this pallet logic @@ -357,40 +534,46 @@ fn remove_successful() { // Removing the same credential again will fail assert_noop!( - PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id), - Error::::CredentialNotFound - ); - - assert_noop!( - PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id), + PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, None,), Error::::CredentialNotFound ); }); } #[test] -fn remove_credential_not_found() { +fn remove_same_attester_wrong_ac() { let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.authorization_id = Some(attester.clone()); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); ExtBuilder::default() .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { - assert_noop!(PublicCredentials::remove( + assert_ok!(PublicCredentials::remove( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, - ), Error::::CredentialNotFound); + Some(MockAccessControl(wrong_submitter)) + )); + + // Test this pallet logic + assert_eq!(Credentials::::get(&subject_id, &credential_id), None); + assert_eq!(CredentialSubjects::::get(&credential_id), None); }); } #[test] fn remove_unauthorized() { let attester = sr25519_did_from_seed(&ALICE_SEED); - let wrong_attester = sr25519_did_from_seed(&BOB_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); let subject_id: ::SubjectId = SUBJECT_ID_00; - let new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester, None); + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.authorization_id = Some(attester.clone()); let credential_id: CredentialIdOf = CredentialIdOf::::default(); let deposit: Balance = ::Deposit::get(); @@ -399,10 +582,57 @@ fn remove_unauthorized() { .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { - assert_noop!(PublicCredentials::remove( - DoubleOrigin(ACCOUNT_00, wrong_attester).into(), - credential_id, - ), Error::::Unauthorized); + assert_noop!( + PublicCredentials::remove( + DoubleOrigin(ACCOUNT_00, wrong_submitter).into(), + credential_id, + Some(MockAccessControl(attester)) + ), + Error::::Unauthorized + ); + }); +} + +#[test] +fn remove_ac_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let wrong_submitter = sr25519_did_from_seed(&BOB_SEED); + let subject_id: ::SubjectId = SUBJECT_ID_00; + let mut new_credential = generate_base_credential_entry::(ACCOUNT_00, 0, attester.clone(), None); + new_credential.authorization_id = Some(attester); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) + .build() + .execute_with(|| { + assert_noop!( + PublicCredentials::remove( + DoubleOrigin(ACCOUNT_00, wrong_submitter.clone()).into(), + credential_id, + Some(MockAccessControl(wrong_submitter)) + ), + Error::::Unauthorized + ); + }); +} + +#[test] +fn remove_credential_not_found() { + let attester = sr25519_did_from_seed(&ALICE_SEED); + let credential_id: CredentialIdOf = CredentialIdOf::::default(); + let deposit: Balance = ::Deposit::get(); + + ExtBuilder::default() + .with_balances(vec![(ACCOUNT_00, deposit)]) + .build() + .execute_with(|| { + assert_noop!( + PublicCredentials::remove(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), credential_id, None,), + Error::::CredentialNotFound + ); }); } @@ -455,10 +685,10 @@ fn reclaim_deposit_credential_not_found() { .with_balances(vec![(ACCOUNT_00, deposit)]) .build() .execute_with(|| { - assert_noop!(PublicCredentials::reclaim_deposit( - Origin::signed(ACCOUNT_00), - credential_id - ), Error::::CredentialNotFound); + assert_noop!( + PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_00), credential_id), + Error::::CredentialNotFound + ); }); } @@ -475,9 +705,9 @@ fn reclaim_deposit_unauthorized() { .with_public_credentials(vec![(subject_id, credential_id, new_credential)]) .build() .execute_with(|| { - assert_noop!(PublicCredentials::reclaim_deposit( - Origin::signed(ACCOUNT_01), - credential_id - ), Error::::Unauthorized); + assert_noop!( + PublicCredentials::reclaim_deposit(Origin::signed(ACCOUNT_01), credential_id), + Error::::Unauthorized + ); }); } From 4348f4b71836e1d20e6490c17115a91df591f956 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 10:04:18 +0200 Subject: [PATCH 08/21] Update benchmarks --- .../public-credentials/src/access_control.rs | 11 +- .../public-credentials/src/benchmarking.rs | 94 +++++++++--- pallets/public-credentials/src/credentials.rs | 4 +- .../public-credentials/src/default_weights.rs | 11 ++ pallets/public-credentials/src/lib.rs | 140 ++++++++++-------- pallets/public-credentials/src/mock.rs | 17 ++- pallets/public-credentials/src/tests.rs | 20 ++- 7 files changed, 200 insertions(+), 97 deletions(-) diff --git a/pallets/public-credentials/src/access_control.rs b/pallets/public-credentials/src/access_control.rs index 5095049820..1a38e7851b 100644 --- a/pallets/public-credentials/src/access_control.rs +++ b/pallets/public-credentials/src/access_control.rs @@ -20,9 +20,10 @@ use frame_support::dispatch::Weight; use sp_runtime::DispatchError; pub trait AccessControl { - /// Decides whether the account is allowed to issue a credential with the given - /// information provided by the sender (&self). - fn can_issue(&self, who: &AttesterId, ctype: &Ctype, credential_id: &CredentialId) -> Result; + /// Decides whether the account is allowed to issue a credential with the + /// given information provided by the sender (&self). + fn can_issue(&self, who: &AttesterId, ctype: &Ctype, credential_id: &CredentialId) + -> Result; /// Decides whether the account is allowed to revoke the credential with /// the `authorization_id` and the access information provided by the sender @@ -77,8 +78,8 @@ pub trait AccessControl { fn can_remove_weight(&self) -> Weight; } -impl -AccessControl for () +impl AccessControl + for () where AuthorizationId: Default, { diff --git a/pallets/public-credentials/src/benchmarking.rs b/pallets/public-credentials/src/benchmarking.rs index 1a5742f710..804987c854 100644 --- a/pallets/public-credentials/src/benchmarking.rs +++ b/pallets/public-credentials/src/benchmarking.rs @@ -26,7 +26,10 @@ use sp_std::{boxed::Box, vec, vec::Vec}; use kilt_support::traits::{GenerateBenchmarkOrigin, GetWorstCase}; -use crate::{mock::generate_base_public_credential_creation_op, *}; +use crate::{ + mock::{generate_base_public_credential_creation_op, generate_credential_id}, + *, +}; const SEED: u32 = 0; @@ -35,7 +38,6 @@ benchmarks! { where T: core::fmt::Debug, T: Config, - T: attestation::Config, T: ctype::Config, ::EnsureOrigin: GenerateBenchmarkOrigin, ::SubjectId: GetWorstCase + Into> + sp_std::fmt::Debug, @@ -52,19 +54,73 @@ benchmarks! { let creation_op = Box::new(generate_base_public_credential_creation_op::( subject_id.clone().into().try_into().expect("Input conversion should not fail."), - claim_hash, ctype_hash, contents, - None, )); + let credential_id = generate_credential_id::(&creation_op, &attester); ctype::Ctypes::::insert(&ctype_hash, attester.clone()); - CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get() + ::Deposit::get() + ::Deposit::get()); + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); let origin = ::EnsureOrigin::generate_origin(sender, attester); }: _(origin, creation_op) verify { - assert!(Credentials::::contains_key(subject_id, claim_hash)); - assert!(CredentialsUnicityIndex::::contains_key(claim_hash)); + assert!(Credentials::::contains_key(subject_id, &credential_id)); + assert!(CredentialSubjects::::contains_key(&credential_id)); + } + + revoke { + let sender: T::AccountId = account("sender", 0, SEED); + let attester: T::AttesterId = account("attester", 0, SEED); + let claim_hash: T::Hash = T::Hash::default(); + let ctype_hash: T::Hash = T::Hash::default(); + let subject_id = ::SubjectId::worst_case(); + let contents = BoundedVec::try_from(vec![0; ::MaxEncodedClaimsLength::get() as usize]).expect("Contents should not fail."); + let origin = ::EnsureOrigin::generate_origin(sender.clone(), attester.clone()); + + let creation_op = Box::new(generate_base_public_credential_creation_op::( + subject_id.clone().into().try_into().expect("Input conversion should not fail."), + ctype_hash, + contents, + )); + let credential_id = generate_credential_id::(&creation_op, &attester); + + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); + + ctype::Ctypes::::insert(&ctype_hash, attester); + Pallet::::add(origin.clone(), creation_op).expect("Pallet::add should not fail"); + let credential_id_clone = credential_id.clone(); + }: _(origin, credential_id_clone, None) + verify { + assert!(!Credentials::::contains_key(subject_id, &credential_id)); + assert!(!CredentialSubjects::::contains_key(credential_id)); + } + + unrevoke { + let sender: T::AccountId = account("sender", 0, SEED); + let attester: T::AttesterId = account("attester", 0, SEED); + let claim_hash: T::Hash = T::Hash::default(); + let ctype_hash: T::Hash = T::Hash::default(); + let subject_id = ::SubjectId::worst_case(); + let contents = BoundedVec::try_from(vec![0; ::MaxEncodedClaimsLength::get() as usize]).expect("Contents should not fail."); + let origin = ::EnsureOrigin::generate_origin(sender.clone(), attester.clone()); + + let creation_op = Box::new(generate_base_public_credential_creation_op::( + subject_id.clone().into().try_into().expect("Input conversion should not fail."), + ctype_hash, + contents, + )); + let credential_id = generate_credential_id::(&creation_op, &attester); + + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); + + ctype::Ctypes::::insert(&ctype_hash, attester); + Pallet::::add(origin.clone(), creation_op).expect("Pallet::add should not fail"); + Pallet::::revoke(origin.clone(), credential_id.clone(), None).expect("Pallet::revoke should not fail"); + let credential_id_clone = credential_id.clone(); + }: _(origin, credential_id_clone, None) + verify { + assert!(!Credentials::::contains_key(subject_id, &credential_id)); + assert!(!CredentialSubjects::::contains_key(credential_id)); } remove { @@ -78,20 +134,20 @@ benchmarks! { let creation_op = Box::new(generate_base_public_credential_creation_op::( subject_id.clone().into().try_into().expect("Input conversion should not fail."), - claim_hash, ctype_hash, contents, - None, )); + let credential_id = generate_credential_id::(&creation_op, &attester); - CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get() + ::Deposit::get() + ::Deposit::get()); + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); ctype::Ctypes::::insert(&ctype_hash, attester); Pallet::::add(origin.clone(), creation_op).expect("Pallet::add should not fail"); - }: _(origin, claim_hash, None) + let credential_id_clone = credential_id.clone(); + }: _(origin, credential_id_clone, None) verify { - assert!(!Credentials::::contains_key(subject_id, claim_hash)); - assert!(!CredentialsUnicityIndex::::contains_key(claim_hash)); + assert!(!Credentials::::contains_key(subject_id, &credential_id)); + assert!(!CredentialSubjects::::contains_key(credential_id)); } reclaim_deposit { @@ -105,21 +161,21 @@ benchmarks! { let creation_op = Box::new(generate_base_public_credential_creation_op::( subject_id.clone().into().try_into().expect("Input conversion should not fail."), - claim_hash, ctype_hash, contents, - None, )); + let credential_id = generate_credential_id::(&creation_op, &attester); - CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get() + ::Deposit::get() + ::Deposit::get()); + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); ctype::Ctypes::::insert(&ctype_hash, attester); Pallet::::add(origin, creation_op).expect("Pallet::add should not fail"); let origin = RawOrigin::Signed(sender); - }: _(origin, claim_hash) + let credential_id_clone = credential_id.clone(); + }: _(origin, credential_id_clone) verify { - assert!(!Credentials::::contains_key(subject_id, claim_hash)); - assert!(!CredentialsUnicityIndex::::contains_key(claim_hash)); + assert!(!Credentials::::contains_key(subject_id, &credential_id)); + assert!(!CredentialSubjects::::contains_key(credential_id)); } } diff --git a/pallets/public-credentials/src/credentials.rs b/pallets/public-credentials/src/credentials.rs index 9b3313d291..c2c1a957b9 100644 --- a/pallets/public-credentials/src/credentials.rs +++ b/pallets/public-credentials/src/credentials.rs @@ -54,7 +54,7 @@ pub struct CredentialEntry, - /// The ID of the authorization information (e.g., a delegation node) used to authorize the - /// operation. + /// The ID of the authorization information (e.g., a delegation node) used + /// to authorize the operation. pub authorization_id: Option, } diff --git a/pallets/public-credentials/src/default_weights.rs b/pallets/public-credentials/src/default_weights.rs index 241991b7fb..74eec21d14 100644 --- a/pallets/public-credentials/src/default_weights.rs +++ b/pallets/public-credentials/src/default_weights.rs @@ -51,6 +51,7 @@ pub trait WeightInfo { fn add(c: u32, ) -> Weight; fn remove() -> Weight; fn revoke() -> Weight; + fn unrevoke() -> Weight; fn reclaim_deposit() -> Weight; } @@ -81,6 +82,11 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } + fn unrevoke() -> Weight { + (35_041_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) // Storage: Attestation Attestations (r:1 w:1) @@ -118,6 +124,11 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } + fn unrevoke() -> Weight { + (35_041_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + } // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) // Storage: Attestation Attestations (r:1 w:1) diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index 13793a05af..2c6a4a6c20 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -124,13 +124,15 @@ pub mod pallet { >; /// The identifier of the credential attester. type AttesterId: Parameter + MaxEncodedLen; - /// The identifier of the authorization info to perform access control for the different operations. + /// The identifier of the authorization info to perform access control + /// for the different operations. type AuthorizationId: Parameter + MaxEncodedLen; /// The origin allowed to issue/revoke/remove public credentials. type EnsureOrigin: EnsureOrigin::OriginSuccess, Self::Origin>; /// The ubiquitous event type. type Event: From> + IsType<::Event>; - /// The hashing algorithm to derive a credential identifier from the credential content. + /// The hashing algorithm to derive a credential identifier from the + /// credential content. type CredentialHash: Hash; /// The type of a credential identifier. type CredentialId: Parameter + MaxEncodedLen; @@ -180,8 +182,8 @@ pub mod pallet { >; // This map ensures that at any time a credential is only linked (i.e., issued) - // to a single subject, as it maps from a credential ID to the subject it was issued to. - // Not exposed to the outside world. + // to a single subject, as it maps from a credential ID to the subject it was + // issued to. Not exposed to the outside world. #[pallet::storage] #[pallet::getter(fn get_credential_subject)] pub(crate) type CredentialSubjects = @@ -240,8 +242,8 @@ pub mod pallet { impl Pallet { /// Register a new public credential on chain. /// - /// This function fails if a credential with the same identifier already exists for the specified - /// subject. + /// This function fails if a credential with the same identifier already + /// exists for the specified subject. /// /// Emits `CredentialStored`. #[allow(clippy::boxed_local)] @@ -250,7 +252,7 @@ pub mod pallet { let ac_weight = credential.authorization.as_ref().map(|ac| ac.can_issue_weight()).unwrap_or(0); xt_weight.saturating_add(ac_weight) })] - pub fn add(origin: OriginFor, credential: InputCredentialOf) -> DispatchResult { + pub fn add(origin: OriginFor, credential: Box>) -> DispatchResultWithPostInfo { let source = ::EnsureOrigin::ensure_origin(origin)?; let attester = source.subject(); @@ -263,7 +265,7 @@ pub mod pallet { subject, claims: _, authorization, - } = credential.clone(); + } = *credential.clone(); ensure!( ctype::Ctypes::::contains_key(&ctype_hash), @@ -275,7 +277,7 @@ pub mod pallet { T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); // Check for validity of the authorization info if specified. - authorization + let ac_weight = authorization .as_ref() .map(|ac| ac.can_issue(&attester, &ctype_hash, &credential_id)) .transpose() @@ -318,101 +320,121 @@ pub mod pallet { credential_id, }); - Ok(()) + Ok(Some( + ::WeightInfo::add(credential.claims.len().saturated_into::()) + .saturating_add(ac_weight.unwrap_or(0)), + ) + .into()) } /// Revokes a public credential. /// - /// If a credential was already revoked, this function does not fail but simply results - /// in a noop. + /// If a credential was already revoked, this function does not fail but + /// simply results in a noop. /// /// Only authorized callers can revoke the credential. /// /// Emits `CredentialRevoked`. #[pallet::weight({ let xt_weight = ::WeightInfo::revoke(); - let ac_weight = credential.authorization.as_ref().map(|ac| ac.can_revoke_weight()).unwrap_or(0); + let ac_weight = authorization.as_ref().map(|ac| ac.can_revoke_weight()).unwrap_or(0); xt_weight.saturating_add(ac_weight) })] pub fn revoke( origin: OriginFor, credential_id: CredentialIdOf, authorization: Option, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let source = ::EnsureOrigin::ensure_origin(origin)?; let caller = source.subject(); let credential_subject = CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; - // Fails if the credential does not exist OR the caller is different than the original attester. - Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { - if let Some(credential) = credential_entry { - if caller != credential.attester { - let credential_auth_id = - credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; - authorization - .ok_or(Error::::Unauthorized)? - .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) - .map_err(|_| Error::::Unauthorized)?; + // Fails if the credential does not exist OR the caller is different than the + // original attester. If successful, saves the additional weight used for access + // control and returns it at the end of the function. + let ac_weight_used = + Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { + if let Some(credential) = credential_entry { + // Additional weight is 0 if the caller is the attester, otherwise it's the + // value returned by the access control check, if it does not fail. + let additional_weight = if caller != credential.attester { + let credential_auth_id = + credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; + authorization + .ok_or(Error::::Unauthorized)? + .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) + .map_err(|_| Error::::Unauthorized)? + } else { + 0 + }; + credential.revoked = true; + Ok(additional_weight) + } else { + // No weight is computed as the error is an early return. + Err(DispatchError::from(Error::::CredentialNotFound)) } - credential.revoked = true; - Ok(()) - } else { - Err(DispatchError::from(Error::::CredentialNotFound)) - } - })?; + })?; Self::deposit_event(Event::CredentialRevoked { credential_id }); - Ok(()) + Ok(Some(::WeightInfo::revoke().saturating_add(ac_weight_used)).into()) } /// Unrevokes a public credential. /// - /// If a credential was not revoked, this function does not fail but simply results - /// in a noop. + /// If a credential was not revoked, this function does not fail but + /// simply results in a noop. /// /// Only authorized callers can unrevoke the credential. /// /// Emits `CredentialUnrevoked`. #[pallet::weight({ let xt_weight = ::WeightInfo::unrevoke(); - let ac_weight = credential.authorization.as_ref().map(|ac| ac.can_unrevoke_weight()).unwrap_or(0); + let ac_weight = authorization.as_ref().map(|ac| ac.can_unrevoke_weight()).unwrap_or(0); xt_weight.saturating_add(ac_weight) })] pub fn unrevoke( origin: OriginFor, credential_id: CredentialIdOf, authorization: Option, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let source = ::EnsureOrigin::ensure_origin(origin)?; let caller = source.subject(); let credential_subject = CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; - // Fails if the credential does not exist OR the caller is different than the original attester. - Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { - if let Some(credential) = credential_entry { - if caller != credential.attester { - let credential_auth_id = - credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; - authorization - .ok_or(Error::::Unauthorized)? - .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) - .map_err(|_| Error::::Unauthorized)?; + // Fails if the credential does not exist OR the caller is different than the + // original attester. If successful, saves the additional weight used for access + // control and returns it at the end of the function. + let ac_weight_used = + Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { + if let Some(credential) = credential_entry { + // Additional weight is 0 if the caller is the attester, otherwise it's the + // value returned by the access control check, if it does not fail. + let additional_weight = if caller != credential.attester { + let credential_auth_id = + credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; + authorization + .ok_or(Error::::Unauthorized)? + .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) + .map_err(|_| Error::::Unauthorized)? + } else { + 0 + }; + credential.revoked = false; + Ok(additional_weight) + } else { + // No weight is computed as the error is an early return. + Err(DispatchError::from(Error::::CredentialNotFound)) } - credential.revoked = false; - Ok(()) - } else { - Err(DispatchError::from(Error::::CredentialNotFound)) - } - })?; + })?; Self::deposit_event(Event::CredentialUnrevoked { credential_id }); - Ok(()) + Ok(Some(::WeightInfo::unrevoke().saturating_add(ac_weight_used)).into()) } /// Removes the information pertaining a public credential from the @@ -434,20 +456,20 @@ pub mod pallet { /// Emits `CredentialRemoved`. #[pallet::weight({ let xt_weight = ::WeightInfo::remove(); - let ac_weight = credential.authorization.as_ref().map(|ac| ac.can_remove_weight()).unwrap_or(0); + let ac_weight = authorization.as_ref().map(|ac| ac.can_remove_weight()).unwrap_or(0); xt_weight.saturating_add(ac_weight) })] pub fn remove( origin: OriginFor, credential_id: CredentialIdOf, authorization: Option, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let source = ::EnsureOrigin::ensure_origin(origin)?; let caller = source.subject(); let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; - if credential_entry.attester != caller { + let ac_weight_used = if credential_entry.attester != caller { let credential_auth_id = credential_entry .authorization_id .as_ref() @@ -460,12 +482,14 @@ pub mod pallet { &credential_id, credential_auth_id, ) - .map_err(|_| Error::::Unauthorized)?; - } + .map_err(|_| Error::::Unauthorized)? + } else { + 0 + }; Self::remove_credential_entry(credential_subject, credential_id, credential_entry); - Ok(()) + Ok(Some(::WeightInfo::remove().saturating_add(ac_weight_used)).into()) } /// Performs the same function as the `remove` extrinsic, with the diff --git a/pallets/public-credentials/src/mock.rs b/pallets/public-credentials/src/mock.rs index 94e7032dc6..305a94a108 100644 --- a/pallets/public-credentials/src/mock.rs +++ b/pallets/public-credentials/src/mock.rs @@ -19,7 +19,9 @@ use codec::Encode; use sp_runtime::traits::Hash; -use crate::{AttesterOf, Config, CredentialIdOf, CtypeHashOf, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf}; +use crate::{ + AttesterOf, Config, CredentialIdOf, CtypeHashOf, InputClaimsContentOf, InputCredentialOf, InputSubjectIdOf, +}; // Generate a public credential using a many Default::default() as possible. pub fn generate_base_public_credential_creation_op( @@ -59,10 +61,9 @@ pub(crate) mod runtime { use scale_info::TypeInfo; use sp_core::{sr25519, Pair}; use sp_runtime::{ - DispatchError, testing::Header, traits::{BlakeTwo256, IdentifyAccount, IdentityLookup, Verify}, - MultiSignature, MultiSigner, + DispatchError, MultiSignature, MultiSigner, }; use kilt_support::{ @@ -72,7 +73,10 @@ pub(crate) mod runtime { use ctype::{CtypeCreatorOf, CtypeHashOf}; - use crate::{BalanceOf, Config, CredentialEntryOf, CredentialSubjects, Credentials, CurrencyOf, Error, InputSubjectIdOf, PublicCredentialsAccessControl}; + use crate::{ + BalanceOf, Config, CredentialEntryOf, CredentialSubjects, Credentials, CurrencyOf, Error, InputSubjectIdOf, + PublicCredentialsAccessControl, + }; pub(crate) type BlockNumber = u64; pub(crate) type Balance = u128; @@ -139,7 +143,7 @@ pub(crate) mod runtime { payer: T::AccountId, block_number: ::BlockNumber, attester: T::AttesterId, - ctype_hash: Option> + ctype_hash: Option>, ) -> CredentialEntryOf { CredentialEntryOf:: { ctype_hash: ctype_hash.unwrap_or_default(), @@ -169,7 +173,8 @@ pub(crate) mod runtime { CredentialSubjects::::insert(credential_id, subject_id); } - /// Authorize iff the subject of the origin and the provided attester id match. + /// Authorize iff the subject of the origin and the provided attester id + /// match. #[derive(Clone, Debug, Encode, Decode, TypeInfo, PartialEq, Eq)] #[scale_info(skip_type_params(T))] pub struct MockAccessControl(pub T::AttesterId); diff --git a/pallets/public-credentials/src/tests.rs b/pallets/public-credentials/src/tests.rs index 1fbd71dfe9..51be35d85a 100644 --- a/pallets/public-credentials/src/tests.rs +++ b/pallets/public-credentials/src/tests.rs @@ -56,7 +56,7 @@ fn add_successful_without_authorization() { assert_ok!(PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - new_credential_1.clone() + Box::new(new_credential_1.clone()) )); let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id_1) .expect("Public credential details should be present on chain."); @@ -76,7 +76,7 @@ fn add_successful_without_authorization() { assert_noop!( PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - new_credential_1.clone() + Box::new(new_credential_1.clone()) ), Error::::CredentialAlreadyIssued ); @@ -89,7 +89,7 @@ fn add_successful_without_authorization() { // Issuing a completely new credential will work assert_ok!(PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - new_credential_2.clone() + Box::new(new_credential_2.clone()) )); let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id_2) @@ -129,7 +129,7 @@ fn add_successful_with_authorization() { .execute_with(|| { assert_ok!(PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - new_credential.clone() + Box::new(new_credential.clone()) )); let stored_public_credential_details = Credentials::::get(&subject_id, &credential_id) .expect("Public credential details should be present on chain."); @@ -166,7 +166,7 @@ fn add_unauthorized() { assert_noop!( PublicCredentials::add( DoubleOrigin(ACCOUNT_00, attester.clone()).into(), - new_credential.clone() + Box::new(new_credential.clone()) ), Error::::Unauthorized ); @@ -191,7 +191,10 @@ fn add_ctype_not_existing() { .build() .execute_with(|| { assert_noop!( - PublicCredentials::add(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), new_credential), + PublicCredentials::add( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + Box::new(new_credential) + ), ctype::Error::::CTypeNotFound ); }); @@ -216,7 +219,10 @@ fn add_not_enough_balance() { .build() .execute_with(|| { assert_noop!( - PublicCredentials::add(DoubleOrigin(ACCOUNT_00, attester.clone()).into(), new_credential), + PublicCredentials::add( + DoubleOrigin(ACCOUNT_00, attester.clone()).into(), + Box::new(new_credential) + ), Error::::UnableToPayFees ); }); From ddf7994f65ab1187d8c6e3fc4c92857bb5a039ce Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 10:28:04 +0200 Subject: [PATCH 09/21] Runtimes compiling --- Cargo.lock | 2 + pallets/delegation/Cargo.toml | 2 + pallets/delegation/src/access_control.rs | 88 +++++++++++++++++++++++ pallets/public-credentials/src/lib.rs | 4 +- runtimes/common/Cargo.toml | 3 + runtimes/common/src/authorization.rs | 89 ++++++++++++++++++++++++ runtimes/peregrine/src/lib.rs | 21 ++---- runtimes/spiritnet/src/lib.rs | 21 ++---- runtimes/standalone/src/lib.rs | 19 ++--- 9 files changed, 206 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 993dc3bcd8..40cab65d8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1946,6 +1946,7 @@ dependencies = [ "log", "pallet-balances", "parity-scale-codec", + "public-credentials", "scale-info", "serde", "sp-core", @@ -8984,6 +8985,7 @@ dependencies = [ "pallet-transaction-payment", "parachain-staking", "parity-scale-codec", + "public-credentials", "scale-info", "serde", "smallvec", diff --git a/pallets/delegation/Cargo.toml b/pallets/delegation/Cargo.toml index 24b5ed8be7..1b227d0a80 100644 --- a/pallets/delegation/Cargo.toml +++ b/pallets/delegation/Cargo.toml @@ -31,6 +31,7 @@ sp-keystore = {branch = "polkadot-v0.9.25", git = "https://github.com/paritytech attestation = {default-features = false, path = "../attestation"} ctype = {default-features = false, path = "../ctype"} kilt-support = {default-features = false, path = "../../support"} +public-credentials = {default-features = false, path = "../public-credentials"} #External dependencies bitflags = {default-features = false, version = "1.3.2"} @@ -77,6 +78,7 @@ std = [ "kilt-support/std", "log/std", "pallet-balances/std", + "public-credentials/std", "scale-info/std", "sp-core/std", "sp-io/std", diff --git a/pallets/delegation/src/access_control.rs b/pallets/delegation/src/access_control.rs index 3702cd4c14..794c82b996 100644 --- a/pallets/delegation/src/access_control.rs +++ b/pallets/delegation/src/access_control.rs @@ -23,6 +23,7 @@ use sp_runtime::DispatchError; use attestation::ClaimHashOf; use ctype::CtypeHashOf; +use public_credentials::CredentialIdOf; use crate::{ default_weights::WeightInfo, Config, DelegationHierarchies, DelegationNodeIdOf, DelegationNodes, DelegatorIdOf, @@ -120,6 +121,93 @@ impl } } +// Duplicates the same logic that exists for attestations, and uses the result of `can_revoke()` to define `can_unrevoke()`. +impl + public_credentials::PublicCredentialsAccessControl, DelegationNodeIdOf, CtypeHashOf, CredentialIdOf> + for DelegationAc +{ + fn can_issue( + &self, + who: &DelegatorIdOf, + ctype: &CtypeHashOf, + _credential_id: &CredentialIdOf, + ) -> Result { + let delegation_node = + DelegationNodes::::get(self.authorization_id()).ok_or(Error::::DelegationNotFound)?; + let root = + DelegationHierarchies::::get(delegation_node.hierarchy_root_id).ok_or(Error::::DelegationNotFound)?; + ensure!( + // has permission + ((delegation_node.details.permissions & Permissions::ATTEST) == Permissions::ATTEST) + // not revoked + && !delegation_node.details.revoked + // is owner of delegation + && &delegation_node.details.owner == who + // delegation matches the ctype + && &root.ctype_hash == ctype, + Error::::AccessDenied + ); + + Ok(::WeightInfo::can_attest()) + } + + fn can_revoke( + &self, + who: &DelegatorIdOf, + _ctype: &CtypeHashOf, + _credential_id: &CredentialIdOf, + attester_node_id: &DelegationNodeIdOf, + ) -> Result { + // NOTE: The node IDs of the sender (provided by the user through `who`) and + // attester (provided by the attestation pallet through on-chain storage) can be + // different! + match Pallet::::is_delegating(who, attester_node_id, self.max_checks)? { + (true, checks) => Ok(::WeightInfo::can_revoke(checks)), + _ => Err(Error::::AccessDenied.into()), + } + } + + fn can_unrevoke( + &self, + who: &DelegatorIdOf, + ctype: &CtypeHashOf, + credential_id: &CredentialIdOf, + attester_node_id: &DelegationNodeIdOf, + ) -> Result { + self.can_revoke(who, ctype, credential_id, attester_node_id) + } + + fn can_remove( + &self, + who: &DelegatorIdOf, + ctype: &CtypeHashOf, + credential_id: &CredentialIdOf, + auth_id: &DelegationNodeIdOf, + ) -> Result { + self.can_revoke(who, ctype, credential_id, auth_id) + } + + fn authorization_id(&self) -> DelegationNodeIdOf { + self.subject_node_id + } + + fn can_issue_weight(&self) -> Weight { + ::WeightInfo::can_attest() + } + + fn can_revoke_weight(&self) -> Weight { + ::WeightInfo::can_revoke(self.max_checks) + } + + fn can_unrevoke_weight(&self) -> Weight { + ::WeightInfo::can_revoke(self.max_checks) + } + + fn can_remove_weight(&self) -> Weight { + ::WeightInfo::can_remove(self.max_checks) + } +} + #[cfg(test)] mod tests { use frame_support::{assert_noop, assert_ok}; diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index 2c6a4a6c20..71a7dfdf7f 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -70,7 +70,7 @@ pub mod pallet { }; use frame_system::pallet_prelude::*; use sp_runtime::traits::{Hash, SaturatedConversion}; - use sp_std::vec::Vec; + use sp_std::{boxed::Box, vec::Vec}; pub use ctype::CtypeHashOf; use kilt_support::traits::CallSources; @@ -106,7 +106,7 @@ pub mod pallet { /// The type of account's balances. pub type BalanceOf = as Currency>>::Balance; pub(crate) type AuthorizationIdOf = ::AuthorizationId; - pub(crate) type CredentialIdOf = <::CredentialHash as sp_runtime::traits::Hash>::Output; + pub type CredentialIdOf = <::CredentialHash as sp_runtime::traits::Hash>::Output; /// The type of a public credential as the pallet expects it. pub type InputCredentialOf = diff --git a/runtimes/common/Cargo.toml b/runtimes/common/Cargo.toml index e372b007fc..9dc7d8d2ed 100644 --- a/runtimes/common/Cargo.toml +++ b/runtimes/common/Cargo.toml @@ -18,6 +18,7 @@ smallvec = "1.8.0" attestation = {default-features = false, path = "../../pallets/attestation"} kilt-support = { default-features = false, optional = true, path = "../../support" } parachain-staking = {default-features = false, path = "../../pallets/parachain-staking"} +public-credentials = {default-features = false, path = "../../pallets/public-credentials"} frame-support = {git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.25"} frame-system = {git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.25"} @@ -42,6 +43,7 @@ runtime-benchmarks = [ "frame-support/runtime-benchmarks", "kilt-support/runtime-benchmarks", "parachain-staking/runtime-benchmarks", + "public-credentials/runtime-benchmarks", "sp-runtime/runtime-benchmarks", ] std = [ @@ -56,6 +58,7 @@ std = [ "pallet-balances/std", "pallet-membership/std", "pallet-authorship/std", + "public-credentials/std", "scale-info/std", "serde", "sp-consensus-aura/std", diff --git a/runtimes/common/src/authorization.rs b/runtimes/common/src/authorization.rs index 50b45e1961..3faa105b2b 100644 --- a/runtimes/common/src/authorization.rs +++ b/runtimes/common/src/authorization.rs @@ -21,6 +21,7 @@ use scale_info::TypeInfo; use sp_runtime::DispatchError; use attestation::AttestationAccessControl; +use public_credentials::PublicCredentialsAccessControl; #[derive(Clone, Debug, Encode, Decode, PartialEq, Eq, TypeInfo, MaxEncodedLen)] pub enum AuthorizationId { @@ -98,3 +99,91 @@ where } } } + +impl + PublicCredentialsAccessControl, Ctype, CredentialId> + for PalletAuthorize +where + DelegationAc: PublicCredentialsAccessControl, +{ + fn can_issue( + &self, + who: &AttesterId, + ctype: &Ctype, + credential_id: &CredentialId, + ) -> Result { + match self { + PalletAuthorize::Delegation(ac) => ac.can_issue(who, ctype, credential_id), + } + } + + fn can_revoke( + &self, + who: &AttesterId, + ctype: &Ctype, + credential_id: &CredentialId, + auth_id: &AuthorizationId, + ) -> Result { + match (self, auth_id) { + (PalletAuthorize::Delegation(ac), AuthorizationId::Delegation(auth_id)) => { + ac.can_revoke(who, ctype, credential_id, auth_id) + } // _ => Err(DispatchError::Other("unauthorized")), + } + } + + fn can_unrevoke( + &self, + who: &AttesterId, + ctype: &Ctype, + credential_id: &CredentialId, + auth_id: &AuthorizationId, + ) -> Result { + match (self, auth_id) { + (PalletAuthorize::Delegation(ac), AuthorizationId::Delegation(auth_id)) => { + ac.can_unrevoke(who, ctype, credential_id, auth_id) + } // _ => Err(DispatchError::Other("unauthorized")), + } + } + + fn can_remove( + &self, + who: &AttesterId, + ctype: &Ctype, + credential_id: &CredentialId, + auth_id: &AuthorizationId, + ) -> Result { + match (self, auth_id) { + (PalletAuthorize::Delegation(ac), AuthorizationId::Delegation(auth_id)) => { + ac.can_remove(who, ctype, credential_id, auth_id) + } // _ => Err(DispatchError::Other("unauthorized")), + } + } + + fn authorization_id(&self) -> AuthorizationId { + match self { + PalletAuthorize::Delegation(ac) => AuthorizationId::Delegation(ac.authorization_id()), + } + } + + fn can_issue_weight(&self) -> Weight { + match self { + PalletAuthorize::Delegation(ac) => ac.can_issue_weight(), + } + } + fn can_revoke_weight(&self) -> Weight { + match self { + PalletAuthorize::Delegation(ac) => ac.can_revoke_weight(), + } + } + + fn can_unrevoke_weight(&self) -> Weight { + match self { + PalletAuthorize::Delegation(ac) => ac.can_unrevoke_weight(), + } + } + fn can_remove_weight(&self) -> Weight { + match self { + PalletAuthorize::Delegation(ac) => ac.can_remove_weight(), + } + } +} diff --git a/runtimes/peregrine/src/lib.rs b/runtimes/peregrine/src/lib.rs index 8126e9f668..26117a798d 100644 --- a/runtimes/peregrine/src/lib.rs +++ b/runtimes/peregrine/src/lib.rs @@ -651,19 +651,12 @@ impl pallet_utility::Config for Runtime { impl pallet_randomness_collective_flip::Config for Runtime {} impl public_credentials::Config for Runtime { - type ClaimerIdentifier = did::DidIdentifierOf; - - #[cfg(not(feature = "runtime-benchmarks"))] - type ClaimerSignature = did::DidSignature; - #[cfg(not(feature = "runtime-benchmarks"))] - type ClaimerSignatureVerification = did::DidSignatureVerify; - - #[cfg(feature = "runtime-benchmarks")] - type ClaimerSignature = runtime_common::benchmarks::DummySignature; - #[cfg(feature = "runtime-benchmarks")] - type ClaimerSignatureVerification = - kilt_support::signature::AlwaysVerify, Self::ClaimerSignature>; - + type AccessControl = PalletAuthorize>; + type AttesterId = DidIdentifier; + type AuthorizationId = AuthorizationId<::DelegationNodeId>; + type CredentialId = Hash; + type CredentialHash = BlakeTwo256; + type Currency = Balances; type Deposit = runtime_common::constants::public_credentials::Deposit; type EnsureOrigin = did::EnsureDidOrigin; type MaxEncodedClaimsLength = runtime_common::constants::public_credentials::MaxEncodedClaimsLength; @@ -671,7 +664,7 @@ impl public_credentials::Config for Runtime { type OriginSuccess = did::DidRawOrigin; type Event = Event; type SubjectId = runtime_common::assets::AssetDid; - type WeightInfo = weights::public_credentials::WeightInfo; + type WeightInfo = (); } /// The type used to represent the kinds of proxying allowed. diff --git a/runtimes/spiritnet/src/lib.rs b/runtimes/spiritnet/src/lib.rs index 4d2538408a..9799f02d07 100644 --- a/runtimes/spiritnet/src/lib.rs +++ b/runtimes/spiritnet/src/lib.rs @@ -646,19 +646,12 @@ impl pallet_utility::Config for Runtime { impl pallet_randomness_collective_flip::Config for Runtime {} impl public_credentials::Config for Runtime { - type ClaimerIdentifier = did::DidIdentifierOf; - - #[cfg(not(feature = "runtime-benchmarks"))] - type ClaimerSignature = did::DidSignature; - #[cfg(not(feature = "runtime-benchmarks"))] - type ClaimerSignatureVerification = did::DidSignatureVerify; - - #[cfg(feature = "runtime-benchmarks")] - type ClaimerSignature = runtime_common::benchmarks::DummySignature; - #[cfg(feature = "runtime-benchmarks")] - type ClaimerSignatureVerification = - kilt_support::signature::AlwaysVerify, Self::ClaimerSignature>; - + type AccessControl = PalletAuthorize>; + type AttesterId = DidIdentifier; + type AuthorizationId = AuthorizationId<::DelegationNodeId>; + type CredentialId = Hash; + type CredentialHash = BlakeTwo256; + type Currency = Balances; type Deposit = runtime_common::constants::public_credentials::Deposit; type EnsureOrigin = did::EnsureDidOrigin; type MaxEncodedClaimsLength = runtime_common::constants::public_credentials::MaxEncodedClaimsLength; @@ -666,7 +659,7 @@ impl public_credentials::Config for Runtime { type OriginSuccess = did::DidRawOrigin; type Event = Event; type SubjectId = runtime_common::assets::AssetDid; - type WeightInfo = weights::public_credentials::WeightInfo; + type WeightInfo = (); } /// The type used to represent the kinds of proxying allowed. diff --git a/runtimes/standalone/src/lib.rs b/runtimes/standalone/src/lib.rs index 1abd862bde..5898697aa3 100644 --- a/runtimes/standalone/src/lib.rs +++ b/runtimes/standalone/src/lib.rs @@ -472,19 +472,12 @@ impl pallet_utility::Config for Runtime { impl pallet_randomness_collective_flip::Config for Runtime {} impl public_credentials::Config for Runtime { - type ClaimerIdentifier = did::DidIdentifierOf; - - #[cfg(not(feature = "runtime-benchmarks"))] - type ClaimerSignature = did::DidSignature; - #[cfg(not(feature = "runtime-benchmarks"))] - type ClaimerSignatureVerification = did::DidSignatureVerify; - - #[cfg(feature = "runtime-benchmarks")] - type ClaimerSignature = runtime_common::benchmarks::DummySignature; - #[cfg(feature = "runtime-benchmarks")] - type ClaimerSignatureVerification = - kilt_support::signature::AlwaysVerify, Self::ClaimerSignature>; - + type AccessControl = PalletAuthorize>; + type AttesterId = DidIdentifier; + type AuthorizationId = AuthorizationId<::DelegationNodeId>; + type CredentialId = Hash; + type CredentialHash = BlakeTwo256; + type Currency = Balances; type Deposit = runtime_common::constants::public_credentials::Deposit; type EnsureOrigin = did::EnsureDidOrigin; type MaxEncodedClaimsLength = runtime_common::constants::public_credentials::MaxEncodedClaimsLength; From f63875c6dbb60c12dd1594dbaace6ac0397f0bbc Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 11:58:02 +0200 Subject: [PATCH 10/21] nodes compiling with new filtering feature --- Cargo.lock | 2 + nodes/common/Cargo.toml | 3 +- nodes/common/src/public_credentials.rs | 59 ++++++++++++++++--- nodes/parachain/src/rpc.rs | 10 ++-- nodes/parachain/src/service.rs | 22 +++---- nodes/standalone/src/rpc.rs | 10 ++-- pallets/delegation/src/access_control.rs | 11 +++- rpc/public-credentials/src/lib.rs | 40 +++++++++++-- runtimes/common/src/authorization.rs | 1 + .../src/weights/public_credentials.rs | 22 +++++-- .../src/weights/public_credentials.rs | 10 ++++ 11 files changed, 149 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 40cab65d8e..f4029e6a01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5105,6 +5105,8 @@ version = "1.7.0" dependencies = [ "kilt-support", "public-credentials", + "public-credentials-rpc", + "runtime-common", "serde", ] diff --git a/nodes/common/Cargo.toml b/nodes/common/Cargo.toml index f407fccfa0..7c3bde394d 100644 --- a/nodes/common/Cargo.toml +++ b/nodes/common/Cargo.toml @@ -12,7 +12,8 @@ serde = {version = "1.0.137", features = ["derive"]} # Runtime dependencies kilt-support = {path = "../../support"} public-credentials = {path = "../../pallets/public-credentials"} - +public-credentials-rpc = {path = "../../rpc/public-credentials"} +runtime-common = {path = "../../runtimes/common"} [features] default = [] diff --git a/nodes/common/src/public_credentials.rs b/nodes/common/src/public_credentials.rs index 56e6bc6f05..6f0d82fbb7 100644 --- a/nodes/common/src/public_credentials.rs +++ b/nodes/common/src/public_credentials.rs @@ -21,32 +21,77 @@ use serde::{Deserialize, Serialize}; use kilt_support::deposit::Deposit; use public_credentials::CredentialEntry; +use public_credentials_rpc::PublicCredentialsFilter; + #[derive(Serialize, Deserialize)] #[serde(bound( serialize = " + CTypeHash: Serialize, + Attester: Serialize, BlockNumber: Serialize, + AccountId: Serialize, Balance: std::fmt::Display, - AccountId: Serialize", + AuthorizationId: Serialize", deserialize = " + CTypeHash: Deserialize<'de>, + Attester: Deserialize<'de>, BlockNumber: Deserialize<'de>, - Balance: std::str::FromStr, - AccountId: Deserialize<'de>", + AccountId: Deserialize<'de>, + Balance: std::str::FromStr, + AuthorizationId: Deserialize<'de>" ))] /// Thin wrapper around a runtime credential entry as specified in the /// `public-credentials` pallet. This wrapper implements all the /// (de-)serialization logic. -pub struct OuterCredentialEntry { +pub struct OuterCredentialEntry { + pub ctype_hash: CTypeHash, + pub attester: Attester, + pub revoked: bool, pub block_number: BlockNumber, pub deposit: Deposit, + pub authorization_id: Option, } -impl From> - for OuterCredentialEntry +impl + From> + for OuterCredentialEntry { - fn from(value: CredentialEntry) -> Self { + fn from(value: CredentialEntry) -> Self { Self { + ctype_hash: value.ctype_hash, + attester: value.attester, + revoked: value.revoked, block_number: value.block_number, deposit: value.deposit, + authorization_id: value.authorization_id, + } + } +} + +#[derive(Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum PublicCredentialFilter { + CTypeHash(CTypeHash), + Attester(Attester), + AuthorizationId(AuthorizationId), +} + +impl + PublicCredentialsFilter> + for PublicCredentialFilter +where + CTypeHash: Eq, + Attester: Eq, + AuthorizationId: Eq, +{ + fn should_include( + &self, + credential: &CredentialEntry, + ) -> bool { + match self { + Self::CTypeHash(ctype_hash) => ctype_hash == &credential.ctype_hash, + Self::Attester(attester) => attester == &credential.attester, + Self::AuthorizationId(authorization_id) => Some(authorization_id) == credential.authorization_id.as_ref(), } } } diff --git a/nodes/parachain/src/rpc.rs b/nodes/parachain/src/rpc.rs index 0e2fa31e26..d536116c30 100644 --- a/nodes/parachain/src/rpc.rs +++ b/nodes/parachain/src/rpc.rs @@ -36,7 +36,7 @@ use sp_blockchain::{Error as BlockChainError, HeaderBackend, HeaderMetadata}; use pallet_did_lookup::linkable_account::LinkableAccountId; use public_credentials::CredentialEntry; -use runtime_common::{AccountId, Balance, Block, BlockNumber, DidIdentifier, Hash, Index}; +use runtime_common::{authorization::AuthorizationId, AccountId, Balance, Block, BlockNumber, DidIdentifier, Hash, Index}; /// Full client dependencies. pub struct FullDeps { @@ -62,7 +62,7 @@ where Block, runtime_common::assets::AssetDid, Hash, - CredentialEntry, + CredentialEntry>, >, P: TransactionPool + 'static, { @@ -99,9 +99,11 @@ where // Runtime credential ID Hash, // Input/output credential entry - node_common::OuterCredentialEntry, + node_common::OuterCredentialEntry>, // Runtime credential entry - CredentialEntry, + CredentialEntry>, + // Credential filter + node_common::PublicCredentialFilter> >::new(client) .into_rpc(), )?; diff --git a/nodes/parachain/src/service.rs b/nodes/parachain/src/service.rs index 68d359bfdb..63ca9c12eb 100644 --- a/nodes/parachain/src/service.rs +++ b/nodes/parachain/src/service.rs @@ -46,7 +46,7 @@ use sp_runtime::traits::BlakeTwo256; use pallet_did_lookup::linkable_account::LinkableAccountId; use public_credentials::CredentialEntry; -use runtime_common::{AccountId, AuthorityId, Balance, BlockNumber, DidIdentifier, Index}; +use runtime_common::{authorization::AuthorizationId, AccountId, AuthorityId, Balance, BlockNumber, DidIdentifier, Index}; type Header = sp_runtime::generic::Header; pub type Block = sp_runtime::generic::Block; @@ -244,11 +244,11 @@ where + frame_rpc_system::AccountNonceApi + did_rpc::DidRuntimeApi + public_credentials_rpc::PublicCredentialsRuntimeApi< - Block, - runtime_common::assets::AssetDid, - Hash, - CredentialEntry, - >, + Block, + runtime_common::assets::AssetDid, + Hash, + CredentialEntry>, + >, sc_client_api::StateBackendFor, Block>: sp_api::StateBackend, Executor: sc_executor::NativeExecutionDispatch + 'static, RB: FnOnce( @@ -493,11 +493,11 @@ where + cumulus_primitives_core::CollectCollationInfo + did_rpc::DidRuntimeApi + public_credentials_rpc::PublicCredentialsRuntimeApi< - Block, - runtime_common::assets::AssetDid, - Hash, - CredentialEntry, - >, + Block, + runtime_common::assets::AssetDid, + Hash, + CredentialEntry>, + >, sc_client_api::StateBackendFor, Block>: sp_api::StateBackend, { start_node_impl::( diff --git a/nodes/standalone/src/rpc.rs b/nodes/standalone/src/rpc.rs index ee3d1d53b3..14a7e9a9bd 100644 --- a/nodes/standalone/src/rpc.rs +++ b/nodes/standalone/src/rpc.rs @@ -36,7 +36,7 @@ use sp_blockchain::{Error as BlockChainError, HeaderBackend, HeaderMetadata}; use pallet_did_lookup::linkable_account::LinkableAccountId; use public_credentials::CredentialEntry; -use runtime_common::{AccountId, Balance, Block, BlockNumber, DidIdentifier, Hash, Index}; +use runtime_common::{authorization::AuthorizationId, AccountId, Balance, Block, BlockNumber, DidIdentifier, Hash, Index}; /// Full client dependencies. pub struct FullDeps { @@ -62,7 +62,7 @@ where Block, runtime_common::assets::AssetDid, Hash, - CredentialEntry, + CredentialEntry>, >, P: TransactionPool + 'static, { @@ -100,9 +100,11 @@ where // Runtime credential ID Hash, // Input/output credential entry - node_common::OuterCredentialEntry, + node_common::OuterCredentialEntry>, // Runtime credential entry - CredentialEntry, + CredentialEntry>, + // Credential filter + node_common::PublicCredentialFilter> >::new(client) .into_rpc(), )?; diff --git a/pallets/delegation/src/access_control.rs b/pallets/delegation/src/access_control.rs index 794c82b996..c0106a88dc 100644 --- a/pallets/delegation/src/access_control.rs +++ b/pallets/delegation/src/access_control.rs @@ -121,10 +121,15 @@ impl } } -// Duplicates the same logic that exists for attestations, and uses the result of `can_revoke()` to define `can_unrevoke()`. +// Duplicates the same logic that exists for attestations, and uses the result +// of `can_revoke()` to define `can_unrevoke()`. impl - public_credentials::PublicCredentialsAccessControl, DelegationNodeIdOf, CtypeHashOf, CredentialIdOf> - for DelegationAc + public_credentials::PublicCredentialsAccessControl< + DelegatorIdOf, + DelegationNodeIdOf, + CtypeHashOf, + CredentialIdOf, + > for DelegationAc { fn can_issue( &self, diff --git a/rpc/public-credentials/src/lib.rs b/rpc/public-credentials/src/lib.rs index ecb868b0b1..b1a00000a0 100644 --- a/rpc/public-credentials/src/lib.rs +++ b/rpc/public-credentials/src/lib.rs @@ -31,8 +31,12 @@ use sp_api::ProvideRuntimeApi; use sp_blockchain::HeaderBackend; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; +pub trait PublicCredentialsFilter { + fn should_include(&self, credential: &Credential) -> bool; +} + #[rpc(client, server)] -pub trait PublicCredentialsApi { +pub trait PublicCredentialsApi { /// Return a credential that matches the provided root hash and issued to /// the provided subject, if found. #[method(name = "get_credential")] @@ -43,12 +47,14 @@ pub trait PublicCredentialsApi, ) -> RpcResult>; - /// Return all the credentials issued to the provided subject. - /// The result is a vector of (credential root hash, credential entry). + /// Return all the credentials issued to the provided subject, optionally + /// filtering with the provided logic. The result is a vector of (credential + /// root hash, credential entry). #[method(name = "get_credentials")] fn get_credentials( &self, subject: OuterSubjectId, + filter: Option, at: Option, ) -> RpcResult>; } @@ -62,6 +68,7 @@ pub struct PublicCredentialsQuery< CredentialId, OuterCredentialEntry, CredentialEntry, + CredentialFilter, > { client: Arc, _marker: std::marker::PhantomData<( @@ -72,6 +79,7 @@ pub struct PublicCredentialsQuery< CredentialId, OuterCredentialEntry, CredentialEntry, + CredentialFilter, )>, } @@ -84,6 +92,7 @@ impl< CredentialId, OuterCredentialEntry, CredentialEntry, + CredentialFilter, > PublicCredentialsQuery< Client, @@ -94,6 +103,7 @@ impl< CredentialId, OuterCredentialEntry, CredentialEntry, + CredentialFilter, > { pub fn new(client: Arc) -> Self { @@ -121,7 +131,15 @@ impl< CredentialId, OuterCredentialEntry, CredentialEntry, - > PublicCredentialsApiServer<::Hash, OuterSubjectId, OuterCredentialId, OuterCredentialEntry> + CredentialFilter, + > + PublicCredentialsApiServer< + ::Hash, + OuterSubjectId, + OuterCredentialId, + OuterCredentialEntry, + CredentialFilter, + > for PublicCredentialsQuery< Client, Block, @@ -131,6 +149,7 @@ impl< CredentialId, OuterCredentialEntry, CredentialEntry, + CredentialFilter, > where Block: BlockT, Client: Send + Sync + 'static + ProvideRuntimeApi + HeaderBackend, @@ -141,6 +160,7 @@ impl< CredentialId: Codec + Send + Sync + 'static + TryFrom + Into, CredentialEntry: Codec + Send + Sync + 'static, OuterCredentialEntry: Send + Sync + 'static + From, + CredentialFilter: Send + Sync + 'static + PublicCredentialsFilter, { fn get_credential( &self, @@ -180,6 +200,7 @@ impl< fn get_credentials( &self, subject: OuterSubjectId, + filter: Option, at: Option<::Hash>, ) -> RpcResult> { let api = self.client.runtime_api(); @@ -201,7 +222,16 @@ impl< )) })?; - Ok(credentials + let filtered_credentials = if let Some(filter) = filter { + credentials + .into_iter() + .filter(|(_, credential_entry)| filter.should_include(credential_entry)) + .collect() + } else { + credentials + }; + + Ok(filtered_credentials .into_iter() .map(|(id, entry)| (id.into(), entry.into())) .collect()) diff --git a/runtimes/common/src/authorization.rs b/runtimes/common/src/authorization.rs index 3faa105b2b..b1f6833868 100644 --- a/runtimes/common/src/authorization.rs +++ b/runtimes/common/src/authorization.rs @@ -24,6 +24,7 @@ use attestation::AttestationAccessControl; use public_credentials::PublicCredentialsAccessControl; #[derive(Clone, Debug, Encode, Decode, PartialEq, Eq, TypeInfo, MaxEncodedLen)] +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] pub enum AuthorizationId { Delegation(DelegationId), } diff --git a/runtimes/peregrine/src/weights/public_credentials.rs b/runtimes/peregrine/src/weights/public_credentials.rs index 7a37d5f50f..1d395f17e6 100644 --- a/runtimes/peregrine/src/weights/public_credentials.rs +++ b/runtimes/peregrine/src/weights/public_credentials.rs @@ -20,13 +20,13 @@ //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev //! DATE: 2022-07-12, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 +//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("spiritnet-dev"), DB CACHE: 1024 // Executed Command: // ./target/release/kilt-parachain // benchmark // pallet -// --chain=dev +// --chain=spiritnet-dev // --steps=50 // --repeat=20 // --pallet=public-credentials @@ -35,7 +35,7 @@ // --wasm-execution=compiled // --heap-pages=4096 // --record-proof -// --output=./runtimes/peregrine/src/weights/public_credentials.rs +// --output=./runtimes/spiritnet/src/weights/public_credentials.rs // --template=.maintain/runtime-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] @@ -55,7 +55,7 @@ impl public_credentials::WeightInfo for WeightInfo { // Storage: Attestation Attestations (r:1 w:1) // Storage: PublicCredentials Credentials (r:0 w:1) fn add(_c: u32, ) -> Weight { - (68_813_000 as Weight) + (62_093_000 as Weight) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } @@ -64,7 +64,17 @@ impl public_credentials::WeightInfo for WeightInfo { // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) fn remove() -> Weight { - (71_537_000 as Weight) + (57_888_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } + fn revoke() -> Weight { + (57_888_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } + fn unrevoke() -> Weight { + (57_888_000 as Weight) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } @@ -73,7 +83,7 @@ impl public_credentials::WeightInfo for WeightInfo { // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) fn reclaim_deposit() -> Weight { - (72_183_000 as Weight) + (59_140_000 as Weight) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } diff --git a/runtimes/spiritnet/src/weights/public_credentials.rs b/runtimes/spiritnet/src/weights/public_credentials.rs index c446fa4a16..1d395f17e6 100644 --- a/runtimes/spiritnet/src/weights/public_credentials.rs +++ b/runtimes/spiritnet/src/weights/public_credentials.rs @@ -68,6 +68,16 @@ impl public_credentials::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } + fn revoke() -> Weight { + (57_888_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } + fn unrevoke() -> Weight { + (57_888_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) // Storage: Attestation Attestations (r:1 w:1) From adf053a13678bbe7d2c53dffa14edfffbb691545 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 12:29:00 +0200 Subject: [PATCH 11/21] Remove authorization id from possible filters --- nodes/common/src/public_credentials.rs | 7 ++----- nodes/parachain/src/rpc.rs | 15 ++++++++++++--- nodes/standalone/src/rpc.rs | 15 ++++++++++++--- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/nodes/common/src/public_credentials.rs b/nodes/common/src/public_credentials.rs index 6f0d82fbb7..6fa9ad04e9 100644 --- a/nodes/common/src/public_credentials.rs +++ b/nodes/common/src/public_credentials.rs @@ -70,19 +70,17 @@ impl #[derive(Serialize, Deserialize)] #[serde(rename_all = "snake_case")] -pub enum PublicCredentialFilter { +pub enum PublicCredentialFilter { CTypeHash(CTypeHash), Attester(Attester), - AuthorizationId(AuthorizationId), } impl PublicCredentialsFilter> - for PublicCredentialFilter + for PublicCredentialFilter where CTypeHash: Eq, Attester: Eq, - AuthorizationId: Eq, { fn should_include( &self, @@ -91,7 +89,6 @@ where match self { Self::CTypeHash(ctype_hash) => ctype_hash == &credential.ctype_hash, Self::Attester(attester) => attester == &credential.attester, - Self::AuthorizationId(authorization_id) => Some(authorization_id) == credential.authorization_id.as_ref(), } } } diff --git a/nodes/parachain/src/rpc.rs b/nodes/parachain/src/rpc.rs index d536116c30..b1bb9dc43f 100644 --- a/nodes/parachain/src/rpc.rs +++ b/nodes/parachain/src/rpc.rs @@ -36,7 +36,9 @@ use sp_blockchain::{Error as BlockChainError, HeaderBackend, HeaderMetadata}; use pallet_did_lookup::linkable_account::LinkableAccountId; use public_credentials::CredentialEntry; -use runtime_common::{authorization::AuthorizationId, AccountId, Balance, Block, BlockNumber, DidIdentifier, Hash, Index}; +use runtime_common::{ + authorization::AuthorizationId, AccountId, Balance, Block, BlockNumber, DidIdentifier, Hash, Index, +}; /// Full client dependencies. pub struct FullDeps { @@ -99,11 +101,18 @@ where // Runtime credential ID Hash, // Input/output credential entry - node_common::OuterCredentialEntry>, + node_common::OuterCredentialEntry< + Hash, + DidIdentifier, + BlockNumber, + AccountId, + Balance, + AuthorizationId, + >, // Runtime credential entry CredentialEntry>, // Credential filter - node_common::PublicCredentialFilter> + node_common::PublicCredentialFilter, >::new(client) .into_rpc(), )?; diff --git a/nodes/standalone/src/rpc.rs b/nodes/standalone/src/rpc.rs index 14a7e9a9bd..b69c9f922a 100644 --- a/nodes/standalone/src/rpc.rs +++ b/nodes/standalone/src/rpc.rs @@ -36,7 +36,9 @@ use sp_blockchain::{Error as BlockChainError, HeaderBackend, HeaderMetadata}; use pallet_did_lookup::linkable_account::LinkableAccountId; use public_credentials::CredentialEntry; -use runtime_common::{authorization::AuthorizationId, AccountId, Balance, Block, BlockNumber, DidIdentifier, Hash, Index}; +use runtime_common::{ + authorization::AuthorizationId, AccountId, Balance, Block, BlockNumber, DidIdentifier, Hash, Index, +}; /// Full client dependencies. pub struct FullDeps { @@ -100,11 +102,18 @@ where // Runtime credential ID Hash, // Input/output credential entry - node_common::OuterCredentialEntry>, + node_common::OuterCredentialEntry< + Hash, + DidIdentifier, + BlockNumber, + AccountId, + Balance, + AuthorizationId, + >, // Runtime credential entry CredentialEntry>, // Credential filter - node_common::PublicCredentialFilter> + node_common::PublicCredentialFilter, >::new(client) .into_rpc(), )?; From 4e9ae3e8cef8509fd193fbc402ac1083fe928c12 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 12:30:18 +0200 Subject: [PATCH 12/21] fmt --- nodes/parachain/src/service.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/nodes/parachain/src/service.rs b/nodes/parachain/src/service.rs index 63ca9c12eb..dbce9093a6 100644 --- a/nodes/parachain/src/service.rs +++ b/nodes/parachain/src/service.rs @@ -46,7 +46,9 @@ use sp_runtime::traits::BlakeTwo256; use pallet_did_lookup::linkable_account::LinkableAccountId; use public_credentials::CredentialEntry; -use runtime_common::{authorization::AuthorizationId, AccountId, AuthorityId, Balance, BlockNumber, DidIdentifier, Index}; +use runtime_common::{ + authorization::AuthorizationId, AccountId, AuthorityId, Balance, BlockNumber, DidIdentifier, Index, +}; type Header = sp_runtime::generic::Header; pub type Block = sp_runtime::generic::Block; @@ -244,11 +246,11 @@ where + frame_rpc_system::AccountNonceApi + did_rpc::DidRuntimeApi + public_credentials_rpc::PublicCredentialsRuntimeApi< - Block, - runtime_common::assets::AssetDid, - Hash, - CredentialEntry>, - >, + Block, + runtime_common::assets::AssetDid, + Hash, + CredentialEntry>, + >, sc_client_api::StateBackendFor, Block>: sp_api::StateBackend, Executor: sc_executor::NativeExecutionDispatch + 'static, RB: FnOnce( @@ -493,11 +495,11 @@ where + cumulus_primitives_core::CollectCollationInfo + did_rpc::DidRuntimeApi + public_credentials_rpc::PublicCredentialsRuntimeApi< - Block, - runtime_common::assets::AssetDid, - Hash, - CredentialEntry>, - >, + Block, + runtime_common::assets::AssetDid, + Hash, + CredentialEntry>, + >, sc_client_api::StateBackendFor, Block>: sp_api::StateBackend, { start_node_impl::( From de14b052f641038109962ccd760af1fc9eba0502 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 13:16:26 +0200 Subject: [PATCH 13/21] update benchmarks and default weights (to be re-executed on bench machine) --- .../public-credentials/src/benchmarking.rs | 21 ++-- .../public-credentials/src/default_weights.rs | 114 +++++++++--------- .../src/weights/public_credentials.rs | 63 +++++----- .../src/weights/public_credentials.rs | 57 ++++----- 4 files changed, 135 insertions(+), 120 deletions(-) diff --git a/pallets/public-credentials/src/benchmarking.rs b/pallets/public-credentials/src/benchmarking.rs index 804987c854..3088c9d873 100644 --- a/pallets/public-credentials/src/benchmarking.rs +++ b/pallets/public-credentials/src/benchmarking.rs @@ -60,7 +60,8 @@ benchmarks! { let credential_id = generate_credential_id::(&creation_op, &attester); ctype::Ctypes::::insert(&ctype_hash, attester.clone()); - CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); + // Has to be more than the deposit, we do 2x just to be safe + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get() + ::Deposit::get()); let origin = ::EnsureOrigin::generate_origin(sender, attester); }: _(origin, creation_op) verify { @@ -84,15 +85,15 @@ benchmarks! { )); let credential_id = generate_credential_id::(&creation_op, &attester); - CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); + // Has to be more than the deposit, we do 2x just to be safe + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get() + ::Deposit::get()); ctype::Ctypes::::insert(&ctype_hash, attester); Pallet::::add(origin.clone(), creation_op).expect("Pallet::add should not fail"); let credential_id_clone = credential_id.clone(); }: _(origin, credential_id_clone, None) verify { - assert!(!Credentials::::contains_key(subject_id, &credential_id)); - assert!(!CredentialSubjects::::contains_key(credential_id)); + assert!(Credentials::::get(subject_id, &credential_id).expect("Credential should be present in storage").revoked); } unrevoke { @@ -111,7 +112,8 @@ benchmarks! { )); let credential_id = generate_credential_id::(&creation_op, &attester); - CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); + // Has to be more than the deposit, we do 2x just to be safe + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get() + ::Deposit::get()); ctype::Ctypes::::insert(&ctype_hash, attester); Pallet::::add(origin.clone(), creation_op).expect("Pallet::add should not fail"); @@ -119,8 +121,7 @@ benchmarks! { let credential_id_clone = credential_id.clone(); }: _(origin, credential_id_clone, None) verify { - assert!(!Credentials::::contains_key(subject_id, &credential_id)); - assert!(!CredentialSubjects::::contains_key(credential_id)); + assert!(!Credentials::::get(subject_id, &credential_id).expect("Credential should be present in storage").revoked); } remove { @@ -139,7 +140,8 @@ benchmarks! { )); let credential_id = generate_credential_id::(&creation_op, &attester); - CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); + // Has to be more than the deposit, we do 2x just to be safe + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get() + ::Deposit::get()); ctype::Ctypes::::insert(&ctype_hash, attester); Pallet::::add(origin.clone(), creation_op).expect("Pallet::add should not fail"); @@ -166,7 +168,8 @@ benchmarks! { )); let credential_id = generate_credential_id::(&creation_op, &attester); - CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get()); + // Has to be more than the deposit, we do 2x just to be safe + CurrencyOf::::make_free_balance_be(&sender, ::Deposit::get() + ::Deposit::get()); ctype::Ctypes::::insert(&ctype_hash, attester); Pallet::::add(origin, creation_op).expect("Pallet::add should not fail"); diff --git a/pallets/public-credentials/src/default_weights.rs b/pallets/public-credentials/src/default_weights.rs index 74eec21d14..9ffb4b2013 100644 --- a/pallets/public-credentials/src/default_weights.rs +++ b/pallets/public-credentials/src/default_weights.rs @@ -19,7 +19,7 @@ //! Autogenerated weights for public_credentials //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-07-12, STEPS: {{cmd.steps}}\, REPEAT: {{cmd.repeat}}\, LOW RANGE: {{cmd.lowest_range_values}}\, HIGH RANGE: {{cmd.highest_range_values}}\ +//! DATE: 2022-08-03, STEPS: {{cmd.steps}}\, REPEAT: {{cmd.repeat}}\, LOW RANGE: {{cmd.lowest_range_values}}\, HIGH RANGE: {{cmd.highest_range_values}}\ //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: @@ -49,93 +49,99 @@ use sp_std::marker::PhantomData; /// Weight functions needed for public_credentials. pub trait WeightInfo { fn add(c: u32, ) -> Weight; - fn remove() -> Weight; fn revoke() -> Weight; fn unrevoke() -> Weight; + fn remove() -> Weight; fn reclaim_deposit() -> Weight; } /// Weights for public_credentials using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) - // Storage: System Account (r:1 w:1) // Storage: Ctype Ctypes (r:1 w:0) - // Storage: Attestation Attestations (r:1 w:1) - // Storage: PublicCredentials Credentials (r:0 w:1) - fn add(_c: u32, ) -> Weight { - (35_683_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) - } - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) - // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) - fn remove() -> Weight { - (35_041_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + // Storage: PublicCredentials CredentialSubjects (r:0 w:1) + fn add(c: u32, ) -> Weight { + (38_122_000 as Weight) + // Standard Error: 0 + .saturating_add((1_000 as Weight).saturating_mul(c as Weight)) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } + // Storage: PublicCredentials CredentialSubjects (r:1 w:0) + // Storage: PublicCredentials Credentials (r:1 w:1) fn revoke() -> Weight { - (35_041_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (19_440_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) } + // Storage: PublicCredentials CredentialSubjects (r:1 w:0) + // Storage: PublicCredentials Credentials (r:1 w:1) fn unrevoke() -> Weight { - (35_041_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (19_376_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } + // Storage: PublicCredentials CredentialSubjects (r:1 w:1) + // Storage: PublicCredentials Credentials (r:1 w:1) + // Storage: System Account (r:1 w:1) + fn remove() -> Weight { + (32_521_000 as Weight) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) + // Storage: PublicCredentials CredentialSubjects (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) - // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) fn reclaim_deposit() -> Weight { - (34_912_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (33_120_000 as Weight) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } } // For backwards compatibility and tests impl WeightInfo for () { - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) - // Storage: System Account (r:1 w:1) // Storage: Ctype Ctypes (r:1 w:0) - // Storage: Attestation Attestations (r:1 w:1) - // Storage: PublicCredentials Credentials (r:0 w:1) - fn add(_c: u32, ) -> Weight { - (35_683_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) - } - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) - // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) - fn remove() -> Weight { - (35_041_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + // Storage: PublicCredentials CredentialSubjects (r:0 w:1) + fn add(c: u32, ) -> Weight { + (38_122_000 as Weight) + // Standard Error: 0 + .saturating_add((1_000 as Weight).saturating_mul(c as Weight)) + .saturating_add(RocksDbWeight::get().reads(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } + // Storage: PublicCredentials CredentialSubjects (r:1 w:0) + // Storage: PublicCredentials Credentials (r:1 w:1) fn revoke() -> Weight { - (35_041_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + (19_440_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } + // Storage: PublicCredentials CredentialSubjects (r:1 w:0) + // Storage: PublicCredentials Credentials (r:1 w:1) fn unrevoke() -> Weight { - (35_041_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + (19_376_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + } + // Storage: PublicCredentials CredentialSubjects (r:1 w:1) + // Storage: PublicCredentials Credentials (r:1 w:1) + // Storage: System Account (r:1 w:1) + fn remove() -> Weight { + (32_521_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) + // Storage: PublicCredentials CredentialSubjects (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) - // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) fn reclaim_deposit() -> Weight { - (34_912_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + (33_120_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } } diff --git a/runtimes/peregrine/src/weights/public_credentials.rs b/runtimes/peregrine/src/weights/public_credentials.rs index 1d395f17e6..567406b20a 100644 --- a/runtimes/peregrine/src/weights/public_credentials.rs +++ b/runtimes/peregrine/src/weights/public_credentials.rs @@ -19,14 +19,14 @@ //! Autogenerated weights for public_credentials //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-07-12, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("spiritnet-dev"), DB CACHE: 1024 +//! DATE: 2022-08-03, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: // ./target/release/kilt-parachain // benchmark // pallet -// --chain=spiritnet-dev +// --chain=dev // --steps=50 // --repeat=20 // --pallet=public-credentials @@ -35,7 +35,7 @@ // --wasm-execution=compiled // --heap-pages=4096 // --record-proof -// --output=./runtimes/spiritnet/src/weights/public_credentials.rs +// --output=./runtimes/peregrine/src/weights/public_credentials.rs // --template=.maintain/runtime-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] @@ -49,42 +49,45 @@ use sp_std::marker::PhantomData; /// Weight functions for `public_credentials`. pub struct WeightInfo(PhantomData); impl public_credentials::WeightInfo for WeightInfo { - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) - // Storage: System Account (r:1 w:1) // Storage: Ctype Ctypes (r:1 w:0) - // Storage: Attestation Attestations (r:1 w:1) - // Storage: PublicCredentials Credentials (r:0 w:1) - fn add(_c: u32, ) -> Weight { - (62_093_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) - } - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) - // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) - fn remove() -> Weight { - (57_888_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + // Storage: PublicCredentials CredentialSubjects (r:0 w:1) + fn add(c: u32, ) -> Weight { + (64_513_000 as Weight) + // Standard Error: 0 + .saturating_add((2_000 as Weight).saturating_mul(c as Weight)) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } + // Storage: PublicCredentials CredentialSubjects (r:1 w:0) + // Storage: PublicCredentials Credentials (r:1 w:1) fn revoke() -> Weight { - (57_888_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (39_447_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) } + // Storage: PublicCredentials CredentialSubjects (r:1 w:0) + // Storage: PublicCredentials Credentials (r:1 w:1) fn unrevoke() -> Weight { - (57_888_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (39_183_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } + // Storage: PublicCredentials CredentialSubjects (r:1 w:1) + // Storage: PublicCredentials Credentials (r:1 w:1) + // Storage: System Account (r:1 w:1) + fn remove() -> Weight { + (62_565_000 as Weight) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) + // Storage: PublicCredentials CredentialSubjects (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) - // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) fn reclaim_deposit() -> Weight { - (59_140_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (62_648_000 as Weight) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } } diff --git a/runtimes/spiritnet/src/weights/public_credentials.rs b/runtimes/spiritnet/src/weights/public_credentials.rs index 1d395f17e6..aca5f7a033 100644 --- a/runtimes/spiritnet/src/weights/public_credentials.rs +++ b/runtimes/spiritnet/src/weights/public_credentials.rs @@ -19,7 +19,7 @@ //! Autogenerated weights for public_credentials //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-07-12, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2022-08-03, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("spiritnet-dev"), DB CACHE: 1024 // Executed Command: @@ -49,42 +49,45 @@ use sp_std::marker::PhantomData; /// Weight functions for `public_credentials`. pub struct WeightInfo(PhantomData); impl public_credentials::WeightInfo for WeightInfo { - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) - // Storage: System Account (r:1 w:1) // Storage: Ctype Ctypes (r:1 w:0) - // Storage: Attestation Attestations (r:1 w:1) - // Storage: PublicCredentials Credentials (r:0 w:1) - fn add(_c: u32, ) -> Weight { - (62_093_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) - } - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) - // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) - fn remove() -> Weight { - (57_888_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + // Storage: PublicCredentials CredentialSubjects (r:0 w:1) + fn add(c: u32, ) -> Weight { + (33_401_000 as Weight) + // Standard Error: 0 + .saturating_add((1_000 as Weight).saturating_mul(c as Weight)) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } + // Storage: PublicCredentials CredentialSubjects (r:1 w:0) + // Storage: PublicCredentials Credentials (r:1 w:1) fn revoke() -> Weight { - (57_888_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (19_510_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) } + // Storage: PublicCredentials CredentialSubjects (r:1 w:0) + // Storage: PublicCredentials Credentials (r:1 w:1) fn unrevoke() -> Weight { - (57_888_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (19_072_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } + // Storage: PublicCredentials CredentialSubjects (r:1 w:1) + // Storage: PublicCredentials Credentials (r:1 w:1) + // Storage: System Account (r:1 w:1) + fn remove() -> Weight { + (32_204_000 as Weight) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } - // Storage: PublicCredentials CredentialsUnicityIndex (r:1 w:1) + // Storage: PublicCredentials CredentialSubjects (r:1 w:1) // Storage: PublicCredentials Credentials (r:1 w:1) - // Storage: Attestation Attestations (r:1 w:1) // Storage: System Account (r:1 w:1) fn reclaim_deposit() -> Weight { - (59_140_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + (32_430_000 as Weight) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } } From eed9c4d100e9b93891956422a59be35e1f008e39 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 13:30:19 +0200 Subject: [PATCH 14/21] change serialization logic for ctype filter --- nodes/common/src/public_credentials.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nodes/common/src/public_credentials.rs b/nodes/common/src/public_credentials.rs index 6fa9ad04e9..f21d4abc1d 100644 --- a/nodes/common/src/public_credentials.rs +++ b/nodes/common/src/public_credentials.rs @@ -71,7 +71,7 @@ impl #[derive(Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum PublicCredentialFilter { - CTypeHash(CTypeHash), + CtypeHash(CTypeHash), Attester(Attester), } From 72600a8e5a957fedd855ece899c0db5e263940ef Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 15:24:34 +0200 Subject: [PATCH 15/21] fixes and chores --- nodes/common/src/public_credentials.rs | 25 +++------- .../public-credentials/src/benchmarking.rs | 2 + pallets/public-credentials/src/lib.rs | 50 ++++++++++--------- rpc/public-credentials/src/lib.rs | 4 +- runtimes/common/Cargo.toml | 1 - runtimes/common/src/constants.rs | 2 +- runtimes/peregrine/src/lib.rs | 2 +- runtimes/spiritnet/src/lib.rs | 2 +- 8 files changed, 42 insertions(+), 46 deletions(-) diff --git a/nodes/common/src/public_credentials.rs b/nodes/common/src/public_credentials.rs index f21d4abc1d..61bc698911 100644 --- a/nodes/common/src/public_credentials.rs +++ b/nodes/common/src/public_credentials.rs @@ -24,22 +24,6 @@ use public_credentials::CredentialEntry; use public_credentials_rpc::PublicCredentialsFilter; #[derive(Serialize, Deserialize)] -#[serde(bound( - serialize = " - CTypeHash: Serialize, - Attester: Serialize, - BlockNumber: Serialize, - AccountId: Serialize, - Balance: std::fmt::Display, - AuthorizationId: Serialize", - deserialize = " - CTypeHash: Deserialize<'de>, - Attester: Deserialize<'de>, - BlockNumber: Deserialize<'de>, - AccountId: Deserialize<'de>, - Balance: std::str::FromStr, - AuthorizationId: Deserialize<'de>" -))] /// Thin wrapper around a runtime credential entry as specified in the /// `public-credentials` pallet. This wrapper implements all the /// (de-)serialization logic. @@ -48,6 +32,10 @@ pub struct OuterCredentialEntry, pub authorization_id: Option, } @@ -68,10 +56,13 @@ impl } } +/// Filter for public credentials retrieved for a provided subject as specified in the RPC interface. #[derive(Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum PublicCredentialFilter { + /// Filter credentials that match a specified Ctype. CtypeHash(CTypeHash), + /// Filter credentials that have been issued by the specified attester. Attester(Attester), } @@ -87,7 +78,7 @@ where credential: &CredentialEntry, ) -> bool { match self { - Self::CTypeHash(ctype_hash) => ctype_hash == &credential.ctype_hash, + Self::CtypeHash(ctype_hash) => ctype_hash == &credential.ctype_hash, Self::Attester(attester) => attester == &credential.attester, } } diff --git a/pallets/public-credentials/src/benchmarking.rs b/pallets/public-credentials/src/benchmarking.rs index 3088c9d873..2f4817b1a7 100644 --- a/pallets/public-credentials/src/benchmarking.rs +++ b/pallets/public-credentials/src/benchmarking.rs @@ -69,6 +69,7 @@ benchmarks! { assert!(CredentialSubjects::::contains_key(&credential_id)); } + // Very similar setup as `remove` revoke { let sender: T::AccountId = account("sender", 0, SEED); let attester: T::AttesterId = account("attester", 0, SEED); @@ -96,6 +97,7 @@ benchmarks! { assert!(Credentials::::get(subject_id, &credential_id).expect("Credential should be present in storage").revoked); } + // Very similar setup as `remove` unrevoke { let sender: T::AccountId = account("sender", 0, SEED); let attester: T::AttesterId = account("attester", 0, SEED); diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index 71a7dfdf7f..d2fb5785f7 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -136,7 +136,7 @@ pub mod pallet { type CredentialHash: Hash; /// The type of a credential identifier. type CredentialId: Parameter + MaxEncodedLen; - /// The currency that is used to reserve funds for each attestation. + /// The currency that is used to reserve funds for each credential. type Currency: ReservableCurrency>; /// The type of the origin when successfully converted from the outer /// origin. @@ -272,7 +272,7 @@ pub mod pallet { ctype::Error::::CTypeNotFound ); - // Credential ID = H( || ) + // Credential ID = H( || ) let credential_id = T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); @@ -359,16 +359,17 @@ pub mod pallet { if let Some(credential) = credential_entry { // Additional weight is 0 if the caller is the attester, otherwise it's the // value returned by the access control check, if it does not fail. - let additional_weight = if caller != credential.attester { + let additional_weight = if caller == credential.attester { + 0 + } else { let credential_auth_id = credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; authorization .ok_or(Error::::Unauthorized)? .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) .map_err(|_| Error::::Unauthorized)? - } else { - 0 }; + // If authorization checks are ok, update the revocation status. credential.revoked = true; Ok(additional_weight) } else { @@ -414,16 +415,17 @@ pub mod pallet { if let Some(credential) = credential_entry { // Additional weight is 0 if the caller is the attester, otherwise it's the // value returned by the access control check, if it does not fail. - let additional_weight = if caller != credential.attester { + let additional_weight = if caller == credential.attester { + 0 + } else { let credential_auth_id = credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; authorization .ok_or(Error::::Unauthorized)? .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) .map_err(|_| Error::::Unauthorized)? - } else { - 0 }; + // If authorization checks are ok, update the revocation status. credential.revoked = false; Ok(additional_weight) } else { @@ -469,7 +471,9 @@ pub mod pallet { let (credential_subject, credential_entry) = Self::retrieve_credential_entry(&credential_id)?; - let ac_weight_used = if credential_entry.attester != caller { + let ac_weight_used = if credential_entry.attester == caller { + 0 + } else { let credential_auth_id = credential_entry .authorization_id .as_ref() @@ -483,8 +487,6 @@ pub mod pallet { credential_auth_id, ) .map_err(|_| Error::::Unauthorized)? - } else { - 0 }; Self::remove_credential_entry(credential_subject, credential_id, credential_entry); @@ -511,19 +513,6 @@ pub mod pallet { } impl Pallet { - fn retrieve_credential_entry( - credential_id: &CredentialIdOf, - ) -> Result<(T::SubjectId, CredentialEntryOf), Error> { - // Verify that the credential exists - let credential_subject = - CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; - - // Should never happen if the line above succeeds - Credentials::::get(&credential_subject, &credential_id) - .map(|entry| (credential_subject, entry)) - .ok_or(Error::::InternalError) - } - // Simple wrapper to remove entries from both storages when deleting a // credential. fn remove_credential_entry( @@ -540,5 +529,18 @@ pub mod pallet { credential_id, }); } + + fn retrieve_credential_entry( + credential_id: &CredentialIdOf, + ) -> Result<(T::SubjectId, CredentialEntryOf), Error> { + // Verify that the credential exists + let credential_subject = + CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; + + // Should never happen if the line above succeeds + Credentials::::get(&credential_subject, &credential_id) + .map(|entry| (credential_subject, entry)) + .ok_or(Error::::InternalError) + } } } diff --git a/rpc/public-credentials/src/lib.rs b/rpc/public-credentials/src/lib.rs index b1a00000a0..3ec45f461e 100644 --- a/rpc/public-credentials/src/lib.rs +++ b/rpc/public-credentials/src/lib.rs @@ -31,6 +31,7 @@ use sp_api::ProvideRuntimeApi; use sp_blockchain::HeaderBackend; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; +/// Filter that can be used after the credentials for a given subject have been retrieved from the blockchain state. pub trait PublicCredentialsFilter { fn should_include(&self, credential: &Credential) -> bool; } @@ -49,7 +50,7 @@ pub trait PublicCredentialsApi { client: Arc, + #[allow(clippy::type_complexity)] _marker: std::marker::PhantomData<( Block, OuterSubjectId, diff --git a/runtimes/common/Cargo.toml b/runtimes/common/Cargo.toml index 9dc7d8d2ed..85f725aa08 100644 --- a/runtimes/common/Cargo.toml +++ b/runtimes/common/Cargo.toml @@ -43,7 +43,6 @@ runtime-benchmarks = [ "frame-support/runtime-benchmarks", "kilt-support/runtime-benchmarks", "parachain-staking/runtime-benchmarks", - "public-credentials/runtime-benchmarks", "sp-runtime/runtime-benchmarks", ] std = [ diff --git a/runtimes/common/src/constants.rs b/runtimes/common/src/constants.rs index 3a67641977..d39290e484 100644 --- a/runtimes/common/src/constants.rs +++ b/runtimes/common/src/constants.rs @@ -434,7 +434,7 @@ pub mod public_credentials { use super::*; /// The size is checked in the runtime by a test. - pub const MAX_PUBLIC_CREDENTIAL_STORAGE_LENGTH: u32 = 257; + pub const MAX_PUBLIC_CREDENTIAL_STORAGE_LENGTH: u32 = 355; // Each credential would have a different deposit, so no multiplier here pub const PUBLIC_CREDENTIAL_DEPOSIT: Balance = deposit(1, MAX_PUBLIC_CREDENTIAL_STORAGE_LENGTH); diff --git a/runtimes/peregrine/src/lib.rs b/runtimes/peregrine/src/lib.rs index 26117a798d..00ca41e5c6 100644 --- a/runtimes/peregrine/src/lib.rs +++ b/runtimes/peregrine/src/lib.rs @@ -664,7 +664,7 @@ impl public_credentials::Config for Runtime { type OriginSuccess = did::DidRawOrigin; type Event = Event; type SubjectId = runtime_common::assets::AssetDid; - type WeightInfo = (); + type WeightInfo = weights::public_credentials::WeightInfo; } /// The type used to represent the kinds of proxying allowed. diff --git a/runtimes/spiritnet/src/lib.rs b/runtimes/spiritnet/src/lib.rs index 9799f02d07..f4f1712a7b 100644 --- a/runtimes/spiritnet/src/lib.rs +++ b/runtimes/spiritnet/src/lib.rs @@ -659,7 +659,7 @@ impl public_credentials::Config for Runtime { type OriginSuccess = did::DidRawOrigin; type Event = Event; type SubjectId = runtime_common::assets::AssetDid; - type WeightInfo = (); + type WeightInfo = weights::public_credentials::WeightInfo; } /// The type used to represent the kinds of proxying allowed. From 886f4e862b085541f7e173fe0e7ba4bcfc4d5235 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Wed, 3 Aug 2022 15:46:08 +0200 Subject: [PATCH 16/21] refactor common function --- nodes/common/src/public_credentials.rs | 3 +- pallets/public-credentials/src/lib.rs | 103 ++++++++++++------------- rpc/public-credentials/src/lib.rs | 3 +- 3 files changed, 54 insertions(+), 55 deletions(-) diff --git a/nodes/common/src/public_credentials.rs b/nodes/common/src/public_credentials.rs index 61bc698911..93de7b6eaf 100644 --- a/nodes/common/src/public_credentials.rs +++ b/nodes/common/src/public_credentials.rs @@ -56,7 +56,8 @@ impl } } -/// Filter for public credentials retrieved for a provided subject as specified in the RPC interface. +/// Filter for public credentials retrieved for a provided subject as specified +/// in the RPC interface. #[derive(Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum PublicCredentialFilter { diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index d2fb5785f7..f8c692cab7 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -272,7 +272,8 @@ pub mod pallet { ctype::Error::::CTypeNotFound ); - // Credential ID = H( || ) + // Credential ID = H( || + // ) let credential_id = T::CredentialHash::hash(&[&credential.encode()[..], &attester.encode()[..]].concat()[..]); @@ -351,32 +352,13 @@ pub mod pallet { let credential_subject = CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; - // Fails if the credential does not exist OR the caller is different than the - // original attester. If successful, saves the additional weight used for access - // control and returns it at the end of the function. - let ac_weight_used = - Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { - if let Some(credential) = credential_entry { - // Additional weight is 0 if the caller is the attester, otherwise it's the - // value returned by the access control check, if it does not fail. - let additional_weight = if caller == credential.attester { - 0 - } else { - let credential_auth_id = - credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; - authorization - .ok_or(Error::::Unauthorized)? - .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) - .map_err(|_| Error::::Unauthorized)? - }; - // If authorization checks are ok, update the revocation status. - credential.revoked = true; - Ok(additional_weight) - } else { - // No weight is computed as the error is an early return. - Err(DispatchError::from(Error::::CredentialNotFound)) - } - })?; + let ac_weight_used = Self::set_credential_revocation_status( + &caller, + &credential_subject, + &credential_id, + authorization, + true, + )?; Self::deposit_event(Event::CredentialRevoked { credential_id }); @@ -407,32 +389,13 @@ pub mod pallet { let credential_subject = CredentialSubjects::::get(&credential_id).ok_or(Error::::CredentialNotFound)?; - // Fails if the credential does not exist OR the caller is different than the - // original attester. If successful, saves the additional weight used for access - // control and returns it at the end of the function. - let ac_weight_used = - Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { - if let Some(credential) = credential_entry { - // Additional weight is 0 if the caller is the attester, otherwise it's the - // value returned by the access control check, if it does not fail. - let additional_weight = if caller == credential.attester { - 0 - } else { - let credential_auth_id = - credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; - authorization - .ok_or(Error::::Unauthorized)? - .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) - .map_err(|_| Error::::Unauthorized)? - }; - // If authorization checks are ok, update the revocation status. - credential.revoked = false; - Ok(additional_weight) - } else { - // No weight is computed as the error is an early return. - Err(DispatchError::from(Error::::CredentialNotFound)) - } - })?; + let ac_weight_used = Self::set_credential_revocation_status( + &caller, + &credential_subject, + &credential_id, + authorization, + false, + )?; Self::deposit_event(Event::CredentialUnrevoked { credential_id }); @@ -542,5 +505,39 @@ pub mod pallet { .map(|entry| (credential_subject, entry)) .ok_or(Error::::InternalError) } + + fn set_credential_revocation_status( + caller: &AttesterOf, + credential_subject: &T::SubjectId, + credential_id: &CredentialIdOf, + authorization: Option, + revocation: bool, + ) -> Result> { + // Fails if the credential does not exist OR the caller is different than the + // original attester. If successful, saves the additional weight used for access + // control and returns it at the end of the function. + Credentials::::try_mutate(&credential_subject, &credential_id, |credential_entry| { + if let Some(credential) = credential_entry { + // Additional weight is 0 if the caller is the attester, otherwise it's the + // value returned by the access control check, if it does not fail. + let additional_weight = if *caller == credential.attester { + 0 + } else { + let credential_auth_id = + credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; + authorization + .ok_or(Error::::Unauthorized)? + .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) + .map_err(|_| Error::::Unauthorized)? + }; + // If authorization checks are ok, update the revocation status. + credential.revoked = revocation; + Ok(additional_weight) + } else { + // No weight is computed as the error is an early return. + Err(Error::::CredentialNotFound) + } + }) + } } } diff --git a/rpc/public-credentials/src/lib.rs b/rpc/public-credentials/src/lib.rs index 3ec45f461e..f3e5b76e89 100644 --- a/rpc/public-credentials/src/lib.rs +++ b/rpc/public-credentials/src/lib.rs @@ -31,7 +31,8 @@ use sp_api::ProvideRuntimeApi; use sp_blockchain::HeaderBackend; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; -/// Filter that can be used after the credentials for a given subject have been retrieved from the blockchain state. +/// Filter that can be used after the credentials for a given subject have been +/// retrieved from the blockchain state. pub trait PublicCredentialsFilter { fn should_include(&self, credential: &Credential) -> bool; } From 3def3d6a3754414f86c2e30e91b74898152b08b7 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Thu, 4 Aug 2022 08:03:55 +0200 Subject: [PATCH 17/21] change get_credential to only take a credential ID parameter --- pallets/public-credentials/src/lib.rs | 7 +++---- rpc/public-credentials/runtime-api/src/lib.rs | 2 +- rpc/public-credentials/src/lib.rs | 15 ++------------- runtimes/peregrine/src/lib.rs | 3 ++- runtimes/spiritnet/src/lib.rs | 3 ++- runtimes/standalone/src/lib.rs | 3 ++- 6 files changed, 12 insertions(+), 21 deletions(-) diff --git a/pallets/public-credentials/src/lib.rs b/pallets/public-credentials/src/lib.rs index f8c692cab7..1bf423e2f6 100644 --- a/pallets/public-credentials/src/lib.rs +++ b/pallets/public-credentials/src/lib.rs @@ -183,11 +183,10 @@ pub mod pallet { // This map ensures that at any time a credential is only linked (i.e., issued) // to a single subject, as it maps from a credential ID to the subject it was - // issued to. Not exposed to the outside world. + // issued to. #[pallet::storage] #[pallet::getter(fn get_credential_subject)] - pub(crate) type CredentialSubjects = - StorageMap<_, Blake2_128Concat, CredentialIdOf, ::SubjectId>; + pub type CredentialSubjects = StorageMap<_, Blake2_128Concat, CredentialIdOf, ::SubjectId>; /// The events generated by this pallet. #[pallet::event] @@ -527,7 +526,7 @@ pub mod pallet { credential.authorization_id.as_ref().ok_or(Error::::Unauthorized)?; authorization .ok_or(Error::::Unauthorized)? - .can_revoke(&caller, &credential.ctype_hash, &credential_id, credential_auth_id) + .can_revoke(caller, &credential.ctype_hash, credential_id, credential_auth_id) .map_err(|_| Error::::Unauthorized)? }; // If authorization checks are ok, update the revocation status. diff --git a/rpc/public-credentials/runtime-api/src/lib.rs b/rpc/public-credentials/runtime-api/src/lib.rs index 0a5517c474..6b0b0ba5a8 100644 --- a/rpc/public-credentials/runtime-api/src/lib.rs +++ b/rpc/public-credentials/runtime-api/src/lib.rs @@ -29,7 +29,7 @@ sp_api::decl_runtime_apis! { CredentialId: Codec, CredentialEntry: Codec { - fn get_credential(subject: SubjectId, credential_id: CredentialId) -> Option; + fn get_credential(credential_id: CredentialId) -> Option; fn get_credentials(subject: SubjectId) -> Vec<(CredentialId, CredentialEntry)>; } } diff --git a/rpc/public-credentials/src/lib.rs b/rpc/public-credentials/src/lib.rs index f3e5b76e89..4faf78db05 100644 --- a/rpc/public-credentials/src/lib.rs +++ b/rpc/public-credentials/src/lib.rs @@ -39,12 +39,10 @@ pub trait PublicCredentialsFilter { #[rpc(client, server)] pub trait PublicCredentialsApi { - /// Return a credential that matches the provided root hash and issued to - /// the provided subject, if found. + /// Return a credential that matches the provided credential ID, if found. #[method(name = "get_credential")] fn get_credential( &self, - subject: OuterSubjectId, credential_id: OuterCredentialId, at: Option, ) -> RpcResult>; @@ -167,21 +165,12 @@ impl< { fn get_credential( &self, - subject: OuterSubjectId, credential_id: OuterCredentialId, at: Option<::Hash>, ) -> RpcResult> { let api = self.client.runtime_api(); let at = BlockId::hash(at.unwrap_or_else(|| self.client.info().best_hash)); - let into_subject: SubjectId = subject.try_into().map_err(|_| { - CallError::Custom(ErrorObject::owned( - Error::Conversion as i32, - "Unable to convert input to a valid subject ID.", - Option::::None, - )) - })?; - let into_credential_id: CredentialId = credential_id.try_into().map_err(|_| { CallError::Custom(ErrorObject::owned( Error::Conversion as i32, @@ -190,7 +179,7 @@ impl< )) })?; - let credential = api.get_credential(&at, into_subject, into_credential_id).map_err(|_| { + let credential = api.get_credential(&at, into_credential_id).map_err(|_| { CallError::Custom(ErrorObject::owned( Error::Runtime as i32, "Unable to get credential.", diff --git a/runtimes/peregrine/src/lib.rs b/runtimes/peregrine/src/lib.rs index 00ca41e5c6..6a05692401 100644 --- a/runtimes/peregrine/src/lib.rs +++ b/runtimes/peregrine/src/lib.rs @@ -1197,7 +1197,8 @@ impl_runtime_apis! { } impl public_credentials_runtime_api::PublicCredentialsApi::SubjectId, attestation::ClaimHashOf, public_credentials::CredentialEntryOf> for Runtime { - fn get_credential(subject: ::SubjectId, credential_id: attestation::ClaimHashOf) -> Option> { + fn get_credential(credential_id: attestation::ClaimHashOf) -> Option> { + let subject = public_credentials::CredentialSubjects::::get(&credential_id)?; public_credentials::Credentials::::get(&subject, &credential_id) } diff --git a/runtimes/spiritnet/src/lib.rs b/runtimes/spiritnet/src/lib.rs index f4f1712a7b..f00a30b624 100644 --- a/runtimes/spiritnet/src/lib.rs +++ b/runtimes/spiritnet/src/lib.rs @@ -1190,7 +1190,8 @@ impl_runtime_apis! { } impl public_credentials_runtime_api::PublicCredentialsApi::SubjectId, attestation::ClaimHashOf, public_credentials::CredentialEntryOf> for Runtime { - fn get_credential(subject: ::SubjectId, credential_id: attestation::ClaimHashOf) -> Option> { + fn get_credential(credential_id: attestation::ClaimHashOf) -> Option> { + let subject = public_credentials::CredentialSubjects::::get(&credential_id)?; public_credentials::Credentials::::get(&subject, &credential_id) } diff --git a/runtimes/standalone/src/lib.rs b/runtimes/standalone/src/lib.rs index 5898697aa3..3d2de742ad 100644 --- a/runtimes/standalone/src/lib.rs +++ b/runtimes/standalone/src/lib.rs @@ -983,7 +983,8 @@ impl_runtime_apis! { } impl public_credentials_runtime_api::PublicCredentialsApi::SubjectId, attestation::ClaimHashOf, public_credentials::CredentialEntryOf> for Runtime { - fn get_credential(subject: ::SubjectId, credential_id: attestation::ClaimHashOf) -> Option> { + fn get_credential(credential_id: attestation::ClaimHashOf) -> Option> { + let subject = public_credentials::CredentialSubjects::::get(&credential_id)?; public_credentials::Credentials::::get(&subject, &credential_id) } From 798e4fbd26b5dddcdedf0be9d4a3fac49688eac1 Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Thu, 4 Aug 2022 09:23:18 +0200 Subject: [PATCH 18/21] rename rpc endpoints for easier decoration --- rpc/public-credentials/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rpc/public-credentials/src/lib.rs b/rpc/public-credentials/src/lib.rs index 4faf78db05..9456372e4c 100644 --- a/rpc/public-credentials/src/lib.rs +++ b/rpc/public-credentials/src/lib.rs @@ -40,7 +40,7 @@ pub trait PublicCredentialsFilter { #[rpc(client, server)] pub trait PublicCredentialsApi { /// Return a credential that matches the provided credential ID, if found. - #[method(name = "get_credential")] + #[method(name = "credentials_get_credential")] fn get_credential( &self, credential_id: OuterCredentialId, @@ -50,7 +50,7 @@ pub trait PublicCredentialsApi Date: Thu, 4 Aug 2022 10:06:42 +0200 Subject: [PATCH 19/21] update RPC method names again --- rpc/public-credentials/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rpc/public-credentials/src/lib.rs b/rpc/public-credentials/src/lib.rs index 9456372e4c..a5fede10c0 100644 --- a/rpc/public-credentials/src/lib.rs +++ b/rpc/public-credentials/src/lib.rs @@ -40,7 +40,7 @@ pub trait PublicCredentialsFilter { #[rpc(client, server)] pub trait PublicCredentialsApi { /// Return a credential that matches the provided credential ID, if found. - #[method(name = "credentials_get_credential")] + #[method(name = "credentials_getCredential")] fn get_credential( &self, credential_id: OuterCredentialId, @@ -50,7 +50,7 @@ pub trait PublicCredentialsApi Date: Thu, 4 Aug 2022 10:30:55 +0200 Subject: [PATCH 20/21] change encoding of filter to be camelCase to be consistent with PolkadotJS --- nodes/common/src/public_credentials.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nodes/common/src/public_credentials.rs b/nodes/common/src/public_credentials.rs index 93de7b6eaf..13ad2d5c7b 100644 --- a/nodes/common/src/public_credentials.rs +++ b/nodes/common/src/public_credentials.rs @@ -59,7 +59,7 @@ impl /// Filter for public credentials retrieved for a provided subject as specified /// in the RPC interface. #[derive(Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] +#[serde(rename_all = "camelCase")] pub enum PublicCredentialFilter { /// Filter credentials that match a specified Ctype. CtypeHash(CTypeHash), From e95d50e7eee6f3542cd090e82930a842d5c0f0fd Mon Sep 17 00:00:00 2001 From: Antonio Antonino Date: Thu, 4 Aug 2022 13:41:17 +0200 Subject: [PATCH 21/21] Apply suggestions from review --- pallets/public-credentials/src/credentials.rs | 2 +- pallets/public-credentials/src/lib.rs | 18 ++++++++++-------- pallets/public-credentials/src/tests.rs | 12 ++++++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/pallets/public-credentials/src/credentials.rs b/pallets/public-credentials/src/credentials.rs index c2c1a957b9..3277f363e1 100644 --- a/pallets/public-credentials/src/credentials.rs +++ b/pallets/public-credentials/src/credentials.rs @@ -47,7 +47,7 @@ pub struct CredentialEntry, >; - // This map ensures that at any time a credential is only linked (i.e., issued) - // to a single subject, as it maps from a credential ID to the subject it was - // issued to. + /// A reverse index mapping from credential ID to the subject the credential + /// was issued to. + /// + /// It it used to perform efficient lookup of credential given their ID. #[pallet::storage] #[pallet::getter(fn get_credential_subject)] pub type CredentialSubjects = StorageMap<_, Blake2_128Concat, CredentialIdOf, ::SubjectId>; @@ -206,12 +207,12 @@ pub mod pallet { /// The id of the removed credential. credential_id: CredentialIdOf, }, - /// A public credentials has been revoked. + /// A public credential has been revoked. CredentialRevoked { /// The id of the revoked credential. credential_id: CredentialIdOf, }, - /// A public credentials has been unrevoked. + /// A public credential has been unrevoked. CredentialUnrevoked { /// The id of the unrevoked credential. credential_id: CredentialIdOf, @@ -332,7 +333,7 @@ pub mod pallet { /// If a credential was already revoked, this function does not fail but /// simply results in a noop. /// - /// Only authorized callers can revoke the credential. + /// /// The dispatch origin must be authorized to revoke the credential. /// /// Emits `CredentialRevoked`. #[pallet::weight({ @@ -369,7 +370,8 @@ pub mod pallet { /// If a credential was not revoked, this function does not fail but /// simply results in a noop. /// - /// Only authorized callers can unrevoke the credential. + /// /// The dispatch origin must be authorized to unrevoke the + /// credential. /// /// Emits `CredentialUnrevoked`. #[pallet::weight({ @@ -415,7 +417,7 @@ pub mod pallet { /// This function fails if a credential already exists for the specified /// subject. /// - /// Only authorized callers can remove the credential. + /// /// The dispatch origin must be authorized to remove the credential. /// /// Emits `CredentialRemoved`. #[pallet::weight({ diff --git a/pallets/public-credentials/src/tests.rs b/pallets/public-credentials/src/tests.rs index 51be35d85a..8112026a5e 100644 --- a/pallets/public-credentials/src/tests.rs +++ b/pallets/public-credentials/src/tests.rs @@ -532,8 +532,8 @@ fn remove_successful() { )); // Test this pallet logic - assert_eq!(Credentials::::get(&subject_id, &credential_id), None); - assert_eq!(CredentialSubjects::::get(&credential_id), None); + assert!(Credentials::::get(&subject_id, &credential_id).is_none()); + assert!(CredentialSubjects::::get(&credential_id).is_none()); // Check deposit release logic assert!(Balances::reserved_balance(ACCOUNT_00).is_zero()); @@ -568,8 +568,8 @@ fn remove_same_attester_wrong_ac() { )); // Test this pallet logic - assert_eq!(Credentials::::get(&subject_id, &credential_id), None); - assert_eq!(CredentialSubjects::::get(&credential_id), None); + assert!(Credentials::::get(&subject_id, &credential_id).is_none()); + assert!(CredentialSubjects::::get(&credential_id).is_none()); }); } @@ -663,8 +663,8 @@ fn reclaim_deposit_successful() { )); // Test this pallet logic - assert_eq!(Credentials::::get(&subject_id, &credential_id), None); - assert_eq!(CredentialSubjects::::get(&credential_id), None); + assert!(Credentials::::get(&subject_id, &credential_id).is_none()); + assert!(CredentialSubjects::::get(&credential_id).is_none()); // Check deposit release logic assert!(Balances::reserved_balance(ACCOUNT_00).is_zero());