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

Adds BlockNumberProvider in alliance, collective and identity pallets #6282

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ impl pallet_collective::Config<AllianceCollective> for Runtime {
type DisapproveOrigin = EnsureRoot<Self::AccountId>;
type KillOrigin = EnsureRoot<Self::AccountId>;
type Consideration = ();
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

pub const MAX_FELLOWS: u32 = ALLIANCE_MAX_MEMBERS;
Expand Down Expand Up @@ -595,6 +596,7 @@ impl pallet_alliance::Config for Runtime {
type MaxMembersCount = ConstU32<ALLIANCE_MAX_MEMBERS>;
type AllyDeposit = AllyDeposit;
type WeightInfo = weights::pallet_alliance::WeightInfo<Runtime>;
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

parameter_types! {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ impl pallet_identity::Config for Runtime {
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

/// The fields that we use to identify the owner of an account with. Each corresponds to a field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ impl pallet_identity::Config for Runtime {
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

/// The fields that we use to identify the owner of an account with. Each corresponds to a field
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/common/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ impl pallet_identity::Config for Test {
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
type WeightInfo = ();
type BlockNumberProvider = frame_system::Pallet<Test>;
}

impl identity_migrator::Config for Test {
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ impl pallet_identity::Config for Runtime {
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

impl pallet_utility::Config for Runtime {
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,7 @@ impl pallet_identity::Config for Runtime {
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

impl pallet_utility::Config for Runtime {
Expand Down
25 changes: 25 additions & 0 deletions prdoc/pr_6282.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
title: Adds `BlockNumberProvider` in alliance, collective and identity pallets

doc:
- audience: Runtime Dev
description: |
This PR adds the ability for these pallets to specify their source of the block number.
This is useful when these pallets are migrated from the relay chain to a parachain and
vice versa.

This change is backwards compatible:
1. If the `BlockNumberProvider` continues to use the system pallet's block number
2. When a pallet deployed on the relay chain is moved to a parachain, but still uses the
relay chain's block number

However, we would need migrations if the deployed pallets are upgraded on an existing parachain,
and the `BlockNumberProvider` uses the relay chain block number.

crates:
- name: pallet-alliance
bump: major
- name: pallet-collective
bump: major
- name: pallet-identity
bump: major

5 changes: 5 additions & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1148,6 +1148,7 @@ impl pallet_collective::Config<CouncilCollective> for Runtime {
>,
u32,
>;
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

parameter_types! {
Expand Down Expand Up @@ -1212,6 +1213,7 @@ impl pallet_collective::Config<TechnicalCollective> for Runtime {
type DisapproveOrigin = EnsureRoot<Self::AccountId>;
type KillOrigin = EnsureRoot<Self::AccountId>;
type Consideration = ();
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

type EnsureRootOrHalfCouncil = EitherOfDiverse<
Expand Down Expand Up @@ -1598,6 +1600,7 @@ impl pallet_identity::Config for Runtime {
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
type WeightInfo = pallet_identity::weights::SubstrateWeight<Runtime>;
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

parameter_types! {
Expand Down Expand Up @@ -2101,6 +2104,7 @@ impl pallet_collective::Config<AllianceCollective> for Runtime {
type DisapproveOrigin = EnsureRoot<Self::AccountId>;
type KillOrigin = EnsureRoot<Self::AccountId>;
type Consideration = ();
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

parameter_types! {
Expand Down Expand Up @@ -2144,6 +2148,7 @@ impl pallet_alliance::Config for Runtime {
type AllyDeposit = AllyDeposit;
type WeightInfo = pallet_alliance::weights::SubstrateWeight<Runtime>;
type RetirementPeriod = RetirementPeriod;
type BlockNumberProvider = frame_system::Pallet<Runtime>;
}

impl frame_benchmarking_pallet_pov::Config for Runtime {
Expand Down
9 changes: 6 additions & 3 deletions substrate/frame/alliance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;
use sp_runtime::{
traits::{Dispatchable, Saturating, StaticLookup, Zero},
traits::{BlockNumberProvider, Dispatchable, Saturating, StaticLookup, Zero},
DispatchError, RuntimeDebug,
};

Expand Down Expand Up @@ -308,6 +308,9 @@ pub mod pallet {
/// The number of blocks a member must wait between giving a retirement notice and retiring.
/// Supposed to be greater than time required to `kick_member`.
type RetirementPeriod: Get<BlockNumberFor<Self>>;

/// Provider for the block number. Normally this is the `frame_system` pallet.
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as some other PR. I think this is fixing the BlockNumber to be the system block number. But there is no need for this.

The block number could be the one of the relay chain, in which case it doesn't need to be = BlockNumberFor<Self>.

}

#[pallet::error]
Expand Down Expand Up @@ -763,7 +766,7 @@ pub mod pallet {
Self::add_member(&who, MemberRole::Retiring)?;
<RetiringMembers<T, I>>::insert(
&who,
frame_system::Pallet::<T>::block_number()
T::BlockNumberProvider::current_block_number()
.saturating_add(T::RetirementPeriod::get()),
);

Expand All @@ -781,7 +784,7 @@ pub mod pallet {
let retirement_period_end = RetiringMembers::<T, I>::get(&who)
.ok_or(Error::<T, I>::RetirementNoticeNotGiven)?;
ensure!(
frame_system::Pallet::<T>::block_number() >= retirement_period_end,
T::BlockNumberProvider::current_block_number() >= retirement_period_end,
Error::<T, I>::RetirementPeriodNotPassed
);

Expand Down
3 changes: 3 additions & 0 deletions substrate/frame/alliance/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ impl pallet_collective::Config<AllianceCollective> for Test {
type DisapproveOrigin = EnsureRoot<Self::AccountId>;
type KillOrigin = EnsureRoot<Self::AccountId>;
type Consideration = ();
type BlockNumberProvider = frame_system::Pallet<Test>;
}

parameter_types! {
Expand Down Expand Up @@ -124,6 +125,7 @@ impl pallet_identity::Config for Test {
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
type WeightInfo = ();
type BlockNumberProvider = frame_system::Pallet<Test>;
}

#[derive(Clone, Debug, Encode, Decode, PartialEq, Eq, TypeInfo)]
Expand Down Expand Up @@ -233,6 +235,7 @@ impl Config for Test {
type AllyDeposit = AllyDeposit;
type WeightInfo = ();
type RetirementPeriod = RetirementPeriod;
type BlockNumberProvider = frame_system::Pallet<Test>;
}

type Block = frame_system::mocking::MockBlock<Test>;
Expand Down
12 changes: 9 additions & 3 deletions substrate/frame/collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use core::{marker::PhantomData, result};
use scale_info::TypeInfo;
use sp_io::storage;
use sp_runtime::{
traits::{Dispatchable, Hash},
traits::{BlockNumberProvider, Dispatchable, Hash},
DispatchError, RuntimeDebug,
};

Expand Down Expand Up @@ -387,6 +387,9 @@ pub mod pallet {
/// consider using a constant cost (e.g., [`crate::deposit::Constant`]) equal to the minimum
/// balance under the `runtime-benchmarks` feature.
type Consideration: MaybeConsideration<Self::AccountId, u32>;

/// Provider for the block number. Normally this is the `frame_system` pallet.
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
}

#[pallet::genesis_config]
Expand Down Expand Up @@ -967,7 +970,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
<ProposalCount<T, I>>::mutate(|i| *i += 1);
<ProposalOf<T, I>>::insert(proposal_hash, proposal);
let votes = {
let end = frame_system::Pallet::<T>::block_number() + T::MotionDuration::get();
let end = T::BlockNumberProvider::current_block_number() + T::MotionDuration::get();
Votes { index, threshold, ayes: vec![], nays: vec![], end }
};
<Voting<T, I>>::insert(proposal_hash, votes);
Expand Down Expand Up @@ -1077,7 +1080,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

// Only allow actual closing of the proposal after the voting period has ended.
ensure!(frame_system::Pallet::<T>::block_number() >= voting.end, Error::<T, I>::TooEarly);
ensure!(
T::BlockNumberProvider::current_block_number() >= voting.end,
Error::<T, I>::TooEarly
);

let prime_vote = Prime::<T, I>::get().map(|who| voting.ayes.iter().any(|a| a == &who));

Expand Down
3 changes: 3 additions & 0 deletions substrate/frame/collective/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ impl Config<Instance1> for Test {
type KillOrigin = EnsureRoot<AccountId>;
type Consideration =
HoldConsideration<AccountId, Balances, ProposalHoldReason, CollectiveDeposit, u32>;
type BlockNumberProvider = frame_system::Pallet<Test>;
}

type CollectiveMajorityDeposit = deposit::Linear<ConstU32<2>, ProposalDepositBase>;
Expand All @@ -153,6 +154,7 @@ impl Config<Instance2> for Test {
type KillOrigin = EnsureRoot<AccountId>;
type Consideration =
HoldConsideration<AccountId, Balances, ProposalHoldReason, CollectiveMajorityDeposit, u32>;
type BlockNumberProvider = frame_system::Pallet<Test>;
}
impl mock_democracy::Config for Test {
type RuntimeEvent = RuntimeEvent;
Expand Down Expand Up @@ -181,6 +183,7 @@ impl Config for Test {
type KillOrigin = EnsureRoot<AccountId>;
type Consideration =
HoldConsideration<AccountId, Balances, ProposalHoldReason, DefaultCollectiveDeposit, u32>;
type BlockNumberProvider = frame_system::Pallet<Test>;
}

pub struct ExtBuilder {
Expand Down
10 changes: 7 additions & 3 deletions substrate/frame/identity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ use frame_support::{
use frame_system::pallet_prelude::*;
pub use pallet::*;
use sp_runtime::traits::{
AppendZerosInput, Hash, IdentifyAccount, Saturating, StaticLookup, Verify, Zero,
AppendZerosInput, BlockNumberProvider, Hash, IdentifyAccount, Saturating, StaticLookup, Verify,
Zero,
};
pub use types::{
Data, IdentityInformationProvider, Judgement, RegistrarIndex, RegistrarInfo, Registration,
Expand Down Expand Up @@ -228,6 +229,9 @@ pub mod pallet {

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;

/// Provider for the block number. Normally this is the `frame_system` pallet.
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
}

const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
Expand Down Expand Up @@ -1198,7 +1202,7 @@ pub mod pallet {
) -> DispatchResultWithPostInfo {
let _ = ensure_signed(origin)?;
if let Some((who, expiration, provider)) = PendingUsernames::<T>::take(&username) {
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
ensure!(now > expiration, Error::<T>::NotExpired);
let actual_weight = match provider {
Provider::AuthorityDeposit(deposit) => {
Expand Down Expand Up @@ -1517,7 +1521,7 @@ impl<T: Config> Pallet<T> {
/// A username was granted by an authority, but must be accepted by `who`. Put the username
/// into a queue for acceptance.
pub fn queue_acceptance(who: &T::AccountId, username: Username<T>, provider: ProviderOf<T>) {
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
let expiration = now.saturating_add(T::PendingUsernameExpiration::get());
PendingUsernames::<T>::insert(&username, (who.clone(), expiration, provider));
Self::deposit_event(Event::UsernameQueued { who: who.clone(), username, expiration });
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/identity/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ impl pallet_identity::Config for Test {
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
type WeightInfo = ();
type BlockNumberProvider = frame_system::Pallet<Test>;
}

pub fn new_test_ext() -> sp_io::TestExternalities {
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/utility/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ impl pallet_collective::Config<CouncilCollective> for Test {
type DisapproveOrigin = EnsureRoot<Self::AccountId>;
type KillOrigin = EnsureRoot<Self::AccountId>;
type Consideration = ();
type BlockNumberProvider = frame_system::Pallet<Test>;
}

impl example::Config for Test {}
Expand Down
Loading