Skip to content

Commit

Permalink
Remove dead code in global (#152)
Browse files Browse the repository at this point in the history
* remove dead code in global
* comment
* regen

---------

Co-authored-by: Maximilian Schneider <[email protected]>
  • Loading branch information
brittcyr and mschneider authored Oct 9, 2024
1 parent 2e5dccc commit aa23c67
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 92 deletions.
6 changes: 3 additions & 3 deletions client/idl/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -1567,15 +1567,15 @@
"type": "publicKey"
},
{
"name": "unclaimedGasDeposits",
"name": "depositIndex",
"type": "u32"
},
{
"name": "depositIndex",
"name": "padding",
"type": "u32"
},
{
"name": "padding",
"name": "padding2",
"type": "u64"
}
]
Expand Down
8 changes: 4 additions & 4 deletions client/ts/src/manifest/types/GlobalTrader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import * as beet from '@metaplex-foundation/beet';
import * as beetSolana from '@metaplex-foundation/beet-solana';
export type GlobalTrader = {
trader: web3.PublicKey;
unclaimedGasDeposits: number;
depositIndex: number;
padding: beet.bignum;
padding: number;
padding2: beet.bignum;
};

/**
Expand All @@ -22,9 +22,9 @@ export type GlobalTrader = {
export const globalTraderBeet = new beet.BeetArgsStruct<GlobalTrader>(
[
['trader', beetSolana.publicKey],
['unclaimedGasDeposits', beet.u32],
['depositIndex', beet.u32],
['padding', beet.u64],
['padding', beet.u32],
['padding2', beet.u64],
],
'GlobalTrader',
);
89 changes: 23 additions & 66 deletions programs/manifest/src/state/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ use crate::{
program::ManifestError,
quantities::{GlobalAtoms, WrapperU64},
require,
validation::{
get_global_address, get_global_vault_address, loaders::GlobalTradeAccounts, ManifestAccount,
},
validation::{get_global_address, get_global_vault_address, ManifestAccount},
};

use super::{
Expand Down Expand Up @@ -58,6 +56,8 @@ pub struct GlobalFixed {
num_bytes_allocated: DataIndex,

vault_bump: u8,

/// Unused, but this byte wasnt being used anyways.
global_bump: u8,

num_seats_claimed: u16,
Expand Down Expand Up @@ -99,13 +99,9 @@ pub struct GlobalTrader {
/// Trader who controls this global trader.
trader: Pubkey,

// Number of gas deposits on the global account. This is the number of gas
// deposits that were paid by the global trader, but were not taken when the
// order was removed. Informational purposes only.
unclaimed_gas_deposits: u32,

deposit_index: DataIndex,
_padding: u64,
_padding: u32,
_padding2: u64,
}
const_assert_eq!(size_of::<GlobalTrader>(), GLOBAL_TRADER_SIZE);
const_assert_eq!(size_of::<GlobalTrader>() % 8, 0);
Expand Down Expand Up @@ -187,9 +183,6 @@ impl GlobalFixed {
num_seats_claimed: 0,
}
}
pub fn get_global_traders_root_index(&self) -> DataIndex {
self.global_traders_root_index
}
pub fn get_mint(&self) -> &Pubkey {
&self.mint
}
Expand All @@ -199,9 +192,6 @@ impl GlobalFixed {
pub fn get_vault_bump(&self) -> u8 {
self.vault_bump
}
pub fn get_global_bump(&self) -> u8 {
self.global_bump
}
}

impl ManifestAccount for GlobalFixed {
Expand All @@ -222,14 +212,11 @@ impl GlobalTrader {
pub fn new_empty(trader: &Pubkey, deposit_index: DataIndex) -> Self {
GlobalTrader {
trader: *trader,
unclaimed_gas_deposits: 0,
deposit_index,
_padding: 0,
_padding2: 0,
}
}
pub fn get_trader(&self) -> &Pubkey {
&self.trader
}
}

impl GlobalDeposit {
Expand All @@ -240,9 +227,6 @@ impl GlobalDeposit {
_padding: 0,
}
}
pub fn get_trader(&self) -> &Pubkey {
&self.trader
}
}

pub type GlobalTraderTree<'a> = RedBlackTree<'a, GlobalTrader>;
Expand Down Expand Up @@ -529,29 +513,6 @@ impl<Fixed: DerefOrBorrowMut<GlobalFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
Ok(())
}

/// Remove global order. Update the GlobalTrader.
pub fn remove_order(
&mut self,
global_trade_owner: &Pubkey,
global_trade_accounts: &GlobalTradeAccounts,
) -> ProgramResult {
let DynamicAccount { fixed, dynamic } = self.borrow_mut_global();
// Might not exist because of eviction.
if let Ok(global_trader) = get_mut_global_trader(fixed, dynamic, global_trade_owner) {
let GlobalTradeAccounts {
gas_receiver_opt: trader,
..
} = global_trade_accounts;
if trader.as_ref().unwrap().info.key != global_trade_owner
|| global_trade_accounts.system_program.is_none()
{
global_trader.unclaimed_gas_deposits += 1;
}
}

Ok(())
}

/// Deposit to global account.
pub fn deposit_global(&mut self, trader: &Pubkey, num_atoms: GlobalAtoms) -> ProgramResult {
let DynamicAccount { fixed, dynamic } = self.borrow_mut_global();
Expand Down Expand Up @@ -613,25 +574,6 @@ fn get_global_trader<'a>(
Some(global_trader)
}

fn get_mut_global_trader<'a>(
fixed: &'a mut GlobalFixed,
dynamic: &'a mut [u8],
trader: &'a Pubkey,
) -> Result<&'a mut GlobalTrader, ProgramError> {
let global_trader_tree: GlobalTraderTree =
GlobalTraderTree::new(dynamic, fixed.global_traders_root_index, NIL);
let global_trader_index: DataIndex =
global_trader_tree.lookup_index(&GlobalTrader::new_empty(trader, NIL));
require!(
global_trader_index != NIL,
ManifestError::MissingGlobal,
"Could not find global trader",
)?;
let global_trader: &mut GlobalTrader =
get_mut_helper::<RBNode<GlobalTrader>>(dynamic, global_trader_index).get_mut_value();
Ok(global_trader)
}

fn get_mut_global_deposit<'a>(
fixed: &'a mut GlobalFixed,
dynamic: &'a mut [u8],
Expand Down Expand Up @@ -673,16 +615,31 @@ mod test {
use super::*;

#[test]
fn test_display() {
fn test_display_trader() {
format!("{}", GlobalTrader::default());
}

#[test]
fn test_cmp() {
fn test_cmp_trader() {
// Just use token program ids since those have known sort order.
let global_trader1: GlobalTrader = GlobalTrader::new_empty(&spl_token::id(), NIL);
let global_trader2: GlobalTrader = GlobalTrader::new_empty(&spl_token_2022::id(), NIL);
assert!(global_trader1 < global_trader2);
assert!(global_trader1 != global_trader2);
}

#[test]
fn test_display_deposit() {
format!("{}", GlobalDeposit::default());
}

#[test]
fn test_cmp_deposit() {
let global_deposit1: GlobalDeposit = GlobalDeposit::new_empty(&Pubkey::new_unique());
let mut global_deposit2: GlobalDeposit = GlobalDeposit::new_empty(&Pubkey::new_unique());
global_deposit2.balance_atoms = GlobalAtoms::new(1);
// Reversed order than expected because Hypertrees give max pointer, but we want a min balance.
assert!(global_deposit1 > global_deposit2);
assert!(global_deposit1 != global_deposit2);
}
}
13 changes: 3 additions & 10 deletions programs/manifest/src/state/market.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ impl<Fixed: DerefOrBorrowMut<MarketFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
} else {
&global_trade_accounts_opts[1]
};
remove_from_global(&global_trade_accounts_opt, &maker)?;
remove_from_global(&global_trade_accounts_opt)?;
}

let next_maker_order_index: DataIndex = get_next_candidate_match_index(
Expand Down Expand Up @@ -1051,10 +1051,7 @@ impl<Fixed: DerefOrBorrowMut<MarketFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
} else {
&global_trade_accounts_opts[0]
};
let trader: &Pubkey = &get_helper::<RBNode<ClaimedSeat>>(dynamic, trader_index)
.get_value()
.trader;
remove_from_global(&global_trade_accounts_opt, trader)?
remove_from_global(&global_trade_accounts_opt)?
} else {
update_balance(dynamic, trader_index, !is_bid, true, amount_atoms)?;
}
Expand Down Expand Up @@ -1259,11 +1256,7 @@ fn remove_and_update_balances(
} else {
&global_trade_accounts_opts[0]
};
let maker: &Pubkey =
&get_helper::<RBNode<ClaimedSeat>>(dynamic, resting_order_to_remove.get_trader_index())
.get_value()
.trader;
remove_from_global(&global_trade_accounts_opt, maker)?;
remove_from_global(&global_trade_accounts_opt)?;
} else {
// Return the exact number of atoms if the resting order is an
// ask. If the resting order is bid, multiply by price and round
Expand Down
9 changes: 0 additions & 9 deletions programs/manifest/src/state/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ pub(crate) fn get_now_slot() -> u32 {

pub(crate) fn remove_from_global(
global_trade_accounts_opt: &Option<GlobalTradeAccounts>,
global_trade_owner: &Pubkey,
) -> ProgramResult {
require!(
global_trade_accounts_opt.is_some(),
Expand All @@ -56,14 +55,6 @@ pub(crate) fn remove_from_global(
..
} = global_trade_accounts;

// This check is a hack since the global data is borrowed in cleaning, so
// avoid reborrowing.
if global_trade_accounts.global_vault_opt.is_some() {
let global_data: &mut RefMut<&mut [u8]> = &mut global.try_borrow_mut_data()?;
let mut global_dynamic_account: GlobalRefMut = get_mut_dynamic_account(global_data);
global_dynamic_account.remove_order(global_trade_owner, global_trade_accounts)?;
}

// The simple implementation gets
//
// **receiver.lamports.borrow_mut() += GAS_DEPOSIT_LAMPORTS;
Expand Down

0 comments on commit aa23c67

Please sign in to comment.