From aa23c67d300d0fb71f5e904b19684408f1870bb1 Mon Sep 17 00:00:00 2001 From: Britt Cyr Date: Wed, 9 Oct 2024 03:09:15 -0400 Subject: [PATCH] Remove dead code in global (#152) * remove dead code in global * comment * regen --------- Co-authored-by: Maximilian Schneider --- client/idl/manifest.json | 6 +- client/ts/src/manifest/types/GlobalTrader.ts | 8 +- programs/manifest/src/state/global.rs | 89 +++++--------------- programs/manifest/src/state/market.rs | 13 +-- programs/manifest/src/state/utils.rs | 9 -- 5 files changed, 33 insertions(+), 92 deletions(-) diff --git a/client/idl/manifest.json b/client/idl/manifest.json index 688f6d58c..cbe98c713 100644 --- a/client/idl/manifest.json +++ b/client/idl/manifest.json @@ -1567,15 +1567,15 @@ "type": "publicKey" }, { - "name": "unclaimedGasDeposits", + "name": "depositIndex", "type": "u32" }, { - "name": "depositIndex", + "name": "padding", "type": "u32" }, { - "name": "padding", + "name": "padding2", "type": "u64" } ] diff --git a/client/ts/src/manifest/types/GlobalTrader.ts b/client/ts/src/manifest/types/GlobalTrader.ts index c771b3446..46507dcf7 100644 --- a/client/ts/src/manifest/types/GlobalTrader.ts +++ b/client/ts/src/manifest/types/GlobalTrader.ts @@ -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; }; /** @@ -22,9 +22,9 @@ export type GlobalTrader = { export const globalTraderBeet = new beet.BeetArgsStruct( [ ['trader', beetSolana.publicKey], - ['unclaimedGasDeposits', beet.u32], ['depositIndex', beet.u32], - ['padding', beet.u64], + ['padding', beet.u32], + ['padding2', beet.u64], ], 'GlobalTrader', ); diff --git a/programs/manifest/src/state/global.rs b/programs/manifest/src/state/global.rs index 387092344..10f9a2252 100644 --- a/programs/manifest/src/state/global.rs +++ b/programs/manifest/src/state/global.rs @@ -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::{ @@ -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, @@ -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::(), GLOBAL_TRADER_SIZE); const_assert_eq!(size_of::() % 8, 0); @@ -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 } @@ -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 { @@ -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 { @@ -240,9 +227,6 @@ impl GlobalDeposit { _padding: 0, } } - pub fn get_trader(&self) -> &Pubkey { - &self.trader - } } pub type GlobalTraderTree<'a> = RedBlackTree<'a, GlobalTrader>; @@ -529,29 +513,6 @@ impl, 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(); @@ -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::>(dynamic, global_trader_index).get_mut_value(); - Ok(global_trader) -} - fn get_mut_global_deposit<'a>( fixed: &'a mut GlobalFixed, dynamic: &'a mut [u8], @@ -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); + } } diff --git a/programs/manifest/src/state/market.rs b/programs/manifest/src/state/market.rs index c31a7cf59..bf5fecae7 100644 --- a/programs/manifest/src/state/market.rs +++ b/programs/manifest/src/state/market.rs @@ -847,7 +847,7 @@ impl, 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( @@ -1051,10 +1051,7 @@ impl, Dynamic: DerefOrBorrowMut<[u8]>> } else { &global_trade_accounts_opts[0] }; - let trader: &Pubkey = &get_helper::>(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)?; } @@ -1259,11 +1256,7 @@ fn remove_and_update_balances( } else { &global_trade_accounts_opts[0] }; - let maker: &Pubkey = - &get_helper::>(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 diff --git a/programs/manifest/src/state/utils.rs b/programs/manifest/src/state/utils.rs index 2994fbf8f..bd582eef6 100644 --- a/programs/manifest/src/state/utils.rs +++ b/programs/manifest/src/state/utils.rs @@ -42,7 +42,6 @@ pub(crate) fn get_now_slot() -> u32 { pub(crate) fn remove_from_global( global_trade_accounts_opt: &Option, - global_trade_owner: &Pubkey, ) -> ProgramResult { require!( global_trade_accounts_opt.is_some(), @@ -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;