Position lifetime, dust etc

- withdraw and deposit may deactivate positions explicitly
- adjusting bank positions may collect dust for positions between
  0 and 1 native tokens
- health check always assumes all active and new banks/oracles are
  passed

programs/mango-v4/src/instructions/create_account.rs

@@ -31,6 +31,7 @@ pub fn create_account(ctx: Context<CreateAccount>, 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(())

programs/mango-v4/src/instructions/deposit.rs

@@ -56,14 +56,18 @@ pub fn deposit(ctx: Context<Deposit>, 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(())
 }

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,
     };
 

programs/mango-v4/src/instructions/withdraw.rs

@@ -58,11 +58,11 @@ pub fn withdraw(ctx: Context<Withdraw>, 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<Withdraw>, 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<Withdraw>, 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<Withdraw>, 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(())
 }

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<Item = &IndexedPosition> {
         self.values.iter().filter(|p| p.is_active())
     }

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
     }
 }

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,

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(())
+}