fix rounding in borrow + tests

contracts/common-token/src/balance.rs

@@ -1,12 +1,12 @@
 use soroban_sdk::{Address, Env};
 
-use crate::require_nonnegative_amount;
+use crate::require_positive_amount;
 use crate::storage::{
     is_authorized, read_balance, read_total_supply, write_balance, write_total_supply,
 };
 
 pub fn receive_balance(e: &Env, addr: Address, amount: i128) {
-    require_nonnegative_amount(amount);
+    require_positive_amount(amount);
     let balance = read_balance(e, addr.clone());
     if !is_authorized(e, addr.clone()) {
         panic!("can't receive when deauthorized");
@@ -15,7 +15,7 @@ pub fn receive_balance(e: &Env, addr: Address, amount: i128) {
 }
 
 pub fn spend_balance(e: &Env, addr: Address, amount: i128) {
-    require_nonnegative_amount(amount);
+    require_positive_amount(amount);
     let balance = read_balance(e, addr.clone());
     if !is_authorized(e, addr.clone()) {
         panic!("can't spend when deauthorized");

contracts/common-token/src/lib.rs

@@ -17,3 +17,9 @@ pub fn require_nonnegative_amount(amount: i128) {
         panic!("negative amount is not allowed: {}", amount)
     }
 }
+
+pub fn require_positive_amount(amount: i128) {
+    if amount <= 0 {
+        panic!("zero or negative amount is not allowed: {}", amount)
+    }
+}

contracts/common/src/fixedi128.rs

@@ -90,6 +90,25 @@ impl FixedI128 {
             .checked_div(self.0)
     }
 
+    /// Calculates division of non fixed int value and fixed value, e.g.  other / self and rounds towards infinity.
+    /// Result is int value
+    pub fn recip_mul_int_ceil<T: Into<i128>>(self, other: T) -> Option<i128> {
+        let other = other.into();
+        if other == 0 {
+            return Some(0);
+        }
+        let mb_res = Self::DENOMINATOR.checked_mul(other)?.checked_div(self.0);
+        mb_res.map(|res| {
+            if res == 0 {
+                1
+            } else if other >= self.0 && other % self.0 == 0 {
+                res
+            } else {
+                res + 1
+            }
+        })
+    }
+
     /// Multiply inner value of fixed
     pub fn mul_inner<T: Into<i128>>(self, value: T) -> Option<FixedI128> {
         self.0.checked_mul(value.into()).map(FixedI128)

contracts/common/src/test.rs

@@ -111,4 +111,43 @@ mod fixedi128 {
         let zero = FixedI128::from_inner(0);
         assert_eq!(zero.recip_mul_int(value), None);
     }
+
+    #[test]
+    fn recip_mul_int_ceil() {
+        let inner = 1000054757;
+        let fixed = FixedI128::from_inner(inner);
+        let value = 1;
+        assert_eq!(fixed.recip_mul_int_ceil(value).unwrap(), 1);
+
+        let value2 = 2;
+        assert_eq!(fixed.recip_mul_int_ceil(value2).unwrap(), 2);
+
+        let zero = FixedI128::from_inner(0);
+        assert_eq!(zero.recip_mul_int_ceil(value), None);
+
+        assert_eq!(
+            fixed.recip_mul_int_ceil(inner).unwrap(),
+            FixedI128::DENOMINATOR
+        );
+
+        let zero = 0;
+        assert_eq!(fixed.recip_mul_int_ceil(zero).unwrap(), 0);
+
+        assert_eq!(
+            fixed.recip_mul_int_ceil(inner + 1).unwrap(),
+            FixedI128::DENOMINATOR + 1
+        );
+        assert_eq!(
+            fixed.recip_mul_int_ceil(inner + 2).unwrap(),
+            FixedI128::DENOMINATOR + 2
+        );
+        assert_eq!(
+            fixed.recip_mul_int_ceil(inner + 9).unwrap(),
+            FixedI128::DENOMINATOR + 9
+        );
+        assert_eq!(
+            fixed.recip_mul_int_ceil(inner + 10).unwrap(),
+            FixedI128::DENOMINATOR + 10
+        );
+    }
 }

contracts/pool/src/methods/borrow.rs

@@ -103,7 +103,7 @@ pub fn do_borrow(
 
     let debt_coeff = get_actual_borrower_accrued_rate(env, reserve)?;
     let amount_of_debt_token = debt_coeff
-        .recip_mul_int(amount)
+        .recip_mul_int_ceil(amount)
         .ok_or(Error::MathOverflowError)?;
 
     require_util_cap_not_exceeded(

contracts/pool/src/tests/borrow.rs

@@ -310,16 +310,16 @@ fn should_change_balances_when_borrow_and_repay() {
     assert_eq!(underlying_supply_before, 100_000_000);
 
     assert_eq!(treasury_after_borrow, 0);
-    assert_eq!(debt_balance_after_borrow, 20_000_000);
-    assert_eq!(debt_total_after_borrow, 20_000_000);
+    assert_eq!(debt_balance_after_borrow, 20_000_001);
+    assert_eq!(debt_total_after_borrow, 20_000_001);
     assert_eq!(borrower_balance_after_borrow, 1_020_000_000);
     assert_eq!(underlying_supply_after_borrow, 80_000_000);
 
     assert_eq!(treasury_after_repay, 37_156);
     assert_eq!(debt_balance_after_repay, 0);
     assert_eq!(debt_total_after_repay, 0);
-    assert_eq!(borrower_balance_after_repay, 999_954_790);
-    assert_eq!(underlying_supply_after_repay, 100_008_054);
+    assert_eq!(borrower_balance_after_repay, 999_954_789);
+    assert_eq!(underlying_supply_after_repay, 100_008_055);
 }
 
 #[test]

contracts/pool/src/tests/collat_coeff.rs

@@ -59,11 +59,11 @@ fn should_update_when_deposit_borrow_withdraw_liquidate() {
     env.ledger().with_mut(|l| l.timestamp = 6 * DAY);
     let collat_coeff_after_liquidate = sut.pool.collat_coeff(&debt_token);
 
-    assert_eq!(collat_coeff_initial, 1_000_000_000);
-    assert_eq!(collat_coeff_after_withdraw, 1_000_000_000);
-    assert_eq!(collat_coeff_after_borrow, 1_000_199_480);
-    assert_eq!(collat_coeff_after_price_change, 1_000_265_990);
-    assert_eq!(collat_coeff_after_liquidate, 1_000_295_540);
+    assert_eq!(collat_coeff_initial, 1_000_000_010);
+    assert_eq!(collat_coeff_after_withdraw, 1_000_000_010);
+    assert_eq!(collat_coeff_after_borrow, 1_000_199_500);
+    assert_eq!(collat_coeff_after_price_change, 1_000_266_010);
+    assert_eq!(collat_coeff_after_liquidate, 1_000_295_560);
 }
 
 #[test]
@@ -83,9 +83,9 @@ fn should_change_over_time() {
     env.ledger().with_mut(|l| l.timestamp = 5 * DAY);
     let collat_coeff_3 = sut.pool.collat_coeff(&debt_token);
 
-    assert_eq!(collat_coeff_1, 1_000_330_690);
-    assert_eq!(collat_coeff_2, 1_000_440_950);
-    assert_eq!(collat_coeff_3, 1_000_551_210);
+    assert_eq!(collat_coeff_1, 1_000_330_700);
+    assert_eq!(collat_coeff_2, 1_000_440_960);
+    assert_eq!(collat_coeff_3, 1_000_551_220);
 }
 
 #[test]

contracts/pool/src/tests/flash_loan.rs

@@ -167,7 +167,7 @@ fn should_borrow_if_borrowing_specified_on_asset() {
     let borrower_debt_after = sut.reserves[2].debt_token.balance(&borrower);
 
     assert_eq!(borrower_debt_before, 0);
-    assert_eq!(borrower_debt_after, 3000000);
+    assert_eq!(borrower_debt_after, 3000001);
 }
 
 #[test]

contracts/pool/src/tests/liquidate.rs

@@ -198,7 +198,7 @@ fn should_liquidate_and_receive_collateral_partially() {
     assert_eq!(underlying_2_supply_before, 150_000_000);
     assert_eq!(borrower_stoken_0_balance_before, 1_000_000);
     assert_eq!(borrower_stoken_2_balance_before, 50_000_000);
-    assert_eq!(borrower_debt_balance_before, 60_000_000);
+    assert_eq!(borrower_debt_balance_before, 60_000_001);
     assert_eq!(liquidator_repayment_balance_before, 1_000_000_000);
     assert_eq!(liquidator_underlying_0_balance_before, 0);
     assert_eq!(liquidator_underlying_2_balance_before, 0);
@@ -207,7 +207,7 @@ fn should_liquidate_and_receive_collateral_partially() {
 
     assert_eq!(underlying_0_supply_after_partial_liquidation, 1_000_000);
     assert_eq!(borrower_stoken_0_balance_after_partial_liquidation, 0);
-    assert_eq!(borrower_debt_balance_after_partial_liquidation, 15_055_058);
+    assert_eq!(borrower_debt_balance_after_partial_liquidation, 15_055_059);
     assert_eq!(
         liquidator_repayment_balance_after_partial_liquidation,
         955_000_000
@@ -307,7 +307,7 @@ fn should_receive_stokens_when_requested() {
     assert_eq!(underlying_2_supply_before, 150_000_000);
     assert_eq!(borrower_stoken_1_balance_before, 1_000_000);
     assert_eq!(borrower_stoken_2_balance_before, 50_000_000);
-    assert_eq!(borrower_debt_balance_before, 60_000_000);
+    assert_eq!(borrower_debt_balance_before, 60_000_001);
     assert_eq!(liquidator_repayment_balance_before, 1_000_000_000);
     assert_eq!(liquidator_underlying_1_balance_before, 0);
     assert_eq!(liquidator_underlying_2_balance_before, 0);
@@ -328,7 +328,7 @@ fn should_receive_stokens_when_requested() {
     assert_eq!(borrower_debt_balance_after_full_liquidation, 0);
     assert_eq!(
         liquidator_repayment_balance_after_full_liquidation,
-        939_933_589
+        939_933_588
     );
     assert_eq!(
         liquidator_stoken_2_balance_after_full_liquidation,

contracts/pool/src/tests/mod.rs

@@ -16,6 +16,7 @@ pub mod liquidate;
 pub mod paused;
 pub mod rates;
 pub mod repay;
+pub mod rounding;
 pub mod set_as_collateral;
 pub mod set_base_asset;
 pub mod set_flash_loan_fee;

contracts/pool/src/tests/repay.rs

@@ -24,7 +24,7 @@ fn should_partially_repay() {
     assert_eq!(stoken_underlying_balance, 60_000_000);
     assert_eq!(user_balance, 1_040_000_000);
     assert_eq!(treasury_balance, 0);
-    assert_eq!(user_debt_balance, 40_000_000);
+    assert_eq!(user_debt_balance, 40_000_001);
 
     sut.pool.repay(&borrower, &debt_token, &20_000_000i128);
 
@@ -36,7 +36,7 @@ fn should_partially_repay() {
     assert_eq!(stoken_underlying_balance, 79_997_089);
     assert_eq!(user_balance, 1_020_000_000);
     assert_eq!(treasury_balance, 2_911);
-    assert_eq!(user_debt_balance, 20_004_548);
+    assert_eq!(user_debt_balance, 20_004_549);
 }
 
 #[test]
@@ -60,7 +60,7 @@ fn should_fully_repay() {
     assert_eq!(stoken_underlying_balance, 60_000_000);
     assert_eq!(user_balance, 1_040_000_000);
     assert_eq!(treasury_balance, 0);
-    assert_eq!(user_debt_balance, 40_000_000);
+    assert_eq!(user_debt_balance, 40_000_001);
 
     sut.pool.repay(&borrower, &debt_token, &i128::MAX);
 
@@ -69,8 +69,8 @@ fn should_fully_repay() {
     let treasury_balance = debt_config.token.balance(&treasury_address);
     let user_debt_balance = debt_config.debt_token.balance(&borrower);
 
-    assert_eq!(stoken_underlying_balance, 100_003_274);
-    assert_eq!(user_balance, 999_990_903);
+    assert_eq!(stoken_underlying_balance, 100_003_275);
+    assert_eq!(user_balance, 999_990_902);
     assert_eq!(treasury_balance, 5_823);
     assert_eq!(user_debt_balance, 0);
 }
@@ -174,7 +174,7 @@ fn should_emit_events() {
             (
                 sut.pool.address.clone(),
                 (Symbol::new(&env, "repay"), borrower.clone()).into_val(&env),
-                (debt_token, 40_009_097i128).into_val(&env)
+                (debt_token, 40_009_098i128).into_val(&env)
             ),
         ]
     );

contracts/pool/src/tests/rounding.rs

@@ -0,0 +1,132 @@
+use soroban_sdk::{testutils::Address as _, Address, Env};
+
+use crate::tests::sut::{fill_pool, init_pool};
+
+#[test]
+fn rounding_deposit_withdraw() {
+    extern crate std;
+
+    let env = Env::default();
+    env.mock_all_auths();
+
+    let sut = init_pool(&env, false);
+    let (_, _borrower, debt_config) = fill_pool(&env, &sut, true);
+    let token_address = debt_config.token.address.clone();
+
+    let attacker = Address::random(&env);
+    sut.reserves[1]
+        .token_admin
+        .mint(&attacker, &100_000_000_000);
+
+    std::println!("collat coeff {:?}", sut.pool.collat_coeff(&token_address));
+    std::println!("debt coeff {:?}", sut.pool.debt_coeff(&token_address));
+    // i = 1 will panic with s-token: invalid mint amount, cause mint_amount would be equal to 0
+    for i in 2..101 {
+        env.budget().reset_unlimited();
+
+        let balance_before = sut.reserves[1].token.balance(&attacker);
+        let s_balance_before = sut.reserves[1].s_token.balance(&attacker);
+
+        sut.pool.deposit(&attacker, &token_address, &i);
+
+        let s_balance_after_deposit = sut.reserves[1].s_token.balance(&attacker);
+
+        let s_token_balance = sut.reserves[1].s_token.balance(&attacker);
+        if s_token_balance == 0 || s_token_balance >= i {
+            std::println!("input {:?}, output {:?}", i, s_token_balance);
+            panic!();
+        }
+
+        sut.pool.withdraw(&attacker, &token_address, &i, &attacker);
+
+        let s_balance_after_withdraw = sut.reserves[1].s_token.balance(&attacker);
+
+        if s_balance_after_deposit <= s_balance_before
+            || s_balance_after_withdraw > s_balance_after_deposit
+        {
+            std::println!("{:?}: s_balance_before {:?}, s_balance_after_deposit {:?}, s_balance_after_withdraw {:?}",
+            i,
+            s_balance_before,
+            s_balance_after_deposit,
+            s_balance_after_withdraw);
+            panic!();
+        }
+
+        let balance_after = sut.reserves[1].token.balance(&attacker);
+
+        if balance_after > balance_before {
+            std::println!(
+                "{:?}: balance_before: {:?} balance_after: {:?}",
+                i,
+                balance_before,
+                balance_after
+            );
+            panic!();
+        }
+    }
+}
+
+#[test]
+fn rounding_borrow_repay() {
+    extern crate std;
+
+    let env = Env::default();
+    env.mock_all_auths();
+
+    let sut = init_pool(&env, false);
+    let (_, _borrower, _debt_config) = fill_pool(&env, &sut, true);
+    let token_address = sut.reserves[1].token.address.clone();
+
+    let attacker = Address::random(&env);
+    sut.reserves[0]
+        .token_admin
+        .mint(&attacker, &100_000_000_000);
+    sut.pool
+        .deposit(&attacker, &sut.reserves[0].token.address, &100_000_000_000);
+
+    std::println!("collat coeff {:?}", sut.pool.collat_coeff(&token_address));
+    std::println!("debt coeff {:?}", sut.pool.debt_coeff(&token_address));
+    // i = 1 will panic with zero or negative amount is not allowed, cause mint_amount would be equal to 0
+    for i in 2..101 {
+        env.budget().reset_unlimited();
+
+        let balance_before = sut.reserves[1].token.balance(&attacker);
+        let d_balance_before = sut.reserves[1].debt_token.balance(&attacker);
+
+        sut.pool.borrow(&attacker, &token_address, &i);
+
+        let d_balance_after_borrow = sut.reserves[1].debt_token.balance(&attacker);
+
+        if d_balance_after_borrow == 0 {
+            std::println!("input {:?}, output {:?}", i, d_balance_after_borrow);
+            panic!();
+        }
+
+        sut.pool.repay(&attacker, &token_address, &i);
+
+        let d_balance_after_repay = sut.reserves[1].debt_token.balance(&attacker);
+
+        if d_balance_after_borrow <= d_balance_before
+            || d_balance_after_repay == d_balance_after_borrow
+        {
+            std::println!("{:?}: d_balance_before {:?}, d_balance_after_borrow {:?}, d_balance_after_repay {:?}",
+            i,
+            d_balance_before,
+            d_balance_after_borrow,
+            d_balance_after_repay);
+            panic!();
+        }
+
+        let balance_after = sut.reserves[1].token.balance(&attacker);
+
+        if balance_after > balance_before {
+            std::println!(
+                "{:?}: balance_before: {:?} balance_after: {:?}",
+                i,
+                balance_before,
+                balance_after
+            );
+            panic!();
+        }
+    }
+}