Cleanups and bugfixes from the audit (#163)

* AccountExpand: cleanups from audit

* Group instructions: cleanups from audit

* AccountClose: cleanups from audit

* TokenAddBank: Audit fixes, including significant bugs

Must not take collected_fees and bump from the existing bank.

* Instruction comment updates

* GroupEdit: Add version/testing flag changes

Ported from mc/vanity
This commit is contained in:
Christian Kamm 2022-08-13 19:54:58 +02:00 committed by GitHub
parent 41e42da620
commit ccba4ee597
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 148 additions and 115 deletions

View File

@ -27,22 +27,19 @@ pub struct AccountClose<'info> {
pub fn account_close(ctx: Context<AccountClose>) -> Result<()> {
let group = ctx.accounts.group.load()?;
{
let account = ctx.accounts.account.load_mut()?;
let account = ctx.accounts.account.load_mut()?;
// don't perform checks if group is just testing
if group.testing == 0 {
require!(!account.fixed.being_liquidated(), MangoError::SomeError);
require_eq!(account.fixed.delegate, Pubkey::default());
for ele in account.token_iter() {
require_eq!(ele.is_active(), false);
}
for ele in account.serum3_iter() {
require_eq!(ele.is_active(), false);
}
for ele in account.perp_iter() {
require_eq!(ele.is_active(), false);
}
// don't perform checks if group is just testing
if !group.is_testing() {
require!(!account.fixed.being_liquidated(), MangoError::SomeError);
for ele in account.token_iter() {
require_eq!(ele.is_active(), false);
}
for ele in account.serum3_iter() {
require_eq!(ele.is_active(), false);
}
for ele in account.perp_iter() {
require_eq!(ele.is_active(), false);
}
}

View File

@ -1,6 +1,7 @@
use anchor_lang::prelude::*;
use crate::state::*;
use crate::util::checked_math as cm;
#[derive(Accounts)]
pub struct AccountExpand<'info> {
@ -32,6 +33,7 @@ pub fn account_expand(
let realloc_account = ctx.accounts.account.as_ref();
let old_space = realloc_account.data_len();
let old_lamports = realloc_account.lamports();
require_gt!(new_space, old_space);
@ -44,13 +46,11 @@ pub fn account_expand(
to: realloc_account.clone(),
},
),
new_rent_minimum
.checked_sub(realloc_account.lamports())
.unwrap(),
cm!(new_rent_minimum - old_lamports),
)?;
// realloc
realloc_account.realloc(new_space, true)?;
// realloc: it's safe to not re-zero-init since we never shrink accounts
realloc_account.realloc(new_space, false)?;
// expand dynamic content, e.g. to grow token positions, we need to slide serum3orders further later, and so on....
let mut account = ctx.accounts.account.load_mut()?;

View File

@ -26,9 +26,7 @@ pub struct GroupClose<'info> {
}
pub fn group_close(ctx: Context<GroupClose>) -> Result<()> {
// TODO: checks
// close insurance vault
// close insurance vault (must be empty)
let group = ctx.accounts.group.load()?;
let group_seeds = group_seeds!(group);
let cpi_accounts = CloseAccount {

View File

@ -46,12 +46,12 @@ pub fn group_create(
) -> Result<()> {
let mut group = ctx.accounts.group.load_init()?;
group.creator = ctx.accounts.creator.key();
group.group_num = group_num;
group.admin = ctx.accounts.creator.key();
group.fast_listing_admin = Pubkey::default();
group.insurance_vault = ctx.accounts.insurance_vault.key();
group.insurance_mint = ctx.accounts.insurance_mint.key();
group.bump = *ctx.bumps.get("group").ok_or(MangoError::SomeError)?;
group.group_num = group_num;
group.testing = testing;
group.version = version;
Ok(())

View File

@ -13,14 +13,31 @@ pub struct GroupEdit<'info> {
}
// use case - transfer group ownership to governance, where
// new_admin and new_fast_listing_admin are PDAs
// admin and fast_listing_admin are PDAs
pub fn group_edit(
ctx: Context<GroupEdit>,
new_admin: Pubkey,
new_fast_listing_admin: Pubkey,
admin_opt: Option<Pubkey>,
fast_listing_admin_opt: Option<Pubkey>,
testing_opt: Option<u8>,
version_opt: Option<u8>,
) -> Result<()> {
let mut group = ctx.accounts.group.load_mut()?;
group.admin = new_admin;
group.fast_listing_admin = new_fast_listing_admin;
if let Some(admin) = admin_opt {
group.admin = admin;
}
if let Some(fast_listing_admin) = fast_listing_admin_opt {
group.fast_listing_admin = fast_listing_admin;
}
if let Some(testing) = testing_opt {
group.testing = testing;
}
if let Some(version) = version_opt {
group.version = version;
}
Ok(())
}

View File

@ -1,6 +1,7 @@
use anchor_lang::prelude::*;
use anchor_spl::token::{Mint, Token, TokenAccount};
use crate::error::*;
use crate::state::*;
#[derive(Accounts)]
@ -44,10 +45,12 @@ pub struct TokenAddBank<'info> {
#[account(
mut,
seeds = [b"MintInfo".as_ref(), group.key().as_ref(), mint.key().as_ref()],
bump
constraint = mint_info.load()?.token_index == token_index,
has_one = group,
has_one = mint,
)]
pub mint_info: AccountLoader<'info, MintInfo>,
#[account(mut)]
pub payer: Signer<'info>,
@ -56,8 +59,6 @@ pub struct TokenAddBank<'info> {
pub rent: Sysvar<'info, Rent>,
}
// TODO: should this be "configure_mint", we pass an explicit index, and allow
// overwriting config as long as the mint account stays the same?
#[allow(clippy::too_many_arguments)]
#[allow(unused_variables)]
pub fn token_add_bank(
@ -65,11 +66,10 @@ pub fn token_add_bank(
token_index: TokenIndex,
bank_num: u32,
) -> Result<()> {
// TODO: Error if mint is already configured (technically, init of vault will fail)
let existing_bank = ctx.accounts.existing_bank.load()?;
let mut bank = ctx.accounts.bank.load_init()?;
*bank = Bank::from_existing_bank(&existing_bank, ctx.accounts.vault.key(), bank_num);
let bump = *ctx.bumps.get("bank").ok_or(MangoError::SomeError)?;
*bank = Bank::from_existing_bank(&existing_bank, ctx.accounts.vault.key(), bank_num, bump);
let mut mint_info = ctx.accounts.mint_info.load_mut()?;
let free_slot = mint_info

View File

@ -48,9 +48,6 @@ impl<'info> TokenDeposit<'info> {
}
}
// TODO: It may make sense to have the token_index passed in from the outside.
// That would save a lot of computation that needs to go into finding the
// right index for the mint.
pub fn token_deposit(ctx: Context<TokenDeposit>, amount: u64) -> Result<()> {
require_msg!(amount > 0, "deposit amount must be positive");

View File

@ -7,6 +7,10 @@ use crate::accounts_zerocopy::LoadMutZeroCopyRef;
use crate::state::*;
/// Changes a token's parameters.
///
/// In addition to these accounts, all banks must be passed as remaining_accounts
/// in MintInfo order.
#[derive(Accounts)]
#[instruction(bank_num: u64)]
pub struct TokenEdit<'info> {

View File

@ -73,8 +73,6 @@ pub struct InterestRateParams {
pub adjustment_factor: f32,
}
// TODO: should this be "configure_mint", we pass an explicit index, and allow
// overwriting config as long as the mint account stays the same?
#[allow(clippy::too_many_arguments)]
pub fn token_register(
ctx: Context<TokenRegister>,

View File

@ -17,6 +17,13 @@ pub mod compute_budget {
declare_id!("ComputeBudget111111111111111111111111111111");
}
/// Updates token interest and interest rates.
///
/// In addition to these accounts, all banks must be passed as remaining_accounts
/// in MintInfo order.
///
/// This instruction may only be used alongside other instructions of the same kind
/// or ComputeBudget instructions.
#[derive(Accounts)]
pub struct TokenUpdateIndexAndRate<'info> {
pub group: AccountLoader<'info, Group>, // Required for group metadata parsing

View File

@ -41,10 +41,18 @@ pub mod mango_v4 {
pub fn group_edit(
ctx: Context<GroupEdit>,
new_admin: Pubkey,
new_fast_listing_admin: Pubkey,
admin_opt: Option<Pubkey>,
fast_listing_admin_opt: Option<Pubkey>,
testing_opt: Option<u8>,
version_opt: Option<u8>,
) -> Result<()> {
instructions::group_edit(ctx, new_admin, new_fast_listing_admin)
instructions::group_edit(
ctx,
admin_opt,
fast_listing_admin_opt,
testing_opt,
version_opt,
)
}
pub fn group_close(ctx: Context<GroupClose>) -> Result<()> {

View File

@ -109,20 +109,36 @@ const_assert_eq!(
const_assert_eq!(size_of::<Bank>() % 8, 0);
impl Bank {
pub fn from_existing_bank(existing_bank: &Bank, vault: Pubkey, bank_num: u32) -> Self {
pub fn from_existing_bank(
existing_bank: &Bank,
vault: Pubkey,
bank_num: u32,
bump: u8,
) -> Self {
Self {
// values that must be reset/changed
vault,
indexed_deposits: I80F48::ZERO,
indexed_borrows: I80F48::ZERO,
collected_fees_native: I80F48::ZERO,
dust: I80F48::ZERO,
flash_loan_approved_amount: 0,
flash_loan_token_account_initial: u64::MAX,
bump,
bank_num,
// values that can be copied
// these are listed explicitly, so someone must make the decision when a
// new field is added!
name: existing_bank.name,
group: existing_bank.group,
mint: existing_bank.mint,
vault,
oracle: existing_bank.oracle,
oracle_config: existing_bank.oracle_config,
deposit_index: existing_bank.deposit_index,
borrow_index: existing_bank.borrow_index,
cached_indexed_total_deposits: existing_bank.cached_indexed_total_deposits,
cached_indexed_total_borrows: existing_bank.cached_indexed_total_borrows,
indexed_deposits: I80F48::ZERO,
indexed_borrows: I80F48::ZERO,
index_last_updated: existing_bank.index_last_updated,
bank_rate_last_updated: existing_bank.bank_rate_last_updated,
avg_utilization: existing_bank.avg_utilization,
@ -132,7 +148,6 @@ impl Bank {
util1: existing_bank.util1,
rate1: existing_bank.rate1,
max_rate: existing_bank.max_rate,
collected_fees_native: existing_bank.collected_fees_native,
loan_origination_fee_rate: existing_bank.loan_origination_fee_rate,
loan_fee_rate: existing_bank.loan_fee_rate,
maint_asset_weight: existing_bank.maint_asset_weight,
@ -140,14 +155,9 @@ impl Bank {
maint_liab_weight: existing_bank.maint_liab_weight,
init_liab_weight: existing_bank.init_liab_weight,
liquidation_fee: existing_bank.liquidation_fee,
dust: I80F48::ZERO,
flash_loan_approved_amount: 0,
flash_loan_token_account_initial: u64::MAX,
token_index: existing_bank.token_index,
bump: existing_bank.bump,
mint_decimals: existing_bank.mint_decimals,
reserved: [0; 2560],
bank_num,
}
}

View File

@ -956,14 +956,10 @@ impl<
}
}
// update header
let header_mut = self.header_mut();
header_mut.token_count = new_token_count;
header_mut.serum3_count = new_serum3_count;
header_mut.perp_count = new_perp_count;
header_mut.perp_oo_count = new_perp_oo_count;
// update the already-parsed header
*self.header_mut() = new_header;
// write new lengths (uses header)
// write new lengths to the dynamic data (uses header)
self.write_token_length();
self.write_serum3_length();
self.write_perp_length();

View File

@ -94,11 +94,18 @@ export class MangoClient {
public async groupEdit(
group: Group,
newAdmin: PublicKey,
newFastListingAdmin: PublicKey,
admin: PublicKey | undefined,
fastListingAdmin: PublicKey | undefined,
testing: number | undefined,
version: number | undefined,
): Promise<TransactionSignature> {
return await this.program.methods
.groupEdit(newAdmin, newFastListingAdmin)
.groupEdit(
admin ?? null,
fastListingAdmin ?? null,
testing ?? null,
version ?? null,
)
.accounts({
group: group.publicKey,
admin: (this.program.provider as AnchorProvider).wallet.publicKey,

View File

@ -110,12 +110,28 @@ export type MangoV4 = {
],
"args": [
{
"name": "newAdmin",
"type": "publicKey"
"name": "adminOpt",
"type": {
"option": "publicKey"
}
},
{
"name": "newFastListingAdmin",
"type": "publicKey"
"name": "fastListingAdminOpt",
"type": {
"option": "publicKey"
}
},
{
"name": "testingOpt",
"type": {
"option": "u8"
}
},
{
"name": "versionOpt",
"type": {
"option": "u8"
}
}
]
},
@ -646,27 +662,7 @@ export type MangoV4 = {
{
"name": "mintInfo",
"isMut": true,
"isSigner": false,
"pda": {
"seeds": [
{
"kind": "const",
"type": "string",
"value": "MintInfo"
},
{
"kind": "account",
"type": "publicKey",
"path": "group"
},
{
"kind": "account",
"type": "publicKey",
"account": "Mint",
"path": "mint"
}
]
}
"isSigner": false
},
{
"name": "payer",
@ -5150,12 +5146,28 @@ export const IDL: MangoV4 = {
],
"args": [
{
"name": "newAdmin",
"type": "publicKey"
"name": "adminOpt",
"type": {
"option": "publicKey"
}
},
{
"name": "newFastListingAdmin",
"type": "publicKey"
"name": "fastListingAdminOpt",
"type": {
"option": "publicKey"
}
},
{
"name": "testingOpt",
"type": {
"option": "u8"
}
},
{
"name": "versionOpt",
"type": {
"option": "u8"
}
}
]
},
@ -5686,27 +5698,7 @@ export const IDL: MangoV4 = {
{
"name": "mintInfo",
"isMut": true,
"isSigner": false,
"pda": {
"seeds": [
{
"kind": "const",
"type": "string",
"value": "MintInfo"
},
{
"kind": "account",
"type": "publicKey",
"path": "group"
},
{
"kind": "account",
"type": "publicKey",
"account": "Mint",
"path": "mint"
}
]
}
"isSigner": false
},
{
"name": "payer",

View File

@ -204,6 +204,8 @@ async function main() {
group,
group.admin,
new PublicKey('Efhak3qj3MiyzgJr3cUUqXXz5wr3oYHt9sPzuqJf9eBN'),
undefined,
undefined,
);
console.log(`sig https://explorer.solana.com/tx/${sig}?cluster=devnet`);
console.log(`Registering MNGO...`);

View File

@ -209,7 +209,7 @@ async function main() {
);
console.log(`Registering MNGO...`);
await client.groupEdit(group, group.admin, group.admin);
await client.groupEdit(group, group.admin, group.admin, undefined, undefined);
const mngoMainnetMint = new PublicKey(MAINNET_MINTS.get('MNGO')!);
const mngoMainnetOracle = new PublicKey(MAINNET_ORACLES.get('MNGO')!);
await client.tokenRegisterTrustless(