From e3a5bb29a76abf5be22cc705b590c6cb3e966186 Mon Sep 17 00:00:00 2001 From: Gav Date: Fri, 7 Oct 2022 17:47:42 +0100 Subject: [PATCH 01/40] More drafting --- Cargo.lock | 17 +++++++++++++++++ Cargo.toml | 1 + frame/ranked-collective/src/lib.rs | 2 ++ frame/support/src/traits.rs | 2 +- frame/support/src/traits/members.rs | 15 +++++++++++++++ 5 files changed, 36 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 2f0a2df0f101b..ac95cde68be78 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6057,6 +6057,23 @@ dependencies = [ "sp-std", ] +[[package]] +name = "pallet-paymaster" +version = "4.0.0-dev" +dependencies = [ + "frame-benchmarking", + "frame-support", + "frame-system", + "log", + "parity-scale-codec", + "scale-info", + "sp-arithmetic", + "sp-core", + "sp-io", + "sp-runtime", + "sp-std", +] + [[package]] name = "pallet-preimage" version = "4.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index 02bc6aede8669..f1d2a6815d901 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -114,6 +114,7 @@ members = [ "frame/nicks", "frame/node-authorization", "frame/offences", + "frame/paymaster", "frame/preimage", "frame/proxy", "frame/nomination-pools", diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index fa3a473fe7d73..1c632a63dc601 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -718,3 +718,5 @@ pub mod pallet { } } } + +// TODO: Impl `RankedMembers` for this pallet. diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index d51c32649a797..8d07d526013c9 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -36,7 +36,7 @@ pub use members::{AllowAll, DenyAll, Filter}; pub use members::{ AsContains, ChangeMembers, Contains, ContainsLengthBound, ContainsPair, Everything, EverythingBut, FromContainsPair, InitializeMembers, InsideBoth, IsInVec, Nothing, - SortedMembers, TheseExcept, + RankedMembers, SortedMembers, TheseExcept, }; mod validation; diff --git a/frame/support/src/traits/members.rs b/frame/support/src/traits/members.rs index daf2d3aa6517d..82540f4249e07 100644 --- a/frame/support/src/traits/members.rs +++ b/frame/support/src/traits/members.rs @@ -18,6 +18,7 @@ //! Traits for dealing with the idea of membership. use impl_trait_for_tuples::impl_for_tuples; +use sp_arithmetic::traits::AtLeast32BitUnsigned; use sp_std::{marker::PhantomData, prelude::*}; /// A trait for querying whether a type can be said to "contain" a value. @@ -265,6 +266,20 @@ pub trait ContainsLengthBound { fn max_len() -> usize; } +/// Ranked membership data structure. +pub trait RankedMembers { + type AccountId; + type Rank: AtLeast32BitUnsigned; + + /// Return the rank of the given ID, or `None` if they are not a member. + fn rank_of(who: &Self::AccountId) -> Option; + + /// Remove a member from the group. This does not result in a call to `removed`. + fn remove(who: &Self::AccountId); + /// Change a member's rank. This does not result in a call to `changed`. + fn change(who: &Self::AccountId, rank: Self::Rank); +} + /// Trait for type that can handle the initialization of account IDs at genesis. pub trait InitializeMembers { /// Initialize the members to the given `members`. From 07ab87a86759a6f81fe087e221d6fead0b777a13 Mon Sep 17 00:00:00 2001 From: Gav Date: Tue, 11 Oct 2022 14:55:17 +0100 Subject: [PATCH 02/40] Paymaster pallet --- frame/paymaster/Cargo.toml | 49 +++ frame/paymaster/README.md | 1 + frame/paymaster/src/benchmarking.rs | 45 +++ frame/paymaster/src/lib.rs | 189 ++++++++++ frame/paymaster/src/tests.rs | 506 ++++++++++++++++++++++++++ frame/paymaster/src/weights.rs | 76 ++++ frame/support/src/traits/preimages.rs | 122 ++++++- 7 files changed, 981 insertions(+), 7 deletions(-) create mode 100644 frame/paymaster/Cargo.toml create mode 100644 frame/paymaster/README.md create mode 100644 frame/paymaster/src/benchmarking.rs create mode 100644 frame/paymaster/src/lib.rs create mode 100644 frame/paymaster/src/tests.rs create mode 100644 frame/paymaster/src/weights.rs diff --git a/frame/paymaster/Cargo.toml b/frame/paymaster/Cargo.toml new file mode 100644 index 0000000000000..4f02964bdafe9 --- /dev/null +++ b/frame/paymaster/Cargo.toml @@ -0,0 +1,49 @@ +[package] +name = "pallet-paymaster" +version = "4.0.0-dev" +authors = ["Parity Technologies "] +edition = "2021" +license = "Apache-2.0" +homepage = "https://substrate.io" +repository = "https://github.com/paritytech/substrate/" +description = "Paymaster" +readme = "README.md" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } +log = { version = "0.4.16", default-features = false } +scale-info = { version = "2.0.1", default-features = false, features = ["derive"] } +frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" } +frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } +frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } +sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../primitives/arithmetic" } +sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } +sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } +sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } +sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } + +[features] +default = ["std"] +std = [ + "codec/std", + "frame-benchmarking?/std", + "frame-support/std", + "frame-system/std", + "log/std", + "scale-info/std", + "sp-arithmetic/std", + "sp-core/std", + "sp-io/std", + "sp-runtime/std", + "sp-std/std", +] +runtime-benchmarks = [ + "frame-benchmarking/runtime-benchmarks", + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks", +] +try-runtime = ["frame-support/try-runtime"] diff --git a/frame/paymaster/README.md b/frame/paymaster/README.md new file mode 100644 index 0000000000000..7270167b7eceb --- /dev/null +++ b/frame/paymaster/README.md @@ -0,0 +1 @@ +# Paymaster \ No newline at end of file diff --git a/frame/paymaster/src/benchmarking.rs b/frame/paymaster/src/benchmarking.rs new file mode 100644 index 0000000000000..a26151654f256 --- /dev/null +++ b/frame/paymaster/src/benchmarking.rs @@ -0,0 +1,45 @@ +// This file is part of Substrate. + +// Copyright (C) 2020-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Staking pallet benchmarking. + +use super::*; +#[allow(unused_imports)] +use crate::Pallet as RankedCollective; + +use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller}; +use frame_support::{assert_ok, dispatch::UnfilteredDispatchable}; +use frame_system::RawOrigin as SystemOrigin; + +const SEED: u32 = 0; + +fn assert_last_event, I: 'static>(generic_event: >::RuntimeEvent) { + frame_system::Pallet::::assert_last_event(generic_event.into()); +} + +benchmarks_instance_pallet! { + add_member { + let origin = T::PromoteOrigin::successful_origin(); + let call = Call::::add_member { }; + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert_eq!(MemberCount::::get(0), 1); + assert_last_event::(Event::MemberAdded { who }.into()); + } + + impl_benchmark_test_suite!(RankedCollective, crate::tests::new_test_ext(), crate::tests::Test); +} diff --git a/frame/paymaster/src/lib.rs b/frame/paymaster/src/lib.rs new file mode 100644 index 0000000000000..05de49b3435ca --- /dev/null +++ b/frame/paymaster/src/lib.rs @@ -0,0 +1,189 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Proof-of-Personhood system. + +#![cfg_attr(not(feature = "std"), no_std)] +#![recursion_limit = "128"] + +use codec::Codec; +use scale_info::TypeInfo; +use sp_arithmetic::traits::Saturating; +use sp_core::bounded::BoundedVec; +use sp_runtime::{ + traits::{AtLeast32BitUnsigned, Convert, StaticLookup}, + ArithmeticError::Overflow, + Perbill, RuntimeDebug, +}; +use sp_std::{marker::PhantomData, prelude::*}; + +use frame_support::{ + codec::{Decode, Encode, MaxEncodedLen}, + dispatch::{DispatchError, DispatchResultWithPostInfo, PostDispatchInfo}, + ensure, + traits::{EnsureOrigin, PollStatus, Polling, RankedMembers, Time, VoteTally}, + CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, +}; + +#[cfg(test)] +mod tests; + +#[cfg(feature = "runtime-benchmarks")] +mod benchmarking; +pub mod weights; + +pub use pallet::*; +pub use weights::WeightInfo; + +/// Payroll cycle. +pub type Cycle = u32; + +// Can be implemented by Pot pallet with a fixed Currency impl, but can also be implemented with +// XCM/MultiAsset and made generic over assets. +pub trait Pay { + /// The type by which we measure units of the currency in which we make payments. + type Balance: AtLeast32BitUnsigned + Codec + MaxEncodedLen; + /// The type by which we identify the individuals to whom a payment may be made. + type AccountId; + /// An identifier given to an individual payment. + type Id; + /// The amount of currency with which we have to make payments in this period. It may be a fixed + /// value or reduce as calls to `pay` are made. It should be called once prior to the series of + /// payments to determine the overall budget and then not again until the next series of + /// payments are to be made. + fn budget() -> Self::Balance; + /// Make a payment and return an identifier for later evaluation of success in some off-chain + /// mechanism (likely an event, but possibly not on this chain). + fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result; +} + +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::{pallet_prelude::*, storage::KeyLenOf, weights::PaysFee}; + use frame_system::pallet_prelude::*; + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(PhantomData<(T, I)>); + + #[pallet::config] + pub trait Config: frame_system::Config { + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; + + /// The runtime event type. + type RuntimeEvent: From> + + IsType<::RuntimeEvent>; + + /// Means by which we can make payments to accounts. This also defines the currency and the + /// balance which we use to denote that currency. + type Paymaster: Pay::AccountId>; + + /// The current membership of payees. + type Members: RankedMembers::AccountId>; + + /// The maximum payout to be made for a single period to an active member of the given rank. + type ActiveSalaryForRank: Convert; + + /// The number of blocks between sequential payout cycles. + #[pallet::constant] + type CyclePeriod: Get; + } + + /// The next payout cycle to pay, and the block nunber at which it should be paid. + #[pallet::storage] + pub(super) type LastPayout, I: 'static = ()> = + StorageValue<_, T::BlockNumber, OptionQuery>; + + /// The most recent cycle which was paid to a member. None implies that a member has not yet + /// been paid. + #[pallet::storage] + pub(super) type LastClaim, I: 'static = ()> = + StorageMap<_, Twox64Concat, T::AccountId, T::BlockNumber, OptionQuery>; + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event, I: 'static = ()> { + /// A member is inducted into the payroll. + Inducted { who: T::AccountId }, + /// A payment happened. + Paid { who: T::AccountId }, + } + + #[pallet::error] + pub enum Error { + /// The account is not a ranked member. + NotMember, + // The account is not yet inducted into the system. + NotInducted, + /// The member does not have a current valid claim. + NoClaim, + /// Cycle is not yet over. + NotYet, + /// The payout cycles have not yet started. + NotStarted, + } + + #[pallet::call] + impl, I: 'static> Pallet { + #[pallet::weight(T::WeightInfo::add_member())] + pub fn induct(origin: OriginFor) -> DispatchResult { + let who = ensure_signed(origin)?; + let _ = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; + let last_payout = LastPayout::::get().unwrap_or_else(Zero::zero); + LastClaim::::put(who, last_payout); + Self::deposit_event(Event::::Inducted { who }); + Ok(Pays::No.into()) + } + + // TODO: preregistration of activity for the next cycle. + + /// Request a payout. + /// + /// - `origin`: A `Signed` origin of an account which is a member of `Members`. + #[pallet::weight(T::WeightInfo::add_member())] + pub fn payout(origin: OriginFor) -> DispatchResult { + let who = ensure_signed(origin)?; + let _rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; + let last_claim = LastClaim::::get(who).ok_or(Error::::NotInducted)?; + let last_payout = LastPayout::::get().or_or(Error::::NotStarted)?; + ensure!(last_claim < last_payout, Error::::NoClaim); + LastClaim::::put(who, last_payout); + + // TODO: make payout according to rank and activity. + let amount = Self::deposit_event(Event::::Paid { who }); + Ok(Pays::No.into()) + } + + /// Move to next payout cycle, assuming that the present block + /// + /// - `origin`: A `Signed` origin of an account which is a member of `Members`. + #[pallet::weight(T::WeightInfo::add_member())] + pub fn next_cycle(origin: OriginFor) -> DispatchResult { + let _ = ensure_signed(origin)?; + let now = frame_system::Pallet::::block_number(); + let last_payout = LastPayout::::get().or_or(Error::::NoClaim)?; + let next_payout = last_payout.saturating_add(T::CyclePeriod::get()); + ensure!(now >= next_payout, Error::::NotYet); + LastPayout::::put(next_payout); + Ok(Pays::No.into()) + } + } + + impl, I: 'static> Pallet {} +} diff --git a/frame/paymaster/src/tests.rs b/frame/paymaster/src/tests.rs new file mode 100644 index 0000000000000..68bb79f3d07f7 --- /dev/null +++ b/frame/paymaster/src/tests.rs @@ -0,0 +1,506 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! The crate's tests. + +use std::collections::BTreeMap; + +use frame_support::{ + assert_noop, assert_ok, + error::BadOrigin, + pallet_prelude::Weight, + parameter_types, + traits::{ConstU16, ConstU32, ConstU64, EitherOf, Everything, MapSuccess, Polling}, +}; +use sp_core::H256; +use sp_runtime::{ + testing::Header, + traits::{BlakeTwo256, Identity, IdentityLookup, ReduceBy}, +}; + +use super::*; +use crate as pallet_ranked_collective; + +type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; +type Block = frame_system::mocking::MockBlock; + +frame_support::construct_runtime!( + pub enum Test where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic, + { + System: frame_system::{Pallet, Call, Config, Storage, Event}, + Club: pallet_ranked_collective::{Pallet, Call, Storage, Event}, + } +); + +parameter_types! { + pub BlockWeights: frame_system::limits::BlockWeights = + frame_system::limits::BlockWeights::simple_max(Weight::from_ref_time(1_000_000)); +} +impl frame_system::Config for Test { + type BaseCallFilter = Everything; + type BlockWeights = (); + type BlockLength = (); + type DbWeight = (); + type RuntimeOrigin = RuntimeOrigin; + type Index = u64; + type BlockNumber = u64; + type RuntimeCall = RuntimeCall; + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type RuntimeEvent = RuntimeEvent; + type BlockHashCount = ConstU64<250>; + type Version = (); + type PalletInfo = PalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type SS58Prefix = (); + type OnSetCode = (); + type MaxConsumers = ConstU32<16>; +} + +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum TestPollState { + Ongoing(TallyOf, Rank), + Completed(u64, bool), +} +use TestPollState::*; + +parameter_types! { + pub static Polls: BTreeMap = vec![ + (1, Completed(1, true)), + (2, Completed(2, false)), + (3, Ongoing(Tally::from_parts(0, 0, 0), 1)), + ].into_iter().collect(); +} + +pub struct TestPolls; +impl Polling> for TestPolls { + type Index = u8; + type Votes = Votes; + type Moment = u64; + type Class = Rank; + fn classes() -> Vec { + vec![0, 1, 2] + } + fn as_ongoing(index: u8) -> Option<(TallyOf, Self::Class)> { + Polls::get().remove(&index).and_then(|x| { + if let TestPollState::Ongoing(t, c) = x { + Some((t, c)) + } else { + None + } + }) + } + fn access_poll( + index: Self::Index, + f: impl FnOnce(PollStatus<&mut TallyOf, Self::Moment, Self::Class>) -> R, + ) -> R { + let mut polls = Polls::get(); + let entry = polls.get_mut(&index); + let r = match entry { + Some(Ongoing(ref mut tally_mut_ref, class)) => + f(PollStatus::Ongoing(tally_mut_ref, *class)), + Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)), + None => f(PollStatus::None), + }; + Polls::set(polls); + r + } + fn try_access_poll( + index: Self::Index, + f: impl FnOnce( + PollStatus<&mut TallyOf, Self::Moment, Self::Class>, + ) -> Result, + ) -> Result { + let mut polls = Polls::get(); + let entry = polls.get_mut(&index); + let r = match entry { + Some(Ongoing(ref mut tally_mut_ref, class)) => + f(PollStatus::Ongoing(tally_mut_ref, *class)), + Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)), + None => f(PollStatus::None), + }?; + Polls::set(polls); + Ok(r) + } + + #[cfg(feature = "runtime-benchmarks")] + fn create_ongoing(class: Self::Class) -> Result { + let mut polls = Polls::get(); + let i = polls.keys().rev().next().map_or(0, |x| x + 1); + polls.insert(i, Ongoing(Tally::new(class), class)); + Polls::set(polls); + Ok(i) + } + + #[cfg(feature = "runtime-benchmarks")] + fn end_ongoing(index: Self::Index, approved: bool) -> Result<(), ()> { + let mut polls = Polls::get(); + match polls.get(&index) { + Some(Ongoing(..)) => {}, + _ => return Err(()), + } + let now = frame_system::Pallet::::block_number(); + polls.insert(index, Completed(now, approved)); + Polls::set(polls); + Ok(()) + } +} + +impl Config for Test { + type WeightInfo = (); + type RuntimeEvent = RuntimeEvent; + type PromoteOrigin = EitherOf< + // Root can promote arbitrarily. + frame_system::EnsureRootWithSuccess>, + // Members can promote up to the rank of 2 below them. + MapSuccess, ReduceBy>>, + >; + type DemoteOrigin = EitherOf< + // Root can demote arbitrarily. + frame_system::EnsureRootWithSuccess>, + // Members can demote up to the rank of 3 below them. + MapSuccess, ReduceBy>>, + >; + type Polls = TestPolls; + type MinRankOfClass = Identity; + type VoteWeight = Geometric; +} + +pub fn new_test_ext() -> sp_io::TestExternalities { + let t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + let mut ext = sp_io::TestExternalities::new(t); + ext.execute_with(|| System::set_block_number(1)); + ext +} + +fn next_block() { + System::set_block_number(System::block_number() + 1); +} + +fn member_count(r: Rank) -> MemberIndex { + MemberCount::::get(r) +} + +#[allow(dead_code)] +fn run_to(n: u64) { + while System::block_number() < n { + next_block(); + } +} + +fn tally(index: u8) -> TallyOf { + >>::as_ongoing(index).expect("No poll").0 +} + +#[test] +#[ignore] +#[should_panic(expected = "No poll")] +fn unknown_poll_should_panic() { + let _ = tally(0); +} + +#[test] +#[ignore] +#[should_panic(expected = "No poll")] +fn completed_poll_should_panic() { + let _ = tally(1); +} + +#[test] +fn basic_stuff() { + new_test_ext().execute_with(|| { + assert_eq!(tally(3), Tally::from_parts(0, 0, 0)); + }); +} + +#[test] +fn member_lifecycle_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 0); + assert_eq!(member_count(1), 0); + }); +} + +#[test] +fn add_remove_works() { + new_test_ext().execute_with(|| { + assert_noop!(Club::add_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 1); + + assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 0); + + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 1); + + assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); + assert_eq!(member_count(0), 2); + + assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); + assert_eq!(member_count(0), 3); + + assert_ok!(Club::demote_member(RuntimeOrigin::root(), 3)); + assert_eq!(member_count(0), 2); + + assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 1); + + assert_ok!(Club::demote_member(RuntimeOrigin::root(), 2)); + assert_eq!(member_count(0), 0); + }); +} + +#[test] +fn promote_demote_works() { + new_test_ext().execute_with(|| { + assert_noop!(Club::add_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 1); + assert_eq!(member_count(1), 0); + + assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); + assert_eq!(member_count(0), 2); + assert_eq!(member_count(1), 0); + + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 2); + assert_eq!(member_count(1), 1); + + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); + assert_eq!(member_count(0), 2); + assert_eq!(member_count(1), 2); + + assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 2); + assert_eq!(member_count(1), 1); + + assert_noop!(Club::demote_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin); + assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 1); + assert_eq!(member_count(1), 1); + }); +} + +#[test] +fn promote_demote_by_rank_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + for _ in 0..7 { + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); + } + + // #1 can add #2 and promote to rank 1 + assert_ok!(Club::add_member(RuntimeOrigin::signed(1), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); + // #2 as rank 1 cannot do anything privileged + assert_noop!(Club::add_member(RuntimeOrigin::signed(2), 3), BadOrigin); + + assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); + // #2 as rank 2 can add #3. + assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 3)); + + // #2 as rank 2 cannot promote #3 to rank 1 + assert_noop!( + Club::promote_member(RuntimeOrigin::signed(2), 3), + Error::::NoPermission + ); + + // #1 as rank 7 can promote #2 only up to rank 5 and once there cannot demote them. + assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); + assert_noop!( + Club::promote_member(RuntimeOrigin::signed(1), 2), + Error::::NoPermission + ); + assert_noop!(Club::demote_member(RuntimeOrigin::signed(1), 2), Error::::NoPermission); + + // #2 as rank 5 can promote #3 only up to rank 3 and once there cannot demote them. + assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 3)); + assert_noop!( + Club::promote_member(RuntimeOrigin::signed(2), 3), + Error::::NoPermission + ); + assert_noop!(Club::demote_member(RuntimeOrigin::signed(2), 3), Error::::NoPermission); + + // #2 can add #4 & #5 as rank 0 and #6 & #7 as rank 1. + assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 4)); + assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 5)); + assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 6)); + assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 6)); + assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 7)); + assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 7)); + + // #3 as rank 3 can demote/remove #4 & #5 but not #6 & #7 + assert_ok!(Club::demote_member(RuntimeOrigin::signed(3), 4)); + assert_ok!(Club::remove_member(RuntimeOrigin::signed(3), 5, 0)); + assert_noop!(Club::demote_member(RuntimeOrigin::signed(3), 6), Error::::NoPermission); + assert_noop!( + Club::remove_member(RuntimeOrigin::signed(3), 7, 1), + Error::::NoPermission + ); + + // #2 as rank 5 can demote/remove #6 & #7 + assert_ok!(Club::demote_member(RuntimeOrigin::signed(2), 6)); + assert_ok!(Club::remove_member(RuntimeOrigin::signed(2), 7, 1)); + }); +} + +#[test] +fn voting_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(RuntimeOrigin::root(), 0)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + + assert_noop!(Club::vote(RuntimeOrigin::signed(0), 3, true), Error::::RankTooLow); + assert_eq!(tally(3), Tally::from_parts(0, 0, 0)); + + assert_ok!(Club::vote(RuntimeOrigin::signed(1), 3, true)); + assert_eq!(tally(3), Tally::from_parts(1, 1, 0)); + assert_ok!(Club::vote(RuntimeOrigin::signed(1), 3, false)); + assert_eq!(tally(3), Tally::from_parts(0, 0, 1)); + + assert_ok!(Club::vote(RuntimeOrigin::signed(2), 3, true)); + assert_eq!(tally(3), Tally::from_parts(1, 3, 1)); + assert_ok!(Club::vote(RuntimeOrigin::signed(2), 3, false)); + assert_eq!(tally(3), Tally::from_parts(0, 0, 4)); + + assert_ok!(Club::vote(RuntimeOrigin::signed(3), 3, true)); + assert_eq!(tally(3), Tally::from_parts(1, 6, 4)); + assert_ok!(Club::vote(RuntimeOrigin::signed(3), 3, false)); + assert_eq!(tally(3), Tally::from_parts(0, 0, 10)); + }); +} + +#[test] +fn cleanup_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + + assert_ok!(Club::vote(RuntimeOrigin::signed(1), 3, true)); + assert_ok!(Club::vote(RuntimeOrigin::signed(2), 3, false)); + assert_ok!(Club::vote(RuntimeOrigin::signed(3), 3, true)); + + assert_noop!(Club::cleanup_poll(RuntimeOrigin::signed(4), 3, 10), Error::::Ongoing); + Polls::set( + vec![(1, Completed(1, true)), (2, Completed(2, false)), (3, Completed(3, true))] + .into_iter() + .collect(), + ); + assert_ok!(Club::cleanup_poll(RuntimeOrigin::signed(4), 3, 10)); + // NOTE: This will fail until #10016 is merged. + // assert_noop!(Club::cleanup_poll(RuntimeOrigin::signed(4), 3, 10), + // Error::::NoneRemaining); + }); +} + +#[test] +fn ensure_ranked_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + + use frame_support::traits::OriginTrait; + type Rank1 = EnsureRanked; + type Rank2 = EnsureRanked; + type Rank3 = EnsureRanked; + type Rank4 = EnsureRanked; + assert_eq!(Rank1::try_origin(RuntimeOrigin::signed(1)).unwrap(), 1); + assert_eq!(Rank1::try_origin(RuntimeOrigin::signed(2)).unwrap(), 2); + assert_eq!(Rank1::try_origin(RuntimeOrigin::signed(3)).unwrap(), 3); + assert_eq!( + Rank2::try_origin(RuntimeOrigin::signed(1)).unwrap_err().as_signed().unwrap(), + 1 + ); + assert_eq!(Rank2::try_origin(RuntimeOrigin::signed(2)).unwrap(), 2); + assert_eq!(Rank2::try_origin(RuntimeOrigin::signed(3)).unwrap(), 3); + assert_eq!( + Rank3::try_origin(RuntimeOrigin::signed(1)).unwrap_err().as_signed().unwrap(), + 1 + ); + assert_eq!( + Rank3::try_origin(RuntimeOrigin::signed(2)).unwrap_err().as_signed().unwrap(), + 2 + ); + assert_eq!(Rank3::try_origin(RuntimeOrigin::signed(3)).unwrap(), 3); + assert_eq!( + Rank4::try_origin(RuntimeOrigin::signed(1)).unwrap_err().as_signed().unwrap(), + 1 + ); + assert_eq!( + Rank4::try_origin(RuntimeOrigin::signed(2)).unwrap_err().as_signed().unwrap(), + 2 + ); + assert_eq!( + Rank4::try_origin(RuntimeOrigin::signed(3)).unwrap_err().as_signed().unwrap(), + 3 + ); + }); +} + +#[test] +fn do_add_member_to_rank_works() { + new_test_ext().execute_with(|| { + let max_rank = 9u16; + assert_ok!(Club::do_add_member_to_rank(69, max_rank / 2)); + assert_ok!(Club::do_add_member_to_rank(1337, max_rank)); + for i in 0..=max_rank { + if i <= max_rank / 2 { + assert_eq!(member_count(i), 2); + } else { + assert_eq!(member_count(i), 1); + } + } + assert_eq!(member_count(max_rank + 1), 0); + }) +} diff --git a/frame/paymaster/src/weights.rs b/frame/paymaster/src/weights.rs new file mode 100644 index 0000000000000..172c3ca85c1cf --- /dev/null +++ b/frame/paymaster/src/weights.rs @@ -0,0 +1,76 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Autogenerated weights for pallet_ranked_collective +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev +//! DATE: 2022-05-19, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 + +// Executed Command: +// /Users/gav/Core/substrate/target/release/substrate +// benchmark +// pallet +// --pallet +// pallet-ranked-collective +// --extrinsic=* +// --chain=dev +// --steps=50 +// --repeat=20 +// --output=../../../frame/ranked-collective/src/weights.rs +// --template=../../../.maintain/frame-weight-template.hbs +// --header=../../../HEADER-APACHE2 +// --record-proof + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] + +use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; +use sp_std::marker::PhantomData; + +/// Weight functions needed for pallet_ranked_collective. +pub trait WeightInfo { + fn add_member() -> Weight; +} + +/// Weights for pallet_ranked_collective using the Substrate node and recommended hardware. +pub struct SubstrateWeight(PhantomData); +impl WeightInfo for SubstrateWeight { + // Storage: RankedCollective Members (r:1 w:1) + // Storage: RankedCollective MemberCount (r:1 w:1) + // Storage: RankedCollective IndexToId (r:0 w:1) + // Storage: RankedCollective IdToIndex (r:0 w:1) + fn add_member() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(4 as u64)) + } +} + +// For backwards compatibility and tests +impl WeightInfo for () { + // Storage: RankedCollective Members (r:1 w:1) + // Storage: RankedCollective MemberCount (r:1 w:1) + // Storage: RankedCollective IndexToId (r:0 w:1) + // Storage: RankedCollective IdToIndex (r:0 w:1) + fn add_member() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(4 as u64)) + } +} diff --git a/frame/support/src/traits/preimages.rs b/frame/support/src/traits/preimages.rs index 594532ba96903..32ea38a2abcde 100644 --- a/frame/support/src/traits/preimages.rs +++ b/frame/support/src/traits/preimages.rs @@ -26,6 +26,58 @@ use sp_std::borrow::Cow; pub type Hash = H256; pub type BoundedInline = crate::BoundedVec>; +#[derive( + Encode, Decode, MaxEncodedLen, Clone, Eq, PartialEq, scale_info::TypeInfo, RuntimeDebug, +)] +#[codec(mel_bound())] +pub enum BoundedBlob { + /// A bounded blob. Its encoding must be at most 128 bytes. + Inline(BoundedInline), + /// A Blake2-256 hash of the blob together with an upper limit for its size. + Lookup { hash: Hash, len: u32 }, +} + +impl BoundedBlob { + /// Returns the hash of the preimage. + /// + /// The hash is re-calculated every time if the preimage is inlined. + pub fn hash(&self) -> H256 { + match self { + Self::Inline(x) => blake2_256(x.as_ref()).into(), + Self::Lookup { hash, .. } => *hash, + } + } + + /// Returns the length of the preimage. + pub fn len(&self) -> u32 { + match self { + Self::Inline(i) => Some(i.len() as u32), + Self::Lookup { len, .. } => Some(*len), + } + } + + /// Returns whether the image will require a lookup to be peeked. + pub fn lookup_needed(&self) -> bool { + match self { + Self::Inline(..) => false, + Self::Lookup { .. } => true, + } + } + + /// The maximum length of the lookup that is needed to peek `Self`. + pub fn lookup_len(&self) -> Option { + match self { + Self::Inline(..) => None, + Self::Lookup { len, .. } => Some(*len), + } + } + + /// Constructs a `Lookup` bounded item. + pub fn unrequested(hash: Hash, len: u32) -> Self { + Self::Lookup { hash, len } + } +} + #[derive( Encode, Decode, MaxEncodedLen, Clone, Eq, PartialEq, scale_info::TypeInfo, RuntimeDebug, )] @@ -35,12 +87,15 @@ pub enum Bounded { /// do not support creation of this except for transitioning from legacy state. /// In the future we will make this a pure `Dummy` item storing only the final `dummy` field. Legacy { hash: Hash, dummy: sp_std::marker::PhantomData }, - /// A an bounded `Call`. Its encoding must be at most 128 bytes. + /// A bounded `T`. Its encoding must be at most 128 bytes. Inline(BoundedInline), - /// A Blake2-256 hash of the call together with an upper limit for its size. + /// A Blake2-256 hash of the data together with an upper limit for its size. Lookup { hash: Hash, len: u32 }, } +// The maximum we expect a single legacy hash lookup to be. +const MAX_LEGACY_LEN: u32 = 1_000_000; + impl Bounded { /// Casts the wrapped type into something that encodes alike. /// @@ -75,12 +130,7 @@ impl Bounded { Lookup { hash, .. } => *hash, } } -} -// The maximum we expect a single legacy hash lookup to be. -const MAX_LEGACY_LEN: u32 = 1_000_000; - -impl Bounded { /// Returns the length of the preimage or `None` if the length is unknown. pub fn len(&self) -> Option { match self { @@ -204,6 +254,64 @@ pub trait QueryPreimage { Self::drop(bounded); Ok(r) } + + /// Request that the data required for decoding the given `bounded` blob is made available. + fn hold_blob(bounded: &BoundedBlob) { + use BoundedBlob::*; + match bounded { + Inline(..) => {}, + Lookup { hash, .. } => Self::request(hash), + } + } + + /// No longer request that the data required for decoding the given `bounded` blob is made + /// available. + fn drop_blob(bounded: &BoundedBlob) { + use BoundedBlob::*; + match bounded { + Inline(..) => {}, + Lookup { hash, .. } => Self::unrequest(hash), + } + } + + /// Check to see if all data required for the given `bounded` blob is available. + fn have_blob(bounded: &BoundedBlob) -> bool { + use BoundedBlob::*; + match bounded { + Inline(..) => true, + Lookup { hash, .. } => Self::len(hash).is_some(), + } + } + + /// Create a `BoundedBlob` based on the `hash` and `len` of the encoded value. This may not + /// be `peek`-able or `realize`-able. + fn pick_blob(hash: Hash, len: u32) -> BoundedBlob { + Self::request(&hash); + BoundedBlob::Lookup { hash, len } + } + + /// Convert the given `bounded` blob back into its original data. + /// + /// NOTE: This does not remove any data needed for realization. If you will no longer use the + /// `bounded`, call `realize` instead or call `drop` afterwards. + fn peek_blob(bounded: &BoundedBlob) -> Result, DispatchError> { + use BoundedBlob::*; + Ok(match bounded { + Inline(data) => data, + Lookup { hash, len } => { + Self::fetch(hash, Some(*len))? + }, + }) + } + + /// Convert the given `bounded` value back into its original data. If successful, + /// `drop` any data backing it. This will not break the realisability of independently + /// created instances of `Bounded` which happen to have identical data. + fn realize_blob(bounded: &BoundedBlob) -> Result, DispatchError> { + let r = Self::peek(bounded)?; + Self::drop(bounded); + Ok(r) + } } /// A interface for managing preimages to hashes on chain. From 958d34f35d63eee71e9f7fa8691cb74cca4f9070 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 13 Feb 2023 11:23:44 +0100 Subject: [PATCH 03/40] Fix build --- frame/paymaster/Cargo.toml | 10 +- frame/paymaster/src/lib.rs | 91 ++++++++------- frame/support/src/traits/preimages.rs | 152 +++----------------------- 3 files changed, 68 insertions(+), 185 deletions(-) diff --git a/frame/paymaster/Cargo.toml b/frame/paymaster/Cargo.toml index 4f02964bdafe9..08e45cbff6365 100644 --- a/frame/paymaster/Cargo.toml +++ b/frame/paymaster/Cargo.toml @@ -19,11 +19,11 @@ scale-info = { version = "2.0.1", default-features = false, features = ["derive" frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } -sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../primitives/arithmetic" } -sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } -sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } -sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } -sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } +sp-arithmetic = { version = "6.0.0", default-features = false, path = "../../primitives/arithmetic" } +sp-core = { version = "7.0.0", default-features = false, path = "../../primitives/core" } +sp-io = { version = "7.0.0", default-features = false, path = "../../primitives/io" } +sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" } +sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" } [features] default = ["std"] diff --git a/frame/paymaster/src/lib.rs b/frame/paymaster/src/lib.rs index 05de49b3435ca..97e60067d4072 100644 --- a/frame/paymaster/src/lib.rs +++ b/frame/paymaster/src/lib.rs @@ -20,23 +20,14 @@ #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "128"] -use codec::Codec; +use codec::{MaxEncodedLen, FullCodec}; use scale_info::TypeInfo; -use sp_arithmetic::traits::Saturating; -use sp_core::bounded::BoundedVec; -use sp_runtime::{ - traits::{AtLeast32BitUnsigned, Convert, StaticLookup}, - ArithmeticError::Overflow, - Perbill, RuntimeDebug, -}; -use sp_std::{marker::PhantomData, prelude::*}; +use sp_arithmetic::traits::{Zero, Saturating}; +use sp_runtime::traits::{AtLeast32BitUnsigned, Convert}; +use sp_std::{marker::PhantomData, fmt::Debug, prelude::*}; use frame_support::{ - codec::{Decode, Encode, MaxEncodedLen}, - dispatch::{DispatchError, DispatchResultWithPostInfo, PostDispatchInfo}, - ensure, - traits::{EnsureOrigin, PollStatus, Polling, RankedMembers, Time, VoteTally}, - CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, + dispatch::DispatchResultWithPostInfo, ensure, traits::RankedMembers, }; #[cfg(test)] @@ -56,11 +47,11 @@ pub type Cycle = u32; // XCM/MultiAsset and made generic over assets. pub trait Pay { /// The type by which we measure units of the currency in which we make payments. - type Balance: AtLeast32BitUnsigned + Codec + MaxEncodedLen; + type Balance: AtLeast32BitUnsigned + FullCodec + MaxEncodedLen + TypeInfo; /// The type by which we identify the individuals to whom a payment may be made. type AccountId; /// An identifier given to an individual payment. - type Id; + type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug; /// The amount of currency with which we have to make payments in this period. It may be a fixed /// value or reduce as calls to `pay` are made. It should be called once prior to the series of /// payments to determine the overall budget and then not again until the next series of @@ -74,7 +65,7 @@ pub trait Pay { #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{pallet_prelude::*, storage::KeyLenOf, weights::PaysFee}; + use frame_support::{pallet_prelude::*, dispatch::Pays}; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -98,23 +89,26 @@ pub mod pallet { type Members: RankedMembers::AccountId>; /// The maximum payout to be made for a single period to an active member of the given rank. - type ActiveSalaryForRank: Convert; + type ActiveSalaryForRank: Convert<::Rank, ::Balance>; /// The number of blocks between sequential payout cycles. #[pallet::constant] type CyclePeriod: Get; } - /// The next payout cycle to pay, and the block nunber at which it should be paid. + pub type CycleIndexOf = ::BlockNumber; + + /// The current payout cycle, the block nunber at which it started and the remaining balance in + /// this cycle's budget. #[pallet::storage] - pub(super) type LastPayout, I: 'static = ()> = - StorageValue<_, T::BlockNumber, OptionQuery>; + pub(super) type Status, I: 'static = ()> = + StorageValue<_, (CycleIndexOf, T::BlockNumber, ::Balance), OptionQuery>; /// The most recent cycle which was paid to a member. None implies that a member has not yet /// been paid. #[pallet::storage] pub(super) type LastClaim, I: 'static = ()> = - StorageMap<_, Twox64Concat, T::AccountId, T::BlockNumber, OptionQuery>; + StorageMap<_, Twox64Concat, T::AccountId, CycleIndexOf, OptionQuery>; #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -122,7 +116,7 @@ pub mod pallet { /// A member is inducted into the payroll. Inducted { who: T::AccountId }, /// A payment happened. - Paid { who: T::AccountId }, + Paid { who: T::AccountId, id: ::Id }, } #[pallet::error] @@ -133,54 +127,69 @@ pub mod pallet { NotInducted, /// The member does not have a current valid claim. NoClaim, + /// The member's claim is zero. + ClaimZero, /// Cycle is not yet over. NotYet, /// The payout cycles have not yet started. NotStarted, + /// There is no budget left for the payout. + Bankrupt, + /// There was some issue with the mechanism of payment. + PayError, } #[pallet::call] impl, I: 'static> Pallet { + /// Induct oneself into the payout system. #[pallet::weight(T::WeightInfo::add_member())] - pub fn induct(origin: OriginFor) -> DispatchResult { + #[pallet::call_index(0)] + pub fn induct(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let _ = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; - let last_payout = LastPayout::::get().unwrap_or_else(Zero::zero); - LastClaim::::put(who, last_payout); + let last_payout = Status::::get().map_or(Zero::zero(), |x| x.1); + LastClaim::::insert(&who, last_payout); Self::deposit_event(Event::::Inducted { who }); Ok(Pays::No.into()) } - // TODO: preregistration of activity for the next cycle. - /// Request a payout. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. #[pallet::weight(T::WeightInfo::add_member())] - pub fn payout(origin: OriginFor) -> DispatchResult { + #[pallet::call_index(1)] + pub fn payout(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - let _rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; - let last_claim = LastClaim::::get(who).ok_or(Error::::NotInducted)?; - let last_payout = LastPayout::::get().or_or(Error::::NotStarted)?; + let rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; + let payout = T::ActiveSalaryForRank::convert(rank); + ensure!(!payout.is_zero(), Error::::ClaimZero); + let last_claim = LastClaim::::get(&who).ok_or(Error::::NotInducted)?; + let (_, last_payout, budget) = Status::::get().ok_or(Error::::NotStarted)?; ensure!(last_claim < last_payout, Error::::NoClaim); - LastClaim::::put(who, last_payout); + ensure!(payout <= budget, Error::::Bankrupt); + + let id = T::Paymaster::pay(&who, payout).map_err(|()| Error::::PayError)?; - // TODO: make payout according to rank and activity. - let amount = Self::deposit_event(Event::::Paid { who }); + LastClaim::::insert(&who, last_payout); + Self::deposit_event(Event::::Paid { who, id }); Ok(Pays::No.into()) } - /// Move to next payout cycle, assuming that the present block + /// Move to next payout cycle, assuming that the present block is now within that cycle. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. #[pallet::weight(T::WeightInfo::add_member())] - pub fn next_cycle(origin: OriginFor) -> DispatchResult { + #[pallet::call_index(2)] + pub fn next_cycle(origin: OriginFor) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; let now = frame_system::Pallet::::block_number(); - let last_payout = LastPayout::::get().or_or(Error::::NoClaim)?; - let next_payout = last_payout.saturating_add(T::CyclePeriod::get()); - ensure!(now >= next_payout, Error::::NotYet); - LastPayout::::put(next_payout); + let (mut cycle_index, mut cycle_start, _) = Status::::get() + .ok_or(Error::::NoClaim)?; + cycle_start.saturating_accrue(T::CyclePeriod::get()); + ensure!(now >= cycle_start, Error::::NotYet); + cycle_index.saturating_inc(); + let budget = T::Paymaster::budget(); + Status::::put((cycle_index, cycle_start, budget)); Ok(Pays::No.into()) } } diff --git a/frame/support/src/traits/preimages.rs b/frame/support/src/traits/preimages.rs index c909da3881265..594532ba96903 100644 --- a/frame/support/src/traits/preimages.rs +++ b/frame/support/src/traits/preimages.rs @@ -26,61 +26,6 @@ use sp_std::borrow::Cow; pub type Hash = H256; pub type BoundedInline = crate::BoundedVec>; -/// The maximum we expect a single legacy hash lookup to be. -const MAX_LEGACY_LEN: u32 = 1_000_000; - -#[derive( - Encode, Decode, MaxEncodedLen, Clone, Eq, PartialEq, scale_info::TypeInfo, RuntimeDebug, -)] -#[codec(mel_bound())] -pub enum BoundedBlob { - /// A bounded blob. Its encoding must be at most 128 bytes. - Inline(BoundedInline), - /// A Blake2-256 hash of the blob together with an upper limit for its size. - Lookup { hash: Hash, len: u32 }, -} - -impl BoundedBlob { - /// Returns the hash of the preimage. - /// - /// The hash is re-calculated every time if the preimage is inlined. - pub fn hash(&self) -> H256 { - match self { - Self::Inline(x) => blake2_256(x.as_ref()).into(), - Self::Lookup { hash, .. } => *hash, - } - } - - /// Returns the length of the preimage. - pub fn len(&self) -> u32 { - match self { - Self::Inline(i) => Some(i.len() as u32), - Self::Lookup { len, .. } => Some(*len), - } - } - - /// Returns whether the image will require a lookup to be peeked. - pub fn lookup_needed(&self) -> bool { - match self { - Self::Inline(..) => false, - Self::Lookup { .. } => true, - } - } - - /// The maximum length of the lookup that is needed to peek `Self`. - pub fn lookup_len(&self) -> Option { - match self { - Self::Inline(..) => None, - Self::Lookup { len, .. } => Some(*len), - } - } - - /// Constructs a `Lookup` bounded item. - pub fn unrequested(hash: Hash, len: u32) -> Self { - Self::Lookup { hash, len } - } -} - #[derive( Encode, Decode, MaxEncodedLen, Clone, Eq, PartialEq, scale_info::TypeInfo, RuntimeDebug, )] @@ -90,15 +35,12 @@ pub enum Bounded { /// do not support creation of this except for transitioning from legacy state. /// In the future we will make this a pure `Dummy` item storing only the final `dummy` field. Legacy { hash: Hash, dummy: sp_std::marker::PhantomData }, - /// A bounded `T`. Its encoding must be at most 128 bytes. + /// A an bounded `Call`. Its encoding must be at most 128 bytes. Inline(BoundedInline), - /// A Blake2-256 hash of the data together with an upper limit for its size. + /// A Blake2-256 hash of the call together with an upper limit for its size. Lookup { hash: Hash, len: u32 }, } -// The maximum we expect a single legacy hash lookup to be. -const MAX_LEGACY_LEN: u32 = 1_000_000; - impl Bounded { /// Casts the wrapped type into something that encodes alike. /// @@ -125,25 +67,20 @@ impl Bounded { /// Returns the hash of the preimage. /// /// The hash is re-calculated every time if the preimage is inlined. - pub fn hash(&self) -> Hash { + pub fn hash(&self) -> H256 { use Bounded::*; match self { - Lookup { hash, .. } | Legacy { hash, .. } => *hash, + Legacy { hash, .. } => *hash, Inline(x) => blake2_256(x.as_ref()).into(), + Lookup { hash, .. } => *hash, } } +} - /// Returns the hash to lookup the preimage. - /// - /// If this is a `Bounded::Inline`, `None` is returned as no lookup is required. - pub fn lookup_hash(&self) -> Option { - use Bounded::*; - match self { - Lookup { hash, .. } | Legacy { hash, .. } => Some(*hash), - Inline(_) => None, - } - } +// The maximum we expect a single legacy hash lookup to be. +const MAX_LEGACY_LEN: u32 = 1_000_000; +impl Bounded { /// Returns the length of the preimage or `None` if the length is unknown. pub fn len(&self) -> Option { match self { @@ -231,11 +168,8 @@ pub trait QueryPreimage { } } - /// Create a `Bounded` instance based on the `hash` and `len` of the encoded value. - /// - /// It also directly requests the given `hash` using [`Self::request`]. - /// - /// This may not be `peek`-able or `realize`-able. + /// Create a `Bounded` instance based on the `hash` and `len` of the encoded value. This may not + /// be `peek`-able or `realize`-able. fn pick(hash: Hash, len: u32) -> Bounded { Self::request(&hash); Bounded::Lookup { hash, len } @@ -270,64 +204,6 @@ pub trait QueryPreimage { Self::drop(bounded); Ok(r) } - - /// Request that the data required for decoding the given `bounded` blob is made available. - fn hold_blob(bounded: &BoundedBlob) { - use BoundedBlob::*; - match bounded { - Inline(..) => {}, - Lookup { hash, .. } => Self::request(hash), - } - } - - /// No longer request that the data required for decoding the given `bounded` blob is made - /// available. - fn drop_blob(bounded: &BoundedBlob) { - use BoundedBlob::*; - match bounded { - Inline(..) => {}, - Lookup { hash, .. } => Self::unrequest(hash), - } - } - - /// Check to see if all data required for the given `bounded` blob is available. - fn have_blob(bounded: &BoundedBlob) -> bool { - use BoundedBlob::*; - match bounded { - Inline(..) => true, - Lookup { hash, .. } => Self::len(hash).is_some(), - } - } - - /// Create a `BoundedBlob` based on the `hash` and `len` of the encoded value. This may not - /// be `peek`-able or `realize`-able. - fn pick_blob(hash: Hash, len: u32) -> BoundedBlob { - Self::request(&hash); - BoundedBlob::Lookup { hash, len } - } - - /// Convert the given `bounded` blob back into its original data. - /// - /// NOTE: This does not remove any data needed for realization. If you will no longer use the - /// `bounded`, call `realize` instead or call `drop` afterwards. - fn peek_blob(bounded: &BoundedBlob) -> Result, DispatchError> { - use BoundedBlob::*; - Ok(match bounded { - Inline(data) => data, - Lookup { hash, len } => { - Self::fetch(hash, Some(*len))? - }, - }) - } - - /// Convert the given `bounded` value back into its original data. If successful, - /// `drop` any data backing it. This will not break the realisability of independently - /// created instances of `Bounded` which happen to have identical data. - fn realize_blob(bounded: &BoundedBlob) -> Result, DispatchError> { - let r = Self::peek(bounded)?; - Self::drop(bounded); - Ok(r) - } } /// A interface for managing preimages to hashes on chain. @@ -352,12 +228,10 @@ pub trait StorePreimage: QueryPreimage { Self::unrequest(hash) } - /// Convert an otherwise unbounded or large value into a type ready for placing in storage. - /// - /// The result is a type whose `MaxEncodedLen` is 131 bytes. + /// Convert an otherwise unbounded or large value into a type ready for placing in storage. The + /// result is a type whose `MaxEncodedLen` is 131 bytes. /// /// NOTE: Once this API is used, you should use either `drop` or `realize`. - /// The value is also noted using [`Self::note`]. fn bound(t: T) -> Result, DispatchError> { let data = t.encode(); let len = data.len() as u32; From 91cb1f58dd9ee13b87105e44975290b5180cea33 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 13 Feb 2023 14:43:16 +0000 Subject: [PATCH 04/40] More tests --- frame/paymaster/src/lib.rs | 99 ++++++-- frame/paymaster/src/tests.rs | 477 +++++++++-------------------------- 2 files changed, 193 insertions(+), 383 deletions(-) diff --git a/frame/paymaster/src/lib.rs b/frame/paymaster/src/lib.rs index 97e60067d4072..65f98aa6694d6 100644 --- a/frame/paymaster/src/lib.rs +++ b/frame/paymaster/src/lib.rs @@ -20,14 +20,14 @@ #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "128"] -use codec::{MaxEncodedLen, FullCodec}; +use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; -use sp_arithmetic::traits::{Zero, Saturating}; -use sp_runtime::traits::{AtLeast32BitUnsigned, Convert}; -use sp_std::{marker::PhantomData, fmt::Debug, prelude::*}; +use sp_arithmetic::traits::{Saturating, Zero}; +use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedSub, Convert}; +use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; use frame_support::{ - dispatch::DispatchResultWithPostInfo, ensure, traits::RankedMembers, + dispatch::DispatchResultWithPostInfo, ensure, traits::RankedMembers, RuntimeDebug, }; #[cfg(test)] @@ -47,7 +47,7 @@ pub type Cycle = u32; // XCM/MultiAsset and made generic over assets. pub trait Pay { /// The type by which we measure units of the currency in which we make payments. - type Balance: AtLeast32BitUnsigned + FullCodec + MaxEncodedLen + TypeInfo; + type Balance: AtLeast32BitUnsigned + FullCodec + MaxEncodedLen + TypeInfo + Debug; /// The type by which we identify the individuals to whom a payment may be made. type AccountId; /// An identifier given to an individual payment. @@ -62,10 +62,17 @@ pub trait Pay { fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result; } +#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub struct StatusType { + cycle_index: CycleIndex, + cycle_start: BlockNumber, + remaining_budget: Balance, +} + #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{pallet_prelude::*, dispatch::Pays}; + use frame_support::{dispatch::Pays, pallet_prelude::*}; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -89,7 +96,10 @@ pub mod pallet { type Members: RankedMembers::AccountId>; /// The maximum payout to be made for a single period to an active member of the given rank. - type ActiveSalaryForRank: Convert<::Rank, ::Balance>; + type ActiveSalaryForRank: Convert< + ::Rank, + ::Balance, + >; /// The number of blocks between sequential payout cycles. #[pallet::constant] @@ -97,15 +107,20 @@ pub mod pallet { } pub type CycleIndexOf = ::BlockNumber; + pub type StatusOf = StatusType< + CycleIndexOf, + ::BlockNumber, + <>::Paymaster as Pay>::Balance, + >; /// The current payout cycle, the block nunber at which it started and the remaining balance in /// this cycle's budget. #[pallet::storage] pub(super) type Status, I: 'static = ()> = - StorageValue<_, (CycleIndexOf, T::BlockNumber, ::Balance), OptionQuery>; + StorageValue<_, StatusOf, OptionQuery>; - /// The most recent cycle which was paid to a member. None implies that a member has not yet - /// been paid. + /// The most recent cycle index which was paid to a member. None implies that a member has not + /// yet been paid. #[pallet::storage] pub(super) type LastClaim, I: 'static = ()> = StorageMap<_, Twox64Concat, T::AccountId, CycleIndexOf, OptionQuery>; @@ -117,12 +132,16 @@ pub mod pallet { Inducted { who: T::AccountId }, /// A payment happened. Paid { who: T::AccountId, id: ::Id }, + /// The next cycle begins. + CycleStarted { index: CycleIndexOf, budget: ::Balance }, } #[pallet::error] pub enum Error { /// The account is not a ranked member. NotMember, + /// The account is already inducted. + AlreadyInducted, // The account is not yet inducted into the system. NotInducted, /// The member does not have a current valid claim. @@ -146,9 +165,12 @@ pub mod pallet { #[pallet::call_index(0)] pub fn induct(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; + let cycle_index = Status::::get().ok_or(Error::::NotStarted)?.cycle_index; let _ = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; - let last_payout = Status::::get().map_or(Zero::zero(), |x| x.1); - LastClaim::::insert(&who, last_payout); + ensure!(!LastClaim::::contains_key(&who), Error::::AlreadyInducted); + + LastClaim::::insert(&who, cycle_index); + Self::deposit_event(Event::::Inducted { who }); Ok(Pays::No.into()) } @@ -163,14 +185,16 @@ pub mod pallet { let rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; let payout = T::ActiveSalaryForRank::convert(rank); ensure!(!payout.is_zero(), Error::::ClaimZero); - let last_claim = LastClaim::::get(&who).ok_or(Error::::NotInducted)?; - let (_, last_payout, budget) = Status::::get().ok_or(Error::::NotStarted)?; - ensure!(last_claim < last_payout, Error::::NoClaim); - ensure!(payout <= budget, Error::::Bankrupt); + let last_claim = Self::last_claim(&who)?; + let mut status = Status::::get().ok_or(Error::::NotStarted)?; + ensure!(last_claim < status.cycle_index, Error::::NoClaim); + status.remaining_budget = + status.remaining_budget.checked_sub(&payout).ok_or(Error::::Bankrupt)?; let id = T::Paymaster::pay(&who, payout).map_err(|()| Error::::PayError)?; + LastClaim::::insert(&who, status.cycle_index); + Status::::put(&status); - LastClaim::::insert(&who, last_payout); Self::deposit_event(Event::::Paid { who, id }); Ok(Pays::No.into()) } @@ -183,16 +207,39 @@ pub mod pallet { pub fn next_cycle(origin: OriginFor) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; let now = frame_system::Pallet::::block_number(); - let (mut cycle_index, mut cycle_start, _) = Status::::get() - .ok_or(Error::::NoClaim)?; - cycle_start.saturating_accrue(T::CyclePeriod::get()); - ensure!(now >= cycle_start, Error::::NotYet); - cycle_index.saturating_inc(); - let budget = T::Paymaster::budget(); - Status::::put((cycle_index, cycle_start, budget)); + let mut status = match Status::::get() { + // Not first time... (move along start block and bump index) + Some(mut status) => { + status.cycle_start.saturating_accrue(T::CyclePeriod::get()); + ensure!(now >= status.cycle_start, Error::::NotYet); + status.cycle_index.saturating_inc(); + status + }, + // First time... (initialize) + None => StatusType { + cycle_index: Zero::zero(), + cycle_start: now, + remaining_budget: Zero::zero(), + }, + }; + status.remaining_budget = T::Paymaster::budget(); + + Status::::put(&status); + + Self::deposit_event(Event::::CycleStarted { + index: status.cycle_index, + budget: status.remaining_budget, + }); Ok(Pays::No.into()) } } - impl, I: 'static> Pallet {} + impl, I: 'static> Pallet { + pub fn status() -> Option> { + Status::::get() + } + pub fn last_claim(who: &T::AccountId) -> Result, DispatchError> { + LastClaim::::get(&who).ok_or(Error::::NotInducted.into()) + } + } } diff --git a/frame/paymaster/src/tests.rs b/frame/paymaster/src/tests.rs index 68bb79f3d07f7..3bf7dc92bc288 100644 --- a/frame/paymaster/src/tests.rs +++ b/frame/paymaster/src/tests.rs @@ -21,19 +21,19 @@ use std::collections::BTreeMap; use frame_support::{ assert_noop, assert_ok, - error::BadOrigin, pallet_prelude::Weight, parameter_types, - traits::{ConstU16, ConstU32, ConstU64, EitherOf, Everything, MapSuccess, Polling}, + traits::{ConstU32, ConstU64, Everything}, }; use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, Identity, IdentityLookup, ReduceBy}, + traits::{BlakeTwo256, Identity, IdentityLookup}, }; +use sp_std::cell::RefCell; use super::*; -use crate as pallet_ranked_collective; +use crate as pallet_paymaster; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -45,7 +45,7 @@ frame_support::construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event}, - Club: pallet_ranked_collective::{Pallet, Call, Storage, Event}, + Paymaster: pallet_paymaster::{Pallet, Call, Storage, Event}, } ); @@ -80,113 +80,62 @@ impl frame_system::Config for Test { type MaxConsumers = ConstU32<16>; } -#[derive(Clone, PartialEq, Eq, Debug)] -pub enum TestPollState { - Ongoing(TallyOf, Rank), - Completed(u64, bool), +thread_local! { + pub static PAID: RefCell> = RefCell::new(BTreeMap::new()); + pub static BUDGET: RefCell = RefCell::new(10u64); + pub static LAST_ID: RefCell = RefCell::new(0u64); } -use TestPollState::*; -parameter_types! { - pub static Polls: BTreeMap = vec![ - (1, Completed(1, true)), - (2, Completed(2, false)), - (3, Ongoing(Tally::from_parts(0, 0, 0), 1)), - ].into_iter().collect(); +fn paid(who: u64) -> u64 { + PAID.with(|p| p.borrow().get(&who).cloned().unwrap_or(0)) } -pub struct TestPolls; -impl Polling> for TestPolls { - type Index = u8; - type Votes = Votes; - type Moment = u64; - type Class = Rank; - fn classes() -> Vec { - vec![0, 1, 2] - } - fn as_ongoing(index: u8) -> Option<(TallyOf, Self::Class)> { - Polls::get().remove(&index).and_then(|x| { - if let TestPollState::Ongoing(t, c) = x { - Some((t, c)) - } else { - None - } - }) - } - fn access_poll( - index: Self::Index, - f: impl FnOnce(PollStatus<&mut TallyOf, Self::Moment, Self::Class>) -> R, - ) -> R { - let mut polls = Polls::get(); - let entry = polls.get_mut(&index); - let r = match entry { - Some(Ongoing(ref mut tally_mut_ref, class)) => - f(PollStatus::Ongoing(tally_mut_ref, *class)), - Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)), - None => f(PollStatus::None), - }; - Polls::set(polls); - r - } - fn try_access_poll( - index: Self::Index, - f: impl FnOnce( - PollStatus<&mut TallyOf, Self::Moment, Self::Class>, - ) -> Result, - ) -> Result { - let mut polls = Polls::get(); - let entry = polls.get_mut(&index); - let r = match entry { - Some(Ongoing(ref mut tally_mut_ref, class)) => - f(PollStatus::Ongoing(tally_mut_ref, *class)), - Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)), - None => f(PollStatus::None), - }?; - Polls::set(polls); - Ok(r) +pub struct TestPay; +impl Pay for TestPay { + type AccountId = u64; + type Balance = u64; + type Id = u64; + + fn budget() -> Self::Balance { + BUDGET.with(|b| *b.borrow()) } - #[cfg(feature = "runtime-benchmarks")] - fn create_ongoing(class: Self::Class) -> Result { - let mut polls = Polls::get(); - let i = polls.keys().rev().next().map_or(0, |x| x + 1); - polls.insert(i, Ongoing(Tally::new(class), class)); - Polls::set(polls); - Ok(i) + fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result { + PAID.with(|paid| *paid.borrow_mut().entry(*who).or_default() += amount); + Ok(LAST_ID.with(|lid| { + let x = *lid.borrow(); + lid.replace(x + 1); + x + })) } +} - #[cfg(feature = "runtime-benchmarks")] - fn end_ongoing(index: Self::Index, approved: bool) -> Result<(), ()> { - let mut polls = Polls::get(); - match polls.get(&index) { - Some(Ongoing(..)) => {}, - _ => return Err(()), - } - let now = frame_system::Pallet::::block_number(); - polls.insert(index, Completed(now, approved)); - Polls::set(polls); - Ok(()) +thread_local! { + pub static CLUB: RefCell> = RefCell::new(BTreeMap::new()); +} + +pub struct TestClub; +impl RankedMembers for TestClub { + type AccountId = u64; + type Rank = u64; + fn rank_of(who: &Self::AccountId) -> Option { + CLUB.with(|club| club.borrow().get(who).cloned()) + } + fn remove(who: &Self::AccountId) { + CLUB.with(|club| club.borrow_mut().remove(&who)); + } + fn change(who: &Self::AccountId, rank: Self::Rank) { + CLUB.with(|club| club.borrow_mut().insert(*who, rank)); } } impl Config for Test { type WeightInfo = (); type RuntimeEvent = RuntimeEvent; - type PromoteOrigin = EitherOf< - // Root can promote arbitrarily. - frame_system::EnsureRootWithSuccess>, - // Members can promote up to the rank of 2 below them. - MapSuccess, ReduceBy>>, - >; - type DemoteOrigin = EitherOf< - // Root can demote arbitrarily. - frame_system::EnsureRootWithSuccess>, - // Members can demote up to the rank of 3 below them. - MapSuccess, ReduceBy>>, - >; - type Polls = TestPolls; - type MinRankOfClass = Identity; - type VoteWeight = Geometric; + type Paymaster = TestPay; + type Members = TestClub; + type ActiveSalaryForRank = Identity; + type CyclePeriod = ConstU64<4>; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -200,10 +149,6 @@ fn next_block() { System::set_block_number(System::block_number() + 1); } -fn member_count(r: Rank) -> MemberIndex { - MemberCount::::get(r) -} - #[allow(dead_code)] fn run_to(n: u64) { while System::block_number() < n { @@ -211,296 +156,114 @@ fn run_to(n: u64) { } } -fn tally(index: u8) -> TallyOf { - >>::as_ongoing(index).expect("No poll").0 -} - -#[test] -#[ignore] -#[should_panic(expected = "No poll")] -fn unknown_poll_should_panic() { - let _ = tally(0); -} - -#[test] -#[ignore] -#[should_panic(expected = "No poll")] -fn completed_poll_should_panic() { - let _ = tally(1); -} - #[test] fn basic_stuff() { new_test_ext().execute_with(|| { - assert_eq!(tally(3), Tally::from_parts(0, 0, 0)); + assert!(Paymaster::last_claim(&0).is_err()); + assert_eq!(Paymaster::status(), None); }); } #[test] -fn member_lifecycle_works() { +fn can_start() { new_test_ext().execute_with(|| { - assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 0); - assert_eq!(member_count(1), 0); - }); -} - -#[test] -fn add_remove_works() { - new_test_ext().execute_with(|| { - assert_noop!(Club::add_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 1); - - assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 0); - - assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 1); - - assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); - assert_eq!(member_count(0), 2); - - assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); - assert_eq!(member_count(0), 3); - - assert_ok!(Club::demote_member(RuntimeOrigin::root(), 3)); - assert_eq!(member_count(0), 2); - - assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 1); - - assert_ok!(Club::demote_member(RuntimeOrigin::root(), 2)); - assert_eq!(member_count(0), 0); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_eq!( + Paymaster::status(), + Some(StatusType { cycle_index: 0, cycle_start: 1, remaining_budget: 10 }) + ); }); } #[test] -fn promote_demote_works() { +fn next_cycle_works() { new_test_ext().execute_with(|| { - assert_noop!(Club::add_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 1); - assert_eq!(member_count(1), 0); - - assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); - assert_eq!(member_count(0), 2); - assert_eq!(member_count(1), 0); - - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 2); - assert_eq!(member_count(1), 1); - - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); - assert_eq!(member_count(0), 2); - assert_eq!(member_count(1), 2); - - assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 2); - assert_eq!(member_count(1), 1); - - assert_noop!(Club::demote_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin); - assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1)); - assert_eq!(member_count(0), 1); - assert_eq!(member_count(1), 1); - }); -} + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + run_to(4); + assert_noop!(Paymaster::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); -#[test] -fn promote_demote_by_rank_works() { - new_test_ext().execute_with(|| { - assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); - for _ in 0..7 { - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); - } - - // #1 can add #2 and promote to rank 1 - assert_ok!(Club::add_member(RuntimeOrigin::signed(1), 2)); - assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); - // #2 as rank 1 cannot do anything privileged - assert_noop!(Club::add_member(RuntimeOrigin::signed(2), 3), BadOrigin); - - assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); - // #2 as rank 2 can add #3. - assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 3)); - - // #2 as rank 2 cannot promote #3 to rank 1 - assert_noop!( - Club::promote_member(RuntimeOrigin::signed(2), 3), - Error::::NoPermission + run_to(5); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_eq!( + Paymaster::status(), + Some(StatusType { cycle_index: 1, cycle_start: 5, remaining_budget: 10 }) ); - // #1 as rank 7 can promote #2 only up to rank 5 and once there cannot demote them. - assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); - assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); - assert_ok!(Club::promote_member(RuntimeOrigin::signed(1), 2)); - assert_noop!( - Club::promote_member(RuntimeOrigin::signed(1), 2), - Error::::NoPermission - ); - assert_noop!(Club::demote_member(RuntimeOrigin::signed(1), 2), Error::::NoPermission); - - // #2 as rank 5 can promote #3 only up to rank 3 and once there cannot demote them. - assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 3)); - assert_noop!( - Club::promote_member(RuntimeOrigin::signed(2), 3), - Error::::NoPermission - ); - assert_noop!(Club::demote_member(RuntimeOrigin::signed(2), 3), Error::::NoPermission); - - // #2 can add #4 & #5 as rank 0 and #6 & #7 as rank 1. - assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 4)); - assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 5)); - assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 6)); - assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 6)); - assert_ok!(Club::add_member(RuntimeOrigin::signed(2), 7)); - assert_ok!(Club::promote_member(RuntimeOrigin::signed(2), 7)); - - // #3 as rank 3 can demote/remove #4 & #5 but not #6 & #7 - assert_ok!(Club::demote_member(RuntimeOrigin::signed(3), 4)); - assert_ok!(Club::remove_member(RuntimeOrigin::signed(3), 5, 0)); - assert_noop!(Club::demote_member(RuntimeOrigin::signed(3), 6), Error::::NoPermission); - assert_noop!( - Club::remove_member(RuntimeOrigin::signed(3), 7, 1), - Error::::NoPermission - ); + run_to(8); + assert_noop!(Paymaster::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); - // #2 as rank 5 can demote/remove #6 & #7 - assert_ok!(Club::demote_member(RuntimeOrigin::signed(2), 6)); - assert_ok!(Club::remove_member(RuntimeOrigin::signed(2), 7, 1)); + BUDGET.with(|b| b.replace(5)); + run_to(9); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_eq!( + Paymaster::status(), + Some(StatusType { cycle_index: 2, cycle_start: 9, remaining_budget: 5 }) + ); }); } #[test] -fn voting_works() { +fn induct_works() { new_test_ext().execute_with(|| { - assert_ok!(Club::add_member(RuntimeOrigin::root(), 0)); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); - - assert_noop!(Club::vote(RuntimeOrigin::signed(0), 3, true), Error::::RankTooLow); - assert_eq!(tally(3), Tally::from_parts(0, 0, 0)); - - assert_ok!(Club::vote(RuntimeOrigin::signed(1), 3, true)); - assert_eq!(tally(3), Tally::from_parts(1, 1, 0)); - assert_ok!(Club::vote(RuntimeOrigin::signed(1), 3, false)); - assert_eq!(tally(3), Tally::from_parts(0, 0, 1)); - - assert_ok!(Club::vote(RuntimeOrigin::signed(2), 3, true)); - assert_eq!(tally(3), Tally::from_parts(1, 3, 1)); - assert_ok!(Club::vote(RuntimeOrigin::signed(2), 3, false)); - assert_eq!(tally(3), Tally::from_parts(0, 0, 4)); - - assert_ok!(Club::vote(RuntimeOrigin::signed(3), 3, true)); - assert_eq!(tally(3), Tally::from_parts(1, 6, 4)); - assert_ok!(Club::vote(RuntimeOrigin::signed(3), 3, false)); - assert_eq!(tally(3), Tally::from_parts(0, 0, 10)); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + + assert_noop!(Paymaster::induct(RuntimeOrigin::signed(1)), Error::::NotMember); + TestClub::change(&1, 1); + assert!(Paymaster::last_claim(&1).is_err()); + assert_ok!(Paymaster::induct(RuntimeOrigin::signed(1))); + assert_eq!(Paymaster::last_claim(&1).unwrap(), 0); + assert_noop!(Paymaster::induct(RuntimeOrigin::signed(1)), Error::::AlreadyInducted); }); } #[test] -fn cleanup_works() { +fn payment_works() { new_test_ext().execute_with(|| { - assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); - - assert_ok!(Club::vote(RuntimeOrigin::signed(1), 3, true)); - assert_ok!(Club::vote(RuntimeOrigin::signed(2), 3, false)); - assert_ok!(Club::vote(RuntimeOrigin::signed(3), 3, true)); - - assert_noop!(Club::cleanup_poll(RuntimeOrigin::signed(4), 3, 10), Error::::Ongoing); - Polls::set( - vec![(1, Completed(1, true)), (2, Completed(2, false)), (3, Completed(3, true))] - .into_iter() - .collect(), - ); - assert_ok!(Club::cleanup_poll(RuntimeOrigin::signed(4), 3, 10)); - // NOTE: This will fail until #10016 is merged. - // assert_noop!(Club::cleanup_poll(RuntimeOrigin::signed(4), 3, 10), - // Error::::NoneRemaining); + TestClub::change(&1, 1); + assert_noop!(Paymaster::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); + assert_ok!(Paymaster::induct(RuntimeOrigin::signed(1))); + // No claim on the cycle active during induction. + assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + + run_to(5); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Paymaster::payout(RuntimeOrigin::signed(1))); + assert_eq!(paid(1), 1); + assert_eq!(Paymaster::status().unwrap().remaining_budget, 9); + assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + run_to(8); + assert_noop!(Paymaster::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); + run_to(9); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Paymaster::payout(RuntimeOrigin::signed(1))); + assert_eq!(paid(1), 2); + assert_eq!(Paymaster::status().unwrap().remaining_budget, 9); }); } #[test] -fn ensure_ranked_works() { +fn zero_payment_fails() { new_test_ext().execute_with(|| { - assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); - assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); - assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); - - use frame_support::traits::OriginTrait; - type Rank1 = EnsureRanked; - type Rank2 = EnsureRanked; - type Rank3 = EnsureRanked; - type Rank4 = EnsureRanked; - assert_eq!(Rank1::try_origin(RuntimeOrigin::signed(1)).unwrap(), 1); - assert_eq!(Rank1::try_origin(RuntimeOrigin::signed(2)).unwrap(), 2); - assert_eq!(Rank1::try_origin(RuntimeOrigin::signed(3)).unwrap(), 3); - assert_eq!( - Rank2::try_origin(RuntimeOrigin::signed(1)).unwrap_err().as_signed().unwrap(), - 1 - ); - assert_eq!(Rank2::try_origin(RuntimeOrigin::signed(2)).unwrap(), 2); - assert_eq!(Rank2::try_origin(RuntimeOrigin::signed(3)).unwrap(), 3); - assert_eq!( - Rank3::try_origin(RuntimeOrigin::signed(1)).unwrap_err().as_signed().unwrap(), - 1 - ); - assert_eq!( - Rank3::try_origin(RuntimeOrigin::signed(2)).unwrap_err().as_signed().unwrap(), - 2 - ); - assert_eq!(Rank3::try_origin(RuntimeOrigin::signed(3)).unwrap(), 3); - assert_eq!( - Rank4::try_origin(RuntimeOrigin::signed(1)).unwrap_err().as_signed().unwrap(), - 1 - ); - assert_eq!( - Rank4::try_origin(RuntimeOrigin::signed(2)).unwrap_err().as_signed().unwrap(), - 2 - ); - assert_eq!( - Rank4::try_origin(RuntimeOrigin::signed(3)).unwrap_err().as_signed().unwrap(), - 3 - ); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + TestClub::change(&1, 0); + assert_ok!(Paymaster::induct(RuntimeOrigin::signed(1))); + run_to(5); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); }); } #[test] -fn do_add_member_to_rank_works() { +fn bankrupt_fails_gracefully() { new_test_ext().execute_with(|| { - let max_rank = 9u16; - assert_ok!(Club::do_add_member_to_rank(69, max_rank / 2)); - assert_ok!(Club::do_add_member_to_rank(1337, max_rank)); - for i in 0..=max_rank { - if i <= max_rank / 2 { - assert_eq!(member_count(i), 2); - } else { - assert_eq!(member_count(i), 1); - } - } - assert_eq!(member_count(max_rank + 1), 0); - }) + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + TestClub::change(&1, 11); + assert_ok!(Paymaster::induct(RuntimeOrigin::signed(1))); + run_to(5); + assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::Bankrupt); + assert_eq!(paid(1), 0); + }); } From c53d372435ad754508e3cd4dde247dc877623771 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 13 Feb 2023 14:46:01 +0000 Subject: [PATCH 05/40] Rename --- Cargo.lock | 2 +- frame/paymaster/README.md | 1 - frame/{paymaster => salary}/Cargo.toml | 2 +- frame/salary/README.md | 3 +++ frame/{paymaster => salary}/src/benchmarking.rs | 0 frame/{paymaster => salary}/src/lib.rs | 2 +- frame/{paymaster => salary}/src/tests.rs | 4 ++-- frame/{paymaster => salary}/src/weights.rs | 0 8 files changed, 8 insertions(+), 6 deletions(-) delete mode 100644 frame/paymaster/README.md rename frame/{paymaster => salary}/Cargo.toml (98%) create mode 100644 frame/salary/README.md rename frame/{paymaster => salary}/src/benchmarking.rs (100%) rename frame/{paymaster => salary}/src/lib.rs (99%) rename frame/{paymaster => salary}/src/tests.rs (98%) rename frame/{paymaster => salary}/src/weights.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index ce7d4a3a4fc09..da84dc55b4138 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6204,7 +6204,7 @@ dependencies = [ ] [[package]] -name = "pallet-paymaster" +name = "pallet-salary" version = "4.0.0-dev" dependencies = [ "frame-benchmarking", diff --git a/frame/paymaster/README.md b/frame/paymaster/README.md deleted file mode 100644 index 7270167b7eceb..0000000000000 --- a/frame/paymaster/README.md +++ /dev/null @@ -1 +0,0 @@ -# Paymaster \ No newline at end of file diff --git a/frame/paymaster/Cargo.toml b/frame/salary/Cargo.toml similarity index 98% rename from frame/paymaster/Cargo.toml rename to frame/salary/Cargo.toml index 08e45cbff6365..86cae16bb28b7 100644 --- a/frame/paymaster/Cargo.toml +++ b/frame/salary/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "pallet-paymaster" +name = "pallet-salary" version = "4.0.0-dev" authors = ["Parity Technologies "] edition = "2021" diff --git a/frame/salary/README.md b/frame/salary/README.md new file mode 100644 index 0000000000000..25c1be0e805d7 --- /dev/null +++ b/frame/salary/README.md @@ -0,0 +1,3 @@ +# Salary + +Make periodic payment to members of a ranked collective according to rank. \ No newline at end of file diff --git a/frame/paymaster/src/benchmarking.rs b/frame/salary/src/benchmarking.rs similarity index 100% rename from frame/paymaster/src/benchmarking.rs rename to frame/salary/src/benchmarking.rs diff --git a/frame/paymaster/src/lib.rs b/frame/salary/src/lib.rs similarity index 99% rename from frame/paymaster/src/lib.rs rename to frame/salary/src/lib.rs index 65f98aa6694d6..3fc393c1f799b 100644 --- a/frame/paymaster/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Proof-of-Personhood system. +//! Make periodic payment to members of a ranked collective according to rank. #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "128"] diff --git a/frame/paymaster/src/tests.rs b/frame/salary/src/tests.rs similarity index 98% rename from frame/paymaster/src/tests.rs rename to frame/salary/src/tests.rs index 3bf7dc92bc288..2e999bdd0f3dc 100644 --- a/frame/paymaster/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -33,7 +33,7 @@ use sp_runtime::{ use sp_std::cell::RefCell; use super::*; -use crate as pallet_paymaster; +use crate as pallet_salary; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -45,7 +45,7 @@ frame_support::construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event}, - Paymaster: pallet_paymaster::{Pallet, Call, Storage, Event}, + Paymaster: pallet_salary::{Pallet, Call, Storage, Event}, } ); diff --git a/frame/paymaster/src/weights.rs b/frame/salary/src/weights.rs similarity index 100% rename from frame/paymaster/src/weights.rs rename to frame/salary/src/weights.rs From 0806559e67386966d7ec49578fd7ec628848f981 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 13 Feb 2023 14:46:49 +0000 Subject: [PATCH 06/40] Rename --- frame/salary/src/tests.rs | 80 +++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 2e999bdd0f3dc..180ebc0c9ed60 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -45,7 +45,7 @@ frame_support::construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event}, - Paymaster: pallet_salary::{Pallet, Call, Storage, Event}, + Salary: pallet_salary::{Pallet, Call, Storage, Event}, } ); @@ -132,7 +132,7 @@ impl RankedMembers for TestClub { impl Config for Test { type WeightInfo = (); type RuntimeEvent = RuntimeEvent; - type Paymaster = TestPay; + type Salary = TestPay; type Members = TestClub; type ActiveSalaryForRank = Identity; type CyclePeriod = ConstU64<4>; @@ -159,17 +159,17 @@ fn run_to(n: u64) { #[test] fn basic_stuff() { new_test_ext().execute_with(|| { - assert!(Paymaster::last_claim(&0).is_err()); - assert_eq!(Paymaster::status(), None); + assert!(Salary::last_claim(&0).is_err()); + assert_eq!(Salary::status(), None); }); } #[test] fn can_start() { new_test_ext().execute_with(|| { - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); assert_eq!( - Paymaster::status(), + Salary::status(), Some(StatusType { cycle_index: 0, cycle_start: 1, remaining_budget: 10 }) ); }); @@ -178,25 +178,25 @@ fn can_start() { #[test] fn next_cycle_works() { new_test_ext().execute_with(|| { - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); run_to(4); - assert_noop!(Paymaster::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); + assert_noop!(Salary::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); run_to(5); - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); assert_eq!( - Paymaster::status(), + Salary::status(), Some(StatusType { cycle_index: 1, cycle_start: 5, remaining_budget: 10 }) ); run_to(8); - assert_noop!(Paymaster::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); + assert_noop!(Salary::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); BUDGET.with(|b| b.replace(5)); run_to(9); - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); assert_eq!( - Paymaster::status(), + Salary::status(), Some(StatusType { cycle_index: 2, cycle_start: 9, remaining_budget: 5 }) ); }); @@ -205,14 +205,14 @@ fn next_cycle_works() { #[test] fn induct_works() { new_test_ext().execute_with(|| { - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); - assert_noop!(Paymaster::induct(RuntimeOrigin::signed(1)), Error::::NotMember); + assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotMember); TestClub::change(&1, 1); - assert!(Paymaster::last_claim(&1).is_err()); - assert_ok!(Paymaster::induct(RuntimeOrigin::signed(1))); - assert_eq!(Paymaster::last_claim(&1).unwrap(), 0); - assert_noop!(Paymaster::induct(RuntimeOrigin::signed(1)), Error::::AlreadyInducted); + assert!(Salary::last_claim(&1).is_err()); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + assert_eq!(Salary::last_claim(&1).unwrap(), 0); + assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::AlreadyInducted); }); } @@ -220,50 +220,50 @@ fn induct_works() { fn payment_works() { new_test_ext().execute_with(|| { TestClub::change(&1, 1); - assert_noop!(Paymaster::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); - assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); - assert_ok!(Paymaster::induct(RuntimeOrigin::signed(1))); + assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); // No claim on the cycle active during induction. - assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(5); - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); - assert_ok!(Paymaster::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 1); - assert_eq!(Paymaster::status().unwrap().remaining_budget, 9); - assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + assert_eq!(Salary::status().unwrap().remaining_budget, 9); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(8); - assert_noop!(Paymaster::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); + assert_noop!(Salary::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); run_to(9); - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); - assert_ok!(Paymaster::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 2); - assert_eq!(Paymaster::status().unwrap().remaining_budget, 9); + assert_eq!(Salary::status().unwrap().remaining_budget, 9); }); } #[test] fn zero_payment_fails() { new_test_ext().execute_with(|| { - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); TestClub::change(&1, 0); - assert_ok!(Paymaster::induct(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(5); - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); - assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); }); } #[test] fn bankrupt_fails_gracefully() { new_test_ext().execute_with(|| { - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); TestClub::change(&1, 11); - assert_ok!(Paymaster::induct(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(5); - assert_ok!(Paymaster::next_cycle(RuntimeOrigin::signed(1))); - assert_noop!(Paymaster::payout(RuntimeOrigin::signed(1)), Error::::Bankrupt); + assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::Bankrupt); assert_eq!(paid(1), 0); }); } From 5cadabf1446a44b4c0aefe370cdbc6f4438ed6f9 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 13 Feb 2023 14:52:36 +0000 Subject: [PATCH 07/40] Renaming --- Cargo.lock | 34 +++++++++++++++++----------------- Cargo.toml | 2 +- frame/salary/src/tests.rs | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index da84dc55b4138..1e2e9ccfe7c5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6203,23 +6203,6 @@ dependencies = [ "sp-std", ] -[[package]] -name = "pallet-salary" -version = "4.0.0-dev" -dependencies = [ - "frame-benchmarking", - "frame-support", - "frame-system", - "log", - "parity-scale-codec", - "scale-info", - "sp-arithmetic", - "sp-core", - "sp-io", - "sp-runtime", - "sp-std", -] - [[package]] name = "pallet-preimage" version = "4.0.0-dev" @@ -6360,6 +6343,23 @@ dependencies = [ "sp-std", ] +[[package]] +name = "pallet-salary" +version = "4.0.0-dev" +dependencies = [ + "frame-benchmarking", + "frame-support", + "frame-system", + "log", + "parity-scale-codec", + "scale-info", + "sp-arithmetic", + "sp-core", + "sp-io", + "sp-runtime", + "sp-std", +] + [[package]] name = "pallet-scheduler" version = "4.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index 377a935e22430..4fa419638b0aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -119,7 +119,6 @@ members = [ "frame/node-authorization", "frame/offences", "frame/offences/benchmarking", - "frame/paymaster", "frame/preimage", "frame/proxy", "frame/message-queue", @@ -134,6 +133,7 @@ members = [ "frame/recovery", "frame/referenda", "frame/remark", + "frame/salary", "frame/scheduler", "frame/scored-pool", "frame/session", diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 180ebc0c9ed60..812a384fff815 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -132,7 +132,7 @@ impl RankedMembers for TestClub { impl Config for Test { type WeightInfo = (); type RuntimeEvent = RuntimeEvent; - type Salary = TestPay; + type Paymaster = TestPay; type Members = TestClub; type ActiveSalaryForRank = Identity; type CyclePeriod = ConstU64<4>; From 712118bc2e3367966029a0111370ece15a71cde0 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 13 Feb 2023 14:54:09 +0000 Subject: [PATCH 08/40] Revert old changes --- frame/support/src/traits/preimages.rs | 35 ++++++++++++++++++--------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/frame/support/src/traits/preimages.rs b/frame/support/src/traits/preimages.rs index 594532ba96903..ce3537c792c08 100644 --- a/frame/support/src/traits/preimages.rs +++ b/frame/support/src/traits/preimages.rs @@ -26,6 +26,9 @@ use sp_std::borrow::Cow; pub type Hash = H256; pub type BoundedInline = crate::BoundedVec>; +/// The maximum we expect a single legacy hash lookup to be. +const MAX_LEGACY_LEN: u32 = 1_000_000; + #[derive( Encode, Decode, MaxEncodedLen, Clone, Eq, PartialEq, scale_info::TypeInfo, RuntimeDebug, )] @@ -67,20 +70,25 @@ impl Bounded { /// Returns the hash of the preimage. /// /// The hash is re-calculated every time if the preimage is inlined. - pub fn hash(&self) -> H256 { + pub fn hash(&self) -> Hash { use Bounded::*; match self { - Legacy { hash, .. } => *hash, + Lookup { hash, .. } | Legacy { hash, .. } => *hash, Inline(x) => blake2_256(x.as_ref()).into(), - Lookup { hash, .. } => *hash, } } -} -// The maximum we expect a single legacy hash lookup to be. -const MAX_LEGACY_LEN: u32 = 1_000_000; + /// Returns the hash to lookup the preimage. + /// + /// If this is a `Bounded::Inline`, `None` is returned as no lookup is required. + pub fn lookup_hash(&self) -> Option { + use Bounded::*; + match self { + Lookup { hash, .. } | Legacy { hash, .. } => Some(*hash), + Inline(_) => None, + } + } -impl Bounded { /// Returns the length of the preimage or `None` if the length is unknown. pub fn len(&self) -> Option { match self { @@ -168,8 +176,11 @@ pub trait QueryPreimage { } } - /// Create a `Bounded` instance based on the `hash` and `len` of the encoded value. This may not - /// be `peek`-able or `realize`-able. + /// Create a `Bounded` instance based on the `hash` and `len` of the encoded value. + /// + /// It also directly requests the given `hash` using [`Self::request`]. + /// + /// This may not be `peek`-able or `realize`-able. fn pick(hash: Hash, len: u32) -> Bounded { Self::request(&hash); Bounded::Lookup { hash, len } @@ -228,10 +239,12 @@ pub trait StorePreimage: QueryPreimage { Self::unrequest(hash) } - /// Convert an otherwise unbounded or large value into a type ready for placing in storage. The - /// result is a type whose `MaxEncodedLen` is 131 bytes. + /// Convert an otherwise unbounded or large value into a type ready for placing in storage. + /// + /// The result is a type whose `MaxEncodedLen` is 131 bytes. /// /// NOTE: Once this API is used, you should use either `drop` or `realize`. + /// The value is also noted using [`Self::note`]. fn bound(t: T) -> Result, DispatchError> { let data = t.encode(); let len = data.len() as u32; From 07b785c4c99f17071d815d37cf9ca1489d326860 Mon Sep 17 00:00:00 2001 From: Gav Date: Wed, 15 Feb 2023 13:20:42 +0000 Subject: [PATCH 09/40] Multi-phase payouts to avoid bank-runs --- .gitignore | 1 + frame/salary/src/lib.rs | 228 ++++++++++++++++++++++++++++---------- frame/salary/src/tests.rs | 52 ++++----- 3 files changed, 196 insertions(+), 85 deletions(-) diff --git a/.gitignore b/.gitignore index 5cd013e054e4f..efb63520e1e1b 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,4 @@ rls*.log *.bin *.iml scripts/ci/node-template-release/Cargo.lock +substrate.code-workspace diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 3fc393c1f799b..aafe237ce0039 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -23,11 +23,17 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; use sp_arithmetic::traits::{Saturating, Zero}; -use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedSub, Convert}; +use sp_runtime::{ + traits::{AtLeast32BitUnsigned, CheckedSub, Convert}, + Perbill, +}; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; use frame_support::{ - dispatch::DispatchResultWithPostInfo, ensure, traits::RankedMembers, RuntimeDebug, + dispatch::DispatchResultWithPostInfo, + ensure, + traits::{schedule::v3::Anon as ScheduleAnon, RankedMembers}, + RuntimeDebug, }; #[cfg(test)] @@ -47,16 +53,11 @@ pub type Cycle = u32; // XCM/MultiAsset and made generic over assets. pub trait Pay { /// The type by which we measure units of the currency in which we make payments. - type Balance: AtLeast32BitUnsigned + FullCodec + MaxEncodedLen + TypeInfo + Debug; + type Balance: AtLeast32BitUnsigned + FullCodec + MaxEncodedLen + TypeInfo + Debug + Copy; /// The type by which we identify the individuals to whom a payment may be made. type AccountId; /// An identifier given to an individual payment. type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug; - /// The amount of currency with which we have to make payments in this period. It may be a fixed - /// value or reduce as calls to `pay` are made. It should be called once prior to the series of - /// payments to determine the overall budget and then not again until the next series of - /// payments are to be made. - fn budget() -> Self::Balance; /// Make a payment and return an identifier for later evaluation of success in some off-chain /// mechanism (likely an event, but possibly not on this chain). fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result; @@ -66,7 +67,17 @@ pub trait Pay { pub struct StatusType { cycle_index: CycleIndex, cycle_start: BlockNumber, - remaining_budget: Balance, + budget: Balance, + total_registrations: Balance, + total_unregistered_paid: Balance, +} + +#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub struct ClaimantStatus { + /// The most recent cycle in which the claimant was active. + last_active: CycleIndex, + /// The amount reserved in `last_active` cycle, or `None` if paid. + unpaid: Option, } #[frame_support::pallet] @@ -101,39 +112,55 @@ pub mod pallet { ::Balance, >; - /// The number of blocks between sequential payout cycles. + /// The number of block within a cycle which accounts have to register their intent to + /// claim. + /// + /// The number of blocks between sequential payout cycles is the sum of this and + /// `PayoutPeriod`. + #[pallet::constant] + type RegistrationPeriod: Get; + + /// The number of block within a cycle which accounts have to claim the payout. + /// + /// The number of blocks between sequential payout cycles is the sum of this and + /// `RegistrationPeriod`. + #[pallet::constant] + type PayoutPeriod: Get; + + /// The total budget per cycle. + /// + /// This may change over the course of a cycle without any problem. #[pallet::constant] - type CyclePeriod: Get; + type Budget: Get>; } pub type CycleIndexOf = ::BlockNumber; - pub type StatusOf = StatusType< - CycleIndexOf, - ::BlockNumber, - <>::Paymaster as Pay>::Balance, - >; - - /// The current payout cycle, the block nunber at which it started and the remaining balance in - /// this cycle's budget. + pub type BalanceOf = <>::Paymaster as Pay>::Balance; + pub type StatusOf = + StatusType, ::BlockNumber, BalanceOf>; + pub type ClaimantStatusOf = ClaimantStatus, BalanceOf>; + + /// The overall status of the system. #[pallet::storage] pub(super) type Status, I: 'static = ()> = StorageValue<_, StatusOf, OptionQuery>; - /// The most recent cycle index which was paid to a member. None implies that a member has not - /// yet been paid. + /// The status of a claimant. #[pallet::storage] - pub(super) type LastClaim, I: 'static = ()> = - StorageMap<_, Twox64Concat, T::AccountId, CycleIndexOf, OptionQuery>; + pub(super) type Claimant, I: 'static = ()> = + StorageMap<_, Twox64Concat, T::AccountId, ClaimantStatusOf, OptionQuery>; #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { /// A member is inducted into the payroll. Inducted { who: T::AccountId }, + /// The next cycle begins. + Registered { who: T::AccountId, amount: BalanceOf }, /// A payment happened. - Paid { who: T::AccountId, id: ::Id }, + Paid { who: T::AccountId, amount: BalanceOf, id: ::Id }, /// The next cycle begins. - CycleStarted { index: CycleIndexOf, budget: ::Balance }, + CycleStarted { index: CycleIndexOf }, } #[pallet::error] @@ -148,6 +175,10 @@ pub mod pallet { NoClaim, /// The member's claim is zero. ClaimZero, + /// Current cycle's registration period is over. + TooLate, + /// Current cycle's payment period is not yet begun. + TooEarly, /// Cycle is not yet over. NotYet, /// The payout cycles have not yet started. @@ -167,79 +198,156 @@ pub mod pallet { let who = ensure_signed(origin)?; let cycle_index = Status::::get().ok_or(Error::::NotStarted)?.cycle_index; let _ = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; - ensure!(!LastClaim::::contains_key(&who), Error::::AlreadyInducted); + ensure!(!Claimant::::contains_key(&who), Error::::AlreadyInducted); - LastClaim::::insert(&who, cycle_index); + Claimant::::insert( + &who, + ClaimantStatus { last_active: cycle_index, unpaid: None }, + ); Self::deposit_event(Event::::Inducted { who }); Ok(Pays::No.into()) } - /// Request a payout. + /// Move to next payout cycle, assuming that the present block is now within that cycle. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. #[pallet::weight(T::WeightInfo::add_member())] #[pallet::call_index(1)] - pub fn payout(origin: OriginFor) -> DispatchResultWithPostInfo { + pub fn bump(origin: OriginFor) -> DispatchResultWithPostInfo { + let _ = ensure_signed(origin)?; + let now = frame_system::Pallet::::block_number(); + let cycle_period = T::RegistrationPeriod::get() + T::PayoutPeriod::get(); + let mut status = match Status::::get() { + // Not first time... (move along start block and bump index) + Some(mut status) => { + status.cycle_start.saturating_accrue(cycle_period); + ensure!(now >= status.cycle_start, Error::::NotYet); + status.cycle_index.saturating_inc(); + status.budget = T::Budget::get(); + status.total_registrations = Zero::zero(); + status.total_unregistered_paid = Zero::zero(); + status + }, + // First time... (initialize) + None => StatusType { + cycle_index: Zero::zero(), + cycle_start: now, + budget: T::Budget::get(), + total_registrations: Zero::zero(), + total_unregistered_paid: Zero::zero(), + }, + }; + Status::::put(&status); + + Self::deposit_event(Event::::CycleStarted { index: status.cycle_index }); + Ok(Pays::No.into()) + } + + /// Register for a payout. + /// + /// Will only work if we are in the first `RegistrationPeriod` blocks since the cycle + /// started. + /// + /// - `origin`: A `Signed` origin of an account which is a member of `Members`. + #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::call_index(2)] + pub fn register(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; + let mut status = Status::::get().ok_or(Error::::NotStarted)?; + let mut claimant = Claimant::::get(&who).ok_or(Error::::NotInducted)?; + let now = frame_system::Pallet::::block_number(); + ensure!( + now < status.cycle_start + T::RegistrationPeriod::get(), + Error::::TooLate + ); + ensure!(claimant.last_active < status.cycle_index, Error::::NoClaim); let payout = T::ActiveSalaryForRank::convert(rank); ensure!(!payout.is_zero(), Error::::ClaimZero); - let last_claim = Self::last_claim(&who)?; - let mut status = Status::::get().ok_or(Error::::NotStarted)?; - ensure!(last_claim < status.cycle_index, Error::::NoClaim); - status.remaining_budget = - status.remaining_budget.checked_sub(&payout).ok_or(Error::::Bankrupt)?; + claimant.last_active = status.cycle_index; + claimant.unpaid = Some(payout); + status.total_registrations.saturating_accrue(payout); let id = T::Paymaster::pay(&who, payout).map_err(|()| Error::::PayError)?; - LastClaim::::insert(&who, status.cycle_index); + Claimant::::insert(&who, &claimant); Status::::put(&status); - Self::deposit_event(Event::::Paid { who, id }); + Self::deposit_event(Event::::Registered { who, amount: payout }); Ok(Pays::No.into()) } - /// Move to next payout cycle, assuming that the present block is now within that cycle. + /// Request a payout. + /// + /// Will only work if we are after the first `RegistrationPeriod` blocks since the cycle + /// started but by no more than `PayoutPeriod` blocks. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. #[pallet::weight(T::WeightInfo::add_member())] - #[pallet::call_index(2)] - pub fn next_cycle(origin: OriginFor) -> DispatchResultWithPostInfo { - let _ = ensure_signed(origin)?; + #[pallet::call_index(3)] + pub fn payout(origin: OriginFor) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin)?; + + let mut status = Status::::get().ok_or(Error::::NotStarted)?; + let mut claimant = Claimant::::get(&who).ok_or(Error::::NotInducted)?; + let now = frame_system::Pallet::::block_number(); - let mut status = match Status::::get() { - // Not first time... (move along start block and bump index) - Some(mut status) => { - status.cycle_start.saturating_accrue(T::CyclePeriod::get()); - ensure!(now >= status.cycle_start, Error::::NotYet); - status.cycle_index.saturating_inc(); - status - }, - // First time... (initialize) - None => StatusType { - cycle_index: Zero::zero(), - cycle_start: now, - remaining_budget: Zero::zero(), - }, + ensure!( + now >= status.cycle_start + T::RegistrationPeriod::get(), + Error::::TooEarly + ); + + let payout = if let Some(unpaid) = claimant.unpaid { + ensure!(claimant.last_active == status.cycle_index, Error::::NoClaim); + + // Registered for this cycle. Pay accordingly. + if status.total_registrations <= status.budget { + // Can pay in full. + unpaid + } else { + // Must be reduced pro-rata + Perbill::from_rational(status.budget, status.total_registrations) * unpaid + } + } else { + ensure!(claimant.last_active < status.cycle_index, Error::::NoClaim); + + // Not registered for this cycle. Pay from whatever is left. + let rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; + let ideal_payout = T::ActiveSalaryForRank::convert(rank); + + let pot = status + .budget + .saturating_sub(status.total_registrations) + .saturating_sub(status.total_unregistered_paid); + + let payout = ideal_payout.min(pot); + ensure!(!payout.is_zero(), Error::::ClaimZero); + + status.total_unregistered_paid.saturating_reduce(payout); + payout }; - status.remaining_budget = T::Paymaster::budget(); + claimant.unpaid = None; + claimant.last_active = status.cycle_index; + + let id = T::Paymaster::pay(&who, payout).map_err(|()| Error::::PayError)?; + + Claimant::::insert(&who, &claimant); Status::::put(&status); - Self::deposit_event(Event::::CycleStarted { - index: status.cycle_index, - budget: status.remaining_budget, - }); + Self::deposit_event(Event::::Paid { who, amount: payout, id }); Ok(Pays::No.into()) } } impl, I: 'static> Pallet { + /* pub fn status() -> Option> { Status::::get() } pub fn last_claim(who: &T::AccountId) -> Result, DispatchError> { - LastClaim::::get(&who).ok_or(Error::::NotInducted.into()) + Claimant::::get(&who).ok_or(Error::::NotInducted.into()) } + */ } } diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 812a384fff815..db78c17fe7751 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -82,7 +82,6 @@ impl frame_system::Config for Test { thread_local! { pub static PAID: RefCell> = RefCell::new(BTreeMap::new()); - pub static BUDGET: RefCell = RefCell::new(10u64); pub static LAST_ID: RefCell = RefCell::new(0u64); } @@ -96,10 +95,6 @@ impl Pay for TestPay { type Balance = u64; type Id = u64; - fn budget() -> Self::Balance { - BUDGET.with(|b| *b.borrow()) - } - fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result { PAID.with(|paid| *paid.borrow_mut().entry(*who).or_default() += amount); Ok(LAST_ID.with(|lid| { @@ -129,13 +124,19 @@ impl RankedMembers for TestClub { } } +parameter_types! { + pub static Budget: u64 = 10; +} + impl Config for Test { type WeightInfo = (); type RuntimeEvent = RuntimeEvent; type Paymaster = TestPay; type Members = TestClub; type ActiveSalaryForRank = Identity; - type CyclePeriod = ConstU64<4>; + type RegistrationPeriod = ConstU64<2>; + type PayoutPeriod = ConstU64<2>; + type Budget = Budget; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -159,15 +160,15 @@ fn run_to(n: u64) { #[test] fn basic_stuff() { new_test_ext().execute_with(|| { - assert!(Salary::last_claim(&0).is_err()); - assert_eq!(Salary::status(), None); + // assert!(Salary::last_claim(&0).is_err()); + // assert_eq!(Salary::status(), None); }); } - +/* #[test] fn can_start() { new_test_ext().execute_with(|| { - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_eq!( Salary::status(), Some(StatusType { cycle_index: 0, cycle_start: 1, remaining_budget: 10 }) @@ -176,25 +177,25 @@ fn can_start() { } #[test] -fn next_cycle_works() { +fn bump_works() { new_test_ext().execute_with(|| { - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); run_to(4); - assert_noop!(Salary::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); + assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); run_to(5); - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_eq!( Salary::status(), Some(StatusType { cycle_index: 1, cycle_start: 5, remaining_budget: 10 }) ); run_to(8); - assert_noop!(Salary::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); + assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); BUDGET.with(|b| b.replace(5)); run_to(9); - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_eq!( Salary::status(), Some(StatusType { cycle_index: 2, cycle_start: 9, remaining_budget: 5 }) @@ -205,7 +206,7 @@ fn next_cycle_works() { #[test] fn induct_works() { new_test_ext().execute_with(|| { - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotMember); TestClub::change(&1, 1); @@ -221,22 +222,22 @@ fn payment_works() { new_test_ext().execute_with(|| { TestClub::change(&1, 1); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); // No claim on the cycle active during induction. assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(5); - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 1); assert_eq!(Salary::status().unwrap().remaining_budget, 9); assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(8); - assert_noop!(Salary::next_cycle(RuntimeOrigin::signed(1)), Error::::NotYet); + assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); run_to(9); - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 2); assert_eq!(Salary::status().unwrap().remaining_budget, 9); @@ -246,11 +247,11 @@ fn payment_works() { #[test] fn zero_payment_fails() { new_test_ext().execute_with(|| { - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); TestClub::change(&1, 0); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(5); - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); }); } @@ -258,12 +259,13 @@ fn zero_payment_fails() { #[test] fn bankrupt_fails_gracefully() { new_test_ext().execute_with(|| { - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); TestClub::change(&1, 11); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(5); - assert_ok!(Salary::next_cycle(RuntimeOrigin::signed(1))); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::Bankrupt); assert_eq!(paid(1), 0); }); } +*/ From 7fb95d0b5496d47c496623dbb6708cedbfe9ca6e Mon Sep 17 00:00:00 2001 From: Gav Date: Wed, 15 Feb 2023 13:45:17 +0000 Subject: [PATCH 10/40] Tests --- frame/salary/src/lib.rs | 18 +++++---------- frame/salary/src/tests.rs | 47 +++++++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index aafe237ce0039..55fd8075bf1b3 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -24,16 +24,13 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; use sp_arithmetic::traits::{Saturating, Zero}; use sp_runtime::{ - traits::{AtLeast32BitUnsigned, CheckedSub, Convert}, + traits::{AtLeast32BitUnsigned, Convert}, Perbill, }; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; use frame_support::{ - dispatch::DispatchResultWithPostInfo, - ensure, - traits::{schedule::v3::Anon as ScheduleAnon, RankedMembers}, - RuntimeDebug, + dispatch::DispatchResultWithPostInfo, ensure, traits::RankedMembers, RuntimeDebug, }; #[cfg(test)] @@ -218,7 +215,7 @@ pub mod pallet { let _ = ensure_signed(origin)?; let now = frame_system::Pallet::::block_number(); let cycle_period = T::RegistrationPeriod::get() + T::PayoutPeriod::get(); - let mut status = match Status::::get() { + let status = match Status::::get() { // Not first time... (move along start block and bump index) Some(mut status) => { status.cycle_start.saturating_accrue(cycle_period); @@ -269,7 +266,6 @@ pub mod pallet { claimant.unpaid = Some(payout); status.total_registrations.saturating_accrue(payout); - let id = T::Paymaster::pay(&who, payout).map_err(|()| Error::::PayError)?; Claimant::::insert(&who, &claimant); Status::::put(&status); @@ -323,7 +319,7 @@ pub mod pallet { let payout = ideal_payout.min(pot); ensure!(!payout.is_zero(), Error::::ClaimZero); - status.total_unregistered_paid.saturating_reduce(payout); + status.total_unregistered_paid.saturating_accrue(payout); payout }; @@ -341,13 +337,11 @@ pub mod pallet { } impl, I: 'static> Pallet { - /* pub fn status() -> Option> { Status::::get() } - pub fn last_claim(who: &T::AccountId) -> Result, DispatchError> { - Claimant::::get(&who).ok_or(Error::::NotInducted.into()) + pub fn last_active(who: &T::AccountId) -> Result, DispatchError> { + Ok(Claimant::::get(&who).ok_or(Error::::NotInducted)?.last_active) } - */ } } diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index db78c17fe7751..6d9225b614f02 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -160,18 +160,24 @@ fn run_to(n: u64) { #[test] fn basic_stuff() { new_test_ext().execute_with(|| { - // assert!(Salary::last_claim(&0).is_err()); - // assert_eq!(Salary::status(), None); + assert!(Salary::last_active(&0).is_err()); + assert_eq!(Salary::status(), None); }); } -/* + #[test] fn can_start() { new_test_ext().execute_with(|| { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_eq!( Salary::status(), - Some(StatusType { cycle_index: 0, cycle_start: 1, remaining_budget: 10 }) + Some(StatusType { + cycle_index: 0, + cycle_start: 1, + budget: 10, + total_registrations: 0, + total_unregistered_paid: 0, + }) ); }); } @@ -187,7 +193,13 @@ fn bump_works() { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_eq!( Salary::status(), - Some(StatusType { cycle_index: 1, cycle_start: 5, remaining_budget: 10 }) + Some(StatusType { + cycle_index: 1, + cycle_start: 5, + budget: 10, + total_registrations: 0, + total_unregistered_paid: 0 + }) ); run_to(8); @@ -198,7 +210,13 @@ fn bump_works() { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_eq!( Salary::status(), - Some(StatusType { cycle_index: 2, cycle_start: 9, remaining_budget: 5 }) + Some(StatusType { + cycle_index: 2, + cycle_start: 9, + budget: 5, + total_registrations: 0, + total_unregistered_paid: 0 + }) ); }); } @@ -210,9 +228,9 @@ fn induct_works() { assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotMember); TestClub::change(&1, 1); - assert!(Salary::last_claim(&1).is_err()); + assert!(Salary::last_active(&1).is_err()); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); - assert_eq!(Salary::last_claim(&1).unwrap(), 0); + assert_eq!(Salary::last_active(&1).unwrap(), 0); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::AlreadyInducted); }); } @@ -226,24 +244,29 @@ fn payment_works() { assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); // No claim on the cycle active during induction. + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::TooEarly); + run_to(3); assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); - run_to(5); + run_to(6); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::TooEarly); + run_to(7); assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 1); - assert_eq!(Salary::status().unwrap().remaining_budget, 9); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(8); assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); run_to(9); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + run_to(11); assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 2); - assert_eq!(Salary::status().unwrap().remaining_budget, 9); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); }); } - +/* #[test] fn zero_payment_fails() { new_test_ext().execute_with(|| { From 5c38814d056d8b31cbe4b2a0da2fd2491f52dfd3 Mon Sep 17 00:00:00 2001 From: Gav Date: Thu, 16 Feb 2023 11:11:03 +0000 Subject: [PATCH 11/40] Tests --- frame/salary/src/tests.rs | 144 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 137 insertions(+), 7 deletions(-) diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 6d9225b614f02..ba5e076f548d8 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -236,7 +236,7 @@ fn induct_works() { } #[test] -fn payment_works() { +fn unregistered_payment_works() { new_test_ext().execute_with(|| { TestClub::change(&1, 1); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); @@ -266,29 +266,159 @@ fn payment_works() { assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); }); } -/* + +#[test] +fn registered_payment_works() { + new_test_ext().execute_with(|| { + TestClub::change(&1, 1); + assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + // No claim on the cycle active during induction. + assert_noop!(Salary::register(RuntimeOrigin::signed(1)), Error::::NoClaim); + run_to(3); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + + run_to(5); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::register(RuntimeOrigin::signed(1))); + assert_eq!(Salary::status().unwrap().total_registrations, 1); + run_to(7); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_eq!(paid(1), 1); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 0); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + + run_to(9); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_eq!(Salary::status().unwrap().total_registrations, 0); + assert_ok!(Salary::register(RuntimeOrigin::signed(1))); + assert_eq!(Salary::status().unwrap().total_registrations, 1); + run_to(11); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_eq!(paid(1), 2); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 0); + }); +} + #[test] fn zero_payment_fails() { new_test_ext().execute_with(|| { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); TestClub::change(&1, 0); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); - run_to(5); + run_to(7); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); }); } #[test] -fn bankrupt_fails_gracefully() { +fn unregistered_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - TestClub::change(&1, 11); + TestClub::change(&1, 2); + TestClub::change(&2, 6); + TestClub::change(&3, 12); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(2))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(3))); + + run_to(7); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); + + assert_eq!(paid(1), 2); + assert_eq!(paid(2), 6); + assert_eq!(paid(3), 2); + }); +} + +#[test] +fn registered_bankrupcy_fails_gracefully() { + new_test_ext().execute_with(|| { + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + TestClub::change(&1, 2); + TestClub::change(&2, 6); + TestClub::change(&3, 12); + + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(2))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(3))); + + run_to(5); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::register(RuntimeOrigin::signed(1))); + assert_ok!(Salary::register(RuntimeOrigin::signed(2))); + assert_ok!(Salary::register(RuntimeOrigin::signed(3))); + + run_to(7); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); + + assert_eq!(paid(1), 1); + assert_eq!(paid(2), 3); + assert_eq!(paid(3), 6); + }); +} + +#[test] +fn mixed_bankrupcy_fails_gracefully() { + new_test_ext().execute_with(|| { + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + TestClub::change(&1, 2); + TestClub::change(&2, 6); + TestClub::change(&3, 12); + + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(2))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(3))); + run_to(5); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::Bankrupt); + assert_ok!(Salary::register(RuntimeOrigin::signed(1))); + assert_ok!(Salary::register(RuntimeOrigin::signed(2))); + + run_to(7); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + + assert_eq!(paid(1), 2); + assert_eq!(paid(2), 6); + assert_eq!(paid(3), 2); + }); +} + +#[test] +fn other_mixed_bankrupcy_fails_gracefully() { + new_test_ext().execute_with(|| { + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + TestClub::change(&1, 2); + TestClub::change(&2, 6); + TestClub::change(&3, 12); + + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(2))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(3))); + + run_to(5); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::register(RuntimeOrigin::signed(2))); + assert_ok!(Salary::register(RuntimeOrigin::signed(3))); + + run_to(7); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); + assert_eq!(paid(1), 0); + assert_eq!(paid(2), 3); + assert_eq!(paid(3), 7); }); } -*/ From bfdf333fcb1d95eb343b426ce2216c43e44df7f4 Mon Sep 17 00:00:00 2001 From: Gav Date: Thu, 16 Feb 2023 11:16:04 +0000 Subject: [PATCH 12/40] Allow payment to be targeted elsewhere --- frame/salary/src/lib.rs | 17 ++++++++++--- frame/salary/src/tests.rs | 53 ++++++++++++++++++++------------------- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 55fd8075bf1b3..1cbbebb0f13c8 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -155,7 +155,12 @@ pub mod pallet { /// The next cycle begins. Registered { who: T::AccountId, amount: BalanceOf }, /// A payment happened. - Paid { who: T::AccountId, amount: BalanceOf, id: ::Id }, + Paid { + who: T::AccountId, + beneficiary: T::AccountId, + amount: BalanceOf, + id: ::Id, + }, /// The next cycle begins. CycleStarted { index: CycleIndexOf }, } @@ -281,7 +286,10 @@ pub mod pallet { /// - `origin`: A `Signed` origin of an account which is a member of `Members`. #[pallet::weight(T::WeightInfo::add_member())] #[pallet::call_index(3)] - pub fn payout(origin: OriginFor) -> DispatchResultWithPostInfo { + pub fn payout( + origin: OriginFor, + beneficiary: T::AccountId, + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let mut status = Status::::get().ok_or(Error::::NotStarted)?; @@ -326,12 +334,13 @@ pub mod pallet { claimant.unpaid = None; claimant.last_active = status.cycle_index; - let id = T::Paymaster::pay(&who, payout).map_err(|()| Error::::PayError)?; + let id = + T::Paymaster::pay(&beneficiary, payout).map_err(|()| Error::::PayError)?; Claimant::::insert(&who, &claimant); Status::::put(&status); - Self::deposit_event(Event::::Paid { who, amount: payout, id }); + Self::deposit_event(Event::::Paid { who, beneficiary, amount: payout, id }); Ok(Pays::No.into()) } } diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index ba5e076f548d8..0f8f6b4ad1ac5 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -241,28 +241,29 @@ fn unregistered_payment_works() { TestClub::change(&1, 1); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NotInducted); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); // No claim on the cycle active during induction. - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::TooEarly); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::TooEarly); run_to(3); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); run_to(6); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::TooEarly); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::TooEarly); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); assert_eq!(paid(1), 1); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); run_to(8); assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); run_to(9); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); run_to(11); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); - assert_eq!(paid(1), 2); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 10)); + assert_eq!(paid(1), 1); + assert_eq!(paid(10), 1); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); }); } @@ -273,22 +274,22 @@ fn registered_payment_works() { TestClub::change(&1, 1); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NotInducted); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); // No claim on the cycle active during induction. assert_noop!(Salary::register(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(3); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); run_to(5); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_ok!(Salary::register(RuntimeOrigin::signed(1))); assert_eq!(Salary::status().unwrap().total_registrations, 1); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); assert_eq!(paid(1), 1); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 0); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); run_to(9); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); @@ -296,7 +297,7 @@ fn registered_payment_works() { assert_ok!(Salary::register(RuntimeOrigin::signed(1))); assert_eq!(Salary::status().unwrap().total_registrations, 1); run_to(11); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); assert_eq!(paid(1), 2); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 0); }); @@ -310,7 +311,7 @@ fn zero_payment_fails() { assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(7); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::ClaimZero); }); } @@ -328,9 +329,9 @@ fn unregistered_bankrupcy_fails_gracefully() { run_to(7); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2), 2)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3), 3)); assert_eq!(paid(1), 2); assert_eq!(paid(2), 6); @@ -357,9 +358,9 @@ fn registered_bankrupcy_fails_gracefully() { assert_ok!(Salary::register(RuntimeOrigin::signed(3))); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2), 2)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3), 3)); assert_eq!(paid(1), 1); assert_eq!(paid(2), 3); @@ -385,9 +386,9 @@ fn mixed_bankrupcy_fails_gracefully() { assert_ok!(Salary::register(RuntimeOrigin::signed(2))); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3), 3)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2), 2)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); assert_eq!(paid(1), 2); assert_eq!(paid(2), 6); @@ -413,9 +414,9 @@ fn other_mixed_bankrupcy_fails_gracefully() { assert_ok!(Salary::register(RuntimeOrigin::signed(3))); run_to(7); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); - assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::ClaimZero); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2), 2)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3), 3)); assert_eq!(paid(1), 0); assert_eq!(paid(2), 3); From 807a3c99c598b06b0e388ccc7361acdf4ad54ab1 Mon Sep 17 00:00:00 2001 From: Gav Date: Thu, 16 Feb 2023 18:46:44 +0000 Subject: [PATCH 13/40] Proper ssync payment failure handling --- frame/salary/src/lib.rs | 103 ++++++++++++++++++++++++-------------- frame/salary/src/tests.rs | 7 +++ 2 files changed, 73 insertions(+), 37 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 1cbbebb0f13c8..ba44b66b69464 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -46,6 +46,14 @@ pub use weights::WeightInfo; /// Payroll cycle. pub type Cycle = u32; +#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub enum PaymentStatus { + InProgress, + Unknown, + Success, + Failure, +} + // Can be implemented by Pot pallet with a fixed Currency impl, but can also be implemented with // XCM/MultiAsset and made generic over assets. pub trait Pay { @@ -54,10 +62,13 @@ pub trait Pay { /// The type by which we identify the individuals to whom a payment may be made. type AccountId; /// An identifier given to an individual payment. - type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug; + type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug + Copy; /// Make a payment and return an identifier for later evaluation of success in some off-chain /// mechanism (likely an event, but possibly not on this chain). fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result; + /// Check how a payment has proceeded. `id` must have been a previously returned by `pay` for + /// the result of this call to be meaningful. + fn check_payment(id: Self::Id) -> PaymentStatus; } #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] @@ -70,11 +81,22 @@ pub struct StatusType { } #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] -pub struct ClaimantStatus { +pub enum ClaimStatus { + /// No claim recorded. + Nothing, + /// Amount reserved when last active. + Registered(Balance), + /// Amount attempted to be paid when last active as well as the identity of the payment. + Attempted { amount: Balance, id: Id }, +} + +use ClaimStatus::*; + +#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub struct ClaimantStatus { /// The most recent cycle in which the claimant was active. last_active: CycleIndex, - /// The amount reserved in `last_active` cycle, or `None` if paid. - unpaid: Option, + status: ClaimStatus, } #[frame_support::pallet] @@ -133,9 +155,10 @@ pub mod pallet { pub type CycleIndexOf = ::BlockNumber; pub type BalanceOf = <>::Paymaster as Pay>::Balance; + pub type IdOf = <>::Paymaster as Pay>::Id; pub type StatusOf = StatusType, ::BlockNumber, BalanceOf>; - pub type ClaimantStatusOf = ClaimantStatus, BalanceOf>; + pub type ClaimantStatusOf = ClaimantStatus, BalanceOf, IdOf>; /// The overall status of the system. #[pallet::storage] @@ -204,7 +227,7 @@ pub mod pallet { Claimant::::insert( &who, - ClaimantStatus { last_active: cycle_index, unpaid: None }, + ClaimantStatus { last_active: cycle_index, status: Nothing }, ); Self::deposit_event(Event::::Inducted { who }); @@ -268,7 +291,7 @@ pub mod pallet { let payout = T::ActiveSalaryForRank::convert(rank); ensure!(!payout.is_zero(), Error::::ClaimZero); claimant.last_active = status.cycle_index; - claimant.unpaid = Some(payout); + claimant.status = Registered(payout); status.total_registrations.saturating_accrue(payout); Claimant::::insert(&who, &claimant); @@ -298,45 +321,51 @@ pub mod pallet { let now = frame_system::Pallet::::block_number(); ensure!( now >= status.cycle_start + T::RegistrationPeriod::get(), - Error::::TooEarly + Error::::TooEarly, ); - let payout = if let Some(unpaid) = claimant.unpaid { - ensure!(claimant.last_active == status.cycle_index, Error::::NoClaim); - - // Registered for this cycle. Pay accordingly. - if status.total_registrations <= status.budget { - // Can pay in full. - unpaid - } else { - // Must be reduced pro-rata - Perbill::from_rational(status.budget, status.total_registrations) * unpaid - } - } else { - ensure!(claimant.last_active < status.cycle_index, Error::::NoClaim); - - // Not registered for this cycle. Pay from whatever is left. - let rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; - let ideal_payout = T::ActiveSalaryForRank::convert(rank); - - let pot = status - .budget - .saturating_sub(status.total_registrations) - .saturating_sub(status.total_unregistered_paid); - - let payout = ideal_payout.min(pot); - ensure!(!payout.is_zero(), Error::::ClaimZero); - - status.total_unregistered_paid.saturating_accrue(payout); - payout + let payout = match claimant.status { + Attempted { amount, id } if claimant.last_active == status.cycle_index => { + let failed = matches!(T::Paymaster::check_payment(id), PaymentStatus::Failure); + ensure!(failed, Error::::NoClaim); + amount + }, + Registered(unpaid) if claimant.last_active == status.cycle_index => { + // Registered for this cycle. Pay accordingly. + if status.total_registrations <= status.budget { + // Can pay in full. + unpaid + } else { + // Must be reduced pro-rata + Perbill::from_rational(status.budget, status.total_registrations) * unpaid + } + }, + Nothing | Attempted { .. } if claimant.last_active < status.cycle_index => { + // Not registered for this cycle. Pay from whatever is left. + let rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; + let ideal_payout = T::ActiveSalaryForRank::convert(rank); + + let pot = status + .budget + .saturating_sub(status.total_registrations) + .saturating_sub(status.total_unregistered_paid); + + let payout = ideal_payout.min(pot); + ensure!(!payout.is_zero(), Error::::ClaimZero); + + status.total_unregistered_paid.saturating_accrue(payout); + payout + }, + _ => return Err(Error::::NoClaim.into()), }; - claimant.unpaid = None; claimant.last_active = status.cycle_index; let id = T::Paymaster::pay(&beneficiary, payout).map_err(|()| Error::::PayError)?; + claimant.status = Attempted { amount: payout, id }; + Claimant::::insert(&who, &claimant); Status::::put(&status); diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 0f8f6b4ad1ac5..94225e1f09f7e 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -82,12 +82,16 @@ impl frame_system::Config for Test { thread_local! { pub static PAID: RefCell> = RefCell::new(BTreeMap::new()); + pub static STATUS: RefCell> = RefCell::new(BTreeMap::new()); pub static LAST_ID: RefCell = RefCell::new(0u64); } fn paid(who: u64) -> u64 { PAID.with(|p| p.borrow().get(&who).cloned().unwrap_or(0)) } +fn set_status(id: u64, s: PaymentStatus) { + STATUS.with(|m| m.borrow_mut().insert(id, s)); +} pub struct TestPay; impl Pay for TestPay { @@ -103,6 +107,9 @@ impl Pay for TestPay { x })) } + fn check_payment(id: Self::Id) -> PaymentStatus { + STATUS.with(|s| s.borrow().get(&id).cloned().unwrap_or(PaymentStatus::Unknown)) + } } thread_local! { From cf1f36540a884b8bdbbf428a0cbbfc7da8d55a62 Mon Sep 17 00:00:00 2001 From: Gav Date: Wed, 22 Feb 2023 15:27:32 +0000 Subject: [PATCH 14/40] Test for repayment --- frame/salary/src/tests.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 94225e1f09f7e..045a32494cb1d 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -89,6 +89,9 @@ thread_local! { fn paid(who: u64) -> u64 { PAID.with(|p| p.borrow().get(&who).cloned().unwrap_or(0)) } +fn unpay(who: u64, amount: u64) { + PAID.with(|p| p.borrow_mut().entry(who).or_default().saturating_reduce(amount)) +} fn set_status(id: u64, s: PaymentStatus) { STATUS.with(|m| m.borrow_mut().insert(id, s)); } @@ -275,6 +278,40 @@ fn unregistered_payment_works() { }); } +#[test] +fn retry_payment_works() { + new_test_ext().execute_with(|| { + TestClub::change(&1, 1); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + run_to(6); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + run_to(7); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + + // Payment failed. + unpay(1, 1); + set_status(0, PaymentStatus::Failure); + + // Allowed to try again. + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + + assert_eq!(paid(1), 1); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + + assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); + run_to(8); + assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); + run_to(9); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + run_to(11); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 10)); + assert_eq!(paid(1), 1); + assert_eq!(paid(10), 1); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + }); +} + #[test] fn registered_payment_works() { new_test_ext().execute_with(|| { From d82907801391078ba04385165906d18e6c3b7899 Mon Sep 17 00:00:00 2001 From: Gav Date: Wed, 22 Feb 2023 18:33:35 +0000 Subject: [PATCH 15/40] Docs --- frame/salary/src/lib.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index ba44b66b69464..d7dcf062a78c4 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -46,11 +46,16 @@ pub use weights::WeightInfo; /// Payroll cycle. pub type Cycle = u32; +/// Status for making a payment via the `Pay::pay` trait function. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] pub enum PaymentStatus { + /// Payment is in progress. Nothing to report yet. InProgress, + /// Payment status is unknowable. It will never be reported successful or failed. Unknown, + /// Payment happened successfully. Success, + /// Payment failed. It may safely be retried. Failure, } @@ -71,17 +76,24 @@ pub trait Pay { fn check_payment(id: Self::Id) -> PaymentStatus; } +/// The status of the pallet instance. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] pub struct StatusType { + /// The index of the "current cycle" (i.e. the last cycle being processed). cycle_index: CycleIndex, + /// The first block of the "current cycle" (i.e. the last cycle being processed) cycle_start: BlockNumber, + /// The total budget available for all payments in the current cycle. budget: Balance, + /// The total amount of the payments registered in the current cycle. total_registrations: Balance, + /// The total amount of unregistered payments which have been made in the current cycle. total_unregistered_paid: Balance, } +/// The state of a specific payment claim. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] -pub enum ClaimStatus { +pub enum ClaimState { /// No claim recorded. Nothing, /// Amount reserved when last active. @@ -90,13 +102,15 @@ pub enum ClaimStatus { Attempted { amount: Balance, id: Id }, } -use ClaimStatus::*; +use ClaimState::*; +/// The status of a single payee/claimant. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] pub struct ClaimantStatus { /// The most recent cycle in which the claimant was active. last_active: CycleIndex, - status: ClaimStatus, + /// The state of the payment/claim with in the above cycle. + status: ClaimState, } #[frame_support::pallet] From 7162964df9835ce60847f5bedacb5daef823fc87 Mon Sep 17 00:00:00 2001 From: Gav Date: Sun, 26 Feb 2023 08:30:48 +0100 Subject: [PATCH 16/40] Impl RankedMembers for RankedCollective --- frame/ranked-collective/src/lib.rs | 77 ++++++++++++++++++++--------- frame/support/src/traits/members.rs | 21 +++++--- primitives/arithmetic/src/traits.rs | 14 ++++++ 3 files changed, 84 insertions(+), 28 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 36c595d3c08bf..f33312c08ba8d 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -54,7 +54,7 @@ use frame_support::{ codec::{Decode, Encode, MaxEncodedLen}, dispatch::{DispatchError, DispatchResultWithPostInfo, PostDispatchInfo}, ensure, - traits::{EnsureOrigin, PollStatus, Polling, VoteTally}, + traits::{EnsureOrigin, PollStatus, Polling, RankedMembers, VoteTally}, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; @@ -465,24 +465,7 @@ pub mod pallet { pub fn demote_member(origin: OriginFor, who: AccountIdLookupOf) -> DispatchResult { let max_rank = T::DemoteOrigin::ensure_origin(origin)?; let who = T::Lookup::lookup(who)?; - let mut record = Self::ensure_member(&who)?; - let rank = record.rank; - ensure!(max_rank >= rank, Error::::NoPermission); - - Self::remove_from_rank(&who, rank)?; - let maybe_rank = rank.checked_sub(1); - match maybe_rank { - None => { - Members::::remove(&who); - Self::deposit_event(Event::MemberRemoved { who, rank: 0 }); - }, - Some(rank) => { - record.rank = rank; - Members::::insert(&who, &record); - Self::deposit_event(Event::RankChanged { who, rank }); - }, - } - Ok(()) + Self::do_demote_member(who, Some(max_rank)) } /// Remove the member entirely. @@ -652,7 +635,7 @@ pub mod pallet { Ok(()) } - /// Promotes a member in the ranked collective into the next role. + /// Promotes a member in the ranked collective into the next higher rank. /// /// A `maybe_max_rank` may be provided to check that the member does not get promoted beyond /// a certain rank. Is `None` is provided, then the rank will be incremented without checks. @@ -674,6 +657,33 @@ pub mod pallet { Ok(()) } + /// Demotes a member in the ranked collective into the next lower rank. + /// + /// A `maybe_max_rank` may be provided to check that the member does not get demoted from + /// a certain rank. Is `None` is provided, then the rank will be decremented without checks. + fn do_demote_member(who: T::AccountId, maybe_max_rank: Option) -> DispatchResult { + let mut record = Self::ensure_member(&who)?; + let rank = record.rank; + if let Some(max_rank) = maybe_max_rank { + ensure!(max_rank >= rank, Error::::NoPermission); + } + + Self::remove_from_rank(&who, rank)?; + let maybe_rank = rank.checked_sub(1); + match maybe_rank { + None => { + Members::::remove(&who); + Self::deposit_event(Event::MemberRemoved { who, rank: 0 }); + }, + Some(rank) => { + record.rank = rank; + Members::::insert(&who, &record); + Self::deposit_event(Event::RankChanged { who, rank }); + }, + } + Ok(()) + } + /// Add a member to the rank collective, and continue to promote them until a certain rank /// is reached. pub fn do_add_member_to_rank(who: T::AccountId, rank: Rank) -> DispatchResult { @@ -684,6 +694,29 @@ pub mod pallet { Ok(()) } } -} -// TODO: Impl `RankedMembers` for this pallet. + impl, I: 'static> RankedMembers for Pallet { + type AccountId = T::AccountId; + type Rank = Rank; + + fn lowest_rank() -> Self::Rank { + 0 + } + + fn rank_of(who: &Self::AccountId) -> Option { + Some(Self::ensure_member(&who).ok()?.rank) + } + + fn induct(who: &Self::AccountId) -> DispatchResult { + Self::do_add_member(who.clone()) + } + + fn promote(who: &Self::AccountId) -> DispatchResult { + Self::do_promote_member(who.clone(), None) + } + + fn demote(who: &Self::AccountId) -> DispatchResult { + Self::do_demote_member(who.clone(), None) + } + } +} diff --git a/frame/support/src/traits/members.rs b/frame/support/src/traits/members.rs index 82540f4249e07..130258d8e47c8 100644 --- a/frame/support/src/traits/members.rs +++ b/frame/support/src/traits/members.rs @@ -18,7 +18,8 @@ //! Traits for dealing with the idea of membership. use impl_trait_for_tuples::impl_for_tuples; -use sp_arithmetic::traits::AtLeast32BitUnsigned; +use sp_arithmetic::traits::AtLeast16BitUnsigned; +use sp_runtime::DispatchResult; use sp_std::{marker::PhantomData, prelude::*}; /// A trait for querying whether a type can be said to "contain" a value. @@ -269,15 +270,23 @@ pub trait ContainsLengthBound { /// Ranked membership data structure. pub trait RankedMembers { type AccountId; - type Rank: AtLeast32BitUnsigned; + type Rank: AtLeast16BitUnsigned; + + /// The lowest rank possible in this membership organisation. + fn lowest_rank() -> Self::Rank; /// Return the rank of the given ID, or `None` if they are not a member. fn rank_of(who: &Self::AccountId) -> Option; - /// Remove a member from the group. This does not result in a call to `removed`. - fn remove(who: &Self::AccountId); - /// Change a member's rank. This does not result in a call to `changed`. - fn change(who: &Self::AccountId, rank: Self::Rank); + /// Add a member to the group at the `lowest_rank()`. + fn induct(who: &Self::AccountId) -> DispatchResult; + + /// Promote a member to the next higher rank. + fn promote(who: &Self::AccountId) -> DispatchResult; + + /// Demote a member to the next lower rank; demoting beyond the `lowest_rank` removes the + /// member entirely. + fn demote(who: &Self::AccountId) -> DispatchResult; } /// Trait for type that can handle the initialization of account IDs at genesis. diff --git a/primitives/arithmetic/src/traits.rs b/primitives/arithmetic/src/traits.rs index dfba046754373..c0b9c7d11fee8 100644 --- a/primitives/arithmetic/src/traits.rs +++ b/primitives/arithmetic/src/traits.rs @@ -148,6 +148,20 @@ impl< { } +/// A meta trait for arithmetic. +/// +/// Arithmetic types do all the usual stuff you'd expect numbers to do. They are guaranteed to +/// be able to represent at least `u16` values without loss, hence the trait implies `From` +/// and smaller integers. All other conversions are fallible. +pub trait AtLeast16Bit: BaseArithmetic + From + From {} + +impl + From> AtLeast16Bit for T {} + +/// A meta trait for arithmetic. Same as [`AtLeast16Bit `], but also bounded to be unsigned. +pub trait AtLeast16BitUnsigned: AtLeast16Bit + Unsigned {} + +impl AtLeast16BitUnsigned for T {} + /// A meta trait for arithmetic. /// /// Arithmetic types do all the usual stuff you'd expect numbers to do. They are guaranteed to From 2e299f4271d3a363b767a2c0c480cfafc16fe1c1 Mon Sep 17 00:00:00 2001 From: Gav Date: Sun, 26 Feb 2023 09:07:46 +0100 Subject: [PATCH 17/40] Implement Pay for Pot (i.e. basic account). --- frame/ranked-collective/src/lib.rs | 2 +- frame/salary/src/lib.rs | 20 ++++++++- frame/salary/src/tests.rs | 67 ++++++++++++++++++++--------- frame/support/src/traits/members.rs | 6 +-- 4 files changed, 69 insertions(+), 26 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index f33312c08ba8d..3ce77b86fd087 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -699,7 +699,7 @@ pub mod pallet { type AccountId = T::AccountId; type Rank = Rank; - fn lowest_rank() -> Self::Rank { + fn min_rank() -> Self::Rank { 0 } diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index d7dcf062a78c4..263e201b61866 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -23,6 +23,7 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; use sp_arithmetic::traits::{Saturating, Zero}; +use sp_core::TypedGet; use sp_runtime::{ traits::{AtLeast32BitUnsigned, Convert}, Perbill, @@ -30,7 +31,10 @@ use sp_runtime::{ use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; use frame_support::{ - dispatch::DispatchResultWithPostInfo, ensure, traits::RankedMembers, RuntimeDebug, + dispatch::DispatchResultWithPostInfo, + ensure, + traits::{tokens::fungible, RankedMembers}, + RuntimeDebug, }; #[cfg(test)] @@ -76,6 +80,20 @@ pub trait Pay { fn check_payment(id: Self::Id) -> PaymentStatus; } +pub struct Pot(sp_std::marker::PhantomData<(F, A)>); +impl> Pay for Pot { + type Balance = F::Balance; + type AccountId = A::Type; + type Id = (); + fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result { + F::transfer(&A::get(), who, amount, false).map_err(|_| ())?; + Ok(()) + } + fn check_payment(_: ()) -> PaymentStatus { + PaymentStatus::Success + } +} + /// The status of the pallet instance. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] pub struct StatusType { diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 045a32494cb1d..3f123682118c9 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -29,6 +29,7 @@ use sp_core::H256; use sp_runtime::{ testing::Header, traits::{BlakeTwo256, Identity, IdentityLookup}, + DispatchResult, }; use sp_std::cell::RefCell; @@ -123,17 +124,41 @@ pub struct TestClub; impl RankedMembers for TestClub { type AccountId = u64; type Rank = u64; + fn min_rank() -> Self::Rank { + 0 + } fn rank_of(who: &Self::AccountId) -> Option { CLUB.with(|club| club.borrow().get(who).cloned()) } - fn remove(who: &Self::AccountId) { - CLUB.with(|club| club.borrow_mut().remove(&who)); + fn induct(who: &Self::AccountId) -> DispatchResult { + CLUB.with(|club| club.borrow_mut().insert(*who, 0)); + Ok(()) + } + fn promote(who: &Self::AccountId) -> DispatchResult { + CLUB.with(|club| { + club.borrow_mut().entry(*who).and_modify(|r| *r += 1); + }); + Ok(()) } - fn change(who: &Self::AccountId, rank: Self::Rank) { - CLUB.with(|club| club.borrow_mut().insert(*who, rank)); + fn demote(who: &Self::AccountId) -> DispatchResult { + CLUB.with(|club| match club.borrow().get(who) { + None => Err(sp_runtime::DispatchError::Unavailable), + Some(&0) => { + club.borrow_mut().remove(&who); + Ok(()) + }, + Some(_) => { + club.borrow_mut().entry(*who).and_modify(|x| *x -= 1); + Ok(()) + }, + }) } } +fn set_rank(who: u64, rank: u64) { + CLUB.with(|club| club.borrow_mut().insert(who, rank)); +} + parameter_types! { pub static Budget: u64 = 10; } @@ -237,7 +262,7 @@ fn induct_works() { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotMember); - TestClub::change(&1, 1); + set_rank(1, 1); assert!(Salary::last_active(&1).is_err()); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); assert_eq!(Salary::last_active(&1).unwrap(), 0); @@ -248,7 +273,7 @@ fn induct_works() { #[test] fn unregistered_payment_works() { new_test_ext().execute_with(|| { - TestClub::change(&1, 1); + set_rank(1, 1); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NotInducted); @@ -281,7 +306,7 @@ fn unregistered_payment_works() { #[test] fn retry_payment_works() { new_test_ext().execute_with(|| { - TestClub::change(&1, 1); + set_rank(1, 1); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(6); @@ -315,7 +340,7 @@ fn retry_payment_works() { #[test] fn registered_payment_works() { new_test_ext().execute_with(|| { - TestClub::change(&1, 1); + set_rank(1, 1); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NotInducted); @@ -351,7 +376,7 @@ fn registered_payment_works() { fn zero_payment_fails() { new_test_ext().execute_with(|| { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - TestClub::change(&1, 0); + set_rank(1, 0); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(7); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); @@ -363,9 +388,9 @@ fn zero_payment_fails() { fn unregistered_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - TestClub::change(&1, 2); - TestClub::change(&2, 6); - TestClub::change(&3, 12); + set_rank(1, 2); + set_rank(2, 6); + set_rank(3, 12); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); assert_ok!(Salary::induct(RuntimeOrigin::signed(2))); @@ -387,9 +412,9 @@ fn unregistered_bankrupcy_fails_gracefully() { fn registered_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - TestClub::change(&1, 2); - TestClub::change(&2, 6); - TestClub::change(&3, 12); + set_rank(1, 2); + set_rank(2, 6); + set_rank(3, 12); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); assert_ok!(Salary::induct(RuntimeOrigin::signed(2))); @@ -416,9 +441,9 @@ fn registered_bankrupcy_fails_gracefully() { fn mixed_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - TestClub::change(&1, 2); - TestClub::change(&2, 6); - TestClub::change(&3, 12); + set_rank(1, 2); + set_rank(2, 6); + set_rank(3, 12); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); assert_ok!(Salary::induct(RuntimeOrigin::signed(2))); @@ -444,9 +469,9 @@ fn mixed_bankrupcy_fails_gracefully() { fn other_mixed_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - TestClub::change(&1, 2); - TestClub::change(&2, 6); - TestClub::change(&3, 12); + set_rank(1, 2); + set_rank(2, 6); + set_rank(3, 12); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); assert_ok!(Salary::induct(RuntimeOrigin::signed(2))); diff --git a/frame/support/src/traits/members.rs b/frame/support/src/traits/members.rs index 130258d8e47c8..c93217e8b8ab6 100644 --- a/frame/support/src/traits/members.rs +++ b/frame/support/src/traits/members.rs @@ -273,18 +273,18 @@ pub trait RankedMembers { type Rank: AtLeast16BitUnsigned; /// The lowest rank possible in this membership organisation. - fn lowest_rank() -> Self::Rank; + fn min_rank() -> Self::Rank; /// Return the rank of the given ID, or `None` if they are not a member. fn rank_of(who: &Self::AccountId) -> Option; - /// Add a member to the group at the `lowest_rank()`. + /// Add a member to the group at the `min_rank()`. fn induct(who: &Self::AccountId) -> DispatchResult; /// Promote a member to the next higher rank. fn promote(who: &Self::AccountId) -> DispatchResult; - /// Demote a member to the next lower rank; demoting beyond the `lowest_rank` removes the + /// Demote a member to the next lower rank; demoting beyond the `min_rank` removes the /// member entirely. fn demote(who: &Self::AccountId) -> DispatchResult; } From 5995eebd9b10712a246bdb74bc75f2aef92f38df Mon Sep 17 00:00:00 2001 From: Gav Date: Sun, 26 Feb 2023 17:41:23 +0100 Subject: [PATCH 18/40] Benchmarks --- frame/salary/src/benchmarking.rs | 172 ++++++++++++++++++++--- frame/salary/src/lib.rs | 215 ++++++++++++++++++++--------- frame/salary/src/tests.rs | 225 +++++++++++++++++++++++++------ 3 files changed, 496 insertions(+), 116 deletions(-) diff --git a/frame/salary/src/benchmarking.rs b/frame/salary/src/benchmarking.rs index a26151654f256..0d609ab3374f4 100644 --- a/frame/salary/src/benchmarking.rs +++ b/frame/salary/src/benchmarking.rs @@ -15,31 +15,169 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Staking pallet benchmarking. +//! Salary pallet benchmarking. + +#![cfg(feature = "runtime-benchmarks")] use super::*; -#[allow(unused_imports)] -use crate::Pallet as RankedCollective; +use crate::Pallet as Salary; -use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller}; -use frame_support::{assert_ok, dispatch::UnfilteredDispatchable}; -use frame_system::RawOrigin as SystemOrigin; +use frame_benchmarking::v2::*; +use frame_system::{Pallet as System, RawOrigin}; +use sp_core::Get; const SEED: u32 = 0; -fn assert_last_event, I: 'static>(generic_event: >::RuntimeEvent) { - frame_system::Pallet::::assert_last_event(generic_event.into()); +fn ensure_member_with_salary, I: 'static>(who: &T::AccountId) { + // induct if not a member. + if T::Members::rank_of(who).is_none() { + T::Members::induct(who).unwrap(); + } + // promote until they have a salary. + for _ in 0..255 { + let r = T::Members::rank_of(who).expect("prior guard ensures `who` is a member; qed"); + if !T::ActiveSalaryForRank::convert(r).is_zero() { + break + } + T::Members::promote(who).unwrap(); + } } -benchmarks_instance_pallet! { - add_member { - let origin = T::PromoteOrigin::successful_origin(); - let call = Call::::add_member { }; - }: { call.dispatch_bypass_filter(origin)? } - verify { - assert_eq!(MemberCount::::get(0), 1); - assert_last_event::(Event::MemberAdded { who }.into()); +#[instance_benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn init() { + let caller: T::AccountId = whitelisted_caller(); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + + assert!(Salary::::status().is_some()); + } + + #[benchmark] + fn bump() { + let caller: T::AccountId = whitelisted_caller(); + Salary::::init(RawOrigin::Signed(caller.clone()).into()).unwrap(); + System::::set_block_number(System::::block_number() + Salary::::cycle_period()); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + + assert_eq!(Salary::::status().unwrap().cycle_index, 1u32.into()); + } + + #[benchmark] + fn induct() { + let caller = whitelisted_caller(); + ensure_member_with_salary::(&caller); + Salary::::init(RawOrigin::Signed(caller.clone()).into()).unwrap(); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + + assert!(Salary::::last_active(&caller).is_ok()); + } + + #[benchmark] + fn register() { + let caller = whitelisted_caller(); + ensure_member_with_salary::(&caller); + Salary::::init(RawOrigin::Signed(caller.clone()).into()).unwrap(); + Salary::::induct(RawOrigin::Signed(caller.clone()).into()).unwrap(); + System::::set_block_number(System::::block_number() + Salary::::cycle_period()); + Salary::::bump(RawOrigin::Signed(caller.clone()).into()).unwrap(); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + + assert_eq!(Salary::::last_active(&caller).unwrap(), 1u32.into()); } - impl_benchmark_test_suite!(RankedCollective, crate::tests::new_test_ext(), crate::tests::Test); + #[benchmark] + fn payout() { + let caller = whitelisted_caller(); + ensure_member_with_salary::(&caller); + Salary::::init(RawOrigin::Signed(caller.clone()).into()).unwrap(); + Salary::::induct(RawOrigin::Signed(caller.clone()).into()).unwrap(); + System::::set_block_number(System::::block_number() + Salary::::cycle_period()); + Salary::::bump(RawOrigin::Signed(caller.clone()).into()).unwrap(); + System::::set_block_number(System::::block_number() + T::RegistrationPeriod::get()); + + let salary = T::ActiveSalaryForRank::convert(T::Members::rank_of(&caller).unwrap()); + T::Paymaster::ensure_successful(&caller, salary); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + + match Claimant::::get(&caller) { + Some(ClaimantStatus { last_active, status: Attempted { id, .. } }) => { + assert_eq!(last_active, 1u32.into()); + assert_ne!(T::Paymaster::check_payment(id), PaymentStatus::Failure); + }, + _ => panic!("No claim made"), + } + assert!(Salary::::payout(RawOrigin::Signed(caller.clone()).into()).is_err()); + } + + #[benchmark] + fn payout_other() { + let caller = whitelisted_caller(); + ensure_member_with_salary::(&caller); + Salary::::init(RawOrigin::Signed(caller.clone()).into()).unwrap(); + Salary::::induct(RawOrigin::Signed(caller.clone()).into()).unwrap(); + System::::set_block_number(System::::block_number() + Salary::::cycle_period()); + Salary::::bump(RawOrigin::Signed(caller.clone()).into()).unwrap(); + System::::set_block_number(System::::block_number() + T::RegistrationPeriod::get()); + + let salary = T::ActiveSalaryForRank::convert(T::Members::rank_of(&caller).unwrap()); + let recipient: T::AccountId = account("recipient", 0, SEED); + T::Paymaster::ensure_successful(&recipient, salary); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), recipient.clone()); + + match Claimant::::get(&caller) { + Some(ClaimantStatus { last_active, status: Attempted { id, .. } }) => { + assert_eq!(last_active, 1u32.into()); + assert_ne!(T::Paymaster::check_payment(id), PaymentStatus::Failure); + }, + _ => panic!("No claim made"), + } + assert!(Salary::::payout(RawOrigin::Signed(caller.clone()).into()).is_err()); + } + + #[benchmark] + fn check_payment() { + let caller = whitelisted_caller(); + ensure_member_with_salary::(&caller); + Salary::::init(RawOrigin::Signed(caller.clone()).into()).unwrap(); + Salary::::induct(RawOrigin::Signed(caller.clone()).into()).unwrap(); + System::::set_block_number(System::::block_number() + Salary::::cycle_period()); + Salary::::bump(RawOrigin::Signed(caller.clone()).into()).unwrap(); + System::::set_block_number(System::::block_number() + T::RegistrationPeriod::get()); + + let salary = T::ActiveSalaryForRank::convert(T::Members::rank_of(&caller).unwrap()); + let recipient: T::AccountId = account("recipient", 0, SEED); + T::Paymaster::ensure_successful(&recipient, salary); + Salary::::payout(RawOrigin::Signed(caller.clone()).into()).unwrap(); + let id = match Claimant::::get(&caller).unwrap().status { + Attempted { id, .. } => id, + _ => panic!("No claim made"), + }; + T::Paymaster::ensure_concluded(id); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + + assert!(!matches!(Claimant::::get(&caller).unwrap().status, Attempted { .. })); + } + + impl_benchmark_test_suite! { + Salary, + crate::tests::new_test_ext(), + crate::tests::Test, + } } diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 263e201b61866..6284c3fa72e89 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -78,20 +78,37 @@ pub trait Pay { /// Check how a payment has proceeded. `id` must have been a previously returned by `pay` for /// the result of this call to be meaningful. fn check_payment(id: Self::Id) -> PaymentStatus; + /// Ensure that a call to pay with the given parameters will be successful if done immediately + /// after this call. Used in benchmarking code. + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(who: &Self::AccountId, amount: Self::Balance); + /// Ensure that a call to [check_payment] with the given parameters will return either [Success] + /// or [Failure]. + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(id: Self::Id); } -pub struct Pot(sp_std::marker::PhantomData<(F, A)>); -impl> Pay for Pot { +/// Simple implementation of `Pay` which makes a payment from a "pot" - i.e. a single account. +pub struct PayFromAccount(sp_std::marker::PhantomData<(F, A)>); +impl + fungible::Mutate> Pay + for PayFromAccount +{ type Balance = F::Balance; type AccountId = A::Type; type Id = (); fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result { - F::transfer(&A::get(), who, amount, false).map_err(|_| ())?; + >::transfer(&A::get(), who, amount, false).map_err(|_| ())?; Ok(()) } fn check_payment(_: ()) -> PaymentStatus { PaymentStatus::Success } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: &Self::AccountId, amount: Self::Balance) { + >::mint_into(&A::get(), amount).unwrap(); + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(_: Self::Id) {} } /// The status of the pallet instance. @@ -117,7 +134,7 @@ pub enum ClaimState { /// Amount reserved when last active. Registered(Balance), /// Amount attempted to be paid when last active as well as the identity of the payment. - Attempted { amount: Balance, id: Id }, + Attempted { registered: Option, id: Id, amount: Balance }, } use ClaimState::*; @@ -158,6 +175,8 @@ pub mod pallet { type Members: RankedMembers::AccountId>; /// The maximum payout to be made for a single period to an active member of the given rank. + /// + /// The benchmarks require that this be non-zero for some rank at most 255. type ActiveSalaryForRank: Convert< ::Rank, ::Balance, @@ -222,6 +241,8 @@ pub mod pallet { #[pallet::error] pub enum Error { + /// The salary system has already been started. + AlreadyStarted, /// The account is not a ranked member. NotMember, /// The account is already inducted. @@ -244,25 +265,33 @@ pub mod pallet { Bankrupt, /// There was some issue with the mechanism of payment. PayError, + /// The payment has neither failed nor succeeded yet. + Inconclusive, + /// The cycle is after that in which the payment was made. + NotCurrent, } #[pallet::call] impl, I: 'static> Pallet { - /// Induct oneself into the payout system. + /// Start the first payout cycle. + /// + /// - `origin`: A `Signed` origin of an account which is a member of `Members`. #[pallet::weight(T::WeightInfo::add_member())] #[pallet::call_index(0)] - pub fn induct(origin: OriginFor) -> DispatchResultWithPostInfo { - let who = ensure_signed(origin)?; - let cycle_index = Status::::get().ok_or(Error::::NotStarted)?.cycle_index; - let _ = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; - ensure!(!Claimant::::contains_key(&who), Error::::AlreadyInducted); - - Claimant::::insert( - &who, - ClaimantStatus { last_active: cycle_index, status: Nothing }, - ); + pub fn init(origin: OriginFor) -> DispatchResultWithPostInfo { + let _ = ensure_signed(origin)?; + let now = frame_system::Pallet::::block_number(); + ensure!(!Status::::exists(), Error::::AlreadyStarted); + let status = StatusType { + cycle_index: Zero::zero(), + cycle_start: now, + budget: T::Budget::get(), + total_registrations: Zero::zero(), + total_unregistered_paid: Zero::zero(), + }; + Status::::put(&status); - Self::deposit_event(Event::::Inducted { who }); + Self::deposit_event(Event::::CycleStarted { index: status.cycle_index }); Ok(Pays::No.into()) } @@ -274,33 +303,38 @@ pub mod pallet { pub fn bump(origin: OriginFor) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; let now = frame_system::Pallet::::block_number(); - let cycle_period = T::RegistrationPeriod::get() + T::PayoutPeriod::get(); - let status = match Status::::get() { - // Not first time... (move along start block and bump index) - Some(mut status) => { - status.cycle_start.saturating_accrue(cycle_period); - ensure!(now >= status.cycle_start, Error::::NotYet); - status.cycle_index.saturating_inc(); - status.budget = T::Budget::get(); - status.total_registrations = Zero::zero(); - status.total_unregistered_paid = Zero::zero(); - status - }, - // First time... (initialize) - None => StatusType { - cycle_index: Zero::zero(), - cycle_start: now, - budget: T::Budget::get(), - total_registrations: Zero::zero(), - total_unregistered_paid: Zero::zero(), - }, - }; + let cycle_period = Self::cycle_period(); + let mut status = Status::::get().ok_or(Error::::NotStarted)?; + status.cycle_start.saturating_accrue(cycle_period); + ensure!(now >= status.cycle_start, Error::::NotYet); + status.cycle_index.saturating_inc(); + status.budget = T::Budget::get(); + status.total_registrations = Zero::zero(); + status.total_unregistered_paid = Zero::zero(); Status::::put(&status); Self::deposit_event(Event::::CycleStarted { index: status.cycle_index }); Ok(Pays::No.into()) } + /// Induct oneself into the payout system. + #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::call_index(2)] + pub fn induct(origin: OriginFor) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin)?; + let cycle_index = Status::::get().ok_or(Error::::NotStarted)?.cycle_index; + let _ = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; + ensure!(!Claimant::::contains_key(&who), Error::::AlreadyInducted); + + Claimant::::insert( + &who, + ClaimantStatus { last_active: cycle_index, status: Nothing }, + ); + + Self::deposit_event(Event::::Inducted { who }); + Ok(Pays::No.into()) + } + /// Register for a payout. /// /// Will only work if we are in the first `RegistrationPeriod` blocks since the cycle @@ -308,7 +342,7 @@ pub mod pallet { /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. #[pallet::weight(T::WeightInfo::add_member())] - #[pallet::call_index(2)] + #[pallet::call_index(3)] pub fn register(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; @@ -340,13 +374,87 @@ pub mod pallet { /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. #[pallet::weight(T::WeightInfo::add_member())] - #[pallet::call_index(3)] - pub fn payout( + #[pallet::call_index(4)] + pub fn payout(origin: OriginFor) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin)?; + Self::do_payout(who.clone(), who)?; + Ok(Pays::No.into()) + } + + /// Request a payout to a secondary account. + /// + /// Will only work if we are after the first `RegistrationPeriod` blocks since the cycle + /// started but by no more than `PayoutPeriod` blocks. + /// + /// - `origin`: A `Signed` origin of an account which is a member of `Members`. + /// - `beneficiary`: The account to receive payment. + #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::call_index(5)] + pub fn payout_other( origin: OriginFor, beneficiary: T::AccountId, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; + Self::do_payout(who, beneficiary)?; + Ok(Pays::No.into()) + } + + /// Update a payment's status; if it failed, alter the state so the payment can be retried. + /// + /// This must be called within the same cycle as the failed payment. It will fail with + /// [Event::NotCurrent] otherwise. + /// + /// - `origin`: A `Signed` origin of an account which is a member of `Members` who has + /// received a payment this cycle. + #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::call_index(6)] + pub fn check_payment(origin: OriginFor) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin)?; + + let mut status = Status::::get().ok_or(Error::::NotStarted)?; + let mut claimant = Claimant::::get(&who).ok_or(Error::::NotInducted)?; + ensure!(claimant.last_active == status.cycle_index, Error::::NotCurrent); + let (id, registered, amount) = match claimant.status { + Attempted { id, registered, amount } => (id, registered, amount), + _ => return Err(Error::::NoClaim.into()), + }; + match T::Paymaster::check_payment(id) { + PaymentStatus::Failure => { + // Payment failed: we reset back to the status prior to payment. + if let Some(amount) = registered { + // Account registered; this makes it simple to roll back and allow retry. + claimant.status = ClaimState::Registered(amount); + } else { + // Account didn't register; we set it to `Nothing` but must decrement + // the `last_active` also to ensure a retry works. + claimant.last_active.saturating_reduce(1u32.into()); + claimant.status = ClaimState::Nothing; + // Since it is not registered, we must walk back our counter for what has + // been paid. + status.total_unregistered_paid.saturating_reduce(amount); + } + }, + PaymentStatus::Success => claimant.status = ClaimState::Nothing, + _ => return Err(Error::::Inconclusive.into()), + } + Claimant::::insert(&who, &claimant); + Status::::put(&status); + Ok(Pays::No.into()) + } + } + + impl, I: 'static> Pallet { + pub fn status() -> Option> { + Status::::get() + } + pub fn last_active(who: &T::AccountId) -> Result, DispatchError> { + Ok(Claimant::::get(&who).ok_or(Error::::NotInducted)?.last_active) + } + pub fn cycle_period() -> T::BlockNumber { + T::RegistrationPeriod::get() + T::PayoutPeriod::get() + } + fn do_payout(who: T::AccountId, beneficiary: T::AccountId) -> DispatchResult { let mut status = Status::::get().ok_or(Error::::NotStarted)?; let mut claimant = Claimant::::get(&who).ok_or(Error::::NotInducted)?; @@ -356,21 +464,17 @@ pub mod pallet { Error::::TooEarly, ); - let payout = match claimant.status { - Attempted { amount, id } if claimant.last_active == status.cycle_index => { - let failed = matches!(T::Paymaster::check_payment(id), PaymentStatus::Failure); - ensure!(failed, Error::::NoClaim); - amount - }, + let (payout, registered) = match claimant.status { Registered(unpaid) if claimant.last_active == status.cycle_index => { // Registered for this cycle. Pay accordingly. - if status.total_registrations <= status.budget { + let payout = if status.total_registrations <= status.budget { // Can pay in full. unpaid } else { // Must be reduced pro-rata Perbill::from_rational(status.budget, status.total_registrations) * unpaid - } + }; + (payout, Some(unpaid)) }, Nothing | Attempted { .. } if claimant.last_active < status.cycle_index => { // Not registered for this cycle. Pay from whatever is left. @@ -386,7 +490,7 @@ pub mod pallet { ensure!(!payout.is_zero(), Error::::ClaimZero); status.total_unregistered_paid.saturating_accrue(payout); - payout + (payout, None) }, _ => return Err(Error::::NoClaim.into()), }; @@ -396,22 +500,13 @@ pub mod pallet { let id = T::Paymaster::pay(&beneficiary, payout).map_err(|()| Error::::PayError)?; - claimant.status = Attempted { amount: payout, id }; + claimant.status = Attempted { registered, id, amount: payout }; Claimant::::insert(&who, &claimant); Status::::put(&status); Self::deposit_event(Event::::Paid { who, beneficiary, amount: payout, id }); - Ok(Pays::No.into()) - } - } - - impl, I: 'static> Pallet { - pub fn status() -> Option> { - Status::::get() - } - pub fn last_active(who: &T::AccountId) -> Result, DispatchError> { - Ok(Claimant::::get(&who).ok_or(Error::::NotInducted)?.last_active) + Ok(()) } } } diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 3f123682118c9..ab785ea166314 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -114,6 +114,12 @@ impl Pay for TestPay { fn check_payment(id: Self::Id) -> PaymentStatus { STATUS.with(|s| s.borrow().get(&id).cloned().unwrap_or(PaymentStatus::Unknown)) } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: &Self::AccountId, _: Self::Balance) {} + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(id: Self::Id) { + set_status(id, PaymentStatus::Failure) + } } thread_local! { @@ -203,7 +209,7 @@ fn basic_stuff() { #[test] fn can_start() { new_test_ext().execute_with(|| { - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); assert_eq!( Salary::status(), Some(StatusType { @@ -220,7 +226,7 @@ fn can_start() { #[test] fn bump_works() { new_test_ext().execute_with(|| { - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); run_to(4); assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); @@ -259,7 +265,7 @@ fn bump_works() { #[test] fn induct_works() { new_test_ext().execute_with(|| { - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotMember); set_rank(1, 1); @@ -275,28 +281,28 @@ fn unregistered_payment_works() { new_test_ext().execute_with(|| { set_rank(1, 1); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NotInducted); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); // No claim on the cycle active during induction. - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::TooEarly); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::TooEarly); run_to(3); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(6); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::TooEarly); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::TooEarly); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 1); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(8); assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); run_to(9); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); run_to(11); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 10)); + assert_ok!(Salary::payout_other(RuntimeOrigin::signed(1), 10)); assert_eq!(paid(1), 1); assert_eq!(paid(10), 1); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); @@ -307,30 +313,171 @@ fn unregistered_payment_works() { fn retry_payment_works() { new_test_ext().execute_with(|| { set_rank(1, 1); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + run_to(6); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + run_to(7); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + + // Payment failed. + unpay(1, 1); + set_status(0, PaymentStatus::Failure); + + assert_eq!(paid(1), 0); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + + // Can't just retry. + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + // Check status. + assert_ok!(Salary::check_payment(RuntimeOrigin::signed(1))); + // Allowed to try again. + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + + assert_eq!(paid(1), 1); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + run_to(8); + assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); + run_to(9); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + run_to(11); + assert_ok!(Salary::payout_other(RuntimeOrigin::signed(1), 10)); + assert_eq!(paid(1), 1); + assert_eq!(paid(10), 1); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + }); +} + +#[test] +fn retry_registered_payment_works() { + new_test_ext().execute_with(|| { + set_rank(1, 1); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(6); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::register(RuntimeOrigin::signed(1))); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); // Payment failed. unpay(1, 1); set_status(0, PaymentStatus::Failure); + assert_eq!(paid(1), 0); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 0); + + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + // Check status. + assert_ok!(Salary::check_payment(RuntimeOrigin::signed(1))); // Allowed to try again. - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + + assert_eq!(paid(1), 1); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 0); + }); +} + +#[test] +fn retry_payment_later_is_not_allowed() { + new_test_ext().execute_with(|| { + set_rank(1, 1); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + run_to(6); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + run_to(7); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + + // Payment failed. + unpay(1, 1); + set_status(0, PaymentStatus::Failure); + + assert_eq!(paid(1), 0); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + + // Can't just retry. + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + + // Next cycle. + run_to(9); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + + // Payment did fail but now too late to retry. + assert_noop!(Salary::check_payment(RuntimeOrigin::signed(1)), Error::::NotCurrent); + // We do get this cycle's payout, but we must wait for the payout period to start. + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::TooEarly); + + run_to(11); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 1); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); + }); +} + +#[test] +fn retry_payment_later_without_bump_is_allowed() { + new_test_ext().execute_with(|| { + set_rank(1, 1); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + run_to(6); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + run_to(7); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + + // Payment failed. + unpay(1, 1); + set_status(0, PaymentStatus::Failure); + + // Next cycle. + run_to(9); + + // Payment did fail but we can still retry as long as we don't `bump`. + assert_ok!(Salary::check_payment(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); + assert_eq!(paid(1), 1); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + }); +} + +#[test] +fn retry_payment_to_other_works() { + new_test_ext().execute_with(|| { + set_rank(1, 1); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); + assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); + run_to(6); + assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + run_to(7); + assert_ok!(Salary::payout_other(RuntimeOrigin::signed(1), 10)); + + // Payment failed. + unpay(10, 1); + set_status(0, PaymentStatus::Failure); + + // Can't just retry. + assert_noop!(Salary::payout_other(RuntimeOrigin::signed(1), 10), Error::::NoClaim); + // Check status. + assert_ok!(Salary::check_payment(RuntimeOrigin::signed(1))); + // Allowed to try again. + assert_ok!(Salary::payout_other(RuntimeOrigin::signed(1), 10)); + + assert_eq!(paid(10), 1); + assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); + + assert_noop!(Salary::payout_other(RuntimeOrigin::signed(1), 10), Error::::NoClaim); run_to(8); assert_noop!(Salary::bump(RuntimeOrigin::signed(1)), Error::::NotYet); run_to(9); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); run_to(11); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 10)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 1); assert_eq!(paid(10), 1); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 1); @@ -342,23 +489,23 @@ fn registered_payment_works() { new_test_ext().execute_with(|| { set_rank(1, 1); assert_noop!(Salary::induct(RuntimeOrigin::signed(1)), Error::::NotStarted); - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NotInducted); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NotInducted); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); // No claim on the cycle active during induction. assert_noop!(Salary::register(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(3); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(5); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); assert_ok!(Salary::register(RuntimeOrigin::signed(1))); assert_eq!(Salary::status().unwrap().total_registrations, 1); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 1); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 0); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::NoClaim); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::NoClaim); run_to(9); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); @@ -366,7 +513,7 @@ fn registered_payment_works() { assert_ok!(Salary::register(RuntimeOrigin::signed(1))); assert_eq!(Salary::status().unwrap().total_registrations, 1); run_to(11); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 2); assert_eq!(Salary::status().unwrap().total_unregistered_paid, 0); }); @@ -375,19 +522,19 @@ fn registered_payment_works() { #[test] fn zero_payment_fails() { new_test_ext().execute_with(|| { - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); set_rank(1, 0); assert_ok!(Salary::induct(RuntimeOrigin::signed(1))); run_to(7); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::ClaimZero); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); }); } #[test] fn unregistered_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); set_rank(1, 2); set_rank(2, 6); set_rank(3, 12); @@ -398,9 +545,9 @@ fn unregistered_bankrupcy_fails_gracefully() { run_to(7); assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); - assert_ok!(Salary::payout(RuntimeOrigin::signed(2), 2)); - assert_ok!(Salary::payout(RuntimeOrigin::signed(3), 3)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); assert_eq!(paid(1), 2); assert_eq!(paid(2), 6); @@ -411,7 +558,7 @@ fn unregistered_bankrupcy_fails_gracefully() { #[test] fn registered_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); set_rank(1, 2); set_rank(2, 6); set_rank(3, 12); @@ -427,9 +574,9 @@ fn registered_bankrupcy_fails_gracefully() { assert_ok!(Salary::register(RuntimeOrigin::signed(3))); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); - assert_ok!(Salary::payout(RuntimeOrigin::signed(2), 2)); - assert_ok!(Salary::payout(RuntimeOrigin::signed(3), 3)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); assert_eq!(paid(1), 1); assert_eq!(paid(2), 3); @@ -440,7 +587,7 @@ fn registered_bankrupcy_fails_gracefully() { #[test] fn mixed_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); set_rank(1, 2); set_rank(2, 6); set_rank(3, 12); @@ -455,9 +602,9 @@ fn mixed_bankrupcy_fails_gracefully() { assert_ok!(Salary::register(RuntimeOrigin::signed(2))); run_to(7); - assert_ok!(Salary::payout(RuntimeOrigin::signed(3), 3)); - assert_ok!(Salary::payout(RuntimeOrigin::signed(2), 2)); - assert_ok!(Salary::payout(RuntimeOrigin::signed(1), 1)); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(1))); assert_eq!(paid(1), 2); assert_eq!(paid(2), 6); @@ -468,7 +615,7 @@ fn mixed_bankrupcy_fails_gracefully() { #[test] fn other_mixed_bankrupcy_fails_gracefully() { new_test_ext().execute_with(|| { - assert_ok!(Salary::bump(RuntimeOrigin::signed(1))); + assert_ok!(Salary::init(RuntimeOrigin::signed(1))); set_rank(1, 2); set_rank(2, 6); set_rank(3, 12); @@ -483,9 +630,9 @@ fn other_mixed_bankrupcy_fails_gracefully() { assert_ok!(Salary::register(RuntimeOrigin::signed(3))); run_to(7); - assert_noop!(Salary::payout(RuntimeOrigin::signed(1), 1), Error::::ClaimZero); - assert_ok!(Salary::payout(RuntimeOrigin::signed(2), 2)); - assert_ok!(Salary::payout(RuntimeOrigin::signed(3), 3)); + assert_noop!(Salary::payout(RuntimeOrigin::signed(1)), Error::::ClaimZero); + assert_ok!(Salary::payout(RuntimeOrigin::signed(2))); + assert_ok!(Salary::payout(RuntimeOrigin::signed(3))); assert_eq!(paid(1), 0); assert_eq!(paid(2), 3); From c9c2ce898824a976ef4a9916d521d616bcae2f51 Mon Sep 17 00:00:00 2001 From: Gav Date: Sun, 26 Feb 2023 17:46:32 +0100 Subject: [PATCH 19/40] Weights --- frame/salary/src/lib.rs | 14 +++---- frame/salary/src/weights.rs | 80 ++++++++++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 6284c3fa72e89..59262592326e6 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -276,7 +276,7 @@ pub mod pallet { /// Start the first payout cycle. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. - #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::weight(T::WeightInfo::init())] #[pallet::call_index(0)] pub fn init(origin: OriginFor) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; @@ -298,7 +298,7 @@ pub mod pallet { /// Move to next payout cycle, assuming that the present block is now within that cycle. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. - #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::weight(T::WeightInfo::bump())] #[pallet::call_index(1)] pub fn bump(origin: OriginFor) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; @@ -318,7 +318,7 @@ pub mod pallet { } /// Induct oneself into the payout system. - #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::weight(T::WeightInfo::induct())] #[pallet::call_index(2)] pub fn induct(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; @@ -341,7 +341,7 @@ pub mod pallet { /// started. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. - #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::weight(T::WeightInfo::register())] #[pallet::call_index(3)] pub fn register(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; @@ -373,7 +373,7 @@ pub mod pallet { /// started but by no more than `PayoutPeriod` blocks. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. - #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::weight(T::WeightInfo::payout())] #[pallet::call_index(4)] pub fn payout(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; @@ -388,7 +388,7 @@ pub mod pallet { /// /// - `origin`: A `Signed` origin of an account which is a member of `Members`. /// - `beneficiary`: The account to receive payment. - #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::weight(T::WeightInfo::payout_other())] #[pallet::call_index(5)] pub fn payout_other( origin: OriginFor, @@ -406,7 +406,7 @@ pub mod pallet { /// /// - `origin`: A `Signed` origin of an account which is a member of `Members` who has /// received a payment this cycle. - #[pallet::weight(T::WeightInfo::add_member())] + #[pallet::weight(T::WeightInfo::check_payment())] #[pallet::call_index(6)] pub fn check_payment(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; diff --git a/frame/salary/src/weights.rs b/frame/salary/src/weights.rs index 172c3ca85c1cf..140e46631e590 100644 --- a/frame/salary/src/weights.rs +++ b/frame/salary/src/weights.rs @@ -45,17 +45,49 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_ranked_collective. pub trait WeightInfo { - fn add_member() -> Weight; + fn init() -> Weight; + fn bump() -> Weight; + fn induct() -> Weight; + fn register() -> Weight; + fn payout() -> Weight; + fn payout_other() -> Weight; + fn check_payment() -> Weight; } /// Weights for pallet_ranked_collective using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - // Storage: RankedCollective Members (r:1 w:1) - // Storage: RankedCollective MemberCount (r:1 w:1) - // Storage: RankedCollective IndexToId (r:0 w:1) - // Storage: RankedCollective IdToIndex (r:0 w:1) - fn add_member() -> Weight { + fn init() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(4 as u64)) + } + fn bump() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(4 as u64)) + } + fn induct() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(4 as u64)) + } + fn register() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(4 as u64)) + } + fn payout() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(4 as u64)) + } + fn payout_other() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(4 as u64)) + } + fn check_payment() -> Weight { Weight::from_ref_time(11_000_000 as u64) .saturating_add(T::DbWeight::get().reads(2 as u64)) .saturating_add(T::DbWeight::get().writes(4 as u64)) @@ -64,11 +96,37 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { - // Storage: RankedCollective Members (r:1 w:1) - // Storage: RankedCollective MemberCount (r:1 w:1) - // Storage: RankedCollective IndexToId (r:0 w:1) - // Storage: RankedCollective IdToIndex (r:0 w:1) - fn add_member() -> Weight { + fn init() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(4 as u64)) + } + fn bump() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(4 as u64)) + } + fn induct() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(4 as u64)) + } + fn register() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(4 as u64)) + } + fn payout() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(4 as u64)) + } + fn payout_other() -> Weight { + Weight::from_ref_time(11_000_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(4 as u64)) + } + fn check_payment() -> Weight { Weight::from_ref_time(11_000_000 as u64) .saturating_add(RocksDbWeight::get().reads(2 as u64)) .saturating_add(RocksDbWeight::get().writes(4 as u64)) From 16bc55ae2dd1d395cb606f25058cc448a5e4bef0 Mon Sep 17 00:00:00 2001 From: Gav Date: Sun, 26 Feb 2023 23:16:49 +0100 Subject: [PATCH 20/40] Introduce Salary benchmark into node --- Cargo.lock | 1 + bin/node/runtime/Cargo.toml | 4 ++++ bin/node/runtime/src/lib.rs | 27 ++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 1e2e9ccfe7c5b..e0b17136b0734 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3571,6 +3571,7 @@ dependencies = [ "pallet-referenda", "pallet-remark", "pallet-root-testing", + "pallet-salary", "pallet-scheduler", "pallet-session", "pallet-session-benchmarking", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 1d2e6f057d517..628790f38ffbf 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -93,6 +93,7 @@ pallet-recovery = { version = "4.0.0-dev", default-features = false, path = "../ pallet-referenda = { version = "4.0.0-dev", default-features = false, path = "../../../frame/referenda" } pallet-remark = { version = "4.0.0-dev", default-features = false, path = "../../../frame/remark" } pallet-root-testing = { version = "1.0.0-dev", default-features = false, path = "../../../frame/root-testing" } +pallet-salary = { version = "4.0.0-dev", default-features = false, path = "../../../frame/salary" } pallet-session = { version = "4.0.0-dev", features = [ "historical" ], path = "../../../frame/session", default-features = false } pallet-session-benchmarking = { version = "4.0.0-dev", path = "../../../frame/session/benchmarking", default-features = false, optional = true } pallet-staking = { version = "4.0.0-dev", default-features = false, path = "../../../frame/staking" } @@ -176,6 +177,7 @@ std = [ "sp-staking/std", "pallet-staking/std", "pallet-state-trie-migration/std", + "pallet-salary/std", "sp-session/std", "pallet-sudo/std", "frame-support/std", @@ -249,6 +251,7 @@ runtime-benchmarks = [ "pallet-referenda/runtime-benchmarks", "pallet-recovery/runtime-benchmarks", "pallet-remark/runtime-benchmarks", + "pallet-salary/runtime-benchmarks", "pallet-session-benchmarking/runtime-benchmarks", "pallet-society/runtime-benchmarks", "pallet-staking/runtime-benchmarks", @@ -306,6 +309,7 @@ try-runtime = [ "pallet-referenda/try-runtime", "pallet-remark/try-runtime", "pallet-root-testing/try-runtime", + "pallet-salary/try-runtime", "pallet-session/try-runtime", "pallet-staking/try-runtime", "pallet-state-trie-migration/try-runtime", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 8f8a7ceef3cfe..595e099b1c480 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -63,7 +63,7 @@ pub use pallet_transaction_payment::{CurrencyAdapter, Multiplier, TargetedFeeAdj use pallet_transaction_payment::{FeeDetails, RuntimeDispatchInfo}; use sp_api::impl_runtime_apis; use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId; -use sp_core::{crypto::KeyTypeId, OpaqueMetadata}; +use sp_core::{crypto::KeyTypeId, ConstU64, OpaqueMetadata}; use sp_inherents::{CheckInherentsResult, InherentData}; use sp_runtime::{ create_runtime_str, @@ -1564,6 +1564,29 @@ impl pallet_uniques::Config for Runtime { type Locker = (); } +parameter_types! { + pub const Budget: Balance = 10_000 * DOLLARS; + pub TreasuryAccount: AccountId = Treasury::account_id(); +} + +pub struct SalaryForRank; +impl Convert for SalaryForRank { + fn convert(a: u16) -> Balance { + Balance::from(a) * 1000 * DOLLARS + } +} + +impl pallet_salary::Config for Runtime { + type WeightInfo = (); + type RuntimeEvent = RuntimeEvent; + type Paymaster = pallet_salary::PayFromAccount; + type Members = RankedCollective; + type ActiveSalaryForRank = SalaryForRank; + type RegistrationPeriod = ConstU32<200>; + type PayoutPeriod = ConstU32<200>; + type Budget = Budget; +} + parameter_types! { pub Features: PalletFeatures = PalletFeatures::all_enabled(); } @@ -1754,6 +1777,7 @@ construct_runtime!( Nis: pallet_nis, Uniques: pallet_uniques, Nfts: pallet_nfts, + Salary: pallet_salary, TransactionStorage: pallet_transaction_storage, VoterList: pallet_bags_list::, StateTrieMigration: pallet_state_trie_migration, @@ -1873,6 +1897,7 @@ mod benches { [pallet_referenda, Referenda] [pallet_recovery, Recovery] [pallet_remark, Remark] + [pallet_salary, Salary] [pallet_scheduler, Scheduler] [pallet_session, SessionBench::] [pallet_staking, Staking] From 2fc7fab216d7db99806c83e37da16dca173fd1c7 Mon Sep 17 00:00:00 2001 From: Gav Date: Sun, 26 Feb 2023 23:17:14 +0100 Subject: [PATCH 21/40] Fix warning --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 595e099b1c480..2ee4302818d6d 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -63,7 +63,7 @@ pub use pallet_transaction_payment::{CurrencyAdapter, Multiplier, TargetedFeeAdj use pallet_transaction_payment::{FeeDetails, RuntimeDispatchInfo}; use sp_api::impl_runtime_apis; use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId; -use sp_core::{crypto::KeyTypeId, ConstU64, OpaqueMetadata}; +use sp_core::{crypto::KeyTypeId, OpaqueMetadata}; use sp_inherents::{CheckInherentsResult, InherentData}; use sp_runtime::{ create_runtime_str, From 42a7927da693946ee00c8a3dcd0b726252ab12a5 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Sun, 26 Feb 2023 22:41:35 +0000 Subject: [PATCH 22/40] ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_salary --- frame/salary/src/weights.rs | 258 ++++++++++++++++++++++++++++-------- 1 file changed, 200 insertions(+), 58 deletions(-) diff --git a/frame/salary/src/weights.rs b/frame/salary/src/weights.rs index 140e46631e590..f678d086dfcd9 100644 --- a/frame/salary/src/weights.rs +++ b/frame/salary/src/weights.rs @@ -1,13 +1,13 @@ // This file is part of Substrate. -// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// Copyright (C) Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -15,26 +15,30 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Autogenerated weights for pallet_ranked_collective +//! Autogenerated weights for pallet_salary //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-05-19, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` -//! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 +//! DATE: 2023-02-26, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `bm3`, CPU: `Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz` +//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: -// /Users/gav/Core/substrate/target/release/substrate +// target/production/substrate // benchmark // pallet -// --pallet -// pallet-ranked-collective -// --extrinsic=* -// --chain=dev // --steps=50 // --repeat=20 -// --output=../../../frame/ranked-collective/src/weights.rs -// --template=../../../.maintain/frame-weight-template.hbs -// --header=../../../HEADER-APACHE2 -// --record-proof +// --extrinsic=* +// --execution=wasm +// --wasm-execution=compiled +// --heap-pages=4096 +// --json-file=/var/lib/gitlab-runner/builds/zyw4fam_/0/parity/mirrors/substrate/.git/.artifacts/bench.json +// --pallet=pallet_salary +// --chain=dev +// --header=./HEADER-APACHE2 +// --output=./frame/salary/src/weights.rs +// --template=./.maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -43,7 +47,7 @@ use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; use sp_std::marker::PhantomData; -/// Weight functions needed for pallet_ranked_collective. +/// Weight functions needed for pallet_salary. pub trait WeightInfo { fn init() -> Weight; fn bump() -> Weight; @@ -54,81 +58,219 @@ pub trait WeightInfo { fn check_payment() -> Weight; } -/// Weights for pallet_ranked_collective using the Substrate node and recommended hardware. +/// Weights for pallet_salary using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) fn init() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(T::DbWeight::get().reads(2 as u64)) - .saturating_add(T::DbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `1120` + // Estimated: `551` + // Minimum execution time: 21_058 nanoseconds. + Weight::from_ref_time(21_381_000) + .saturating_add(Weight::from_proof_size(551)) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) } + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) fn bump() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(T::DbWeight::get().reads(2 as u64)) - .saturating_add(T::DbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `1234` + // Estimated: `551` + // Minimum execution time: 22_272 nanoseconds. + Weight::from_ref_time(22_923_000) + .saturating_add(Weight::from_proof_size(551)) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) } + /// Storage: Salary Status (r:1 w:0) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: RankedCollective Members (r:1 w:0) + /// Proof: RankedCollective Members (max_values: None, max_size: Some(42), added: 2517, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) fn induct() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(T::DbWeight::get().reads(2 as u64)) - .saturating_add(T::DbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `1510` + // Estimated: `5621` + // Minimum execution time: 32_223 nanoseconds. + Weight::from_ref_time(32_663_000) + .saturating_add(Weight::from_proof_size(5621)) + .saturating_add(T::DbWeight::get().reads(3_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) } + /// Storage: RankedCollective Members (r:1 w:0) + /// Proof: RankedCollective Members (max_values: None, max_size: Some(42), added: 2517, mode: MaxEncodedLen) + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) fn register() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(T::DbWeight::get().reads(2 as u64)) - .saturating_add(T::DbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `1616` + // Estimated: `5621` + // Minimum execution time: 38_279 nanoseconds. + Weight::from_ref_time(38_996_000) + .saturating_add(Weight::from_proof_size(5621)) + .saturating_add(T::DbWeight::get().reads(3_u64)) + .saturating_add(T::DbWeight::get().writes(2_u64)) } + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) + /// Storage: RankedCollective Members (r:1 w:0) + /// Proof: RankedCollective Members (max_values: None, max_size: Some(42), added: 2517, mode: MaxEncodedLen) fn payout() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(T::DbWeight::get().reads(2 as u64)) - .saturating_add(T::DbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `2241` + // Estimated: `5621` + // Minimum execution time: 68_868 nanoseconds. + Weight::from_ref_time(70_160_000) + .saturating_add(Weight::from_proof_size(5621)) + .saturating_add(T::DbWeight::get().reads(3_u64)) + .saturating_add(T::DbWeight::get().writes(2_u64)) } + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) + /// Storage: RankedCollective Members (r:1 w:0) + /// Proof: RankedCollective Members (max_values: None, max_size: Some(42), added: 2517, mode: MaxEncodedLen) + /// Storage: System Account (r:1 w:1) + /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) fn payout_other() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(T::DbWeight::get().reads(2 as u64)) - .saturating_add(T::DbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `2189` + // Estimated: `8224` + // Minimum execution time: 68_804 nanoseconds. + Weight::from_ref_time(69_223_000) + .saturating_add(Weight::from_proof_size(8224)) + .saturating_add(T::DbWeight::get().reads(4_u64)) + .saturating_add(T::DbWeight::get().writes(3_u64)) } + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) fn check_payment() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(T::DbWeight::get().reads(2 as u64)) - .saturating_add(T::DbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `880` + // Estimated: `3104` + // Minimum execution time: 19_027 nanoseconds. + Weight::from_ref_time(19_360_000) + .saturating_add(Weight::from_proof_size(3104)) + .saturating_add(T::DbWeight::get().reads(2_u64)) + .saturating_add(T::DbWeight::get().writes(2_u64)) } } // For backwards compatibility and tests impl WeightInfo for () { + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) fn init() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(RocksDbWeight::get().reads(2 as u64)) - .saturating_add(RocksDbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `1120` + // Estimated: `551` + // Minimum execution time: 21_058 nanoseconds. + Weight::from_ref_time(21_381_000) + .saturating_add(Weight::from_proof_size(551)) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) } + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) fn bump() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(RocksDbWeight::get().reads(2 as u64)) - .saturating_add(RocksDbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `1234` + // Estimated: `551` + // Minimum execution time: 22_272 nanoseconds. + Weight::from_ref_time(22_923_000) + .saturating_add(Weight::from_proof_size(551)) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) } + /// Storage: Salary Status (r:1 w:0) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: RankedCollective Members (r:1 w:0) + /// Proof: RankedCollective Members (max_values: None, max_size: Some(42), added: 2517, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) fn induct() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(RocksDbWeight::get().reads(2 as u64)) - .saturating_add(RocksDbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `1510` + // Estimated: `5621` + // Minimum execution time: 32_223 nanoseconds. + Weight::from_ref_time(32_663_000) + .saturating_add(Weight::from_proof_size(5621)) + .saturating_add(RocksDbWeight::get().reads(3_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) } + /// Storage: RankedCollective Members (r:1 w:0) + /// Proof: RankedCollective Members (max_values: None, max_size: Some(42), added: 2517, mode: MaxEncodedLen) + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) fn register() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(RocksDbWeight::get().reads(2 as u64)) - .saturating_add(RocksDbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `1616` + // Estimated: `5621` + // Minimum execution time: 38_279 nanoseconds. + Weight::from_ref_time(38_996_000) + .saturating_add(Weight::from_proof_size(5621)) + .saturating_add(RocksDbWeight::get().reads(3_u64)) + .saturating_add(RocksDbWeight::get().writes(2_u64)) } + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) + /// Storage: RankedCollective Members (r:1 w:0) + /// Proof: RankedCollective Members (max_values: None, max_size: Some(42), added: 2517, mode: MaxEncodedLen) fn payout() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(RocksDbWeight::get().reads(2 as u64)) - .saturating_add(RocksDbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `2241` + // Estimated: `5621` + // Minimum execution time: 68_868 nanoseconds. + Weight::from_ref_time(70_160_000) + .saturating_add(Weight::from_proof_size(5621)) + .saturating_add(RocksDbWeight::get().reads(3_u64)) + .saturating_add(RocksDbWeight::get().writes(2_u64)) } + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) + /// Storage: RankedCollective Members (r:1 w:0) + /// Proof: RankedCollective Members (max_values: None, max_size: Some(42), added: 2517, mode: MaxEncodedLen) + /// Storage: System Account (r:1 w:1) + /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) fn payout_other() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(RocksDbWeight::get().reads(2 as u64)) - .saturating_add(RocksDbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `2189` + // Estimated: `8224` + // Minimum execution time: 68_804 nanoseconds. + Weight::from_ref_time(69_223_000) + .saturating_add(Weight::from_proof_size(8224)) + .saturating_add(RocksDbWeight::get().reads(4_u64)) + .saturating_add(RocksDbWeight::get().writes(3_u64)) } + /// Storage: Salary Status (r:1 w:1) + /// Proof: Salary Status (max_values: Some(1), max_size: Some(56), added: 551, mode: MaxEncodedLen) + /// Storage: Salary Claimant (r:1 w:1) + /// Proof: Salary Claimant (max_values: None, max_size: Some(78), added: 2553, mode: MaxEncodedLen) fn check_payment() -> Weight { - Weight::from_ref_time(11_000_000 as u64) - .saturating_add(RocksDbWeight::get().reads(2 as u64)) - .saturating_add(RocksDbWeight::get().writes(4 as u64)) + // Proof Size summary in bytes: + // Measured: `880` + // Estimated: `3104` + // Minimum execution time: 19_027 nanoseconds. + Weight::from_ref_time(19_360_000) + .saturating_add(Weight::from_proof_size(3104)) + .saturating_add(RocksDbWeight::get().reads(2_u64)) + .saturating_add(RocksDbWeight::get().writes(2_u64)) } } From 7dc1529d662a93e497539a2aea36e23c4ec8c557 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Mon, 27 Feb 2023 15:46:53 +0100 Subject: [PATCH 23/40] Update primitives/arithmetic/src/traits.rs Co-authored-by: Jegor Sidorenko <5252494+jsidorenko@users.noreply.github.com> --- primitives/arithmetic/src/traits.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/arithmetic/src/traits.rs b/primitives/arithmetic/src/traits.rs index 80ba74cccc697..84e3f60aa6a7d 100644 --- a/primitives/arithmetic/src/traits.rs +++ b/primitives/arithmetic/src/traits.rs @@ -153,9 +153,9 @@ impl< /// Arithmetic types do all the usual stuff you'd expect numbers to do. They are guaranteed to /// be able to represent at least `u16` values without loss, hence the trait implies `From` /// and smaller integers. All other conversions are fallible. -pub trait AtLeast16Bit: BaseArithmetic + From + From {} +pub trait AtLeast16Bit: BaseArithmetic + From {} -impl + From> AtLeast16Bit for T {} +impl> AtLeast16Bit for T {} /// A meta trait for arithmetic. Same as [`AtLeast16Bit `], but also bounded to be unsigned. pub trait AtLeast16BitUnsigned: AtLeast16Bit + Unsigned {} From 366267ca70cc5928924ebe21ab15cf699ae3bc6e Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Mon, 27 Feb 2023 15:47:53 +0100 Subject: [PATCH 24/40] Update frame/salary/src/lib.rs Co-authored-by: Jegor Sidorenko <5252494+jsidorenko@users.noreply.github.com> --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 59262592326e6..4ef3ece3d160a 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -67,7 +67,7 @@ pub enum PaymentStatus { // XCM/MultiAsset and made generic over assets. pub trait Pay { /// The type by which we measure units of the currency in which we make payments. - type Balance: AtLeast32BitUnsigned + FullCodec + MaxEncodedLen + TypeInfo + Debug + Copy; + type Balance: Balance; /// The type by which we identify the individuals to whom a payment may be made. type AccountId; /// An identifier given to an individual payment. From 26f1ab04c928277de5bdc0a490454b8c3b0e85f2 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Mon, 27 Feb 2023 15:49:37 +0100 Subject: [PATCH 25/40] Update lib.rs --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 4ef3ece3d160a..06ceadef64bb3 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -33,7 +33,7 @@ use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, - traits::{tokens::fungible, RankedMembers}, + traits::{tokens::{Balance, fungible}, RankedMembers}, RuntimeDebug, }; From 872a047b5ac48d5102346d1545d0295037117675 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Mon, 27 Feb 2023 15:50:35 +0100 Subject: [PATCH 26/40] Update frame/salary/src/lib.rs Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> --- frame/salary/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 06ceadef64bb3..c6b9725d0ca51 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -63,8 +63,8 @@ pub enum PaymentStatus { Failure, } -// Can be implemented by Pot pallet with a fixed Currency impl, but can also be implemented with -// XCM/MultiAsset and made generic over assets. +/// Can be implemented by Pot pallet with a fixed Currency impl, but can also be implemented with +/// XCM/MultiAsset and made generic over assets. pub trait Pay { /// The type by which we measure units of the currency in which we make payments. type Balance: Balance; From e2fd13431d528c264357b8d6e2745a0c47d131cc Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 27 Feb 2023 15:52:24 +0100 Subject: [PATCH 27/40] Docs --- frame/salary/src/lib.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index c6b9725d0ca51..d6a11e108a4f2 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -33,7 +33,10 @@ use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, - traits::{tokens::{Balance, fungible}, RankedMembers}, + traits::{ + tokens::{fungible, Balance}, + RankedMembers, + }, RuntimeDebug, }; @@ -63,7 +66,7 @@ pub enum PaymentStatus { Failure, } -/// Can be implemented by Pot pallet with a fixed Currency impl, but can also be implemented with +/// Can be implemented by `PayFromAccount` using a `fungible` impl, but can also be implemented with /// XCM/MultiAsset and made generic over assets. pub trait Pay { /// The type by which we measure units of the currency in which we make payments. From 8d8a5bfe104da2a1b73af8bbb54fe9068668b1e1 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Mon, 27 Feb 2023 15:58:20 +0100 Subject: [PATCH 28/40] Update frame/salary/src/lib.rs Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index d6a11e108a4f2..bb30d9a7b56e9 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -119,7 +119,7 @@ impl + fungible::Mutate> Pa pub struct StatusType { /// The index of the "current cycle" (i.e. the last cycle being processed). cycle_index: CycleIndex, - /// The first block of the "current cycle" (i.e. the last cycle being processed) + /// The first block of the "current cycle" (i.e. the last cycle being processed). cycle_start: BlockNumber, /// The total budget available for all payments in the current cycle. budget: Balance, From 9849a6546274ce2147ee01ad5ed8843155b0b3a8 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Mon, 27 Feb 2023 15:58:37 +0100 Subject: [PATCH 29/40] Update frame/salary/src/lib.rs Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index bb30d9a7b56e9..7d5a0979e249d 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -229,7 +229,7 @@ pub mod pallet { pub enum Event, I: 'static = ()> { /// A member is inducted into the payroll. Inducted { who: T::AccountId }, - /// The next cycle begins. + /// A member registered for a payout. Registered { who: T::AccountId, amount: BalanceOf }, /// A payment happened. Paid { From 0b4aafa0143c86ff4065b0cc5638938677549b8b Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 27 Feb 2023 16:01:04 +0100 Subject: [PATCH 30/40] Fix --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index d6a11e108a4f2..2fa216e253789 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -278,7 +278,7 @@ pub mod pallet { impl, I: 'static> Pallet { /// Start the first payout cycle. /// - /// - `origin`: A `Signed` origin of an account which is a member of `Members`. + /// - `origin`: A `Signed` origin of an account. #[pallet::weight(T::WeightInfo::init())] #[pallet::call_index(0)] pub fn init(origin: OriginFor) -> DispatchResultWithPostInfo { From a21b1bcc0365166db81e88142c8b129a6b86de24 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 27 Feb 2023 16:01:55 +0100 Subject: [PATCH 31/40] Fixes --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 169e80d8f49eb..6265a94e5b2d4 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -300,7 +300,7 @@ pub mod pallet { /// Move to next payout cycle, assuming that the present block is now within that cycle. /// - /// - `origin`: A `Signed` origin of an account which is a member of `Members`. + /// - `origin`: A `Signed` origin of an account. #[pallet::weight(T::WeightInfo::bump())] #[pallet::call_index(1)] pub fn bump(origin: OriginFor) -> DispatchResultWithPostInfo { From 4c85f02c43fd044ab3aa5985f6d250b0d36af054 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 27 Feb 2023 16:26:04 +0100 Subject: [PATCH 32/40] Fixes --- bin/node/runtime/src/lib.rs | 6 +++--- frame/salary/src/benchmarking.rs | 8 ++++---- frame/salary/src/lib.rs | 27 ++++++++++++++++++++------- frame/salary/src/tests.rs | 2 +- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 8365481e6b677..4498ce8c21b06 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1576,8 +1576,8 @@ parameter_types! { } pub struct SalaryForRank; -impl Convert for SalaryForRank { - fn convert(a: u16) -> Balance { +impl pallet_salary::GetSalary for SalaryForRank { + fn get_salary(a: u16, _: &AccountId) -> Balance { Balance::from(a) * 1000 * DOLLARS } } @@ -1587,7 +1587,7 @@ impl pallet_salary::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Paymaster = pallet_salary::PayFromAccount; type Members = RankedCollective; - type ActiveSalaryForRank = SalaryForRank; + type Salary = SalaryForRank; type RegistrationPeriod = ConstU32<200>; type PayoutPeriod = ConstU32<200>; type Budget = Budget; diff --git a/frame/salary/src/benchmarking.rs b/frame/salary/src/benchmarking.rs index 0d609ab3374f4..339185b37cb7b 100644 --- a/frame/salary/src/benchmarking.rs +++ b/frame/salary/src/benchmarking.rs @@ -36,7 +36,7 @@ fn ensure_member_with_salary, I: 'static>(who: &T::AccountId) { // promote until they have a salary. for _ in 0..255 { let r = T::Members::rank_of(who).expect("prior guard ensures `who` is a member; qed"); - if !T::ActiveSalaryForRank::convert(r).is_zero() { + if !T::Salary::get_salary(r, &who).is_zero() { break } T::Members::promote(who).unwrap(); @@ -106,7 +106,7 @@ mod benchmarks { Salary::::bump(RawOrigin::Signed(caller.clone()).into()).unwrap(); System::::set_block_number(System::::block_number() + T::RegistrationPeriod::get()); - let salary = T::ActiveSalaryForRank::convert(T::Members::rank_of(&caller).unwrap()); + let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller); T::Paymaster::ensure_successful(&caller, salary); #[extrinsic_call] @@ -132,7 +132,7 @@ mod benchmarks { Salary::::bump(RawOrigin::Signed(caller.clone()).into()).unwrap(); System::::set_block_number(System::::block_number() + T::RegistrationPeriod::get()); - let salary = T::ActiveSalaryForRank::convert(T::Members::rank_of(&caller).unwrap()); + let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller); let recipient: T::AccountId = account("recipient", 0, SEED); T::Paymaster::ensure_successful(&recipient, salary); @@ -159,7 +159,7 @@ mod benchmarks { Salary::::bump(RawOrigin::Signed(caller.clone()).into()).unwrap(); System::::set_block_number(System::::block_number() + T::RegistrationPeriod::get()); - let salary = T::ActiveSalaryForRank::convert(T::Members::rank_of(&caller).unwrap()); + let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller); let recipient: T::AccountId = account("recipient", 0, SEED); T::Paymaster::ensure_successful(&recipient, salary); Salary::::payout(RawOrigin::Signed(caller.clone()).into()).unwrap(); diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 6265a94e5b2d4..40f7484ddae6f 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -24,10 +24,7 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; use sp_arithmetic::traits::{Saturating, Zero}; use sp_core::TypedGet; -use sp_runtime::{ - traits::{AtLeast32BitUnsigned, Convert}, - Perbill, -}; +use sp_runtime::{traits::Convert, Perbill}; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; use frame_support::{ @@ -53,6 +50,21 @@ pub use weights::WeightInfo; /// Payroll cycle. pub type Cycle = u32; +/// Retrieve the salary for a member of a particular rank. +pub trait GetSalary { + /// Retrieve the salary for a given rank. The account ID is also supplied in case this changes + /// things. + fn get_salary(rank: Rank, who: &AccountId) -> Balance; +} + +/// Adapter for a rank-to-salary `Convert` implementation into a `GetSalary` implementation. +pub struct ConvertRank(sp_std::marker::PhantomData); +impl> GetSalary for ConvertRank { + fn get_salary(rank: R, _: &A) -> B { + C::convert(rank) + } +} + /// Status for making a payment via the `Pay::pay` trait function. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] pub enum PaymentStatus { @@ -180,8 +192,9 @@ pub mod pallet { /// The maximum payout to be made for a single period to an active member of the given rank. /// /// The benchmarks require that this be non-zero for some rank at most 255. - type ActiveSalaryForRank: Convert< + type Salary: GetSalary< ::Rank, + Self::AccountId, ::Balance, >; @@ -357,7 +370,7 @@ pub mod pallet { Error::::TooLate ); ensure!(claimant.last_active < status.cycle_index, Error::::NoClaim); - let payout = T::ActiveSalaryForRank::convert(rank); + let payout = T::Salary::get_salary(rank, &who); ensure!(!payout.is_zero(), Error::::ClaimZero); claimant.last_active = status.cycle_index; claimant.status = Registered(payout); @@ -482,7 +495,7 @@ pub mod pallet { Nothing | Attempted { .. } if claimant.last_active < status.cycle_index => { // Not registered for this cycle. Pay from whatever is left. let rank = T::Members::rank_of(&who).ok_or(Error::::NotMember)?; - let ideal_payout = T::ActiveSalaryForRank::convert(rank); + let ideal_payout = T::Salary::get_salary(rank, &who); let pot = status .budget diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index ab785ea166314..726ee3f13bbb1 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -174,7 +174,7 @@ impl Config for Test { type RuntimeEvent = RuntimeEvent; type Paymaster = TestPay; type Members = TestClub; - type ActiveSalaryForRank = Identity; + type Salary = ConvertRank; type RegistrationPeriod = ConstU64<2>; type PayoutPeriod = ConstU64<2>; type Budget = Budget; From d5696c1f03d9f1ff2170b089ff88de4c31ec6556 Mon Sep 17 00:00:00 2001 From: Gav Date: Wed, 1 Mar 2023 13:01:41 +0100 Subject: [PATCH 33/40] Move some salary traits stuff to a shared location --- bin/node/runtime/src/lib.rs | 4 ++-- frame/salary/src/lib.rs | 21 +++------------------ frame/salary/src/tests.rs | 2 +- frame/support/src/traits/tokens.rs | 2 +- frame/support/src/traits/tokens/misc.rs | 15 +++++++++++++++ 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 4498ce8c21b06..55be34f21fd62 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -35,7 +35,7 @@ use frame_support::{ fungible::ItemOf, tokens::nonfungibles_v2::Inspect, AsEnsureOriginWithArg, ConstBool, ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse, EqualPrivilegeOnly, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, - U128CurrencyToVote, WithdrawReasons, + U128CurrencyToVote, WithdrawReasons, tokens::GetSalary, }, weights::{ constants::{ @@ -1576,7 +1576,7 @@ parameter_types! { } pub struct SalaryForRank; -impl pallet_salary::GetSalary for SalaryForRank { +impl GetSalary for SalaryForRank { fn get_salary(a: u16, _: &AccountId) -> Balance { Balance::from(a) * 1000 * DOLLARS } diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 40f7484ddae6f..c49584d84ad1b 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -22,7 +22,7 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; -use sp_arithmetic::traits::{Saturating, Zero}; +use sp_arithmetic::traits::{Saturating, Zero, AtLeast16BitUnsigned}; use sp_core::TypedGet; use sp_runtime::{traits::Convert, Perbill}; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; @@ -31,7 +31,7 @@ use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, traits::{ - tokens::{fungible, Balance}, + tokens::{fungible, Balance, GetSalary}, RankedMembers, }, RuntimeDebug, @@ -50,21 +50,6 @@ pub use weights::WeightInfo; /// Payroll cycle. pub type Cycle = u32; -/// Retrieve the salary for a member of a particular rank. -pub trait GetSalary { - /// Retrieve the salary for a given rank. The account ID is also supplied in case this changes - /// things. - fn get_salary(rank: Rank, who: &AccountId) -> Balance; -} - -/// Adapter for a rank-to-salary `Convert` implementation into a `GetSalary` implementation. -pub struct ConvertRank(sp_std::marker::PhantomData); -impl> GetSalary for ConvertRank { - fn get_salary(rank: R, _: &A) -> B { - C::convert(rank) - } -} - /// Status for making a payment via the `Pay::pay` trait function. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] pub enum PaymentStatus { @@ -525,4 +510,4 @@ pub mod pallet { Ok(()) } } -} +} \ No newline at end of file diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 726ee3f13bbb1..518a171baca1b 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -23,7 +23,7 @@ use frame_support::{ assert_noop, assert_ok, pallet_prelude::Weight, parameter_types, - traits::{ConstU32, ConstU64, Everything}, + traits::{ConstU32, ConstU64, Everything, tokens::ConvertRank}, }; use sp_core::H256; use sp_runtime::{ diff --git a/frame/support/src/traits/tokens.rs b/frame/support/src/traits/tokens.rs index 6055fde2180f0..3accc4979341d 100644 --- a/frame/support/src/traits/tokens.rs +++ b/frame/support/src/traits/tokens.rs @@ -29,5 +29,5 @@ pub mod nonfungibles_v2; pub use imbalance::Imbalance; pub use misc::{ AssetId, AttributeNamespace, Balance, BalanceConversion, BalanceStatus, DepositConsequence, - ExistenceRequirement, Locker, WithdrawConsequence, WithdrawReasons, + ExistenceRequirement, Locker, WithdrawConsequence, WithdrawReasons, GetSalary, ConvertRank, }; diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index b4bd4640116a7..e42f386ed6bdf 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -225,3 +225,18 @@ impl Locker for () { false } } + +/// Retrieve the salary for a member of a particular rank. +pub trait GetSalary { + /// Retrieve the salary for a given rank. The account ID is also supplied in case this changes + /// things. + fn get_salary(rank: Rank, who: &AccountId) -> Balance; +} + +/// Adapter for a rank-to-salary `Convert` implementation into a `GetSalary` implementation. +pub struct ConvertRank(sp_std::marker::PhantomData); +impl> GetSalary for ConvertRank { + fn get_salary(rank: R, _: &A) -> B { + C::convert(rank) + } +} From ddd4d3e7ebd1b5bb443277a1a5e5bf700af16628 Mon Sep 17 00:00:00 2001 From: Gav Date: Thu, 2 Mar 2023 15:55:45 +0100 Subject: [PATCH 34/40] Fix --- frame/support/src/traits/tokens/misc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index e42f386ed6bdf..eceee45c6ccfa 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -20,7 +20,7 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use sp_arithmetic::traits::{AtLeast32BitUnsigned, Zero}; use sp_core::RuntimeDebug; -use sp_runtime::{ArithmeticError, DispatchError, TokenError}; +use sp_runtime::{ArithmeticError, DispatchError, TokenError, traits::Convert}; use sp_std::fmt::Debug; /// One of a number of consequences of withdrawing a fungible from an account. From 3704efe9bac7a6451e9af5b1826a7e1db539aad0 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Fri, 3 Mar 2023 11:43:51 +0000 Subject: [PATCH 35/40] Update frame/salary/src/lib.rs Co-authored-by: Oliver Tale-Yazdi --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index c49584d84ad1b..addef2d73745e 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -183,7 +183,7 @@ pub mod pallet { ::Balance, >; - /// The number of block within a cycle which accounts have to register their intent to + /// The number of blocks within a cycle which accounts have to register their intent to /// claim. /// /// The number of blocks between sequential payout cycles is the sum of this and From 93a845dc886adb999bc855e06e509b544bcba19d Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Fri, 3 Mar 2023 11:43:58 +0000 Subject: [PATCH 36/40] Update frame/salary/src/lib.rs Co-authored-by: Oliver Tale-Yazdi --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index addef2d73745e..efe77a7cb9254 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -191,7 +191,7 @@ pub mod pallet { #[pallet::constant] type RegistrationPeriod: Get; - /// The number of block within a cycle which accounts have to claim the payout. + /// The number of blocks within a cycle which accounts have to claim the payout. /// /// The number of blocks between sequential payout cycles is the sum of this and /// `RegistrationPeriod`. From e5c07330a49f53463756229edcbe10be6ac617fb Mon Sep 17 00:00:00 2001 From: Gav Date: Fri, 3 Mar 2023 11:50:12 +0000 Subject: [PATCH 37/40] Mul floor --- frame/salary/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index c49584d84ad1b..0cb42d65fbc87 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -473,7 +473,7 @@ pub mod pallet { unpaid } else { // Must be reduced pro-rata - Perbill::from_rational(status.budget, status.total_registrations) * unpaid + Perbill::from_rational(status.budget, status.total_registrations).mul_floor(unpaid) }; (payout, Some(unpaid)) }, From b3d160e08d6fc9d60317f3b86809ac19d8cfe6e8 Mon Sep 17 00:00:00 2001 From: Gav Date: Fri, 3 Mar 2023 12:48:45 +0000 Subject: [PATCH 38/40] Fix warnings --- bin/node/runtime/src/lib.rs | 9 +++++---- frame/salary/src/lib.rs | 9 +++++---- frame/salary/src/tests.rs | 2 +- frame/support/src/traits/tokens.rs | 5 +++-- frame/support/src/traits/tokens/misc.rs | 2 +- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 55be34f21fd62..d5fd46a4c09a6 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -32,10 +32,11 @@ use frame_support::{ pallet_prelude::Get, parameter_types, traits::{ - fungible::ItemOf, tokens::nonfungibles_v2::Inspect, AsEnsureOriginWithArg, ConstBool, - ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse, EqualPrivilegeOnly, Everything, - Imbalance, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, - U128CurrencyToVote, WithdrawReasons, tokens::GetSalary, + fungible::ItemOf, + tokens::{nonfungibles_v2::Inspect, GetSalary}, + AsEnsureOriginWithArg, ConstBool, ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse, + EqualPrivilegeOnly, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, + LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, WithdrawReasons, }, weights::{ constants::{ diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 24db91da428af..b6e838e672f40 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -22,9 +22,9 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; -use sp_arithmetic::traits::{Saturating, Zero, AtLeast16BitUnsigned}; +use sp_arithmetic::traits::{Saturating, Zero}; use sp_core::TypedGet; -use sp_runtime::{traits::Convert, Perbill}; +use sp_runtime::Perbill; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; use frame_support::{ @@ -473,7 +473,8 @@ pub mod pallet { unpaid } else { // Must be reduced pro-rata - Perbill::from_rational(status.budget, status.total_registrations).mul_floor(unpaid) + Perbill::from_rational(status.budget, status.total_registrations) + .mul_floor(unpaid) }; (payout, Some(unpaid)) }, @@ -510,4 +511,4 @@ pub mod pallet { Ok(()) } } -} \ No newline at end of file +} diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 518a171baca1b..1a98e17218b98 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -23,7 +23,7 @@ use frame_support::{ assert_noop, assert_ok, pallet_prelude::Weight, parameter_types, - traits::{ConstU32, ConstU64, Everything, tokens::ConvertRank}, + traits::{tokens::ConvertRank, ConstU32, ConstU64, Everything}, }; use sp_core::H256; use sp_runtime::{ diff --git a/frame/support/src/traits/tokens.rs b/frame/support/src/traits/tokens.rs index 3accc4979341d..f8dcf68159f1a 100644 --- a/frame/support/src/traits/tokens.rs +++ b/frame/support/src/traits/tokens.rs @@ -28,6 +28,7 @@ pub mod nonfungibles; pub mod nonfungibles_v2; pub use imbalance::Imbalance; pub use misc::{ - AssetId, AttributeNamespace, Balance, BalanceConversion, BalanceStatus, DepositConsequence, - ExistenceRequirement, Locker, WithdrawConsequence, WithdrawReasons, GetSalary, ConvertRank, + AssetId, AttributeNamespace, Balance, BalanceConversion, BalanceStatus, ConvertRank, + DepositConsequence, ExistenceRequirement, GetSalary, Locker, WithdrawConsequence, + WithdrawReasons, }; diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index eceee45c6ccfa..eff65f3620a32 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -20,7 +20,7 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use sp_arithmetic::traits::{AtLeast32BitUnsigned, Zero}; use sp_core::RuntimeDebug; -use sp_runtime::{ArithmeticError, DispatchError, TokenError, traits::Convert}; +use sp_runtime::{traits::Convert, ArithmeticError, DispatchError, TokenError}; use sp_std::fmt::Debug; /// One of a number of consequences of withdrawing a fungible from an account. From b9623a460cd82b5c4a890fb039ac6136c2d5374b Mon Sep 17 00:00:00 2001 From: Gav Date: Fri, 3 Mar 2023 13:07:11 +0000 Subject: [PATCH 39/40] Fix test --- frame/salary/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 1a98e17218b98..8649291225bc3 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -636,6 +636,6 @@ fn other_mixed_bankrupcy_fails_gracefully() { assert_eq!(paid(1), 0); assert_eq!(paid(2), 3); - assert_eq!(paid(3), 7); + assert_eq!(paid(3), 6); }); } From c08c01087895fc53e6f83e4f737618e6219fac81 Mon Sep 17 00:00:00 2001 From: Gav Date: Fri, 3 Mar 2023 15:57:36 +0000 Subject: [PATCH 40/40] Docs --- frame/salary/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index b6e838e672f40..2169952f1a7f3 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -82,8 +82,8 @@ pub trait Pay { /// after this call. Used in benchmarking code. #[cfg(feature = "runtime-benchmarks")] fn ensure_successful(who: &Self::AccountId, amount: Self::Balance); - /// Ensure that a call to [check_payment] with the given parameters will return either [Success] - /// or [Failure]. + /// Ensure that a call to check_payment with the given parameters will return either Success + /// or Failure. #[cfg(feature = "runtime-benchmarks")] fn ensure_concluded(id: Self::Id); } @@ -403,7 +403,7 @@ pub mod pallet { /// Update a payment's status; if it failed, alter the state so the payment can be retried. /// /// This must be called within the same cycle as the failed payment. It will fail with - /// [Event::NotCurrent] otherwise. + /// `Event::NotCurrent` otherwise. /// /// - `origin`: A `Signed` origin of an account which is a member of `Members` who has /// received a payment this cycle.