From 07c0f8b282c4699967ecff58914edb3699c6ad77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Thu, 21 Mar 2024 23:17:26 -0500 Subject: [PATCH 1/6] feat(pallet-referenda): implement Finalizing state via service_auction --- substrate/frame/referenda/src/lib.rs | 248 ++++++++++++++++++++++----- 1 file changed, 202 insertions(+), 46 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index e616056c3022..ae3d2c16cd38 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -64,7 +64,7 @@ #![recursion_limit = "256"] #![cfg_attr(not(feature = "std"), no_std)] -use codec::{Codec, Encode}; +use codec::{Codec, Decode, Encode}; use frame_support::{ dispatch::DispatchResult, ensure, @@ -74,7 +74,7 @@ use frame_support::{ DispatchTime, }, Currency, LockIdentifier, OnUnbalanced, OriginTrait, PollStatus, Polling, QueryPreimage, - ReservableCurrency, StorePreimage, VoteTally, + Randomness, ReservableCurrency, StorePreimage, VoteTally, }, BoundedVec, }; @@ -174,6 +174,8 @@ pub mod pallet { PalletsOriginOf, Hasher = Self::Hashing, >; + /// Something that provides randomness in the runtime. + type Randomness: Randomness>; /// Currency type for this pallet. type Currency: ReservableCurrency; // Origins and unbalances. @@ -247,6 +249,18 @@ pub mod pallet { pub type ReferendumInfoFor, I: 'static = ()> = StorageMap<_, Blake2_128Concat, ReferendumIndex, ReferendumInfoOf>; + /// State change within confirm period. + #[pallet::storage] + pub type PassingStatusInConfirmPeriod, I: 'static = ()> = StorageDoubleMap< + _, + Blake2_128Concat, + ReferendumIndex, + Blake2_128Concat, + BlockNumberFor, + bool, + ValueQuery, + >; + /// The sorted list of referenda ready to be decided but not yet being decided, ordered by /// conviction-weighted approvals. /// @@ -613,10 +627,23 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { ensure_root(origin)?; let now = frame_system::Pallet::::block_number(); - let mut status = Self::ensure_ongoing(index)?; - // This is our wake-up, so we can disregard the alarm. - status.alarm = None; - let (info, dirty, branch) = Self::service_referendum(now, index, status); + + let info = ReferendumInfoFor::::get(index); + + let (info, dirty, branch) = match info { + Some(ReferendumInfo::Ongoing(mut status)) => { + // This is our wake-up, so we can disregard the alarm. + status.alarm = None; + Self::service_referendum(now, index, status) + }, + Some(ReferendumInfo::Finalizing(mut status)) => { + status.alarm = None; + Self::service_auction(now, index, status) + }, + Some(info) => (info, false, ServiceBranch::Fail), + _ => Err(Error::::NotOngoing)?, + }; + if dirty { ReferendumInfoFor::::insert(index, info); } @@ -1055,8 +1082,10 @@ impl, I: 'static> Pallet { /// Advance the state of a referendum, which comes down to: /// - If it's ready to be decided, start deciding; /// - If it's not ready to be decided and non-deciding timeout has passed, fail; - /// - If it's ongoing and passing, ensure confirming; if at end of confirmation period, pass. - /// - If it's ongoing and not passing, stop confirming; if it has reached end time, fail. + /// - If it's ongoing and passing, ensure confirming; if at end of confirmation period, decide + /// by "candle". + /// - If it's ongoing and not passing, stop confirming; if it has reached end time, decide by + /// "candle". /// /// Weight will be a bit different depending on what it does, but it's designed so as not to /// differ dramatically, especially if `MaxQueue` is kept small. In particular _there are no @@ -1159,26 +1188,32 @@ impl, I: 'static> Pallet { branch = if is_passing { match deciding.confirming { Some(t) if now >= t => { - // Passed! - Self::ensure_no_alarm(&mut status); - Self::note_one_fewer_deciding(status.track); - let (desired, call) = (status.enactment, status.proposal); - Self::schedule_enactment(index, track, desired, status.origin, call); - Self::deposit_event(Event::::Confirmed { + // Ongoing is now Finished. From now on, it'll be impossible to modify the + // poll. The passing status will be now decided by the auction, and the + // information stored under `PassingStatusInConfirmPeriod`. + + // Sent to Auction only after confirm period finishes. Whether it will pass + // or not depends on the status history when confirming, and a random point + // in time within confirm period. + PassingStatusInConfirmPeriod::::insert( index, - tally: status.tally, - }); + now.saturating_less_one(), + is_passing, + ); + Self::ensure_alarm_at(&mut status, index, now.saturating_plus_one()); + return ( - ReferendumInfo::Approved( - now, - Some(status.submission_deposit), - status.decision_deposit, - ), + ReferendumInfo::Finalizing(status), true, - ServiceBranch::Approved, + ServiceBranch::ContinueConfirming, ) }, - Some(_) => ServiceBranch::ContinueConfirming, + Some(_) => { + // We don't care if failing within confirm period. + // Report the change of state, and continue. + PassingStatusInConfirmPeriod::::insert(index, now, is_passing); + ServiceBranch::ContinueConfirming + }, None => { // Start confirming dirty = true; @@ -1188,29 +1223,64 @@ impl, I: 'static> Pallet { }, } } else { - if now >= deciding.since.saturating_add(track.decision_period) { - // Failed! - Self::ensure_no_alarm(&mut status); - Self::note_one_fewer_deciding(status.track); - Self::deposit_event(Event::::Rejected { index, tally: status.tally }); - return ( - ReferendumInfo::Rejected( - now, - Some(status.submission_deposit), - status.decision_deposit, - ), - true, - ServiceBranch::Rejected, - ) - } - if deciding.confirming.is_some() { - // Stop confirming - dirty = true; - deciding.confirming = None; - Self::deposit_event(Event::::ConfirmAborted { index }); - ServiceBranch::EndConfirming - } else { - ServiceBranch::ContinueNotConfirming + match deciding.confirming { + Some(_) if now <= deciding.since.saturating_add(track.decision_period) => { + // Stop confirming + dirty = true; + deciding.confirming = None; + Self::deposit_event(Event::::ConfirmAborted { index }); + ServiceBranch::EndConfirming + }, + Some(t) if now >= t => { + // Ongoing is now Finished. From now on, it'll be impossible to modify the + // poll. The passing status will be now decided by the auction, and the + // information stored under `PassingStatusInConfirmPeriod`. + + // Sent to Auction only after confirm period finishes. Whether it will pass + // or not depends on the status history when confirming, and a random point + // in time within confirm period. + PassingStatusInConfirmPeriod::::insert( + index, + now.saturating_less_one(), + is_passing, + ); + Self::ensure_alarm_at(&mut status, index, now.saturating_plus_one()); + + return ( + ReferendumInfo::Finalizing(status), + true, + ServiceBranch::ContinueConfirming, + ) + }, + Some(_) => { + // We don't care if failing within confirm period. + // Report the change of state, and continue. + PassingStatusInConfirmPeriod::::insert(index, now, is_passing); + ServiceBranch::ContinueConfirming + }, + None => { + if now >= deciding.since.saturating_add(track.decision_period) { + // Exceeded decision period without having passed in first place. + // Failed! + Self::ensure_no_alarm(&mut status); + Self::note_one_fewer_deciding(status.track); + Self::deposit_event(Event::::Rejected { + index, + tally: status.tally, + }); + return ( + ReferendumInfo::Rejected( + now, + Some(status.submission_deposit), + status.decision_deposit, + ), + true, + ServiceBranch::Rejected, + ); + } else { + ServiceBranch::ContinueNotConfirming + } + }, } }; alarm = Self::decision_time(deciding, &status.tally, status.track, track); @@ -1225,6 +1295,92 @@ impl, I: 'static> Pallet { (ReferendumInfo::Ongoing(status), dirty_alarm || dirty, branch) } + /// "Candle" auction to decide the winning status of confirm period. + /// + /// The "candle": passing or failing of a referendum is ultimately decided as a candle auction + /// where, given a random point in time (defined as `t`), the definitive status of the the + /// referendum is decided by the last status registered before `t`. + fn service_auction( + now: BlockNumberFor, + index: ReferendumIndex, + mut status: ReferendumStatusOf, + ) -> (ReferendumInfoOf, bool, ServiceBranch) { + // Note: it'd be weird to come up to this point and yet not have a track + let track = match Self::track(status.clone().track) { + Some(x) => x, + None => return (ReferendumInfo::Finalizing(status), false, ServiceBranch::Fail), + }; + + let confirming_until = status + .clone() + .deciding + .expect("having passed ongoing, we should have times for decision; qed") + .confirming + .expect("having finished confirming, we should have a confirming_until time; qed"); + + let confirming_since = confirming_until.saturating_sub(track.confirm_period); + + let (raw_offset, known_since) = T::Randomness::random(&b"confirm_auction"[..]); + + // Do not use random until made sure random seed is not known before confirm ends + if known_since <= confirming_until { + Self::ensure_alarm_at(&mut status, index, now.saturating_plus_one()); + return (ReferendumInfo::Finalizing(status), false, ServiceBranch::ContinueConfirming); + } + + let raw_offset_block_number = >::decode(&mut raw_offset.as_ref()) + .expect("secure hashes should always be bigger than the block number; qed"); + + let candle_block_number = if confirming_since <= raw_offset_block_number + && raw_offset_block_number < confirming_until + { + raw_offset_block_number + } else { + confirming_since.saturating_add(raw_offset_block_number % track.confirm_period) + }; + + let mut statuses = + PassingStatusInConfirmPeriod::::iter_prefix(index).collect::>(); + statuses.sort_by(|(a, _), (b, _)| b.cmp(a)); + + let (_, winning_status) = statuses + .into_iter() + .find(|(when, _)| when <= &candle_block_number) + .unwrap_or((now, true)); + + if winning_status { + // Passed! + Self::ensure_no_alarm(&mut status); + Self::note_one_fewer_deciding(status.track); + let (desired, call) = (status.enactment, status.proposal); + Self::schedule_enactment(index, track, desired, status.origin, call); + Self::deposit_event(Event::::Confirmed { index, tally: status.tally }); + ( + ReferendumInfo::Approved( + now, + Some(status.submission_deposit), + status.decision_deposit, + ), + true, + ServiceBranch::Approved, + ) + } else { + // Failed! + Self::ensure_no_alarm(&mut status); + Self::note_one_fewer_deciding(status.track); + Self::deposit_event(Event::::Rejected { index, tally: status.tally }); + return ( + ReferendumInfo::Rejected( + now, + Some(status.submission_deposit), + status.decision_deposit, + ), + true, + ServiceBranch::Rejected, + ); + } + } + /// 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( From dbc60fabf62ae649091be5aab0b767b9554052cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Thu, 21 Mar 2024 23:19:41 -0500 Subject: [PATCH 2/6] change(pallet-referenda): adjust testing to consider finalizing state --- substrate/frame/referenda/src/mock.rs | 112 +++++++++++++++++++++++-- substrate/frame/referenda/src/tests.rs | 95 ++++++++++++++------- 2 files changed, 171 insertions(+), 36 deletions(-) diff --git a/substrate/frame/referenda/src/mock.rs b/substrate/frame/referenda/src/mock.rs index 135476d7cb13..ee58dce4f15e 100644 --- a/substrate/frame/referenda/src/mock.rs +++ b/substrate/frame/referenda/src/mock.rs @@ -21,7 +21,7 @@ use super::*; use crate as pallet_referenda; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ - assert_ok, derive_impl, ord_parameter_types, parameter_types, + assert_ok, derive_impl, ord_parameter_types, parameter_types, storage_alias, traits::{ ConstU32, ConstU64, Contains, EqualPrivilegeOnly, OnInitialize, OriginTrait, Polling, SortedMembers, @@ -29,6 +29,7 @@ use frame_support::{ weights::Weight, }; use frame_system::{EnsureRoot, EnsureSignedBy}; +use sp_core::H256; use sp_runtime::{ traits::{BlakeTwo256, Hash}, BuildStorage, DispatchResult, Perbill, @@ -118,12 +119,44 @@ impl SortedMembers for OneToFive { fn add(_m: &u64) {} } +#[storage_alias] +pub type TestRandomnessBlockNumberOffset, I: 'static> = + StorageValue, BlockNumberFor>; + +pub struct TestRandomness; + +impl TestRandomness { + pub fn place_candle() { + TestRandomnessBlockNumberOffset::::set(Some(System::block_number())); + } + + fn hash_256_like_for(n: BlockNumberFor) -> [u8; 32] { + let mut encoded = n.encode(); + while encoded.len() < 32 { + encoded.push(0); + } + + encoded.try_into().expect("added bytes until 32") + } +} + +impl frame_support::traits::Randomness> for TestRandomness { + fn random(_: &[u8]) -> (H256, BlockNumberFor) { + let block_number = TestRandomnessBlockNumberOffset::::get() + .unwrap_or(System::block_number().saturating_less_one()); + + let block_hash: H256 = Self::hash_256_like_for(block_number).into(); + + (block_hash, System::block_number()) + } +} + pub struct TestTracksInfo; impl TracksInfo for TestTracksInfo { type Id = u8; type RuntimeOrigin = ::PalletsOrigin; fn tracks() -> &'static [(Self::Id, TrackInfo)] { - static DATA: [(u8, TrackInfo); 2] = [ + static DATA: [(u8, TrackInfo); 3] = [ ( 0u8, TrackInfo { @@ -168,6 +201,28 @@ impl TracksInfo for TestTracksInfo { }, }, ), + ( + 2u8, + TrackInfo { + name: "signed by 100", + max_deciding: 1, + decision_deposit: 1, + prepare_period: 1, + decision_period: 14, + confirm_period: 7, + min_enactment_period: 2, + min_approval: Curve::LinearDecreasing { + length: Perbill::from_percent(100), + floor: Perbill::from_percent(50), + ceil: Perbill::from_percent(100), + }, + min_support: Curve::LinearDecreasing { + length: Perbill::from_percent(50), + floor: Perbill::from_percent(0), + ceil: Perbill::from_percent(100), + }, + }, + ), ]; &DATA[..] } @@ -176,6 +231,7 @@ impl TracksInfo for TestTracksInfo { match system_origin { frame_system::RawOrigin::Root => Ok(0), frame_system::RawOrigin::None => Ok(1), + frame_system::RawOrigin::Signed(100) => Ok(2), _ => Err(()), } } else { @@ -190,6 +246,7 @@ impl Config for Test { type RuntimeCall = RuntimeCall; type RuntimeEvent = RuntimeEvent; type Scheduler = Scheduler; + type Randomness = TestRandomness; type Currency = pallet_balances::Pallet; type SubmitOrigin = frame_system::EnsureSigned; type CancelOrigin = EnsureSignedBy; @@ -215,7 +272,7 @@ impl Default for ExtBuilder { impl ExtBuilder { pub fn build(self) -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - let balances = vec![(1, 100), (2, 100), (3, 100), (4, 100), (5, 100), (6, 100)]; + let balances = vec![(1, 100), (2, 100), (3, 100), (4, 100), (5, 100), (6, 100), (100, 100)]; pallet_balances::GenesisConfig:: { balances } .assimilate_storage(&mut t) .unwrap(); @@ -293,6 +350,11 @@ pub fn set_balance_proposal_bounded(value: u64) -> BoundedCallOf { ::bound(c).unwrap() } +pub fn transfer_balance_proposal_bounded(value: u64) -> BoundedCallOf { + let c = RuntimeCall::Balances(pallet_balances::Call::transfer_keep_alive { dest: 42, value }); + ::bound(c).unwrap() +} + #[allow(dead_code)] pub fn propose_set_balance(who: u64, value: u64, delay: u64) -> DispatchResult { Referenda::submit( @@ -358,6 +420,10 @@ pub fn deciding_and_failing_since(i: ReferendumIndex) -> u64 { deciding: Some(DecidingStatus { since, confirming: None, .. }), .. }) => since, + ReferendumInfo::Finalizing(ReferendumStatus { + deciding: Some(DecidingStatus { since, confirming: None, .. }), + .. + }) => since, _ => panic!("Not deciding"), } } @@ -422,13 +488,47 @@ pub enum RefState { } impl RefState { + /// Create default referendum: + /// + /// - **Track**: _Root_ (`0`) + /// - **Call**: `Balances::force_set_balance(42, 1)` pub fn create(self) -> ReferendumIndex { + Self::create_with_params( + self, + frame_support::dispatch::RawOrigin::Root.into(), + set_balance_proposal_bounded(1), + ) + } + + /// Create referendum with long duration: + /// + /// - **Track**: _Signed by 100_ (`2`) + /// - **Call**: `Balances::transfer_keep_alive(42, 10)` + pub fn create_long(self) -> ReferendumIndex { + Self::create_with_params( + self, + frame_support::dispatch::RawOrigin::Signed(100).into(), + transfer_balance_proposal_bounded(10), + ) + } + + pub fn create_with_params( + self, + origin: PalletsOriginOf, + call: BoundedCallOf, + ) -> ReferendumIndex { assert_ok!(Referenda::submit( RuntimeOrigin::signed(1), - Box::new(frame_support::dispatch::RawOrigin::Root.into()), - set_balance_proposal_bounded(1), + Box::new(origin.clone()), + call, DispatchTime::At(10), )); + + let track_id = TestTracksInfo::track_for(&origin) + .expect("track should exist as the ref was successfully submitted; qed"); + let tracks = TestTracksInfo::tracks(); + let (_, track) = &tracks[track_id as usize]; + assert_ok!(Referenda::place_decision_deposit(RuntimeOrigin::signed(2), 0)); if matches!(self, RefState::Confirming { immediate: true }) { set_tally(0, 100, 0); @@ -442,7 +542,7 @@ impl RefState { run_to(System::block_number() + 1); } if matches!(self, RefState::Confirming { .. }) { - assert_eq!(confirming_until(index), System::block_number() + 2); + assert_eq!(confirming_until(index), System::block_number() + track.confirm_period); } if matches!(self, RefState::Passing) { set_tally(0, 100, 99); diff --git a/substrate/frame/referenda/src/tests.rs b/substrate/frame/referenda/src/tests.rs index 8f51136de0bf..66ad2eae2d64 100644 --- a/substrate/frame/referenda/src/tests.rs +++ b/substrate/frame/referenda/src/tests.rs @@ -30,7 +30,7 @@ fn params_should_work() { ExtBuilder::default().build_and_execute(|| { assert_eq!(ReferendumCount::::get(), 0); assert_eq!(Balances::free_balance(42), 0); - assert_eq!(Balances::total_issuance(), 600); + assert_eq!(Balances::total_issuance(), 700); }); } @@ -57,14 +57,14 @@ fn basic_happy_path_works() { set_tally(0, 100, 0); run_to(7); assert_eq!(confirming_until(0), 9); - run_to(9); + run_to(10); // #8: Should be confirmed & ended. - assert_eq!(approved_since(0), 9); + assert_eq!(approved_since(0), 10); assert_ok!(Referenda::refund_decision_deposit(RuntimeOrigin::signed(2), 0)); - run_to(12); + run_to(13); // #9: Should not yet be enacted. assert_eq!(Balances::free_balance(&42), 0); - run_to(13); + run_to(14); // #10: Proposal should be executed. assert_eq!(Balances::free_balance(&42), 1); }); @@ -89,8 +89,8 @@ fn confirm_then_reconfirm_with_elapsed_trigger_works() { set_tally(r, 100, 99); run_to(8); assert_eq!(deciding_and_failing_since(r), 5); - run_to(11); - assert_eq!(approved_since(r), 11); + run_to(12); + assert_eq!(approved_since(r), 12); }); } @@ -103,8 +103,8 @@ fn instaconfirm_then_reconfirm_with_elapsed_trigger_works() { set_tally(r, 100, 99); run_to(7); assert_eq!(deciding_and_failing_since(r), 5); - run_to(11); - assert_eq!(approved_since(r), 11); + run_to(12); + assert_eq!(approved_since(r), 12); }); } @@ -121,8 +121,8 @@ fn instaconfirm_then_reconfirm_with_voting_trigger_works() { set_tally(r, 100, 0); run_to(9); assert_eq!(confirming_until(r), 11); - run_to(11); - assert_eq!(approved_since(r), 11); + run_to(12); + assert_eq!(approved_since(r), 12); }); } @@ -132,21 +132,22 @@ fn voting_should_extend_for_late_confirmation() { let r = Passing.create(); run_to(10); assert_eq!(confirming_until(r), 11); - run_to(11); - assert_eq!(approved_since(r), 11); + run_to(12); + assert_eq!(approved_since(r), 12); }); } #[test] -fn should_instafail_during_extension_confirmation() { +fn should_fails_during_extension_confirmation_if_on_candle() { ExtBuilder::default().build_and_execute(|| { let r = Passing.create(); run_to(10); assert_eq!(confirming_until(r), 11); - // Should insta-fail since it's now past the normal voting time. + TestRandomness::place_candle(); + // Should record failure. I'll fail anyways set_tally(r, 100, 101); - run_to(11); - assert_eq!(rejected_since(r), 11); + run_to(12); + assert_eq!(rejected_since(r), 12); }); } @@ -165,6 +166,41 @@ fn confirming_then_fail_works() { }); } +// TODO: Add a new test to decide using candle method +// Note: use a deterministic (i.e. manually set the seed) +// method to be able to accurately determine which is +// the "winner" block +#[test] +fn confirming_decided_by_candle() { + ExtBuilder::default().build_and_execute(|| { + let r = Passing.create_long(); + // Normally ends at 5 + 4 (voting period) = 9. + assert_eq!(deciding_and_failing_since(r), 2); + run_to(16); + assert_eq!(confirming_until(r), 23); + run_to(19); + TestRandomness::place_candle(); + run_to(20); + set_tally(r, 100, 101); + run_to(24); + assert_eq!(approved_since(r), 24); + }); + + ExtBuilder::default().build_and_execute(|| { + let r = Passing.create_long(); + // Normally ends at 5 + 4 (voting period) = 9. + assert_eq!(deciding_and_failing_since(r), 2); + run_to(16); + assert_eq!(confirming_until(r), 23); + run_to(20); + set_tally(r, 100, 101); + run_to(21); + TestRandomness::place_candle(); + run_to(24); + assert_eq!(rejected_since(r), 24); + }); +} + #[test] fn queueing_works() { ExtBuilder::default().build_and_execute(|| { @@ -231,35 +267,34 @@ fn queueing_works() { println!("{:?}", Vec::<_>::from(TrackQueue::::get(0))); // Let confirmation period end. - run_to(9); + run_to(10); // #4 should have been confirmed. - assert_eq!(approved_since(4), 9); + assert_eq!(approved_since(4), 10); // On to the next block to select the new referendum - run_to(10); + run_to(11); // #1 (the one with the most approvals) should now be being decided. - assert_eq!(deciding_since(1), 10); + assert_eq!(deciding_since(1), 11); // Let it end unsuccessfully. - run_to(14); - assert_eq!(rejected_since(1), 14); + run_to(15); + assert_eq!(rejected_since(1), 15); // Service queue. - run_to(15); + run_to(16); // #2 should now be being decided. It will (barely) pass. - assert_eq!(deciding_and_failing_since(2), 15); + assert_eq!(deciding_and_failing_since(2), 16); // #2 moves into confirming at the last moment with a 50% approval. - run_to(19); - assert_eq!(confirming_until(2), 21); + run_to(20); + assert_eq!(confirming_until(2), 22); // #2 gets approved. - run_to(21); - assert_eq!(approved_since(2), 21); + run_to(23); + assert_eq!(approved_since(2), 23); // The final one has since timed out. - run_to(22); assert_eq!(timed_out_since(3), 22); }); } From 37798c42e0437dd20f174e1b3c4a8baf53c1f703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 26 Mar 2024 16:18:39 -0500 Subject: [PATCH 3/6] change(pallet-referenda): treat `Finalizing` state as a logical (intrinsic) period within `Ongoing` state. --- substrate/frame/referenda/src/lib.rs | 99 ++++++++++++++------------- substrate/frame/referenda/src/mock.rs | 4 -- 2 files changed, 53 insertions(+), 50 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index ae3d2c16cd38..6c053dfb95e6 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -628,22 +628,10 @@ pub mod pallet { ensure_root(origin)?; let now = frame_system::Pallet::::block_number(); - let info = ReferendumInfoFor::::get(index); - - let (info, dirty, branch) = match info { - Some(ReferendumInfo::Ongoing(mut status)) => { - // This is our wake-up, so we can disregard the alarm. - status.alarm = None; - Self::service_referendum(now, index, status) - }, - Some(ReferendumInfo::Finalizing(mut status)) => { - status.alarm = None; - Self::service_auction(now, index, status) - }, - Some(info) => (info, false, ServiceBranch::Fail), - _ => Err(Error::::NotOngoing)?, - }; - + let mut status = Self::ensure_ongoing(index)?; + // This is our wake-up, so we can disregard the alarm. + status.alarm = None; + let (info, dirty, branch) = Self::service_referendum(now, index, status); if dirty { ReferendumInfoFor::::insert(index, info); } @@ -769,8 +757,17 @@ impl, I: 'static> Polling for Pallet { ) -> R { 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 is_finalizing = status + .clone() + .deciding + .map(|d| d.confirming.map(|x| now >= x).unwrap_or(false)) + .unwrap(); + if is_finalizing { + return f(PollStatus::None); + } + + let result = f(PollStatus::Ongoing(&mut status.tally, status.track)); Self::ensure_alarm_at(&mut status, index, now + One::one()); ReferendumInfoFor::::insert(index, ReferendumInfo::Ongoing(status)); result @@ -789,8 +786,17 @@ impl, I: 'static> Polling for Pallet { ) -> Result { 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 is_finalizing = status + .clone() + .deciding + .map(|d| d.confirming.map(|x| now >= x).unwrap_or(false)) + .unwrap(); + if is_finalizing { + return f(PollStatus::None); + } + + let result = f(PollStatus::Ongoing(&mut status.tally, status.track))?; Self::ensure_alarm_at(&mut status, index, now + One::one()); ReferendumInfoFor::::insert(index, ReferendumInfo::Ongoing(status)); Ok(result) @@ -867,7 +873,16 @@ impl, I: 'static> Pallet { index: ReferendumIndex, ) -> Result, DispatchError> { match ReferendumInfoFor::::get(index) { - Some(ReferendumInfo::Ongoing(status)) => Ok(status), + Some(ReferendumInfo::Ongoing(status)) => { + match status.clone().deciding.map(|d| d.confirming) { + Some(Some(confirming)) + if frame_system::Pallet::::block_number() >= confirming => + { + Err(Error::::NotOngoing.into()) + }, + _ => Ok(status), + } + }, _ => Err(Error::::NotOngoing.into()), } } @@ -928,8 +943,8 @@ impl, I: 'static> Pallet { 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`. - let when = (when.saturating_add(alarm_interval.saturating_sub(One::one())) / - alarm_interval) + let when = (when.saturating_add(alarm_interval.saturating_sub(One::one())) + / alarm_interval) .saturating_mul(alarm_interval); let result = T::Scheduler::schedule( DispatchTime::At(when), @@ -1042,7 +1057,7 @@ impl, I: 'static> Pallet { Ok(c) => c, Err(_) => { debug_assert!(false, "Unable to create a bounded call from `one_fewer_deciding`??",); - return + return; }, }; Self::set_alarm(call, next_block); @@ -1069,7 +1084,7 @@ impl, I: 'static> Pallet { false, "Unable to create a bounded call from `nudge_referendum`??", ); - return false + return false; }, }; status.alarm = Self::set_alarm(call, alarm); @@ -1173,7 +1188,7 @@ impl, I: 'static> Pallet { ), true, ServiceBranch::TimedOut, - ) + ); } }, Some(deciding) => { @@ -1185,6 +1200,7 @@ impl, I: 'static> Pallet { &track.min_approval, status.track, ); + branch = if is_passing { match deciding.confirming { Some(t) if now >= t => { @@ -1195,18 +1211,13 @@ impl, I: 'static> Pallet { // Sent to Auction only after confirm period finishes. Whether it will pass // or not depends on the status history when confirming, and a random point // in time within confirm period. + PassingStatusInConfirmPeriod::::insert( index, now.saturating_less_one(), is_passing, ); - Self::ensure_alarm_at(&mut status, index, now.saturating_plus_one()); - - return ( - ReferendumInfo::Finalizing(status), - true, - ServiceBranch::ContinueConfirming, - ) + return Self::service_auction(now, index, status); }, Some(_) => { // We don't care if failing within confirm period. @@ -1244,13 +1255,7 @@ impl, I: 'static> Pallet { now.saturating_less_one(), is_passing, ); - Self::ensure_alarm_at(&mut status, index, now.saturating_plus_one()); - - return ( - ReferendumInfo::Finalizing(status), - true, - ServiceBranch::ContinueConfirming, - ) + return Self::service_auction(now, index, status); }, Some(_) => { // We don't care if failing within confirm period. @@ -1308,7 +1313,7 @@ impl, I: 'static> Pallet { // Note: it'd be weird to come up to this point and yet not have a track let track = match Self::track(status.clone().track) { Some(x) => x, - None => return (ReferendumInfo::Finalizing(status), false, ServiceBranch::Fail), + None => return (ReferendumInfo::Ongoing(status), false, ServiceBranch::Fail), }; let confirming_until = status @@ -1325,7 +1330,9 @@ impl, I: 'static> Pallet { // Do not use random until made sure random seed is not known before confirm ends if known_since <= confirming_until { Self::ensure_alarm_at(&mut status, index, now.saturating_plus_one()); - return (ReferendumInfo::Finalizing(status), false, ServiceBranch::ContinueConfirming); + return (ReferendumInfo::Ongoing(status), false, ServiceBranch::ContinueConfirming); + } else { + Self::ensure_no_alarm(&mut status); } let raw_offset_block_number = >::decode(&mut raw_offset.as_ref()) @@ -1454,8 +1461,8 @@ impl, I: 'static> Pallet { id: TrackIdOf, ) -> bool { let x = Perbill::from_rational(elapsed.min(period), period); - support_needed.passing(x, tally.support(id)) && - approval_needed.passing(x, tally.approval(id)) + support_needed.passing(x, tally.support(id)) + && approval_needed.passing(x, tally.approval(id)) } /// Clear metadata if exist for a given referendum index. @@ -1477,8 +1484,8 @@ impl, I: 'static> Pallet { #[cfg(any(feature = "try-runtime", test))] fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { ensure!( - ReferendumCount::::get() as usize == - ReferendumInfoFor::::iter_keys().count(), + ReferendumCount::::get() as usize + == ReferendumInfoFor::::iter_keys().count(), "Number of referenda in `ReferendumInfoFor` is different than `ReferendumCount`" ); @@ -1516,8 +1523,8 @@ impl, I: 'static> Pallet { if let Some(deciding) = status.deciding { ensure!( - deciding.since < - deciding.confirming.unwrap_or(BlockNumberFor::::max_value()), + deciding.since + < deciding.confirming.unwrap_or(BlockNumberFor::::max_value()), "Deciding status cannot begin before confirming stage." ) } diff --git a/substrate/frame/referenda/src/mock.rs b/substrate/frame/referenda/src/mock.rs index ee58dce4f15e..35068fb61a2b 100644 --- a/substrate/frame/referenda/src/mock.rs +++ b/substrate/frame/referenda/src/mock.rs @@ -420,10 +420,6 @@ pub fn deciding_and_failing_since(i: ReferendumIndex) -> u64 { deciding: Some(DecidingStatus { since, confirming: None, .. }), .. }) => since, - ReferendumInfo::Finalizing(ReferendumStatus { - deciding: Some(DecidingStatus { since, confirming: None, .. }), - .. - }) => since, _ => panic!("Not deciding"), } } From 31a4a1a96ae55969a17f548c50af8ed71440ee3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 26 Mar 2024 17:07:16 -0500 Subject: [PATCH 4/6] fix(pallet-referenda): adjust ongoing checks when finalizing --- substrate/frame/referenda/src/lib.rs | 9 ++++++--- substrate/frame/referenda/src/tests.rs | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index 6c053dfb95e6..c2f11d83737b 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -628,7 +628,10 @@ pub mod pallet { ensure_root(origin)?; let now = frame_system::Pallet::::block_number(); - let mut status = Self::ensure_ongoing(index)?; + let mut status = match ReferendumInfoFor::::get(index) { + Some(ReferendumInfo::Ongoing(status)) => Ok(status), + _ => Err(Error::::NotOngoing), + }?; // This is our wake-up, so we can disregard the alarm. status.alarm = None; let (info, dirty, branch) = Self::service_referendum(now, index, status); @@ -762,7 +765,7 @@ impl, I: 'static> Polling for Pallet { .clone() .deciding .map(|d| d.confirming.map(|x| now >= x).unwrap_or(false)) - .unwrap(); + .unwrap_or(false); if is_finalizing { return f(PollStatus::None); } @@ -791,7 +794,7 @@ impl, I: 'static> Polling for Pallet { .clone() .deciding .map(|d| d.confirming.map(|x| now >= x).unwrap_or(false)) - .unwrap(); + .unwrap_or(false); if is_finalizing { return f(PollStatus::None); } diff --git a/substrate/frame/referenda/src/tests.rs b/substrate/frame/referenda/src/tests.rs index 66ad2eae2d64..bb9ffe4a0dc9 100644 --- a/substrate/frame/referenda/src/tests.rs +++ b/substrate/frame/referenda/src/tests.rs @@ -58,14 +58,14 @@ fn basic_happy_path_works() { run_to(7); assert_eq!(confirming_until(0), 9); run_to(10); - // #8: Should be confirmed & ended. + // #10: Should confirm via auction. assert_eq!(approved_since(0), 10); assert_ok!(Referenda::refund_decision_deposit(RuntimeOrigin::signed(2), 0)); run_to(13); - // #9: Should not yet be enacted. + // #13: Should not yet be enacted. assert_eq!(Balances::free_balance(&42), 0); run_to(14); - // #10: Proposal should be executed. + // #14: Proposal should be executed. assert_eq!(Balances::free_balance(&42), 1); }); } From a391c393102a7fc93103f6faf42f66a9fe734585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 26 Mar 2024 18:05:41 -0500 Subject: [PATCH 5/6] fix(pallet-referenda): use map to store the confirm state changes --- substrate/frame/referenda/src/lib.rs | 50 ++++++++++++++++++---------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index c2f11d83737b..054d3775a668 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -84,7 +84,7 @@ use sp_runtime::{ traits::{AtLeast32BitUnsigned, Bounded, Dispatchable, One, Saturating, Zero}, DispatchError, Perbill, }; -use sp_std::{fmt::Debug, prelude::*}; +use sp_std::{fmt::Debug, ops::Bound::Included, prelude::*}; mod branch; pub mod migration; @@ -142,6 +142,7 @@ pub mod pallet { use super::*; use frame_support::{pallet_prelude::*, traits::EnsureOriginWithArg}; use frame_system::pallet_prelude::*; + use sp_std::collections::btree_map::BTreeMap; /// The in-code storage version. const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); @@ -251,13 +252,12 @@ pub mod pallet { /// State change within confirm period. #[pallet::storage] - pub type PassingStatusInConfirmPeriod, I: 'static = ()> = StorageDoubleMap< + #[pallet::unbounded] + pub type PassingStatusInConfirmPeriod, I: 'static = ()> = StorageMap< _, Blake2_128Concat, ReferendumIndex, - Blake2_128Concat, - BlockNumberFor, - bool, + BTreeMap, bool>, ValueQuery, >; @@ -1097,6 +1097,16 @@ impl, I: 'static> Pallet { } } + fn insert_or_update_passing_status_on_confirm( + index: ReferendumIndex, + when: BlockNumberFor, + state: bool, + ) { + PassingStatusInConfirmPeriod::::mutate(index, |confirm_status| { + confirm_status.insert(when, state); + }); + } + /// Advance the state of a referendum, which comes down to: /// - If it's ready to be decided, start deciding; /// - If it's not ready to be decided and non-deciding timeout has passed, fail; @@ -1215,7 +1225,7 @@ impl, I: 'static> Pallet { // or not depends on the status history when confirming, and a random point // in time within confirm period. - PassingStatusInConfirmPeriod::::insert( + Self::insert_or_update_passing_status_on_confirm( index, now.saturating_less_one(), is_passing, @@ -1225,7 +1235,11 @@ impl, I: 'static> Pallet { Some(_) => { // We don't care if failing within confirm period. // Report the change of state, and continue. - PassingStatusInConfirmPeriod::::insert(index, now, is_passing); + Self::insert_or_update_passing_status_on_confirm( + index, + now.saturating_less_one(), + is_passing, + ); ServiceBranch::ContinueConfirming }, None => { @@ -1253,7 +1267,7 @@ impl, I: 'static> Pallet { // Sent to Auction only after confirm period finishes. Whether it will pass // or not depends on the status history when confirming, and a random point // in time within confirm period. - PassingStatusInConfirmPeriod::::insert( + Self::insert_or_update_passing_status_on_confirm( index, now.saturating_less_one(), is_passing, @@ -1263,7 +1277,11 @@ impl, I: 'static> Pallet { Some(_) => { // We don't care if failing within confirm period. // Report the change of state, and continue. - PassingStatusInConfirmPeriod::::insert(index, now, is_passing); + Self::insert_or_update_passing_status_on_confirm( + index, + now.saturating_less_one(), + is_passing, + ); ServiceBranch::ContinueConfirming }, None => { @@ -1349,16 +1367,12 @@ impl, I: 'static> Pallet { confirming_since.saturating_add(raw_offset_block_number % track.confirm_period) }; - let mut statuses = - PassingStatusInConfirmPeriod::::iter_prefix(index).collect::>(); - statuses.sort_by(|(a, _), (b, _)| b.cmp(a)); - - let (_, winning_status) = statuses - .into_iter() - .find(|(when, _)| when <= &candle_block_number) - .unwrap_or((now, true)); + let statuses = PassingStatusInConfirmPeriod::::get(index); + let statuses = + statuses.range((Included(&confirming_since), Included(&candle_block_number))); + let (_, winning_status) = statuses.last().unwrap_or((&now, &true)); - if winning_status { + if *winning_status { // Passed! Self::ensure_no_alarm(&mut status); Self::note_one_fewer_deciding(status.track); From 8c339c9d74bba917fbe49a755facbf0aeb5c402c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 2 Apr 2024 10:05:21 -0500 Subject: [PATCH 6/6] change(pallet-referenda): apply requested changes --- substrate/frame/referenda/src/lib.rs | 92 +++++++++++++--------------- 1 file changed, 42 insertions(+), 50 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index 054d3775a668..ad2dcb349e69 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -627,11 +627,7 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { ensure_root(origin)?; let now = frame_system::Pallet::::block_number(); - - let mut status = match ReferendumInfoFor::::get(index) { - Some(ReferendumInfo::Ongoing(status)) => Ok(status), - _ => Err(Error::::NotOngoing), - }?; + let mut status = Self::ensure_ongoing(index)?; // This is our wake-up, so we can disregard the alarm. status.alarm = None; let (info, dirty, branch) = Self::service_referendum(now, index, status); @@ -744,6 +740,19 @@ pub mod pallet { } } +#[inline] +fn is_finalizing, I: 'static>(status: &ReferendumStatusOf) -> bool { + status + .clone() + .deciding + .map(|d| { + d.confirming + .map(|x| frame_system::Pallet::::block_number() >= x) + .unwrap_or(false) + }) + .unwrap_or(false) +} + impl, I: 'static> Polling for Pallet { type Index = ReferendumIndex; type Votes = VotesOf; @@ -760,17 +769,11 @@ impl, I: 'static> Polling for Pallet { ) -> R { match ReferendumInfoFor::::get(index) { Some(ReferendumInfo::Ongoing(mut status)) => { - let now = frame_system::Pallet::::block_number(); - let is_finalizing = status - .clone() - .deciding - .map(|d| d.confirming.map(|x| now >= x).unwrap_or(false)) - .unwrap_or(false); - if is_finalizing { + if is_finalizing(&status) { return f(PollStatus::None); } - let result = f(PollStatus::Ongoing(&mut status.tally, status.track)); + let now = frame_system::Pallet::::block_number(); Self::ensure_alarm_at(&mut status, index, now + One::one()); ReferendumInfoFor::::insert(index, ReferendumInfo::Ongoing(status)); result @@ -789,17 +792,11 @@ impl, I: 'static> Polling for Pallet { ) -> Result { match ReferendumInfoFor::::get(index) { Some(ReferendumInfo::Ongoing(mut status)) => { - let now = frame_system::Pallet::::block_number(); - let is_finalizing = status - .clone() - .deciding - .map(|d| d.confirming.map(|x| now >= x).unwrap_or(false)) - .unwrap_or(false); - if is_finalizing { + if is_finalizing(&status) { return f(PollStatus::None); } - let result = f(PollStatus::Ongoing(&mut status.tally, status.track))?; + let now = frame_system::Pallet::::block_number(); Self::ensure_alarm_at(&mut status, index, now + One::one()); ReferendumInfoFor::::insert(index, ReferendumInfo::Ongoing(status)); Ok(result) @@ -876,16 +873,7 @@ impl, I: 'static> Pallet { index: ReferendumIndex, ) -> Result, DispatchError> { match ReferendumInfoFor::::get(index) { - Some(ReferendumInfo::Ongoing(status)) => { - match status.clone().deciding.map(|d| d.confirming) { - Some(Some(confirming)) - if frame_system::Pallet::::block_number() >= confirming => - { - Err(Error::::NotOngoing.into()) - }, - _ => Ok(status), - } - }, + Some(ReferendumInfo::Ongoing(status)) => Ok(status), _ => Err(Error::::NotOngoing.into()), } } @@ -946,8 +934,8 @@ impl, I: 'static> Pallet { 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`. - let when = (when.saturating_add(alarm_interval.saturating_sub(One::one())) - / alarm_interval) + let when = (when.saturating_add(alarm_interval.saturating_sub(One::one())) / + alarm_interval) .saturating_mul(alarm_interval); let result = T::Scheduler::schedule( DispatchTime::At(when), @@ -1060,7 +1048,7 @@ impl, I: 'static> Pallet { Ok(c) => c, Err(_) => { debug_assert!(false, "Unable to create a bounded call from `one_fewer_deciding`??",); - return; + return }, }; Self::set_alarm(call, next_block); @@ -1087,7 +1075,7 @@ impl, I: 'static> Pallet { false, "Unable to create a bounded call from `nudge_referendum`??", ); - return false; + return false }, }; status.alarm = Self::set_alarm(call, alarm); @@ -1201,7 +1189,7 @@ impl, I: 'static> Pallet { ), true, ServiceBranch::TimedOut, - ); + ) } }, Some(deciding) => { @@ -1230,7 +1218,7 @@ impl, I: 'static> Pallet { now.saturating_less_one(), is_passing, ); - return Self::service_auction(now, index, status); + return Self::confirm_by_candle(now, index, status); }, Some(_) => { // We don't care if failing within confirm period. @@ -1272,7 +1260,7 @@ impl, I: 'static> Pallet { now.saturating_less_one(), is_passing, ); - return Self::service_auction(now, index, status); + return Self::confirm_by_candle(now, index, status); }, Some(_) => { // We don't care if failing within confirm period. @@ -1321,12 +1309,13 @@ impl, I: 'static> Pallet { (ReferendumInfo::Ongoing(status), dirty_alarm || dirty, branch) } - /// "Candle" auction to decide the winning status of confirm period. + /// Find "candle" moment within the confirm period to decide whether the referendum is + /// confirmed or not. /// /// The "candle": passing or failing of a referendum is ultimately decided as a candle auction /// where, given a random point in time (defined as `t`), the definitive status of the the /// referendum is decided by the last status registered before `t`. - fn service_auction( + fn confirm_by_candle( now: BlockNumberFor, index: ReferendumIndex, mut status: ReferendumStatusOf, @@ -1340,9 +1329,9 @@ impl, I: 'static> Pallet { let confirming_until = status .clone() .deciding - .expect("having passed ongoing, we should have times for decision; qed") + .expect("called after having deciding, we have deciding.since time; qed") .confirming - .expect("having finished confirming, we should have a confirming_until time; qed"); + .expect("called after finished confirming, we have a confirming_until time; qed"); let confirming_since = confirming_until.saturating_sub(track.confirm_period); @@ -1370,9 +1359,12 @@ impl, I: 'static> Pallet { let statuses = PassingStatusInConfirmPeriod::::get(index); let statuses = statuses.range((Included(&confirming_since), Included(&candle_block_number))); - let (_, winning_status) = statuses.last().unwrap_or((&now, &true)); + let (_, is_confirmed) = statuses.last().unwrap_or((&now, &true)); + + // Cleanup passing status + PassingStatusInConfirmPeriod::::remove(index); - if *winning_status { + if *is_confirmed { // Passed! Self::ensure_no_alarm(&mut status); Self::note_one_fewer_deciding(status.track); @@ -1478,8 +1470,8 @@ impl, I: 'static> Pallet { id: TrackIdOf, ) -> bool { let x = Perbill::from_rational(elapsed.min(period), period); - support_needed.passing(x, tally.support(id)) - && approval_needed.passing(x, tally.approval(id)) + support_needed.passing(x, tally.support(id)) && + approval_needed.passing(x, tally.approval(id)) } /// Clear metadata if exist for a given referendum index. @@ -1501,8 +1493,8 @@ impl, I: 'static> Pallet { #[cfg(any(feature = "try-runtime", test))] fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { ensure!( - ReferendumCount::::get() as usize - == ReferendumInfoFor::::iter_keys().count(), + ReferendumCount::::get() as usize == + ReferendumInfoFor::::iter_keys().count(), "Number of referenda in `ReferendumInfoFor` is different than `ReferendumCount`" ); @@ -1540,8 +1532,8 @@ impl, I: 'static> Pallet { if let Some(deciding) = status.deciding { ensure!( - deciding.since - < deciding.confirming.unwrap_or(BlockNumberFor::::max_value()), + deciding.since < + deciding.confirming.unwrap_or(BlockNumberFor::::max_value()), "Deciding status cannot begin before confirming stage." ) }