Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: prevent DID deletions if there's linked resources #843

Merged
merged 18 commits into from
Jan 15, 2025
Merged
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions dip-template/runtimes/dip-provider/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ impl did::Config for Runtime {
type BaseDeposit = ConstU128<UNIT>;
type Currency = Balances;
type DidIdentifier = DidIdentifier;
type DidLifecycleHooks = ();
type EnsureOrigin = EnsureDidOrigin<DidIdentifier, AccountId>;
type Fee = ConstU128<MILLIUNIT>;
type FeeCollector = ();
Expand Down
30 changes: 28 additions & 2 deletions pallets/did/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ pub mod errors;
pub mod migrations;
pub mod origin;
pub mod service_endpoints;
pub mod traits;

#[cfg(test)]
mod mock;
Expand Down Expand Up @@ -171,6 +172,7 @@ pub mod pallet {
DidEncryptionKey, DidSignature, DidVerifiableIdentifier, DidVerificationKey, RelationshipDeriveError,
},
service_endpoints::{utils as service_endpoints_utils, ServiceEndpointId},
traits::{DidDeletionHook, DidLifecycleHooks},
};

/// The current storage version.
Expand Down Expand Up @@ -328,6 +330,10 @@ pub mod pallet {

/// Migration manager to handle new created entries
type BalanceMigrationManager: BalanceMigrationManager<AccountIdOf<Self>, BalanceOf<Self>>;

/// Runtime-injected logic to be called at each stage of a DID's
/// lifecycle.
type DidLifecycleHooks: DidLifecycleHooks<Self>;
}

#[pallet::pallet]
Expand Down Expand Up @@ -455,6 +461,9 @@ pub mod pallet {
/// The number of service endpoints stored under the DID is larger than
/// the number of endpoints to delete.
MaxStoredEndpointsCountExceeded,
/// The DID cannot be deleted because the runtime logic returned an
/// error.
CannotDelete,
/// An error that is not supposed to take place, yet it happened.
Internal,
}
Expand Down Expand Up @@ -972,7 +981,10 @@ pub mod pallet {
/// - Kills: Did entry associated to the DID identifier
/// # </weight>
#[pallet::call_index(10)]
#[pallet::weight(<T as pallet::Config>::WeightInfo::delete(*endpoints_to_remove))]
#[pallet::weight({
let max_hook_weight = <<T::DidLifecycleHooks as DidLifecycleHooks<T>>::DeletionHook as DidDeletionHook<T>>::MAX_WEIGHT;
<T as pallet::Config>::WeightInfo::delete(*endpoints_to_remove).saturating_add(max_hook_weight)
})]
pub fn delete(origin: OriginFor<T>, endpoints_to_remove: u32) -> DispatchResult {
let source = T::EnsureOrigin::ensure_origin(origin)?;
let did_subject = source.subject();
Expand Down Expand Up @@ -1002,7 +1014,10 @@ pub mod pallet {
/// - Kills: Did entry associated to the DID identifier
/// # </weight>
#[pallet::call_index(11)]
#[pallet::weight(<T as pallet::Config>::WeightInfo::reclaim_deposit(*endpoints_to_remove))]
#[pallet::weight({
let max_hook_weight = <<T::DidLifecycleHooks as DidLifecycleHooks<T>>::DeletionHook as DidDeletionHook<T>>::MAX_WEIGHT;
<T as pallet::Config>::WeightInfo::reclaim_deposit(*endpoints_to_remove).saturating_add(max_hook_weight)
})]
pub fn reclaim_deposit(
origin: OriginFor<T>,
did_subject: DidIdentifierOf<T>,
Expand Down Expand Up @@ -1512,6 +1527,17 @@ pub mod pallet {
// `take` calls `kill` internally
let did_entry = Did::<T>::take(&did_subject).ok_or(Error::<T>::NotFound)?;

// Make sure this check happens after the line where we check if a DID exists,
// else we would start getting `CannotDelete` errors when we should be getting
// `NotFound`.
ensure!(
<<T::DidLifecycleHooks as DidLifecycleHooks<T>>::DeletionHook as DidDeletionHook<T>>::can_delete(
&did_subject,
)
.is_ok(),
Error::<T>::CannotDelete
);
Comment on lines +1533 to +1539
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is feasible in substrate, but I'd much rather raise errors in the hook and have them propagated, so that we can inform callers on which deletion conditions have not been met.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see this approach hitting a wall though if we can't come up with a way to define errors that are not already defined in any of the pallets. Substrate seems to be somewhat limited in that regard.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing we can do is return u8 or u16 error codes, which are returned by the runtime hook. Then we would need to come up with a scheme that would be able to encode in 8 or 16 bits all the different "components" that are linked to the DID, to be able to return an aggregate error of all the pending stuff instead of returning the error of the first item that was encountered.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we will add runtime APIs to ease deletion, I would say that a dry run of this extrinsic + a call to the API to return the list of call is sufficient to solve this issue. I would not over-complicate the runtime code.


DidEndpointsCount::<T>::remove(&did_subject);

let is_key_migrated =
Expand Down
1 change: 1 addition & 0 deletions pallets/did/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ impl Config for Test {
type MaxNumberOfTypesPerService = MaxNumberOfTypesPerService;
type MaxNumberOfUrlsPerService = MaxNumberOfUrlsPerService;
type BalanceMigrationManager = ();
type DidLifecycleHooks = ();
}

parameter_types! {
Expand Down
76 changes: 76 additions & 0 deletions pallets/did/src/traits/lifecycle_hooks/deletion/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// KILT Blockchain – https://botlabs.org
// Copyright (C) 2019-2024 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 <https://www.gnu.org/licenses/>.

// If you feel like getting in touch with us, you can do so at [email protected]

#[cfg(test)]
mod tests;

use sp_std::marker::PhantomData;
use sp_weights::Weight;

use crate::{Config, DidIdentifierOf};

/// Runtime logic evaluated by the DID pallet upon deleting an existing DID.
pub trait DidDeletionHook<T>
where
T: Config,
{
/// The statically computed maximum weight the implementation will consume
/// to verify if a DID can be deleted.
const MAX_WEIGHT: Weight;

/// Return whether the DID can be deleted (`Ok(())`), or not. In case of
/// error, the consumed weight (less than or equal to `MAX_WEIGHT`) is
/// returned.
fn can_delete(did: &DidIdentifierOf<T>) -> Result<(), Weight>;
}

impl<T> DidDeletionHook<T> for ()
where
T: Config,
{
const MAX_WEIGHT: Weight = Weight::from_parts(0, 0);

fn can_delete(_did: &DidIdentifierOf<T>) -> Result<(), Weight> {
Ok(())
}
}

/// Implementation of [`DidDeletionHook`] that iterates over both
/// components, bailing out early if the first one fails. The `MAX_WEIGHT` is
/// the sum of both components.
pub struct RequireBoth<A, B>(PhantomData<(A, B)>);

impl<T, A, B> DidDeletionHook<T> for RequireBoth<A, B>
where
T: Config,
A: DidDeletionHook<T>,
B: DidDeletionHook<T>,
{
const MAX_WEIGHT: Weight = A::MAX_WEIGHT.saturating_add(B::MAX_WEIGHT);

/// In case of failure, the returned weight is either the weight consumed by
/// the first component, or the sum of the first component's maximum weight
/// and the weight consumed by the second component.
fn can_delete(did: &DidIdentifierOf<T>) -> Result<(), Weight> {
// Bail out early with A's weight if A fails.
A::can_delete(did)?;
// Bail out early with A's max weight + B's if B fails.
B::can_delete(did).map_err(|consumed_weight| A::MAX_WEIGHT.saturating_add(consumed_weight))?;
Ok(())
}
}
84 changes: 84 additions & 0 deletions pallets/did/src/traits/lifecycle_hooks/deletion/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// KILT Blockchain – https://botlabs.org
// Copyright (C) 2019-2024 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 <https://www.gnu.org/licenses/>.

// If you feel like getting in touch with us, you can do so at [email protected]

//! Test module for the `RequireBoth` type. It verifies that the type works as
//! expected in case of failure of one of its components.

use sp_runtime::AccountId32;
use sp_weights::Weight;

use crate::{
traits::lifecycle_hooks::{deletion::RequireBoth, mock::TestRuntime, DidDeletionHook},
DidIdentifierOf,
};

struct AlwaysDeny;

impl DidDeletionHook<TestRuntime> for AlwaysDeny {
const MAX_WEIGHT: Weight = Weight::from_all(10);

fn can_delete(_did: &DidIdentifierOf<TestRuntime>) -> Result<(), Weight> {
Err(Weight::from_all(5))
}
}

struct AlwaysAllow;

impl DidDeletionHook<TestRuntime> for AlwaysAllow {
const MAX_WEIGHT: Weight = Weight::from_all(20);

fn can_delete(_did: &DidIdentifierOf<TestRuntime>) -> Result<(), Weight> {
Ok(())
}
}

#[test]
fn first_false() {
type TestSubject = RequireBoth<AlwaysDeny, AlwaysAllow>;

// Max weight is the sum.
assert_eq!(TestSubject::MAX_WEIGHT, Weight::from_all(30));
// Failure consumes `False`'s weight.
assert_eq!(
TestSubject::can_delete(&AccountId32::new([0u8; 32])),
Err(Weight::from_all(5))
);
}

#[test]
fn second_false() {
type TestSubject = RequireBoth<AlwaysAllow, AlwaysDeny>;

// Max weight is the sum.
assert_eq!(TestSubject::MAX_WEIGHT, Weight::from_all(30));
// Failure consumes the sum of `True`'s max weight and `False`'s weight.
assert_eq!(
TestSubject::can_delete(&AccountId32::new([0u8; 32])),
Err(Weight::from_all(25))
);
}

#[test]
fn both_true() {
type TestSubject = RequireBoth<AlwaysAllow, AlwaysAllow>;

// Max weight is the sum.
assert_eq!(TestSubject::MAX_WEIGHT, Weight::from_all(40));
// Overall result is `Ok`.
assert_eq!(TestSubject::can_delete(&AccountId32::new([0u8; 32])), Ok(()));
}
117 changes: 117 additions & 0 deletions pallets/did/src/traits/lifecycle_hooks/mock.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// KILT Blockchain – https://botlabs.org
// Copyright (C) 2019-2024 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 <https://www.gnu.org/licenses/>.

// If you feel like getting in touch with us, you can do so at [email protected]

use frame_support::{construct_runtime, parameter_types};
use frame_system::{mocking::MockBlock, EnsureSigned};
use kilt_support::mock::MockCurrency;
use parity_scale_codec::{Decode, Encode};
use scale_info::TypeInfo;
use sp_core::{ConstU32, ConstU64, H256};
use sp_runtime::{
traits::{BlakeTwo256, IdentityLookup},
AccountId32,
};

use crate::{
Config, DeriveDidCallAuthorizationVerificationKeyRelationship, DeriveDidCallKeyRelationshipResult,
DidVerificationKeyRelationship,
};

construct_runtime!(
pub enum TestRuntime
{
System: frame_system,
Did: crate,
}
);

impl frame_system::Config for TestRuntime {
type AccountData = ();
type AccountId = AccountId32;
type BaseCallFilter = ();
type Block = MockBlock<TestRuntime>;
type BlockHashCount = ConstU64<1>;
type BlockLength = ();
type BlockWeights = ();
type DbWeight = ();
type Hash = H256;
type Hashing = BlakeTwo256;
type Lookup = IdentityLookup<Self::AccountId>;
type MaxConsumers = ConstU32<1>;
type Nonce = u64;
type OnKilledAccount = ();
type OnNewAccount = ();
type OnSetCode = ();
type PalletInfo = PalletInfo;
type RuntimeEvent = ();
type RuntimeOrigin = RuntimeOrigin;
type RuntimeCall = RuntimeCall;
type RuntimeTask = ();
type SS58Prefix = ();
type SystemWeightInfo = ();
type Version = ();
}

parameter_types! {
#[derive(TypeInfo, Debug, PartialEq, Eq, Clone, Encode, Decode)]
pub const MaxNewKeyAgreementKeys: u32 = 1;
#[derive(TypeInfo, Debug, PartialEq, Eq, Clone, Encode, Decode)]
pub const MaxTotalKeyAgreementKeys: u32 = 1;
}

impl DeriveDidCallAuthorizationVerificationKeyRelationship for RuntimeCall {
fn derive_verification_key_relationship(&self) -> DeriveDidCallKeyRelationshipResult {
Ok(DidVerificationKeyRelationship::Authentication)
}

#[cfg(feature = "runtime-benchmarks")]
fn get_call_for_did_call_benchmark() -> Self {
Self::System(frame_system::Call::remark {
remark: b"test".to_vec(),
})
}
}

impl Config for TestRuntime {
type BalanceMigrationManager = ();
type BaseDeposit = ConstU64<1>;
type Currency = MockCurrency<u64, RuntimeHoldReason>;
type DidIdentifier = AccountId32;
type DidLifecycleHooks = ();
type EnsureOrigin = EnsureSigned<AccountId32>;
type Fee = ConstU64<1>;
type FeeCollector = ();
type KeyDeposit = ConstU64<1>;
type MaxBlocksTxValidity = ConstU64<1>;
type MaxNewKeyAgreementKeys = MaxNewKeyAgreementKeys;
type MaxNumberOfServicesPerDid = ConstU32<1>;
type MaxNumberOfTypesPerService = ConstU32<1>;
type MaxNumberOfUrlsPerService = ConstU32<1>;
type MaxPublicKeysPerDid = ConstU32<1>;
type MaxServiceIdLength = ConstU32<1>;
type MaxServiceTypeLength = ConstU32<1>;
type MaxServiceUrlLength = ConstU32<1>;
type MaxTotalKeyAgreementKeys = MaxTotalKeyAgreementKeys;
type OriginSuccess = AccountId32;
type RuntimeCall = RuntimeCall;
type RuntimeEvent = ();
type RuntimeHoldReason = RuntimeHoldReason;
type RuntimeOrigin = RuntimeOrigin;
type ServiceEndpointDeposit = ConstU64<1>;
type WeightInfo = ();
}
Loading
Loading