Skip to content

Commit

Permalink
refactored DID errors + one forgoten Unauthorized in attestation pallet.
Browse files Browse the repository at this point in the history
  • Loading branch information
@trusch
trusch committed Jan 5, 2023
1 parent b274b12 commit 788191c
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 125 deletions.
16 changes: 8 additions & 8 deletions pallets/attestation/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ pub mod pallet {
/// delegation hierarchy root.
CTypeMismatch,
/// The call origin is not authorized to change the attestation.
Unauthorized,
NotAuthorized,
/// The maximum number of delegated attestations has already been
/// reached for the corresponding delegation id such that another one
/// cannot be added.
Expand Down Expand Up @@ -320,8 +320,8 @@ pub mod pallet {
ensure!(!attestation.revoked, Error::<T>::AlreadyRevoked);

if attestation.attester != who {
let attestation_auth_id = attestation.authorization_id.as_ref().ok_or(Error::<T>::Unauthorized)?;
authorization.ok_or(Error::<T>::Unauthorized)?.can_revoke(
let attestation_auth_id = attestation.authorization_id.as_ref().ok_or(Error::<T>::NotAuthorized)?;
authorization.ok_or(Error::<T>::NotAuthorized)?.can_revoke(
&who,
&attestation.ctype_hash,
&claim_hash,
Expand Down Expand Up @@ -377,8 +377,8 @@ pub mod pallet {
let attestation = Attestations::<T>::get(claim_hash).ok_or(Error::<T>::NotFound)?;

if attestation.attester != who {
let attestation_auth_id = attestation.authorization_id.as_ref().ok_or(Error::<T>::Unauthorized)?;
authorization.ok_or(Error::<T>::Unauthorized)?.can_remove(
let attestation_auth_id = attestation.authorization_id.as_ref().ok_or(Error::<T>::NotAuthorized)?;
authorization.ok_or(Error::<T>::NotAuthorized)?.can_remove(
&who,
&attestation.ctype_hash,
&claim_hash,
Expand Down Expand Up @@ -410,7 +410,7 @@ pub mod pallet {
let who = ensure_signed(origin)?;
let attestation = Attestations::<T>::get(claim_hash).ok_or(Error::<T>::NotFound)?;

ensure!(attestation.deposit.owner == who, Error::<T>::Unauthorized);
ensure!(attestation.deposit.owner == who, Error::<T>::NotAuthorized);

// *** No Fail beyond this point ***

Expand All @@ -436,7 +436,7 @@ pub mod pallet {
let sender = source.sender();

let attestation = Attestations::<T>::get(claim_hash).ok_or(Error::<T>::NotFound)?;
ensure!(attestation.attester == subject, Error::<T>::Unauthorized);
ensure!(attestation.attester == subject, Error::<T>::NotAuthorized);

AttestationStorageDepositCollector::<T>::change_deposit_owner(&claim_hash, sender)?;

Expand All @@ -451,7 +451,7 @@ pub mod pallet {
let sender = ensure_signed(origin)?;

let attestation = Attestations::<T>::get(claim_hash).ok_or(Error::<T>::NotFound)?;
ensure!(attestation.deposit.owner == sender, Error::<T>::Unauthorized);
ensure!(attestation.deposit.owner == sender, Error::<T>::NotAuthorized);

AttestationStorageDepositCollector::<T>::update_deposit(&claim_hash)?;

Expand Down
8 changes: 4 additions & 4 deletions pallets/attestation/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ fn test_remove_unauthorized() {
claim_hash,
authorization_info
),
attestation::Error::<Test>::Unauthorized
attestation::Error::<Test>::NotAuthorized
);
});
}
Expand Down Expand Up @@ -465,7 +465,7 @@ fn test_reclaim_unauthorized() {
.execute_with(|| {
assert_noop!(
Attestation::reclaim_deposit(RuntimeOrigin::signed(ACCOUNT_01), claim_hash),
attestation::Error::<Test>::Unauthorized,
attestation::Error::<Test>::NotAuthorized,
);
});
}
Expand Down Expand Up @@ -564,7 +564,7 @@ fn test_change_deposit_owner_unauthorized() {
.execute_with(|| {
assert_noop!(
Attestation::change_deposit_owner(DoubleOrigin(ACCOUNT_00, evil_actor).into(), claim_hash),
attestation::Error::<Test>::Unauthorized,
attestation::Error::<Test>::NotAuthorized,
);
});
}
Expand Down Expand Up @@ -650,7 +650,7 @@ fn test_update_deposit_unauthorized() {
);
assert_noop!(
Attestation::update_deposit(RuntimeOrigin::signed(ACCOUNT_01), claim_hash),
Error::<Test>::Unauthorized
Error::<Test>::NotAuthorized
);
});
}
62 changes: 31 additions & 31 deletions pallets/did/src/did_details.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use sp_std::{convert::TryInto, vec::Vec};
use kilt_support::deposit::Deposit;

use crate::{
errors::{DidError, InputError, SignatureError, StorageError},
errors::{Error, Input, Signature, Storage},
service_endpoints::DidEndpoint,
utils, AccountIdOf, BalanceOf, BlockNumberOf, Config, DidCallableOf, DidIdentifierOf, KeyIdOf, Payload,
};
Expand All @@ -49,22 +49,22 @@ pub enum DidVerificationKey {

impl DidVerificationKey {
/// Verify a DID signature using one of the DID keys.
pub fn verify_signature(&self, payload: &Payload, signature: &DidSignature) -> Result<(), SignatureError> {
pub fn verify_signature(&self, payload: &Payload, signature: &DidSignature) -> Result<(), Signature> {
match (self, signature) {
(DidVerificationKey::Ed25519(public_key), DidSignature::Ed25519(sig)) => {
ensure!(sig.verify(payload, public_key), SignatureError::InvalidSignature);
ensure!(sig.verify(payload, public_key), Signature::InvalidSignature);
Ok(())
}
// Follows same process as above, but using a Sr25519 instead
(DidVerificationKey::Sr25519(public_key), DidSignature::Sr25519(sig)) => {
ensure!(sig.verify(payload, public_key), SignatureError::InvalidSignature);
ensure!(sig.verify(payload, public_key), Signature::InvalidSignature);
Ok(())
}
(DidVerificationKey::Ecdsa(public_key), DidSignature::Ecdsa(sig)) => {
ensure!(sig.verify(payload, public_key), SignatureError::InvalidSignature);
ensure!(sig.verify(payload, public_key), Signature::InvalidSignature);
Ok(())
}
_ => Err(SignatureError::InvalidSignatureFormat),
_ => Err(Signature::InvalidSignatureFormat),
}
}
}
Expand Down Expand Up @@ -176,15 +176,15 @@ pub trait DidVerifiableIdentifier {
&self,
payload: &Payload,
signature: &DidSignature,
) -> Result<DidVerificationKey, SignatureError>;
) -> Result<DidVerificationKey, Signature>;
}

impl<I: AsRef<[u8; 32]>> DidVerifiableIdentifier for I {
fn verify_and_recover_signature(
&self,
payload: &Payload,
signature: &DidSignature,
) -> Result<DidVerificationKey, SignatureError> {
) -> Result<DidVerificationKey, Signature> {
// So far, either the raw Ed25519/Sr25519 public key or the Blake2-256 hashed
// ECDSA public key.
let raw_public_key: &[u8; 32] = self.as_ref();
Expand All @@ -207,17 +207,17 @@ impl<I: AsRef<[u8; 32]>> DidVerifiableIdentifier for I {
let ecdsa_signature: [u8; 65] = signature
.encode()
.try_into()
.map_err(|_| SignatureError::InvalidSignature)?;
.map_err(|_| Signature::InvalidSignature)?;
// ECDSA uses blake2-256 hashing algorithm for signatures, so we hash the given
// message to recover the public key.
let hashed_message = sp_io::hashing::blake2_256(payload);
let recovered_pk: [u8; 33] =
sp_io::crypto::secp256k1_ecdsa_recover_compressed(&ecdsa_signature, &hashed_message)
.map_err(|_| SignatureError::InvalidSignature)?;
.map_err(|_| Signature::InvalidSignature)?;
let hashed_recovered_pk = sp_io::hashing::blake2_256(&recovered_pk);
// The hashed recovered public key must be equal to the AccountId32 value, which
// is the hashed key.
ensure!(&hashed_recovered_pk == raw_public_key, SignatureError::InvalidSignature);
ensure!(&hashed_recovered_pk == raw_public_key, Signature::InvalidSignature);
// Safe to reconstruct the public key using the recovered value from
// secp256k1_ecdsa_recover_compressed
Ok(DidVerificationKey::from(ecdsa::Public(recovered_pk)))
Expand Down Expand Up @@ -287,7 +287,7 @@ impl<T: Config> DidDetails<T> {
authentication_key: DidVerificationKey,
block_number: BlockNumberOf<T>,
deposit: Deposit<AccountIdOf<T>, BalanceOf<T>>,
) -> Result<Self, StorageError> {
) -> Result<Self, Storage> {
let mut public_keys = DidPublicKeyMap::<T>::default();
let authentication_key_id = utils::calculate_key_id::<T>(&authentication_key.clone().into());
public_keys
Expand All @@ -298,7 +298,7 @@ impl<T: Config> DidDetails<T> {
block_number,
},
)
.map_err(|_| StorageError::MaxPublicKeysExceeded)?;
.map_err(|_| Storage::MaxPublicKeysExceeded)?;
Ok(Self {
authentication_key: authentication_key_id,
key_agreement_keys: DidKeyAgreementKeySet::<T>::default(),
Expand All @@ -315,11 +315,11 @@ impl<T: Config> DidDetails<T> {
pub fn from_creation_details(
details: DidCreationDetails<T>,
new_auth_key: DidVerificationKey,
) -> Result<Self, DidError> {
) -> Result<Self, Error> {
ensure!(
details.new_key_agreement_keys.len()
<= <<T as Config>::MaxNewKeyAgreementKeys>::get().saturated_into::<usize>(),
InputError::MaxKeyAgreementKeysLimitExceeded
Input::MaxKeyAgreementKeysLimitExceeded
);

let current_block_number = frame_system::Pallet::<T>::block_number();
Expand Down Expand Up @@ -354,7 +354,7 @@ impl<T: Config> DidDetails<T> {
&mut self,
new_authentication_key: DidVerificationKey,
block_number: BlockNumberOf<T>,
) -> Result<(), StorageError> {
) -> Result<(), Storage> {
let old_authentication_key_id = self.authentication_key;
let new_authentication_key_id = utils::calculate_key_id::<T>(&new_authentication_key.clone().into());
self.authentication_key = new_authentication_key_id;
Expand All @@ -370,7 +370,7 @@ impl<T: Config> DidDetails<T> {
block_number,
},
)
.map_err(|_| StorageError::MaxPublicKeysExceeded)?;
.map_err(|_| Storage::MaxPublicKeysExceeded)?;
Ok(())
}

Expand All @@ -381,7 +381,7 @@ impl<T: Config> DidDetails<T> {
&mut self,
new_key_agreement_keys: DidNewKeyAgreementKeySet<T>,
block_number: BlockNumberOf<T>,
) -> Result<(), StorageError> {
) -> Result<(), Storage> {
for new_key_agreement_key in new_key_agreement_keys {
self.add_key_agreement_key(new_key_agreement_key, block_number)?;
}
Expand All @@ -395,7 +395,7 @@ impl<T: Config> DidDetails<T> {
&mut self,
new_key_agreement_key: DidEncryptionKey,
block_number: BlockNumberOf<T>,
) -> Result<(), StorageError> {
) -> Result<(), Storage> {
let new_key_agreement_id = utils::calculate_key_id::<T>(&new_key_agreement_key.into());
self.public_keys
.try_insert(
Expand All @@ -405,17 +405,17 @@ impl<T: Config> DidDetails<T> {
block_number,
},
)
.map_err(|_| StorageError::MaxPublicKeysExceeded)?;
.map_err(|_| Storage::MaxPublicKeysExceeded)?;
self.key_agreement_keys
.try_insert(new_key_agreement_id)
.map_err(|_| StorageError::MaxTotalKeyAgreementKeysExceeded)?;
.map_err(|_| Storage::MaxTotalKeyAgreementKeysExceeded)?;
Ok(())
}

/// Remove a key agreement key from both the set of key agreement keys and
/// the one of public keys.
pub fn remove_key_agreement_key(&mut self, key_id: KeyIdOf<T>) -> Result<(), StorageError> {
ensure!(self.key_agreement_keys.remove(&key_id), StorageError::KeyNotFound);
pub fn remove_key_agreement_key(&mut self, key_id: KeyIdOf<T>) -> Result<(), Storage> {
ensure!(self.key_agreement_keys.remove(&key_id), Storage::KeyNotFound);
self.remove_key_if_unused(key_id);
Ok(())
}
Expand All @@ -429,7 +429,7 @@ impl<T: Config> DidDetails<T> {
&mut self,
new_attestation_key: DidVerificationKey,
block_number: BlockNumberOf<T>,
) -> Result<(), StorageError> {
) -> Result<(), Storage> {
let new_attestation_key_id = utils::calculate_key_id::<T>(&new_attestation_key.clone().into());
if let Some(old_attestation_key_id) = self.attestation_key.take() {
self.remove_key_if_unused(old_attestation_key_id);
Expand All @@ -443,7 +443,7 @@ impl<T: Config> DidDetails<T> {
block_number,
},
)
.map_err(|_| StorageError::MaxPublicKeysExceeded)?;
.map_err(|_| Storage::MaxPublicKeysExceeded)?;
Ok(())
}

Expand All @@ -452,8 +452,8 @@ impl<T: Config> DidDetails<T> {
/// The old key is deleted from the set of public keys if it is
/// not used in any other part of the DID. The new key is added to the
/// set of public keys.
pub fn remove_attestation_key(&mut self) -> Result<(), StorageError> {
let old_key_id = self.attestation_key.take().ok_or(StorageError::KeyNotFound)?;
pub fn remove_attestation_key(&mut self) -> Result<(), Storage> {
let old_key_id = self.attestation_key.take().ok_or(Storage::KeyNotFound)?;
self.remove_key_if_unused(old_key_id);
Ok(())
}
Expand All @@ -467,7 +467,7 @@ impl<T: Config> DidDetails<T> {
&mut self,
new_delegation_key: DidVerificationKey,
block_number: BlockNumberOf<T>,
) -> Result<(), StorageError> {
) -> Result<(), Storage> {
let new_delegation_key_id = utils::calculate_key_id::<T>(&new_delegation_key.clone().into());
if let Some(old_delegation_key_id) = self.delegation_key.take() {
self.remove_key_if_unused(old_delegation_key_id);
Expand All @@ -481,7 +481,7 @@ impl<T: Config> DidDetails<T> {
block_number,
},
)
.map_err(|_| StorageError::MaxPublicKeysExceeded)?;
.map_err(|_| Storage::MaxPublicKeysExceeded)?;
Ok(())
}

Expand All @@ -490,8 +490,8 @@ impl<T: Config> DidDetails<T> {
/// The old key is deleted from the set of public keys if it is
/// not used in any other part of the DID. The new key is added to the
/// set of public keys.
pub fn remove_delegation_key(&mut self) -> Result<(), StorageError> {
let old_key_id = self.delegation_key.take().ok_or(StorageError::KeyNotFound)?;
pub fn remove_delegation_key(&mut self) -> Result<(), Storage> {
let old_key_id = self.delegation_key.take().ok_or(Storage::KeyNotFound)?;
self.remove_key_if_unused(old_key_id);
Ok(())
}
Expand Down
26 changes: 13 additions & 13 deletions pallets/did/src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,32 @@ use crate::did_details::DidVerificationKeyRelationship;

/// All the errors that can be generated when validating a DID operation.
#[derive(Debug, Eq, PartialEq, TypeInfo)]
pub enum DidError {
pub enum Error {
/// See [StorageError].
StorageError(StorageError),
Storage(Storage),
/// See [SignatureError].
SignatureError(SignatureError),
Signature(Signature),
/// See [InputError].
InputError(InputError),
Input(Input),
/// An error that is not supposed to take place, yet it happened.
Internal,
}

impl From<StorageError> for DidError {
fn from(err: StorageError) -> Self {
DidError::StorageError(err)
impl From<Storage> for Error {
fn from(err: Storage) -> Self {
Error::Storage(err)
}
}

impl From<InputError> for DidError {
fn from(err: InputError) -> Self {
DidError::InputError(err)
impl From<Input> for Error {
fn from(err: Input) -> Self {
Error::Input(err)
}
}

/// Error involving the pallet's storage.
#[derive(Debug, Eq, PartialEq, TypeInfo)]
pub enum StorageError {
pub enum Storage {
/// The DID being created is already present on chain.
AlreadyExists,
/// The expected DID cannot be found on chain.
Expand All @@ -69,7 +69,7 @@ pub enum StorageError {

/// Error generated when validating a DID operation.
#[derive(Debug, Eq, PartialEq, TypeInfo)]
pub enum SignatureError {
pub enum Signature {
/// The signature is not in the format the verification key expects.
InvalidSignatureFormat,
/// The signature is invalid for the payload and the verification key
Expand All @@ -84,7 +84,7 @@ pub enum SignatureError {
/// Error generated when some extrinsic input does not respect the pallet's
/// constraints.
#[derive(Debug, Eq, PartialEq, TypeInfo)]
pub enum InputError {
pub enum Input {
/// A number of new key agreement keys greater than the maximum allowed has
/// been provided.
MaxKeyAgreementKeysLimitExceeded,
Expand Down
Loading

0 comments on commit 788191c

Please sign in to comment.