Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add BlockNumberProvider to salary pallet config #7000

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
6481857
Update salary logic to use BlockNumberProvider
PolkadotDom Dec 21, 2024
0e650b6
Salary v0 to v1 migration
PolkadotDom Dec 23, 2024
4359f23
Set block provider to system for now
PolkadotDom Dec 23, 2024
7752f6c
Set to block number provider to relay chain
PolkadotDom Dec 23, 2024
52790c5
Fix storage version issue
PolkadotDom Dec 23, 2024
cb119b0
Add migrations
PolkadotDom Dec 23, 2024
bdd0727
Remove unused import
PolkadotDom Dec 23, 2024
77741b3
fmt
PolkadotDom Dec 23, 2024
a31302c
Add alloc crate back in
PolkadotDom Dec 24, 2024
a071803
Fix noop issue
PolkadotDom Dec 24, 2024
47e7079
Update spec version
PolkadotDom Dec 24, 2024
331469e
Add space
PolkadotDom Dec 25, 2024
5cea0ca
Remove testing functions
PolkadotDom Dec 25, 2024
30cc991
Remove unused imports
PolkadotDom Dec 25, 2024
25053cc
Update prdoc and cargo tomls
PolkadotDom Dec 25, 2024
e53d8b9
Cargo reversions, one doc
PolkadotDom Jan 6, 2025
0de9976
Add to docs for migration block converter
PolkadotDom Jan 6, 2025
b208d52
Update migration.rs
PolkadotDom Jan 7, 2025
606812b
Some more doc and naming updates, migration checks WIP
PolkadotDom Jan 7, 2025
1cbf8bd
fmt
PolkadotDom Jan 7, 2025
0a47eb6
fix migration checks
PolkadotDom Jan 7, 2025
cbc90eb
fmt
PolkadotDom Jan 7, 2025
724795d
Allow for future moments, some wording and syntax
PolkadotDom Jan 7, 2025
7a77e7e
Add runtime user to prdoc
PolkadotDom Jan 8, 2025
519a71d
Fix missing config type issue
PolkadotDom Jan 9, 2025
8d4cf70
Update pr_7000.prdoc
PolkadotDom Jan 9, 2025
d8e7a4e
Documentation fixes
PolkadotDom Jan 9, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions Cargo.lock

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

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "collectives-westend-runtime"
version = "3.0.0"
version = "3.1.0"
PolkadotDom marked this conversation as resolved.
Show resolved Hide resolved
authors.workspace = true
edition.workspace = true
license = "Apache-2.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,4 +270,6 @@ impl pallet_salary::Config<AmbassadorSalaryInstance> for Runtime {
type PayoutPeriod = ConstU32<{ 15 * DAYS }>;
// Total monthly salary budget.
type Budget = ConstU128<{ 10_000 * DOLLARS }>;

type BlockNumberProvider = cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ impl pallet_salary::Config<FellowshipSalaryInstance> for Runtime {
type PayoutPeriod = ConstU32<{ 15 * DAYS }>;
// Total monthly salary budget.
type Budget = ConstU128<{ 100_000 * USDT_UNITS }>;

type BlockNumberProvider = cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>;
}

parameter_types! {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@ extern crate alloc;
pub use ambassador::pallet_ambassador_origins;

use alloc::{vec, vec::Vec};
use ambassador::AmbassadorCoreInstance;
use ambassador::{AmbassadorCoreInstance, AmbassadorSalaryInstance};
use cumulus_pallet_parachain_system::RelayNumberMonotonicallyIncreases;
use fellowship::{pallet_fellowship_origins, Fellows, FellowshipCoreInstance};
use fellowship::{
pallet_fellowship_origins, Fellows, FellowshipCoreInstance, FellowshipSalaryInstance,
};
use impls::{AllianceProposalProvider, EqualOrGreatestRootCmp};
use sp_api::impl_runtime_apis;
use sp_core::{crypto::KeyTypeId, OpaqueMetadata};
Expand Down Expand Up @@ -126,7 +128,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: alloc::borrow::Cow::Borrowed("collectives-westend"),
impl_name: alloc::borrow::Cow::Borrowed("collectives-westend"),
authoring_version: 1,
spec_version: 1_017_001,
spec_version: 1_018_000,
PolkadotDom marked this conversation as resolved.
Show resolved Hide resolved
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 6,
Expand Down Expand Up @@ -762,8 +764,52 @@ type Migrations = (
pallet_core_fellowship::migration::MigrateV0ToV1<Runtime, FellowshipCoreInstance>,
// unreleased
pallet_core_fellowship::migration::MigrateV0ToV1<Runtime, AmbassadorCoreInstance>,
// unreleased
pallet_salary::migration::MigrateV0ToV1<
Runtime,
SalaryBlockNumberConverter,
FellowshipSalaryInstance,
>,
pallet_salary::migration::MigrateV0ToV1<
Runtime,
SalaryBlockNumberConverter,
AmbassadorSalaryInstance,
>,
);

// Helpers for the salary pallet v0->v1 storage migration.
use sp_runtime::traits::BlockNumberProvider;
type SalaryLocalBlockNumber = <System as BlockNumberProvider>::BlockNumber;
type SalaryNewBlockNumber = <cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>
as BlockNumberProvider>::BlockNumber;
pub struct SalaryBlockNumberConverter;
impl pallet_salary::migration::v1::ConvertBlockNumber<SalaryLocalBlockNumber, SalaryNewBlockNumber>
for SalaryBlockNumberConverter
{
/// Simply convert the types. Cycle index storage item uses block number but is agnostic to the
/// time that denotes for instance
fn convert(local: SalaryLocalBlockNumber) -> SalaryNewBlockNumber {
local
}

/// The equivalent moment in time from the perspective of the relay chain, starting from a
/// local moment in time (system block number)
fn equivalent_moment_in_time(local: SalaryLocalBlockNumber) -> SalaryNewBlockNumber {
let block_number = System::block_number();
let local_duration = block_number.saturating_sub(local);
Copy link
Contributor

Choose a reason for hiding this comment

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

same maybe better to make it general if local > block_number, to be safe if this code gets copied and pasted.

let relay_duration = Self::equivalent_block_duration(local_duration); //6s to 6s
let relay_block_number = ParachainSystem::last_relay_block_number();
relay_block_number.saturating_sub(relay_duration)
}

/// The equivalent duration from the perspective of the relay chain, starting from
/// a local duration (number of block). Identity function for Westend, since both
/// relay and collectives chain run 6s block times
fn equivalent_block_duration(local: SalaryLocalBlockNumber) -> SalaryNewBlockNumber {
local
}
}

/// Executive: handles dispatch to the various modules.
pub type Executive = frame_executive::Executive<
Runtime,
Expand Down
15 changes: 15 additions & 0 deletions prdoc/pr_7000.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
title: 'Adds BlockNumberProvider to pallet-core-fellowship'
doc:
- audience: Runtime Dev
description: |-
This PR adds a parameter 'BlockNumberProvider' to the pallet-salary
config such that a block provider can be set for use in the pallet. This would
usually be the frame system pallet or the appropriate relay chain. Previously
it defaulted to the frame system pallet.
crates:
- name: pallet-salary
bump: major
- name: collectives-westend-runtime
bump: minor
- name: kitchensink-runtime
bump: patch
2 changes: 1 addition & 1 deletion substrate/bin/node/runtime/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "kitchensink-runtime"
version = "3.0.0-dev"
version = "3.0.1-dev"
authors.workspace = true
description = "Substrate node kitchensink runtime."
edition.workspace = true
Expand Down
3 changes: 2 additions & 1 deletion substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 268,
impl_version: 0,
impl_version: 1,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
impl_version: 1,
impl_version: 0,

The kitchensink runtime needs to bumping.

Copy link
Contributor Author

@PolkadotDom PolkadotDom Jan 6, 2025

Choose a reason for hiding this comment

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

The kitchensink runtime needs to bumping.

Assuming 'no' bumping was meant - is this because its also bumped automatically on release like the spec_version or because the change to the runtime was sufficient to warrant a bump?

In any case, will revert. Just curious!

Copy link
Contributor

Choose a reason for hiding this comment

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

impl_version is when it is a change in the code without a change in the specification in the runtime.

I guess, both spec_version and impl_version will be bumped automatically using the prdoc information, but I can't confirm

apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
system_version: 1,
Expand Down Expand Up @@ -1976,6 +1976,7 @@ impl pallet_salary::Config for Runtime {
type RegistrationPeriod = ConstU32<200>;
type PayoutPeriod = ConstU32<200>;
type Budget = Budget;
type BlockNumberProvider = System;
}

impl pallet_core_fellowship::Config for Runtime {
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/salary/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pallet-salary"
version = "13.0.0"
version = "14.0.0"
authors.workspace = true
edition.workspace = true
license = "Apache-2.0"
Expand Down
29 changes: 19 additions & 10 deletions substrate/frame/salary/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@
#![cfg(feature = "runtime-benchmarks")]

use super::*;
use crate::Pallet as Salary;
use crate::{BlockNumberFor as SalaryBlockNumberFor, Pallet as Salary};

use frame_benchmarking::v2::*;
use frame_system::{Pallet as System, RawOrigin};
use frame_system::RawOrigin;
use sp_core::Get;
use sp_runtime::traits::BlockNumberProvider;

const SEED: u32 = 0;

Expand All @@ -47,6 +48,14 @@ fn ensure_member_with_salary<T: Config<I>, I: 'static>(who: &T::AccountId) {
mod benchmarks {
use super::*;

fn get_block_number<T: Config<I>, I: 'static>() -> SalaryBlockNumberFor<T, I> {
T::BlockNumberProvider::current_block_number()
}

fn set_block_number<T: Config<I>, I: 'static>(n: SalaryBlockNumberFor<T, I>) {
T::BlockNumberProvider::set_block_number(n);
}

#[benchmark]
fn init() {
let caller: T::AccountId = whitelisted_caller();
Expand All @@ -61,7 +70,7 @@ mod benchmarks {
fn bump() {
let caller: T::AccountId = whitelisted_caller();
Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());

#[extrinsic_call]
_(RawOrigin::Signed(caller.clone()));
Expand All @@ -87,7 +96,7 @@ mod benchmarks {
ensure_member_with_salary::<T, I>(&caller);
Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
Salary::<T, I>::bump(RawOrigin::Signed(caller.clone()).into()).unwrap();

#[extrinsic_call]
Expand All @@ -102,9 +111,9 @@ mod benchmarks {
ensure_member_with_salary::<T, I>(&caller);
Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
Salary::<T, I>::bump(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + T::RegistrationPeriod::get());
set_block_number::<T, I>(get_block_number::<T, I>() + T::RegistrationPeriod::get());

let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller);
T::Paymaster::ensure_successful(&caller, (), salary);
Expand All @@ -128,9 +137,9 @@ mod benchmarks {
ensure_member_with_salary::<T, I>(&caller);
Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
Salary::<T, I>::bump(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + T::RegistrationPeriod::get());
set_block_number::<T, I>(get_block_number::<T, I>() + T::RegistrationPeriod::get());

let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller);
let recipient: T::AccountId = account("recipient", 0, SEED);
Expand All @@ -155,9 +164,9 @@ mod benchmarks {
ensure_member_with_salary::<T, I>(&caller);
Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
Salary::<T, I>::bump(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + T::RegistrationPeriod::get());
set_block_number::<T, I>(get_block_number::<T, I>() + T::RegistrationPeriod::get());

let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller);
let recipient: T::AccountId = account("recipient", 0, SEED);
Expand Down
Loading
Loading