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 block provider to core-fellowship #6978

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ff8af08
Initial setup
PolkadotDom Nov 16, 2024
7ac7223
testing cont'd
PolkadotDom Nov 16, 2024
5871063
Add to test configs
PolkadotDom Nov 16, 2024
4434d98
Finish switch to generic block provider
PolkadotDom Nov 18, 2024
cdd6c92
Update v0 to v1 migration in preparation of v2
PolkadotDom Nov 18, 2024
b717aaf
Bind block numbers provider to block number
PolkadotDom Nov 18, 2024
0992736
Revert block number bind, add default
PolkadotDom Nov 19, 2024
ea932ff
Set westend configs
PolkadotDom Nov 19, 2024
1c6cc1a
Update comments
PolkadotDom Nov 19, 2024
78c05e6
Update comments
PolkadotDom Nov 19, 2024
c6faf72
Add BlockNumberProvider to kitchen sink
PolkadotDom Nov 19, 2024
d70ff75
Set block number provider to frame_system
PolkadotDom Nov 19, 2024
ed0cad8
fmt
PolkadotDom Nov 19, 2024
b3f5112
Revert "Update v0 to v1 migration in preparation of v2"
PolkadotDom Nov 21, 2024
eb1d578
Update migration
PolkadotDom Nov 23, 2024
8bbd0ce
Core fellowship migration v1 to v2
PolkadotDom Dec 19, 2024
ae916de
Syntax and accessibility changes
PolkadotDom Dec 19, 2024
c9f6274
Use relay chain block number instead of system
PolkadotDom Dec 19, 2024
8f53c22
implement migration for westend, spec version increase, remove old mi…
PolkadotDom Dec 19, 2024
963a121
Merge remote-tracking branch 'upstream/master' into dom/fellowship-co…
PolkadotDom Dec 19, 2024
8b78e1b
Couple fixes
PolkadotDom Dec 20, 2024
1f2a1c4
Update comments
PolkadotDom Dec 20, 2024
2a1b783
Fix benchmarking
PolkadotDom Dec 20, 2024
b9318a7
fmt
PolkadotDom Dec 21, 2024
2e6268c
Merge remote-tracking branch 'upstream/master' into dom/fellowship-co…
PolkadotDom Dec 21, 2024
9992f1d
Prdoc and cargo files
PolkadotDom Dec 21, 2024
9c8b473
Update semver bumps
PolkadotDom Dec 25, 2024
6a1ada8
Update Cargo.lock
PolkadotDom Dec 25, 2024
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
710 changes: 355 additions & 355 deletions Cargo.lock

Large diffs are not rendered by default.

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"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know exactly the process for versioning, but AFAIK we don't bump crate versions on master, instead we provide the prdoc, and from prdoc information crate versions will be changed on release

authors.workspace = true
edition.workspace = true
license = "Apache-2.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ impl pallet_core_fellowship::Config<AmbassadorCoreInstance> for Runtime {
type FastPromoteOrigin = Self::PromoteOrigin;
type EvidenceSize = ConstU32<65536>;
type MaxRank = ConstU32<9>;
type BlockNumberProvider = cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>;
}

pub type AmbassadorSalaryInstance = pallet_salary::Instance2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
weights,
xcm_config::{FellowshipAdminBodyId, LocationToAccountId, TreasurerBodyId, UsdtAssetHub},
AccountId, AssetRate, Balance, Balances, FellowshipReferenda, GovernanceLocation,
ParachainInfo, Preimage, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, Scheduler,
ParachainInfo, Preimage, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, Scheduler, System,
WestendTreasuryAccount, DAYS,
};
use cumulus_primitives_core::ParaId;
Expand Down Expand Up @@ -211,6 +211,7 @@ impl pallet_core_fellowship::Config<FellowshipCoreInstance> for Runtime {
type FastPromoteOrigin = Self::PromoteOrigin;
type EvidenceSize = ConstU32<65536>;
type MaxRank = ConstU32<9>;
type BlockNumberProvider = cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>;
}

pub type FellowshipSalaryInstance = pallet_salary::Instance1;
Expand Down Expand Up @@ -333,5 +334,5 @@ impl pallet_treasury::Config<FellowshipTreasuryInstance> for Runtime {
sp_core::ConstU8<1>,
ConstU32<1000>,
>;
type BlockNumberProvider = crate::System;
type BlockNumberProvider = System;
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 6,
Expand Down Expand Up @@ -751,19 +751,56 @@ pub type UncheckedExtrinsic =
/// All migrations executed on runtime upgrade as a nested tuple of types implementing
/// `OnRuntimeUpgrade`. Included migrations must be idempotent.
type Migrations = (
// unreleased
pallet_collator_selection::migration::v2::MigrationToV2<Runtime>,
// unreleased
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
// unreleased
pallet_core_fellowship::migration::MigrateV0ToV1<Runtime, FellowshipCoreInstance>,
// unreleased
pallet_core_fellowship::migration::MigrateV0ToV1<Runtime, AmbassadorCoreInstance>,
pallet_core_fellowship::migration::MigrateV1ToV2<
Runtime,
BlockNumberConverter,
FellowshipCoreInstance,
>,
pallet_core_fellowship::migration::MigrateV1ToV2<
Runtime,
BlockNumberConverter,
AmbassadorCoreInstance,
>,
);

// Helpers for the core fellowship pallet v1->v2 storage migration.
use sp_runtime::traits::BlockNumberProvider;
type CoreFellowshipLocalBlockNumber = <System as BlockNumberProvider>::BlockNumber;
type CoreFellowshipNewBlockNumber = <cumulus_pallet_parachain_system::RelaychainDataProvider<
Runtime,
> as BlockNumberProvider>::BlockNumber;
pub struct BlockNumberConverter;
impl
pallet_core_fellowship::migration::v2::ConvertBlockNumber<
CoreFellowshipLocalBlockNumber,
CoreFellowshipNewBlockNumber,
> for BlockNumberConverter
{
/// 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: CoreFellowshipLocalBlockNumber,
) -> CoreFellowshipNewBlockNumber {
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.

if local represent a future block number, then the saturation will happen.
In the trait description it is not specified that local must be past.
I guess it works in our case because the pallet don't store any future block number.

Maybe we can handle the general case with a comparison if local<block_number ....
All similar migration will make use of a similar trait, and other pallet might store a future block number.

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: CoreFellowshipLocalBlockNumber,
) -> CoreFellowshipNewBlockNumber {
local
}
}

/// Executive: handles dispatch to the various modules.
pub type Executive = frame_executive::Executive<
Runtime,
Expand Down
17 changes: 17 additions & 0 deletions prdoc/pr_6978.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
title: 'Adds BlockNumberProvider to pallet-core-fellowship'
doc:
- audience: Runtime Dev
description: |-
This PR adds a parameter 'BlockNumberProvider' to the pallet-core-fellowship
config such that a block provider can be set for use in the pallet. This would
usually be the local system pallet or the appropriate relay chain. Previously
it defaulted to the local system pallet.
Copy link
Contributor

Choose a reason for hiding this comment

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

westend is a testnet, should we notify Runtime User that westend collective fellowship pallet migrated from using parachain block number to relay chain block number?

(More generally I wonder how easy it is for tools to differentiate between both types. Some tools have to be careful and get the correct meaning for the block number.)

crates:
- name: pallet-core-fellowship
bump: major
- name: sp-runtime
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,
apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
system_version: 1,
Expand Down Expand Up @@ -1990,6 +1990,7 @@ impl pallet_core_fellowship::Config for Runtime {
type FastPromoteOrigin = Self::PromoteOrigin;
type EvidenceSize = ConstU32<16_384>;
type MaxRank = ConstU32<9>;
type BlockNumberProvider = System;
}

parameter_types! {
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/core-fellowship/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pallet-core-fellowship"
version = "12.0.0"
version = "13.0.0"
authors.workspace = true
edition.workspace = true
license = "Apache-2.0"
Expand Down
21 changes: 13 additions & 8 deletions substrate/frame/core-fellowship/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! Salary pallet benchmarking.
//! Core Fellowship pallet benchmarking.

#![cfg(feature = "runtime-benchmarks")]

use super::*;
use crate::Pallet as CoreFellowship;
use crate::{BlockNumberFor as CoreFellowshipBlockNumberFor, Pallet as CoreFellowship};

use alloc::{boxed::Box, vec};
use frame_benchmarking::v2::*;
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use frame_system::RawOrigin;
use sp_arithmetic::traits::Bounded;
use sp_runtime::traits::BlockNumberProvider;

const SEED: u32 = 0;

Expand All @@ -35,6 +36,10 @@ type BenchResult = Result<(), BenchmarkError>;
mod benchmarks {
use super::*;

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

fn ensure_evidence<T: Config<I>, I: 'static>(who: &T::AccountId) -> BenchResult {
let evidence = BoundedVec::try_from(vec![0; Evidence::<T, I>::bound()]).unwrap();
let wish = Wish::Retention;
Expand Down Expand Up @@ -132,7 +137,7 @@ mod benchmarks {
let member = make_member::<T, I>(0)?;

// Set it to the max value to ensure that any possible auto-demotion period has passed.
frame_system::Pallet::<T>::set_block_number(BlockNumberFor::<T>::max_value());
set_block_number::<T, I>(CoreFellowshipBlockNumberFor::<T, I>::max_value());
ensure_evidence::<T, I>(&member)?;
assert!(Member::<T, I>::contains_key(&member));

Expand All @@ -151,7 +156,7 @@ mod benchmarks {
let member = make_member::<T, I>(2)?;

// Set it to the max value to ensure that any possible auto-demotion period has passed.
frame_system::Pallet::<T>::set_block_number(BlockNumberFor::<T>::max_value());
set_block_number::<T, I>(CoreFellowshipBlockNumberFor::<T, I>::max_value());
ensure_evidence::<T, I>(&member)?;
assert!(Member::<T, I>::contains_key(&member));
assert_eq!(T::Members::rank_of(&member), Some(2));
Expand Down Expand Up @@ -200,7 +205,7 @@ mod benchmarks {
let member = make_member::<T, I>(1)?;

// Set it to the max value to ensure that any possible auto-demotion period has passed.
frame_system::Pallet::<T>::set_block_number(BlockNumberFor::<T>::max_value());
set_block_number::<T, I>(CoreFellowshipBlockNumberFor::<T, I>::max_value());
ensure_evidence::<T, I>(&member)?;

#[extrinsic_call]
Expand Down Expand Up @@ -263,9 +268,9 @@ mod benchmarks {
#[benchmark]
fn approve() -> Result<(), BenchmarkError> {
let member = make_member::<T, I>(1)?;
let then = frame_system::Pallet::<T>::block_number();
let then = T::BlockNumberProvider::current_block_number();
let now = then.saturating_plus_one();
frame_system::Pallet::<T>::set_block_number(now);
set_block_number::<T, I>(now);
ensure_evidence::<T, I>(&member)?;

assert_eq!(Member::<T, I>::get(&member).unwrap().last_proof, then);
Expand Down
52 changes: 30 additions & 22 deletions substrate/frame/core-fellowship/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,11 @@ impl<
#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)]
pub struct MemberStatus<BlockNumber> {
/// Are they currently active?
is_active: bool,
pub is_active: bool,
/// The block number at which we last promoted them.
last_promotion: BlockNumber,
pub last_promotion: BlockNumber,
/// The last time a member was demoted, promoted or proved their rank.
last_proof: BlockNumber,
pub last_proof: BlockNumber,
}

#[frame_support::pallet]
Expand All @@ -169,8 +169,9 @@ pub mod pallet {
traits::{tokens::GetSalary, EnsureOrigin},
};
use frame_system::{ensure_root, pallet_prelude::*};
use sp_runtime::traits::BlockNumberProvider;
/// The in-code storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
Expand Down Expand Up @@ -225,27 +226,34 @@ pub mod pallet {
/// Increasing this value is supported, but decreasing it may lead to a broken state.
#[pallet::constant]
type MaxRank: Get<u32>;

/// Provides the current block number.
///
/// This is usually `cumulus_pallet_parachain_system::RelaychainDataProvider` if a
/// parachain, or `frame_system::Pallet` if a solochain.
type BlockNumberProvider: BlockNumberProvider;
}

pub type BlockNumberFor<T, I = ()> =
<<T as Config<I>>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;
pub type ParamsOf<T, I> =
ParamsType<<T as Config<I>>::Balance, BlockNumberFor<T>, <T as Config<I>>::MaxRank>;
ParamsType<<T as Config<I>>::Balance, BlockNumberFor<T, I>, <T as Config<I>>::MaxRank>;
pub type PartialParamsOf<T, I> = ParamsType<
Option<<T as Config<I>>::Balance>,
Option<BlockNumberFor<T>>,
Option<BlockNumberFor<T, I>>,
<T as Config<I>>::MaxRank,
>;
pub type MemberStatusOf<T> = MemberStatus<BlockNumberFor<T>>;
pub type MemberStatusOf<T, I> = MemberStatus<BlockNumberFor<T, I>>;
pub type RankOf<T, I> = <<T as Config<I>>::Members as RankedMembers>::Rank;

/// The overall status of the system.
#[pallet::storage]
pub(super) type Params<T: Config<I>, I: 'static = ()> =
StorageValue<_, ParamsOf<T, I>, ValueQuery>;
pub type Params<T: Config<I>, I: 'static = ()> = StorageValue<_, ParamsOf<T, I>, ValueQuery>;

/// The status of a claimant.
#[pallet::storage]
pub(super) type Member<T: Config<I>, I: 'static = ()> =
StorageMap<_, Twox64Concat, T::AccountId, MemberStatusOf<T>, OptionQuery>;
pub type Member<T: Config<I>, I: 'static = ()> =
StorageMap<_, Twox64Concat, T::AccountId, MemberStatusOf<T, I>, OptionQuery>;

/// Some evidence together with the desired outcome for which it was presented.
#[pallet::storage]
Expand Down Expand Up @@ -341,13 +349,13 @@ pub mod pallet {
};

if demotion_period.is_zero() {
return Err(Error::<T, I>::NothingDoing.into())
return Err(Error::<T, I>::NothingDoing.into());
}

let demotion_block = member.last_proof.saturating_add(demotion_period);

// Ensure enough time has passed.
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
if now >= demotion_block {
T::Members::demote(&who)?;
let maybe_to_rank = T::Members::rank_of(&who);
Expand All @@ -361,7 +369,7 @@ pub mod pallet {
Event::<T, I>::Offboarded { who }
};
Self::deposit_event(event);
return Ok(Pays::No.into())
return Ok(Pays::No.into());
}

Err(Error::<T, I>::NothingDoing.into())
Expand Down Expand Up @@ -426,7 +434,7 @@ pub mod pallet {
ensure!(rank == at_rank, Error::<T, I>::UnexpectedRank);
let mut member = Member::<T, I>::get(&who).ok_or(Error::<T, I>::NotTracked)?;

member.last_proof = frame_system::Pallet::<T>::block_number();
member.last_proof = T::BlockNumberProvider::current_block_number();
Member::<T, I>::insert(&who, &member);

Self::dispose_evidence(who.clone(), at_rank, Some(at_rank));
Expand All @@ -450,7 +458,7 @@ pub mod pallet {
ensure!(T::Members::rank_of(&who).is_none(), Error::<T, I>::Ranked);

T::Members::induct(&who)?;
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
Member::<T, I>::insert(
&who,
MemberStatus { is_active: true, last_promotion: now, last_proof: now },
Expand Down Expand Up @@ -483,7 +491,7 @@ pub mod pallet {
);

let mut member = Member::<T, I>::get(&who).ok_or(Error::<T, I>::NotTracked)?;
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();

let params = Params::<T, I>::get();
let rank_index = Self::rank_to_index(to_rank).ok_or(Error::<T, I>::InvalidRank)?;
Expand Down Expand Up @@ -526,7 +534,7 @@ pub mod pallet {
ensure!(to_rank > curr_rank, Error::<T, I>::UnexpectedRank);

let mut member = Member::<T, I>::get(&who).ok_or(Error::<T, I>::NotTracked)?;
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
member.last_promotion = now;
member.last_proof = now;

Expand Down Expand Up @@ -599,7 +607,7 @@ pub mod pallet {
ensure!(!Member::<T, I>::contains_key(&who), Error::<T, I>::AlreadyInducted);
let rank = T::Members::rank_of(&who).ok_or(Error::<T, I>::Unranked)?;

let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
Member::<T, I>::insert(
&who,
MemberStatus { is_active: true, last_promotion: 0u32.into(), last_proof: now },
Expand Down Expand Up @@ -739,15 +747,15 @@ impl<T: Config<I>, I: 'static> RankedMembersSwapHandler<T::AccountId, u16> for P
fn swapped(old: &T::AccountId, new: &T::AccountId, _rank: u16) {
if old == new {
defensive!("Should not try to swap with self");
return
return;
}
if !Member::<T, I>::contains_key(old) {
defensive!("Should not try to swap non-member");
return
return;
}
if Member::<T, I>::contains_key(new) {
defensive!("Should not try to overwrite existing member");
return
return;
}

if let Some(member) = Member::<T, I>::take(old) {
Expand Down
Loading
Loading