Fix taker fee lots vs native (#251)

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

@@ -267,16 +267,16 @@ impl<'a> Orderbook<'a> {
 
         // Record the taker trade in the account already, even though it will only be
         // realized when the fill event gets executed
-        let mut taker_fees = 0_u64;
+        let mut taker_fees_native = 0_u64;
         if total_quote_lots_taken > 0 || total_base_lots_taken > 0 {
             let total_quote_taken_native_wo_self =
                 ((total_quote_lots_taken - decremented_quote_lots) * market.quote_lot_size) as u64;
 
             if total_quote_taken_native_wo_self > 0 {
-                taker_fees = market.taker_fees_ceil(total_quote_taken_native_wo_self);
+                taker_fees_native = market.taker_fees_ceil(total_quote_taken_native_wo_self);
 
                 // Only account taker fees now. Maker fees accounted once processing the event
-                referrer_amount = taker_fees - maker_rebates_acc;
+                referrer_amount = taker_fees_native - maker_rebates_acc;
                 market.fees_accrued += referrer_amount as u128;
             };
 
@@ -286,7 +286,7 @@ impl<'a> Orderbook<'a> {
                     side,
                     total_base_taken_native,
                     total_quote_taken_native,
-                    taker_fees,
+                    taker_fees_native,
                     referrer_amount,
                 );
             } else {
@@ -295,12 +295,12 @@ impl<'a> Orderbook<'a> {
 
             let (total_quantity_paid, total_quantity_received) = match side {
                 Side::Bid => (
-                    total_quote_taken_native + taker_fees,
+                    total_quote_taken_native + taker_fees_native,
                     total_base_taken_native,
                 ),
                 Side::Ask => (
                     total_base_taken_native,
-                    total_quote_taken_native - taker_fees,
+                    total_quote_taken_native - taker_fees_native,
                 ),
             };
 
@@ -309,13 +309,21 @@ impl<'a> Orderbook<'a> {
                 taker: *owner,
                 total_quantity_paid,
                 total_quantity_received,
-                fees: taker_fees,
+                fees: taker_fees_native,
             });
         }
 
+        // The native taker fees in lots, rounded up.
+        //
+        // Imagine quote_lot_size = 10. A new bid comes in with max_quote lots = 10. It matches against
+        // other orders for 5 quote lots total. The taker_fees_native is 15, taker_fees_lots is 2. That
+        // means only up the 3 quote lots may be placed on the book.
+        let taker_fees_lots =
+            (taker_fees_native as i64 + market.quote_lot_size - 1) / market.quote_lot_size;
+
         // Update remaining based on quote_lots taken. If nothing taken, same as the beginning
         remaining_quote_lots =
-            order.max_quote_lots_including_fees - total_quote_lots_taken - (taker_fees as i64);
+            order.max_quote_lots_including_fees - total_quote_lots_taken - taker_fees_lots;
 
         // Apply changes to matched asks (handles invalidate on delete!)
         for (handle, new_quantity) in matched_order_changes {
@@ -367,7 +375,7 @@ impl<'a> Orderbook<'a> {
             return err!(OpenBookError::WouldExecutePartially);
         }
 
-        let mut maker_fees = 0;
+        let mut maker_fees_native = 0;
         let mut posted_base_native = 0;
         let mut posted_quote_native = 0;
 
@@ -386,12 +394,12 @@ impl<'a> Orderbook<'a> {
 
             // Subtract maker fees in bid.
             if side == Side::Bid {
-                maker_fees = market
+                maker_fees_native = market
                     .maker_fees_ceil(posted_quote_native)
                     .try_into()
                     .unwrap();
 
-                open_orders.position.locked_maker_fees += maker_fees;
+                open_orders.position.locked_maker_fees += maker_fees_native;
             }
 
             let bookside = self.bookside_mut(side);
@@ -477,8 +485,8 @@ impl<'a> Orderbook<'a> {
             total_base_taken_native,
             total_quote_taken_native,
             referrer_amount,
-            taker_fees,
-            maker_fees,
+            taker_fees: taker_fees_native,
+            maker_fees: maker_fees_native,
         })
     }
 

programs/openbook-v2/tests/cases/test_place_order_remaining.rs

@@ -276,3 +276,104 @@ async fn test_cancel_order_yourself() -> Result<(), TransportError> {
 
     Ok(())
 }
+
+#[tokio::test]
+async fn test_place_order_taker_fees() -> Result<(), TransportError> {
+    let TestInitialize {
+        context,
+        collect_fee_admin,
+        owner,
+        owner_token_0,
+        owner_token_1,
+        market,
+        market_base_vault,
+        market_quote_vault,
+        price_lots,
+        tokens,
+        account_1,
+        account_2,
+        bids,
+        ..
+    } = TestContext::new_with_market(TestNewMarketInitialize {
+        taker_fee: 11000, // 1.1%
+        maker_fee: 0,
+        quote_lot_size: 1000,
+        base_lot_size: 1,
+        ..Default::default()
+    })
+    .await?;
+    let solana = &context.solana.clone();
+
+    // Set the initial oracle price
+    set_stub_oracle_price(solana, &tokens[1], collect_fee_admin, 1000.0).await;
+
+    send_tx(
+        solana,
+        PlaceOrderInstruction {
+            open_orders_account: account_1,
+            open_orders_admin: None,
+            market,
+            signer: owner,
+            user_token_account: owner_token_0,
+            market_vault: market_base_vault,
+            side: Side::Ask,
+            price_lots,
+            max_base_lots: 500,
+            max_quote_lots_including_fees: 500,
+            client_order_id: 0,
+            expiry_timestamp: 0,
+            order_type: PlaceOrderType::Limit,
+            self_trade_behavior: SelfTradeBehavior::default(),
+            remainings: vec![],
+        },
+    )
+    .await
+    .unwrap();
+
+    // Now place place a bid that fills the ask fully and has some remainder go to the book
+    let before_quote_balance = solana.token_account_balance(owner_token_1).await;
+    send_tx(
+        solana,
+        PlaceOrderInstruction {
+            open_orders_account: account_2,
+            open_orders_admin: None,
+            market,
+            signer: owner,
+            user_token_account: owner_token_1,
+            market_vault: market_quote_vault,
+            side: Side::Bid,
+            price_lots,
+            max_base_lots: 9999999, // unlimited
+            max_quote_lots_including_fees: 1000,
+            client_order_id: 0,
+            expiry_timestamp: 0,
+            order_type: PlaceOrderType::Limit,
+            self_trade_behavior: SelfTradeBehavior::default(),
+            remainings: vec![],
+        },
+    )
+    .await
+    .unwrap();
+    let after_quote_balance = solana.token_account_balance(owner_token_1).await;
+
+    // What should have happened is:
+    // - match against the ask, paying 500 quote lots for 500 base lots
+    // - taker fee native is 1.1% * 500 * 1000 = 5500 native
+    // - which is 5.5 quote lots, so only 500 - 6 = 494 quote lots can be placed on the book
+
+    let open_orders_account_2 = solana.get_account::<OpenOrdersAccount>(account_2).await;
+    assert_eq!(open_orders_account_2.position.bids_quote_lots, 494);
+    assert_eq!(open_orders_account_2.position.base_free_native, 500);
+
+    assert_eq!(
+        before_quote_balance - after_quote_balance,
+        // cost of buying 500 base lots
+        500 * 1000
+        // taker fee
+        + 5500
+        // order on the book
+        + 494 * 1000
+    );
+
+    Ok(())
+}