From a80738cf378113811f99e9e9abbd620abeaa3a7f Mon Sep 17 00:00:00 2001 From: J Date: Tue, 7 Nov 2023 11:02:34 +0100 Subject: [PATCH 1/4] fix: assert bank operational states in withdaw_all/repay_all --- programs/marginfi/src/state/marginfi_account.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/programs/marginfi/src/state/marginfi_account.rs b/programs/marginfi/src/state/marginfi_account.rs index 6d81fda4..f0739593 100644 --- a/programs/marginfi/src/state/marginfi_account.rs +++ b/programs/marginfi/src/state/marginfi_account.rs @@ -754,6 +754,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 +806,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())?; From 023d72318843b2b5b7339d7363cdc18158f49e8f Mon Sep 17 00:00:00 2001 From: J Date: Tue, 7 Nov 2023 14:35:17 +0100 Subject: [PATCH 2/4] fix: explicit liquidation checks --- .../instructions/marginfi_account/liquidate.rs | 14 +++++++++++++- programs/marginfi/src/macros.rs | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/programs/marginfi/src/instructions/marginfi_account/liquidate.rs b/programs/marginfi/src/instructions/marginfi_account/liquidate.rs index 77a61de9..ee6f4704 100644 --- a/programs/marginfi/src/instructions/marginfi_account/liquidate.rs +++ b/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, 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, diff --git a/programs/marginfi/src/macros.rs b/programs/marginfi/src/macros.rs index 472853a8..995da6f5 100644 --- a/programs/marginfi/src/macros.rs +++ b/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] From eaf80bae9901b0802b53497b661e021aa24a545f Mon Sep 17 00:00:00 2001 From: J Date: Tue, 7 Nov 2023 14:35:39 +0100 Subject: [PATCH 3/4] feat: close balance ix --- programs/marginfi/src/errors.rs | 11 +- .../marginfi_account/close_balance.rs | 64 +++++++++++ .../src/instructions/marginfi_account/mod.rs | 2 + programs/marginfi/src/lib.rs | 6 ++ .../marginfi/src/state/marginfi_account.rs | 42 +++++++- programs/marginfi/tests/marginfi_account.rs | 102 ++++++++++++++++++ scripts/test.sh | 2 +- test-utils/src/marginfi_account.rs | 31 ++++++ 8 files changed, 249 insertions(+), 11 deletions(-) create mode 100644 programs/marginfi/src/instructions/marginfi_account/close_balance.rs diff --git a/programs/marginfi/src/errors.rs b/programs/marginfi/src/errors.rs index 485efdb1..8d00ae80 100644 --- a/programs/marginfi/src/errors.rs +++ b/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 for ProgramError { diff --git a/programs/marginfi/src/instructions/marginfi_account/close_balance.rs b/programs/marginfi/src/instructions/marginfi_account/close_balance.rs new file mode 100644 index 00000000..83e71d42 --- /dev/null +++ b/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) -> 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>, +} diff --git a/programs/marginfi/src/instructions/marginfi_account/mod.rs b/programs/marginfi/src/instructions/marginfi_account/mod.rs index 1896c8c5..7c5bca0c 100644 --- a/programs/marginfi/src/instructions/marginfi_account/mod.rs +++ b/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::*; diff --git a/programs/marginfi/src/lib.rs b/programs/marginfi/src/lib.rs index 1b364535..5bc1fc88 100644 --- a/programs/marginfi/src/lib.rs +++ b/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, + ) -> MarginfiResult { + marginfi_account::lending_account_close_balance(ctx) + } + pub fn lending_account_withdraw_emissions( ctx: Context, ) -> MarginfiResult { diff --git a/programs/marginfi/src/state/marginfi_account.rs b/programs/marginfi/src/state/marginfi_account.rs index f0739593..80fd9a1f 100644 --- a/programs/marginfi/src/state/marginfi_account.rs +++ b/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) @@ -848,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( diff --git a/programs/marginfi/tests/marginfi_account.rs b/programs/marginfi/tests/marginfi_account.rs index e5a4ab1c..ed9ef7e2 100644 --- a/programs/marginfi/tests/marginfi_account.rs +++ b/programs/marginfi/tests/marginfi_account.rs @@ -1733,3 +1733,105 @@ 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(()) +} diff --git a/scripts/test.sh b/scripts/test.sh index 3fd93830..9a8c24d8 100755 --- a/scripts/test.sh +++ b/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 \ No newline at end of file +RUST_LOG=error cargo test-sbf --features=test -- --skip marginfi_account_liquidation_success_many_balances --test-threads=1 diff --git a/test-utils/src/marginfi_account.rs b/test-utils/src/marginfi_account.rs index d993a356..0259ee98 100644 --- a/test-utils/src/marginfi_account.rs +++ b/test-utils/src/marginfi_account.rs @@ -245,6 +245,37 @@ impl MarginfiAccountFixture { Ok(()) } + pub async fn try_balance_close( + &self, + bank: &BankFixture, + ) -> anyhow::Result<(), BanksClientError> { + let marginfi_account = self.load().await; + let mut ctx = self.ctx.borrow_mut(); + + let ix = Instruction { + program_id: marginfi::id(), + accounts: marginfi::accounts::LendingAccountCloseBalance { + marginfi_group: marginfi_account.group, + marginfi_account: self.key, + signer: ctx.payer.pubkey(), + bank: bank.key, + } + .to_account_metas(Some(true)), + data: marginfi::instruction::LendingAccountCloseBalance.data(), + }; + + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&ctx.payer.pubkey().clone()), + &[&ctx.payer], + ctx.last_blockhash, + ); + + ctx.banks_client.process_transaction(tx).await?; + + Ok(()) + } + pub async fn try_liquidate>( &self, liquidatee: &MarginfiAccountFixture, From 7d1eb5cfa1e44ffec0ecc79c42816d82dc0c184b Mon Sep 17 00:00:00 2001 From: J Date: Tue, 7 Nov 2023 14:49:09 +0100 Subject: [PATCH 4/4] fix: fmt --- programs/marginfi/tests/marginfi_account.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/programs/marginfi/tests/marginfi_account.rs b/programs/marginfi/tests/marginfi_account.rs index ed9ef7e2..5a3628ed 100644 --- a/programs/marginfi/tests/marginfi_account.rs +++ b/programs/marginfi/tests/marginfi_account.rs @@ -1824,7 +1824,12 @@ async fn lending_account_close_balance() -> anyhow::Result<()> { // 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)) + .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);