Merge pull request #1104 from gregdhill/refactor/launch-timestamp

refactor: simplify launch timestamp logic

crates/democracy/Cargo.toml

@@ -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 }

crates/democracy/src/lib.rs

@@ -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,
@@ -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>;
@@ -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,
             }
         }
     }
@@ -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);
         }
     }
 
@@ -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() {
@@ -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.

crates/democracy/src/tests.rs

@@ -148,17 +148,15 @@ impl pallet_balances::Config for Test {
     type MaxHolds = ();
 }
 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;
@@ -193,8 +191,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>;
 }
@@ -287,6 +284,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 = [
@@ -295,38 +293,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
+        );
     });
 }

parachain/runtime/interlay/src/lib.rs

@@ -492,6 +492,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
@@ -500,7 +501,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 {
@@ -521,8 +521,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;
 }

parachain/runtime/kintsugi/src/lib.rs

@@ -491,6 +491,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
@@ -500,7 +501,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 {
@@ -521,8 +521,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;
 }