Merge pull request #767 from blockworks-foundation/ckamm/audit-fixes

Address audit comments
This commit is contained in:
Christian Kamm 2023-10-31 08:21:19 +01:00 committed by GitHub
commit c667a6615b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 94 additions and 102 deletions

View File

@ -95,7 +95,7 @@ impl State {
for startable_chunk in startable.chunks(8) {
let mut instructions = vec![];
let mut ix_targets = vec![];
let mut caller_account = mango_client.mango_account().await?;
let mut liqor_account = mango_client.mango_account().await?;
for (pubkey, tcs_id, incentive_token_index) in startable_chunk {
let ix = match self.make_start_ix(pubkey, *tcs_id).await {
Ok(v) => v,
@ -110,14 +110,14 @@ impl State {
};
instructions.push(ix);
ix_targets.push((*pubkey, *tcs_id));
caller_account.ensure_token_position(*incentive_token_index)?;
liqor_account.ensure_token_position(*incentive_token_index)?;
}
// Clear newly created token positions, so the caller account is mostly empty
// Clear newly created token positions, so the liqor account is mostly empty
for token_index in startable_chunk.iter().map(|(_, _, ti)| *ti).unique() {
let mint = mango_client.context.token(token_index).mint_info.mint;
instructions.append(&mut mango_client.token_withdraw_instructions(
&caller_account,
&liqor_account,
mint,
u64::MAX,
false,

View File

@ -1355,9 +1355,9 @@ impl MangoClient {
let mut ams = anchor_lang::ToAccountMetas::to_account_metas(
&mango_v4::accounts::TokenConditionalSwapStart {
group: self.group(),
account: *account.0,
caller: self.mango_account_address,
caller_authority: self.owner(),
liqee: *account.0,
liqor: self.mango_account_address,
liqor_authority: self.owner(),
},
None,
);

View File

@ -5611,7 +5611,7 @@
"isSigner": false
},
{
"name": "account",
"name": "liqee",
"isMut": true,
"isSigner": false,
"relations": [
@ -5619,7 +5619,7 @@
]
},
{
"name": "caller",
"name": "liqor",
"isMut": true,
"isSigner": false,
"relations": [
@ -5627,7 +5627,7 @@
]
},
{
"name": "callerAuthority",
"name": "liqorAuthority",
"isMut": false,
"isSigner": true
}

View File

@ -12,17 +12,17 @@ pub struct TokenConditionalSwapStart<'info> {
#[account(
mut,
has_one = group,
constraint = account.load()?.is_operational() @ MangoError::AccountIsFrozen
constraint = liqee.load()?.is_operational() @ MangoError::AccountIsFrozen
)]
pub account: AccountLoader<'info, MangoAccountFixed>,
pub liqee: AccountLoader<'info, MangoAccountFixed>,
#[account(
mut,
has_one = group,
constraint = caller.load()?.is_operational() @ MangoError::AccountIsFrozen,
constraint = caller.load()?.is_owner_or_delegate(caller_authority.key()),
constraint = caller.key() != account.key(),
constraint = liqor.load()?.is_operational() @ MangoError::AccountIsFrozen,
constraint = liqor.load()?.is_owner_or_delegate(liqor_authority.key()),
constraint = liqor.key() != liqee.key(),
)]
pub caller: AccountLoader<'info, MangoAccountFixed>,
pub caller_authority: Signer<'info>,
pub liqor: AccountLoader<'info, MangoAccountFixed>,
pub liqor_authority: Signer<'info>,
}

View File

@ -16,21 +16,18 @@ pub fn token_conditional_swap_start(
token_conditional_swap_id: u64,
) -> Result<()> {
let group_pk = &ctx.accounts.group.key();
let account_key = ctx.accounts.account.key();
let caller_key = ctx.accounts.caller.key();
let liqee_key = ctx.accounts.liqee.key();
let liqor_key = ctx.accounts.liqor.key();
let mut account = ctx.accounts.account.load_full_mut()?;
require!(
!account.fixed.being_liquidated(),
MangoError::BeingLiquidated,
);
let mut liqee = ctx.accounts.liqee.load_full_mut()?;
require!(!liqee.fixed.being_liquidated(), MangoError::BeingLiquidated,);
let mut caller = ctx.accounts.caller.load_full_mut()?;
let mut liqor = ctx.accounts.liqor.load_full_mut()?;
let mut account_retriever = ScanningAccountRetriever::new(ctx.remaining_accounts, group_pk)
.context("create account retriever")?;
let tcs = account
let tcs = liqee
.token_conditional_swap_by_index(token_conditional_swap_index)?
.clone();
require!(tcs.is_configured(), MangoError::TokenConditionalSwapNotSet);
@ -47,9 +44,9 @@ pub fn token_conditional_swap_start(
MangoError::TokenConditionalSwapTypeNotStartable
);
let mut health_cache = new_health_cache(&account.borrow(), &account_retriever)
let mut health_cache = new_health_cache(&liqee.borrow(), &account_retriever)
.context("create liqee health cache")?;
let pre_init_health = account.check_health_pre(&health_cache)?;
let pre_init_health = liqee.check_health_pre(&health_cache)?;
let (sell_bank, sell_oracle_price, buy_bank_and_oracle_opt) =
account_retriever.banks_mut_and_oracles(sell_token_index, buy_token_index)?;
@ -73,18 +70,17 @@ pub fn token_conditional_swap_start(
// not accounting the incentive fee perfectly.
let incentive_native = incentive.clamp_to_u64();
let (account_sell_token, account_sell_raw_index) =
account.token_position_mut(sell_token_index)?;
let (caller_sell_token, caller_sell_raw_index, _) =
caller.ensure_token_position(sell_token_index)?;
let (liqee_sell_token, liqee_sell_raw_index) = liqee.token_position_mut(sell_token_index)?;
let (liqor_sell_token, liqor_sell_raw_index, _) =
liqor.ensure_token_position(sell_token_index)?;
sell_bank.deposit(caller_sell_token, incentive, now_ts)?;
sell_bank.deposit(liqor_sell_token, incentive, now_ts)?;
// This withdraw might be a borrow, so can fail due to net borrows or reduce-only
let account_sell_pre_balance = account_sell_token.native(sell_bank);
sell_bank.withdraw_with_fee(account_sell_token, incentive, now_ts)?;
let account_sell_post_balance = account_sell_token.native(sell_bank);
if account_sell_post_balance < 0 {
let liqee_sell_pre_balance = liqee_sell_token.native(sell_bank);
sell_bank.withdraw_with_fee(liqee_sell_token, incentive, now_ts)?;
let liqee_sell_post_balance = liqee_sell_token.native(sell_bank);
if liqee_sell_post_balance < 0 {
require!(
tcs.allow_creating_borrows(),
MangoError::TokenConditionalSwapCantPayIncentive
@ -96,31 +92,29 @@ pub fn token_conditional_swap_start(
sell_bank.check_net_borrows(sell_oracle_price)?;
}
health_cache.adjust_token_balance(
sell_bank,
account_sell_post_balance - account_sell_pre_balance,
)?;
health_cache
.adjust_token_balance(sell_bank, liqee_sell_post_balance - liqee_sell_pre_balance)?;
emit!(TokenBalanceLog {
mango_group: *group_pk,
mango_account: account_key,
mango_account: liqee_key,
token_index: sell_token_index,
indexed_position: account_sell_token.indexed_position.to_bits(),
indexed_position: liqee_sell_token.indexed_position.to_bits(),
deposit_index: sell_bank.deposit_index.to_bits(),
borrow_index: sell_bank.borrow_index.to_bits(),
});
emit!(TokenBalanceLog {
mango_group: *group_pk,
mango_account: caller_key,
mango_account: liqor_key,
token_index: sell_token_index,
indexed_position: caller_sell_token.indexed_position.to_bits(),
indexed_position: liqor_sell_token.indexed_position.to_bits(),
deposit_index: sell_bank.deposit_index.to_bits(),
borrow_index: sell_bank.borrow_index.to_bits(),
});
emit!(TokenConditionalSwapStartLog {
mango_group: *group_pk,
mango_account: account_key,
caller: caller_key,
mango_account: liqee_key,
caller: liqor_key,
token_conditional_swap_id: tcs.id,
incentive_token_index: sell_token_index,
incentive_amount: incentive_native,
@ -129,12 +123,14 @@ pub fn token_conditional_swap_start(
//
// Start the tcs
//
let tcs = account.token_conditional_swap_mut_by_index(token_conditional_swap_index)?;
let tcs = liqee.token_conditional_swap_mut_by_index(token_conditional_swap_index)?;
tcs.start_timestamp = now_ts;
tcs.sold += incentive_native;
assert!(tcs.passed_start(now_ts));
account.check_health_post(&health_cache, pre_init_health)?;
tcs.sold += incentive_native;
assert!(tcs.sold <= tcs.max_sell);
liqee.check_health_post(&health_cache, pre_init_health)?;
Ok(())
}

View File

@ -59,9 +59,7 @@ pub fn token_conditional_swap_trigger(
// Possibly wipe the tcs and exit, if it's already expired
if tcs_is_expired {
if min_buy_token > 0 {
require!(!tcs_is_expired, MangoError::TokenConditionalSwapExpired);
}
require!(min_buy_token == 0, MangoError::TokenConditionalSwapExpired);
let (buy_bank, _buy_token_price, sell_bank_and_oracle_opt) =
account_retriever.banks_mut_and_oracles(buy_token_index, sell_token_index)?;
@ -280,18 +278,19 @@ fn action(
let sell_token_amount =
(I80F48::from(buy_token_amount) * I80F48::from_num(premium_price)).floor();
let maker_fee = tcs.maker_fee(sell_token_amount);
let sell_token_amount_u64 = sell_token_amount.to_num::<u64>();
let maker_fee = sell_token_amount_with_maker_fee - sell_token_amount_u64;
let taker_fee = tcs.taker_fee(sell_token_amount);
let sell_token_amount_from_liqee = sell_token_amount_with_maker_fee;
let sell_token_amount_to_liqor = sell_token_amount_with_maker_fee - maker_fee - taker_fee;
let sell_token_amount_to_liqor = sell_token_amount_u64 - taker_fee;
// do the token transfer between liqee and liqor
let buy_token_amount_i80f48 = I80F48::from(buy_token_amount);
let (liqee_buy_token, liqee_buy_raw_index) = liqee.token_position_mut(tcs.buy_token_index)?;
let (liqor_buy_token, liqor_buy_raw_index) = liqor.token_position_mut(tcs.buy_token_index)?;
let liqee_buy_active = buy_bank.deposit(liqee_buy_token, buy_token_amount_i80f48, now_ts)?;
buy_bank.deposit(liqee_buy_token, buy_token_amount_i80f48, now_ts)?;
let liqor_buy_withdraw =
buy_bank.withdraw_with_fee(liqor_buy_token, buy_token_amount_i80f48, now_ts)?;
@ -331,13 +330,8 @@ fn action(
let liqee_sell_indexed_position = liqee_sell_token.indexed_position;
let liqor_sell_indexed_position = liqor_sell_token.indexed_position;
// With a scanning account retriever, it's safe to deactivate inactive token positions immediately
if !liqee_buy_active {
liqee.deactivate_token_position_and_log(liqee_buy_raw_index, liqee_key);
}
if !liqee_sell_withdraw.position_is_active {
liqee.deactivate_token_position_and_log(liqee_sell_raw_index, liqee_key);
}
// With a scanning account retriever, it's safe to deactivate inactive token positions immediately.
// Liqee positions can only be deactivated if the tcs is closed (see below).
if !liqor_buy_withdraw.position_is_active {
liqor.deactivate_token_position_and_log(liqor_buy_raw_index, liqor_key);
}

View File

@ -740,9 +740,9 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
let res = send_tx(
solana,
TokenConditionalSwapStartInstruction {
account,
caller: liqor,
caller_owner: owner,
liqee: account,
liqor,
liqor_owner: owner,
index: 0,
},
)
@ -780,9 +780,9 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
send_tx(
solana,
TokenConditionalSwapStartInstruction {
account,
caller: liqor,
caller_owner: owner,
liqee: account,
liqor,
liqor_owner: owner,
index: 0,
},
)
@ -846,8 +846,9 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
liqor,
liqor_owner: owner,
index: 0,
max_buy_token_to_liqee: 10000,
max_sell_token_to_liqor: 11055, // expected price of 1.0+0.5% premium sell per buy, with 10% maker fee
max_buy_token_to_liqee: 90000,
// expected price of 1.0+0.5% premium sell per buy, with 10% maker fee, so buy token transfer of 10k
max_sell_token_to_liqor: 11055,
min_buy_token: 1,
min_taker_price: 0.0,
},
@ -858,7 +859,7 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
account_quote_expected += 10000.0;
account_base_expected += -11055.0;
liqor_quote_expected += -10000.0;
liqor_base_expected += 9549.0; // roughly 10000 * 1 * 1.005 premium * 0.95 taker fee
liqor_base_expected += 9547.0; // 10000 * 1 * 1.005 premium * 0.95 taker fee
let account_quote = account_position_f64(solana, account, quote_token.bank).await;
let account_base = account_position_f64(solana, account, base_token.bank).await;
@ -890,7 +891,8 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
liqor_owner: owner,
index: 0,
max_buy_token_to_liqee: 10000,
max_sell_token_to_liqor: 11110, // expected price of 1.0+1% premium sell per buy, with 10% maker fee
// expected price of 1.0+1% premium sell per buy, with 10% maker fee, so sell token transfer of 11110
max_sell_token_to_liqor: 90000,
min_buy_token: 1,
min_taker_price: 0.0,
},
@ -901,7 +903,7 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
account_quote_expected += 10000.0;
account_base_expected += -11110.0;
liqor_quote_expected += -10000.0;
liqor_base_expected += 9595.0; // roughly 10000 * 1 * 1.01 premium * 0.95 taker fee
liqor_base_expected += 9595.0; // 10000 * 1 * 1.01 premium * 0.95 taker fee
let account_quote = account_position_f64(solana, account, quote_token.bank).await;
let account_base = account_position_f64(solana, account, base_token.bank).await;
@ -952,9 +954,9 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
let res = send_tx(
solana,
TokenConditionalSwapStartInstruction {
account,
caller: liqor,
caller_owner: owner,
liqee: account,
liqor,
liqor_owner: owner,
index: 1,
},
)
@ -973,9 +975,9 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
send_tx(
solana,
TokenConditionalSwapStartInstruction {
account,
caller: liqor,
caller_owner: owner,
liqee: account,
liqor,
liqor_owner: owner,
index: 1,
},
)
@ -996,9 +998,9 @@ async fn test_token_conditional_swap_premium_auction() -> Result<(), TransportEr
let res = send_tx(
solana,
TokenConditionalSwapStartInstruction {
account,
caller: liqor,
caller_owner: owner,
liqee: account,
liqor,
liqor_owner: owner,
index: 1,
},
)

View File

@ -4810,9 +4810,9 @@ impl ClientInstruction for TokenConditionalSwapTriggerInstruction {
#[derive(Clone)]
pub struct TokenConditionalSwapStartInstruction {
pub account: Pubkey,
pub caller: Pubkey,
pub caller_owner: TestKeypair,
pub liqee: Pubkey,
pub liqor: Pubkey,
pub liqor_owner: TestKeypair,
pub index: u8,
}
#[async_trait::async_trait(?Send)]
@ -4825,18 +4825,18 @@ impl ClientInstruction for TokenConditionalSwapStartInstruction {
) -> (Self::Accounts, instruction::Instruction) {
let program_id = mango_v4::id();
let account = account_loader
.load_mango_account(&self.account)
let liqee = account_loader
.load_mango_account(&self.liqee)
.await
.unwrap();
let tcs = account
let tcs = liqee
.token_conditional_swap_by_index(self.index.into())
.unwrap()
.clone();
let sell_mint_info =
get_mint_info_by_token_index(&account_loader, &account, tcs.sell_token_index).await;
get_mint_info_by_token_index(&account_loader, &liqee, tcs.sell_token_index).await;
let instruction = Self::Instruction {
token_conditional_swap_index: self.index,
@ -4845,7 +4845,7 @@ impl ClientInstruction for TokenConditionalSwapStartInstruction {
let health_check_metas = derive_health_check_remaining_account_metas(
&account_loader,
&account,
&liqee,
Some(sell_mint_info.first_bank()),
true,
None,
@ -4853,10 +4853,10 @@ impl ClientInstruction for TokenConditionalSwapStartInstruction {
.await;
let accounts = Self::Accounts {
group: account.fixed.group,
account: self.account,
caller: self.caller,
caller_authority: self.caller_owner.pubkey(),
group: liqee.fixed.group,
liqee: self.liqee,
liqor: self.liqor,
liqor_authority: self.liqor_owner.pubkey(),
};
let mut instruction = make_instruction(program_id, &accounts, &instruction);
@ -4865,6 +4865,6 @@ impl ClientInstruction for TokenConditionalSwapStartInstruction {
}
fn signers(&self) -> Vec<TestKeypair> {
vec![self.caller_owner]
vec![self.liqor_owner]
}
}

View File

@ -5611,7 +5611,7 @@ export type MangoV4 = {
"isSigner": false
},
{
"name": "account",
"name": "liqee",
"isMut": true,
"isSigner": false,
"relations": [
@ -5619,7 +5619,7 @@ export type MangoV4 = {
]
},
{
"name": "caller",
"name": "liqor",
"isMut": true,
"isSigner": false,
"relations": [
@ -5627,7 +5627,7 @@ export type MangoV4 = {
]
},
{
"name": "callerAuthority",
"name": "liqorAuthority",
"isMut": false,
"isSigner": true
}
@ -18823,7 +18823,7 @@ export const IDL: MangoV4 = {
"isSigner": false
},
{
"name": "account",
"name": "liqee",
"isMut": true,
"isSigner": false,
"relations": [
@ -18831,7 +18831,7 @@ export const IDL: MangoV4 = {
]
},
{
"name": "caller",
"name": "liqor",
"isMut": true,
"isSigner": false,
"relations": [
@ -18839,7 +18839,7 @@ export const IDL: MangoV4 = {
]
},
{
"name": "callerAuthority",
"name": "liqorAuthority",
"isMut": false,
"isSigner": true
}