Skip to content

Commit

Permalink
refactor!: change rounding parameters to use defaults (#89)
Browse files Browse the repository at this point in the history
* refactor!: change rounding parameters to use defaults

This commit updates the rounding parameters in multiple modules to use `Option<Rounding>` with a default value if none is specified. This change simplifies method calls and improves code flexibility by allowing optional rounding strategies.

* Update constants.rs

* Update constants.rs

---------

Co-authored-by: malik <[email protected]>
  • Loading branch information
shuhuiluo and malik672 authored Nov 9, 2024
1 parent 8ae28ac commit 29e893d
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 105 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "uniswap-sdk-core"
version = "3.1.0"
version = "3.2.0"
edition = "2021"
authors = ["malik <[email protected]>", "Shuhui Luo <twitter.com/aureliano_law>"]
description = "The Uniswap SDK Core in Rust provides essential functionality for interacting with the Uniswap decentralized exchange"
Expand Down
3 changes: 2 additions & 1 deletion src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ pub enum TradeType {
}

/// Represents three various ways to round
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq)]
pub enum Rounding {
/// Rounds down to the nearest whole number.
RoundDown,

/// Rounds to the nearest whole number, rounding halfway cases away from zero.
#[default]
RoundHalfUp,

/// Rounds up to the nearest whole number.
Expand Down
50 changes: 20 additions & 30 deletions src/entities/fractions/currency_amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,27 +113,29 @@ impl<T: BaseCurrency> CurrencyAmount<T> {
pub fn to_significant(
&self,
significant_digits: u8,
rounding: Rounding,
rounding: Option<Rounding>,
) -> Result<String, Error> {
(self.as_fraction() / Fraction::new(self.decimal_scale.clone(), 1))
.to_significant(significant_digits, rounding)
(self.as_fraction() / Fraction::new(self.decimal_scale.clone(), 1)).to_significant(
significant_digits,
Some(rounding.unwrap_or(Rounding::RoundDown)),
)
}

/// Convert the currency amount to a string with a fixed number of decimal places
#[inline]
pub fn to_fixed(&self, decimal_places: u8, rounding: Rounding) -> Result<String, Error> {
pub fn to_fixed(
&self,
decimal_places: u8,
rounding: Option<Rounding>,
) -> Result<String, Error> {
if decimal_places > self.currency.decimals() {
return Err(Error::Invalid("DECIMALS"));
}

if decimal_places == 0 {
// Directly convert the numerator to a string for zero decimal places
return Ok(self.numerator().to_string());
}

Ok(
(self.as_fraction() / Fraction::new(self.decimal_scale.clone(), 1))
.to_fixed(decimal_places, rounding),
(self.as_fraction() / Fraction::new(self.decimal_scale.clone(), 1)).to_fixed(
decimal_places,
Some(rounding.unwrap_or(Rounding::RoundDown)),
),
)
}

Expand Down Expand Up @@ -218,50 +220,38 @@ mod tests {
#[test]
fn to_fixed_decimals_exceeds_currency_decimals() {
let amount = CurrencyAmount::from_raw_amount(TOKEN0.clone(), 1000).unwrap();
let _w = amount.to_fixed(3, Rounding::RoundDown);
let _w = amount.to_fixed(3, None);
assert!(_w.is_err(), "DECIMALS");
}

#[test]
fn to_fixed_0_decimals() {
let amount = CurrencyAmount::from_raw_amount(TOKEN0.clone(), 123456).unwrap();
assert_eq!(amount.to_fixed(0, Rounding::RoundDown).unwrap(), "123456");
assert_eq!(amount.to_fixed(0, None).unwrap(), "123456");
}

#[test]
fn to_fixed_18_decimals() {
let amount = CurrencyAmount::from_raw_amount(TOKEN18.clone(), 1e15 as i64).unwrap();
assert_eq!(
amount.to_fixed(9, Rounding::RoundDown).unwrap(),
"0.001000000"
);
assert_eq!(amount.to_fixed(9, None).unwrap(), "0.001000000");
}

#[test]
fn to_significant_does_not_throw() {
let amount = CurrencyAmount::from_raw_amount(TOKEN0.clone(), 1000).unwrap();
assert_eq!(
amount.to_significant(3, Rounding::RoundDown).unwrap(),
"1000"
);
assert_eq!(amount.to_significant(3, None).unwrap(), "1000");
}

#[test]
fn to_significant_0_decimals() {
let amount = CurrencyAmount::from_raw_amount(TOKEN0.clone(), 123456).unwrap();
assert_eq!(
amount.to_significant(4, Rounding::RoundDown).unwrap(),
"123400"
);
assert_eq!(amount.to_significant(4, None).unwrap(), "123400");
}

#[test]
fn to_significant_18_decimals() {
let amount = CurrencyAmount::from_raw_amount(TOKEN18.clone(), 1e15 as i64).unwrap();
assert_eq!(
amount.to_significant(9, Rounding::RoundDown).unwrap(),
"0.001"
);
assert_eq!(amount.to_significant(9, None).unwrap(), "0.001");
}

#[test]
Expand Down
13 changes: 8 additions & 5 deletions src/entities/fractions/fraction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,15 @@ pub trait FractionBase<M: Clone>: Sized {
/// Converts the fraction to a string with a specified number of significant digits and rounding
/// strategy
#[inline]
fn to_significant(&self, significant_digits: u8, rounding: Rounding) -> Result<String, Error> {
fn to_significant(
&self,
significant_digits: u8,
rounding: Option<Rounding>,
) -> Result<String, Error> {
if significant_digits == 0 {
return Err(Error::Invalid("SIGNIFICANT_DIGITS"));
}
let rounding_strategy = to_rounding_strategy(rounding);
let rounding_strategy = to_rounding_strategy(rounding.unwrap_or_default());
let quotient = self.to_decimal().with_precision_round(
NonZeroU64::new(significant_digits as u64).unwrap(),
rounding_strategy,
Expand All @@ -137,8 +141,8 @@ pub trait FractionBase<M: Clone>: Sized {
/// Converts the fraction to a string with a fixed number of decimal places and rounding
/// strategy
#[inline]
fn to_fixed(&self, decimal_places: u8, rounding: Rounding) -> String {
let rounding_strategy = to_rounding_strategy(rounding);
fn to_fixed(&self, decimal_places: u8, rounding: Option<Rounding>) -> String {
let rounding_strategy = to_rounding_strategy(rounding.unwrap_or_default());
self.to_decimal()
.with_scale_round(decimal_places as i64, rounding_strategy)
.to_string()
Expand All @@ -158,7 +162,6 @@ impl<M: Clone> FractionBase<M> for FractionLike<M> {
#[inline]
fn new(numerator: impl Into<BigInt>, denominator: impl Into<BigInt>, meta: M) -> Self {
let denominator = denominator.into();
// Ensure the denominator is not zero
assert!(!denominator.is_zero(), "denominator is zero");
Self {
numerator: numerator.into(),
Expand Down
14 changes: 4 additions & 10 deletions src/entities/fractions/percent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@ impl Percent {
pub fn to_significant(
&self,
significant_digits: u8,
rounding: Rounding,
rounding: Option<Rounding>,
) -> Result<String, Error> {
// Convert the Percent to a simple Fraction, multiply by 100, and then call to_significant
// on the result
(self.as_fraction() * ONE_HUNDRED.as_fraction())
.to_significant(significant_digits, rounding)
}
Expand All @@ -36,9 +34,7 @@ impl Percent {
/// strategy
#[inline]
#[must_use]
pub fn to_fixed(&self, decimal_places: u8, rounding: Rounding) -> String {
// Convert the Percent to a simple Fraction, multiply by 100, and then call to_fixed on the
// result
pub fn to_fixed(&self, decimal_places: u8, rounding: Option<Rounding>) -> String {
(self.as_fraction() * ONE_HUNDRED.as_fraction()).to_fixed(decimal_places, rounding)
}
}
Expand Down Expand Up @@ -98,17 +94,15 @@ mod tests {
#[test]
fn test_to_significant() {
assert_eq!(
Percent::new(154, 10000)
.to_significant(3, Rounding::RoundHalfUp)
.unwrap(),
Percent::new(154, 10000).to_significant(3, None).unwrap(),
"1.54".to_string()
);
}

#[test]
fn test_to_fixed() {
assert_eq!(
Percent::new(154, 10000).to_fixed(2, Rounding::RoundHalfUp),
Percent::new(154, 10000).to_fixed(2, None),
"1.54".to_string()
);
}
Expand Down
108 changes: 50 additions & 58 deletions src/entities/fractions/price.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ where
if !self.quote_currency.equals(&other.base_currency) {
return Err(Error::CurrencyMismatch);
}

let fraction = self.as_fraction() * other.as_fraction();
Ok(Price::new(
self.base_currency.clone(),
Expand Down Expand Up @@ -124,7 +123,7 @@ where
pub fn to_significant(
&self,
significant_digits: u8,
rounding: Rounding,
rounding: Option<Rounding>,
) -> Result<String, Error> {
self.adjusted_for_decimals()
.to_significant(significant_digits, rounding)
Expand All @@ -133,7 +132,7 @@ where
/// Converts the adjusted price to a string with a fixed number of decimal places and rounding
/// strategy
#[inline]
pub fn to_fixed(&self, decimal_places: u8, rounding: Rounding) -> String {
pub fn to_fixed(&self, decimal_places: u8, rounding: Option<Rounding>) -> String {
self.adjusted_for_decimals()
.to_fixed(decimal_places, rounding)
}
Expand All @@ -153,29 +152,27 @@ mod test {
static ref TOKEN1: Token = token!(1, ADDRESS_ONE, 18);
}

#[test]
fn test_constructor_array_format_works() {
let price = Price::new(TOKEN0.clone(), TOKEN1.clone(), 1, 54321);
assert_eq!(
price.to_significant(5, Rounding::RoundDown).unwrap(),
"54321"
);
assert!(price.base_currency.equals(&TOKEN0.clone()));
assert!(price.quote_currency.equals(&TOKEN1.clone()));
}
mod constructor {
use super::*;

#[test]
fn test_constructor_object_format_works() {
let price = Price::from_currency_amounts(
CurrencyAmount::from_raw_amount(TOKEN0.clone(), 1).unwrap(),
CurrencyAmount::from_raw_amount(TOKEN1.clone(), 54321).unwrap(),
);
assert_eq!(
price.to_significant(5, Rounding::RoundDown).unwrap(),
"54321"
);
assert!(price.base_currency.equals(&TOKEN0.clone()));
assert!(price.quote_currency.equals(&TOKEN1.clone()));
#[test]
fn array_format_works() {
let price = Price::new(TOKEN0.clone(), TOKEN1.clone(), 1, 54321);
assert_eq!(price.to_significant(5, None).unwrap(), "54321");
assert!(price.base_currency.equals(&TOKEN0.clone()));
assert!(price.quote_currency.equals(&TOKEN1.clone()));
}

#[test]
fn object_format_works() {
let price = Price::from_currency_amounts(
CurrencyAmount::from_raw_amount(TOKEN0.clone(), 1).unwrap(),
CurrencyAmount::from_raw_amount(TOKEN1.clone(), 54321).unwrap(),
);
assert_eq!(price.to_significant(5, None).unwrap(), "54321");
assert!(price.base_currency.equals(&TOKEN0.clone()));
assert!(price.quote_currency.equals(&TOKEN1.clone()));
}
}

#[test]
Expand All @@ -189,42 +186,37 @@ mod test {
);
}

#[test]
fn test_to_significant_no_decimals() {
let p = Price::new(TOKEN0.clone(), TOKEN1.clone(), 123, 456);
assert_eq!(p.to_significant(4, Rounding::RoundDown).unwrap(), "3.707");
}
mod to_significant {
use super::*;

#[test]
fn test_to_significant_no_decimals_flip_ratio() {
let p = Price::new(TOKEN0.clone(), TOKEN1.clone(), 456, 123);
assert_eq!(p.to_significant(4, Rounding::RoundDown).unwrap(), "0.2697");
}
#[test]
fn no_decimals() {
let p = Price::new(TOKEN0.clone(), TOKEN1.clone(), 123, 456);
assert_eq!(p.to_significant(4, None).unwrap(), "3.707");
}

#[test]
fn test_to_significant_with_decimal_difference() {
let p = Price::new(TOKEN0_6.clone(), TOKEN1.clone(), 123, 456);
assert_eq!(
p.to_significant(4, Rounding::RoundDown).unwrap(),
"3.707E-12"
);
}
#[test]
fn no_decimals_flip_ratio() {
let p = Price::new(TOKEN0.clone(), TOKEN1.clone(), 456, 123);
assert_eq!(p.to_significant(4, None).unwrap(), "0.2697");
}

#[test]
fn test_to_significant_with_decimal_difference_flipped() {
let p = Price::new(TOKEN0_6.clone(), TOKEN1.clone(), 456, 123);
assert_eq!(
p.to_significant(4, Rounding::RoundDown).unwrap(),
"2.697E-13"
);
}
#[test]
fn with_decimal_difference() {
let p = Price::new(TOKEN0_6.clone(), TOKEN1.clone(), 123, 456);
assert_eq!(p.to_significant(4, None).unwrap(), "3.707E-12");
}

#[test]
fn test_to_significant_with_decimal_difference_flipped_base_quote_flipped() {
let p = Price::new(TOKEN1.clone(), TOKEN0_6.clone(), 456, 123);
assert_eq!(
p.to_significant(4, Rounding::RoundDown).unwrap(),
"269700000000"
);
#[test]
fn with_decimal_difference_flipped() {
let p = Price::new(TOKEN0_6.clone(), TOKEN1.clone(), 456, 123);
assert_eq!(p.to_significant(4, None).unwrap(), "2.697E-13");
}

#[test]
fn with_decimal_difference_flipped_base_quote_flipped() {
let p = Price::new(TOKEN1.clone(), TOKEN0_6.clone(), 456, 123);
assert_eq!(p.to_significant(4, None).unwrap(), "269700000000");
}
}
}

0 comments on commit 29e893d

Please sign in to comment.