diff --git a/programs/mango-v4/src/instructions/create_account.rs b/programs/mango-v4/src/instructions/create_account.rs index 82f0bb9df..4a69725e8 100644 --- a/programs/mango-v4/src/instructions/create_account.rs +++ b/programs/mango-v4/src/instructions/create_account.rs @@ -31,6 +31,7 @@ pub fn create_account(ctx: Context, account_num: u8) -> Result<() account.group = ctx.accounts.group.key(); account.owner = ctx.accounts.owner.key(); account.account_num = account_num; + account.indexed_positions = IndexedPositions::new(); account.bump = *ctx.bumps.get("account").ok_or(MangoError::SomeError)?; Ok(()) diff --git a/programs/mango-v4/src/instructions/deposit.rs b/programs/mango-v4/src/instructions/deposit.rs index 90737ff02..765f60f6f 100644 --- a/programs/mango-v4/src/instructions/deposit.rs +++ b/programs/mango-v4/src/instructions/deposit.rs @@ -56,14 +56,18 @@ pub fn deposit(ctx: Context, amount: u64) -> Result<()> { // Get the account's position for that token index let mut account = ctx.accounts.account.load_mut()?; - let position = account.indexed_positions.get_mut_or_create(token_index)?; + let (position, position_index) = account.indexed_positions.get_mut_or_create(token_index)?; // Update the bank and position let mut bank = ctx.accounts.bank.load_mut()?; - bank.deposit(position, amount); + let position_is_active = bank.deposit(position, amount); // Transfer the actual tokens token::transfer(ctx.accounts.transfer_ctx(), amount)?; + if !position_is_active { + account.indexed_positions.deactivate(position_index); + } + Ok(()) } diff --git a/programs/mango-v4/src/instructions/register_token.rs b/programs/mango-v4/src/instructions/register_token.rs index 6c49d85d1..1ca068f38 100644 --- a/programs/mango-v4/src/instructions/register_token.rs +++ b/programs/mango-v4/src/instructions/register_token.rs @@ -112,6 +112,7 @@ pub fn register_token( init_asset_weight: I80F48::from_num(init_asset_weight), maint_liab_weight: I80F48::from_num(maint_liab_weight), init_liab_weight: I80F48::from_num(init_liab_weight), + dust: I80F48::ZERO, token_index: token_index as TokenIndex, }; diff --git a/programs/mango-v4/src/instructions/withdraw.rs b/programs/mango-v4/src/instructions/withdraw.rs index 454fa8e50..164c2837b 100644 --- a/programs/mango-v4/src/instructions/withdraw.rs +++ b/programs/mango-v4/src/instructions/withdraw.rs @@ -58,11 +58,11 @@ pub fn withdraw(ctx: Context, amount: u64, allow_borrow: bool) -> Resu // Get the account's position for that token index let mut account = ctx.accounts.account.load_mut()?; - let position = account.indexed_positions.get_mut_or_create(token_index)?; + let (position, position_index) = account.indexed_positions.get_mut_or_create(token_index)?; // The bank will also be passed in remainingAccounts. Use an explicit scope // to drop the &mut before we borrow it immutably again later. - { + let position_is_active = { let mut bank = ctx.accounts.bank.load_mut()?; let native_position = position.native(&bank); @@ -85,7 +85,7 @@ pub fn withdraw(ctx: Context, amount: u64, allow_borrow: bool) -> Resu ); // Update the bank and position - bank.withdraw(position, amount); + let position_is_active = bank.withdraw(position, amount); // Transfer the actual tokens let group_seeds = group_seeds!(group); @@ -93,10 +93,12 @@ pub fn withdraw(ctx: Context, amount: u64, allow_borrow: bool) -> Resu ctx.accounts.transfer_ctx().with_signer(&[group_seeds]), amount, )?; - } + + position_is_active + }; // - // Health check (WIP) + // Health check // let active_len = account.indexed_positions.iter_active().count(); require!( @@ -109,7 +111,16 @@ pub fn withdraw(ctx: Context, amount: u64, allow_borrow: bool) -> Resu let health = compute_health(&mut account, &banks, &oracles)?; msg!("health: {}", health); - require!(health > 0, MangoError::SomeError); + require!(health >= 0, MangoError::SomeError); + + // + // Deactivate the position only after the health check because the user passed in + // remaining_accounts for all banks/oracles, including the account that will now be + // deactivated. + // + if !position_is_active { + account.indexed_positions.deactivate(position_index); + } Ok(()) } diff --git a/programs/mango-v4/src/state/mango_account.rs b/programs/mango-v4/src/state/mango_account.rs index 0a01be19c..add2f4c7a 100644 --- a/programs/mango-v4/src/state/mango_account.rs +++ b/programs/mango-v4/src/state/mango_account.rs @@ -23,13 +23,11 @@ pub struct IndexedPosition { impl IndexedPosition { pub fn is_active(&self) -> bool { - // maybe want to reserve token_index == 0? - // TODO: possibly consider inactive if there's less than one native token there? - that's impossible to withdraw... - self.indexed_value != I80F48::ZERO + self.token_index != TokenIndex::MAX } - pub fn is_active_for_index(&self, index: usize) -> bool { - self.token_index as usize == index && self.is_active() + pub fn is_active_for_token(&self, token_index: usize) -> bool { + self.token_index as usize == token_index } pub fn native(&self, bank: &TokenBank) -> I80F48 { @@ -47,21 +45,33 @@ pub struct IndexedPositions { } impl IndexedPositions { + pub fn new() -> Self { + Self { + values: [IndexedPosition { + indexed_value: I80F48::ZERO, + token_index: TokenIndex::MAX, + }; MAX_INDEXED_POSITIONS], + } + } + pub fn get_mut(&mut self, token_index: usize) -> Result<&mut IndexedPosition> { self.values .iter_mut() - .find(|p| p.is_active_for_index(token_index)) + .find(|p| p.is_active_for_token(token_index)) .ok_or_else(|| error!(MangoError::SomeError)) // TODO: not found error } - pub fn get_mut_or_create(&mut self, token_index: usize) -> Result<&mut IndexedPosition> { + pub fn get_mut_or_create( + &mut self, + token_index: usize, + ) -> Result<(&mut IndexedPosition, usize)> { // This function looks complex because of lifetimes. // Maybe there's a smart way to write it with double iter_mut() // that doesn't confuse the borrow checker. let mut pos = self .values .iter() - .position(|p| p.is_active_for_index(token_index)); + .position(|p| p.is_active_for_token(token_index)); if pos.is_none() { pos = self.values.iter().position(|p| !p.is_active()); if let Some(i) = pos { @@ -72,12 +82,16 @@ impl IndexedPositions { } } if let Some(i) = pos { - Ok(&mut self.values[i]) + Ok((&mut self.values[i], i)) } else { err!(MangoError::SomeError) // TODO: No free space } } + pub fn deactivate(&mut self, index: usize) { + self.values[index].token_index = TokenIndex::MAX; + } + pub fn iter_active(&self) -> impl Iterator { self.values.iter().filter(|p| p.is_active()) } diff --git a/programs/mango-v4/src/state/token_bank.rs b/programs/mango-v4/src/state/token_bank.rs index a871cb623..bd87be256 100644 --- a/programs/mango-v4/src/state/token_bank.rs +++ b/programs/mango-v4/src/state/token_bank.rs @@ -31,6 +31,9 @@ pub struct TokenBank { pub maint_liab_weight: I80F48, pub init_liab_weight: I80F48, + // Collection of all fractions-of-native-tokens that got rounded away + pub dust: I80F48, + // Index into TokenInfo on the group pub token_index: TokenIndex, } @@ -40,53 +43,81 @@ impl TokenBank { self.deposit_index * self.indexed_total_deposits } - pub fn deposit(&mut self, position: &mut IndexedPosition, native_amount: u64) { + /// Returns whether the position is active + pub fn deposit(&mut self, position: &mut IndexedPosition, native_amount: u64) -> bool { let mut native_amount = I80F48::from_num(native_amount); let native_position = position.native(self); if native_position.is_negative() { - if -native_position >= native_amount { - // pay back borrows only - let indexed_change = native_amount / self.borrow_index; + let new_native_position = native_position + native_amount; + if new_native_position.is_negative() { + // pay back borrows only, leaving a negative position + let indexed_change = native_amount / self.borrow_index + I80F48::DELTA; self.indexed_total_borrows -= indexed_change; position.indexed_value += indexed_change; - return; + return true; + } else if new_native_position < I80F48::ONE { + // if there's less than one token deposited, zero the position + self.dust += new_native_position; + self.indexed_total_borrows += position.indexed_value; + position.indexed_value = I80F48::ZERO; + return false; } - // pay back all borrows first + // pay back all borrows self.indexed_total_borrows += position.indexed_value; // position.value is negative position.indexed_value = I80F48::ZERO; + // deposit the rest native_amount += native_position; } // add to deposits - let indexed_change = native_amount / self.deposit_index; + // Adding DELTA to amount/index helps because (amount/index)*index <= amount, but + // we want to ensure that users can withdraw the same amount they have deposited, so + // (amount/index + delta)*index >= amount is a better guarantee. + let indexed_change = native_amount / self.deposit_index + I80F48::DELTA; self.indexed_total_deposits += indexed_change; position.indexed_value += indexed_change; + + true } - pub fn withdraw(&mut self, position: &mut IndexedPosition, native_amount: u64) { + /// Returns whether the position is active + pub fn withdraw(&mut self, position: &mut IndexedPosition, native_amount: u64) -> bool { let mut native_amount = I80F48::from_num(native_amount); let native_position = position.native(self); if native_position.is_positive() { - if native_position >= native_amount { + let new_native_position = native_position - native_amount; + if !new_native_position.is_negative() { // withdraw deposits only - let indexed_change = native_amount / self.deposit_index; - self.indexed_total_deposits -= indexed_change; - position.indexed_value -= indexed_change; - return; + if new_native_position < I80F48::ONE { + // zero the account collecting the leftovers in `dust` + self.dust += new_native_position; + self.indexed_total_deposits -= position.indexed_value; + position.indexed_value = I80F48::ZERO; + return false; + } else { + // withdraw some deposits leaving >1 native token + let indexed_change = native_amount / self.deposit_index; + self.indexed_total_deposits -= indexed_change; + position.indexed_value -= indexed_change; + return true; + } } - // withdraw all deposits first + // withdraw all deposits self.indexed_total_deposits -= position.indexed_value; position.indexed_value = I80F48::ZERO; - native_amount -= native_position; + // borrow the rest + native_amount = -new_native_position; } // add to borrows let indexed_change = native_amount / self.borrow_index; self.indexed_total_borrows += indexed_change; position.indexed_value -= indexed_change; + + true } } diff --git a/programs/mango-v4/tests/program_test/mango_client.rs b/programs/mango-v4/tests/program_test/mango_client.rs index 45fb29cbe..4011991d3 100644 --- a/programs/mango-v4/tests/program_test/mango_client.rs +++ b/programs/mango-v4/tests/program_test/mango_client.rs @@ -116,11 +116,8 @@ impl<'keypair> ClientInstruction for WithdrawInstruction<'keypair> { ) .0; - // figure out all the banks/oracles that need to be passed for the health check - let mut banks = vec![]; - let mut oracles = vec![]; - for position in account.indexed_positions.iter_active() { - let mint_pk = group.tokens.infos[position.token_index as usize].mint; + let account_loader = &account_loader; + let get_mint_info = move |mint_pk: Pubkey| async move { let mint_info_pk = Pubkey::find_program_address( &[ account.group.as_ref(), @@ -131,6 +128,15 @@ impl<'keypair> ClientInstruction for WithdrawInstruction<'keypair> { ) .0; let mint_info: MintInfo = account_loader.load(&mint_info_pk).await.unwrap(); + mint_info + }; + + // figure out all the banks/oracles that need to be passed for the health check + let mut banks = vec![]; + let mut oracles = vec![]; + for position in account.indexed_positions.iter_active() { + let mint_pk = group.tokens.infos[position.token_index as usize].mint; + let mint_info = get_mint_info(mint_pk).await; let lookup_table = account_loader .load_bytes(&mint_info.address_lookup_table) .await @@ -139,6 +145,19 @@ impl<'keypair> ClientInstruction for WithdrawInstruction<'keypair> { banks.push(addresses[mint_info.address_lookup_table_bank_index as usize]); oracles.push(addresses[mint_info.address_lookup_table_oracle_index as usize]); } + if banks.iter().find(|&&v| v == bank).is_none() { + // If there is not yet an active position for the token, we need to pass + // the bank/oracle for health check anyway. + let new_position = account + .indexed_positions + .values + .iter() + .position(|p| !p.is_active()) + .unwrap(); + let mint_info = get_mint_info(token_account.mint).await; + banks.insert(new_position, bank); + oracles.insert(new_position, mint_info.oracle); + } let accounts = Self::Accounts { group: account.group, diff --git a/programs/mango-v4/tests/test_position_lifetime.rs b/programs/mango-v4/tests/test_position_lifetime.rs new file mode 100644 index 000000000..d51db08bf --- /dev/null +++ b/programs/mango-v4/tests/test_position_lifetime.rs @@ -0,0 +1,261 @@ +#![cfg(feature = "test-bpf")] + +use anchor_lang::prelude::*; +use solana_program_test::*; +use solana_sdk::signature::Keypair; + +use mango_v4::state::*; +use program_test::*; + +mod program_test; + +// Check opening and closing positions +#[tokio::test] +async fn test_position_lifetime() -> Result<()> { + let context = TestContext::new().await; + let solana = &context.solana.clone(); + + let admin = &Keypair::new(); + let owner = &context.users[0].key; + let payer = &context.users[1].key; + let mint0 = &context.mints[0]; + let mint1 = &context.mints[1]; + let mint2 = &context.mints[2]; + + let payer_mint_accounts = &context.users[1].token_accounts[0..=2]; + + // + // SETUP: Create a group and accounts + // + + let group = send_tx(solana, CreateGroupInstruction { admin, payer }) + .await + .unwrap() + .group; + + let account = send_tx( + solana, + CreateAccountInstruction { + account_num: 0, + group, + owner, + payer, + }, + ) + .await + .unwrap() + .account; + + let funding_account = send_tx( + solana, + CreateAccountInstruction { + account_num: 1, + group, + owner, + payer, + }, + ) + .await + .unwrap() + .account; + + // + // SETUP: Register three mints (and make oracles for them) + // + + let address_lookup_table = solana.create_address_lookup_table(admin, payer).await; + + let register_mint = |mint: MintCookie| async move { + let create_stub_oracle_accounts = send_tx( + solana, + CreateStubOracle { + mint: mint.pubkey, + payer, + }, + ) + .await + .unwrap(); + let oracle = create_stub_oracle_accounts.oracle; + send_tx( + solana, + SetStubOracle { + mint: mint.pubkey, + payer, + price: "1.0", + }, + ) + .await + .unwrap(); + let register_token_accounts = send_tx( + solana, + RegisterTokenInstruction { + decimals: mint.decimals, + maint_asset_weight: 0.9, + init_asset_weight: 0.8, + maint_liab_weight: 1.1, + init_liab_weight: 1.2, + group, + admin, + mint: mint.pubkey, + address_lookup_table, + payer, + }, + ) + .await + .unwrap(); + let bank = register_token_accounts.bank; + + (oracle, bank) + }; + register_mint(mint0.clone()).await; + register_mint(mint1.clone()).await; + register_mint(mint2.clone()).await; + + // + // SETUP: Put some tokens into the funding account to allow borrowing + // + { + let funding_amount = 1000000; + for &payer_token in payer_mint_accounts { + send_tx( + solana, + DepositInstruction { + amount: funding_amount, + account: funding_account, + token_account: payer_token, + token_authority: payer, + }, + ) + .await + .unwrap(); + } + } + + // + // TEST: Deposit and withdraw tokens for all mints + // + { + let start_balance = solana.token_account_balance(payer_mint_accounts[0]).await; + + // this activates the positions + let deposit_amount = 100; + for &payer_token in payer_mint_accounts { + send_tx( + solana, + DepositInstruction { + amount: deposit_amount, + account, + token_account: payer_token, + token_authority: payer, + }, + ) + .await + .unwrap(); + } + + // this closes the positions + for &payer_token in payer_mint_accounts { + send_tx( + solana, + WithdrawInstruction { + amount: u64::MAX, + allow_borrow: false, + account, + owner, + token_account: payer_token, + }, + ) + .await + .unwrap(); + } + + // Check that positions are fully deactivated + let account: MangoAccount = solana.get_account(account).await; + assert_eq!(account.indexed_positions.iter_active().count(), 0); + + // No user tokens got lost + for &payer_token in payer_mint_accounts { + assert_eq!( + start_balance, + solana.token_account_balance(payer_token).await + ); + } + } + + // + // TEST: Activate a position by borrowing, then close the borrow + // + { + let start_balance = solana.token_account_balance(payer_mint_accounts[0]).await; + + // collateral for the incoming borrow + let collateral_amount = 1000; + send_tx( + solana, + DepositInstruction { + amount: collateral_amount, + account, + token_account: payer_mint_accounts[0], + token_authority: payer, + }, + ) + .await + .unwrap(); + + // borrow some of mint1, activating the position + let borrow_amount = 10; + send_tx( + solana, + WithdrawInstruction { + amount: borrow_amount, + allow_borrow: true, + account, + owner, + token_account: payer_mint_accounts[1], + }, + ) + .await + .unwrap(); + + // give it back, closing the position + send_tx( + solana, + DepositInstruction { + amount: borrow_amount, + account, + token_account: payer_mint_accounts[1], + token_authority: payer, + }, + ) + .await + .unwrap(); + + // withdraw the collateral, closing the position + send_tx( + solana, + WithdrawInstruction { + amount: collateral_amount, + allow_borrow: false, + account, + owner, + token_account: payer_mint_accounts[0], + }, + ) + .await + .unwrap(); + + // Check that positions are fully deactivated + let account: MangoAccount = solana.get_account(account).await; + assert_eq!(account.indexed_positions.iter_active().count(), 0); + + // No user tokens got lost + for &payer_token in payer_mint_accounts { + assert_eq!( + start_balance, + solana.token_account_balance(payer_token).await + ); + } + } + + Ok(()) +}