From 66c6926419799c01bcb9a8f9905a207ab6221c42 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Thu, 15 Jun 2023 10:43:31 +0200 Subject: [PATCH] Audit fixes for the fee buyback feature (#608) * reduce_buyback_fees_accrued: forbid reducing more than accrued * Buyback fees: Expire accrued during event processing --- .../account_buyback_fees_with_mngo.rs | 2 +- .../src/instructions/perp_consume_events.rs | 9 ++++++-- programs/mango-v4/src/state/mango_account.rs | 22 +++++++++++++------ programs/mango-v4/src/state/orderbook/mod.rs | 5 +++-- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/programs/mango-v4/src/instructions/account_buyback_fees_with_mngo.rs b/programs/mango-v4/src/instructions/account_buyback_fees_with_mngo.rs index ccc2df95f..b6ea14ab2 100644 --- a/programs/mango-v4/src/instructions/account_buyback_fees_with_mngo.rs +++ b/programs/mango-v4/src/instructions/account_buyback_fees_with_mngo.rs @@ -93,7 +93,7 @@ pub fn account_buyback_fees_with_mngo( let mut max_buyback_mngo = max_buyback_usd / mngo_buyback_price; // buyback is restricted to account's token position max_buyback_mngo = max_buyback_mngo.min(account_mngo); - max_buyback_usd = max_buyback_mngo * mngo_buyback_price; + max_buyback_usd = max_buyback_usd.min(max_buyback_mngo * mngo_buyback_price); let max_buyback_fees = max_buyback_usd / fees_liab_price; // move mngo from user to dao diff --git a/programs/mango-v4/src/instructions/perp_consume_events.rs b/programs/mango-v4/src/instructions/perp_consume_events.rs index e4dcc2f0f..670665cd5 100644 --- a/programs/mango-v4/src/instructions/perp_consume_events.rs +++ b/programs/mango-v4/src/instructions/perp_consume_events.rs @@ -74,7 +74,12 @@ pub fn perp_consume_events(ctx: Context, limit: usize) -> Res group, event_queue ); - maker_taker.execute_perp_maker(perp_market_index, &mut perp_market, fill)?; + maker_taker.execute_perp_maker( + perp_market_index, + &mut perp_market, + fill, + &group, + )?; maker_taker.execute_perp_taker(perp_market_index, &mut perp_market, fill)?; emit_perp_balances( group_key, @@ -86,7 +91,7 @@ pub fn perp_consume_events(ctx: Context, limit: usize) -> Res load_mango_account!(maker, fill.maker, mango_account_ais, group, event_queue); load_mango_account!(taker, fill.taker, mango_account_ais, group, event_queue); - maker.execute_perp_maker(perp_market_index, &mut perp_market, fill)?; + maker.execute_perp_maker(perp_market_index, &mut perp_market, fill, &group)?; taker.execute_perp_taker(perp_market_index, &mut perp_market, fill)?; emit_perp_balances( group_key, diff --git a/programs/mango-v4/src/state/mango_account.rs b/programs/mango-v4/src/state/mango_account.rs index 6b2a2b0ac..79f8aa4c6 100644 --- a/programs/mango-v4/src/state/mango_account.rs +++ b/programs/mango-v4/src/state/mango_account.rs @@ -14,7 +14,6 @@ use crate::error::*; use crate::health::{HealthCache, HealthType}; use crate::logs::{DeactivatePerpPositionLog, DeactivateTokenPositionLog}; -use super::dynamic_account::*; use super::BookSideOrderTree; use super::FillEvent; use super::LeafNode; @@ -24,6 +23,7 @@ use super::PerpOpenOrder; use super::Serum3MarketIndex; use super::TokenIndex; use super::FREE_ORDER_SLOT; +use super::{dynamic_account::*, Group}; use super::{PerpPosition, Serum3Orders, TokenPosition}; use super::{Side, SideAndOrderTree}; @@ -300,17 +300,22 @@ impl MangoAccountFixed { } /// Add new fees that are usable with the buyback fees feature. + /// + /// Any call to this should be preceeded by a call to expire_buyback_fees earlier + /// in the same instruction. pub fn accrue_buyback_fees(&mut self, amount: u64) { self.buyback_fees_accrued_current = self.buyback_fees_accrued_current.saturating_add(amount); } /// Reduce the available buyback fees amount because it was used up. + /// + /// Panics if `amount` exceeds the available accrued amount pub fn reduce_buyback_fees_accrued(&mut self, amount: u64) { if amount > self.buyback_fees_accrued_previous { - self.buyback_fees_accrued_current = self - .buyback_fees_accrued_current - .saturating_sub(amount - self.buyback_fees_accrued_previous); + let remaining_amount = amount - self.buyback_fees_accrued_previous; + assert!(remaining_amount <= self.buyback_fees_accrued_current); + self.buyback_fees_accrued_current -= remaining_amount; self.buyback_fees_accrued_previous = 0; } else { self.buyback_fees_accrued_previous -= amount; @@ -948,14 +953,17 @@ impl< perp_market_index: PerpMarketIndex, perp_market: &mut PerpMarket, fill: &FillEvent, + group: &Group, ) -> Result<()> { let side = fill.taker_side().invert_side(); let (base_change, quote_change) = fill.base_quote_change(side); let quote = I80F48::from(perp_market.quote_lot_size) * I80F48::from(quote_change); let fees = quote.abs() * I80F48::from_num(fill.maker_fee); if fees.is_positive() { - self.fixed_mut() - .accrue_buyback_fees(fees.floor().to_num::()); + let f = self.fixed_mut(); + let now_ts = Clock::get().unwrap().unix_timestamp.try_into().unwrap(); + f.expire_buyback_fees(now_ts, group.buyback_fees_expiry_interval); + f.accrue_buyback_fees(fees.floor().to_num::()); } let pa = self.perp_position_mut(perp_market_index)?; pa.settle_funding(perp_market); @@ -1568,7 +1576,7 @@ mod tests { assert_eq!(fixed.buyback_fees_expiry_timestamp, 1070); assert_eq!(fixed.buyback_fees_accrued(), 12); - fixed.reduce_buyback_fees_accrued(100); + fixed.reduce_buyback_fees_accrued(12); assert_eq!(fixed.buyback_fees_accrued(), 0); } } diff --git a/programs/mango-v4/src/state/orderbook/mod.rs b/programs/mango-v4/src/state/orderbook/mod.rs index ea4c6fb0e..232eed2f3 100644 --- a/programs/mango-v4/src/state/orderbook/mod.rs +++ b/programs/mango-v4/src/state/orderbook/mod.rs @@ -21,7 +21,7 @@ mod queue; #[cfg(test)] mod tests { use super::*; - use crate::state::{MangoAccount, MangoAccountValue, PerpMarket, FREE_ORDER_SLOT}; + use crate::state::{Group, MangoAccount, MangoAccountValue, PerpMarket, FREE_ORDER_SLOT}; use anchor_lang::prelude::*; use bytemuck::Zeroable; use fixed::types::I80F48; @@ -238,6 +238,7 @@ mod tests { #[test] fn book_new_order() { + let group = Group::zeroed(); let (mut market, oracle_price, mut event_queue, book_accs) = test_setup(1000.0); let mut book = book_accs.orderbook(); let settle_token_index = 0; @@ -396,7 +397,7 @@ mod tests { // simulate event queue processing maker - .execute_perp_maker(market.perp_market_index, &mut market, fill) + .execute_perp_maker(market.perp_market_index, &mut market, fill, &group) .unwrap(); taker .execute_perp_taker(market.perp_market_index, &mut market, fill)