Fix: check bonus config in crowdloan (#1901)

* add more check * improve check * add test * fix ci
parallel-finance · Oct 31, 2022 · f15d2ca · f15d2ca
1 parent cf61201
commit f15d2ca
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 10 deletions.
diff --git a/pallets/crowdloans/src/benchmarking.rs b/pallets/crowdloans/src/benchmarking.rs
@@ -224,8 +224,8 @@ benchmarks! {
     update_leases_bonus {
         let bonus_config = BonusConfig {
             bonus_per_token: 5,
-            start_time: Default::default(),
-            end_time: Default::default(),
+            start_time: 1,
+            end_time: 2,
         };
     }: _(
         SystemOrigin::Root,

diff --git a/pallets/crowdloans/src/lib.rs b/pallets/crowdloans/src/lib.rs
@@ -336,6 +336,8 @@ pub mod pallet {
         NotReadyToDissolve,
         /// Proxy address is empty
         EmptyProxyAddress,
+        /// BonusConfig is wrong
+        WrongBonusConfig,
     }
 
     #[pallet::storage]
@@ -1178,6 +1180,12 @@ pub mod pallet {
             bonus_config: BonusConfig<BalanceOf<T>>,
         ) -> DispatchResult {
             ensure_origin!(UpdateOrigin, origin)?;
+            ensure!(
+                lease_start <= lease_end,
+                Error::<T>::LastPeriodBeforeFirstPeriod
+            );
+            ensure!(bonus_config.check(), Error::<T>::WrongBonusConfig);
+
             LeasesBonus::<T>::insert((&lease_start, &lease_end), bonus_config);
             Self::deposit_event(Event::<T>::LeasesBonusUpdated(
                 (lease_start, lease_end),

diff --git a/pallets/crowdloans/src/tests.rs b/pallets/crowdloans/src/tests.rs
@@ -1858,6 +1858,8 @@ fn update_leases_bonus_should_work() {
 
         let mut config = BonusConfig::default();
         config.bonus_per_token = 5;
+        config.start_time = 1;
+        config.end_time = 2;
         assert_ok!(Crowdloans::update_leases_bonus(
             frame_system::RawOrigin::Root.into(),
             start_lease,
@@ -1868,6 +1870,39 @@ fn update_leases_bonus_should_work() {
     })
 }
 
+#[test]
+fn update_leases_bonus_should_fail_when_wrong_bonus_config() {
+    new_test_ext().execute_with(|| {
+        let start_lease = 7;
+        let end_lease = 6;
+
+        let mut config = BonusConfig::default();
+        config.bonus_per_token = 5;
+        assert_err!(
+            Crowdloans::update_leases_bonus(
+                frame_system::RawOrigin::Root.into(),
+                start_lease,
+                end_lease,
+                config,
+            ),
+            Error::<Test>::LastPeriodBeforeFirstPeriod,
+        );
+        let start_lease = 6;
+        let end_lease = 7;
+        config.start_time = 11;
+        config.end_time = 2;
+        assert_err!(
+            Crowdloans::update_leases_bonus(
+                frame_system::RawOrigin::Root.into(),
+                start_lease,
+                end_lease,
+                config,
+            ),
+            Error::<Test>::WrongBonusConfig,
+        );
+    })
+}
+
 #[test]
 fn normalized_amount_should_work() {
     new_test_ext().execute_with(|| {

diff --git a/pallets/crowdloans/src/types.rs b/pallets/crowdloans/src/types.rs
@@ -164,3 +164,9 @@ impl<Balance: Default> Default for BonusConfig<Balance> {
         }
     }
 }
+
+impl<Balance> BonusConfig<Balance> {
+    pub fn check(&self) -> bool {
+        self.end_time > self.start_time
+    }
+}
diff --git a/pallets/streaming/src/lib.rs b/pallets/streaming/src/lib.rs
@@ -234,6 +234,7 @@ pub mod pallet {
                 deposit >= minimum_deposit,
                 Error::<T>::DepositLowerThanMinimum
             );
+            Self::ensure_valid_duration(start_time, end_time)?;
             let stream_id = Self::do_create(
                 sender.clone(),
                 recipient.clone(),
@@ -393,18 +394,13 @@ impl<T: Config> Pallet<T> {
     pub fn ensure_valid_duration(
         start_time: Timestamp,
         end_time: Timestamp,
-    ) -> Result<Timestamp, DispatchError> {
+    ) -> Result<(), DispatchError> {
         ensure!(
             start_time >= T::UnixTime::now().as_secs(),
             Error::<T>::StartTimeBeforeCurrentTime
         );
         ensure!(end_time > start_time, Error::<T>::EndTimeBeforeStartTime);
-
-        let duration = end_time
-            .checked_sub(start_time)
-            .ok_or(Error::<T>::InvalidDuration)?;
-
-        Ok(duration)
+        Ok(())
     }
 
     pub fn update_finished_stream_library(
@@ -500,7 +496,9 @@ impl<T: Config> Pallet<T> {
     ) -> Result<StreamId, DispatchError> {
         ensure!(sender != recipient, Error::<T>::RecipientIsAlsoSender);
 
-        let duration = Self::ensure_valid_duration(start_time, end_time)?;
+        let duration = end_time
+            .checked_sub(start_time)
+            .ok_or(Error::<T>::InvalidDuration)?;
         let rate_per_sec = deposit
             .checked_div(duration as u128)
             .ok_or(Error::<T>::InvalidRatePerSecond)?;