Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: simplify launch timestamp logic #1104

Merged
merged 2 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/democracy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ log = { version = "0.4.17", default-features = false }
serde = { version = "1.0.130", default-features = false, features = ["derive"], optional = true }
codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false, features = ["derive", "max-encoded-len"] }
scale-info = { version = "2.2.0", default-features = false, features = ["derive"] }
chrono = {version = "0.4.23", default-features = false }

sp-api = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.31", default-features = false }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.31", default-features = false }
Expand Down
56 changes: 20 additions & 36 deletions crates/democracy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@
#![recursion_limit = "256"]
#![cfg_attr(not(feature = "std"), no_std)]

use core::time::Duration;

use chrono::Days;
use codec::{Decode, Encode};
use frame_support::{
ensure,
Expand Down Expand Up @@ -198,11 +195,8 @@ pub mod pallet {
/// Unix time
type UnixTime: UnixTime;

/// Duration
type Moment: TryInto<i64>;

/// Millisecond offset into week from Monday.
type LaunchOffsetMillis: Get<Self::Moment>;
/// Period from previous launch timestamp.
type LaunchPeriod: Get<u64>;

/// Account from which is transferred in `spend_from_treasury`.
type TreasuryAccount: Get<Self::AccountId>;
Expand Down Expand Up @@ -263,13 +257,15 @@ pub mod pallet {
#[pallet::genesis_config]
pub struct GenesisConfig<T: Config> {
_phantom: sp_std::marker::PhantomData<T>,
next_launch_timestamp: u64,
}

#[cfg(feature = "std")]
impl<T: Config> Default for GenesisConfig<T> {
fn default() -> Self {
GenesisConfig {
_phantom: Default::default(),
next_launch_timestamp: 0,
}
}
}
Expand All @@ -280,6 +276,7 @@ pub mod pallet {
PublicPropCount::<T>::put(0 as PropIndex);
ReferendumCount::<T>::put(0 as ReferendumIndex);
LowestUnbaked::<T>::put(0 as ReferendumIndex);
NextLaunchTimestamp::<T>::put(self.next_launch_timestamp);
}
}

Expand Down Expand Up @@ -938,7 +935,7 @@ impl<T: Config> Pallet<T> {

// pick out another public referendum if it's time.
let current_time = T::UnixTime::now();
if Self::should_launch(current_time)? {
if Self::should_launch(current_time.as_secs()) {
// Errors come from the queue being empty. If the queue is not empty, it will take
// full block weight.
if Self::launch_next(now).is_ok() {
Expand All @@ -964,37 +961,24 @@ impl<T: Config> Pallet<T> {

/// determine whether or not a new referendum should be launched. This will return true
/// once every week.
fn should_launch(now: Duration) -> Result<bool, DispatchError> {
if now.as_secs() < NextLaunchTimestamp::<T>::get() {
return Ok(false);
fn should_launch(now: u64) -> bool {
if now < NextLaunchTimestamp::<T>::get() {
return false;
}

// time to launch - calculate the date of next launch.

// convert to format used by `chrono`
let secs: i64 = now.as_secs().try_into().map_err(Error::<T>::from)?;
let now =
chrono::NaiveDateTime::from_timestamp_opt(secs, now.subsec_nanos()).ok_or(Error::<T>::TryIntoIntError)?;

// calculate next week boundary
let beginning_of_week = now.date().week(chrono::Weekday::Mon).first_day();
let next_week = beginning_of_week
.checked_add_days(Days::new(7))
.ok_or(Error::<T>::TryIntoIntError)?
.and_time(Default::default());

let offset = T::LaunchOffsetMillis::get()
.try_into()
.map_err(|_| Error::<T>::TryIntoIntError)?;
let next_launch = next_week
.checked_add_signed(chrono::Duration::milliseconds(offset))
.ok_or(ArithmeticError::Overflow)?;

// update storage
let next_timestamp: u64 = next_launch.timestamp().try_into().map_err(Error::<T>::from)?;
NextLaunchTimestamp::<T>::set(next_timestamp);
NextLaunchTimestamp::<T>::mutate(|next_launch_timestamp| {
// period is number of seconds - e.g. to next week (mon 9am)
let launch_period = T::LaunchPeriod::get();
next_launch_timestamp.saturating_accrue(
(now.saturating_sub(*next_launch_timestamp)
.saturating_div(launch_period)
.saturating_add(One::one()))
.saturating_mul(launch_period),
);
});

Ok(true)
true
}

/// Reads the length of account in DepositOf without getting the complete value in the runtime.
Expand Down
41 changes: 17 additions & 24 deletions crates/democracy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,15 @@ impl pallet_balances::Config for Test {
type WeightInfo = ();
}
parameter_types! {
pub const LaunchPeriod: u64 = 2;
pub const LaunchPeriod: u64 = 60 * 60 * 24 * 7; // one week
pub const VotingPeriod: u64 = 4;
pub const FastTrackVotingPeriod: u64 = 2;
pub const MinimumDeposit: u64 = 1;
pub const EnactmentPeriod: u64 = 2;
pub const MaxVotes: u32 = 100;
pub const MaxProposals: u32 = MAX_PROPOSALS;
pub static PreimageByteDeposit: u64 = 0;
pub LaunchOffsetMillis: u64 = 9 * 60 * 60 * 1000; // 9 hours offset, i.e. MON 9 AM
pub const TreasuryAccount:u64 = 232323;

}
ord_parameter_types! {
pub const One: u64 = 1;
Expand Down Expand Up @@ -186,8 +184,7 @@ impl Config for Test {
type PalletsOrigin = OriginCaller;
type WeightInfo = ();
type UnixTime = Timestamp;
type Moment = u64;
type LaunchOffsetMillis = LaunchOffsetMillis;
type LaunchPeriod = LaunchPeriod;
type TreasuryAccount = TreasuryAccount;
type TreasuryCurrency = pallet_balances::Pallet<Self>;
}
Expand Down Expand Up @@ -281,6 +278,7 @@ fn tally(r: ReferendumIndex) -> Tally<u64> {
#[test]
fn should_launch_works() {
new_test_ext().execute_with(|| {
NextLaunchTimestamp::<Test>::put(1670835600); // Mon Dec 12 2022 09:00:00 UTC
let arbitrary_timestamp = 1670864631; // Mon Dec 12 2022 17:03:51 UTC

let week_boundaries = [
Expand All @@ -289,38 +287,33 @@ fn should_launch_works() {
1672650000, // Mon Jan 02 2023 09:00:00 UTC
];
// first launch immediately after launch of chain / first runtime upgrade
assert!(Democracy::should_launch(Duration::from_secs(arbitrary_timestamp)).unwrap());
assert!(Democracy::should_launch(arbitrary_timestamp));
// second time it should return false
assert!(!Democracy::should_launch(Duration::from_secs(arbitrary_timestamp)).unwrap());
assert!(!Democracy::should_launch(arbitrary_timestamp));

for boundary in week_boundaries {
// one second before the next week it should still return false
assert!(!Democracy::should_launch(Duration::from_secs(boundary - 1)).unwrap());
assert!(!Democracy::should_launch(boundary - 1));

// first second of next week it should return true exactly once
assert!(Democracy::should_launch(Duration::from_secs(boundary)).unwrap());
assert!(!Democracy::should_launch(Duration::from_secs(boundary)).unwrap());
assert!(Democracy::should_launch(boundary));
assert!(!Democracy::should_launch(boundary));
}
});
}

#[test]
fn should_launch_edge_case_behavior() {
fn should_launch_skipped_works() {
new_test_ext().execute_with(|| {
// test edge case where we launch on monday before 9 am. Next launch will be
// in slightly more than 7 days
let initial_launch = 1670828400; // Mon Dec 12 2022 07:00:00 UTC
let next_launch = 1671440400; // Mon Dec 19 2022 09:00:00 UTC

// first launch immediately after launch of chain / first runtime upgrade
assert!(Democracy::should_launch(Duration::from_secs(initial_launch)).unwrap());
assert!(!Democracy::should_launch(Duration::from_secs(initial_launch)).unwrap());
NextLaunchTimestamp::<Test>::put(1671440400); // Mon Dec 19 2022 09:00:00 GMT

// one second before the next week it should still return false
assert!(!Democracy::should_launch(Duration::from_secs(next_launch - 1)).unwrap());
// skip 3 weeks + 1 day + 1 hour + 5 minutes
let now = 1673345100; // Tue Jan 10 2023 10:05:00 GMT

// first second of next week it should return true exactly once
assert!(Democracy::should_launch(Duration::from_secs(next_launch)).unwrap());
assert!(!Democracy::should_launch(Duration::from_secs(next_launch)).unwrap());
assert!(Democracy::should_launch(now));
assert_eq!(
NextLaunchTimestamp::<Test>::get(),
1673859600 // Mon Jan 16 2023 09:00:00 GMT
);
});
}
5 changes: 2 additions & 3 deletions parachain/runtime/interlay/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ type EnsureRootOrAllTechnicalCommittee = EitherOfDiverse<
>;

parameter_types! {
pub const LaunchPeriod: u64 = 1000 * 60 * 60 * 24 * 7; // one week
pub const VotingPeriod: BlockNumber = 7 * DAYS;
pub const FastTrackVotingPeriod: BlockNumber = 3 * HOURS;
// Require 250 vINTR to make a proposal. Given the crowdloan airdrop, this qualifies about 7500
Expand All @@ -484,7 +485,6 @@ parameter_types! {
pub const EnactmentPeriod: BlockNumber = DAYS;
pub const MaxVotes: u32 = 100;
pub const MaxProposals: u32 = 100;
pub LaunchOffsetMillis: u64 = 9 * 60 * 60 * 1000; // 9 hours offset, i.e. MON 9 AM
}

impl democracy::Config for Runtime {
Expand All @@ -505,8 +505,7 @@ impl democracy::Config for Runtime {
type PalletsOrigin = OriginCaller;
type WeightInfo = weights::democracy::WeightInfo<Runtime>;
type UnixTime = Timestamp;
type Moment = Moment;
type LaunchOffsetMillis = LaunchOffsetMillis;
type LaunchPeriod = LaunchPeriod;
type TreasuryAccount = TreasuryAccount;
type TreasuryCurrency = NativeCurrency;
}
Expand Down
5 changes: 2 additions & 3 deletions parachain/runtime/kintsugi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ type EnsureRootOrAllTechnicalCommittee = EitherOfDiverse<
>;

parameter_types! {
pub const LaunchPeriod: u64 = 1000 * 60 * 60 * 24 * 7; // one week
pub const VotingPeriod: BlockNumber = 2 * DAYS;
pub const FastTrackVotingPeriod: BlockNumber = 3 * HOURS;
// Require 5 vKINT to make a proposal. Given the crowdloan airdrop, this qualifies about 3500
Expand All @@ -482,7 +483,6 @@ parameter_types! {
pub const EnactmentPeriod: BlockNumber = 6 * HOURS;
pub const MaxVotes: u32 = 100;
pub const MaxProposals: u32 = 100;
pub LaunchOffsetMillis: u64 = 9 * 60 * 60 * 1000; // 9 hours offset, i.e. MON 9 AM
}

impl democracy::Config for Runtime {
Expand All @@ -503,8 +503,7 @@ impl democracy::Config for Runtime {
type PalletsOrigin = OriginCaller;
type WeightInfo = weights::democracy::WeightInfo<Runtime>;
type UnixTime = Timestamp;
type Moment = Moment;
type LaunchOffsetMillis = LaunchOffsetMillis;
type LaunchPeriod = LaunchPeriod;
type TreasuryAccount = TreasuryAccount;
type TreasuryCurrency = NativeCurrency;
}
Expand Down