Skip to content

Commit

Permalink
sort by regular price only (#108)
Browse files Browse the repository at this point in the history 
* sort by regular price only
* correctly handle reduce on partial fill
* add unit tests for matching edge cases
* document test scenarios
* fix require to run on non-solana runtimes
* new f64 conversion
* remove msg prints to not crash benchmark
* add test cases for new rounding in swap
  • Loading branch information
@mschneider
mschneider authored Sep 29, 2024
1 parent 27d5232 commit 7d675c3
Show file tree
Hide file tree
Showing 10 changed files with 614 additions and 147 deletions.
3 changes: 3 additions & 0 deletions programs/manifest/src/program/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ macro_rules! require {
if $test {
Ok(())
} else {
#[cfg(target_os = "solana")]
solana_program::msg!("[{}:{}] {}", std::file!(), std::line!(), std::format_args!($($arg)*));
#[cfg(not(target_os = "solana"))]
std::println!("[{}:{}] {}", std::file!(), std::line!(), std::format_args!($($arg)*));
Err(($err))
}
};
Expand Down
69 changes: 48 additions & 21 deletions programs/manifest/src/quantities.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use crate::program::ManifestError;
use borsh::{BorshDeserialize as Deserialize, BorshSerialize as Serialize};
use bytemuck::{Pod, Zeroable};
use hypertree::trace;
use shank::ShankAccount;
use solana_program::{msg, program_error::ProgramError};
use solana_program::program_error::ProgramError;
use static_assertions::{const_assert, const_assert_eq};
use std::{
cmp::Ordering,
Expand Down Expand Up @@ -318,16 +319,12 @@ impl QuoteAtomsPerBaseAtom {
mantissa: u32,
exponent: i8,
) -> Result<Self, PriceConversionError> {
if mantissa == 0 {
msg!("price can not be zero");
return Err(PriceConversionError(0x0));
}
if exponent > Self::MAX_EXP {
msg!("invalid exponent {exponent} > 8 would truncate",);
trace!("invalid exponent {exponent} > 8 would truncate",);
return Err(PriceConversionError(0x1));
}
if exponent < Self::MIN_EXP {
msg!("invalid exponent {exponent} < -18 would truncate",);
trace!("invalid exponent {exponent} < -18 would truncate",);
return Err(PriceConversionError(0x2));
}
Ok(Self::from_mantissa_and_exponent_(mantissa, exponent))
Expand All @@ -349,6 +346,11 @@ impl QuoteAtomsPerBaseAtom {
// this doesn't need a check, will never overflow: u64::MAX * D18 < u128::MAX
let dividend = D18.wrapping_mul(quote_atoms.inner as u128);
let inner: u128 = u64_slice_to_u128(self.inner);
trace!(
"checked_base_for_quote {dividend}/{inner} {round_up} {}>{}",
dividend.div_ceil(inner),
dividend.div(inner)
);
let base_atoms = if round_up {
dividend.div_ceil(inner)
} else {
Expand Down Expand Up @@ -464,30 +466,53 @@ impl From<PriceConversionError> for ProgramError {
}
}

#[inline(always)]
fn encode_mantissa_and_exponent(value: f64) -> (u32, i8) {
let mut exponent: i8 = 0;
// prevent overflow when casting to u32
while exponent < QuoteAtomsPerBaseAtom::MAX_EXP
&& calculate_mantissa(value, exponent) > u32::MAX as f64
{
exponent += 1;
}
// prevent underflow and maximize precision available
while exponent > QuoteAtomsPerBaseAtom::MIN_EXP
&& calculate_mantissa(value, exponent) < (u32::MAX / 10) as f64
{
exponent -= 1;
}
(calculate_mantissa(value, exponent) as u32, exponent)
}

#[inline(always)]
fn calculate_mantissa(value: f64, exp: i8) -> f64 {
(value * 10f64.powi(-exp as i32)).round()
}

impl TryFrom<f64> for QuoteAtomsPerBaseAtom {
type Error = PriceConversionError;

fn try_from(value: f64) -> Result<Self, Self::Error> {
let mantissa = value * D18F;
if mantissa.is_infinite() {
msg!("infinite can not be expressed as fixed point decimal");
if value.is_infinite() {
trace!("infinite can not be expressed as fixed point decimal");
return Err(PriceConversionError(0xC));
}
if mantissa.is_nan() {
msg!("nan can not be expressed as fixed point decimal");
if value.is_nan() {
trace!("nan can not be expressed as fixed point decimal");
return Err(PriceConversionError(0xD));
}
if mantissa > u128::MAX as f64 {
msg!("price is too large");
if value.is_sign_negative() {
trace!("price {value} can not be negative");
return Err(PriceConversionError(0xE));
}
if mantissa.is_sign_negative() {
msg!("price can not be negative");
if calculate_mantissa(value, Self::MAX_EXP) > u32::MAX as f64 {
trace!("price {value} is too large");
return Err(PriceConversionError(0xF));
}
Ok(QuoteAtomsPerBaseAtom {
inner: u128_to_u64_slice(mantissa.round() as u128),
})

let (mantissa, exponent) = encode_mantissa_and_exponent(value);

Self::try_from_mantissa_and_exponent(mantissa, exponent)
}
}

Expand Down Expand Up @@ -594,10 +619,12 @@ fn test_price_limits() {
)
.is_ok());
assert!(QuoteAtomsPerBaseAtom::try_from(0f64).is_ok());
assert!(QuoteAtomsPerBaseAtom::try_from(u64::MAX as f64).is_ok());
assert!(QuoteAtomsPerBaseAtom::try_from(
u32::MAX as f64 * 10f64.powi(QuoteAtomsPerBaseAtom::MAX_EXP as i32)
)
.is_ok());

// failures
assert!(QuoteAtomsPerBaseAtom::try_from_mantissa_and_exponent(0, 0).is_err());
assert!(QuoteAtomsPerBaseAtom::try_from_mantissa_and_exponent(
1,
QuoteAtomsPerBaseAtom::MAX_EXP + 1
Expand Down
2 changes: 1 addition & 1 deletion programs/manifest/src/state/global.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ impl<Fixed: DerefOrBorrowMut<GlobalFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
GlobalAtoms::new(
resting_order
.get_num_base_atoms()
.checked_mul(resting_order.get_price(), false)
.checked_mul(resting_order.get_price(), true)
.unwrap()
.as_u64(),
)
Expand Down
102 changes: 52 additions & 50 deletions programs/manifest/src/state/market.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ impl<Fixed: DerefOrBorrow<MarketFixed>, Dynamic: DerefOrBorrow<[u8]>>
fixed.get_quote_mint()
}

// TODO: adapt to new rounding
pub fn impact_quote_atoms(
&self,
is_bid: bool,
Expand Down Expand Up @@ -326,6 +327,7 @@ impl<Fixed: DerefOrBorrow<MarketFixed>, Dynamic: DerefOrBorrow<[u8]>>
return Ok(total_quote_atoms_matched);
}

// TODO: adapt to new rounding
pub fn impact_base_atoms(
&self,
is_bid: bool,
Expand All @@ -341,7 +343,7 @@ impl<Fixed: DerefOrBorrow<MarketFixed>, Dynamic: DerefOrBorrow<[u8]>>
self.get_bids()
};

let mut total_base_atoms_matched: BaseAtoms = BaseAtoms::ZERO;
let mut total_matched_base_atoms: BaseAtoms = BaseAtoms::ZERO;
let mut remaining_quote_atoms: QuoteAtoms = limit_quote_atoms;
for (_, other_order) in book.iter::<RestingOrder>() {
if other_order.is_expired(now_slot) {
Expand All @@ -350,11 +352,19 @@ impl<Fixed: DerefOrBorrow<MarketFixed>, Dynamic: DerefOrBorrow<[u8]>>

let matched_price: QuoteAtomsPerBaseAtom = other_order.get_price();
// caller signal can ensure quote is a lower or upper bound by rounding of base amount
let base_atoms_limit =
let price_limited_base_atoms =
matched_price.checked_base_for_quote(remaining_quote_atoms, round_up)?;
let matched_base_atoms = other_order.get_num_base_atoms().min(base_atoms_limit);
let matched_quote_atoms =
matched_price.checked_quote_for_base(matched_base_atoms, is_bid)?;
let did_fully_match_resting_order =
price_limited_base_atoms >= other_order.get_num_base_atoms();
let matched_base_atoms = if did_fully_match_resting_order {
other_order.get_num_base_atoms()
} else {
price_limited_base_atoms
};
let matched_quote_atoms = matched_price.checked_quote_for_base(
matched_base_atoms,
is_bid != did_fully_match_resting_order,
)?;

// TODO: Clean this up into a separate function.
if other_order.get_order_type() == OrderType::Global {
Expand Down Expand Up @@ -382,9 +392,9 @@ impl<Fixed: DerefOrBorrow<MarketFixed>, Dynamic: DerefOrBorrow<[u8]>>
}
}

total_base_atoms_matched = total_base_atoms_matched.checked_add(matched_base_atoms)?;
total_matched_base_atoms = total_matched_base_atoms.checked_add(matched_base_atoms)?;

if matched_base_atoms == base_atoms_limit {
if matched_base_atoms == price_limited_base_atoms {
break;
}

Expand All @@ -394,7 +404,7 @@ impl<Fixed: DerefOrBorrow<MarketFixed>, Dynamic: DerefOrBorrow<[u8]>>
}
}

return Ok(total_base_atoms_matched);
return Ok(total_matched_base_atoms);
}

pub fn get_order_by_index(&self, index: DataIndex) -> &RestingOrder {
Expand Down Expand Up @@ -617,6 +627,10 @@ impl<Fixed: DerefOrBorrowMut<MarketFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
// Got a match. First make sure we are allowed to match. We check
// inside the matching rather than skipping the matching altogether
// because post only orders should fail, not produce a crossed book.
trace!(
"match {} {order_type:?} {price:?} with {other_order:?}",
if is_bid { "bid" } else { "ask" }
);
assert_can_take(order_type)?;

let maker_sequence_number = other_order.get_sequence_number();
Expand All @@ -631,10 +645,12 @@ impl<Fixed: DerefOrBorrowMut<MarketFixed>, Dynamic: DerefOrBorrowMut<[u8]>>

let matched_price: QuoteAtomsPerBaseAtom = other_order.get_price();

// Round in favor of the maker. There is a later check that this
// rounding still results in a price that is fine for the taker.
let quote_atoms_traded: QuoteAtoms =
matched_price.checked_quote_for_base(base_atoms_traded, is_bid)?;
// on full fill: round in favor of the taker
// on partial fill: round in favor of the maker
let quote_atoms_traded: QuoteAtoms = matched_price.checked_quote_for_base(
base_atoms_traded,
is_bid != did_fully_match_resting_order,
)?;

// If it is a global order, just in time bring the funds over, or
// remove from the tree and continue on to the next order.
Expand Down Expand Up @@ -699,14 +715,14 @@ impl<Fixed: DerefOrBorrowMut<MarketFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
// These are only used when is_bid, included up here for borrow checker reasons.
let other_order: &RestingOrder =
get_helper::<RBNode<RestingOrder>>(dynamic, current_order_index).get_value();
let previous_maker_quote_atoms_allocated: QuoteAtoms = matched_price
.checked_quote_for_base(other_order.get_num_base_atoms(), false)?;
let previous_maker_quote_atoms_allocated: QuoteAtoms =
matched_price.checked_quote_for_base(other_order.get_num_base_atoms(), true)?;
let new_maker_quote_atoms_allocated: QuoteAtoms = matched_price
.checked_quote_for_base(
other_order
.get_num_base_atoms()
.checked_sub(base_atoms_traded)?,
false,
true,
)?;
update_balance(
dynamic,
Expand Down Expand Up @@ -793,23 +809,10 @@ impl<Fixed: DerefOrBorrowMut<MarketFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
remaining_base_atoms = remaining_base_atoms.checked_sub(base_atoms_traded)?;
current_order_index = next_order_index;
} else {
let mut cloned_other_order: RestingOrder =
get_helper::<RBNode<RestingOrder>>(dynamic, current_order_index)
.get_value()
.clone();
cloned_other_order.reduce(base_atoms_traded)?;
// Remove and reinsert the other order. Their effective price
// and thus their sorting in the orderbook may have changed.
remove_order_from_tree(fixed, dynamic, current_order_index, !is_bid)?;
insert_order_into_tree(
!is_bid,
fixed,
dynamic,
current_order_index,
&cloned_other_order,
);
get_mut_helper::<RBNode<RestingOrder>>(dynamic, current_order_index)
.set_payload_type(MarketDataTreeNodeType::RestingOrder as u8);
let other_order: &mut RestingOrder =
get_mut_helper::<RBNode<RestingOrder>>(dynamic, current_order_index)
.get_mut_value();
other_order.reduce(base_atoms_traded)?;
remaining_base_atoms = BaseAtoms::ZERO;
break;
}
Expand Down Expand Up @@ -888,15 +891,15 @@ impl<Fixed: DerefOrBorrowMut<MarketFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
)?;
try_to_add_to_global(&global_trade_accounts_opt.as_ref().unwrap(), &resting_order)?;
} else {
// Place the remaining. This rounds down quote atoms because it is a best
// case for the maker.
// Place the remaining.
// Rounds up quote atoms so price can be rounded in favor of taker
update_balance(
dynamic,
trader_index,
!is_bid,
false,
if is_bid {
(remaining_base_atoms.checked_mul(price, false))
(remaining_base_atoms.checked_mul(price, true))
.unwrap()
.into()
} else {
Expand Down Expand Up @@ -1130,7 +1133,7 @@ fn insert_order_into_tree(
tree.insert(free_address, *resting_order);
if is_bid {
trace!(
"insert order bid root:{}->{} max:{}->{}->{}",
"insert order bid {resting_order:?} root:{}->{} max:{}->{}->{}",
fixed.bids_root_index,
tree.get_root_index(),
fixed.bids_best_index,
Expand All @@ -1141,7 +1144,7 @@ fn insert_order_into_tree(
fixed.bids_best_index = tree.get_max_index();
} else {
trace!(
"insert order ask root:{}->{} max:{}->{}->{}",
"insert order ask {resting_order:?} root:{}->{} max:{}->{}->{}",
fixed.asks_root_index,
tree.get_root_index(),
fixed.asks_best_index,
Expand Down Expand Up @@ -1192,19 +1195,6 @@ fn remove_and_update_balances(
get_helper::<RBNode<RestingOrder>>(dynamic, order_to_remove_index).get_value();
let other_is_bid: bool = other_order.get_is_bid();

// Return the exact number of atoms if the resting order is an
// ask. If the resting order is bid, multiply by price and round
// in favor of the maker which here means down. The maker places
// the minimum number of atoms required.
let amount_atoms_to_return: u64 = if other_is_bid {
other_order
.get_price()
.checked_quote_for_base(other_order.get_num_base_atoms(), false)?
.as_u64()
} else {
other_order.get_num_base_atoms().as_u64()
};

// Global order balances are accounted for on the global accounts, not on the market.
if other_order.is_global() {
let global_trade_accounts_opt: &Option<GlobalTradeAccounts> = if other_is_bid {
Expand All @@ -1218,6 +1208,18 @@ fn remove_and_update_balances(
.trader;
remove_from_global(&global_trade_accounts_opt, maker)?;
} else {
// Return the exact number of atoms if the resting order is an
// ask. If the resting order is bid, multiply by price and round
// in favor of the taker which here means up. The maker places
// the minimum number of atoms required.
let amount_atoms_to_return: u64 = if other_is_bid {
other_order
.get_price()
.checked_quote_for_base(other_order.get_num_base_atoms(), true)?
.as_u64()
} else {
other_order.get_num_base_atoms().as_u64()
};
update_balance(
dynamic,
other_order.get_trader_index(),
Expand Down
Loading

0 comments on commit 7d675c3

Please sign in to comment.