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

Update Pallet Referenda to support Block Number Provider #6338

Open
wants to merge 2 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 @@ -154,6 +154,7 @@ impl pallet_referenda::Config<AmbassadorReferendaInstance> for Runtime {
type AlarmInterval = AlarmInterval;
type Tracks = tracks::TracksInfo;
type Preimages = Preimage;
type BlockNumberProvider = System;
}

parameter_types! {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ impl pallet_referenda::Config<FellowshipReferendaInstance> for Runtime {
type AlarmInterval = ConstU32<1>;
type Tracks = tracks::TracksInfo;
type Preimages = Preimage;
type BlockNumberProvider = crate::System;
}

pub type FellowshipCollectiveInstance = pallet_ranked_collective::Instance1;
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/rococo/src/governance/fellowship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ impl pallet_referenda::Config<FellowshipReferendaInstance> for Runtime {
type AlarmInterval = AlarmInterval;
type Tracks = TracksInfo;
type Preimages = Preimage;
type BlockNumberProvider = System;
}

pub type FellowshipCollectiveInstance = pallet_ranked_collective::Instance1;
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/rococo/src/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,5 @@ impl pallet_referenda::Config for Runtime {
type AlarmInterval = AlarmInterval;
type Tracks = TracksInfo;
type Preimages = Preimage;
type BlockNumberProvider = System;
}
1 change: 1 addition & 0 deletions polkadot/runtime/westend/src/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,5 @@ impl pallet_referenda::Config for Runtime {
type AlarmInterval = AlarmInterval;
type Tracks = TracksInfo;
type Preimages = Preimage;
type BlockNumberProvider = System;
}
14 changes: 14 additions & 0 deletions prdoc/pr_6338.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Update Referenda to Support Block Number Provider

doc:
- audience: Runtime Dev
description: |
This PR makes the referenda pallet uses the relay chain as a block provider for a parachain on a regular schedule.
To migrate existing referenda implementations, simply add `type BlockNumberProvider = System` to have the same behavior as before.

crates:
- name: pallet-referenda
bump: minor
2 changes: 2 additions & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,7 @@ impl pallet_referenda::Config for Runtime {
type AlarmInterval = AlarmInterval;
type Tracks = TracksInfo;
type Preimages = Preimage;
type BlockNumberProvider = System;
}

impl pallet_referenda::Config<pallet_referenda::Instance2> for Runtime {
Expand All @@ -1025,6 +1026,7 @@ impl pallet_referenda::Config<pallet_referenda::Instance2> for Runtime {
type AlarmInterval = AlarmInterval;
type Tracks = TracksInfo;
type Preimages = Preimage;
type BlockNumberProvider = System;
}

impl pallet_ranked_collective::Config for Runtime {
Expand Down
12 changes: 8 additions & 4 deletions substrate/frame/referenda/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ use sp_runtime::traits::Bounded as ArithBounded;

const SEED: u32 = 0;

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

fn assert_last_event<T: Config<I>, I: 'static>(generic_event: <T as Config<I>>::RuntimeEvent) {
frame_system::Pallet::<T>::assert_last_event(generic_event.into());
}
Expand Down Expand Up @@ -151,25 +155,25 @@ fn make_failing<T: Config<I>, I: 'static>(index: ReferendumIndex) {
fn skip_prepare_period<T: Config<I>, I: 'static>(index: ReferendumIndex) {
let status = Referenda::<T, I>::ensure_ongoing(index).unwrap();
let prepare_period_over = status.submitted + info::<T, I>(index).prepare_period;
frame_system::Pallet::<T>::set_block_number(prepare_period_over);
set_block_number::<T, I>(prepare_period_over);
}

fn skip_decision_period<T: Config<I>, I: 'static>(index: ReferendumIndex) {
let status = Referenda::<T, I>::ensure_ongoing(index).unwrap();
let decision_period_over = status.deciding.unwrap().since + info::<T, I>(index).decision_period;
frame_system::Pallet::<T>::set_block_number(decision_period_over);
set_block_number::<T, I>(decision_period_over);
}

fn skip_confirm_period<T: Config<I>, I: 'static>(index: ReferendumIndex) {
let status = Referenda::<T, I>::ensure_ongoing(index).unwrap();
let confirm_period_over = status.deciding.unwrap().confirming.unwrap();
frame_system::Pallet::<T>::set_block_number(confirm_period_over);
set_block_number::<T, I>(confirm_period_over);
}

fn skip_timeout_period<T: Config<I>, I: 'static>(index: ReferendumIndex) {
let status = Referenda::<T, I>::ensure_ongoing(index).unwrap();
let timeout_period_over = status.submitted + T::UndecidingTimeout::get();
frame_system::Pallet::<T>::set_block_number(timeout_period_over);
set_block_number::<T, I>(timeout_period_over);
}

fn alarm_time<T: Config<I>, I: 'static>(
Expand Down
32 changes: 18 additions & 14 deletions substrate/frame/referenda/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ pub use self::{
},
weights::WeightInfo,
};
use sp_runtime::traits::BlockNumberProvider;
pub use alloc::vec::Vec;

#[cfg(test)]
Expand Down Expand Up @@ -239,6 +240,9 @@ pub mod pallet {

/// The preimage provider.
type Preimages: QueryPreimage<H = Self::Hashing> + StorePreimage;

/// 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.

Why do we force the block number to be the one in system?
If the block number provider is RelayChainDataProvider, then the block number can be different.

It would be more clean to keep it generic here.
Then we should use a new type:

type BlockNumberFor<T> = <<Self as Config>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;
Suggested change
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
type BlockNumberProvider: BlockNumberProvider;

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we should not require the same block number type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gui1117 shouldn't it be

pub type BlockNumberFor<T, I> = <<T as Config<I>>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;

since the referenda pallet is an Instantiable one?

}

/// The next free referendum index, aka the number of referenda started so far.
Expand Down Expand Up @@ -485,7 +489,7 @@ pub mod pallet {
*x += 1;
r
});
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
let nudge_call =
T::Preimages::bound(CallOf::<T, I>::from(Call::nudge_referendum { index }))?;
let status = ReferendumStatus {
Expand Down Expand Up @@ -527,7 +531,7 @@ pub mod pallet {
let track = Self::track(status.track).ok_or(Error::<T, I>::NoTrack)?;
status.decision_deposit =
Some(Self::take_deposit(who.clone(), track.decision_deposit)?);
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
let (info, _, branch) = Self::service_referendum(now, index, status);
ReferendumInfoFor::<T, I>::insert(index, info);
let e =
Expand Down Expand Up @@ -584,7 +588,7 @@ pub mod pallet {
Self::note_one_fewer_deciding(status.track);
Self::deposit_event(Event::<T, I>::Cancelled { index, tally: status.tally });
let info = ReferendumInfo::Cancelled(
frame_system::Pallet::<T>::block_number(),
T::BlockNumberProvider::current_block_number(),
Some(status.submission_deposit),
status.decision_deposit,
);
Expand All @@ -611,7 +615,7 @@ pub mod pallet {
Self::slash_deposit(Some(status.submission_deposit.clone()));
Self::slash_deposit(status.decision_deposit.clone());
Self::do_clear_metadata(index);
let info = ReferendumInfo::Killed(frame_system::Pallet::<T>::block_number());
let info = ReferendumInfo::Killed(T::BlockNumberProvider::current_block_number());
ReferendumInfoFor::<T, I>::insert(index, info);
Ok(())
}
Expand All @@ -627,7 +631,7 @@ pub mod pallet {
index: ReferendumIndex,
) -> DispatchResultWithPostInfo {
ensure_root(origin)?;
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
let mut status = Self::ensure_ongoing(index)?;
// This is our wake-up, so we can disregard the alarm.
status.alarm = None;
Expand Down Expand Up @@ -658,7 +662,7 @@ pub mod pallet {
let mut track_queue = TrackQueue::<T, I>::get(track);
let branch =
if let Some((index, mut status)) = Self::next_for_deciding(&mut track_queue) {
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
let (maybe_alarm, branch) =
Self::begin_deciding(&mut status, index, now, track_info);
if let Some(set_alarm) = maybe_alarm {
Expand Down Expand Up @@ -758,7 +762,7 @@ impl<T: Config<I>, I: 'static> Polling<T::Tally> for Pallet<T, I> {
match ReferendumInfoFor::<T, I>::get(index) {
Some(ReferendumInfo::Ongoing(mut status)) => {
let result = f(PollStatus::Ongoing(&mut status.tally, status.track));
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
Self::ensure_alarm_at(&mut status, index, now + One::one());
ReferendumInfoFor::<T, I>::insert(index, ReferendumInfo::Ongoing(status));
result
Expand All @@ -778,7 +782,7 @@ impl<T: Config<I>, I: 'static> Polling<T::Tally> for Pallet<T, I> {
match ReferendumInfoFor::<T, I>::get(index) {
Some(ReferendumInfo::Ongoing(mut status)) => {
let result = f(PollStatus::Ongoing(&mut status.tally, status.track))?;
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
Self::ensure_alarm_at(&mut status, index, now + One::one());
ReferendumInfoFor::<T, I>::insert(index, ReferendumInfo::Ongoing(status));
Ok(result)
Expand All @@ -800,7 +804,7 @@ impl<T: Config<I>, I: 'static> Polling<T::Tally> for Pallet<T, I> {
*x += 1;
r
});
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
let dummy_account_id =
codec::Decode::decode(&mut sp_runtime::traits::TrailingZeroInput::new(&b"dummy"[..]))
.expect("infinite length input; no invalid inputs for type; qed");
Expand Down Expand Up @@ -828,7 +832,7 @@ impl<T: Config<I>, I: 'static> Polling<T::Tally> for Pallet<T, I> {
let mut status = Self::ensure_ongoing(index).map_err(|_| ())?;
Self::ensure_no_alarm(&mut status);
Self::note_one_fewer_deciding(status.track);
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
let info = if approved {
ReferendumInfo::Approved(now, Some(status.submission_deposit), status.decision_deposit)
} else {
Expand Down Expand Up @@ -868,7 +872,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
ReferendumInfo::Ongoing(status) => {
let track = Self::track(status.track).ok_or(Error::<T, I>::NoTrack)?;
let elapsed = if let Some(deciding) = status.deciding {
frame_system::Pallet::<T>::block_number().saturating_sub(deciding.since)
T::BlockNumberProvider::current_block_number().saturating_sub(deciding.since)
} else {
Zero::zero()
};
Expand All @@ -893,7 +897,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
origin: PalletsOriginOf<T>,
call: BoundedCallOf<T, I>,
) {
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
// Earliest allowed block is always at minimum the next block.
let earliest_allowed = now.saturating_add(track.min_enactment_period.max(One::one()));
let desired = desired.evaluate(now);
Expand Down Expand Up @@ -931,7 +935,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
result.is_ok(),
"Unable to schedule a new alarm at #{:?} (now: #{:?}), scheduler error: `{:?}`",
when,
frame_system::Pallet::<T>::block_number(),
T::BlockNumberProvider::current_block_number(),
result.unwrap_err(),
);
result.ok().map(|x| (when, x))
Expand Down Expand Up @@ -1023,7 +1027,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// overall more efficient), however the weights become rather less easy to measure.
fn note_one_fewer_deciding(track: TrackIdOf<T, I>) {
// Set an alarm call for the next block to nudge the track along.
let now = frame_system::Pallet::<T>::block_number();
let now = T::BlockNumberProvider::current_block_number();
let next_block = now + One::one();
let call = match T::Preimages::bound(CallOf::<T, I>::from(Call::one_fewer_deciding {
track,
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/referenda/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ impl Config for Test {
type AlarmInterval = AlarmInterval;
type Tracks = TestTracksInfo;
type Preimages = Preimage;
type BlockNumberProvider = System;
}
pub struct ExtBuilder {}

Expand Down
Loading