[FIX-7] Missing Invaraint Check

osmosis-labs · Sep 21, 2023 · c5249a4 · c5249a4
1 parent c9ca823
commit c5249a4
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 4 deletions.
diff --git a/contracts/transmuter/src/error.rs b/contracts/transmuter/src/error.rs
@@ -1,6 +1,6 @@
 use cosmwasm_std::{
-    CheckedFromRatioError, Coin, Decimal, DivideByZeroError, OverflowError, StdError, Uint128,
-    Uint64,
+    CheckedFromRatioError, Coin, Decimal, DivideByZeroError, OverflowError, StdError, Timestamp,
+    Uint128, Uint64,
 };
 use thiserror::Error;
 
@@ -122,9 +122,17 @@ pub enum ContractError {
     #[error("Moving average is undefined due to zero elapsed time since limiter started tracking")]
     UndefinedMovingAverage {},
 
+    /// Time invariant error, this should never happen
     #[error("Time must be monotonically increasing")]
     NonMonotonicTime {},
 
+    /// Time invariant error, this should never happen
+    #[error("Division's update should occur before division ended: updated_at: {updated_at}, ended_at: {ended_at}")]
+    UpdateAfterDivisionEnded {
+        updated_at: Timestamp,
+        ended_at: Timestamp,
+    },
+
     #[error("Limiter does not exist for denom: {denom}, label: {label}")]
     LimiterDoesNotExist { denom: String, label: String },
 

diff --git a/contracts/transmuter/src/limiter/division.rs b/contracts/transmuter/src/limiter/division.rs
@@ -59,6 +59,11 @@ impl Division {
     }
 
     pub fn update(&self, updated_at: Timestamp, value: Decimal) -> Result<Self, ContractError> {
+        ensure!(
+            updated_at >= self.started_at,
+            ContractError::NonMonotonicTime {}
+        );
+
         let prev_updated_at = self.updated_at.nanos();
         let elapsed_time = elapsed_time(prev_updated_at, updated_at.nanos())?;
         Ok(Self {

diff --git a/contracts/transmuter/src/limiter/limiters.rs b/contracts/transmuter/src/limiter/limiters.rs
@@ -162,6 +162,9 @@ impl ChangeLimiter {
         updated_limiter.latest_value = value;
 
         updated_limiter.divisions = if updated_limiter.divisions.is_empty() {
+            // no need to ensure time invariant since
+            // started_at = updated_at so
+            // `updated_at <= started_at + division_size` is always true
             vec![Division::new(block_time, block_time, value, value)?]
         } else {
             // If the division is over, create a new division
@@ -173,14 +176,39 @@ impl ChangeLimiter {
 
             if latest_division.elapsed_time(block_time)? >= division_size {
                 let started_at = latest_division.next_started_at(division_size, block_time)?;
+                let updated_at = block_time;
+                let ended_at = started_at.plus_nanos(division_size.u64());
 
-                let new_division = Division::new(started_at, block_time, value, prev_value)?;
+                // ensure time invariant
+                ensure!(
+                    updated_at <= ended_at,
+                    ContractError::UpdateAfterDivisionEnded {
+                        updated_at,
+                        ended_at
+                    }
+                );
+
+                let new_division = Division::new(started_at, updated_at, value, prev_value)?;
                 divisions.push(new_division);
             }
             // else update the current division
             else {
                 let last_index = divisions.len() - 1;
-                divisions[last_index] = latest_division.update(block_time, value)?;
+
+                let updated_at = block_time;
+                let ended_at =
+                    Timestamp::from_nanos(latest_division.ended_at(division_size)?.u64());
+
+                // ensure time invariant
+                ensure!(
+                    updated_at <= ended_at,
+                    ContractError::UpdateAfterDivisionEnded {
+                        updated_at,
+                        ended_at
+                    }
+                );
+
+                divisions[last_index] = latest_division.update(updated_at, value)?;
             }
 
             divisions