Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
913d46b
wip: use bounded vec in staking
wischli Jul 20, 2021
29f8e78
fix: add try_insert_replace
wischli Jul 28, 2021
ca92b10
fix: staking benchmarks
wischli Jul 28, 2021
7882dc7
fix: migrations by adding deprecated
wischli Jul 28, 2021
6ad5961
fix: clippy
wischli Jul 28, 2021
8b1d5b5
refactor: apply migration structure from #226
wischli Jul 29, 2021
1571513
chore: add staking migration to v5
wischli Jul 29, 2021
e720d19
chore: migrate to BoundedBTreeMap in staking
wischli Jul 30, 2021
b8b7f94
fix: remove v5 migration
wischli Aug 2, 2021
f3f859d
chore: migrate UnlockingAt to BoundedVec
wischli Aug 3, 2021
e4b85f5
chore: use BoundedVec in DelegatedAttestations
wischli Aug 3, 2021
1c350d5
refactor: align exceeded errors
wischli Aug 3, 2021
a555fa8
feat: delegation pallet storage refactor (#226)
ntn-x2 Aug 3, 2021
64d46bc
chore: convert BTreeSet to bounded version in delegations
wischli Aug 3, 2021
e7a526a
refactor: add missing const declarations
wischli Aug 3, 2021
ac73fb7
wip: migrate did pallet to bounded
wischli Aug 3, 2021
a47eac2
Merge remote-tracking branch 'origin/develop' into wf-bounded-vec
wischli Aug 4, 2021
84c93d0
fix: did tests
wischli Aug 4, 2021
3a61f3e
Merge remote-tracking branch 'origin/develop' into wf-bounded-vec
wischli Aug 5, 2021
7e83fa5
refactor: rm redundancy, improve mock by splitting
wischli Aug 5, 2021
ebe38e3
fix: clippy
wischli Aug 5, 2021
00f8d80
docs: improve insertion attempt comments
wischli Aug 5, 2021
0553b90
fix: incorrect names in Debug
wischli Aug 6, 2021
f1dac50
refactor: remove redundant isCandidate check
wischli Aug 9, 2021
080c93f
fix: migrate Url to BoundedVec
wischli Aug 9, 2021
d166529
Merge remote-tracking branch 'origin/develop' into wf-bounded-vec
wischli Aug 9, 2021
b90ed76
refactor: MaxPublicKeysPerDid
wischli Aug 9, 2021
a807d3f
fix: bounds for did structs
wischli Aug 10, 2021
5120472
refactor: apply shorter unlocking mutation
wischli Aug 10, 2021
038677c
fix: remove unused pub
wischli Aug 10, 2021
158c0cc
refactor: try_insert, try_upsert
wischli Aug 10, 2021
1201ef8
fix: rm expect in delegator constructor
wischli Aug 10, 2021
522c885
fix: allow delegator constructor to throw
wischli Aug 10, 2021
77b653b
refactor: apply suggestion for sort_greatest_to_lowest
wischli Aug 10, 2021
3bb2acc
refactor: remove deprecated in staking
wischli Aug 10, 2021
54d17ce
fix: clippy
wischli Aug 10, 2021
7ffd5ab
fix: split did::mock to make benchmarks work again
wischli Aug 10, 2021
cf09493
refactor: DidKeyAgreementKeys
wischli Aug 10, 2021
c0422cb
chore: update benchmarks
wischli Aug 11, 2021
9b92141
chore: bump spec version to 20
wischli Aug 11, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions pallets/attestation/src/default_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,22 @@
//! Autogenerated weights for attestation
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0
//! DATE: 2021-07-21, STEPS: {{cmd.steps}}\, REPEAT: {{cmd.repeat}}\, LOW RANGE: {{cmd.lowest_range_values}}\, HIGH RANGE: {{cmd.highest_range_values}}\
//! DATE: 2021-08-11, 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
// /home/willi/mashnet-node/target/release/kilt-parachain
// benchmark
// --chain=dev
// --steps=50
// --repeat=20
// --pallet=attestation
// --extrinsic=*
// --execution=wasm
// --wasm-execution=Compiled
// --wasm-execution=compiled
// --heap-pages=4096
// --extrinsic=*
// --pallet=attestation
// --steps=1
// --repeat=20
// --template
// .maintain/weight-template.hbs
// --output
// pallets/attestation/src/default_weights.rs
// --output=../../pallets/attestation/src/default_weights.rs
// --template=../../.maintain/weight-template.hbs


#![cfg_attr(rustfmt, rustfmt_skip)]
Expand All @@ -56,14 +54,14 @@ pub trait WeightInfo {
pub struct SubstrateWeight<T>(PhantomData<T>);
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
fn add() -> Weight {
(38_091_000_u64)
(67_918_000_u64)
.saturating_add(T::DbWeight::get().reads(5_u64))
.saturating_add(T::DbWeight::get().writes(2_u64))
}
fn revoke(d: u32, ) -> Weight {
(25_042_000_u64)
// Standard Error: 28_000
.saturating_add((4_866_000_u64).saturating_mul(d as Weight))
(46_093_000_u64)
// Standard Error: 202_000
.saturating_add((8_252_000_u64).saturating_mul(d as Weight))
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(d as Weight)))
.saturating_add(T::DbWeight::get().writes(1_u64))
Expand All @@ -73,14 +71,14 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// For backwards compatibility and tests
impl WeightInfo for () {
fn add() -> Weight {
(38_091_000_u64)
(67_918_000_u64)
.saturating_add(RocksDbWeight::get().reads(5_u64))
.saturating_add(RocksDbWeight::get().writes(2_u64))
}
fn revoke(d: u32, ) -> Weight {
(25_042_000_u64)
// Standard Error: 28_000
.saturating_add((4_866_000_u64).saturating_mul(d as Weight))
(46_093_000_u64)
// Standard Error: 202_000
.saturating_add((8_252_000_u64).saturating_mul(d as Weight))
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(d as Weight)))
.saturating_add(RocksDbWeight::get().writes(1_u64))
Expand Down
24 changes: 19 additions & 5 deletions pallets/attestation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,14 @@ pub mod benchmarking;
#[cfg(test)]
mod tests;

use sp_std::vec::Vec;

pub use crate::{attestations::*, default_weights::WeightInfo, pallet::*};

use frame_support::traits::Get;

#[frame_support::pallet]
pub mod pallet {
use super::*;
use frame_support::pallet_prelude::*;
use frame_support::{pallet_prelude::*, BoundedVec};
use frame_system::pallet_prelude::*;

/// Type of a claim hash.
Expand All @@ -114,6 +112,11 @@ pub mod pallet {
type EnsureOrigin: EnsureOrigin<Success = AttesterOf<Self>, <Self as frame_system::Config>::Origin>;
type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
type WeightInfo: WeightInfo;

/// The maximum number of delegated attestations which can be made by
/// the same delegation.
#[pallet::constant]
type MaxDelegatedAttestations: Get<u32>;
}

#[pallet::pallet]
Expand All @@ -135,7 +138,12 @@ pub mod pallet {
/// It maps from a delegation ID to a vector of claim hashes.
#[pallet::storage]
#[pallet::getter(fn delegated_attestations)]
pub type DelegatedAttestations<T> = StorageMap<_, Blake2_128Concat, DelegationNodeIdOf<T>, Vec<ClaimHashOf<T>>>;
pub type DelegatedAttestations<T> = StorageMap<
_,
Blake2_128Concat,
DelegationNodeIdOf<T>,
BoundedVec<ClaimHashOf<T>, <T as Config>::MaxDelegatedAttestations>,
>;

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
Expand Down Expand Up @@ -178,6 +186,10 @@ pub mod pallet {
/// is but it has been revoked. Only when the revoker is not the
/// original attester.
UnauthorizedRevocation,
/// The maximum number of delegated attestations has already been
/// reached for the corresponding delegation id such that another one
/// cannot be added.
MaxDelegatedAttestationsExceeded,
}

#[pallet::call]
Expand Down Expand Up @@ -244,7 +256,9 @@ pub mod pallet {

// If the attestation is based on a delegation, store separately
let mut delegated_attestations = <DelegatedAttestations<T>>::get(delegation_id).unwrap_or_default();
delegated_attestations.push(claim_hash);
delegated_attestations
.try_push(claim_hash)
.map_err(|_| Error::<T>::MaxDelegatedAttestationsExceeded)?;
<DelegatedAttestations<T>>::insert(delegation_id, delegated_attestations);
}

Expand Down
21 changes: 18 additions & 3 deletions pallets/attestation/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::*;
use ctype::mock as ctype_mock;

use codec::Decode;
use frame_support::{ensure, parameter_types, weights::constants::RocksDbWeight};
use frame_support::{ensure, parameter_types, weights::constants::RocksDbWeight, BoundedVec};
use frame_system::EnsureSigned;
use sp_core::{ed25519, sr25519, Pair};
use sp_keystore::{testing::KeyStore, KeystoreExt};
Expand Down Expand Up @@ -100,6 +100,8 @@ parameter_types! {
pub const MaxSignatureByteLength: u16 = 64;
pub const MaxParentChecks: u32 = 5;
pub const MaxRevocations: u32 = 5;
#[derive(Clone)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like deriving Debug, Clone, Ord or PartialEq should not be required for u32. Yet, the compiler complained if the corresponding associated types defined in the Config required to implement these traits and I did not derive them in the const declarations. How can this be improved @weichweich ?

You will notice a bunch of these more below. I just commented here because this is the first occasion.

Copy link
Contributor

Choose a reason for hiding this comment

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

interesting. The thing here is that this is not a "normal" const but a new type called MaxChildren and that MaxChildren doesn't implement the clone trait!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. But why though? Shouldn't this be implemented by default if the type is a u32?

pub const MaxChildren: u32 = 1000;
}

impl delegation::Config for Test {
Expand All @@ -111,13 +113,20 @@ impl delegation::Config for Test {
type MaxSignatureByteLength = MaxSignatureByteLength;
type MaxParentChecks = MaxParentChecks;
type MaxRevocations = MaxRevocations;
type MaxChildren = MaxChildren;
type WeightInfo = ();
}

parameter_types! {
// TODO: Find reasonable number
pub const MaxDelegatedAttestations: u32 = 1000;
}

impl Config for Test {
type EnsureOrigin = EnsureSigned<TestAttester>;
type Event = ();
type WeightInfo = ();
type MaxDelegatedAttestations = MaxDelegatedAttestations;
}

impl delegation::VerifyDelegateSignature for Test {
Expand Down Expand Up @@ -227,7 +236,10 @@ pub fn generate_base_attestation(attester: TestAttester) -> AttestationDetails<T
#[derive(Clone)]
pub struct ExtBuilder {
attestations_stored: Vec<(TestClaimHash, AttestationDetails<Test>)>,
delegated_attestations_stored: Vec<(TestDelegationNodeId, Vec<TestClaimHash>)>,
delegated_attestations_stored: Vec<(
TestDelegationNodeId,
BoundedVec<TestClaimHash, <Test as Config>::MaxDelegatedAttestations>,
)>,
}

impl Default for ExtBuilder {
Expand All @@ -247,7 +259,10 @@ impl ExtBuilder {

pub fn with_delegated_attestations(
mut self,
delegated_attestations: Vec<(TestDelegationNodeId, Vec<TestClaimHash>)>,
delegated_attestations: Vec<(
TestDelegationNodeId,
BoundedVec<TestClaimHash, <Test as Config>::MaxDelegatedAttestations>,
)>,
) -> Self {
self.delegated_attestations_stored = delegated_attestations;
self
Expand Down
6 changes: 3 additions & 3 deletions pallets/delegation/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@

use codec::Encode;
use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite};
use frame_support::dispatch::DispatchErrorWithPostInfo;
use frame_support::{dispatch::DispatchErrorWithPostInfo, storage::bounded_btree_set::BoundedBTreeSet};
use frame_system::RawOrigin;
use sp_core::{offchain::KeyTypeId, sr25519};
use sp_io::crypto::sr25519_generate;
use sp_runtime::MultiSignature;
use sp_std::{collections::btree_set::BTreeSet, num::NonZeroU32, vec::Vec};
use sp_std::{num::NonZeroU32, vec::Vec};

use crate::*;

Expand Down Expand Up @@ -223,7 +223,7 @@ benchmarks! {
let c in 1 .. T::MaxParentChecks::get();
let (_, hierarchy_id, leaf_acc, leaf_id) = setup_delegations::<T>(r, ONE_CHILD_PER_LEVEL.expect(">0"), Permissions::DELEGATE)?;
let root_node = DelegationNodes::<T>::get(hierarchy_id).expect("Root hierarchy node should be present on chain.");
let children: BTreeSet<T::DelegationNodeId> = root_node.children;
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")?;
}: revoke_delegation(RawOrigin::Signed(child_delegation.details.owner.clone()), child_id, c, r)
Expand Down
60 changes: 29 additions & 31 deletions pallets/delegation/src/default_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,22 @@
//! Autogenerated weights for delegation
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0
//! DATE: 2021-07-21, STEPS: {{cmd.steps}}\, REPEAT: {{cmd.repeat}}\, LOW RANGE: {{cmd.lowest_range_values}}\, HIGH RANGE: {{cmd.highest_range_values}}\
//! DATE: 2021-08-11, 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
// /home/willi/mashnet-node/target/release/kilt-parachain
// benchmark
// --chain=dev
// --steps=50
// --repeat=20
// --pallet=delegation
// --extrinsic=*
// --execution=wasm
// --wasm-execution=Compiled
// --wasm-execution=compiled
// --heap-pages=4096
// --extrinsic=*
// --pallet=delegation
// --steps=1
// --repeat=20
// --template
// .maintain/weight-template.hbs
// --output
// pallets/delegation/src/default_weights.rs
// --output=../../pallets/delegation/src/default_weights.rs
// --template=../../.maintain/weight-template.hbs


#![cfg_attr(rustfmt, rustfmt_skip)]
Expand All @@ -58,29 +56,29 @@ pub trait WeightInfo {
pub struct SubstrateWeight<T>(PhantomData<T>);
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
fn create_hierarchy() -> Weight {
(24_797_000_u64)
(49_402_000_u64)
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().writes(2_u64))
}
fn add_delegation() -> Weight {
(80_841_000_u64)
(131_777_000_u64)
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().writes(2_u64))
}
fn revoke_delegation_root_child(r: u32, _c: u32, ) -> Weight {
(17_804_000_u64)
// Standard Error: 94_000
.saturating_add((17_445_000_u64).saturating_mul(r as Weight))
(32_769_000_u64)
// Standard Error: 373_000
.saturating_add((29_614_000_u64).saturating_mul(r 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)))
}
fn revoke_delegation_leaf(r: u32, c: u32, ) -> Weight {
(31_213_000_u64)
// Standard Error: 61_000
.saturating_add((327_000_u64).saturating_mul(r as Weight))
// Standard Error: 61_000
.saturating_add((5_210_000_u64).saturating_mul(c as Weight))
(54_214_000_u64)
// Standard Error: 196_000
.saturating_add((484_000_u64).saturating_mul(r as Weight))
// Standard Error: 196_000
.saturating_add((8_692_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))
Expand All @@ -90,29 +88,29 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// For backwards compatibility and tests
impl WeightInfo for () {
fn create_hierarchy() -> Weight {
(24_797_000_u64)
(49_402_000_u64)
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().writes(2_u64))
}
fn add_delegation() -> Weight {
(80_841_000_u64)
(131_777_000_u64)
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().writes(2_u64))
}
fn revoke_delegation_root_child(r: u32, _c: u32, ) -> Weight {
(17_804_000_u64)
// Standard Error: 94_000
.saturating_add((17_445_000_u64).saturating_mul(r as Weight))
(32_769_000_u64)
// Standard Error: 373_000
.saturating_add((29_614_000_u64).saturating_mul(r 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 {
(31_213_000_u64)
// Standard Error: 61_000
.saturating_add((327_000_u64).saturating_mul(r as Weight))
// Standard Error: 61_000
.saturating_add((5_210_000_u64).saturating_mul(c as Weight))
(54_214_000_u64)
// Standard Error: 196_000
.saturating_add((484_000_u64).saturating_mul(r as Weight))
// Standard Error: 196_000
.saturating_add((8_692_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))
Expand Down
17 changes: 10 additions & 7 deletions pallets/delegation/src/delegation_hierarchy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use bitflags::bitflags;
use codec::{Decode, Encode};
use ctype::CtypeHashOf;
use sp_std::collections::btree_set::BTreeSet;
use frame_support::{dispatch::DispatchResult, storage::bounded_btree_set::BoundedBTreeSet};

use crate::*;

Expand Down Expand Up @@ -59,14 +59,14 @@ impl Default for Permissions {
/// For quicker lookups of the hierarchy details, all nodes maintain a direct
/// link to the hierarchy root node. Furthermore, all nodes have a parent except
/// the root nodes, which point to themselves for the hierarchy root node link.
#[derive(Clone, Debug, Encode, Decode, PartialEq)]
#[derive(Clone, Encode, Decode, PartialEq)]
pub struct DelegationNode<T: Config> {
/// The ID of the delegation hierarchy the node is part of.
pub hierarchy_root_id: DelegationNodeIdOf<T>,
/// The ID of the parent. For all but root nodes this is not None.
pub parent: Option<DelegationNodeIdOf<T>>,
/// The set of IDs of all the children nodes.
pub children: BTreeSet<DelegationNodeIdOf<T>>,
pub children: BoundedBTreeSet<DelegationNodeIdOf<T>, T::MaxChildren>,
/// The additional information attached to the delegation node.
pub details: DelegationDetails<T>,
}
Expand All @@ -78,7 +78,7 @@ impl<T: Config> DelegationNode<T> {
Self {
hierarchy_root_id: id,
parent: None,
children: BTreeSet::new(),
children: BoundedBTreeSet::<DelegationNodeIdOf<T>, T::MaxChildren>::new(),
details,
}
}
Expand All @@ -93,14 +93,17 @@ impl<T: Config> DelegationNode<T> {
Self {
hierarchy_root_id,
parent: Some(parent),
children: BTreeSet::new(),
children: BoundedBTreeSet::<DelegationNodeIdOf<T>, T::MaxChildren>::new(),
details,
}
}

/// Adds a node by its ID to the current node's children.
pub fn add_child(&mut self, child_id: DelegationNodeIdOf<T>) {
self.children.insert(child_id);
pub fn try_add_child(&mut self, child_id: DelegationNodeIdOf<T>) -> DispatchResult {
self.children
.try_insert(child_id)
.map_err(|_| Error::<T>::MaxChildrenExceeded)?;
Ok(())
}
}

Expand Down
Loading