From 6439791233c8c701a4bd5f2506662206effe87c7 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 20 Dec 2022 11:24:19 +0100 Subject: [PATCH 1/5] Token deposit: limit while being_liquidated While the flag is true, deposits that don't bring the account health over the init threshold are forbidden. This makes it impossible for users to drag out liquidation by continuously depositing tiny amounts. --- programs/mango-v4/src/error.rs | 2 ++ programs/mango-v4/src/instructions/token_deposit.rs | 7 ++++++- programs/mango-v4/tests/test_liq_perps.rs | 6 +++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/programs/mango-v4/src/error.rs b/programs/mango-v4/src/error.rs index bf871b390..bfc5b8ae5 100644 --- a/programs/mango-v4/src/error.rs +++ b/programs/mango-v4/src/error.rs @@ -63,6 +63,8 @@ pub enum MangoError { BankNetBorrowsLimitReached, #[msg("token position does not exist")] TokenPositionDoesNotExist, + #[msg("token deposits into accounts that are being liquidated must bring their health above the init threshold")] + DepositsIntoLiquidatingMustRecover, } impl MangoError { diff --git a/programs/mango-v4/src/instructions/token_deposit.rs b/programs/mango-v4/src/instructions/token_deposit.rs index 9a8e9e54f..f31d028e8 100644 --- a/programs/mango-v4/src/instructions/token_deposit.rs +++ b/programs/mango-v4/src/instructions/token_deposit.rs @@ -156,7 +156,12 @@ impl<'a, 'info> DepositCommon<'a, 'info> { let health = compute_health(&account.borrow(), HealthType::Init, &retriever) .context("post-deposit init health")?; msg!("health: {}", health); - account.fixed.maybe_recover_from_being_liquidated(health); + let was_being_liquidated = account.being_liquidated(); + let recovered = account.fixed.maybe_recover_from_being_liquidated(health); + require!( + !was_being_liquidated || recovered, + MangoError::DepositsIntoLiquidatingMustRecover + ); } // diff --git a/programs/mango-v4/tests/test_liq_perps.rs b/programs/mango-v4/tests/test_liq_perps.rs index 08dbe4d1e..e0bf8d858 100644 --- a/programs/mango-v4/tests/test_liq_perps.rs +++ b/programs/mango-v4/tests/test_liq_perps.rs @@ -601,8 +601,11 @@ async fn test_liq_perps_base_position_and_bankruptcy() -> Result<(), TransportEr // // SETUP: We want pnl settling to cause a negative quote position, - // thus we deposit some base token collateral + // thus we deposit some base token collateral. To be able to do that, + // we need to temporarily raise health > 0, deposit, then bring health + // negative again for the test // + set_bank_stub_oracle_price(solana, group, quote_token, admin, 2.0).await; send_tx( solana, TokenDepositInstruction { @@ -616,6 +619,7 @@ async fn test_liq_perps_base_position_and_bankruptcy() -> Result<(), TransportEr ) .await .unwrap(); + set_bank_stub_oracle_price(solana, group, quote_token, admin, 1.0).await; // // TEST: Can settle-pnl even though health is negative From fb43ec0e7febcea3dfe36e29c00fd36d319cdf3b Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 20 Dec 2022 12:20:31 +0100 Subject: [PATCH 2/5] liquidator: Don't panic when accounts not yet available This can happen when users created a new serum open orders account and it has not yet been received even though the account itself has arrived. --- liquidator/src/liquidate.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/liquidator/src/liquidate.rs b/liquidator/src/liquidate.rs index 43ae383ba..0b53cacbb 100644 --- a/liquidator/src/liquidate.rs +++ b/liquidator/src/liquidate.rs @@ -621,7 +621,7 @@ pub async fn maybe_liquidate_account( let account = account_fetcher.fetch_mango_account(pubkey)?; let health_cache = health_cache::new(&mango_client.context, account_fetcher, &account) .await - .expect("always ok"); + .context("creating health cache 1")?; let maint_health = health_cache.health(HealthType::Maint); if !health_cache.is_liquidatable() { return Ok(false); @@ -640,7 +640,7 @@ pub async fn maybe_liquidate_account( let account = account_fetcher.fetch_fresh_mango_account(pubkey).await?; let health_cache = health_cache::new(&mango_client.context, account_fetcher, &account) .await - .expect("always ok"); + .context("creating health cache 2")?; if !health_cache.is_liquidatable() { return Ok(false); } From 4bd1cf1ececd60427d40ab99f2bda75a2c1c5774 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 20 Dec 2022 12:21:30 +0100 Subject: [PATCH 3/5] liquidator: Shuffle liquidation check This way the liquidator is less likely to get stuck on one account if there are multiple that need liquidation. --- liquidator/src/liquidate.rs | 42 +++++++++++++++++-------------------- liquidator/src/main.rs | 15 +++++++++++-- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/liquidator/src/liquidate.rs b/liquidator/src/liquidate.rs index 0b53cacbb..d7c359d35 100644 --- a/liquidator/src/liquidate.rs +++ b/liquidator/src/liquidate.rs @@ -686,34 +686,30 @@ pub async fn maybe_liquidate_account( } #[allow(clippy::too_many_arguments)] -pub async fn maybe_liquidate_one<'a>( +pub async fn maybe_liquidate( mango_client: &MangoClient, account_fetcher: &chain_data::AccountFetcher, - accounts: impl Iterator, + pubkey: &Pubkey, config: &Config, ) -> bool { - for pubkey in accounts { - match maybe_liquidate_account(mango_client, account_fetcher, pubkey, config).await { - Err(err) => { - // Not all errors need to be raised to the user's attention. - let mut log_level = log::Level::Error; + match maybe_liquidate_account(mango_client, account_fetcher, pubkey, config).await { + Err(err) => { + // Not all errors need to be raised to the user's attention. + let mut log_level = log::Level::Error; - // Simulation errors due to liqee precondition failures on the liquidation instructions - // will commonly happen if our liquidator is late or if there are chain forks. - match err.downcast_ref::() { - Some(MangoClientError::SendTransactionPreflightFailure { logs }) => { - if logs.contains("HealthMustBeNegative") || logs.contains("IsNotBankrupt") { - log_level = log::Level::Trace; - } + // Simulation errors due to liqee precondition failures on the liquidation instructions + // will commonly happen if our liquidator is late or if there are chain forks. + match err.downcast_ref::() { + Some(MangoClientError::SendTransactionPreflightFailure { logs }) => { + if logs.contains("HealthMustBeNegative") || logs.contains("IsNotBankrupt") { + log_level = log::Level::Trace; } - _ => {} - }; - log::log!(log_level, "liquidating account {}: {:?}", pubkey, err); - } - Ok(true) => return true, - _ => {} - }; + } + _ => {} + }; + log::log!(log_level, "liquidating account {}: {:?}", pubkey, err); + false + } + Ok(liquidated) => liquidated, } - - false } diff --git a/liquidator/src/main.rs b/liquidator/src/main.rs index 6877932a7..5b8d8290e 100644 --- a/liquidator/src/main.rs +++ b/liquidator/src/main.rs @@ -338,11 +338,22 @@ async fn main() -> anyhow::Result<()> { async fn liquidate<'a>( mango_client: &MangoClient, account_fetcher: &chain_data::AccountFetcher, - accounts: impl Iterator, + accounts_iter: impl Iterator, config: &liquidate::Config, rebalance_config: &rebalance::Config, ) -> anyhow::Result<()> { - if !liquidate::maybe_liquidate_one(mango_client, account_fetcher, accounts, config).await { + use rand::seq::SliceRandom; + let mut rng = rand::thread_rng(); + + let mut accounts = accounts_iter.collect::>(); + accounts.shuffle(&mut rng); + + let mut liquidated_one = false; + for pubkey in accounts { + liquidated_one |= + liquidate::maybe_liquidate(mango_client, account_fetcher, pubkey, config).await; + } + if !liquidated_one { return Ok(()); } From 21c13fa9b150ebce4a721f0908d260ec18c8f035 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 20 Dec 2022 14:14:20 +0100 Subject: [PATCH 4/5] liquidator: Skip accounts with many errors in short time --- liquidator/src/liquidate.rs | 31 +------ liquidator/src/main.rs | 163 +++++++++++++++++++++++++++--------- liquidator/src/rebalance.rs | 1 + 3 files changed, 126 insertions(+), 69 deletions(-) diff --git a/liquidator/src/liquidate.rs b/liquidator/src/liquidate.rs index d7c359d35..d0c9d0b08 100644 --- a/liquidator/src/liquidate.rs +++ b/liquidator/src/liquidate.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use std::time::Duration; -use client::{chain_data, health_cache, AccountFetcher, MangoClient, MangoClientError}; +use client::{chain_data, health_cache, AccountFetcher, MangoClient}; use mango_v4::accounts_zerocopy::KeyedAccountSharedData; use mango_v4::health::{HealthCache, HealthType}; use mango_v4::state::{ @@ -684,32 +684,3 @@ pub async fn maybe_liquidate_account( Ok(true) } - -#[allow(clippy::too_many_arguments)] -pub async fn maybe_liquidate( - mango_client: &MangoClient, - account_fetcher: &chain_data::AccountFetcher, - pubkey: &Pubkey, - config: &Config, -) -> bool { - match maybe_liquidate_account(mango_client, account_fetcher, pubkey, config).await { - Err(err) => { - // Not all errors need to be raised to the user's attention. - let mut log_level = log::Level::Error; - - // Simulation errors due to liqee precondition failures on the liquidation instructions - // will commonly happen if our liquidator is late or if there are chain forks. - match err.downcast_ref::() { - Some(MangoClientError::SendTransactionPreflightFailure { logs }) => { - if logs.contains("HealthMustBeNegative") || logs.contains("IsNotBankrupt") { - log_level = log::Level::Trace; - } - } - _ => {} - }; - log::log!(log_level, "liquidating account {}: {:?}", pubkey, err); - false - } - Ok(liquidated) => liquidated, - } -} diff --git a/liquidator/src/main.rs b/liquidator/src/main.rs index 5b8d8290e..e1d389f9e 100644 --- a/liquidator/src/main.rs +++ b/liquidator/src/main.rs @@ -228,6 +228,17 @@ async fn main() -> anyhow::Result<()> { refresh_timeout: Duration::from_secs(30), }; + let mut liquidation = LiquidationState { + mango_client: &mango_client, + account_fetcher: &account_fetcher, + liquidation_config: liq_config, + rebalance_config: rebalance_config.clone(), + accounts_with_errors: Default::default(), + error_skip_threshold: 5, + error_skip_duration: std::time::Duration::from_secs(120), + error_reset_duration: std::time::Duration::from_secs(360), + }; + info!("main loop"); loop { tokio::select! { @@ -255,12 +266,8 @@ async fn main() -> anyhow::Result<()> { continue; } - liquidate( - &mango_client, - &account_fetcher, + liquidation.maybe_liquidate_one_and_rebalance( std::iter::once(&account_write.pubkey), - &liq_config, - &rebalance_config, ).await?; } @@ -277,12 +284,8 @@ async fn main() -> anyhow::Result<()> { log::debug!("change to oracle {}", &account_write.pubkey); } - liquidate( - &mango_client, - &account_fetcher, + liquidation.maybe_liquidate_one_and_rebalance( mango_accounts.iter(), - &liq_config, - &rebalance_config, ).await?; } } @@ -310,12 +313,8 @@ async fn main() -> anyhow::Result<()> { snapshot_source::update_chain_data(&mut chain_data.write().unwrap(), message); one_snapshot_done = true; - liquidate( - &mango_client, - &account_fetcher, + liquidation.maybe_liquidate_one_and_rebalance( mango_accounts.iter(), - &liq_config, - &rebalance_config, ).await?; }, @@ -335,35 +334,121 @@ async fn main() -> anyhow::Result<()> { } } -async fn liquidate<'a>( - mango_client: &MangoClient, - account_fetcher: &chain_data::AccountFetcher, - accounts_iter: impl Iterator, - config: &liquidate::Config, - rebalance_config: &rebalance::Config, -) -> anyhow::Result<()> { - use rand::seq::SliceRandom; - let mut rng = rand::thread_rng(); +struct ErrorTracking { + count: u64, + last_at: std::time::Instant, +} - let mut accounts = accounts_iter.collect::>(); - accounts.shuffle(&mut rng); +struct LiquidationState<'a> { + mango_client: &'a MangoClient, + account_fetcher: &'a chain_data::AccountFetcher, + liquidation_config: liquidate::Config, + rebalance_config: rebalance::Config, + accounts_with_errors: HashMap, + error_skip_threshold: u64, + error_skip_duration: std::time::Duration, + error_reset_duration: std::time::Duration, +} - let mut liquidated_one = false; - for pubkey in accounts { - liquidated_one |= - liquidate::maybe_liquidate(mango_client, account_fetcher, pubkey, config).await; - } - if !liquidated_one { - return Ok(()); +impl<'a> LiquidationState<'a> { + async fn maybe_liquidate_one_and_rebalance<'b>( + &mut self, + accounts_iter: impl Iterator, + ) -> anyhow::Result<()> { + use rand::seq::SliceRandom; + let mut rng = rand::thread_rng(); + + let mut accounts = accounts_iter.collect::>(); + accounts.shuffle(&mut rng); + + let mut liquidated_one = false; + for pubkey in accounts { + if self + .maybe_liquidate_and_log_error(pubkey) + .await + .unwrap_or(false) + { + liquidated_one = true; + break; + } + } + if !liquidated_one { + return Ok(()); + } + + let liqor = &self.mango_client.mango_account_address; + if let Err(err) = rebalance::zero_all_non_quote( + self.mango_client, + self.account_fetcher, + liqor, + &self.rebalance_config, + ) + .await + { + log::error!("failed to rebalance liqor: {:?}", err); + } + Ok(()) } - let liqor = &mango_client.mango_account_address; - if let Err(err) = - rebalance::zero_all_non_quote(mango_client, account_fetcher, liqor, rebalance_config).await - { - log::error!("failed to rebalance liqor: {:?}", err); + async fn maybe_liquidate_and_log_error(&mut self, pubkey: &Pubkey) -> anyhow::Result { + let now = std::time::Instant::now(); + + // Skip a pubkey if there've been too many errors recently + if let Some(error_entry) = self.accounts_with_errors.get(pubkey) { + if error_entry.count >= self.error_skip_threshold + && now.duration_since(error_entry.last_at) < self.error_skip_duration + { + log::trace!( + "skip checking account {pubkey}, had {} errors recently", + error_entry.count + ); + return Ok(false); + } + } + + let result = liquidate::maybe_liquidate_account( + self.mango_client, + self.account_fetcher, + pubkey, + &self.liquidation_config, + ) + .await; + + if let Err(err) = result.as_ref() { + // Keep track of pubkeys that had errors + let error_entry = self + .accounts_with_errors + .entry(*pubkey) + .or_insert(ErrorTracking { + count: 0, + last_at: now, + }); + if now.duration_since(error_entry.last_at) > self.error_reset_duration { + error_entry.count = 0; + } + error_entry.count += 1; + error_entry.last_at = now; + + // Not all errors need to be raised to the user's attention. + let mut log_level = log::Level::Error; + + // Simulation errors due to liqee precondition failures on the liquidation instructions + // will commonly happen if our liquidator is late or if there are chain forks. + match err.downcast_ref::() { + Some(client::MangoClientError::SendTransactionPreflightFailure { logs }) => { + if logs.contains("HealthMustBeNegative") || logs.contains("IsNotBankrupt") { + log_level = log::Level::Trace; + } + } + _ => {} + }; + log::log!(log_level, "liquidating account {}: {:?}", pubkey, err); + } else { + self.accounts_with_errors.remove(pubkey); + } + + result } - Ok(()) } fn start_chain_data_metrics(chain: Arc>, metrics: &metrics::Metrics) { diff --git a/liquidator/src/rebalance.rs b/liquidator/src/rebalance.rs index c289e4d7b..8ec837658 100644 --- a/liquidator/src/rebalance.rs +++ b/liquidator/src/rebalance.rs @@ -9,6 +9,7 @@ use {fixed::types::I80F48, solana_sdk::pubkey::Pubkey}; use futures::{stream, StreamExt, TryStreamExt}; use std::{collections::HashMap, time::Duration}; +#[derive(Clone)] pub struct Config { /// Maximum slippage allowed in Jupiter pub slippage: f64, From 770b5a80560f8bf99bb570265e20c706a8c0ae44 Mon Sep 17 00:00:00 2001 From: microwavedcola1 <89031858+microwavedcola1@users.noreply.github.com> Date: Wed, 21 Dec 2022 10:21:10 +0100 Subject: [PATCH 5/5] pop perp events without processing, when mango account is closed (#351) * pop perp events without processing, when mango account is closed Signed-off-by: microwavedcola1 * fix Signed-off-by: microwavedcola1 Signed-off-by: microwavedcola1 --- .../src/instructions/perp_consume_events.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/programs/mango-v4/src/instructions/perp_consume_events.rs b/programs/mango-v4/src/instructions/perp_consume_events.rs index a7dc48949..68fcbc9d8 100644 --- a/programs/mango-v4/src/instructions/perp_consume_events.rs +++ b/programs/mango-v4/src/instructions/perp_consume_events.rs @@ -22,6 +22,8 @@ pub struct PerpConsumeEvents<'info> { } pub fn perp_consume_events(ctx: Context, limit: usize) -> Result<()> { + let group = ctx.accounts.group.load()?; + let limit = std::cmp::min(limit, 8); let mut perp_market = ctx.accounts.perp_market.load_mut()?; @@ -47,6 +49,12 @@ pub fn perp_consume_events(ctx: Context, limit: usize) -> Res } Some(ai) => { + if group.is_testing() && ai.owner != &crate::id() { + msg!("Mango account (maker+taker) not owned by mango program"); + event_queue.pop_front()?; + continue; + } + let mal: AccountLoaderDynamic = AccountLoaderDynamic::try_from(ai)?; let mut ma = mal.load_mut()?; @@ -76,6 +84,12 @@ pub fn perp_consume_events(ctx: Context, limit: usize) -> Res return Ok(()); } Some(ai) => { + if group.is_testing() && ai.owner != &crate::id() { + msg!("Mango account (maker) not owned by mango program"); + event_queue.pop_front()?; + continue; + } + let mal: AccountLoaderDynamic = AccountLoaderDynamic::try_from(ai)?; let mut maker = mal.load_mut()?; @@ -86,6 +100,12 @@ pub fn perp_consume_events(ctx: Context, limit: usize) -> Res return Ok(()); } Some(ai) => { + if group.is_testing() && ai.owner != &crate::id() { + msg!("Mango account (taker) not owned by mango program"); + event_queue.pop_front()?; + continue; + } + let mal: AccountLoaderDynamic = AccountLoaderDynamic::try_from(ai)?; let mut taker = mal.load_mut()?; @@ -148,6 +168,12 @@ pub fn perp_consume_events(ctx: Context, limit: usize) -> Res return Ok(()); } Some(ai) => { + if group.is_testing() && ai.owner != &crate::id() { + msg!("Mango account (event owner) not owned by mango program"); + event_queue.pop_front()?; + continue; + } + let mal: AccountLoaderDynamic = AccountLoaderDynamic::try_from(ai)?; let mut ma = mal.load_mut()?;