From 46abe420ea681e39a5067d44addac571be129d54 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Sat, 2 Nov 2024 12:11:06 +0100 Subject: [PATCH 01/16] Update Pallet Referenda to support relay chain Block Number Provider --- substrate/frame/referenda/src/benchmarking.rs | 12 ++++--- substrate/frame/referenda/src/lib.rs | 32 +++++++++++-------- substrate/frame/referenda/src/mock.rs | 1 + 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/substrate/frame/referenda/src/benchmarking.rs b/substrate/frame/referenda/src/benchmarking.rs index 67ac82787d31..e24a97c51bf3 100644 --- a/substrate/frame/referenda/src/benchmarking.rs +++ b/substrate/frame/referenda/src/benchmarking.rs @@ -33,6 +33,10 @@ use sp_runtime::traits::Bounded as ArithBounded; const SEED: u32 = 0; +fn set_block_number, I: 'static>(n: BlockNumberFor) { + >::BlockNumberProvider::set_block_number(n); +} + fn assert_last_event, I: 'static>(generic_event: >::RuntimeEvent) { frame_system::Pallet::::assert_last_event(generic_event.into()); } @@ -151,25 +155,25 @@ fn make_failing, I: 'static>(index: ReferendumIndex) { fn skip_prepare_period, I: 'static>(index: ReferendumIndex) { let status = Referenda::::ensure_ongoing(index).unwrap(); let prepare_period_over = status.submitted + info::(index).prepare_period; - frame_system::Pallet::::set_block_number(prepare_period_over); + set_block_number::(prepare_period_over); } fn skip_decision_period, I: 'static>(index: ReferendumIndex) { let status = Referenda::::ensure_ongoing(index).unwrap(); let decision_period_over = status.deciding.unwrap().since + info::(index).decision_period; - frame_system::Pallet::::set_block_number(decision_period_over); + set_block_number::(decision_period_over); } fn skip_confirm_period, I: 'static>(index: ReferendumIndex) { let status = Referenda::::ensure_ongoing(index).unwrap(); let confirm_period_over = status.deciding.unwrap().confirming.unwrap(); - frame_system::Pallet::::set_block_number(confirm_period_over); + set_block_number::(confirm_period_over); } fn skip_timeout_period, I: 'static>(index: ReferendumIndex) { let status = Referenda::::ensure_ongoing(index).unwrap(); let timeout_period_over = status.submitted + T::UndecidingTimeout::get(); - frame_system::Pallet::::set_block_number(timeout_period_over); + set_block_number::(timeout_period_over); } fn alarm_time, I: 'static>( diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index e72dd7f11cbb..ce3d911d0b1a 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -105,6 +105,7 @@ pub use self::{ }, weights::WeightInfo, }; +use sp_runtime::traits::BlockNumberProvider; pub use alloc::vec::Vec; #[cfg(test)] @@ -239,6 +240,9 @@ pub mod pallet { /// The preimage provider. type Preimages: QueryPreimage + StorePreimage; + + /// Provider for the block number. Normally this is the `frame_system` pallet. + type BlockNumberProvider: BlockNumberProvider>; } /// The next free referendum index, aka the number of referenda started so far. @@ -485,7 +489,7 @@ pub mod pallet { *x += 1; r }); - let now = frame_system::Pallet::::block_number(); + let now = T::BlockNumberProvider::current_block_number(); let nudge_call = T::Preimages::bound(CallOf::::from(Call::nudge_referendum { index }))?; let status = ReferendumStatus { @@ -527,7 +531,7 @@ pub mod pallet { let track = Self::track(status.track).ok_or(Error::::NoTrack)?; status.decision_deposit = Some(Self::take_deposit(who.clone(), track.decision_deposit)?); - let now = frame_system::Pallet::::block_number(); + let now = T::BlockNumberProvider::current_block_number(); let (info, _, branch) = Self::service_referendum(now, index, status); ReferendumInfoFor::::insert(index, info); let e = @@ -584,7 +588,7 @@ pub mod pallet { Self::note_one_fewer_deciding(status.track); Self::deposit_event(Event::::Cancelled { index, tally: status.tally }); let info = ReferendumInfo::Cancelled( - frame_system::Pallet::::block_number(), + T::BlockNumberProvider::current_block_number(), Some(status.submission_deposit), status.decision_deposit, ); @@ -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::::block_number()); + let info = ReferendumInfo::Killed(T::BlockNumberProvider::current_block_number()); ReferendumInfoFor::::insert(index, info); Ok(()) } @@ -627,7 +631,7 @@ pub mod pallet { index: ReferendumIndex, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - let now = frame_system::Pallet::::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; @@ -658,7 +662,7 @@ pub mod pallet { let mut track_queue = TrackQueue::::get(track); let branch = if let Some((index, mut status)) = Self::next_for_deciding(&mut track_queue) { - let now = frame_system::Pallet::::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 { @@ -758,7 +762,7 @@ impl, I: 'static> Polling for Pallet { match ReferendumInfoFor::::get(index) { Some(ReferendumInfo::Ongoing(mut status)) => { let result = f(PollStatus::Ongoing(&mut status.tally, status.track)); - let now = frame_system::Pallet::::block_number(); + let now = T::BlockNumberProvider::current_block_number(); Self::ensure_alarm_at(&mut status, index, now + One::one()); ReferendumInfoFor::::insert(index, ReferendumInfo::Ongoing(status)); result @@ -778,7 +782,7 @@ impl, I: 'static> Polling for Pallet { match ReferendumInfoFor::::get(index) { Some(ReferendumInfo::Ongoing(mut status)) => { let result = f(PollStatus::Ongoing(&mut status.tally, status.track))?; - let now = frame_system::Pallet::::block_number(); + let now = T::BlockNumberProvider::current_block_number(); Self::ensure_alarm_at(&mut status, index, now + One::one()); ReferendumInfoFor::::insert(index, ReferendumInfo::Ongoing(status)); Ok(result) @@ -800,7 +804,7 @@ impl, I: 'static> Polling for Pallet { *x += 1; r }); - let now = frame_system::Pallet::::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"); @@ -828,7 +832,7 @@ impl, I: 'static> Polling for Pallet { 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::::block_number(); + let now = T::BlockNumberProvider::current_block_number(); let info = if approved { ReferendumInfo::Approved(now, Some(status.submission_deposit), status.decision_deposit) } else { @@ -868,7 +872,7 @@ impl, I: 'static> Pallet { ReferendumInfo::Ongoing(status) => { let track = Self::track(status.track).ok_or(Error::::NoTrack)?; let elapsed = if let Some(deciding) = status.deciding { - frame_system::Pallet::::block_number().saturating_sub(deciding.since) + T::BlockNumberProvider::current_block_number().saturating_sub(deciding.since) } else { Zero::zero() }; @@ -893,7 +897,7 @@ impl, I: 'static> Pallet { origin: PalletsOriginOf, call: BoundedCallOf, ) { - let now = frame_system::Pallet::::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); @@ -931,7 +935,7 @@ impl, I: 'static> Pallet { result.is_ok(), "Unable to schedule a new alarm at #{:?} (now: #{:?}), scheduler error: `{:?}`", when, - frame_system::Pallet::::block_number(), + T::BlockNumberProvider::current_block_number(), result.unwrap_err(), ); result.ok().map(|x| (when, x)) @@ -1023,7 +1027,7 @@ impl, I: 'static> Pallet { /// overall more efficient), however the weights become rather less easy to measure. fn note_one_fewer_deciding(track: TrackIdOf) { // Set an alarm call for the next block to nudge the track along. - let now = frame_system::Pallet::::block_number(); + let now = T::BlockNumberProvider::current_block_number(); let next_block = now + One::one(); let call = match T::Preimages::bound(CallOf::::from(Call::one_fewer_deciding { track, diff --git a/substrate/frame/referenda/src/mock.rs b/substrate/frame/referenda/src/mock.rs index c96a50af8658..3f2c331ef861 100644 --- a/substrate/frame/referenda/src/mock.rs +++ b/substrate/frame/referenda/src/mock.rs @@ -206,6 +206,7 @@ impl Config for Test { type AlarmInterval = AlarmInterval; type Tracks = TestTracksInfo; type Preimages = Preimage; + type BlockNumberProvider = System; } pub struct ExtBuilder {} From fd92baadb101f4589c907f29d08d8784db6ce4aa Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Sat, 2 Nov 2024 12:38:13 +0100 Subject: [PATCH 02/16] Include BlockNumberProvider for all runtime configs PR doc --- .../collectives-westend/src/ambassador/mod.rs | 1 + .../collectives-westend/src/fellowship/mod.rs | 1 + .../runtime/rococo/src/governance/fellowship.rs | 1 + polkadot/runtime/rococo/src/governance/mod.rs | 1 + polkadot/runtime/westend/src/governance/mod.rs | 1 + prdoc/pr_6338.prdoc | 14 ++++++++++++++ substrate/bin/node/runtime/src/lib.rs | 2 ++ 7 files changed, 21 insertions(+) create mode 100644 prdoc/pr_6338.prdoc diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs index a052a9d3800c..8b0842697237 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs @@ -154,6 +154,7 @@ impl pallet_referenda::Config for Runtime { type AlarmInterval = AlarmInterval; type Tracks = tracks::TracksInfo; type Preimages = Preimage; + type BlockNumberProvider = System; } parameter_types! { diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs index 1e8212cf6ac2..e699f2eeaf43 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs @@ -106,6 +106,7 @@ impl pallet_referenda::Config for Runtime { type AlarmInterval = ConstU32<1>; type Tracks = tracks::TracksInfo; type Preimages = Preimage; + type BlockNumberProvider = crate::System; } pub type FellowshipCollectiveInstance = pallet_ranked_collective::Instance1; diff --git a/polkadot/runtime/rococo/src/governance/fellowship.rs b/polkadot/runtime/rococo/src/governance/fellowship.rs index 27a58a0eebd1..231defab6aa5 100644 --- a/polkadot/runtime/rococo/src/governance/fellowship.rs +++ b/polkadot/runtime/rococo/src/governance/fellowship.rs @@ -308,6 +308,7 @@ impl pallet_referenda::Config for Runtime { type AlarmInterval = AlarmInterval; type Tracks = TracksInfo; type Preimages = Preimage; + type BlockNumberProvider = System; } pub type FellowshipCollectiveInstance = pallet_ranked_collective::Instance1; diff --git a/polkadot/runtime/rococo/src/governance/mod.rs b/polkadot/runtime/rococo/src/governance/mod.rs index ef2adf60753d..2be549be29ed 100644 --- a/polkadot/runtime/rococo/src/governance/mod.rs +++ b/polkadot/runtime/rococo/src/governance/mod.rs @@ -90,4 +90,5 @@ impl pallet_referenda::Config for Runtime { type AlarmInterval = AlarmInterval; type Tracks = TracksInfo; type Preimages = Preimage; + type BlockNumberProvider = System; } diff --git a/polkadot/runtime/westend/src/governance/mod.rs b/polkadot/runtime/westend/src/governance/mod.rs index d027f788d71f..abc25ebaa470 100644 --- a/polkadot/runtime/westend/src/governance/mod.rs +++ b/polkadot/runtime/westend/src/governance/mod.rs @@ -94,4 +94,5 @@ impl pallet_referenda::Config for Runtime { type AlarmInterval = AlarmInterval; type Tracks = TracksInfo; type Preimages = Preimage; + type BlockNumberProvider = System; } diff --git a/prdoc/pr_6338.prdoc b/prdoc/pr_6338.prdoc new file mode 100644 index 000000000000..7ea43f924d0d --- /dev/null +++ b/prdoc/pr_6338.prdoc @@ -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 \ No newline at end of file diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 7712d8ba9540..49fc360e399f 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -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 for Runtime { @@ -1025,6 +1026,7 @@ impl pallet_referenda::Config for Runtime { type AlarmInterval = AlarmInterval; type Tracks = TracksInfo; type Preimages = Preimage; + type BlockNumberProvider = System; } impl pallet_ranked_collective::Config for Runtime { From 57be43c44365469f3a0255329d564b20cfec5c9c Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Mon, 18 Nov 2024 15:52:07 +0100 Subject: [PATCH 03/16] introduce custom BlockProvider type --- substrate/frame/referenda/src/lib.rs | 65 +++++++++---------- substrate/frame/referenda/src/migration.rs | 2 +- substrate/frame/referenda/src/types.rs | 15 +++-- .../primitives/runtime/src/traits/mod.rs | 3 +- 4 files changed, 44 insertions(+), 41 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index ce3d911d0b1a..8a605f4b628d 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -82,7 +82,6 @@ use frame_support::{ }, BoundedVec, }; -use frame_system::pallet_prelude::BlockNumberFor; use scale_info::TypeInfo; use sp_runtime::{ traits::{AtLeast32BitUnsigned, Bounded, Dispatchable, One, Saturating, Zero}, @@ -101,7 +100,7 @@ pub use self::{ BalanceOf, BoundedCallOf, CallOf, Curve, DecidingStatus, DecidingStatusOf, Deposit, InsertSorted, NegativeImbalanceOf, PalletsOriginOf, ReferendumIndex, ReferendumInfo, ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, ScheduleAddressOf, TallyOf, - TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf, + TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf, BlockNumberFor }, weights::WeightInfo, }; @@ -145,7 +144,7 @@ const ASSEMBLY_ID: LockIdentifier = *b"assembly"; pub mod pallet { use super::*; use frame_support::{pallet_prelude::*, traits::EnsureOriginWithArg}; - use frame_system::pallet_prelude::*; + use frame_system::pallet_prelude::{OriginFor, BlockNumberFor as SystemBlockNumberFor, ensure_root, ensure_signed_or_root, ensure_signed}; /// The in-code storage version. const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); @@ -168,12 +167,12 @@ pub mod pallet { type WeightInfo: WeightInfo; /// The Scheduler. type Scheduler: ScheduleAnon< - BlockNumberFor, + BlockNumberFor, CallOf, PalletsOriginOf, Hasher = Self::Hashing, > + ScheduleNamed< - BlockNumberFor, + BlockNumberFor, CallOf, PalletsOriginOf, Hasher = Self::Hashing, @@ -216,25 +215,25 @@ pub mod pallet { /// The number of blocks after submission that a referendum must begin being decided by. /// Once this passes, then anyone may cancel the referendum. #[pallet::constant] - type UndecidingTimeout: Get>; + type UndecidingTimeout: Get>; /// Quantization level for the referendum wakeup scheduler. A higher number will result in /// fewer storage reads/writes needed for smaller voters, but also result in delays to the /// automatic referendum status changes. Explicit servicing instructions are unaffected. #[pallet::constant] - type AlarmInterval: Get>; + type AlarmInterval: Get>; // The other stuff. /// Information concerning the different referendum tracks. #[pallet::constant] type Tracks: Get< Vec<( - , BlockNumberFor>>::Id, - TrackInfo, BlockNumberFor>, + , BlockNumberFor>>::Id, + TrackInfo, BlockNumberFor>, )>, > + TracksInfo< BalanceOf, - BlockNumberFor, + BlockNumberFor, RuntimeOrigin = ::PalletsOrigin, >; @@ -242,7 +241,7 @@ pub mod pallet { type Preimages: QueryPreimage + StorePreimage; /// Provider for the block number. Normally this is the `frame_system` pallet. - type BlockNumberProvider: BlockNumberProvider>; + type BlockNumberProvider: BlockNumberProvider; } /// The next free referendum index, aka the number of referenda started so far. @@ -436,9 +435,9 @@ pub mod pallet { } #[pallet::hooks] - impl, I: 'static> Hooks> for Pallet { + impl, I: 'static> Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + fn try_state(_n: SystemBlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { Self::do_try_state()?; Ok(()) } @@ -466,7 +465,7 @@ pub mod pallet { origin: OriginFor, proposal_origin: Box>, proposal: BoundedCallOf, - enactment_moment: DispatchTime>, + enactment_moment: DispatchTime>, ) -> DispatchResult { let proposal_origin = *proposal_origin; let who = T::SubmitOrigin::ensure_origin(origin, &proposal_origin)?; @@ -748,7 +747,7 @@ pub mod pallet { impl, I: 'static> Polling for Pallet { type Index = ReferendumIndex; type Votes = VotesOf; - type Moment = BlockNumberFor; + type Moment = BlockNumberFor; type Class = TrackIdOf; fn classes() -> Vec { @@ -757,7 +756,7 @@ impl, I: 'static> Polling for Pallet { fn access_poll( index: Self::Index, - f: impl FnOnce(PollStatus<&mut T::Tally, BlockNumberFor, TrackIdOf>) -> R, + f: impl FnOnce(PollStatus<&mut T::Tally, BlockNumberFor, TrackIdOf>) -> R, ) -> R { match ReferendumInfoFor::::get(index) { Some(ReferendumInfo::Ongoing(mut status)) => { @@ -776,7 +775,7 @@ impl, I: 'static> Polling for Pallet { fn try_access_poll( index: Self::Index, f: impl FnOnce( - PollStatus<&mut T::Tally, BlockNumberFor, TrackIdOf>, + PollStatus<&mut T::Tally, BlockNumberFor, TrackIdOf>, ) -> Result, ) -> Result { match ReferendumInfoFor::::get(index) { @@ -893,7 +892,7 @@ impl, I: 'static> Pallet { fn schedule_enactment( index: ReferendumIndex, track: &TrackInfoOf, - desired: DispatchTime>, + desired: DispatchTime>, origin: PalletsOriginOf, call: BoundedCallOf, ) { @@ -916,8 +915,8 @@ impl, I: 'static> Pallet { /// Set an alarm to dispatch `call` at block number `when`. fn set_alarm( call: BoundedCallOf, - when: BlockNumberFor, - ) -> Option<(BlockNumberFor, ScheduleAddressOf)> { + when: BlockNumberFor, + ) -> Option<(BlockNumberFor, ScheduleAddressOf)> { let alarm_interval = T::AlarmInterval::get().max(One::one()); // Alarm must go off no earlier than `when`. // This rounds `when` upwards to the next multiple of `alarm_interval`. @@ -950,9 +949,9 @@ impl, I: 'static> Pallet { fn begin_deciding( status: &mut ReferendumStatusOf, index: ReferendumIndex, - now: BlockNumberFor, + now: BlockNumberFor, track: &TrackInfoOf, - ) -> (Option>, BeginDecidingBranch) { + ) -> (Option>, BeginDecidingBranch) { let is_passing = Self::is_passing( &status.tally, Zero::zero(), @@ -988,11 +987,11 @@ impl, I: 'static> Pallet { /// /// If `None`, then it is queued and should be nudged automatically as the queue gets drained. fn ready_for_deciding( - now: BlockNumberFor, + now: BlockNumberFor, track: &TrackInfoOf, index: ReferendumIndex, status: &mut ReferendumStatusOf, - ) -> (Option>, ServiceBranch) { + ) -> (Option>, ServiceBranch) { let deciding_count = DecidingCount::::get(status.track); if deciding_count < track.max_deciding { // Begin deciding. @@ -1049,7 +1048,7 @@ impl, I: 'static> Pallet { fn ensure_alarm_at( status: &mut ReferendumStatusOf, index: ReferendumIndex, - alarm: BlockNumberFor, + alarm: BlockNumberFor, ) -> bool { if status.alarm.as_ref().map_or(true, |&(when, _)| when != alarm) { // Either no alarm or one that was different @@ -1094,7 +1093,7 @@ impl, I: 'static> Pallet { /// `TrackQueue`. Basically this happens when a referendum is in the deciding queue and receives /// a vote, or when it moves into the deciding queue. fn service_referendum( - now: BlockNumberFor, + now: BlockNumberFor, index: ReferendumIndex, mut status: ReferendumStatusOf, ) -> (ReferendumInfoOf, bool, ServiceBranch) { @@ -1106,7 +1105,7 @@ impl, I: 'static> Pallet { }; // Default the alarm to the end of the world. let timeout = status.submitted + T::UndecidingTimeout::get(); - let mut alarm = BlockNumberFor::::max_value(); + let mut alarm = BlockNumberFor::::max_value(); let branch; match &mut status.deciding { None => { @@ -1237,7 +1236,7 @@ impl, I: 'static> Pallet { }, } - let dirty_alarm = if alarm < BlockNumberFor::::max_value() { + let dirty_alarm = if alarm < BlockNumberFor::::max_value() { Self::ensure_alarm_at(&mut status, index, alarm) } else { Self::ensure_no_alarm(&mut status) @@ -1248,11 +1247,11 @@ impl, I: 'static> Pallet { /// Determine the point at which a referendum will be accepted, move into confirmation with the /// given `tally` or end with rejection (whichever happens sooner). fn decision_time( - deciding: &DecidingStatusOf, + deciding: &DecidingStatusOf, tally: &T::Tally, track_id: TrackIdOf, track: &TrackInfoOf, - ) -> BlockNumberFor { + ) -> BlockNumberFor { deciding.confirming.unwrap_or_else(|| { // Set alarm to the point where the current voting would make it pass. let approval = tally.approval(track_id); @@ -1311,8 +1310,8 @@ impl, I: 'static> Pallet { /// `approval_needed`. fn is_passing( tally: &T::Tally, - elapsed: BlockNumberFor, - period: BlockNumberFor, + elapsed: BlockNumberFor, + period: BlockNumberFor, support_needed: &Curve, approval_needed: &Curve, id: TrackIdOf, @@ -1381,7 +1380,7 @@ impl, I: 'static> Pallet { if let Some(deciding) = status.deciding { ensure!( deciding.since < - deciding.confirming.unwrap_or(BlockNumberFor::::max_value()), + deciding.confirming.unwrap_or(BlockNumberFor::::max_value()), "Deciding status cannot begin before confirming stage." ) } diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 631eb7340e56..2c1af128066c 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -37,7 +37,7 @@ pub mod v0 { pub type ReferendumInfoOf = ReferendumInfo< TrackIdOf, PalletsOriginOf, - frame_system::pallet_prelude::BlockNumberFor, + BlockNumberFor, BoundedCallOf, BalanceOf, TallyOf, diff --git a/substrate/frame/referenda/src/types.rs b/substrate/frame/referenda/src/types.rs index e83f28b472cd..d360cf043826 100644 --- a/substrate/frame/referenda/src/types.rs +++ b/substrate/frame/referenda/src/types.rs @@ -30,6 +30,9 @@ use sp_runtime::{FixedI64, PerThing, RuntimeDebug}; pub type BalanceOf = <>::Currency as Currency<::AccountId>>::Balance; + +pub type BlockNumberFor = <>::BlockNumberProvider as BlockNumberProvider>::BlockNumber; + pub type NegativeImbalanceOf = <>::Currency as Currency< ::AccountId, >>::NegativeImbalance; @@ -43,7 +46,7 @@ pub type PalletsOriginOf = pub type ReferendumInfoOf = ReferendumInfo< TrackIdOf, PalletsOriginOf, - BlockNumberFor, + BlockNumberFor, BoundedCallOf, BalanceOf, TallyOf, @@ -53,19 +56,19 @@ pub type ReferendumInfoOf = ReferendumInfo< pub type ReferendumStatusOf = ReferendumStatus< TrackIdOf, PalletsOriginOf, - BlockNumberFor, + BlockNumberFor, BoundedCallOf, BalanceOf, TallyOf, ::AccountId, ScheduleAddressOf, >; -pub type DecidingStatusOf = DecidingStatus>; -pub type TrackInfoOf = TrackInfo, BlockNumberFor>; +pub type DecidingStatusOf = DecidingStatus>; +pub type TrackInfoOf = TrackInfo, BlockNumberFor>; pub type TrackIdOf = - <>::Tracks as TracksInfo, BlockNumberFor>>::Id; + <>::Tracks as TracksInfo, BlockNumberFor>>::Id; pub type ScheduleAddressOf = <>::Scheduler as Anon< - BlockNumberFor, + BlockNumberFor, CallOf, PalletsOriginOf, >>::Address; diff --git a/substrate/primitives/runtime/src/traits/mod.rs b/substrate/primitives/runtime/src/traits/mod.rs index e6906cdb3877..74b948fc5d18 100644 --- a/substrate/primitives/runtime/src/traits/mod.rs +++ b/substrate/primitives/runtime/src/traits/mod.rs @@ -2348,7 +2348,8 @@ pub trait BlockNumberProvider { + TypeInfo + Debug + MaxEncodedLen - + Copy; + + Copy + + EncodeLike; /// Returns the current block number. /// From e62fe61a5613253674768f70f5e1782bf440feca Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Mon, 18 Nov 2024 15:54:48 +0100 Subject: [PATCH 04/16] fmt --- substrate/frame/referenda/src/lib.rs | 19 ++++++++++++------- substrate/frame/referenda/src/types.rs | 3 ++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index 8a605f4b628d..daa3dc56191d 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -97,15 +97,15 @@ use self::branch::{BeginDecidingBranch, OneFewerDecidingBranch, ServiceBranch}; pub use self::{ pallet::*, types::{ - BalanceOf, BoundedCallOf, CallOf, Curve, DecidingStatus, DecidingStatusOf, Deposit, - InsertSorted, NegativeImbalanceOf, PalletsOriginOf, ReferendumIndex, ReferendumInfo, - ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, ScheduleAddressOf, TallyOf, - TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf, BlockNumberFor + BalanceOf, BlockNumberFor, BoundedCallOf, CallOf, Curve, DecidingStatus, DecidingStatusOf, + Deposit, InsertSorted, NegativeImbalanceOf, PalletsOriginOf, ReferendumIndex, + ReferendumInfo, ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, ScheduleAddressOf, + TallyOf, TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf, }, weights::WeightInfo, }; -use sp_runtime::traits::BlockNumberProvider; pub use alloc::vec::Vec; +use sp_runtime::traits::BlockNumberProvider; #[cfg(test)] mod mock; @@ -144,7 +144,10 @@ const ASSEMBLY_ID: LockIdentifier = *b"assembly"; pub mod pallet { use super::*; use frame_support::{pallet_prelude::*, traits::EnsureOriginWithArg}; - use frame_system::pallet_prelude::{OriginFor, BlockNumberFor as SystemBlockNumberFor, ensure_root, ensure_signed_or_root, ensure_signed}; + use frame_system::pallet_prelude::{ + ensure_root, ensure_signed, ensure_signed_or_root, BlockNumberFor as SystemBlockNumberFor, + OriginFor, + }; /// The in-code storage version. const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); @@ -1380,7 +1383,9 @@ impl, I: 'static> Pallet { if let Some(deciding) = status.deciding { ensure!( deciding.since < - deciding.confirming.unwrap_or(BlockNumberFor::::max_value()), + deciding + .confirming + .unwrap_or(BlockNumberFor::::max_value()), "Deciding status cannot begin before confirming stage." ) } diff --git a/substrate/frame/referenda/src/types.rs b/substrate/frame/referenda/src/types.rs index d360cf043826..e97e7cc8df6d 100644 --- a/substrate/frame/referenda/src/types.rs +++ b/substrate/frame/referenda/src/types.rs @@ -31,7 +31,8 @@ use sp_runtime::{FixedI64, PerThing, RuntimeDebug}; pub type BalanceOf = <>::Currency as Currency<::AccountId>>::Balance; -pub type BlockNumberFor = <>::BlockNumberProvider as BlockNumberProvider>::BlockNumber; +pub type BlockNumberFor = + <>::BlockNumberProvider as BlockNumberProvider>::BlockNumber; pub type NegativeImbalanceOf = <>::Currency as Currency< ::AccountId, From da6dbe1a4b14060b8e9a1cccbed2c0f442dafafb Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Mon, 18 Nov 2024 16:18:48 +0100 Subject: [PATCH 05/16] fix benchmarks --- substrate/frame/referenda/src/benchmarking.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/referenda/src/benchmarking.rs b/substrate/frame/referenda/src/benchmarking.rs index e24a97c51bf3..42141cb53b2f 100644 --- a/substrate/frame/referenda/src/benchmarking.rs +++ b/substrate/frame/referenda/src/benchmarking.rs @@ -33,7 +33,7 @@ use sp_runtime::traits::Bounded as ArithBounded; const SEED: u32 = 0; -fn set_block_number, I: 'static>(n: BlockNumberFor) { +fn set_block_number, I: 'static>(n: BlockNumberFor) { >::BlockNumberProvider::set_block_number(n); } @@ -178,7 +178,7 @@ fn skip_timeout_period, I: 'static>(index: ReferendumIndex) { fn alarm_time, I: 'static>( index: ReferendumIndex, -) -> frame_system::pallet_prelude::BlockNumberFor { +) -> BlockNumberFor { let status = Referenda::::ensure_ongoing(index).unwrap(); status.alarm.unwrap().0 } From 0cd84f0a5dda3539f5a66d19d710f03f7a8f7b70 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Mon, 18 Nov 2024 16:44:56 +0100 Subject: [PATCH 06/16] fmt benchmarks --- substrate/frame/referenda/src/benchmarking.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/frame/referenda/src/benchmarking.rs b/substrate/frame/referenda/src/benchmarking.rs index 42141cb53b2f..895f95dbec55 100644 --- a/substrate/frame/referenda/src/benchmarking.rs +++ b/substrate/frame/referenda/src/benchmarking.rs @@ -176,9 +176,7 @@ fn skip_timeout_period, I: 'static>(index: ReferendumIndex) { set_block_number::(timeout_period_over); } -fn alarm_time, I: 'static>( - index: ReferendumIndex, -) -> BlockNumberFor { +fn alarm_time, I: 'static>(index: ReferendumIndex) -> BlockNumberFor { let status = Referenda::::ensure_ongoing(index).unwrap(); status.alarm.unwrap().0 } From 76e5f94f9e9472d0acca05dd55fb18ce7795648a Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Tue, 14 Jan 2025 15:40:30 +0100 Subject: [PATCH 07/16] migration --- substrate/frame/referenda/src/migration.rs | 119 ++++++++++++++++++++- 1 file changed, 114 insertions(+), 5 deletions(-) diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 2c1af128066c..fcef265d0b83 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -25,6 +25,8 @@ use log; #[cfg(feature = "try-runtime")] use sp_runtime::TryRuntimeError; +type SystemBlockNumberFor = frame_system::pallet_prelude::BlockNumberFor; + /// Initial version of storage types. pub mod v0 { use super::*; @@ -37,7 +39,7 @@ pub mod v0 { pub type ReferendumInfoOf = ReferendumInfo< TrackIdOf, PalletsOriginOf, - BlockNumberFor, + SystemBlockNumberFor, BoundedCallOf, BalanceOf, TallyOf, @@ -93,6 +95,21 @@ pub mod v1 { /// The log target. const TARGET: &'static str = "runtime::referenda::migration::v1"; + pub(crate) type ReferendumInfoOf = ReferendumInfo< + TrackIdOf, + PalletsOriginOf, + SystemBlockNumberFor, + BoundedCallOf, + BalanceOf, + TallyOf, + ::AccountId, + ScheduleAddressOf, + >; + + #[storage_alias] + pub type ReferendumInfoFor, I: 'static> = + StorageMap, Blake2_128Concat, ReferendumIndex, ReferendumInfoOf>; + /// Transforms a submission deposit of ReferendumInfo(Approved|Rejected|Cancelled|TimedOut) to /// optional value, making it refundable. pub struct MigrateV0ToV1(PhantomData<(T, I)>); @@ -137,7 +154,7 @@ pub mod v1 { if let Some(new_value) = maybe_new_value { weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); log::info!(target: TARGET, "migrating referendum #{:?}", &key); - ReferendumInfoFor::::insert(key, new_value); + v1::ReferendumInfoFor::::insert(key, new_value); } else { weight.saturating_accrue(T::DbWeight::get().reads(1)); } @@ -161,6 +178,99 @@ pub mod v1 { } } +pub mod v2 { + use super::*; + + /// The log target. + const TARGET: &'static str = "runtime::referenda::migration::v2"; + + pub trait BlockToRelayHeightConversion, I: 'static> { + fn convert_block_number_to_relay_height( + block_number: SystemBlockNumberFor, + ) -> BlockNumberFor; + } + + /// Transforms a submission deposit of ReferendumInfo(Approved|Rejected|Cancelled|TimedOut) to + /// optional value, making it refundable. + pub struct MigrateV1ToV2( + PhantomData<(T, I)>, + PhantomData, + ); + impl, T: Config, I: 'static> + OnRuntimeUpgrade for crate::migration::v2::MigrateV1ToV2 + { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, TryRuntimeError> { + let referendum_count = v1::ReferendumInfoFor::::iter().count(); + log::info!( + target: TARGET, + "pre-upgrade state contains '{}' referendums.", + referendum_count + ); + Ok((referendum_count as u32).encode()) + } + + fn on_runtime_upgrade() -> Weight { + let in_code_version = Pallet::::in_code_storage_version(); + let on_chain_version = Pallet::::on_chain_storage_version(); + let mut weight = T::DbWeight::get().reads(1); + log::info!( + target: TARGET, + "running migration with in-code storage version {:?} / onchain {:?}.", + in_code_version, + on_chain_version + ); + if on_chain_version != 1 { + log::warn!(target: TARGET, "skipping migration from v1 to v2."); + return weight + } + v1::ReferendumInfoFor::::iter().for_each(|(key, value)| { + let maybe_new_value = match value { + ReferendumInfo::Ongoing(_) | ReferendumInfo::Killed(_) => None, + ReferendumInfo::Approved(e, s, d) => { + let new_e = BlockConversion::convert_block_number_to_relay_height(e); + Some(ReferendumInfo::Approved(new_e, s, d)) + }, + ReferendumInfo::Rejected(e, s, d) => { + let new_e = BlockConversion::convert_block_number_to_relay_height(e); + Some(ReferendumInfo::Rejected(new_e, s, d)) + }, + ReferendumInfo::Cancelled(e, s, d) => { + let new_e = BlockConversion::convert_block_number_to_relay_height(e); + Some(ReferendumInfo::Cancelled(new_e, s, d)) + }, + ReferendumInfo::TimedOut(e, s, d) => { + let new_e = BlockConversion::convert_block_number_to_relay_height(e); + Some(ReferendumInfo::TimedOut(new_e, s, d)) + }, + }; + if let Some(new_value) = maybe_new_value { + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + log::info!(target: TARGET, "migrating referendum #{:?}", &key); + ReferendumInfoFor::::insert(key, new_value); + } else { + weight.saturating_accrue(T::DbWeight::get().reads(1)); + } + }); + StorageVersion::new(1).put::>(); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + weight + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { + let on_chain_version = Pallet::::on_chain_storage_version(); + ensure!(on_chain_version == 1, "must upgrade from version 1 to 2."); + let pre_referendum_count: u32 = Decode::decode(&mut &state[..]) + .expect("failed to decode the state from pre-upgrade."); + let post_referendum_count = ReferendumInfoFor::::iter().count() as u32; + ensure!(post_referendum_count == pre_referendum_count, "must migrate all referendums."); + log::info!(target: TARGET, "migrated all referendums."); + Ok(()) + } + } +} + #[cfg(test)] pub mod test { use super::*; @@ -185,7 +295,6 @@ pub mod test { deciding: None, } } - #[test] pub fn referendum_status_v0() { // make sure the bytes of the encoded referendum v0 is decodable. @@ -214,11 +323,11 @@ pub mod test { // run migration from v0 to v1. v1::MigrateV0ToV1::::on_runtime_upgrade(); // fetch and assert migrated into v1 the ongoing referendum. - let ongoing_v1 = ReferendumInfoFor::::get(2).unwrap(); + let ongoing_v1 = v1::ReferendumInfoFor::::get(2).unwrap(); // referendum status schema is the same for v0 and v1. assert_eq!(ReferendumInfoOf::::Ongoing(status_v0), ongoing_v1); // fetch and assert migrated into v1 the approved referendum. - let approved_v1 = ReferendumInfoFor::::get(5).unwrap(); + let approved_v1 = v1::ReferendumInfoFor::::get(5).unwrap(); assert_eq!( approved_v1, ReferendumInfoOf::::Approved( From 3214f28a03e09ef768a1b6d5f595470a48d34534 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Tue, 14 Jan 2025 15:42:30 +0100 Subject: [PATCH 08/16] doc --- substrate/frame/referenda/src/migration.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index fcef265d0b83..3f9b821c9edd 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -190,8 +190,7 @@ pub mod v2 { ) -> BlockNumberFor; } - /// Transforms a submission deposit of ReferendumInfo(Approved|Rejected|Cancelled|TimedOut) to - /// optional value, making it refundable. + /// Transforms SystemBlockNumberFor to BlockNumberFor pub struct MigrateV1ToV2( PhantomData<(T, I)>, PhantomData, From c905dcf29e5875181ee33aa87350a7157587cd0a Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Tue, 14 Jan 2025 23:18:37 +0100 Subject: [PATCH 09/16] conversation nits --- substrate/frame/referenda/src/lib.rs | 4 +++- substrate/frame/referenda/src/migration.rs | 19 +++++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index daa3dc56191d..e6a895f9c593 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -243,7 +243,9 @@ pub mod pallet { /// The preimage provider. type Preimages: QueryPreimage + StorePreimage; - /// Provider for the block number. Normally this is the `frame_system` pallet. + /// Provider for the block number. + /// + /// Normally this is the `frame_system` pallet. type BlockNumberProvider: BlockNumberProvider; } diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 3f9b821c9edd..32221ff92522 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -178,13 +178,19 @@ pub mod v1 { } } -pub mod v2 { +/// Migration for when changing the block number provider. +/// +/// This migration is not guarded +pub mod switch_block_number_provider { use super::*; /// The log target. - const TARGET: &'static str = "runtime::referenda::migration::v2"; - + const TARGET: &'static str = "runtime::referenda::migration::change_block_number_provider"; + /// Convert from one to another block number provider/type. pub trait BlockToRelayHeightConversion, I: 'static> { + /// Convert the `old` block number type to the new block number type. + /// + /// Any changes in the rate of blocks need to be taken into account. fn convert_block_number_to_relay_height( block_number: SystemBlockNumberFor, ) -> BlockNumberFor; @@ -196,7 +202,7 @@ pub mod v2 { PhantomData, ); impl, T: Config, I: 'static> - OnRuntimeUpgrade for crate::migration::v2::MigrateV1ToV2 + OnRuntimeUpgrade for MigrateV1ToV2 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { @@ -219,10 +225,7 @@ pub mod v2 { in_code_version, on_chain_version ); - if on_chain_version != 1 { - log::warn!(target: TARGET, "skipping migration from v1 to v2."); - return weight - } + v1::ReferendumInfoFor::::iter().for_each(|(key, value)| { let maybe_new_value = match value { ReferendumInfo::Ongoing(_) | ReferendumInfo::Killed(_) => None, From f488c5107a2628196dcdf5ab5ba353a1bf5ebdac Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Thu, 16 Jan 2025 14:24:07 +0100 Subject: [PATCH 10/16] change trait for conversion in migration --- substrate/frame/referenda/src/migration.rs | 25 +++++++++++----------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 32221ff92522..16027dea7c78 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -187,22 +187,23 @@ pub mod switch_block_number_provider { /// The log target. const TARGET: &'static str = "runtime::referenda::migration::change_block_number_provider"; /// Convert from one to another block number provider/type. - pub trait BlockToRelayHeightConversion, I: 'static> { + pub trait BlockNumberConversion { /// Convert the `old` block number type to the new block number type. /// /// Any changes in the rate of blocks need to be taken into account. - fn convert_block_number_to_relay_height( - block_number: SystemBlockNumberFor, - ) -> BlockNumberFor; + fn convert_block_number(block_number: Old) -> New; } /// Transforms SystemBlockNumberFor to BlockNumberFor - pub struct MigrateV1ToV2( + pub struct MigrateV1ToV2( PhantomData<(T, I)>, - PhantomData, + PhantomData, ); - impl, T: Config, I: 'static> - OnRuntimeUpgrade for MigrateV1ToV2 + impl, T: Config, I: 'static> OnRuntimeUpgrade + for MigrateV1ToV2 + where + BlockConverter: BlockNumberConversion, BlockNumberFor>, + T: Config, { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { @@ -230,19 +231,19 @@ pub mod switch_block_number_provider { let maybe_new_value = match value { ReferendumInfo::Ongoing(_) | ReferendumInfo::Killed(_) => None, ReferendumInfo::Approved(e, s, d) => { - let new_e = BlockConversion::convert_block_number_to_relay_height(e); + let new_e = BlockConverter::convert_block_number(e); Some(ReferendumInfo::Approved(new_e, s, d)) }, ReferendumInfo::Rejected(e, s, d) => { - let new_e = BlockConversion::convert_block_number_to_relay_height(e); + let new_e = BlockConverter::convert_block_number(e); Some(ReferendumInfo::Rejected(new_e, s, d)) }, ReferendumInfo::Cancelled(e, s, d) => { - let new_e = BlockConversion::convert_block_number_to_relay_height(e); + let new_e = BlockConverter::convert_block_number(e); Some(ReferendumInfo::Cancelled(new_e, s, d)) }, ReferendumInfo::TimedOut(e, s, d) => { - let new_e = BlockConversion::convert_block_number_to_relay_height(e); + let new_e = BlockConverter::convert_block_number(e); Some(ReferendumInfo::TimedOut(new_e, s, d)) }, }; From 12c1476af9fc7de2545e0df2bbb0034c97048ca8 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Thu, 16 Jan 2025 15:12:54 +0100 Subject: [PATCH 11/16] standalone function for migration --- substrate/frame/referenda/src/migration.rs | 92 ++++++++++++---------- 1 file changed, 52 insertions(+), 40 deletions(-) diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 16027dea7c78..9404e6c88ebb 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -217,46 +217,8 @@ pub mod switch_block_number_provider { } fn on_runtime_upgrade() -> Weight { - let in_code_version = Pallet::::in_code_storage_version(); - let on_chain_version = Pallet::::on_chain_storage_version(); - let mut weight = T::DbWeight::get().reads(1); - log::info!( - target: TARGET, - "running migration with in-code storage version {:?} / onchain {:?}.", - in_code_version, - on_chain_version - ); - - v1::ReferendumInfoFor::::iter().for_each(|(key, value)| { - let maybe_new_value = match value { - ReferendumInfo::Ongoing(_) | ReferendumInfo::Killed(_) => None, - ReferendumInfo::Approved(e, s, d) => { - let new_e = BlockConverter::convert_block_number(e); - Some(ReferendumInfo::Approved(new_e, s, d)) - }, - ReferendumInfo::Rejected(e, s, d) => { - let new_e = BlockConverter::convert_block_number(e); - Some(ReferendumInfo::Rejected(new_e, s, d)) - }, - ReferendumInfo::Cancelled(e, s, d) => { - let new_e = BlockConverter::convert_block_number(e); - Some(ReferendumInfo::Cancelled(new_e, s, d)) - }, - ReferendumInfo::TimedOut(e, s, d) => { - let new_e = BlockConverter::convert_block_number(e); - Some(ReferendumInfo::TimedOut(new_e, s, d)) - }, - }; - if let Some(new_value) = maybe_new_value { - weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); - log::info!(target: TARGET, "migrating referendum #{:?}", &key); - ReferendumInfoFor::::insert(key, new_value); - } else { - weight.saturating_accrue(T::DbWeight::get().reads(1)); - } - }); - StorageVersion::new(1).put::>(); - weight.saturating_accrue(T::DbWeight::get().writes(1)); + let mut weight = Weight::zero(); + weight.saturating_accrue(migrate_v1_to_v2::()); weight } @@ -272,6 +234,56 @@ pub mod switch_block_number_provider { Ok(()) } } + + pub fn migrate_v1_to_v2() -> Weight + where + BlockConverter: BlockNumberConversion, BlockNumberFor>, + T: Config, + { + let in_code_version = Pallet::::in_code_storage_version(); + let on_chain_version = Pallet::::on_chain_storage_version(); + let mut weight = T::DbWeight::get().reads(1); + log::info!( + target: "runtime::referenda::migration::change_block_number_provider", + "running migration with in-code storage version {:?} / onchain {:?}.", + in_code_version, + on_chain_version + ); + + // Migration logic here + v1::ReferendumInfoFor::::iter().for_each(|(key, value)| { + let maybe_new_value = match value { + ReferendumInfo::Ongoing(_) | ReferendumInfo::Killed(_) => None, + ReferendumInfo::Approved(e, s, d) => { + let new_e = BlockConverter::convert_block_number(e); + Some(ReferendumInfo::Approved(new_e, s, d)) + } + ReferendumInfo::Rejected(e, s, d) => { + let new_e = BlockConverter::convert_block_number(e); + Some(ReferendumInfo::Rejected(new_e, s, d)) + } + ReferendumInfo::Cancelled(e, s, d) => { + let new_e = BlockConverter::convert_block_number(e); + Some(ReferendumInfo::Cancelled(new_e, s, d)) + } + ReferendumInfo::TimedOut(e, s, d) => { + let new_e = BlockConverter::convert_block_number(e); + Some(ReferendumInfo::TimedOut(new_e, s, d)) + } + }; + if let Some(new_value) = maybe_new_value { + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + log::info!(target: "runtime::referenda::migration::change_block_number_provider", "migrating referendum #{:?}", &key); + ReferendumInfoFor::::insert(key, new_value); + } else { + weight.saturating_accrue(T::DbWeight::get().reads(1)); + } + }); + + StorageVersion::new(1).put::>(); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + weight + } } #[cfg(test)] From 678991b5dfd44b08a35e438e8886e07a9bdf1ee1 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Thu, 16 Jan 2025 15:20:45 +0100 Subject: [PATCH 12/16] standalone function for migration --- substrate/frame/referenda/src/migration.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 9404e6c88ebb..7fd708d843af 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -218,7 +218,7 @@ pub mod switch_block_number_provider { fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); - weight.saturating_accrue(migrate_v1_to_v2::()); + weight.saturating_accrue(migrate_block_number_provider::()); weight } @@ -235,7 +235,7 @@ pub mod switch_block_number_provider { } } - pub fn migrate_v1_to_v2() -> Weight + pub fn migrate_block_number_provider() -> Weight where BlockConverter: BlockNumberConversion, BlockNumberFor>, T: Config, @@ -257,23 +257,23 @@ pub mod switch_block_number_provider { ReferendumInfo::Approved(e, s, d) => { let new_e = BlockConverter::convert_block_number(e); Some(ReferendumInfo::Approved(new_e, s, d)) - } + }, ReferendumInfo::Rejected(e, s, d) => { let new_e = BlockConverter::convert_block_number(e); Some(ReferendumInfo::Rejected(new_e, s, d)) - } + }, ReferendumInfo::Cancelled(e, s, d) => { let new_e = BlockConverter::convert_block_number(e); Some(ReferendumInfo::Cancelled(new_e, s, d)) - } + }, ReferendumInfo::TimedOut(e, s, d) => { let new_e = BlockConverter::convert_block_number(e); Some(ReferendumInfo::TimedOut(new_e, s, d)) - } + }, }; if let Some(new_value) = maybe_new_value { weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); - log::info!(target: "runtime::referenda::migration::change_block_number_provider", "migrating referendum #{:?}", &key); + log::info!(target: TARGET, "migrating referendum #{:?}", &key); ReferendumInfoFor::::insert(key, new_value); } else { weight.saturating_accrue(T::DbWeight::get().reads(1)); From 77ab73d6315de62375c41667c223d45d1c010b84 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Sat, 1 Feb 2025 09:53:19 +0100 Subject: [PATCH 13/16] test to migrate from v1 to block number provider --- substrate/frame/referenda/src/migration.rs | 55 ++++++++++++++++++++-- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 7fd708d843af..939ebc0dcfdc 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -195,12 +195,12 @@ pub mod switch_block_number_provider { } /// Transforms SystemBlockNumberFor to BlockNumberFor - pub struct MigrateV1ToV2( + pub struct MigrateBlockNumberProvider( PhantomData<(T, I)>, PhantomData, ); impl, T: Config, I: 'static> OnRuntimeUpgrade - for MigrateV1ToV2 + for MigrateBlockNumberProvider where BlockConverter: BlockNumberConversion, BlockNumberFor>, T: Config, @@ -289,7 +289,12 @@ pub mod switch_block_number_provider { #[cfg(test)] pub mod test { use super::*; - use crate::mock::{Test as T, *}; + use crate::{ + migration::switch_block_number_provider::{ + migrate_block_number_provider, BlockNumberConversion, + }, + mock::{Test as T, *}, + }; use core::str::FromStr; // create referendum status v0. @@ -353,4 +358,48 @@ pub mod test { ); }); } + + #[test] + fn migration_v1_to_switch_block_number_provider_works() { + ExtBuilder::default().build_and_execute(|| { + pub struct MockBlockConverter; + + impl BlockNumberConversion, BlockNumberFor> for MockBlockConverter { + fn convert_block_number(block_number: SystemBlockNumberFor) -> BlockNumberFor { + block_number as u64 + } + } + + let referendum_ongoing = v1::ReferendumInfoOf::::Ongoing(create_status_v0()); + let referendum_approved = v1::ReferendumInfoOf::::Approved( + 50, //old block number + Some(Deposit { who: 1, amount: 10 }), + Some(Deposit { who: 2, amount: 20 }), + ); + + ReferendumCount::::mutate(|x| x.saturating_inc()); + v1::ReferendumInfoFor::::insert(1, referendum_ongoing); + + ReferendumCount::::mutate(|x| x.saturating_inc()); + v1::ReferendumInfoFor::::insert(2, referendum_approved); + + let weight = migrate_block_number_provider::(); + + let ongoing_v2 = ReferendumInfoFor::::get(1).unwrap(); + assert_eq!( + ongoing_v2, + ReferendumInfoOf::::Ongoing(create_status_v0()) + ); + + let approved_v2 = ReferendumInfoFor::::get(2).unwrap(); + assert_eq!( + approved_v2, + ReferendumInfoOf::::Approved( + 50, + Some(Deposit { who: 1, amount: 10 }), + Some(Deposit { who: 2, amount: 20 }) + ) + ); + }); + } } From f79afd8144ebbec931b5c3686297a8dad72e3fb8 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Sat, 1 Feb 2025 10:13:02 +0100 Subject: [PATCH 14/16] nit on conversation --- substrate/frame/referenda/src/migration.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 939ebc0dcfdc..fc5d3565af16 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -249,6 +249,10 @@ pub mod switch_block_number_provider { in_code_version, on_chain_version ); + if on_chain_version == 0 { + log::error!(target: TARGET, "skipping migration from v0 to switch_block_number_provider."); + return weight + } // Migration logic here v1::ReferendumInfoFor::::iter().for_each(|(key, value)| { @@ -280,8 +284,6 @@ pub mod switch_block_number_provider { } }); - StorageVersion::new(1).put::>(); - weight.saturating_accrue(T::DbWeight::get().writes(1)); weight } } From b95e2285fdcf97d47b5045e5b522b81ad4d6c0eb Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Sat, 1 Feb 2025 13:18:59 +0100 Subject: [PATCH 15/16] nit based on conversation --- prdoc/pr_6338.prdoc | 2 +- substrate/frame/referenda/src/migration.rs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/prdoc/pr_6338.prdoc b/prdoc/pr_6338.prdoc index 7ea43f924d0d..68d01904d071 100644 --- a/prdoc/pr_6338.prdoc +++ b/prdoc/pr_6338.prdoc @@ -11,4 +11,4 @@ doc: crates: - name: pallet-referenda - bump: minor \ No newline at end of file + bump: major \ No newline at end of file diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index fc5d3565af16..8c15c5b2b821 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -194,7 +194,7 @@ pub mod switch_block_number_provider { fn convert_block_number(block_number: Old) -> New; } - /// Transforms SystemBlockNumberFor to BlockNumberFor + /// Transforms `SystemBlockNumberFor` to `BlockNumberFor` pub struct MigrateBlockNumberProvider( PhantomData<(T, I)>, PhantomData, @@ -368,7 +368,7 @@ pub mod test { impl BlockNumberConversion, BlockNumberFor> for MockBlockConverter { fn convert_block_number(block_number: SystemBlockNumberFor) -> BlockNumberFor { - block_number as u64 + block_number as u64 + 10u64 } } @@ -385,8 +385,6 @@ pub mod test { ReferendumCount::::mutate(|x| x.saturating_inc()); v1::ReferendumInfoFor::::insert(2, referendum_approved); - let weight = migrate_block_number_provider::(); - let ongoing_v2 = ReferendumInfoFor::::get(1).unwrap(); assert_eq!( ongoing_v2, From 0f9749abd8370c22fd84eb9ada37daa8269411ad Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Sat, 1 Feb 2025 13:30:49 +0100 Subject: [PATCH 16/16] revert --- substrate/frame/referenda/src/migration.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs index 8c15c5b2b821..c94896649bea 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -385,6 +385,8 @@ pub mod test { ReferendumCount::::mutate(|x| x.saturating_inc()); v1::ReferendumInfoFor::::insert(2, referendum_approved); + migrate_block_number_provider::(); + let ongoing_v2 = ReferendumInfoFor::::get(1).unwrap(); assert_eq!( ongoing_v2,