Changing liquidation bonus rounding (#71)

Changing liquidation bonus rounding to floor

contracts/credit-manager/src/liquidate_coin.rs

@@ -1,6 +1,9 @@
 use std::ops::{Add, Div};
 
-use cosmwasm_std::{Coin, CosmosMsg, Decimal, DepsMut, Env, Response, StdError, Storage, Uint128};
+use cosmwasm_std::{
+    Coin, CosmosMsg, Decimal, DepsMut, Env, QuerierWrapper, Response, StdError, Storage, Uint128,
+};
+use mars_rover::adapters::Oracle;
 
 use mars_rover::error::{ContractError, ContractResult};
 use mars_rover::msg::execute::CallbackMsg;
@@ -120,14 +123,10 @@ pub fn calculate_liquidation(
         .add(Decimal::one())
         .checked_mul(debt_res.price.checked_mul(final_debt_to_repay.to_dec()?)?)?
         .div(request_res.price)
-        // Given the nature of integers, these operations will round down. This means the liquidation balance will get
-        // closer and closer to 0, but never actually get there and stay as a single denom unit.
-        // The remediation for this is to round up at the very end of the calculation.
-        .ceil()
         .uint128();
 
     // (Debt Coin, Request Coin)
-    Ok((
+    let result = (
         Coin {
             denom: debt_coin.denom.clone(),
             amount: final_debt_to_repay,
@@ -136,7 +135,11 @@ pub fn calculate_liquidation(
             denom: request_coin.to_string(),
             amount: request_amount,
         },
-    ))
+    );
+
+    assert_liquidation_profitable(&deps.querier, &oracle, result.clone())?;
+
+    Ok(result)
 }
 
 pub fn repay_debt(
@@ -158,3 +161,23 @@ pub fn repay_debt(
     .into_cosmos_msg(&env.contract.address)?;
     Ok(msg)
 }
+
+/// In scenarios with small amounts or large gap between coin prices, there is a possibility
+/// that the liquidation will result in loss for the liquidator. This assertion prevents this.
+fn assert_liquidation_profitable(
+    querier: &QuerierWrapper,
+    oracle: &Oracle,
+    (debt_coin, request_coin): (Coin, Coin),
+) -> ContractResult<()> {
+    let debt_value = oracle.query_total_value(querier, &[debt_coin.clone()])?;
+    let request_value = oracle.query_total_value(querier, &[request_coin.clone()])?;
+
+    if debt_value >= request_value {
+        return Err(ContractError::LiquidationNotProfitable {
+            debt_coin,
+            request_coin,
+        });
+    }
+
+    Ok(())
+}

contracts/credit-manager/tests/test_liquidate_coin.rs

@@ -2,7 +2,7 @@ use cosmwasm_std::{coins, Addr, Coin, Decimal, OverflowError, OverflowOperation,
 
 use mars_mock_oracle::msg::CoinPrice;
 use mars_rover::error::ContractError;
-use mars_rover::error::ContractError::{AboveMaxLTV, NotLiquidatable};
+use mars_rover::error::ContractError::{AboveMaxLTV, LiquidationNotProfitable, NotLiquidatable};
 use mars_rover::msg::execute::Action::{Borrow, Deposit, EnterVault, LiquidateCoin};
 use mars_rover::traits::IntoDecimal;
 
@@ -381,6 +381,74 @@ fn test_liquidator_left_in_unhealthy_state() {
     )
 }
 
+#[test]
+fn test_liquidation_not_profitable_after_calculations() {
+    let uosmo_info = uosmo_info();
+    let uatom_info = uatom_info();
+    let ujake_info = ujake_info();
+    let liquidator = Addr::unchecked("liquidator");
+    let liquidatee = Addr::unchecked("liquidatee");
+    let mut mock = MockEnv::new()
+        .allowed_coins(&[uosmo_info.clone(), uatom_info.clone(), ujake_info.clone()])
+        .fund_account(AccountToFund {
+            addr: liquidatee.clone(),
+            funds: coins(300, uosmo_info.denom.clone()),
+        })
+        .fund_account(AccountToFund {
+            addr: liquidator.clone(),
+            funds: coins(300, uatom_info.denom.clone()),
+        })
+        .build()
+        .unwrap();
+    let liquidatee_account_id = mock.create_credit_account(&liquidatee).unwrap();
+
+    mock.update_credit_account(
+        &liquidatee_account_id,
+        &liquidatee,
+        vec![
+            Deposit(uosmo_info.to_coin(300)),
+            Borrow(uatom_info.to_coin(100)),
+            Borrow(ujake_info.to_coin(25)),
+        ],
+        &[Coin::new(300, uosmo_info.denom.clone())],
+    )
+    .unwrap();
+
+    mock.price_change(CoinPrice {
+        denom: ujake_info.denom,
+        price: Decimal::from_atomics(100u128, 0).unwrap(),
+    });
+
+    mock.price_change(CoinPrice {
+        denom: uosmo_info.denom.clone(),
+        price: Decimal::from_atomics(2u128, 0).unwrap(),
+    });
+
+    let liquidator_account_id = mock.create_credit_account(&liquidator).unwrap();
+
+    let res = mock.update_credit_account(
+        &liquidator_account_id,
+        &liquidator,
+        vec![
+            Deposit(uatom_info.to_coin(10)),
+            LiquidateCoin {
+                liquidatee_account_id: liquidatee_account_id.clone(),
+                debt_coin: uatom_info.to_coin(5),
+                request_coin_denom: uosmo_info.denom.clone(),
+            },
+        ],
+        &[uatom_info.to_coin(10)],
+    );
+
+    assert_err(
+        res,
+        LiquidationNotProfitable {
+            debt_coin: uatom_info.to_coin(5),
+            request_coin: uosmo_info.to_coin(2),
+        },
+    )
+}
+
 #[test]
 fn test_debt_amount_adjusted_to_close_factor_max() {
     let uosmo_info = uosmo_info();
@@ -517,7 +585,7 @@ fn test_debt_amount_adjusted_to_total_debt_for_denom() {
     let position = mock.query_positions(&liquidatee_account_id);
     assert_eq!(position.coins.len(), 3);
     let osmo_balance = get_coin("uosmo", &position.coins);
-    assert_eq!(osmo_balance.amount, Uint128::new(180));
+    assert_eq!(osmo_balance.amount, Uint128::new(181));
     let atom_balance = get_coin("uatom", &position.coins);
     assert_eq!(atom_balance.amount, Uint128::new(100));
     let jake_balance = get_coin("ujake", &position.coins);
@@ -534,7 +602,7 @@ fn test_debt_amount_adjusted_to_total_debt_for_denom() {
     let jake_balance = get_coin("ujake", &position.coins);
     assert_eq!(jake_balance.amount, Uint128::new(39));
     let osmo_balance = get_coin("uosmo", &position.coins);
-    assert_eq!(osmo_balance.amount, Uint128::new(120));
+    assert_eq!(osmo_balance.amount, Uint128::new(119));
 }
 
 #[test]

contracts/credit-manager/tests/test_liquidate_vault.rs

@@ -381,7 +381,7 @@ fn test_liquidate_unlocked_vault() {
     let position = mock.query_positions(&liquidatee_account_id);
     assert_eq!(position.vaults.len(), 1);
     let vault_balance = position.vaults.first().unwrap().amount.unlocked();
-    assert_eq!(vault_balance, Uint128::new(883_532)); // 1M - 116_468
+    assert_eq!(vault_balance, Uint128::new(883_533)); // 1M - 116_467
 
     assert_eq!(position.coins.len(), 1);
     let jake_balance = get_coin("ujake", &position.coins);
@@ -468,8 +468,8 @@ fn test_liquidate_locked_vault() {
     let position = mock.query_positions(&liquidatee_account_id);
     assert_eq!(position.vaults.len(), 1);
     let vault_amount = position.vaults.first().unwrap().amount.clone();
-    // 1M - 835_528 vault tokens liquidated = 164,472
-    assert_eq!(vault_amount.locked(), Uint128::new(164_472));
+    // 1M - 835,527 vault tokens liquidated = 164,473
+    assert_eq!(vault_amount.locked(), Uint128::new(164_473));
     assert_eq!(vault_amount.unlocking().positions().len(), 0);
     assert_eq!(vault_amount.unlocked(), Uint128::zero());
 
@@ -580,7 +580,7 @@ fn test_liquidate_unlocking_liquidation_order() {
     // Total liquidated:                   24 LP tokens
     //   First bucket drained:                 2 of 2
     //   Second bucket drained:              10 of 10
-    //   Third bucket partially liquidated:  12 of 20
+    //   Third bucket partially liquidated:  11 of 20
     //   Fourth bucket retained:             0 of 168
     assert_eq!(vault_amount.unlocking().positions().len(), 2);
     assert_eq!(
@@ -591,7 +591,7 @@ fn test_liquidate_unlocking_liquidation_order() {
             .unwrap()
             .coin
             .amount,
-        Uint128::new(8)
+        Uint128::new(9)
     );
     assert_eq!(
         vault_amount
@@ -617,7 +617,7 @@ fn test_liquidate_unlocking_liquidation_order() {
     assert_eq!(position.coins.len(), 1);
     assert_eq!(position.debts.len(), 0);
     let lp_balance = get_coin(&lp_token.denom, &position.coins);
-    assert_eq!(lp_balance.amount, Uint128::new(24));
+    assert_eq!(lp_balance.amount, Uint128::new(23));
 }
 
 // NOTE: liquidation calculation+adjustments are quite complex, full cases in test_liquidate_coin.rs
@@ -693,7 +693,7 @@ fn test_liquidation_calculation_adjustment() {
     let position = mock.query_positions(&liquidatee_account_id);
     assert_eq!(position.vaults.len(), 1);
     let vault_balance = position.vaults.first().unwrap().amount.unlocked();
-    assert_eq!(vault_balance, Uint128::new(10_026)); // Vault position liquidated by 99%
+    assert_eq!(vault_balance, Uint128::new(10_027)); // Vault position liquidated by 99%
 
     assert_eq!(position.coins.len(), 1);
     let jake_balance = get_coin("ujake", &position.coins);

packages/rover/src/error.rs

@@ -1,5 +1,5 @@
 use cosmwasm_std::{
-    CheckedFromRatioError, CheckedMultiplyRatioError, DecimalRangeExceeded, OverflowError,
+    CheckedFromRatioError, CheckedMultiplyRatioError, Coin, DecimalRangeExceeded, OverflowError,
     StdError, Uint128,
 };
 use cw_controllers_admin_fork::AdminError;
@@ -61,6 +61,9 @@ pub enum ContractError {
     #[error("{reason:?}")]
     InvalidConfig { reason: String },
 
+    #[error("Paying down {debt_coin:?} for {request_coin:?} does not result in a profit for the liquidator")]
+    LiquidationNotProfitable { debt_coin: Coin, request_coin: Coin },
+
     #[error("Issued incorrect action for vault type")]
     MismatchedVaultType,