Merge pull request #133 from mrgnlabs/j/various-improvements

J/various improvements

programs/marginfi/src/errors.rs

@@ -29,8 +29,7 @@ pub enum MarginfiError {
     LendingAccountBalanceSlotsFull,
     #[msg("Bank already exists")] // 6012
     BankAlreadyExists,
-    // 6013
-    #[msg("Illegal post liquidation state, account is either not unhealthy or liquidation was too big")]
+    #[msg("Illegal liquidation")] // 6013
     IllegalLiquidation,
     #[msg("Account is not bankrupt")] // 6014
     AccountNotBankrupt,
@@ -65,8 +64,8 @@ pub enum MarginfiError {
     #[msg("Bank borrow cap exceeded")] // 6029
     BankLiabilityCapacityExceeded,
     #[msg("Invalid Price")] // 6030
-    InvalidPrice, // 6031
-    #[msg("Account can have only one liablity when account is under isolated risk")]
+    InvalidPrice,
+    #[msg("Account can have only one liablity when account is under isolated risk")] // 6031
     IsolatedAccountIllegalState, // 6032
     #[msg("Emissions already setup")]
     EmissionsAlreadySetup,
@@ -80,8 +79,10 @@ pub enum MarginfiError {
     EmissionsUpdateError,
     #[msg("Account disabled")] // 6037
     AccountDisabled,
-    #[msg("Account can't temporarily open 3 balances, please close a balance first")]
+    #[msg("Account can't temporarily open new balances, please close a balance first")]
     AccountTempActiveBalanceLimitExceeded,
+    #[msg("Illegal balance state")] // 6038
+    IllegalBalanceState,
 }
 
 impl From<MarginfiError> for ProgramError {

programs/marginfi/src/instructions/marginfi_account/close_balance.rs

@@ -0,0 +1,64 @@
+use anchor_lang::prelude::*;
+
+use crate::{
+    check,
+    prelude::*,
+    state::{
+        marginfi_account::{BankAccountWrapper, MarginfiAccount, DISABLED_FLAG},
+        marginfi_group::Bank,
+    },
+};
+
+pub fn lending_account_close_balance(ctx: Context<LendingAccountCloseBalance>) -> MarginfiResult {
+    let LendingAccountCloseBalance {
+        marginfi_account,
+        bank: bank_loader,
+        ..
+    } = ctx.accounts;
+
+    let mut marginfi_account = marginfi_account.load_mut()?;
+    let mut bank = bank_loader.load_mut()?;
+
+    check!(
+        !marginfi_account.get_flag(DISABLED_FLAG),
+        MarginfiError::AccountDisabled
+    );
+
+    bank.accrue_interest(
+        Clock::get()?.unix_timestamp,
+        #[cfg(not(feature = "client"))]
+        bank_loader.key(),
+    )?;
+
+    let mut bank_account = BankAccountWrapper::find(
+        &bank_loader.key(),
+        &mut bank,
+        &mut marginfi_account.lending_account,
+    )?;
+
+    bank_account.close_balance()?;
+
+    Ok(())
+}
+
+#[derive(Accounts)]
+pub struct LendingAccountCloseBalance<'info> {
+    pub marginfi_group: AccountLoader<'info, MarginfiGroup>,
+
+    #[account(
+        mut,
+        constraint = marginfi_account.load()?.group == marginfi_group.key(),
+    )]
+    pub marginfi_account: AccountLoader<'info, MarginfiAccount>,
+
+    #[account(
+        address = marginfi_account.load()?.authority,
+    )]
+    pub signer: Signer<'info>,
+
+    #[account(
+        mut,
+        constraint = bank.load()?.group == marginfi_group.key(),
+    )]
+    pub bank: AccountLoader<'info, Bank>,
+}

programs/marginfi/src/instructions/marginfi_account/liquidate.rs

@@ -2,7 +2,6 @@ use crate::constants::{
     INSURANCE_VAULT_SEED, LIQUIDATION_INSURANCE_FEE, LIQUIDATION_LIQUIDATOR_FEE, MAX_PRICE_AGE_SEC,
 };
 use crate::events::{AccountEventHeader, LendingAccountLiquidateEvent, LiquidationBalances};
-use crate::prelude::*;
 use crate::state::marginfi_account::{
     calc_asset_amount, calc_asset_value, RiskEngine, RiskRequirementType,
 };
@@ -13,6 +12,7 @@ use crate::{
     constants::{LIQUIDITY_VAULT_AUTHORITY_SEED, LIQUIDITY_VAULT_SEED},
     state::marginfi_account::{BankAccountWrapper, MarginfiAccount},
 };
+use crate::{check, prelude::*};
 use anchor_lang::prelude::*;
 use anchor_spl::token::{Token, TokenAccount, Transfer};
 use fixed::types::I80F48;
@@ -71,6 +71,18 @@ pub fn lending_account_liquidate(
     ctx: Context<LendingAccountLiquidate>,
     asset_amount: u64,
 ) -> MarginfiResult {
+    check!(
+        asset_amount > 0,
+        MarginfiError::IllegalLiquidation,
+        "Asset amount must be positive"
+    );
+
+    check!(
+        ctx.accounts.asset_bank.key() != ctx.accounts.liab_bank.key(),
+        MarginfiError::IllegalLiquidation,
+        "Asset and liability bank cannot be the same"
+    );
+
     let LendingAccountLiquidate {
         liquidator_marginfi_account: liquidator_marginfi_account_loader,
         liquidatee_marginfi_account: liquidatee_marginfi_account_loader,

programs/marginfi/src/instructions/marginfi_account/mod.rs

@@ -1,4 +1,5 @@
 mod borrow;
+mod close_balance;
 mod deposit;
 mod emissions;
 mod initialize;
@@ -7,6 +8,7 @@ mod repay;
 mod withdraw;
 
 pub use borrow::*;
+pub use close_balance::*;
 pub use deposit::*;
 pub use emissions::*;
 pub use initialize::*;

programs/marginfi/src/lib.rs

@@ -118,6 +118,12 @@ pub mod marginfi {
         marginfi_account::lending_account_borrow(ctx, amount)
     }
 
+    pub fn lending_account_close_balance(
+        ctx: Context<LendingAccountCloseBalance>,
+    ) -> MarginfiResult {
+        marginfi_account::lending_account_close_balance(ctx)
+    }
+
     pub fn lending_account_withdraw_emissions(
         ctx: Context<LendingAccountWithdrawEmissions>,
     ) -> MarginfiResult {

programs/marginfi/src/macros.rs

@@ -15,6 +15,22 @@ macro_rules! check {
             return Err(error_code.into());
         }
     };
+
+    ($cond:expr, $err:expr, $($arg:tt)*) => {
+        if !($cond) {
+            let error_code: $crate::errors::MarginfiError = $err;
+            #[cfg(not(feature = "test-bpf"))]
+            anchor_lang::prelude::msg!(
+                "Error \"{}\" thrown at {}:{}",
+                error_code,
+                file!(),
+                line!()
+            );
+            #[cfg(not(feature = "test-bpf"))]
+            anchor_lang::prelude::msg!($($arg)*);
+            return Err(error_code.into());
+        }
+    };
 }
 
 #[macro_export]

programs/marginfi/src/state/marginfi_account.rs

@@ -408,7 +408,8 @@ impl<'a, 'b> RiskEngine<'a, 'b> {
 
         check!(
             account_health <= I80F48::ZERO,
-            MarginfiError::IllegalLiquidation
+            MarginfiError::IllegalLiquidation,
+            "Account not unhealthy"
         );
 
         Ok(account_health)
@@ -438,12 +439,14 @@ impl<'a, 'b> RiskEngine<'a, 'b> {
             liability_bank_balance
                 .is_empty(BalanceSide::Liabilities)
                 .not(),
-            MarginfiError::IllegalLiquidation
+            MarginfiError::IllegalLiquidation,
+            "Liability payoff too severe"
         );
 
         check!(
             liability_bank_balance.is_empty(BalanceSide::Assets),
-            MarginfiError::IllegalLiquidation
+            MarginfiError::IllegalLiquidation,
+            "Liability payoff too severe"
         );
 
         let (assets, liabs) =
@@ -453,7 +456,8 @@ impl<'a, 'b> RiskEngine<'a, 'b> {
 
         check!(
             account_health <= I80F48::ZERO,
-            MarginfiError::IllegalLiquidation
+            MarginfiError::IllegalLiquidation,
+            "Liquidation too severe"
         );
 
         msg!(
@@ -466,7 +470,8 @@ impl<'a, 'b> RiskEngine<'a, 'b> {
 
         check!(
             account_health > pre_liquidation_health,
-            MarginfiError::IllegalLiquidation
+            MarginfiError::IllegalLiquidation,
+            "Post liquidation health worse"
         );
 
         Ok(account_health)
@@ -754,6 +759,8 @@ impl<'a> BankAccountWrapper<'a> {
         let balance = &mut self.balance;
         let bank = &mut self.bank;
 
+        bank.assert_operational_mode(None)?;
+
         let total_asset_shares: I80F48 = balance.asset_shares.into();
         let current_asset_amount = bank.get_asset_amount(total_asset_shares)?;
         let current_liability_amount =
@@ -804,6 +811,8 @@ impl<'a> BankAccountWrapper<'a> {
         let balance = &mut self.balance;
         let bank = &mut self.bank;
 
+        bank.assert_operational_mode(None)?;
+
         let total_liability_shares: I80F48 = balance.liability_shares.into();
         let current_liability_amount = bank.get_liability_amount(total_liability_shares)?;
         let current_asset_amount = bank.get_asset_amount(balance.asset_shares.into())?;
@@ -844,6 +853,33 @@ impl<'a> BankAccountWrapper<'a> {
             .ok_or_else(math_error!())?)
     }
 
+    pub fn close_balance(&mut self) -> MarginfiResult<()> {
+        self.claim_emissions(Clock::get()?.unix_timestamp as u64)?;
+
+        let balance = &mut self.balance;
+        let bank = &mut self.bank;
+
+        let current_liability_amount =
+            bank.get_liability_amount(balance.liability_shares.into())?;
+        let current_asset_amount = bank.get_asset_amount(balance.asset_shares.into())?;
+
+        check!(
+            current_liability_amount.is_zero_with_tolerance(ZERO_AMOUNT_THRESHOLD),
+            MarginfiError::IllegalBalanceState,
+            "Balance has existing debt"
+        );
+
+        check!(
+            current_asset_amount.is_zero_with_tolerance(ZERO_AMOUNT_THRESHOLD),
+            MarginfiError::IllegalBalanceState,
+            "Balance has existing assets"
+        );
+
+        balance.close()?;
+
+        Ok(())
+    }
+
     // ------------ Internal accounting logic
 
     fn increase_balance_internal(

programs/marginfi/tests/marginfi_account.rs

@@ -1733,3 +1733,110 @@ async fn emissions_test_2() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+#[tokio::test]
+async fn lending_account_close_balance() -> anyhow::Result<()> {
+    let test_f = TestFixture::new(Some(TestSettings::all_banks_payer_not_admin())).await;
+
+    let usdc_bank = test_f.get_bank(&BankMint::USDC);
+    let sol_eq_bank = test_f.get_bank(&BankMint::SolEquivalent);
+    let sol_bank = test_f.get_bank(&BankMint::SOL);
+
+    // Fund SOL lender
+    let lender_mfi_account_f = test_f.create_marginfi_account().await;
+    let lender_token_account_sol = test_f
+        .sol_equivalent_mint
+        .create_token_account_and_mint_to(1_000)
+        .await;
+    lender_mfi_account_f
+        .try_bank_deposit(lender_token_account_sol.key, sol_eq_bank, 1_000)
+        .await?;
+
+    let lender_token_account_sol = test_f
+        .sol_mint
+        .create_token_account_and_mint_to(1_000)
+        .await;
+    lender_mfi_account_f
+        .try_bank_deposit(lender_token_account_sol.key, sol_bank, 1_000)
+        .await?;
+
+    let res = lender_mfi_account_f.try_balance_close(sol_bank).await;
+
+    assert!(res.is_err());
+    assert_custom_error!(res.unwrap_err(), MarginfiError::IllegalBalanceState);
+
+    // Fund SOL borrower
+    let borrower_mfi_account_f = test_f.create_marginfi_account().await;
+    let borrower_token_account_f_usdc = test_f
+        .usdc_mint
+        .create_token_account_and_mint_to(1_000)
+        .await;
+    let borrower_token_account_f_sol = test_f
+        .sol_mint
+        .create_token_account_and_mint_to(1_000)
+        .await;
+    let borrower_token_account_f_sol_eq = test_f
+        .sol_equivalent_mint
+        .create_token_account_and_mint_to(1_000)
+        .await;
+    borrower_mfi_account_f
+        .try_bank_deposit(borrower_token_account_f_usdc.key, usdc_bank, 1_000)
+        .await?;
+
+    // Borrow SOL EQ
+    let res = borrower_mfi_account_f
+        .try_bank_borrow(borrower_token_account_f_sol_eq.key, sol_eq_bank, 0.01)
+        .await;
+
+    assert!(res.is_ok());
+
+    // Borrow SOL
+    let res = borrower_mfi_account_f
+        .try_bank_borrow(borrower_token_account_f_sol.key, sol_bank, 0.01)
+        .await;
+
+    assert!(res.is_ok());
+
+    // This issue is not that bad, because the user can still borrow other assets (isolated liab < empty threshold)
+    let res = borrower_mfi_account_f.try_balance_close(sol_bank).await;
+    assert!(res.is_err());
+    assert_custom_error!(res.unwrap_err(), MarginfiError::IllegalBalanceState);
+
+    // Let a second go b
+    {
+        let mut ctx = test_f.context.borrow_mut();
+        let mut clock: Clock = ctx.banks_client.get_sysvar().await?;
+        // Advance clock by 1 second
+        clock.unix_timestamp += 1;
+        ctx.set_sysvar(&clock);
+    }
+
+    // Repay isolated SOL EQ borrow successfully
+    let res = borrower_mfi_account_f
+        .try_bank_repay(
+            borrower_token_account_f_sol_eq.key,
+            sol_eq_bank,
+            0.01,
+            Some(false),
+        )
+        .await;
+    assert!(res.is_ok());
+
+    // Liability share in balance is smaller than 0.0001, so repay all should fail
+    let res = borrower_mfi_account_f
+        .try_bank_repay(
+            borrower_token_account_f_sol_eq.key,
+            sol_eq_bank,
+            1,
+            Some(true),
+        )
+        .await;
+    assert!(res.is_err());
+    assert_custom_error!(res.unwrap_err(), MarginfiError::NoLiabilityFound);
+
+    // This issue is not that bad, because the user can still borrow other assets (isolated liab < empty threshold)
+    let res = borrower_mfi_account_f.try_balance_close(sol_eq_bank).await;
+    assert!(res.is_ok());
+
+    Ok(())
+}

scripts/test.sh

@@ -1,3 +1,3 @@
 #!/usr/bin/env bash
 anchor build --program-name marginfi
-RUST_LOG=error cargo test-sbf --features=test -- --test-threads=1
+RUST_LOG=error cargo test-sbf --features=test -- --skip marginfi_account_liquidation_success_many_balances --test-threads=1