Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send take #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ cache: cargo
env:
global:
- NODE_VERSION="v14.7.0"
- SOLANA_VERSION="v1.9.1"
- SOLANA_VERSION="v1.10.2"

_defaults: &defaults
language: rust
Expand All @@ -26,10 +26,10 @@ jobs:
name: Dex tests
script:
- ./scripts/travis/dex-tests.sh
- <<: *defaults
name: Permissioned Dex tests
script:
- cd dex/tests/permissioned/ && yarn && yarn build && yarn test && cd ../../../
# - <<: *defaults
# name: Permissioned Dex tests
# script:
# - cd dex/tests/permissioned/ && yarn && yarn build && yarn test && cd ../../../
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes in this file sound unrelated

- <<: *defaults
name: Fmt and Common Tests
script:
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions dex/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ pub enum DexErrorCode {
InvalidOpenOrdersAuthority,
OrderMaxTimestampExceeded,

MinAmountNotMet,

Unknown = 1000,

// This contains the line number in the lower 16 bits,
Expand Down
60 changes: 59 additions & 1 deletion dex/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,8 @@ pub enum MarketInstruction {
/// 8. `[writable]` coin vault
/// 9. `[writable]` pc vault
/// 10. `[]` spl token program
/// 11. `[]` (optional) the (M)SRM account used for fee discounts
/// 11. `[]` vault signer
/// 12. `[]` (optional) the (M)SRM account used for fee discounts
SendTake(SendTakeInstruction),
/// 0. `[writable]` OpenOrders
/// 1. `[signer]` the OpenOrders owner
Expand Down Expand Up @@ -1037,6 +1038,63 @@ pub fn sweep_fees(
})
}

pub fn send_take(
market: &Pubkey,
request_queue: &Pubkey,
event_queue: &Pubkey,
market_bids: &Pubkey,
market_asks: &Pubkey,
coin_wallet: &Pubkey,
pc_wallet: &Pubkey,
wallet_owner: &Pubkey,
coin_vault: &Pubkey,
pc_vault: &Pubkey,
spl_token_program_id: &Pubkey,
vault_signer: &Pubkey,
srm_account_referral: Option<&Pubkey>,
program_id: &Pubkey,
side: Side,
limit_price: NonZeroU64,
max_coin_qty: NonZeroU64,
max_native_pc_qty_including_fees: NonZeroU64,
min_coin_qty: u64,
min_native_pc_qty: u64,
limit: u16,
) -> Result<Instruction, DexError> {
let data = MarketInstruction::SendTake(SendTakeInstruction {
side,
limit_price,
max_coin_qty,
max_native_pc_qty_including_fees,
min_coin_qty,
min_native_pc_qty,
limit,
})
.pack();
let mut accounts = vec![
AccountMeta::new(*market, false),
AccountMeta::new(*request_queue, false),
AccountMeta::new(*event_queue, false),
AccountMeta::new(*market_bids, false),
AccountMeta::new(*market_asks, false),
AccountMeta::new(*coin_wallet, false),
AccountMeta::new(*pc_wallet, false),
AccountMeta::new_readonly(*wallet_owner, true),
AccountMeta::new(*coin_vault, false),
AccountMeta::new(*pc_vault, false),
AccountMeta::new_readonly(*spl_token_program_id, false),
AccountMeta::new_readonly(*vault_signer, false),
];
if let Some(key) = srm_account_referral {
accounts.push(AccountMeta::new_readonly(*key, false))
}
Ok(Instruction {
program_id: *program_id,
data,
accounts,
})
}

pub fn close_open_orders(
program_id: &Pubkey,
open_orders: &Pubkey,
Expand Down
71 changes: 47 additions & 24 deletions dex/src/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use num_enum::{IntoPrimitive, TryFromPrimitive};
use proptest_derive::Arbitrary;
use serde::{Deserialize, Serialize};
#[cfg(feature = "program")]
use solana_program::msg;
use solana_program::{msg, pubkey::Pubkey, system_program};

use crate::critbit::SlabTreeError;
use crate::error::{DexErrorCode, DexResult, SourceFileId};
Expand All @@ -16,12 +16,25 @@ use crate::{
state::{Event, EventQueue, EventView, MarketState, OpenOrders, RequestView},
};

use bytemuck::cast;

#[cfg(not(feature = "program"))]
macro_rules! msg {
($($i:expr),*) => { { ($($i),*) } };
}
declare_check_assert_macros!(SourceFileId::Matching);

pub trait ToAlignedBytes {
fn to_aligned_bytes(&self) -> [u64; 4];
}

impl ToAlignedBytes for Pubkey {
#[inline]
fn to_aligned_bytes(&self) -> [u64; 4] {
cast(self.to_bytes())
}
}

#[derive(
Eq, PartialEq, Copy, Clone, TryFromPrimitive, IntoPrimitive, Debug, Serialize, Deserialize,
)]
Expand Down Expand Up @@ -243,7 +256,7 @@ impl<'ob> OrderBookState<'ob> {
client_order_id,
self_trade_behavior,
} = params;
let (mut post_only, mut post_allowed) = match order_type {
let (mut post_only, post_allowed) = match order_type {
OrderType::Limit => (false, true),
OrderType::ImmediateOrCancel => (false, false),
OrderType::PostOnly => (true, true),
Expand All @@ -253,7 +266,9 @@ impl<'ob> OrderBookState<'ob> {
if *limit == 0 {
// Stop matching and release funds if we're out of cycles
post_only = true;
post_allowed = true;
// Remove this block of code as it can lead to undefined behavior where
// an ImmediateOrCancel order is allowed to place orders on the book
// post_allowed = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good, but unrelated

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely a good change to have though

}

let remaining_order = match side {
Expand Down Expand Up @@ -341,6 +356,8 @@ impl<'ob> OrderBookState<'ob> {
client_order_id,
self_trade_behavior,
} = params;

let is_send_take = system_program::ID.to_aligned_bytes() == owner;
let mut unfilled_qty = max_qty.get();
let mut accum_fill_price = 0;

Expand Down Expand Up @@ -504,7 +521,7 @@ impl<'ob> OrderBookState<'ob> {
to_release.credit_native_pc(net_taker_pc_qty);
to_release.debit_coin(coin_lots_traded);

if native_taker_pc_qty > 0 {
if native_taker_pc_qty > 0 && !is_send_take {
let taker_fill = Event::new(EventView::Fill {
side: Side::Ask,
maker: false,
Expand All @@ -531,6 +548,7 @@ impl<'ob> OrderBookState<'ob> {
self.market_state.pc_fees_accrued += net_fees;
self.market_state.pc_deposits_total -= net_fees_before_referrer_rebate;


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace only

if !done {
if let Some(coin_qty_remaining) = NonZeroU64::new(unfilled_qty) {
return Ok(Some(OrderRemaining {
Expand All @@ -541,6 +559,7 @@ impl<'ob> OrderBookState<'ob> {
}

if post_allowed && !crossed && unfilled_qty > 0 {
check_assert!(!is_send_take)?;
let offers = self.orders_mut(Side::Ask);
let new_order = LeafNode::new(
owner_slot,
Expand Down Expand Up @@ -572,7 +591,7 @@ impl<'ob> OrderBookState<'ob> {
} else {
insert_result.unwrap();
}
} else {
} else if !is_send_take {
to_release.unlock_coin(unfilled_qty);
let out = Event::new(EventView::Out {
side: Side::Ask,
Expand Down Expand Up @@ -631,6 +650,8 @@ impl<'ob> OrderBookState<'ob> {
check_assert!(limit_price.is_some())?;
}

let is_send_take = system_program::ID.to_aligned_bytes() == owner;

let pc_lot_size = self.market_state.pc_lot_size;
let coin_lot_size = self.market_state.coin_lot_size;

Expand Down Expand Up @@ -822,7 +843,7 @@ impl<'ob> OrderBookState<'ob> {
to_release.credit_coin(coin_lots_received);
to_release.debit_native_pc(native_pc_paid);

if native_accum_fill_price > 0 {
if native_accum_fill_price > 0 && !is_send_take {
let taker_fill = Event::new(EventView::Fill {
side: Side::Bid,
maker: false,
Expand Down Expand Up @@ -869,26 +890,28 @@ impl<'ob> OrderBookState<'ob> {
_ => (0, 0),
};

let out = {
let native_qty_still_locked = pc_qty_to_keep_locked * pc_lot_size;
let native_qty_unlocked = native_pc_qty_remaining - native_qty_still_locked;
if !is_send_take {
let out = {
let native_qty_still_locked = pc_qty_to_keep_locked * pc_lot_size;
let native_qty_unlocked = native_pc_qty_remaining - native_qty_still_locked;

to_release.unlock_native_pc(native_qty_unlocked);
to_release.unlock_native_pc(native_qty_unlocked);

Event::new(EventView::Out {
side: Side::Bid,
release_funds: false,
native_qty_unlocked,
native_qty_still_locked,
order_id,
owner,
owner_slot,
client_order_id: NonZeroU64::new(client_order_id),
})
};
event_q
.push_back(out)
.map_err(|_| DexErrorCode::EventQueueFull)?;
Event::new(EventView::Out {
side: Side::Bid,
release_funds: false,
native_qty_unlocked,
native_qty_still_locked,
order_id,
owner,
owner_slot,
client_order_id: NonZeroU64::new(client_order_id),
})
};
event_q
.push_back(out)
.map_err(|_| DexErrorCode::EventQueueFull)?;
}

if pc_qty_to_keep_locked > 0 {
let bids = self.orders_mut(Side::Bid);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other side has a check_assert!(!is_send_take)?; here to guard against putting an order on the book accidentally (even if post_allowed should just always be false). Maybe nice to assert!(!(post_allowed && send_take)) further up.

Expand Down
Loading