From 29e893d2fa979e032ef26c143700cf282f55c2ba Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Sat, 9 Nov 2024 05:18:19 -0500 Subject: [PATCH] refactor!: change rounding parameters to use defaults (#89) * refactor!: change rounding parameters to use defaults This commit updates the rounding parameters in multiple modules to use `Option` 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 --- Cargo.toml | 2 +- src/constants.rs | 3 +- src/entities/fractions/currency_amount.rs | 50 ++++------ src/entities/fractions/fraction.rs | 13 ++- src/entities/fractions/percent.rs | 14 +-- src/entities/fractions/price.rs | 108 ++++++++++------------ 6 files changed, 85 insertions(+), 105 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 84bcafc..e704554 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "uniswap-sdk-core" -version = "3.1.0" +version = "3.2.0" edition = "2021" authors = ["malik ", "Shuhui Luo "] description = "The Uniswap SDK Core in Rust provides essential functionality for interacting with the Uniswap decentralized exchange" diff --git a/src/constants.rs b/src/constants.rs index 75a8988..5954b1c 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -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. diff --git a/src/entities/fractions/currency_amount.rs b/src/entities/fractions/currency_amount.rs index 141f00c..995942f 100644 --- a/src/entities/fractions/currency_amount.rs +++ b/src/entities/fractions/currency_amount.rs @@ -113,27 +113,29 @@ impl CurrencyAmount { pub fn to_significant( &self, significant_digits: u8, - rounding: Rounding, + rounding: Option, ) -> Result { - (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 { + pub fn to_fixed( + &self, + decimal_places: u8, + rounding: Option, + ) -> Result { 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)), + ), ) } @@ -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] diff --git a/src/entities/fractions/fraction.rs b/src/entities/fractions/fraction.rs index 24508ba..843d798 100644 --- a/src/entities/fractions/fraction.rs +++ b/src/entities/fractions/fraction.rs @@ -121,11 +121,15 @@ pub trait FractionBase: 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 { + fn to_significant( + &self, + significant_digits: u8, + rounding: Option, + ) -> Result { 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, @@ -137,8 +141,8 @@ pub trait FractionBase: 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) -> 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() @@ -158,7 +162,6 @@ impl FractionBase for FractionLike { #[inline] fn new(numerator: impl Into, denominator: impl Into, meta: M) -> Self { let denominator = denominator.into(); - // Ensure the denominator is not zero assert!(!denominator.is_zero(), "denominator is zero"); Self { numerator: numerator.into(), diff --git a/src/entities/fractions/percent.rs b/src/entities/fractions/percent.rs index 3cbbdab..dc4a808 100644 --- a/src/entities/fractions/percent.rs +++ b/src/entities/fractions/percent.rs @@ -24,10 +24,8 @@ impl Percent { pub fn to_significant( &self, significant_digits: u8, - rounding: Rounding, + rounding: Option, ) -> Result { - // 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) } @@ -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) -> String { (self.as_fraction() * ONE_HUNDRED.as_fraction()).to_fixed(decimal_places, rounding) } } @@ -98,9 +94,7 @@ 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() ); } @@ -108,7 +102,7 @@ mod tests { #[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() ); } diff --git a/src/entities/fractions/price.rs b/src/entities/fractions/price.rs index fce0a50..6e10c69 100644 --- a/src/entities/fractions/price.rs +++ b/src/entities/fractions/price.rs @@ -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(), @@ -124,7 +123,7 @@ where pub fn to_significant( &self, significant_digits: u8, - rounding: Rounding, + rounding: Option, ) -> Result { self.adjusted_for_decimals() .to_significant(significant_digits, rounding) @@ -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) -> String { self.adjusted_for_decimals() .to_fixed(decimal_places, rounding) } @@ -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] @@ -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"); + } } }