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: thanks and much gratitude! #282

Merged
merged 49 commits into from
Oct 28, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
a76f37d
No need for lookup here
weichweich Oct 26, 2021
6299284
gratitude functionallity
weichweich Oct 26, 2021
4995444
test: receive_gratitude
weichweich Oct 26, 2021
fe3312b
feat: benchmark
weichweich Oct 26, 2021
7760917
fix benchmarks
weichweich Oct 26, 2021
11237fb
fix benchmark
weichweich Oct 27, 2021
e037a8f
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 27, 2021
21ebaff
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 27, 2021
1403f6f
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 27, 2021
09655f4
doc: document changes
weichweich Oct 27, 2021
06a1711
use the same test style
weichweich Oct 27, 2021
c3e5b8b
minor fixes
weichweich Oct 27, 2021
3941b2a
clippy
weichweich Oct 27, 2021
a90a2cf
clean dependencies
weichweich Oct 27, 2021
10d57db
🧹🐢
weichweich Oct 27, 2021
883e59d
replace perquintill with permill and add test
weichweich Oct 27, 2021
4cf1509
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 27, 2021
979b8b1
🧹🐢
weichweich Oct 27, 2021
5e544df
fix dependencies
weichweich Oct 27, 2021
ad96003
check in validate_unsigned for successful TX
weichweich Oct 27, 2021
ea0dec3
add validate_unsigned to runtimes
weichweich Oct 27, 2021
572b9e6
reduce diff between runtimes
weichweich Oct 27, 2021
ab7f9ed
custom errors
weichweich Oct 27, 2021
ebcba63
remove redundant test
weichweich Oct 27, 2021
1fc4b3c
clipy
weichweich Oct 27, 2021
d246ab6
document error
weichweich Oct 27, 2021
0c07692
fixes
weichweich Oct 27, 2021
4cc3c04
Merge remote-tracking branch 'origin/develop' into aw-thanks
weichweich Oct 27, 2021
2133775
🧹🐢
weichweich Oct 27, 2021
1962bf6
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 27, 2021
feef542
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 27, 2021
e72f5f7
default config
weichweich Oct 27, 2021
28ec259
filter gratitude
weichweich Oct 27, 2021
a83a697
handle error cases
weichweich Oct 28, 2021
e9c782e
ensure can vest & optimisations
weichweich Oct 28, 2021
865af56
remove trait bound from benchmark
weichweich Oct 28, 2021
e4ecf0d
update weight template
weichweich Oct 28, 2021
cc60d3b
clippy
weichweich Oct 28, 2021
67ac756
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 28, 2021
9cb4c23
allow unnecessary_cast
weichweich Oct 28, 2021
2f811aa
Merge remote-tracking branch 'origin/develop' into aw-thanks
weichweich Oct 28, 2021
f9266bc
clippy again
weichweich Oct 28, 2021
0acd108
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 28, 2021
d45ad85
cargo run --quiet --release -p kilt-parachain --features=runtime-benc…
Oct 28, 2021
e32f92d
Update pallets/crowdloan/src/lib.rs
weichweich Oct 28, 2021
99162bf
fix spelling
weichweich Oct 28, 2021
1c4d440
🧹🐢
weichweich Oct 28, 2021
1dba99b
fix renamed clippy
weichweich Oct 28, 2021
86bf869
fix clippy once and for all!
weichweich Oct 28, 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
5 changes: 3 additions & 2 deletions Cargo.lock

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

10 changes: 6 additions & 4 deletions pallets/crowdloan/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ description = "Substrate pallet allowing to register crowdloan contributors."
edition = "2018"
name = "crowdloan"
repository = "https://github.com/KILTprotocol/mashnet-node"
version = "0.26.2"
version = "1.0.0"

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand All @@ -15,19 +15,21 @@ substrate-wasm-builder-runner = {version = "3.0.0"}
[dev-dependencies]
kilt-primitives = {default-features = false, path = "../../primitives"}
pallet-balances = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}
pallet-vesting = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}
serde = {version = "1.0.101"}
sp-keystore = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-io = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-keystore = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}

[dependencies]
serde = {optional = true, version = "1.0.101"}

# Substrate dependencies
codec = {default-features = false, features = ["derive"], package = "parity-scale-codec", version = "2.0.0"}
scale-info = {version = "1.0", default-features = false, features = ["derive"]}

# Substrate dependencies
frame-benchmarking = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate", optional = true}
frame-support = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}
frame-system = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}
scale-info = { version = "1.0", default-features = false, features = ["derive"] }
sp-runtime = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-std = {branch = "polkadot-v0.9.11", default-features = false, git = "https://github.com/paritytech/substrate"}

Expand Down
90 changes: 87 additions & 3 deletions pallets/crowdloan/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,21 @@

use crate::*;

use codec::{Decode, Encode};
use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite};
use frame_support::{dispatch::UnfilteredDispatchable, traits::Currency, unsigned::ValidateUnsigned};
use frame_system::RawOrigin;
use sp_runtime::traits::{One, StaticLookup};
use sp_runtime::{
traits::{One, StaticLookup},
Permill,
};

const SEED_1: u32 = 1;
const SEED_2: u32 = 2;

benchmarks! {
where_clause { where <T as frame_system::Config>::BlockNumber: From<u32> }
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be needed? I think it is already configured that this primitive must always be at least U32.

See here: https://github.com/paritytech/substrate/blob/master/frame/system/src/lib.rs#L201

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! That's super handy! 865af56


set_registrar_account {
let registrar: AccountIdOf<T> = account("registrar", 0, SEED_1);
let new_registrar: AccountIdOf<T> = account("new_registrar", 0, SEED_2);
Expand All @@ -44,7 +51,7 @@ benchmarks! {
let contributor: AccountIdOf<T> = account("contributor", 0, SEED_2);
let contribution: BalanceOf<T> = BalanceOf::<T>::one();
RegistrarAccount::<T>::set(registrar.clone());
}: _(RawOrigin::Signed(registrar), T::Lookup::unlookup(contributor.clone()), contribution)
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, you should be weary of placing a function like unlookup here, since it will impact the complexity and timing of your benchmark. (I think)

Imagine any of the code after the setup is triggering after the timer starts.

}: _(RawOrigin::Signed(registrar), contributor.clone(), contribution)
verify {
assert_eq!(
Contributions::<T>::get(&contributor),
Expand All @@ -53,13 +60,90 @@ benchmarks! {
);
}

set_config {
let registrar: AccountIdOf<T> = account("registrar", 0, SEED_1);
RegistrarAccount::<T>::set(registrar.clone());

let config = GratitudeConfig::<T::BlockNumber> {
vested_share: Permill::from_percent(42),
start_block: 1.into(),
vesting_length: 10.into(),
};
}: _(RawOrigin::Signed(registrar), config.clone())
verify {
assert_eq!(
Configuration::<T>::get(),
config,
);
}

set_reserve_accounts {
let registrar: AccountIdOf<T> = account("registrar", 0, SEED_1);
let reserve_free: AccountIdOf<T> = account("reserve_free", 0, SEED_1);
let reserve_vested: AccountIdOf<T> = account("reserve_vested", 0, SEED_1);
RegistrarAccount::<T>::set(registrar.clone());

ntn-x2 marked this conversation as resolved.
Show resolved Hide resolved
}: _(
RawOrigin::Signed(registrar),
T::Lookup::unlookup(reserve_vested.clone()),
T::Lookup::unlookup(reserve_free.clone())
Copy link
Contributor

@shawntabrizi shawntabrizi Oct 27, 2021

Choose a reason for hiding this comment

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

same warning here, just do this above and pass as a new variable.

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 this probably increases the weight a bit. fixed in 865af56

)
verify {
assert_eq!(
crate::Reserve::<T>::get(),
ntn-x2 marked this conversation as resolved.
Show resolved Hide resolved
ReserveAccounts {
vested: reserve_vested,
free: reserve_free,
}
);
}

// receive_gratitude is benchmarked together with validate_unsigned to accommodate for the additional cost of validate_unsigned
receive_gratitude {
let registrar: AccountIdOf<T> = account("registrar", 0, SEED_1);
let reserve_free: AccountIdOf<T> = account("reserve_free", 0, SEED_1);
let reserve_vested: AccountIdOf<T> = account("reserve_vested", 0, SEED_1);
let contributor: AccountIdOf<T> = account("contributor", 0, SEED_1);

let contribution: BalanceOf<T> = CurrencyOf::<T>::minimum_balance()
Copy link
Contributor

Choose a reason for hiding this comment

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

CurrencyOf::<T>::minimum_balance() * 3u32.into()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice! fixed in 865af56

+ CurrencyOf::<T>::minimum_balance()
+ CurrencyOf::<T>::minimum_balance();

RegistrarAccount::<T>::set(registrar);
Reserve::<T>::set(ReserveAccounts {
vested: reserve_vested.clone(),
free: reserve_free.clone(),
});
Contributions::<T>::insert(&contributor, contribution);
Configuration::<T>::set(GratitudeConfig {
vested_share: Permill::from_percent(50),
start_block: 1.into(),
vesting_length: 10.into(),
});
CurrencyOf::<T>::make_free_balance_be(&reserve_vested, contribution);
CurrencyOf::<T>::make_free_balance_be(&reserve_free, contribution);

let source = sp_runtime::transaction_validity::TransactionSource::External;
let call_enc = Call::<T>::receive_gratitude {
ntn-x2 marked this conversation as resolved.
Show resolved Hide resolved
receiver: contributor.clone(),
}.encode();
}: {
let call = <Call<T> as Decode>::decode(&mut &*call_enc)
.expect("call is encoded above, encoding must be correct");
crate::Pallet::<T>::validate_unsigned(source, &call).map_err(|e| -> &'static str { e.into() })?;
ntn-x2 marked this conversation as resolved.
Show resolved Hide resolved
call.dispatch_bypass_filter(RawOrigin::None.into())?;
}
verify {
assert!(crate::Contributions::<T>::get(contributor).is_none());
Copy link
Contributor

Choose a reason for hiding this comment

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

S: Let's also check whether the contributor received the gratitude.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

remove_contribution {
let registrar: AccountIdOf<T> = account("registrar", 0, SEED_1);
let contributor: AccountIdOf<T> = account("contributor", 0, SEED_2);
let contribution: BalanceOf<T> = BalanceOf::<T>::one();
RegistrarAccount::<T>::set(registrar.clone());
Contributions::<T>::insert(&contributor, contribution);
}: _(RawOrigin::Signed(registrar), T::Lookup::unlookup(contributor.clone()))
}: _(RawOrigin::Signed(registrar), contributor.clone())
verify {
assert!(
Contributions::<T>::get(&contributor).is_none(),
Expand Down
47 changes: 40 additions & 7 deletions pallets/crowdloan/src/default_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
//! Autogenerated weights for crowdloan
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2021-10-20, STEPS: {{cmd.steps}}\, REPEAT: {{cmd.repeat}}\, LOW RANGE: {{cmd.lowest_range_values}}\, HIGH RANGE: {{cmd.highest_range_values}}\
//! DATE: 2021-10-27, 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:
Expand Down Expand Up @@ -48,24 +48,42 @@ use sp_std::marker::PhantomData;
pub trait WeightInfo {
fn set_registrar_account() -> Weight;
fn set_contribution() -> Weight;
fn set_config() -> Weight;
fn set_reserve_accounts() -> Weight;
fn receive_gratitude() -> Weight;
fn remove_contribution() -> Weight;
}

/// Weights for crowdloan using the Substrate node and recommended hardware.
pub struct SubstrateWeight<T>(PhantomData<T>);
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
fn set_registrar_account() -> Weight {
(18_544_000_u64)
(21_644_000_u64)
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
fn set_contribution() -> Weight {
(20_679_000_u64)
(23_815_000_u64)
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
fn set_config() -> Weight {
(21_247_000_u64)
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

are you using an old benchmarking template / version?

You should be getting metadata information about the reads and writes from the benchmarking now.

https://github.com/paritytech/substrate/blob/master/frame/democracy/src/weights.rs#L79

https://github.com/paritytech/substrate/blob/master/utils/frame/benchmarking-cli/src/template.hbs#L24

fn set_reserve_accounts() -> Weight {
(24_336_000_u64)
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
fn receive_gratitude() -> Weight {
(178_299_000_u64)
.saturating_add(T::DbWeight::get().reads(8_u64))
.saturating_add(T::DbWeight::get().writes(6_u64))
}
fn remove_contribution() -> Weight {
(21_661_000_u64)
(25_459_000_u64)
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
Expand All @@ -74,17 +92,32 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// For backwards compatibility and tests
impl WeightInfo for () {
fn set_registrar_account() -> Weight {
(18_544_000_u64)
(21_644_000_u64)
.saturating_add(RocksDbWeight::get().reads(1_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
fn set_contribution() -> Weight {
(20_679_000_u64)
(23_815_000_u64)
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
fn set_config() -> Weight {
(21_247_000_u64)
.saturating_add(RocksDbWeight::get().reads(1_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
fn set_reserve_accounts() -> Weight {
(24_336_000_u64)
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
fn receive_gratitude() -> Weight {
(178_299_000_u64)
.saturating_add(RocksDbWeight::get().reads(8_u64))
.saturating_add(RocksDbWeight::get().writes(6_u64))
}
fn remove_contribution() -> Weight {
(21_661_000_u64)
(25_459_000_u64)
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
Expand Down
Loading