Skip to content

Commit

Permalink
feat: add deposit reclaim extrinsic for delegations (#295)
Browse files Browse the repository at this point in the history 
* feat: add reclaim_deposit extrinsic

* tests: tests + benches

* chore: wording

* chore: clippy

* chore: Oxford grammar check ✅

* chore: delegation pallet cargo file

* chore: remove final comma from benchmark comment

* chore: remove wrong comment

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=delegation --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/delegation.rs --template=.maintain/runtime-weight-template.hbs

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=delegation --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=pallets/delegation/src/default_weights.rs --template=.maintain/weight-template.hbs

* cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=delegation --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/peregrine/src/weights/delegation.rs --template=.maintain/runtime-weight-template.hbs

Co-authored-by: kiltbot <>
  • Loading branch information
@ntn-x2
ntn-x2 authored Nov 8, 2021
1 parent 1d2595c commit fc56483
Show file tree
Hide file tree
Showing 8 changed files with 458 additions and 110 deletions.
20 changes: 13 additions & 7 deletions pallets/delegation/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@ targets = ["x86_64-unknown-linux-gnu"]
substrate-wasm-builder-runner = {version = "3.0.0"}

[dev-dependencies]
# Internal dependencies
ctype = {features = ["mock"], path = "../ctype", version = "1.1.1"}
env_logger = {version = "0.8.4"}
kilt-primitives = {default-features = false, path = "../../primitives"}
pallet-balances = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate"}

# External dependencies
env_logger = {version = "0.8.4"}
serde = {version = "1.0.124"}

# Substrate dependencies
pallet-balances = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-core = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-keystore = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate"}

Expand All @@ -30,13 +35,15 @@ kilt-support = {default-features = false, path = "../../support"}
#External dependencies
bitflags = {default-features = false, version = "1.3.2"}
codec = {default-features = false, features = ["derive"], package = "parity-scale-codec", version = "2.3.1"}
log = {default-features = false, version = "0.4.14"}
scale-info = { version = "1.0", default-features = false, features = ["derive"] }
serde = {version = "1.0.124", optional = true}

# Substrate dependencies
frame-benchmarking = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate", optional = true}
frame-support = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate"}
frame-system = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate"}
log = {default-features = false, version = "0.4.14"}
pallet-balances = {optional = true, branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate"}
scale-info = { version = "1.0", default-features = false, features = ["derive"] }
serde = {optional = true, version = "1.0.124"}
pallet-balances = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate", optional = true}
sp-core = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate", optional = true}
sp-io = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate", optional = true}
sp-keystore = {branch = "polkadot-v0.9.12", default-features = false, git = "https://github.com/paritytech/substrate", optional = true}
Expand All @@ -46,7 +53,6 @@ sp-std = {branch = "polkadot-v0.9.12", default-features = false, git = "https://
[features]
default = ["std"]
mock = [
"pallet-balances",
"pallet-balances",
"serde",
"sp-core",
Expand Down
22 changes: 20 additions & 2 deletions pallets/delegation/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ benchmarks! {
}
// TODO: Might want to add variant iterating over children instead of depth at some later point

// worst case is achieved by removing the root node, since `is_delegating` is not called in remove extrinsic,
// worst case is achieved by removing the root node, since `is_delegating` is not called in remove extrinsic
remove_delegation {
let r in 1 .. T::MaxRemovals::get();
let (root_acc, hierarchy_id, _, leaf_id) = setup_delegations::<T>(r, ONE_CHILD_PER_LEVEL.expect(">0"), Permissions::DELEGATE)?;
Expand All @@ -329,7 +329,25 @@ benchmarks! {
let child_id: T::DelegationNodeId = *children.iter().next().ok_or("Root should have children")?;
let child_delegation = DelegationNodes::<T>::get(child_id).ok_or("Child of root should have delegation id")?;
assert!(!<T as Config>::Currency::reserved_balance(&root_acc.into()).is_zero());
}: remove_delegation(RawOrigin::Signed(root_owner.clone()), hierarchy_id, r)
}: _(RawOrigin::Signed(root_owner.clone()), hierarchy_id, r)
verify {
assert!(!DelegationNodes::<T>::contains_key(hierarchy_id));
assert!(!DelegationNodes::<T>::contains_key(child_id));
assert!(!DelegationNodes::<T>::contains_key(leaf_id));
assert!(<T as Config>::Currency::reserved_balance(&root_acc.into()).is_zero());
}

// worst case is achieved by removing the root node, since `is_delegating` is not called in remove extrinsic
reclaim_deposit {
let r in 1 .. T::MaxRemovals::get();
let (root_acc, hierarchy_id, _, leaf_id) = setup_delegations::<T>(r, ONE_CHILD_PER_LEVEL.expect(">0"), Permissions::DELEGATE)?;
let root_owner = T::AccountId::from(root_acc).into();
let root_node = DelegationNodes::<T>::get(hierarchy_id).expect("Root hierarchy node should be present on chain.");
let children: BoundedBTreeSet<T::DelegationNodeId, T::MaxChildren> = root_node.children;
let child_id: T::DelegationNodeId = *children.iter().next().ok_or("Root should have children")?;
let child_delegation = DelegationNodes::<T>::get(child_id).ok_or("Child of root should have delegation id")?;
assert!(!<T as Config>::Currency::reserved_balance(&root_acc.into()).is_zero());
}: _(RawOrigin::Signed(root_owner.clone()), hierarchy_id, r)
verify {
assert!(!DelegationNodes::<T>::contains_key(hierarchy_id));
assert!(!DelegationNodes::<T>::contains_key(child_id));
Expand Down
102 changes: 69 additions & 33 deletions pallets/delegation/src/default_weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
//! Autogenerated weights for delegation
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2021-10-15, STEPS: {{cmd.steps}}\, REPEAT: {{cmd.repeat}}\, LOW RANGE: {{cmd.lowest_range_values}}\, HIGH RANGE: {{cmd.highest_range_values}}\
//! DATE: 2021-11-08, 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: 128
// Executed Command:
// ./target/release/kilt-parachain
// target/release/kilt-parachain
// benchmark
// --chain=dev
// --steps=50
Expand All @@ -33,13 +33,14 @@
// --execution=wasm
// --wasm-execution=compiled
// --heap-pages=4096
// --output=./pallets/delegation/src/default_weights.rs
// --output=pallets/delegation/src/default_weights.rs
// --template=.maintain/weight-template.hbs


#![cfg_attr(rustfmt, rustfmt_skip)]
#![allow(unused_parens)]
#![allow(unused_imports)]
#![allow(clippy::unnecessary_cast)]

use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}};
use sp_std::marker::PhantomData;
Expand All @@ -51,91 +52,126 @@ pub trait WeightInfo {
fn revoke_delegation_root_child(r: u32, c: u32, ) -> Weight;
fn revoke_delegation_leaf(r: u32, c: u32, ) -> Weight;
fn remove_delegation(r: u32, ) -> Weight;
fn reclaim_deposit(r: u32, ) -> Weight;
}

/// Weights for delegation using the Substrate node and recommended hardware.
pub struct SubstrateWeight<T>(PhantomData<T>);
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Storage: Delegation DelegationHierarchies (r:1 w:1)
// Storage: Ctype Ctypes (r:1 w:0)
// Storage: System Account (r:1 w:1)
// Storage: Delegation DelegationNodes (r:0 w:1)
fn create_hierarchy() -> Weight {
(55_494_000_u64)
(47_416_000_u64)
.saturating_add(T::DbWeight::get().reads(3_u64))
.saturating_add(T::DbWeight::get().writes(3_u64))
}
// Storage: Delegation DelegationNodes (r:2 w:2)
// Storage: System Account (r:1 w:1)
fn add_delegation() -> Weight {
(66_064_000_u64)
(57_220_000_u64)
.saturating_add(T::DbWeight::get().reads(3_u64))
.saturating_add(T::DbWeight::get().writes(3_u64))
}
// Storage: Delegation DelegationNodes (r:1 w:1)
// Storage: Delegation DelegationHierarchies (r:1 w:0)
fn revoke_delegation_root_child(r: u32, c: u32, ) -> Weight {
(24_683_000_u64)
// Standard Error: 46_000
.saturating_add((21_861_000_u64).saturating_mul(r as Weight))
// Standard Error: 46_000
.saturating_add((53_000_u64).saturating_mul(c as Weight))
(22_152_000_u64)
// Standard Error: 55_000
.saturating_add((18_681_000_u64).saturating_mul(r as Weight))
// Standard Error: 55_000
.saturating_add((95_000_u64).saturating_mul(c as Weight))
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(r as Weight)))
.saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(r as Weight)))
}
// Storage: Delegation DelegationNodes (r:6 w:1)
// Storage: Delegation DelegationHierarchies (r:1 w:0)
fn revoke_delegation_leaf(r: u32, c: u32, ) -> Weight {
(45_562_000_u64)
// Standard Error: 32_000
.saturating_add((9_000_u64).saturating_mul(r as Weight))
// Standard Error: 32_000
.saturating_add((6_396_000_u64).saturating_mul(c as Weight))
(39_250_000_u64)
// Standard Error: 34_000
.saturating_add((147_000_u64).saturating_mul(r as Weight))
// Standard Error: 34_000
.saturating_add((5_748_000_u64).saturating_mul(c as Weight))
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(c as Weight)))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
// Storage: Delegation DelegationNodes (r:2 w:2)
// Storage: System Account (r:1 w:1)
// Storage: Delegation DelegationHierarchies (r:1 w:1)
fn remove_delegation(r: u32, ) -> Weight {
(69_441_000_u64)
// Standard Error: 70_000
.saturating_add((45_490_000_u64).saturating_mul(r as Weight))
(61_171_000_u64)
// Standard Error: 99_000
.saturating_add((37_949_000_u64).saturating_mul(r as Weight))
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().reads((2_u64).saturating_mul(r as Weight)))
.saturating_add(T::DbWeight::get().writes(2_u64))
.saturating_add(T::DbWeight::get().writes((2_u64).saturating_mul(r as Weight)))
}
// Storage: Delegation DelegationNodes (r:2 w:2)
// Storage: System Account (r:1 w:1)
// Storage: Delegation DelegationHierarchies (r:0 w:1)
fn reclaim_deposit(r: u32, ) -> Weight {
(49_884_000_u64)
// Standard Error: 64_000
.saturating_add((38_418_000_u64).saturating_mul(r as Weight))
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().reads((2_u64).saturating_mul(r as Weight)))
.saturating_add(T::DbWeight::get().writes(2_u64))
.saturating_add(T::DbWeight::get().writes((2_u64).saturating_mul(r as Weight)))
}
}

// For backwards compatibility and tests
impl WeightInfo for () {
fn create_hierarchy() -> Weight {
(55_494_000_u64)
(47_416_000_u64)
.saturating_add(RocksDbWeight::get().reads(3_u64))
.saturating_add(RocksDbWeight::get().writes(3_u64))
}
fn add_delegation() -> Weight {
(66_064_000_u64)
(57_220_000_u64)
.saturating_add(RocksDbWeight::get().reads(3_u64))
.saturating_add(RocksDbWeight::get().writes(3_u64))
}
fn revoke_delegation_root_child(r: u32, c: u32, ) -> Weight {
(24_683_000_u64)
// Standard Error: 46_000
.saturating_add((21_861_000_u64).saturating_mul(r as Weight))
// Standard Error: 46_000
.saturating_add((53_000_u64).saturating_mul(c as Weight))
(22_152_000_u64)
// Standard Error: 55_000
.saturating_add((18_681_000_u64).saturating_mul(r as Weight))
// Standard Error: 55_000
.saturating_add((95_000_u64).saturating_mul(c as Weight))
.saturating_add(RocksDbWeight::get().reads(1_u64))
.saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(r as Weight)))
.saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(r as Weight)))
}
fn revoke_delegation_leaf(r: u32, c: u32, ) -> Weight {
(45_562_000_u64)
// Standard Error: 32_000
.saturating_add((9_000_u64).saturating_mul(r as Weight))
// Standard Error: 32_000
.saturating_add((6_396_000_u64).saturating_mul(c as Weight))
(39_250_000_u64)
// Standard Error: 34_000
.saturating_add((147_000_u64).saturating_mul(r as Weight))
// Standard Error: 34_000
.saturating_add((5_748_000_u64).saturating_mul(c as Weight))
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(c as Weight)))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
fn remove_delegation(r: u32, ) -> Weight {
(69_441_000_u64)
// Standard Error: 70_000
.saturating_add((45_490_000_u64).saturating_mul(r as Weight))
(61_171_000_u64)
// Standard Error: 99_000
.saturating_add((37_949_000_u64).saturating_mul(r as Weight))
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().reads((2_u64).saturating_mul(r as Weight)))
.saturating_add(RocksDbWeight::get().writes(2_u64))
.saturating_add(RocksDbWeight::get().writes((2_u64).saturating_mul(r as Weight)))
}
fn reclaim_deposit(r: u32, ) -> Weight {
(49_884_000_u64)
// Standard Error: 64_000
.saturating_add((38_418_000_u64).saturating_mul(r as Weight))
.saturating_add(RocksDbWeight::get().reads(1_u64))
.saturating_add(RocksDbWeight::get().reads((2_u64).saturating_mul(r as Weight)))
.saturating_add(RocksDbWeight::get().writes(2_u64))
.saturating_add(RocksDbWeight::get().writes((2_u64).saturating_mul(r as Weight)))
}
}
62 changes: 59 additions & 3 deletions pallets/delegation/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ pub mod pallet {
/// A delegation has been removed.
/// \[remover ID, delegation node ID\]
DelegationRemoved(AccountIdOf<T>, DelegationNodeIdOf<T>),
/// The deposit owner reclaimed a deposit by removing a delegation
/// subtree. \[revoker ID, delegation node ID\]
DepositReclaimed(AccountIdOf<T>, DelegationNodeIdOf<T>),
}

#[pallet::error]
Expand Down Expand Up @@ -553,7 +556,6 @@ pub mod pallet {
Self::deposit_event(Event::HierarchyRevoked(invoker, delegation_id));
}

// Add worst case reads from `is_delegating`
Ok(Some(
<T as Config>::WeightInfo::revoke_delegation_root_child(revocation_checks, parent_checks).max(
<T as Config>::WeightInfo::revoke_delegation_leaf(revocation_checks, parent_checks),
Expand All @@ -565,7 +567,7 @@ pub mod pallet {
/// Remove a delegation node (potentially a root node) and all its
/// children.
///
/// Returns the delegation deposit back to the deposit owner for each
/// Returns the delegation deposit to the deposit owner for each
/// removed DelegationNode by unreserving it.
///
/// Removing a delegation node results in the trust hierarchy starting
Expand Down Expand Up @@ -622,7 +624,61 @@ pub mod pallet {
Self::deposit_event(Event::HierarchyRemoved(invoker, delegation_id));
}

// Add worst case reads from `is_delegating`
Ok(Some(<T as Config>::WeightInfo::remove_delegation(removal_checks)).into())
}

/// Reclaim the deposit for a delegation node (potentially a root
/// node), removing the node and all its children.
///
/// Returns the delegation deposit to the deposit owner for each
/// removed DelegationNode by unreserving it.
///
/// Removing a delegation node results in the trust hierarchy starting
/// from the given node being removed. Nevertheless, removal starts
/// from the leave nodes upwards, so if the operation ends prematurely
/// because it runs out of gas, the delegation state would be consistent
/// as no child would "survive" its parent. As a consequence, if the
/// given node is removed, the trust hierarchy with the node as root is
/// to be considered removed.
///
/// The dispatch origin must be signed by the delegation deposit owner.
///
/// `DepositReclaimed`.
///
/// # <weight>
/// Weight: O(C) where C is the number of children of the delegation
/// node which is bounded by `max_removals`.
/// - Reads: [Origin Account], Roots, C * Delegations, C * Children.
/// - Writes: Roots, 2 * C * Delegations
/// # </weight>
#[pallet::weight(<T as Config>::WeightInfo::reclaim_deposit(*max_removals))]
pub fn reclaim_deposit(
origin: OriginFor<T>,
delegation_id: DelegationNodeIdOf<T>,
max_removals: u32,
) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin)?;

let delegation = DelegationNodes::<T>::get(&delegation_id).ok_or(Error::<T>::DelegationNotFound)?;

// Deposit can only be removed by the owner of the deposit, not the
// parent or another ancestor.
ensure!(delegation.deposit.owner == who, Error::<T>::UnauthorizedRemoval);

ensure!(max_removals <= T::MaxRemovals::get(), Error::<T>::MaxRemovalsTooLarge);

// *** No Fail except during removal beyond this point ***

// Remove the delegation and recursively all of its children (add 1 to
// max_removals to account for the node itself), releasing the associated
// deposit
let (removal_checks, _) = Self::remove(&delegation_id, max_removals.saturating_add(1))?;

// Delete the delegation hierarchy details, if the provided ID was for a root
// node. No event generated as we don't have information about the owner DID
// here.
DelegationHierarchies::<T>::remove(&delegation_id);

Ok(Some(<T as Config>::WeightInfo::remove_delegation(removal_checks)).into())
}
}
Expand Down
14 changes: 14 additions & 0 deletions pallets/delegation/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,11 @@ pub struct DelegationRevocationOperation {
pub max_revocations: u32,
}

pub struct DelegationDepositClaimOperation {
pub delegation_id: TestDelegationNodeId,
pub max_removals: u32,
}

pub fn generate_base_delegation_revocation_operation(
delegation_id: TestDelegationNodeId,
) -> DelegationRevocationOperation {
Expand All @@ -348,6 +353,15 @@ pub fn generate_base_delegation_revocation_operation(
}
}

pub fn generate_base_delegation_deposit_claim_operation(
delegation_id: TestDelegationNodeId,
) -> DelegationDepositClaimOperation {
DelegationDepositClaimOperation {
delegation_id,
max_removals: 0u32,
}
}

pub fn initialize_pallet<T: Config>(
delegations: Vec<(T::DelegationNodeId, DelegationNode<T>)>,
delegation_hierarchies: Vec<(
Expand Down
Loading

0 comments on commit fc56483

Please sign in to comment.