Skip to content

Commit

Permalink
change(pallet-referenda): treat Finalizing state as a logical (intr…
Browse files Browse the repository at this point in the history 
…insic) period within `Ongoing` state.
  • Loading branch information
@pandres95
pandres95 committed Mar 26, 2024
1 parent 0a8ea80 commit 3a275ea
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 200 deletions.
102 changes: 56 additions & 46 deletions substrate/frame/referenda/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -628,22 +628,10 @@ pub mod pallet {
ensure_root(origin)?;
let now = frame_system::Pallet::<T>::block_number();

let info = ReferendumInfoFor::<T, I>::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::<T, I>::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::<T, I>::insert(index, info);
}
Expand Down Expand Up @@ -769,8 +757,17 @@ impl<T: Config<I>, I: 'static> Polling<T::Tally> for Pallet<T, I> {
) -> R {
match ReferendumInfoFor::<T, I>::get(index) {
Some(ReferendumInfo::Ongoing(mut status)) => {
let result = f(PollStatus::Ongoing(&mut status.tally, status.track));
let now = frame_system::Pallet::<T>::block_number();
let 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::<T, I>::insert(index, ReferendumInfo::Ongoing(status));
result
Expand All @@ -789,8 +786,17 @@ impl<T: Config<I>, I: 'static> Polling<T::Tally> for Pallet<T, I> {
) -> Result<R, DispatchError> {
match ReferendumInfoFor::<T, I>::get(index) {
Some(ReferendumInfo::Ongoing(mut status)) => {
let result = f(PollStatus::Ongoing(&mut status.tally, status.track))?;
let now = frame_system::Pallet::<T>::block_number();
let 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::<T, I>::insert(index, ReferendumInfo::Ongoing(status));
Ok(result)
Expand Down Expand Up @@ -867,7 +873,16 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
index: ReferendumIndex,
) -> Result<ReferendumStatusOf<T, I>, DispatchError> {
match ReferendumInfoFor::<T, I>::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::<T>::block_number() >= confirming =>
{
Err(Error::<T, I>::NotOngoing.into())
},
_ => Ok(status),
}
},
_ => Err(Error::<T, I>::NotOngoing.into()),
}
}
Expand Down Expand Up @@ -928,8 +943,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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),
Expand Down Expand Up @@ -1042,7 +1057,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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);
Expand All @@ -1069,7 +1084,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
false,
"Unable to create a bounded call from `nudge_referendum`??",
);
return false
return false;
},
};
status.alarm = Self::set_alarm(call, alarm);
Expand Down Expand Up @@ -1173,7 +1188,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
),
true,
ServiceBranch::TimedOut,
)
);
}
},
Some(deciding) => {
Expand All @@ -1185,6 +1200,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
&track.min_approval,
status.track,
);

branch = if is_passing {
match deciding.confirming {
Some(t) if now >= t => {
Expand All @@ -1195,18 +1211,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
// 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::<T, I>::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,
)
log::debug!("going to auction at {now}");
return Self::service_auction(now, index, status);
},
Some(_) => {
// We don't care if failing within confirm period.
Expand Down Expand Up @@ -1244,13 +1256,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
now.saturating_less_one(),
is_passing,
);
Self::ensure_alarm_at(&mut status, index, now.saturating_plus_one());

return (
ReferendumInfo::Finalizing(status),
true,
ServiceBranch::ContinueConfirming,
)
log::debug!("going to auction at {now}");
return Self::service_auction(now, index, status);
},
Some(_) => {
// We don't care if failing within confirm period.
Expand Down Expand Up @@ -1308,7 +1315,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
// 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
Expand All @@ -1324,8 +1331,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

// Do not use random until made sure random seed is not known before confirm ends
if known_since <= confirming_until {
log::debug!("auction: better luck next block");
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 = <BlockNumberFor<T>>::decode(&mut raw_offset.as_ref())
Expand Down Expand Up @@ -1454,8 +1464,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
id: TrackIdOf<T, I>,
) -> 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.
Expand All @@ -1477,8 +1487,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
#[cfg(any(feature = "try-runtime", test))]
fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> {
ensure!(
ReferendumCount::<T, I>::get() as usize ==
ReferendumInfoFor::<T, I>::iter_keys().count(),
ReferendumCount::<T, I>::get() as usize
== ReferendumInfoFor::<T, I>::iter_keys().count(),
"Number of referenda in `ReferendumInfoFor` is different than `ReferendumCount`"
);

Expand Down Expand Up @@ -1516,8 +1526,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

if let Some(deciding) = status.deciding {
ensure!(
deciding.since <
deciding.confirming.unwrap_or(BlockNumberFor::<T>::max_value()),
deciding.since
< deciding.confirming.unwrap_or(BlockNumberFor::<T>::max_value()),
"Deciding status cannot begin before confirming stage."
)
}
Expand Down
126 changes: 0 additions & 126 deletions substrate/frame/referenda/src/migration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,57 +90,6 @@ pub mod v0 {
pub mod v1 {
use super::*;

pub type ReferendumInfoOf<T, I> = ReferendumInfo<
TrackIdOf<T, I>,
PalletsOriginOf<T>,
frame_system::pallet_prelude::BlockNumberFor<T>,
BoundedCallOf<T, I>,
BalanceOf<T, I>,
TallyOf<T, I>,
<T as frame_system::Config>::AccountId,
ScheduleAddressOf<T, I>,
>;

#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub enum ReferendumInfo<
TrackId: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
RuntimeOrigin: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
Moment: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone + EncodeLike,
Call: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
Balance: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
Tally: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
AccountId: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
ScheduleAddress: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
> {
/// Referendum has been submitted and is being voted on.
Ongoing(
ReferendumStatus<
TrackId,
RuntimeOrigin,
Moment,
Call,
Balance,
Tally,
AccountId,
ScheduleAddress,
>,
),
/// Referendum finished with approval. Submission deposit is held.
Approved(Moment, Option<Deposit<AccountId, Balance>>, Option<Deposit<AccountId, Balance>>),
/// Referendum finished with rejection. Submission deposit is held.
Rejected(Moment, Option<Deposit<AccountId, Balance>>, Option<Deposit<AccountId, Balance>>),
/// Referendum finished with cancellation. Submission deposit is held.
Cancelled(Moment, Option<Deposit<AccountId, Balance>>, Option<Deposit<AccountId, Balance>>),
/// Referendum finished and was never decided. Submission deposit is held.
TimedOut(Moment, Option<Deposit<AccountId, Balance>>, Option<Deposit<AccountId, Balance>>),
/// Referendum finished with a kill.
Killed(Moment),
}

#[storage_alias]
pub type ReferendumInfoFor<T: Config<I>, I: 'static> =
StorageMap<Pallet<T, I>, Blake2_128Concat, ReferendumIndex, ReferendumInfoOf<T, I>>;

/// The log target.
const TARGET: &'static str = "runtime::referenda::migration::v1";

Expand Down Expand Up @@ -212,81 +161,6 @@ pub mod v1 {
}
}

pub mod v2 {
use super::*;

/// The log target.
const TARGET: &'static str = "runtime::referenda::migration::v2";

/// Transforms a submission deposit of ReferendumInfo(Approved|Rejected|Cancelled|TimedOut) to
/// optional value, making it refundable.
pub struct MigrateV1ToV2<T, I = ()>(PhantomData<(T, I)>);
impl<T: Config<I>, I: 'static> OnRuntimeUpgrade for MigrateV1ToV2<T, I> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
let referendum_count = v1::ReferendumInfoFor::<T, I>::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::<T, I>::in_code_storage_version();
let on_chain_version = Pallet::<T, I>::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::<T, I>::iter().for_each(|(key, value)| {
let maybe_new_value = match value {
v1::ReferendumInfo::Ongoing(s) => Some(ReferendumInfo::Ongoing(s)),
v1::ReferendumInfo::Killed(e) => Some(ReferendumInfo::Killed(e)),
v1::ReferendumInfo::Approved(e, s, d) =>
Some(ReferendumInfo::Approved(e, s, d)),
v1::ReferendumInfo::Rejected(e, s, d) =>
Some(ReferendumInfo::Rejected(e, s, d)),
v1::ReferendumInfo::Cancelled(e, s, d) =>
Some(ReferendumInfo::Cancelled(e, s, d)),
v1::ReferendumInfo::TimedOut(e, s, d) =>
Some(ReferendumInfo::TimedOut(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::<T, I>::insert(key, new_value);
} else {
weight.saturating_accrue(T::DbWeight::get().reads(1));
}
});
StorageVersion::new(2).put::<Pallet<T, I>>();
weight.saturating_accrue(T::DbWeight::get().writes(1));
weight
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(state: Vec<u8>) -> Result<(), TryRuntimeError> {
let on_chain_version = Pallet::<T, I>::on_chain_storage_version();
ensure!(on_chain_version == 2, "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::<T, I>::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::*;
Expand Down
4 changes: 0 additions & 4 deletions substrate/frame/referenda/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
}
Expand Down
Loading

0 comments on commit 3a275ea

Please sign in to comment.