[audit-fix(#11)] Miscellaneous performance optimizations

osmosis-labs · May 3, 2024 · 40720bb · 40720bb
1 parent 1a25423
commit 40720bb
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 36 deletions.
diff --git a/contracts/transmuter/src/contract.rs b/contracts/transmuter/src/contract.rs
@@ -3395,20 +3395,18 @@ mod tests {
                 token_in: Coin::new(1000, "axlusdc"),
                 token_out_denom: "axlusdc".to_string(),
                 swap_fee: Decimal::zero(),
-                expected: Err(StdError::generic_err(
-                    "token_in_denom and token_out_denom cannot be the same",
-                )
-                .into()),
+                expected: Err(ContractError::SameDenomNotAllowed {
+                    denom: "axlusdc".to_string(),
+                }),
             },
             Case {
                 name: String::from("same denom error (alloyed asset)"),
                 token_in: Coin::new(1000, "alloyedusdc"),
                 token_out_denom: "alloyedusdc".to_string(),
                 swap_fee: Decimal::zero(),
-                expected: Err(StdError::generic_err(
-                    "token_in_denom and token_out_denom cannot be the same",
-                )
-                .into()),
+                expected: Err(ContractError::SameDenomNotAllowed {
+                    denom: "alloyedusdc".to_string(),
+                }),
             },
             Case {
                 name: String::from("alloyedusdc to axlusdc - ok"),
@@ -3611,20 +3609,18 @@ mod tests {
                 token_in_denom: "axlusdc".to_string(),
                 token_out: Coin::new(1000, "axlusdc"),
                 swap_fee: Decimal::zero(),
-                expected: Err(StdError::generic_err(
-                    "token_in_denom and token_out_denom cannot be the same",
-                )
-                .into()),
+                expected: Err(ContractError::SameDenomNotAllowed {
+                    denom: "axlusdc".to_string(),
+                }),
             },
             Case {
                 name: String::from("same denom error (alloyed asset)"),
                 token_in_denom: "alloyedusdc".to_string(),
                 token_out: Coin::new(1000, "alloyedusdc"),
                 swap_fee: Decimal::zero(),
-                expected: Err(StdError::generic_err(
-                    "token_in_denom and token_out_denom cannot be the same",
-                )
-                .into()),
+                expected: Err(ContractError::SameDenomNotAllowed {
+                    denom: "alloyedusdc".to_string(),
+                }),
             },
             Case {
                 name: String::from("alloyedusdc to axlusdc - ok"),

diff --git a/contracts/transmuter/src/swap.rs b/contracts/transmuter/src/swap.rs
@@ -427,12 +427,6 @@ impl Transmuter<'_> {
         token_out: Coin,
         token_in_denom: String,
     ) -> Result<(TransmuterPool, Coin), ContractError> {
-        // ensure token in denom and token out denom are not the same
-        ensure!(
-            token_out.denom != token_in_denom,
-            StdError::generic_err("token_in_denom and token_out_denom cannot be the same")
-        );
-
         let swap_variant = self.swap_variant(&token_in_denom, &token_out.denom, deps)?;
         let mut pool = self.pool.load(deps.storage)?;
 
@@ -494,12 +488,6 @@ impl Transmuter<'_> {
         token_in: Coin,
         token_out_denom: &str,
     ) -> Result<(TransmuterPool, Coin), ContractError> {
-        // ensure token in denom and token out denom are not the same
-        ensure!(
-            token_out_denom != token_in.denom,
-            StdError::generic_err("token_in_denom and token_out_denom cannot be the same")
-        );
-
         let mut pool = self.pool.load(deps.storage)?;
         let swap_variant = self.swap_variant(&token_in.denom, token_out_denom, deps)?;
 

diff --git a/contracts/transmuter/src/transmuter_pool/add_new_assets.rs b/contracts/transmuter/src/transmuter_pool/add_new_assets.rs
@@ -6,8 +6,8 @@ impl TransmuterPool {
     pub fn add_new_assets(&mut self, assets: Vec<Asset>) -> Result<(), ContractError> {
         self.pool_assets.extend(assets);
 
-        self.ensure_no_duplicated_denom()?;
-        self.ensure_pool_asset_count_within_range()
+        self.ensure_pool_asset_count_within_range()?;
+        self.ensure_no_duplicated_denom()
     }
 }
 

diff --git a/contracts/transmuter/src/transmuter_pool/corrupted_assets.rs b/contracts/transmuter/src/transmuter_pool/corrupted_assets.rs
@@ -87,19 +87,18 @@ impl TransmuterPool {
     where
         A: FnOnce(&mut Self) -> Result<R, ContractError>,
     {
-        // early return result without any checks if no corrupted assets
-        let has_no_corrupted_assets = self.pool_assets.iter().all(|asset| !asset.is_corrupted());
-        if has_no_corrupted_assets {
-            return action(self);
-        }
-
         let pool_asset_pre_action = self.pool_assets.clone();
         let corrupted_assets_pre_action = pool_asset_pre_action
             .iter()
             .filter(|asset| asset.is_corrupted())
             .map(|asset| (asset.denom().to_string(), asset))
             .collect::<HashMap<_, _>>();
 
+        // early return result without any checks if no corrupted assets
+        if corrupted_assets_pre_action.is_empty() {
+            return action(self);
+        }
+
         // if total pool value == 0 -> Empty mapping, later unwrap weight will be 0
         let weight_pre_action = self.weights()?.unwrap_or_default();
         let weight_pre_action = weight_pre_action.into_iter().collect::<HashMap<_, _>>();