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.

Sign up for GitHub

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

optimize settle_funds and accrue fee rebate at time of trade instead of event queue #149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

optimize settle_funds and accrue fee rebate at time of trade instead of event queue #149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions dex/src/matching.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ impl<'ob> OrderBookState<'ob> {
pub(crate) fn process_orderbook_request(
&mut self,
request: &RequestView,
open_orders: &mut OpenOrders,
event_q: &mut EventQueue,
proceeds: &mut RequestProceeds,
limit: &mut u16,
Expand Down Expand Up @@ -104,6 +105,7 @@ impl<'ob> OrderBookState<'ob> {
client_order_id: client_order_id.map_or(0, NonZeroU64::get),
self_trade_behavior,
},
open_orders,
event_q,
proceeds,
limit,
Expand Down Expand Up @@ -205,6 +207,7 @@ impl<'ob> OrderBookState<'ob> {
&mut self,

params: NewOrderParams,
open_orders: &mut OpenOrders,
event_q: &mut EventQueue,

proceeds: &mut RequestProceeds,
Expand Down Expand Up @@ -251,6 +254,7 @@ impl<'ob> OrderBookState<'ob> {
client_order_id,
self_trade_behavior,
},
open_orders,
event_q,
proceeds,
),
Expand All @@ -269,6 +273,7 @@ impl<'ob> OrderBookState<'ob> {
client_order_id,
self_trade_behavior,
},
open_orders,
event_q,
proceeds,
)
Expand Down Expand Up @@ -306,6 +311,7 @@ impl<'ob> OrderBookState<'ob> {
fn new_ask(
&mut self,
params: NewAskParams,
open_orders: &mut OpenOrders,
event_q: &mut EventQueue,
to_release: &mut RequestProceeds,
) -> DexResult<Option<OrderRemaining>> {
Expand Down Expand Up @@ -507,6 +513,7 @@ impl<'ob> OrderBookState<'ob> {
let referrer_rebate = fees::referrer_rebate(native_taker_fee);
let net_fees = net_fees_before_referrer_rebate - referrer_rebate;

open_orders.referrer_rebates_accrued += referrer_rebate;
self.market_state.referrer_rebates_accrued += referrer_rebate;
self.market_state.pc_fees_accrued += net_fees;
self.market_state.pc_deposits_total -= net_fees_before_referrer_rebate;
Expand Down Expand Up @@ -591,6 +598,7 @@ impl<'ob> OrderBookState<'ob> {
fn new_bid(
&mut self,
params: NewBidParams,
open_orders: &mut OpenOrders,
event_q: &mut EventQueue,
to_release: &mut RequestProceeds,
) -> DexResult<Option<OrderRemaining>> {
Expand Down Expand Up @@ -825,6 +833,7 @@ impl<'ob> OrderBookState<'ob> {
let referrer_rebate = fees::referrer_rebate(native_taker_fee);
let net_fees = net_fees_before_referrer_rebate - referrer_rebate;

open_orders.referrer_rebates_accrued += referrer_rebate;
self.market_state.referrer_rebates_accrued += referrer_rebate;
self.market_state.pc_fees_accrued += net_fees;
self.market_state.pc_deposits_total -= net_fees_before_referrer_rebate;
Expand Down
58 changes: 31 additions & 27 deletions dex/src/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2245,7 +2245,7 @@ impl State {
} = args;

let native_coin_amount = open_orders.native_coin_free;
let native_pc_amount = open_orders.native_pc_free;
let mut native_pc_amount = open_orders.native_pc_free;

market.coin_deposits_total -= native_coin_amount;
market.pc_deposits_total -= native_pc_amount;
Expand All @@ -2262,6 +2262,35 @@ impl State {
.checked_sub(native_pc_amount)
.unwrap();

let nonce = market.vault_signer_nonce;
let market_pubkey = market.pubkey();
let vault_signer_seeds = gen_vault_signer_seeds(&nonce, &market_pubkey);

match referrer {
Some(referrer_pc_wallet) if open_orders.referrer_rebates_accrued > 0 => {
// If referrer is same as user, send_from_vault once
if referrer_pc_wallet.account().key == pc_wallet.account().key {
native_pc_amount = native_pc_amount
.checked_add(open_orders.referrer_rebates_accrued)
.unwrap();
} else {
send_from_vault(
open_orders.referrer_rebates_accrued,
referrer_pc_wallet.token_account(),
pc_vault.token_account(),
spl_token_program,
vault_signer,
&vault_signer_seeds,
)?;
};
}
_ => {
market.pc_fees_accrued += open_orders.referrer_rebates_accrued;
}
};
market.referrer_rebates_accrued -= open_orders.referrer_rebates_accrued;
open_orders.referrer_rebates_accrued = 0;

let token_infos: [(
u64,
account_parser::TokenAccount,
Expand All @@ -2279,10 +2308,6 @@ impl State {
),
];

let nonce = market.vault_signer_nonce;
let market_pubkey = market.pubkey();
let vault_signer_seeds = gen_vault_signer_seeds(&nonce, &market_pubkey);

for &(token_amount, wallet_account, vault) in token_infos.iter() {
send_from_vault(
token_amount,
Expand All @@ -2294,24 +2319,6 @@ impl State {
)?;
}

match referrer {
Some(referrer_pc_wallet) if open_orders.referrer_rebates_accrued > 0 => {
send_from_vault(
open_orders.referrer_rebates_accrued,
referrer_pc_wallet.token_account(),
pc_vault.token_account(),
spl_token_program,
vault_signer,
&vault_signer_seeds,
)?;
}
_ => {
market.pc_fees_accrued += open_orders.referrer_rebates_accrued;
}
};
market.referrer_rebates_accrued -= open_orders.referrer_rebates_accrued;
open_orders.referrer_rebates_accrued = 0;

Ok(())
}

Expand Down Expand Up @@ -2422,10 +2429,6 @@ impl State {
}
_ => (),
};
if !maker {
let referrer_rebate = fees::referrer_rebate(native_fee_or_rebate);
open_orders.referrer_rebates_accrued += 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.

I think that if this is deployed in a program upgrade, any rebates sitting in the event queues will be lost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right. So the total loss would be the 20% of the fee value of all trades that are on the event queue at the time this deployment goes out. My instinct is that it's a small loss because cranks usually clear out the event queue very quickly and most queues don't have trades on them, but I don't actually know the stats on this.

It simplifies the settling process for us by a lot. With the referrer rebate being granted at time of trade, we can put a SettleFunds instruction right after a NewOrder instruction and not have to wait to send a settle funds. We try to hide the settling thing as much as possible from users 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

My guesstimate is that both the UX improvement and the 20k compute units saved are worth more than the 20% of the fee value of all trades at time of the deployment.

Copy link
Contributor

Choose a reason for hiding this comment

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

20k compute units 😮 ? Spl token transfer CPI usually is ~3k units.
But even 3k units saving is good.

if let Some(client_id) = client_order_id {
debug_assert_eq!(
client_id.get(),
Expand Down Expand Up @@ -2601,6 +2604,7 @@ impl State {
let mut limit = instruction.limit;
let unfilled_portion = order_book_state.process_orderbook_request(
&request,
open_orders,
&mut event_q,
&mut proceeds,
&mut limit,
Expand Down