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

Crispheaney/guard bad limit order fills #304

Merged
merged 7 commits into from
Dec 22, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
14 changes: 13 additions & 1 deletion programs/drift/src/controller/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1731,12 +1731,24 @@ pub fn fulfill_perp_order_with_match(
let oracle_price = oracle_map.get_price_data(&market.amm.oracle)?.price;
let taker_direction = taker.orders[taker_order_index].direction;
let taker_fallback_price = get_fallback_price(&taker_direction, bid_price, ask_price);
let taker_price = taker.orders[taker_order_index].force_get_limit_price(
let mut taker_price = taker.orders[taker_order_index].force_get_limit_price(
Some(oracle_price),
Some(taker_fallback_price),
slot,
market.amm.order_tick_size,
)?;

// if the auction isn't complete, cant fill against vamm yet
// use the vamm price to guard against bad fill for taker
if taker.orders[taker_order_index].is_limit_order()
&& !taker.orders[taker_order_index].is_auction_complete(slot)?
Copy link
Member Author

Choose a reason for hiding this comment

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

should check vs min_perp_auction_duration instead of auction completeness

{
taker_price = match taker_direction {
PositionDirection::Long => taker_price.min(ask_price),
PositionDirection::Short => taker_price.max(bid_price),
};
}

let taker_existing_position = taker
.get_perp_position(market.market_index)?
.base_asset_amount;
Expand Down
34 changes: 33 additions & 1 deletion sdk/src/dlob/DLOB.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import {
UserMap,
OrderRecord,
OrderActionRecord,
isLimitOrder,
ZERO,
BN_MAX,
} from '..';
import { PublicKey } from '@solana/web3.js';
import { DLOBNode, DLOBNodeType, TriggerOrderNode } from '..';
Expand Down Expand Up @@ -1009,7 +1012,9 @@ export class DLOB {
marketIndex: number,
slot: number,
marketType: MarketType,
oraclePriceData: OraclePriceData
oraclePriceData: OraclePriceData,
fallbackAsk: BN | undefined,
fallbackBid: BN | undefined
): NodeToFill[] {
const nodesToFill = new Array<NodeToFill>();

Expand Down Expand Up @@ -1047,6 +1052,33 @@ export class DLOB {
bidNode
);

// extra guard against bad fills for perp limit orders where auction is incomplete
if (
isVariant(takerNode.order.marketType, 'perp') &&
Copy link
Member

Choose a reason for hiding this comment

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

do we really want to enforce market type here? this seems like a nice check for spot as well?

Copy link
Member

Choose a reason for hiding this comment

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

it's a bit tricky to do it on the smart contract, but we always try to fill against serum first, so i think it's ok to remove the perp check here

isLimitOrder(takerNode.order) &&
!isAuctionComplete(takerNode.order, slot)
) {
let bidPrice: BN;
let askPrice: BN;
if (isVariant(takerNode.order.direction, 'long')) {
bidPrice = BN.min(
takerNode.getPrice(oraclePriceData, slot),
fallbackAsk || BN_MAX
);
askPrice = makerNode.getPrice(oraclePriceData, slot);
} else {
bidPrice = makerNode.getPrice(oraclePriceData, slot);
askPrice = BN.max(
takerNode.getPrice(oraclePriceData, slot),
fallbackBid || ZERO
);
}

if (bidPrice.lt(askPrice)) {
continue;
}
}

const bidBaseRemaining = bidOrder.baseAssetAmount.sub(
bidOrder.baseAssetAmountFilled
);
Expand Down