From d72bf5a9c63d3e158e388e47381121168dc91bb3 Mon Sep 17 00:00:00 2001 From: Binye Barwe Date: Sun, 25 Feb 2024 20:20:39 -0800 Subject: [PATCH 1/5] do not post invalid price ask if not match --- programs/openbook-v2/src/state/orderbook/book.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/programs/openbook-v2/src/state/orderbook/book.rs b/programs/openbook-v2/src/state/orderbook/book.rs index 0a5dbbf3e..3c523d412 100644 --- a/programs/openbook-v2/src/state/orderbook/book.rs +++ b/programs/openbook-v2/src/state/orderbook/book.rs @@ -174,6 +174,9 @@ impl<'a> Orderbook<'a> { let max_match_by_quote = remaining_quote_lots / best_opposing_price; if max_match_by_quote == 0 { + if best_opposing_price > price_lots && side == Side::Ask { + post_target = None; + } break; } From 1a83df52f7528ed86e68a496097deef640e1e9c2 Mon Sep 17 00:00:00 2001 From: Binye Barwe Date: Mon, 26 Feb 2024 14:59:35 -0800 Subject: [PATCH 2/5] add tests --- .../openbook-v2/src/state/orderbook/book.rs | 11 +- .../tests/cases/test_take_order.rs | 234 ++++++++++++++++++ 2 files changed, 240 insertions(+), 5 deletions(-) diff --git a/programs/openbook-v2/src/state/orderbook/book.rs b/programs/openbook-v2/src/state/orderbook/book.rs index 3c523d412..4d5e20037 100644 --- a/programs/openbook-v2/src/state/orderbook/book.rs +++ b/programs/openbook-v2/src/state/orderbook/book.rs @@ -162,21 +162,22 @@ impl<'a> Orderbook<'a> { if !side.is_price_within_limit(best_opposing_price, price_lots) { break; - } else if post_only { + } + if post_only { msg!("Order could not be placed due to PostOnly"); post_target = None; break; // return silently to not fail other instructions in tx - } else if limit == 0 { + } + if limit == 0 { msg!("Order matching limit reached"); post_target = None; break; } let max_match_by_quote = remaining_quote_lots / best_opposing_price; + // Do not post orders in the book due to bad pricing and negative spread if max_match_by_quote == 0 { - if best_opposing_price > price_lots && side == Side::Ask { - post_target = None; - } + post_target = None; break; } diff --git a/programs/openbook-v2/tests/cases/test_take_order.rs b/programs/openbook-v2/tests/cases/test_take_order.rs index 1912ddcef..356701767 100644 --- a/programs/openbook-v2/tests/cases/test_take_order.rs +++ b/programs/openbook-v2/tests/cases/test_take_order.rs @@ -288,3 +288,237 @@ async fn test_take_bid_order() -> Result<(), TransportError> { Ok(()) } + +#[tokio::test] +async fn test_negative_spread_ask() -> Result<(), TransportError> { + let context = TestContext::new().await; + let solana = &context.solana.clone(); + + let collect_fee_admin = TestKeypair::new(); + let close_market_admin = TestKeypair::new(); + let owner = context.users[0].key; + let payer = context.users[1].key; + let mints = &context.mints[0..=2]; + + let owner_token_0 = context.users[0].token_accounts[0]; + + let meta_owner_token_2 = context.users[1].token_accounts[2]; + + let tokens = Token::create(mints.to_vec(), solana, collect_fee_admin, payer).await; + + let market = TestKeypair::new(); + + let openbook_v2::accounts::CreateMarket { + market, + market_base_vault, + market_quote_vault, + event_heap, + .. + } = send_tx( + solana, + CreateMarketInstruction { + collect_fee_admin: collect_fee_admin.pubkey(), + open_orders_admin: None, + close_market_admin: Some(close_market_admin.pubkey()), + payer, + market, + quote_lot_size: 100, + base_lot_size: 1_000_000_000, + maker_fee: 0, + taker_fee: 400, + base_mint: mints[2].pubkey, + quote_mint: mints[0].pubkey, + ..CreateMarketInstruction::with_new_book_and_heap(solana, Some(tokens[1].oracle), None) + .await + }, + ) + .await + .unwrap(); + + let _indexer = create_open_orders_indexer(solana, &context.users[0], owner, market).await; + let _indexer_2 = create_open_orders_indexer(solana, &context.users[1], payer, market).await; + let maker_1 = + create_open_orders_account(solana, owner, market, 1, &context.users[0], None).await; + + let maker_2 = + create_open_orders_account(solana, payer, market, 1, &context.users[1], None).await; + + send_tx( + solana, + PlaceOrderInstruction { + open_orders_account: maker_1, + open_orders_admin: None, + market, + signer: owner, + user_token_account: owner_token_0, + market_vault: market_quote_vault, + side: Side::Bid, + price_lots: 10_000, // $1 + max_base_lots: 1000000, // wahtever + max_quote_lots_including_fees: 10_000, + client_order_id: 1, + expiry_timestamp: 0, + order_type: PlaceOrderType::Limit, + self_trade_behavior: SelfTradeBehavior::default(), + remainings: vec![], + }, + ) + .await + .unwrap(); + + // This order doesn't take any due max_quote_lots_including_fees but it's also don't post in on the book + send_tx( + solana, + PlaceOrderInstruction { + open_orders_account: maker_2, + open_orders_admin: None, + market, + signer: payer, + user_token_account: meta_owner_token_2, + market_vault: market_base_vault, + side: Side::Ask, + price_lots: 7_500, + max_base_lots: 1, + max_quote_lots_including_fees: 7_500, + client_order_id: 25, + expiry_timestamp: 0, + order_type: PlaceOrderType::Limit, + self_trade_behavior: SelfTradeBehavior::AbortTransaction, + remainings: vec![], + }, + ) + .await + .unwrap(); + + let position = solana + .get_account::(maker_2) + .await + .position; + + assert_eq!(position.asks_base_lots, 0); + assert_eq!(position.bids_base_lots, 0); + + { + let event_heap = solana.get_account_boxed::(event_heap).await; + assert_eq!(event_heap.header.count(), 0); + } + + Ok(()) +} + +#[tokio::test] +async fn test_negative_spread_bid() -> Result<(), TransportError> { + let context = TestContext::new().await; + let solana = &context.solana.clone(); + + let collect_fee_admin = TestKeypair::new(); + let close_market_admin = TestKeypair::new(); + let owner = context.users[0].key; + let payer = context.users[1].key; + let mints = &context.mints[0..=2]; + + let owner_token_1 = context.users[0].token_accounts[2]; + + let meta_owner_token_1 = context.users[1].token_accounts[0]; + + let tokens = Token::create(mints.to_vec(), solana, collect_fee_admin, payer).await; + + let market = TestKeypair::new(); + + let openbook_v2::accounts::CreateMarket { + market, + market_base_vault, + market_quote_vault, + event_heap, + .. + } = send_tx( + solana, + CreateMarketInstruction { + collect_fee_admin: collect_fee_admin.pubkey(), + open_orders_admin: None, + close_market_admin: Some(close_market_admin.pubkey()), + payer, + market, + quote_lot_size: 1_000_000_000, + base_lot_size: 100, + maker_fee: 0, + taker_fee: 400, + base_mint: mints[2].pubkey, + quote_mint: mints[0].pubkey, + ..CreateMarketInstruction::with_new_book_and_heap(solana, Some(tokens[1].oracle), None) + .await + }, + ) + .await + .unwrap(); + + let _indexer = create_open_orders_indexer(solana, &context.users[0], owner, market).await; + let _indexer_2 = create_open_orders_indexer(solana, &context.users[1], payer, market).await; + let maker_1 = + create_open_orders_account(solana, owner, market, 1, &context.users[0], None).await; + + let maker_2 = + create_open_orders_account(solana, payer, market, 1, &context.users[1], None).await; + + send_tx( + solana, + PlaceOrderInstruction { + open_orders_account: maker_1, + open_orders_admin: None, + market, + signer: owner, + user_token_account: owner_token_1, + market_vault: market_base_vault, + side: Side::Ask, + price_lots: 1000000, // whatever + max_base_lots: 10_000, // $1 + max_quote_lots_including_fees: 10_000, + client_order_id: 1, + expiry_timestamp: 0, + order_type: PlaceOrderType::Limit, + self_trade_behavior: SelfTradeBehavior::default(), + remainings: vec![], + }, + ) + .await + .unwrap(); + + // This order doesn't take any due max_base_lots but it's also don't post in on the book + send_tx( + solana, + PlaceOrderInstruction { + open_orders_account: maker_2, + open_orders_admin: None, + market, + signer: payer, + user_token_account: meta_owner_token_1, + market_vault: market_quote_vault, + side: Side::Bid, + price_lots: 7_500, + max_base_lots: 7_500, + max_quote_lots_including_fees: 1, + client_order_id: 25, + expiry_timestamp: 0, + order_type: PlaceOrderType::Limit, + self_trade_behavior: SelfTradeBehavior::AbortTransaction, + remainings: vec![], + }, + ) + .await + .unwrap(); + + let position = solana + .get_account::(maker_2) + .await + .position; + + assert_eq!(position.asks_base_lots, 0); + assert_eq!(position.bids_base_lots, 0); + + { + let event_heap = solana.get_account_boxed::(event_heap).await; + assert_eq!(event_heap.header.count(), 0); + } + + Ok(()) +} From 95c92f53d70514368102c175930635deb745c566 Mon Sep 17 00:00:00 2001 From: Binye Barwe Date: Mon, 26 Feb 2024 15:32:42 -0800 Subject: [PATCH 3/5] use testframe --- .../tests/cases/test_take_order.rs | 150 +++++------------- 1 file changed, 38 insertions(+), 112 deletions(-) diff --git a/programs/openbook-v2/tests/cases/test_take_order.rs b/programs/openbook-v2/tests/cases/test_take_order.rs index 356701767..fddbeaf5e 100644 --- a/programs/openbook-v2/tests/cases/test_take_order.rs +++ b/programs/openbook-v2/tests/cases/test_take_order.rs @@ -291,66 +291,34 @@ async fn test_take_bid_order() -> Result<(), TransportError> { #[tokio::test] async fn test_negative_spread_ask() -> Result<(), TransportError> { - let context = TestContext::new().await; - let solana = &context.solana.clone(); - - let collect_fee_admin = TestKeypair::new(); - let close_market_admin = TestKeypair::new(); - let owner = context.users[0].key; - let payer = context.users[1].key; - let mints = &context.mints[0..=2]; - - let owner_token_0 = context.users[0].token_accounts[0]; - - let meta_owner_token_2 = context.users[1].token_accounts[2]; - - let tokens = Token::create(mints.to_vec(), solana, collect_fee_admin, payer).await; - - let market = TestKeypair::new(); - - let openbook_v2::accounts::CreateMarket { + let TestInitialize { + context, + owner, + owner_token_1, market, market_base_vault, market_quote_vault, - event_heap, + account_1, + account_2, .. - } = send_tx( - solana, - CreateMarketInstruction { - collect_fee_admin: collect_fee_admin.pubkey(), - open_orders_admin: None, - close_market_admin: Some(close_market_admin.pubkey()), - payer, - market, - quote_lot_size: 100, - base_lot_size: 1_000_000_000, - maker_fee: 0, - taker_fee: 400, - base_mint: mints[2].pubkey, - quote_mint: mints[0].pubkey, - ..CreateMarketInstruction::with_new_book_and_heap(solana, Some(tokens[1].oracle), None) - .await - }, - ) - .await - .unwrap(); - - let _indexer = create_open_orders_indexer(solana, &context.users[0], owner, market).await; - let _indexer_2 = create_open_orders_indexer(solana, &context.users[1], payer, market).await; - let maker_1 = - create_open_orders_account(solana, owner, market, 1, &context.users[0], None).await; + } = TestContext::new_with_market(TestNewMarketInitialize { + quote_lot_size: 100, + base_lot_size: 1_000_000_000, + ..TestNewMarketInitialize::default() + }) + .await?; + let solana = &context.solana.clone(); - let maker_2 = - create_open_orders_account(solana, payer, market, 1, &context.users[1], None).await; + let meta_owner_token_2 = context.users[1].token_accounts[0]; send_tx( solana, PlaceOrderInstruction { - open_orders_account: maker_1, + open_orders_account: account_1, open_orders_admin: None, market, signer: owner, - user_token_account: owner_token_0, + user_token_account: owner_token_1, market_vault: market_quote_vault, side: Side::Bid, price_lots: 10_000, // $1 @@ -370,10 +338,10 @@ async fn test_negative_spread_ask() -> Result<(), TransportError> { send_tx( solana, PlaceOrderInstruction { - open_orders_account: maker_2, + open_orders_account: account_2, open_orders_admin: None, market, - signer: payer, + signer: owner, user_token_account: meta_owner_token_2, market_vault: market_base_vault, side: Side::Ask, @@ -391,83 +359,46 @@ async fn test_negative_spread_ask() -> Result<(), TransportError> { .unwrap(); let position = solana - .get_account::(maker_2) + .get_account::(account_2) .await .position; assert_eq!(position.asks_base_lots, 0); assert_eq!(position.bids_base_lots, 0); - { - let event_heap = solana.get_account_boxed::(event_heap).await; - assert_eq!(event_heap.header.count(), 0); - } - Ok(()) } #[tokio::test] async fn test_negative_spread_bid() -> Result<(), TransportError> { - let context = TestContext::new().await; - let solana = &context.solana.clone(); - - let collect_fee_admin = TestKeypair::new(); - let close_market_admin = TestKeypair::new(); - let owner = context.users[0].key; - let payer = context.users[1].key; - let mints = &context.mints[0..=2]; - - let owner_token_1 = context.users[0].token_accounts[2]; - - let meta_owner_token_1 = context.users[1].token_accounts[0]; - - let tokens = Token::create(mints.to_vec(), solana, collect_fee_admin, payer).await; - - let market = TestKeypair::new(); - - let openbook_v2::accounts::CreateMarket { + let TestInitialize { + context, + owner, + owner_token_0, market, market_base_vault, market_quote_vault, - event_heap, + account_1, + account_2, .. - } = send_tx( - solana, - CreateMarketInstruction { - collect_fee_admin: collect_fee_admin.pubkey(), - open_orders_admin: None, - close_market_admin: Some(close_market_admin.pubkey()), - payer, - market, - quote_lot_size: 1_000_000_000, - base_lot_size: 100, - maker_fee: 0, - taker_fee: 400, - base_mint: mints[2].pubkey, - quote_mint: mints[0].pubkey, - ..CreateMarketInstruction::with_new_book_and_heap(solana, Some(tokens[1].oracle), None) - .await - }, - ) - .await - .unwrap(); - - let _indexer = create_open_orders_indexer(solana, &context.users[0], owner, market).await; - let _indexer_2 = create_open_orders_indexer(solana, &context.users[1], payer, market).await; - let maker_1 = - create_open_orders_account(solana, owner, market, 1, &context.users[0], None).await; + } = TestContext::new_with_market(TestNewMarketInitialize { + quote_lot_size: 1_000_000_000, + base_lot_size: 100, + ..TestNewMarketInitialize::default() + }) + .await?; + let solana = &context.solana.clone(); - let maker_2 = - create_open_orders_account(solana, payer, market, 1, &context.users[1], None).await; + let meta_owner_token_1 = context.users[1].token_accounts[1]; send_tx( solana, PlaceOrderInstruction { - open_orders_account: maker_1, + open_orders_account: account_1, open_orders_admin: None, market, signer: owner, - user_token_account: owner_token_1, + user_token_account: owner_token_0, market_vault: market_base_vault, side: Side::Ask, price_lots: 1000000, // whatever @@ -487,10 +418,10 @@ async fn test_negative_spread_bid() -> Result<(), TransportError> { send_tx( solana, PlaceOrderInstruction { - open_orders_account: maker_2, + open_orders_account: account_2, open_orders_admin: None, market, - signer: payer, + signer: owner, user_token_account: meta_owner_token_1, market_vault: market_quote_vault, side: Side::Bid, @@ -508,17 +439,12 @@ async fn test_negative_spread_bid() -> Result<(), TransportError> { .unwrap(); let position = solana - .get_account::(maker_2) + .get_account::(account_2) .await .position; assert_eq!(position.asks_base_lots, 0); assert_eq!(position.bids_base_lots, 0); - { - let event_heap = solana.get_account_boxed::(event_heap).await; - assert_eq!(event_heap.header.count(), 0); - } - Ok(()) } From 5e7e0bc630fd339fc98568632c712324c79bb3f8 Mon Sep 17 00:00:00 2001 From: Binye Barwe Date: Mon, 26 Feb 2024 15:44:54 -0800 Subject: [PATCH 4/5] fix meta token --- programs/openbook-v2/tests/cases/test_take_order.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/programs/openbook-v2/tests/cases/test_take_order.rs b/programs/openbook-v2/tests/cases/test_take_order.rs index fddbeaf5e..8d7984b6b 100644 --- a/programs/openbook-v2/tests/cases/test_take_order.rs +++ b/programs/openbook-v2/tests/cases/test_take_order.rs @@ -294,6 +294,7 @@ async fn test_negative_spread_ask() -> Result<(), TransportError> { let TestInitialize { context, owner, + owner_token_0, owner_token_1, market, market_base_vault, @@ -309,8 +310,6 @@ async fn test_negative_spread_ask() -> Result<(), TransportError> { .await?; let solana = &context.solana.clone(); - let meta_owner_token_2 = context.users[1].token_accounts[0]; - send_tx( solana, PlaceOrderInstruction { @@ -342,7 +341,7 @@ async fn test_negative_spread_ask() -> Result<(), TransportError> { open_orders_admin: None, market, signer: owner, - user_token_account: meta_owner_token_2, + user_token_account: owner_token_0, market_vault: market_base_vault, side: Side::Ask, price_lots: 7_500, @@ -375,6 +374,7 @@ async fn test_negative_spread_bid() -> Result<(), TransportError> { context, owner, owner_token_0, + owner_token_1, market, market_base_vault, market_quote_vault, @@ -389,8 +389,6 @@ async fn test_negative_spread_bid() -> Result<(), TransportError> { .await?; let solana = &context.solana.clone(); - let meta_owner_token_1 = context.users[1].token_accounts[1]; - send_tx( solana, PlaceOrderInstruction { @@ -422,7 +420,7 @@ async fn test_negative_spread_bid() -> Result<(), TransportError> { open_orders_admin: None, market, signer: owner, - user_token_account: meta_owner_token_1, + user_token_account: owner_token_1, market_vault: market_quote_vault, side: Side::Bid, price_lots: 7_500, From 4a1a1a66f65c93706d11904049b0a5db9ad2b1e2 Mon Sep 17 00:00:00 2001 From: Binye Barwe Date: Mon, 26 Feb 2024 16:30:39 -0800 Subject: [PATCH 5/5] remove invalid test --- .../tests/cases/test_take_order.rs | 79 ------------------- 1 file changed, 79 deletions(-) diff --git a/programs/openbook-v2/tests/cases/test_take_order.rs b/programs/openbook-v2/tests/cases/test_take_order.rs index 8d7984b6b..618264e43 100644 --- a/programs/openbook-v2/tests/cases/test_take_order.rs +++ b/programs/openbook-v2/tests/cases/test_take_order.rs @@ -367,82 +367,3 @@ async fn test_negative_spread_ask() -> Result<(), TransportError> { Ok(()) } - -#[tokio::test] -async fn test_negative_spread_bid() -> Result<(), TransportError> { - let TestInitialize { - context, - owner, - owner_token_0, - owner_token_1, - market, - market_base_vault, - market_quote_vault, - account_1, - account_2, - .. - } = TestContext::new_with_market(TestNewMarketInitialize { - quote_lot_size: 1_000_000_000, - base_lot_size: 100, - ..TestNewMarketInitialize::default() - }) - .await?; - let solana = &context.solana.clone(); - - 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: 1000000, // whatever - max_base_lots: 10_000, // $1 - max_quote_lots_including_fees: 10_000, - client_order_id: 1, - expiry_timestamp: 0, - order_type: PlaceOrderType::Limit, - self_trade_behavior: SelfTradeBehavior::default(), - remainings: vec![], - }, - ) - .await - .unwrap(); - - // This order doesn't take any due max_base_lots but it's also don't post in on the book - 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: 7_500, - max_base_lots: 7_500, - max_quote_lots_including_fees: 1, - client_order_id: 25, - expiry_timestamp: 0, - order_type: PlaceOrderType::Limit, - self_trade_behavior: SelfTradeBehavior::AbortTransaction, - remainings: vec![], - }, - ) - .await - .unwrap(); - - let position = solana - .get_account::(account_2) - .await - .position; - - assert_eq!(position.asks_base_lots, 0); - assert_eq!(position.bids_base_lots, 0); - - Ok(()) -}