Merge pull request #63 from confio/61-splippage-error-msg

tfi-pair: Better error messages for spread and slippage exceeding
confio · Jan 31, 2022 · dcb639c · dcb639c
2 parents 9a732ae + 51f9382
commit dcb639c
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 29 deletions.
diff --git a/contracts/tfi-pair/src/contract.rs b/contracts/tfi-pair/src/contract.rs
@@ -613,14 +613,20 @@ pub fn assert_max_spread(
             .checked_sub(return_amount)
             .unwrap_or_else(|_| Uint128::zero());
 
-        if return_amount < expected_return
-            && Decimal::from_ratio(spread_amount, expected_return) > max_spread
-        {
-            return Err(ContractError::MaxSpreadAssertion {});
+        let spread_ratio = Decimal::from_ratio(spread_amount, expected_return);
+        if return_amount < expected_return && spread_ratio > max_spread {
+            return Err(ContractError::MaxSpreadAssertion {
+                spread_ratio,
+                max_spread,
+            });
         }
     } else if let Some(max_spread) = max_spread {
-        if Decimal::from_ratio(spread_amount, return_amount + spread_amount) > max_spread {
-            return Err(ContractError::MaxSpreadAssertion {});
+        let spread_ratio = Decimal::from_ratio(spread_amount, return_amount + spread_amount);
+        if spread_ratio > max_spread {
+            return Err(ContractError::MaxSpreadAssertion {
+                spread_ratio,
+                max_spread,
+            });
         }
     }
 
@@ -636,16 +642,24 @@ fn assert_slippage_tolerance(
         let one_minus_slippage_tolerance = decimal_subtraction(Decimal::one(), slippage_tolerance)?;
 
         // Ensure each prices are not dropped as much as slippage tolerance rate
-        if decimal_multiplication(
-            Decimal::from_ratio(deposits[0], deposits[1]),
-            one_minus_slippage_tolerance,
-        ) > Decimal::from_ratio(pools[0].amount, pools[1].amount)
-            || decimal_multiplication(
-                Decimal::from_ratio(deposits[1], deposits[0]),
-                one_minus_slippage_tolerance,
-            ) > Decimal::from_ratio(pools[1].amount, pools[0].amount)
-        {
-            return Err(ContractError::MaxSlippageAssertion {});
+        let deposits_ratio = Decimal::from_ratio(deposits[0], deposits[1]);
+        let pools_ratio = Decimal::from_ratio(pools[0].amount, pools[1].amount);
+        if decimal_multiplication(deposits_ratio, one_minus_slippage_tolerance) > pools_ratio {
+            return Err(ContractError::MaxSlippageAssertion {
+                deposits_ratio,
+                pools_ratio,
+                slippage_tolerance,
+            });
+        }
+
+        let deposits_ratio = Decimal::from_ratio(deposits[1], deposits[0]);
+        let pools_ratio = Decimal::from_ratio(pools[1].amount, pools[0].amount);
+        if decimal_multiplication(deposits_ratio, one_minus_slippage_tolerance) > pools_ratio {
+            return Err(ContractError::MaxSlippageAssertion {
+                deposits_ratio,
+                pools_ratio,
+                slippage_tolerance,
+            });
         }
     }
 

diff --git a/contracts/tfi-pair/src/error.rs b/contracts/tfi-pair/src/error.rs
@@ -15,11 +15,18 @@ pub enum ContractError {
     #[error("Invalid zero amount")]
     InvalidZeroAmount {},
 
-    #[error("Max spread assertion")]
-    MaxSpreadAssertion {},
-
-    #[error("Max slippage assertion")]
-    MaxSlippageAssertion {},
+    #[error("Max spread exceeded, spread ratio: {spread_ratio}, max spread: {max_spread}")]
+    MaxSpreadAssertion {
+        spread_ratio: Decimal,
+        max_spread: Decimal,
+    },
+
+    #[error("Max slippage exceeded, deposits ratio: {deposits_ratio}, pools ratio: {pools_ratio}, slippage tolerance: {slippage_tolerance}")]
+    MaxSlippageAssertion {
+        deposits_ratio: Decimal,
+        pools_ratio: Decimal,
+        slippage_tolerance: Decimal,
+    },
 
     #[error("Asset mismatch: {0}")]
     AssetMismatch(String),

diff --git a/contracts/tfi-pair/src/testing.rs b/contracts/tfi-pair/src/testing.rs
@@ -357,10 +357,7 @@ fn provide_liquidity() {
         }],
     );
     let res = execute(deps.as_mut(), env, info, msg).unwrap_err();
-    match res {
-        ContractError::MaxSlippageAssertion {} => {}
-        _ => panic!("DO NOT ENTER HERE"),
-    }
+    assert!(matches!(res, ContractError::MaxSlippageAssertion { .. }));
 
     // initialize token balance to 1:1
     deps.querier.with_balance(&[(
@@ -395,10 +392,7 @@ fn provide_liquidity() {
         }],
     );
     let res = execute(deps.as_mut(), env, info, msg).unwrap_err();
-    match res {
-        ContractError::MaxSlippageAssertion {} => {}
-        _ => panic!("DO NOT ENTER HERE"),
-    }
+    assert!(matches!(res, ContractError::MaxSlippageAssertion { .. }));
 
     // initialize token balance to 1:1
     deps.querier.with_balance(&[(