diff --git a/programs/mango-v4/src/instructions/margin_trade.rs b/programs/mango-v4/src/instructions/margin_trade.rs index 5602aa6bb..0d1f8c931 100644 --- a/programs/mango-v4/src/instructions/margin_trade.rs +++ b/programs/mango-v4/src/instructions/margin_trade.rs @@ -1,6 +1,6 @@ use crate::error::MangoError; -use crate::state::{compute_health, MangoAccount, Group, Bank}; -use crate::{group_seeds, util, Mango}; +use crate::state::{compute_health, Bank, Group, MangoAccount}; +use crate::{group_seeds, Mango}; use anchor_lang::prelude::*; use anchor_spl::token::TokenAccount; use solana_program::instruction::Instruction; @@ -20,49 +20,77 @@ pub struct MarginTrade<'info> { pub owner: Signer<'info>, } -/// reference https://github.com/blockworks-foundation/mango-v3/blob/mc/flash_loan/program/src/processor.rs#L5323 pub fn margin_trade<'key, 'accounts, 'remaining, 'info>( ctx: Context<'key, 'accounts, 'remaining, 'info, MarginTrade<'info>>, + banks_len: usize, cpi_data: Vec, ) -> Result<()> { let group = ctx.accounts.group.load()?; let mut account = ctx.accounts.account.load_mut()?; - let active_len = account.indexed_positions.iter_active().count(); // remaining_accounts layout is expected as follows - // * active_len number of banks - // * active_len number of oracles + // * banks_len number of banks + // * banks_len number of oracles // * cpi_program // * cpi_accounts - let banks = &ctx.remaining_accounts[0..active_len]; - let oracles = &ctx.remaining_accounts[active_len..active_len * 2]; + // assert that user has passed in enough banks, this might be greater than his current + // total number of indexed positions, since + // user might end up withdrawing or depositing and activating a new indexed position + require!( + banks_len >= account.indexed_positions.iter_active().count(), + MangoError::SomeError // todo: SomeError + ); - let cpi_program_id = *ctx.remaining_accounts[active_len * 2].key; + // unpack remaining_accounts + let banks = &ctx.remaining_accounts[0..banks_len]; + let oracles = &ctx.remaining_accounts[banks_len..banks_len * 2]; + let cpi_program_id = *ctx.remaining_accounts[banks_len * 2].key; - // prepare for cpi + // prepare account for cpi ix let (cpi_ais, cpi_ams) = { // we also need the group let mut cpi_ais = [ctx.accounts.group.to_account_info()].to_vec(); // skip banks, oracles and cpi program from the remaining_accounts - let mut remaining_cpi_ais = ctx.remaining_accounts[active_len * 2 + 1..].to_vec(); + let mut remaining_cpi_ais = ctx.remaining_accounts[banks_len * 2 + 1..].to_vec(); cpi_ais.append(&mut remaining_cpi_ais); + // todo: I'm wondering if there's a way to do this without putting cpi_ais on the heap. + // But fine to defer to the future let mut cpi_ams = cpi_ais.to_account_metas(Option::None); - // we want group to be the signer, so that loans can be taken from the token vaults + // we want group to be the signer, so that token vaults can be credited to or withdrawn from cpi_ams[0].is_signer = true; (cpi_ais, cpi_ams) }; - // since we are using group signer seeds to invoke cpi, - // assert that none of the cpi accounts is the mango program to prevent that invoker doesn't - // abuse this ix to do unwanted changes + // sanity checks for cpi_ai in &cpi_ais { + // since we are using group signer seeds to invoke cpi, + // assert that none of the cpi accounts is the mango program to prevent that invoker doesn't + // abuse this ix to do unwanted changes require!( cpi_ai.key() != Mango::id(), MangoError::InvalidMarginTradeTargetCpiProgram ); + + // assert that user has passed in the bank for every + // token account he wants to deposit/withdraw from in cpi + if cpi_ai.owner == &TokenAccount::owner() { + let maybe_mango_vault_token_account = + Account::::try_from(cpi_ai).unwrap(); + if maybe_mango_vault_token_account.owner == ctx.accounts.group.key() { + require!( + banks.iter().any(|bank_ai| { + let bank_loader = AccountLoader::<'_, Bank>::try_from(bank_ai).unwrap(); + let bank = bank_loader.load().unwrap(); + bank.mint == maybe_mango_vault_token_account.mint + }), + // todo: errorcode + MangoError::SomeError + ) + } + } } // compute pre cpi health @@ -77,12 +105,12 @@ pub fn margin_trade<'key, 'accounts, 'remaining, 'info>( accounts: cpi_ams, }; let group_seeds = group_seeds!(group); - let pre_cpi_token_vault_amounts = get_pre_cpi_token_amounts(&ctx, &cpi_ais); + let pre_cpi_amounts = get_pre_cpi_amounts(&ctx, &cpi_ais); solana_program::program::invoke_signed(&cpi_ix, &cpi_ais, &[group_seeds])?; - adjust_for_post_cpi_token_amounts( + adjust_for_post_cpi_amounts( &ctx, &cpi_ais, - pre_cpi_token_vault_amounts, + pre_cpi_amounts, group, &mut banks.to_vec(), &mut account, @@ -98,53 +126,59 @@ pub fn margin_trade<'key, 'accounts, 'remaining, 'info>( Ok(()) } -fn get_pre_cpi_token_amounts(ctx: &Context, cpi_ais: &Vec) -> Vec { - let mut mango_vault_token_account_amounts = vec![]; - for maybe_token_account in cpi_ais +fn get_pre_cpi_amounts(ctx: &Context, cpi_ais: &Vec) -> Vec { + let mut amounts = vec![]; + for token_account in cpi_ais .iter() .filter(|ai| ai.owner == &TokenAccount::owner()) { - let maybe_mango_vault_token_account = - Account::::try_from(maybe_token_account).unwrap(); - if maybe_mango_vault_token_account.owner == ctx.accounts.group.key() { - mango_vault_token_account_amounts.push(maybe_mango_vault_token_account.amount) + let vault = Account::::try_from(token_account).unwrap(); + if vault.owner == ctx.accounts.group.key() { + amounts.push(vault.amount) } } - mango_vault_token_account_amounts + amounts } -/// withdraws from bank, on users behalf, if he hasn't returned back entire loan amount -fn adjust_for_post_cpi_token_amounts( +fn adjust_for_post_cpi_amounts( ctx: &Context, cpi_ais: &Vec, - pre_cpi_token_vault_amounts: Vec, + pre_cpi_amounts: Vec, group: Ref, banks: &mut Vec, account: &mut RefMut, ) -> Result<()> { - let x = cpi_ais + let token_accounts_iter = cpi_ais .iter() .filter(|ai| ai.owner == &TokenAccount::owner()); - for (maybe_token_account, (pre_cpi_token_vault_amount, bank_ai)) in - util::zip!(x, pre_cpi_token_vault_amounts.iter(), banks.iter()) + for (token_account, pre_cpi_amount) in + // token_accounts and pre_cpi_amounts are assumed to be in correct order + token_accounts_iter.zip(pre_cpi_amounts.iter()) { - let maybe_mango_vault_token_account = - Account::::try_from(maybe_token_account).unwrap(); - if maybe_mango_vault_token_account.owner == ctx.accounts.group.key() { - let still_loaned_amount = - pre_cpi_token_vault_amount - maybe_mango_vault_token_account.amount; - if still_loaned_amount <= 0 { - continue; - } - - let token_index = group - .tokens - .index_for_mint(&maybe_mango_vault_token_account.mint)?; + let vault = Account::::try_from(token_account).unwrap(); + if vault.owner == ctx.accounts.group.key() { + let token_index = group.tokens.index_for_mint(&vault.mint)?; let mut position = *account.indexed_positions.get_mut_or_create(token_index)?.0; + + // find bank for token account + let bank_ai = banks + .iter() + .find(|bank_ai| { + let bank_loader = AccountLoader::<'_, Bank>::try_from(bank_ai).unwrap(); + let bank = bank_loader.load().unwrap(); + bank.mint == vault.mint + }) + .ok_or(MangoError::SomeError)?; // todo: replace SomeError let bank_loader = AccountLoader::<'_, Bank>::try_from(bank_ai)?; let mut bank = bank_loader.load_mut()?; - bank.withdraw(&mut position, still_loaned_amount); + + // user has either withdrawn or deposited + if *pre_cpi_amount > vault.amount { + bank.withdraw(&mut position, pre_cpi_amount - vault.amount); + } else { + bank.deposit(&mut position, vault.amount - pre_cpi_amount); + } } } Ok(()) diff --git a/programs/mango-v4/src/lib.rs b/programs/mango-v4/src/lib.rs index eb8daac64..935d4a5f8 100644 --- a/programs/mango-v4/src/lib.rs +++ b/programs/mango-v4/src/lib.rs @@ -69,9 +69,10 @@ pub mod mango_v4 { pub fn margin_trade<'key, 'accounts, 'remaining, 'info>( ctx: Context<'key, 'accounts, 'remaining, 'info, MarginTrade<'info>>, + banks_len: usize, cpi_data: Vec, ) -> Result<()> { - instructions::margin_trade(ctx, cpi_data) + instructions::margin_trade(ctx, banks_len, cpi_data) } } diff --git a/programs/mango-v4/tests/fixtures/margin_trade.so b/programs/mango-v4/tests/fixtures/margin_trade.so new file mode 100755 index 000000000..97ff6cfe9 Binary files /dev/null and b/programs/mango-v4/tests/fixtures/margin_trade.so differ diff --git a/programs/mango-v4/tests/program_test/mango_client.rs b/programs/mango-v4/tests/program_test/mango_client.rs index f327053c1..4216d106e 100644 --- a/programs/mango-v4/tests/program_test/mango_client.rs +++ b/programs/mango-v4/tests/program_test/mango_client.rs @@ -141,10 +141,9 @@ pub struct MarginTradeInstruction<'keypair> { pub account: Pubkey, pub owner: &'keypair Keypair, pub mango_token_vault: Pubkey, - pub mango_group: Pubkey, pub margin_trade_program_id: Pubkey, - pub loan_token_account: Pubkey, - pub loan_token_account_owner: Pubkey, + pub deposit_account: Pubkey, + pub deposit_account_owner: Pubkey, pub margin_trade_program_ix_cpi_data: Vec, } #[async_trait::async_trait(?Send)] @@ -156,12 +155,14 @@ impl<'keypair> ClientInstruction for MarginTradeInstruction<'keypair> { account_loader: impl ClientAccountLoader + 'async_trait, ) -> (Self::Accounts, instruction::Instruction) { let program_id = mango_v4::id(); - let instruction = Self::Instruction { - cpi_data: self.margin_trade_program_ix_cpi_data.clone(), - }; let account: MangoAccount = account_loader.load(&self.account).await.unwrap(); + let instruction = Self::Instruction { + banks_len: account.indexed_positions.iter_active().count(), + cpi_data: self.margin_trade_program_ix_cpi_data.clone(), + }; + let accounts = Self::Accounts { group: account.group, account: self.account, @@ -185,12 +186,12 @@ impl<'keypair> ClientInstruction for MarginTradeInstruction<'keypair> { is_signer: false, }); instruction.accounts.push(AccountMeta { - pubkey: self.loan_token_account, + pubkey: self.deposit_account, is_writable: true, is_signer: false, }); instruction.accounts.push(AccountMeta { - pubkey: self.loan_token_account_owner, + pubkey: self.deposit_account_owner, is_writable: false, is_signer: false, }); diff --git a/programs/mango-v4/tests/program_test/mod.rs b/programs/mango-v4/tests/program_test/mod.rs index a6c89f814..fcd2b7c12 100644 --- a/programs/mango-v4/tests/program_test/mod.rs +++ b/programs/mango-v4/tests/program_test/mod.rs @@ -106,7 +106,7 @@ impl TestContext { })); // intentionally set to half the limit, to catch potential problems early - test.set_compute_max_units(200000); + test.set_compute_max_units(100000); // Setup the environment @@ -177,7 +177,7 @@ impl TestContext { &Account { mint: mints[0].pubkey, owner: *mtta_owner.unwrap(), - amount: 10, + amount: 0, state: AccountState::Initialized, is_native: COption::None, ..Account::default() diff --git a/programs/mango-v4/tests/test_basic.rs b/programs/mango-v4/tests/test_basic.rs index c576fae27..5d7488b1f 100644 --- a/programs/mango-v4/tests/test_basic.rs +++ b/programs/mango-v4/tests/test_basic.rs @@ -17,18 +17,7 @@ mod program_test; // that they work in principle. It should be split up / renamed. #[tokio::test] async fn test_basic() -> Result<(), TransportError> { - let margin_trade_program_id = - Pubkey::from_str("J83w4HKfqxwcq3BEMMkPFSppX3gqekLyLJBexebFVkix").unwrap(); - let margin_trade_token_account = Keypair::new(); - let (mtta_owner, mtta_bump_seeds) = - Pubkey::find_program_address(&[b"margintrade"], &margin_trade_program_id); - let context = TestContext::new( - Option::None, - Some(&margin_trade_program_id), - Some(&margin_trade_token_account), - Some(&mtta_owner), - ) - .await; + let context = TestContext::new(Option::None, Option::None, Option::None, Option::None).await; let solana = &context.solana.clone(); let admin = &Keypair::new(); @@ -140,35 +129,6 @@ async fn test_basic() -> Result<(), TransportError> { ); } - // - // TEST: Margin trade - // - { - send_tx( - solana, - MarginTradeInstruction { - account, - owner, - mango_token_vault: vault, - mango_group: group, - margin_trade_program_id, - loan_token_account: margin_trade_token_account.pubkey(), - loan_token_account_owner: mtta_owner, - margin_trade_program_ix_cpi_data: { - let ix = margin_trade::instruction::MarginTrade { - amount_from: 2, - amount_to: 1, - loan_token_account_owner_bump_seeds: mtta_bump_seeds, - }; - ix.data() - }, - }, - ) - .await - .unwrap(); - } - let margin_trade_loan = 1; - // // TEST: Withdraw funds // @@ -189,10 +149,7 @@ async fn test_basic() -> Result<(), TransportError> { .await .unwrap(); - assert_eq!( - solana.token_account_balance(vault).await, - withdraw_amount - margin_trade_loan - ); + assert_eq!(solana.token_account_balance(vault).await, withdraw_amount); assert_eq!( solana.token_account_balance(payer_mint0_account).await, start_balance + withdraw_amount diff --git a/programs/mango-v4/tests/test_margin_trade.rs b/programs/mango-v4/tests/test_margin_trade.rs new file mode 100644 index 000000000..61a03b2a3 --- /dev/null +++ b/programs/mango-v4/tests/test_margin_trade.rs @@ -0,0 +1,187 @@ +#![cfg(feature = "test-bpf")] + +use anchor_lang::InstructionData; +use fixed::types::I80F48; +use solana_program::pubkey::Pubkey; +use solana_program_test::*; +use solana_sdk::signature::Signer; +use solana_sdk::{signature::Keypair, transport::TransportError}; +use std::str::FromStr; + +use mango_v4::state::*; +use program_test::*; + +mod program_test; + +// This is an unspecific happy-case test that just runs a few instructions to check +// that they work in principle. It should be split up / renamed. +#[tokio::test] +async fn test_margin_trade() -> Result<(), TransportError> { + let margin_trade_program_id = + Pubkey::from_str("J83w4HKfqxwcq3BEMMkPFSppX3gqekLyLJBexebFVkix").unwrap(); + let margin_trade_token_account = Keypair::new(); + let (mtta_owner, mtta_bump_seeds) = + Pubkey::find_program_address(&[b"margintrade"], &margin_trade_program_id); + let context = TestContext::new( + Option::None, + Some(&margin_trade_program_id), + Some(&margin_trade_token_account), + Some(&mtta_owner), + ) + .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 payer_mint0_account = context.users[1].token_accounts[0]; + let dust_threshold = 0.01; + + // + // SETUP: Create a group, account, register a token (mint0) + // + + 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 create_stub_oracle_accounts = send_tx( + solana, + CreateStubOracle { + mint: mint0.pubkey, + payer, + }, + ) + .await + .unwrap(); + let _oracle = create_stub_oracle_accounts.oracle; + + send_tx( + solana, + SetStubOracle { + mint: mint0.pubkey, + payer, + price: "1.0", + }, + ) + .await + .unwrap(); + + let address_lookup_table = solana.create_address_lookup_table(admin, payer).await; + + let register_token_accounts = send_tx( + solana, + RegisterTokenInstruction { + decimals: mint0.decimals, + maint_asset_weight: 0.9, + init_asset_weight: 0.8, + maint_liab_weight: 1.1, + init_liab_weight: 1.2, + group, + admin, + mint: mint0.pubkey, + address_lookup_table, + payer, + }, + ) + .await + .unwrap(); + let bank = register_token_accounts.bank; + let vault = register_token_accounts.vault; + + // + // TEST: Deposit funds + // + let deposit_amount_initial = 100; + { + let start_balance = solana.token_account_balance(payer_mint0_account).await; + + send_tx( + solana, + DepositInstruction { + amount: deposit_amount_initial, + account, + token_account: payer_mint0_account, + token_authority: payer, + }, + ) + .await + .unwrap(); + + assert_eq!( + solana.token_account_balance(vault).await, + deposit_amount_initial + ); + assert_eq!( + solana.token_account_balance(payer_mint0_account).await, + start_balance - deposit_amount_initial + ); + 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_initial) + < dust_threshold + ); + assert!( + bank_data.native_total_deposits() - I80F48::from_num(deposit_amount_initial) + < dust_threshold + ); + } + + // + // TEST: Margin trade + // + let withdraw_amount = 2; + let deposit_amount = 1; + { + send_tx( + solana, + MarginTradeInstruction { + account, + owner, + mango_token_vault: vault, + margin_trade_program_id, + deposit_account: margin_trade_token_account.pubkey(), + deposit_account_owner: mtta_owner, + margin_trade_program_ix_cpi_data: { + let ix = margin_trade::instruction::MarginTrade { + amount_from: 2, + amount_to: 1, + deposit_account_owner_bump_seeds: mtta_bump_seeds, + }; + ix.data() + }, + }, + ) + .await + .unwrap(); + } + assert_eq!( + solana.token_account_balance(vault).await, + deposit_amount_initial - withdraw_amount + deposit_amount + ); + assert_eq!( + solana + .token_account_balance(margin_trade_token_account.pubkey()) + .await, + withdraw_amount - deposit_amount + ); + + Ok(()) +} diff --git a/programs/margin-trade/build-and-copy-fixture.sh b/programs/margin-trade/build-and-copy-fixture.sh new file mode 100755 index 000000000..83ee7469c --- /dev/null +++ b/programs/margin-trade/build-and-copy-fixture.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash + +set -euo pipefail + +anchor build +cp ../../target/deploy/margin_trade.so ../mango-v4/tests/fixtures/ \ No newline at end of file diff --git a/programs/margin-trade/src/lib.rs b/programs/margin-trade/src/lib.rs index b3d3d4c07..33b5b90b2 100644 --- a/programs/margin-trade/src/lib.rs +++ b/programs/margin-trade/src/lib.rs @@ -11,27 +11,24 @@ pub mod margin_trade { pub fn margin_trade( ctx: Context, amount_from: u64, - loan_token_account_owner_bump_seeds: u8, + deposit_account_owner_bump_seeds: u8, amount_to: u64, ) -> Result<()> { msg!( - "taking amount({}) loan from mango for mint {:?}", + "withdrawing({}) for mint {:?}", amount_from, - ctx.accounts.mango_token_vault.mint + ctx.accounts.withdraw_account.mint ); token::transfer(ctx.accounts.transfer_from_mango_vault_ctx(), amount_from)?; msg!("TODO: do something with the loan"); msg!( - "transferring amount({}) loan back to mango for mint {:?}", + "depositing amount({}) back to mint {:?}", amount_to, - ctx.accounts.loan_token_account.mint + ctx.accounts.deposit_account.mint ); - let seeds = &[ - b"margintrade".as_ref(), - &[loan_token_account_owner_bump_seeds], - ]; + let seeds = &[b"margintrade".as_ref(), &[deposit_account_owner_bump_seeds]]; token::transfer( ctx.accounts .transfer_back_to_mango_vault_ctx() @@ -55,16 +52,17 @@ impl anchor_lang::Id for MarginTrade { #[derive(Accounts)] pub struct MarginTradeCtx<'info> { - pub mango_group: Signer<'info>, + pub withdraw_account_owner: Signer<'info>, #[account(mut)] - pub mango_token_vault: Account<'info, TokenAccount>, + pub withdraw_account: Account<'info, TokenAccount>, #[account(mut)] - pub loan_token_account: Account<'info, TokenAccount>, + pub deposit_account: Account<'info, TokenAccount>, // todo: can we do better than UncheckedAccount? - pub loan_token_account_owner: UncheckedAccount<'info>, + /// CHECK + pub deposit_account_owner: UncheckedAccount<'info>, pub token_program: Program<'info, Token>, } @@ -73,9 +71,9 @@ impl<'info> MarginTradeCtx<'info> { pub fn transfer_from_mango_vault_ctx(&self) -> CpiContext<'_, '_, '_, 'info, Transfer<'info>> { let program = self.token_program.to_account_info(); let accounts = Transfer { - from: self.mango_token_vault.to_account_info(), - to: self.loan_token_account.to_account_info(), - authority: self.mango_group.to_account_info(), + from: self.withdraw_account.to_account_info(), + to: self.deposit_account.to_account_info(), + authority: self.withdraw_account_owner.to_account_info(), }; CpiContext::new(program, accounts) } @@ -85,9 +83,9 @@ impl<'info> MarginTradeCtx<'info> { ) -> CpiContext<'_, '_, '_, 'info, Transfer<'info>> { let program = self.token_program.to_account_info(); let accounts = Transfer { - from: self.loan_token_account.to_account_info(), - to: self.mango_token_vault.to_account_info(), - authority: self.loan_token_account_owner.to_account_info(), + from: self.deposit_account.to_account_info(), + to: self.withdraw_account.to_account_info(), + authority: self.deposit_account_owner.to_account_info(), }; CpiContext::new(program, accounts) }