From baa980c65952bac9d64a13a2fa847191c244e6d9 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 15 Mar 2022 14:44:47 +0100 Subject: [PATCH] PlaceSerumOrder: Track vault balances before and after --- .../instructions/create_serum_open_orders.rs | 12 ++ .../src/instructions/place_serum_order.rs | 138 ++++++++++++------ programs/mango-v4/src/state/bank.rs | 14 +- programs/mango-v4/src/state/mango_account.rs | 16 ++ .../tests/program_test/mango_client.rs | 13 ++ programs/mango-v4/tests/test_basic.rs | 22 ++- .../mango-v4/tests/test_position_lifetime.rs | 6 +- programs/mango-v4/tests/test_serum.rs | 21 ++- 8 files changed, 170 insertions(+), 72 deletions(-) diff --git a/programs/mango-v4/src/instructions/create_serum_open_orders.rs b/programs/mango-v4/src/instructions/create_serum_open_orders.rs index c4b367782..ec7703888 100644 --- a/programs/mango-v4/src/instructions/create_serum_open_orders.rs +++ b/programs/mango-v4/src/instructions/create_serum_open_orders.rs @@ -75,5 +75,17 @@ pub fn create_serum_open_orders(ctx: Context) -> Result<( .create(serum_market.market_index)?; oos.open_orders = ctx.accounts.open_orders.key(); + // Make it so that the indexed_positions for the base and quote currency + // stay permanently blocked. Otherwise users may end up in situations where + // they can't settle a market because they don't have free indexed_positions! + let (quote_position, _) = account + .indexed_positions + .get_mut_or_create(serum_market.quote_token_index)?; + quote_position.in_use_count += 1; + let (base_position, _) = account + .indexed_positions + .get_mut_or_create(serum_market.base_token_index)?; + base_position.in_use_count += 1; + Ok(()) } diff --git a/programs/mango-v4/src/instructions/place_serum_order.rs b/programs/mango-v4/src/instructions/place_serum_order.rs index 35a58d8fa..552c5ac0d 100644 --- a/programs/mango-v4/src/instructions/place_serum_order.rs +++ b/programs/mango-v4/src/instructions/place_serum_order.rs @@ -1,14 +1,16 @@ use anchor_lang::prelude::*; -use anchor_spl::dex; use anchor_spl::token::{Token, TokenAccount}; use arrayref::array_refs; use borsh::{BorshDeserialize, BorshSerialize}; -use dex::serum_dex; use num_enum::TryFromPrimitive; -use serum_dex::matching::Side; use std::io::Write; use std::num::NonZeroU64; +use anchor_spl::dex; +use dex::serum_dex; +use serum_dex::instruction::NewOrderInstructionV3; +use serum_dex::matching::Side; + use crate::error::*; use crate::state::*; @@ -163,50 +165,101 @@ pub fn place_serum_order( ctx: Context, order: NewOrderInstructionData, ) -> Result<()> { - let account = ctx.accounts.account.load()?; - let serum_market = ctx.accounts.serum_market.load()?; + // + // Validation + // + { + let account = ctx.accounts.account.load()?; + let serum_market = ctx.accounts.serum_market.load()?; - // Validate open_orders - require!( - account - .serum_open_orders_map - .find(serum_market.market_index) - .ok_or(error!(MangoError::SomeError))? - .open_orders - == ctx.accounts.open_orders.key(), - MangoError::SomeError - ); + // Validate open_orders + require!( + account + .serum_open_orders_map + .find(serum_market.market_index) + .ok_or(error!(MangoError::SomeError))? + .open_orders + == ctx.accounts.open_orders.key(), + MangoError::SomeError + ); - // Validate banks and vaults - let quote_bank = ctx.accounts.quote_bank.load()?; - require!( - quote_bank.vault == ctx.accounts.quote_vault.key(), - MangoError::SomeError - ); - require!( - quote_bank.token_index == serum_market.quote_token_index, - MangoError::SomeError - ); - let base_bank = ctx.accounts.base_bank.load()?; - require!( - base_bank.vault == ctx.accounts.base_vault.key(), - MangoError::SomeError - ); - require!( - base_bank.token_index == serum_market.base_token_index, - MangoError::SomeError - ); + // Validate banks and vaults + let quote_bank = ctx.accounts.quote_bank.load()?; + require!( + quote_bank.vault == ctx.accounts.quote_vault.key(), + MangoError::SomeError + ); + require!( + quote_bank.token_index == serum_market.quote_token_index, + MangoError::SomeError + ); + let base_bank = ctx.accounts.base_bank.load()?; + require!( + base_bank.vault == ctx.accounts.base_vault.key(), + MangoError::SomeError + ); + require!( + base_bank.token_index == serum_market.base_token_index, + MangoError::SomeError + ); + } + + // + // Before-order tracking + // + + let before_base_vault = ctx.accounts.base_vault.amount; + let before_quote_vault = ctx.accounts.quote_vault.amount; // TODO: pre-health check - // TODO: track vault balance before // // Place the order // + cpi_place_order(&ctx, &order.0)?; - // unwrap our newtype - let order = order.0; + // TODO: immediately call settle_funds? + // + // After-order tracking + // + ctx.accounts.base_vault.reload()?; + ctx.accounts.quote_vault.reload()?; + let after_base_vault = ctx.accounts.base_vault.amount; + let after_quote_vault = ctx.accounts.quote_vault.amount; + + // Charge the difference in vault balances to the user's account + { + let mut account = ctx.accounts.account.load_mut()?; + + let mut base_bank = ctx.accounts.base_bank.load_mut()?; + let (base_position, _) = account + .indexed_positions + .get_mut_or_create(base_bank.token_index)?; + base_bank.change(base_position, (after_base_vault - before_base_vault) as i64)?; + + let mut quote_bank = ctx.accounts.quote_bank.load_mut()?; + let (quote_position, _) = account + .indexed_positions + .get_mut_or_create(quote_bank.token_index)?; + quote_bank.change( + quote_position, + (after_quote_vault - before_quote_vault) as i64, + )?; + } + + // + // Health check + // + let account = ctx.accounts.account.load()?; + let health = compute_health(&account, &ctx.remaining_accounts)?; + msg!("health: {}", health); + require!(health >= 0, MangoError::SomeError); + + Ok(()) +} + +fn cpi_place_order(ctx: &Context, order: &NewOrderInstructionV3) -> Result<()> { let order_payer_token_account = match order.side { Side::Bid => ctx.accounts.quote_vault.to_account_info(), Side::Ask => ctx.accounts.base_vault.to_account_info(), @@ -248,16 +301,5 @@ pub fn place_serum_order( order.limit, )?; - // TODO: immediately call settle_funds? - // TODO: track vault balance after, apply to user position - - // - // Health check - // - let account = ctx.accounts.account.load()?; - let health = compute_health(&account, &ctx.remaining_accounts)?; - msg!("health: {}", health); - require!(health >= 0, MangoError::SomeError); - Ok(()) } diff --git a/programs/mango-v4/src/state/bank.rs b/programs/mango-v4/src/state/bank.rs index 1b1880034..b38887a19 100644 --- a/programs/mango-v4/src/state/bank.rs +++ b/programs/mango-v4/src/state/bank.rs @@ -57,7 +57,7 @@ impl Bank { self.indexed_total_borrows = cm!(self.indexed_total_borrows - indexed_change); position.indexed_value = cm!(position.indexed_value + indexed_change); return Ok(true); - } else if new_native_position < I80F48::ONE { + } else if new_native_position < I80F48::ONE && !position.is_in_use() { // if there's less than one token deposited, zero the position self.dust = cm!(self.dust + new_native_position); self.indexed_total_borrows = @@ -93,7 +93,7 @@ impl Bank { let new_native_position = cm!(native_position - native_amount); if !new_native_position.is_negative() { // withdraw deposits only - if new_native_position < I80F48::ONE { + if new_native_position < I80F48::ONE && !position.is_in_use() { // zero the account collecting the leftovers in `dust` self.dust = cm!(self.dust + new_native_position); self.indexed_total_deposits = @@ -101,7 +101,7 @@ impl Bank { position.indexed_value = I80F48::ZERO; return Ok(false); } else { - // withdraw some deposits leaving >1 native token + // withdraw some deposits leaving a positive balance let indexed_change = cm!(native_amount / self.deposit_index); self.indexed_total_deposits = cm!(self.indexed_total_deposits - indexed_change); position.indexed_value = cm!(position.indexed_value - indexed_change); @@ -123,4 +123,12 @@ impl Bank { Ok(true) } + + pub fn change(&mut self, position: &mut IndexedPosition, native_amount: i64) -> Result { + if native_amount >= 0 { + self.deposit(position, native_amount as u64) + } else { + self.withdraw(position, (-native_amount) as u64) + } + } } diff --git a/programs/mango-v4/src/state/mango_account.rs b/programs/mango-v4/src/state/mango_account.rs index 2857da567..a9db7a71a 100644 --- a/programs/mango-v4/src/state/mango_account.rs +++ b/programs/mango-v4/src/state/mango_account.rs @@ -19,6 +19,9 @@ pub struct IndexedPosition { /// index into Group.tokens pub token_index: TokenIndex, + + /// incremented when a market requires this position to stay alive + pub in_use_count: u8, } // TODO: static assert the size and alignment @@ -38,6 +41,10 @@ impl IndexedPosition { self.indexed_value * bank.borrow_index } } + + pub fn is_in_use(&self) -> bool { + self.in_use_count > 0 + } } #[zero_copy] @@ -51,6 +58,7 @@ impl IndexedPositions { values: [IndexedPosition { indexed_value: I80F48::ZERO, token_index: TokenIndex::MAX, + in_use_count: 0, }; MAX_INDEXED_POSITIONS], } } @@ -79,6 +87,7 @@ impl IndexedPositions { self.values[i] = IndexedPosition { indexed_value: I80F48::ZERO, token_index: token_index, + in_use_count: 0, }; } } @@ -90,12 +99,19 @@ impl IndexedPositions { } pub fn deactivate(&mut self, index: usize) { + assert!(self.values[index].in_use_count == 0); self.values[index].token_index = TokenIndex::MAX; } pub fn iter_active(&self) -> impl Iterator { self.values.iter().filter(|p| p.is_active()) } + + pub fn find(&self, token_index: TokenIndex) -> Option<&IndexedPosition> { + self.values + .iter() + .find(|p| p.is_active_for_token(token_index)) + } } #[zero_copy] diff --git a/programs/mango-v4/tests/program_test/mango_client.rs b/programs/mango-v4/tests/program_test/mango_client.rs index 89e07d1b2..805525664 100644 --- a/programs/mango-v4/tests/program_test/mango_client.rs +++ b/programs/mango-v4/tests/program_test/mango_client.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] + use anchor_lang::prelude::*; use anchor_lang::solana_program::sysvar::{self, SysvarId}; use anchor_spl::dex::serum_dex; @@ -153,6 +155,17 @@ fn from_serum_style_pubkey(d: &[u64; 4]) -> Pubkey { Pubkey::new(bytemuck::cast_slice(d as &[_])) } +pub async fn account_position(solana: &SolanaCookie, account: Pubkey, bank: Pubkey) -> i64 { + let account_data: MangoAccount = solana.get_account(account).await; + let bank_data: Bank = solana.get_account(bank).await; + let native = account_data + .indexed_positions + .find(bank_data.token_index) + .unwrap() + .native(&bank_data); + native.round().to_num::() +} + // // a struct for each instruction along with its // ClientInstruction impl diff --git a/programs/mango-v4/tests/test_basic.rs b/programs/mango-v4/tests/test_basic.rs index e94673416..8fddd9dc0 100644 --- a/programs/mango-v4/tests/test_basic.rs +++ b/programs/mango-v4/tests/test_basic.rs @@ -114,13 +114,11 @@ async fn test_basic() -> Result<(), TransportError> { solana.token_account_balance(payer_mint0_account).await, start_balance - deposit_amount ); - let account_data: MangoAccount = solana.get_account(account).await; - let bank_data: Bank = solana.get_account(bank).await; - assert!( - account_data.indexed_positions.values[0].native(&bank_data) - - I80F48::from_num(deposit_amount) - < dust_threshold + assert_eq!( + account_position(solana, account, bank).await, + deposit_amount as i64 ); + let bank_data: Bank = solana.get_account(bank).await; assert!( bank_data.native_total_deposits() - I80F48::from_num(deposit_amount) < dust_threshold ); @@ -130,6 +128,7 @@ async fn test_basic() -> Result<(), TransportError> { // TEST: Withdraw funds // { + let start_amount = 100; let withdraw_amount = 50; let start_balance = solana.token_account_balance(payer_mint0_account).await; @@ -151,16 +150,15 @@ async fn test_basic() -> Result<(), TransportError> { solana.token_account_balance(payer_mint0_account).await, start_balance + withdraw_amount ); - let account_data: MangoAccount = solana.get_account(account).await; + assert_eq!( + account_position(solana, account, bank).await, + (start_amount - withdraw_amount) as i64 + ); let bank_data: Bank = solana.get_account(bank).await; assert!( - account_data.indexed_positions.values[0].native(&bank_data) - - I80F48::from_num(withdraw_amount) + bank_data.native_total_deposits() - I80F48::from_num(start_amount - withdraw_amount) < dust_threshold ); - assert!( - bank_data.native_total_deposits() - I80F48::from_num(withdraw_amount) < dust_threshold - ); } Ok(()) diff --git a/programs/mango-v4/tests/test_position_lifetime.rs b/programs/mango-v4/tests/test_position_lifetime.rs index 676ca555d..37f77f7b2 100644 --- a/programs/mango-v4/tests/test_position_lifetime.rs +++ b/programs/mango-v4/tests/test_position_lifetime.rs @@ -109,7 +109,7 @@ async fn test_position_lifetime() -> Result<()> { (oracle, bank) }; register_mint(0, mint0.clone()).await; - register_mint(1, mint1.clone()).await; + let (_oracle1, bank1) = register_mint(1, mint1.clone()).await; register_mint(2, mint2.clone()).await; // @@ -217,6 +217,10 @@ async fn test_position_lifetime() -> Result<()> { ) .await .unwrap(); + assert_eq!( + account_position(solana, account, bank1).await, + -(borrow_amount as i64) + ); // give it back, closing the position send_tx( diff --git a/programs/mango-v4/tests/test_serum.rs b/programs/mango-v4/tests/test_serum.rs index 2aee5ee13..37f2feefc 100644 --- a/programs/mango-v4/tests/test_serum.rs +++ b/programs/mango-v4/tests/test_serum.rs @@ -98,17 +98,17 @@ async fn test_serum() -> Result<(), TransportError> { let address_lookup_table = solana.create_address_lookup_table(admin, payer).await; let base_token_index = 0; - let (_oracle0, _bank0) = + let (_oracle0, bank0) = register_mint(base_token_index, mint0.clone(), address_lookup_table).await; let quote_token_index = 1; - let (_oracle1, _bank1) = + let (_oracle1, bank1) = register_mint(quote_token_index, mint1.clone(), address_lookup_table).await; // // SETUP: Deposit user funds // { - let deposit_amount = 100; + let deposit_amount = 1000; send_tx( solana, @@ -187,12 +187,12 @@ async fn test_serum() -> Result<(), TransportError> { send_tx( solana, PlaceSerumOrderInstruction { - side: 0, - limit_price: 1, - max_base_qty: 1, - max_native_quote_qty_including_fees: 1, + side: 0, // TODO: Bid + limit_price: 10, // in quote_lot (10) per base lot (100) + max_base_qty: 1, // in base lot (100) + max_native_quote_qty_including_fees: 100, self_trade_behavior: 0, - order_type: 0, + order_type: 0, // TODO: Limit client_order_id: 0, limit: 10, account, @@ -203,5 +203,10 @@ async fn test_serum() -> Result<(), TransportError> { .await .unwrap(); + let native0 = account_position(solana, account, bank0).await; + let native1 = account_position(solana, account, bank1).await; + assert_eq!(native0, 1000); + assert_eq!(native1, 900); + Ok(()) }