Fix: Replace raw u128 with layout-stable U128Bytes in zero-copy structs

.github/workflows/ci-code-review-rust.yml

@@ -77,7 +77,7 @@ jobs:
       - name: Install Solana
         uses: metadaoproject/[email protected]
         with:
-          solana-cli-version: $SOLANA_VERSION
+          solana-cli-version: ${{ env.SOLANA_VERSION }}
 
       - name: Build OpenBook V2
         run: cargo build-sbf --features enable-gpl
@@ -103,7 +103,7 @@ jobs:
       - name: Install Solana
         uses: metadaoproject/[email protected]
         with:
-          solana-cli-version: $SOLANA_VERSION
+          solana-cli-version: ${{ env.SOLANA_VERSION }}
 
       - name: Create keypair
         run: solana-keygen new --no-bip39-passphrase

lib/client/src/client.rs

@@ -566,8 +566,8 @@ impl OpenBookClient {
                         open_orders_account: self.open_orders_account,
                         signer: self.owner(),
                         open_orders_admin: market.open_orders_admin.into(),
-                        user_quote_account: user_quote_account,
-                        user_base_account: user_base_account,
+                        user_quote_account,
+                        user_base_account,
                         market: market_address,
                         bids: market.bids,
                         asks: market.asks,

programs/openbook-v2/src/instructions/cancel_order.rs

@@ -14,7 +14,7 @@ pub fn cancel_order(ctx: Context<CancelOrder>, order_id: u128) -> Result<()> {
             error_msg_typed!(OpenBookError::OpenOrdersOrderNotFound, "id = {order_id}")
         })?;
 
-    let order_id = oo.id;
+    let order_id = oo.id.to_u128();
     let order_side_and_tree = oo.side_and_tree();
 
     let market = ctx.accounts.market.load()?;

programs/openbook-v2/src/instructions/create_market.rs

@@ -86,10 +86,10 @@ pub fn create_market(
         registration_time,
         maker_fee,
         taker_fee,
-        fees_accrued: 0,
-        fees_to_referrers: 0,
-        maker_volume: 0,
-        taker_volume_wo_oo: 0,
+        fees_accrued: U128Bytes::from_u128(0),
+        fees_to_referrers: U128Bytes::from_u128(0),
+        maker_volume: U128Bytes::from_u128(0),
+        taker_volume_wo_oo: U128Bytes::from_u128(0),
         base_mint: ctx.accounts.base_mint.key(),
         quote_mint: ctx.accounts.quote_mint.key(),
         market_base_vault: ctx.accounts.market_base_vault.key(),

programs/openbook-v2/src/instructions/settle_funds.rs

@@ -21,7 +21,9 @@ pub fn settle_funds<'info>(ctx: Context<'_, '_, '_, 'info, SettleFunds<'info>>)
     let referrer_rebate = pa.referrer_rebates_available + roundoff_maker_fees;
 
     if ctx.accounts.referrer_account.is_some() {
-        market.fees_to_referrers += referrer_rebate as u128;
+        market
+            .fees_to_referrers
+            .wrapping_add_assign(referrer_rebate as u128);
         market.quote_deposit_total -= referrer_rebate;
     } else {
         market.fees_available += referrer_rebate;

programs/openbook-v2/src/state/market.rs

@@ -9,6 +9,7 @@ use crate::pubkey_option::NonZeroPubkeyOption;
 use crate::state::oracle;
 use crate::{accounts_zerocopy::KeyedAccountReader, state::orderbook::Side};
 
+use super::orderbook::nodes::U128Bytes;
 use super::{orderbook, OracleConfig};
 
 // For a 1bps taker fee, set taker_fee to 100, so taker_fee/FEES_SCALE_FACTOR = 10e-4
@@ -91,9 +92,9 @@ pub struct Market {
     pub taker_fee: i64,
 
     /// Total fees accrued in native quote
-    pub fees_accrued: u128,
+    pub fees_accrued: U128Bytes,
     /// Total fees settled in native quote
-    pub fees_to_referrers: u128,
+    pub fees_to_referrers: U128Bytes,
 
     /// Referrer rebates to be distributed
     pub referrer_rebates_accrued: u64,
@@ -102,10 +103,10 @@ pub struct Market {
     pub fees_available: u64,
 
     /// Cumulative maker volume (same as taker volume) in quote native units
-    pub maker_volume: u128,
+    pub maker_volume: U128Bytes,
 
     /// Cumulative taker volume in quote native units due to place take orders
-    pub taker_volume_wo_oo: u128,
+    pub taker_volume_wo_oo: U128Bytes,
 
     pub base_mint: Pubkey,
     pub quote_mint: Pubkey,

programs/openbook-v2/src/state/open_orders_account.rs

@@ -7,6 +7,7 @@ use crate::logs::{emit_stack, FillLog};
 use crate::pubkey_option::NonZeroPubkeyOption;
 use crate::{error::*, logs::OpenOrdersPositionLog};
 
+use super::orderbook::nodes::U128Bytes;
 use super::{BookSideOrderTree, FillEvent, LeafNode, Market, Side, SideAndOrderTree};
 
 pub const MAX_OPEN_ORDERS: usize = 24;
@@ -120,7 +121,8 @@ impl OpenOrdersAccount {
     }
 
     pub fn find_order_with_order_id(&self, order_id: u128) -> Option<&OpenOrder> {
-        self.all_orders_in_use().find(|&oo| oo.id == order_id)
+        self.all_orders_in_use()
+            .find(|&oo| oo.id.to_u128() == order_id)
     }
 
     pub fn open_order_by_raw_index(&self, raw_index: usize) -> &OpenOrder {
@@ -179,11 +181,13 @@ impl OpenOrdersAccount {
                 }
             };
 
-            pa.maker_volume += quote_native as u128;
+            pa.maker_volume.wrapping_add_assign(quote_native as u128);
             pa.referrer_rebates_available += maker_fees;
             market.referrer_rebates_accrued += maker_fees;
-            market.maker_volume += quote_native as u128;
-            market.fees_accrued += maker_fees as u128;
+            market
+                .maker_volume
+                .wrapping_add_assign(quote_native as u128);
+            market.fees_accrued.wrapping_add_assign(maker_fees as u128);
 
             if fill.maker_out() {
                 self.remove_order(fill.maker_slot as usize, fill.quantity, locked_price);
@@ -235,8 +239,8 @@ impl OpenOrdersAccount {
             quote_free_native: pa.quote_free_native,
             locked_maker_fees: pa.locked_maker_fees,
             referrer_rebates_available: pa.referrer_rebates_available,
-            maker_volume: pa.maker_volume,
-            taker_volume: pa.taker_volume,
+            maker_volume: pa.maker_volume.to_u128(),
+            taker_volume: pa.taker_volume.to_u128(),
         })
     }
 
@@ -256,7 +260,7 @@ impl OpenOrdersAccount {
             Side::Ask => pa.quote_free_native += quote_native - taker_fees,
         };
 
-        pa.taker_volume += quote_native as u128;
+        pa.taker_volume.wrapping_add_assign(quote_native as u128);
         pa.referrer_rebates_available += referrer_amount;
         market.referrer_rebates_accrued += referrer_amount;
 
@@ -271,8 +275,8 @@ impl OpenOrdersAccount {
             quote_free_native: pa.quote_free_native,
             locked_maker_fees: pa.locked_maker_fees,
             referrer_rebates_available: pa.referrer_rebates_available,
-            maker_volume: pa.maker_volume,
-            taker_volume: pa.taker_volume,
+            maker_volume: pa.maker_volume.to_u128(),
+            taker_volume: pa.taker_volume.to_u128(),
         })
     }
 
@@ -363,9 +367,9 @@ pub struct Position {
     pub penalty_heap_count: u64,
 
     /// Cumulative maker volume in quote native units (display only)
-    pub maker_volume: u128,
+    pub maker_volume: U128Bytes,
     /// Cumulative taker volume in quote native units (display only)
-    pub taker_volume: u128,
+    pub taker_volume: U128Bytes,
 
     /// Quote lots in open bids
     pub bids_quote_lots: i64,
@@ -391,8 +395,8 @@ impl Default for Position {
             locked_maker_fees: 0,
             referrer_rebates_available: 0,
             penalty_heap_count: 0,
-            maker_volume: 0,
-            taker_volume: 0,
+            maker_volume: U128Bytes::from_u128(0),
+            taker_volume: U128Bytes::from_u128(0),
             bids_quote_lots: 0,
             reserved: [0; 64],
         }
@@ -424,7 +428,7 @@ impl Position {
 #[zero_copy]
 #[derive(Debug)]
 pub struct OpenOrder {
-    pub id: u128,
+    pub id: U128Bytes,
     pub client_id: u64,
     /// Price at which user's assets were locked
     pub locked_price: i64,
@@ -444,7 +448,7 @@ impl Default for OpenOrder {
             side_and_tree: SideAndOrderTree::BidFixed.into(),
             client_id: 0,
             locked_price: 0,
-            id: 0,
+            id: U128Bytes::from_u128(0),
             padding: [0; 6],
         }
     }

programs/openbook-v2/src/state/orderbook/book.rs

@@ -149,7 +149,7 @@ impl<'a> Orderbook<'a> {
                         remaining_accs,
                     )?;
                     matched_order_deletes
-                        .push((best_opposing.handle.order_tree, best_opposing.node.key));
+                        .push((best_opposing.handle.order_tree, best_opposing.node.key()));
                 }
                 continue;
             }
@@ -197,7 +197,7 @@ impl<'a> Orderbook<'a> {
                             *market,
                         );
                         matched_order_deletes
-                            .push((best_opposing.handle.order_tree, best_opposing.node.key));
+                            .push((best_opposing.handle.order_tree, best_opposing.node.key()));
 
                         // skip actual matching
                         continue;
@@ -220,7 +220,7 @@ impl<'a> Orderbook<'a> {
             let maker_out = new_best_opposing_quantity == 0;
             if maker_out {
                 matched_order_deletes
-                    .push((best_opposing.handle.order_tree, best_opposing.node.key));
+                    .push((best_opposing.handle.order_tree, best_opposing.node.key()));
             } else {
                 matched_order_changes.push((best_opposing.handle, new_best_opposing_quantity));
             }
@@ -277,7 +277,9 @@ impl<'a> Orderbook<'a> {
 
                 // Only account taker fees now. Maker fees accounted once processing the event
                 referrer_amount = taker_fees_native - maker_rebates_acc;
-                market.fees_accrued += referrer_amount as u128;
+                market
+                    .fees_accrued
+                    .wrapping_add_assign(referrer_amount as u128);
             };
 
             if let Some(open_orders_account) = &mut open_orders_account {
@@ -290,7 +292,9 @@ impl<'a> Orderbook<'a> {
                     referrer_amount,
                 );
             } else {
-                market.taker_volume_wo_oo += total_quote_taken_native as u128;
+                market
+                    .taker_volume_wo_oo
+                    .wrapping_add_assign(total_quote_taken_native as u128);
             }
 
             let (total_quantity_paid, total_quantity_received) = match side {
@@ -526,7 +530,7 @@ impl<'a> Orderbook<'a> {
                 break;
             }
 
-            let order_id = oo.id;
+            let order_id = oo.id.to_u128();
 
             let cancel_result = self.cancel_order(
                 open_orders_account,

programs/openbook-v2/src/state/orderbook/bookside.rs

@@ -116,7 +116,7 @@ impl BookSide {
             oracle_price_lots,
         )?;
         let price = worse.price_lots;
-        let key = worse.node.key;
+        let key = worse.node.key();
         let order_tree = worse.handle.order_tree;
         let n = self.remove_by_key(order_tree, key)?;
         Some((n, price))
@@ -271,7 +271,7 @@ mod tests {
             let mut last_price = if ascending { 0 } else { i64::MAX };
             for order in bookside.iter_all_including_invalid(0, Some(oracle_price_lots)) {
                 let price = order.price_lots;
-                println!("{} {:?} {price}", order.node.key, order.handle.order_tree);
+                println!("{} {:?} {price}", order.node.key(), order.handle.order_tree);
                 if ascending {
                     assert!(price >= last_price);
                 } else {

programs/openbook-v2/src/state/orderbook/bookside_iterator.rs

@@ -152,7 +152,7 @@ pub fn rank_orders<'a>(
                 |a, b| a < b
             };
 
-            if is_better(f.1.key, key_for_fixed_price(o.1.key, o.2)) ^ return_worse {
+            if is_better(f.1.key(), key_for_fixed_price(o.1.key(), o.2)) ^ return_worse {
                 Some(fixed_to_result(f, now_ts))
             } else {
                 Some(oracle_pegged_to_result(o, now_ts))