From eccfda3ea70c7ef528773fe4b88ee523c9646def Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Thu, 12 Sep 2024 22:53:09 -0400 Subject: [PATCH 1/3] fix(clippy): Add #[inline] annotations to various functions Inline annotations were added to functions across multiple modules to suggest the compiler to replace function calls with the function's body. This could potentially optimize runtime performance by reducing the overhead of function calls. --- src/entities/native_currency.rs | 2 ++ src/entities/weth9.rs | 1 + src/lib.rs | 11 +++++++++++ src/utils/sorted_insert.rs | 1 + src/utils/sqrt.rs | 1 + src/utils/validate_and_parse_address.rs | 2 ++ 6 files changed, 18 insertions(+) diff --git a/src/entities/native_currency.rs b/src/entities/native_currency.rs index 3691e05..58a89e6 100644 --- a/src/entities/native_currency.rs +++ b/src/entities/native_currency.rs @@ -2,10 +2,12 @@ use crate::prelude::*; /// Represents the native currency of the chain on which it resides pub trait NativeCurrency: Currency { + #[inline] fn is_native(&self) -> bool { true } + #[inline] fn is_token(&self) -> bool { false } diff --git a/src/entities/weth9.rs b/src/entities/weth9.rs index bca6b31..1200e41 100644 --- a/src/entities/weth9.rs +++ b/src/entities/weth9.rs @@ -12,6 +12,7 @@ pub struct WETH9 { /// Default implementation for [`WETH9`], creating an instance with predefined WETH tokens on /// various chains. impl Default for WETH9 { + #[inline] fn default() -> Self { Self::new() } diff --git a/src/lib.rs b/src/lib.rs index c9c17e0..d10bac5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,12 +8,23 @@ missing_debug_implementations, unreachable_pub, clippy::missing_const_for_fn, + clippy::missing_inline_in_public_items, + // clippy::needless_pass_by_value, clippy::redundant_clone, + // clippy::explicit_iter_loop, + // clippy::manual_assert, + // clippy::must_use_candidate, + // clippy::semicolon_if_nothing_returned, + // clippy::suspicious_operation_groupings, + // clippy::unseparated_literal_suffix, + // clippy::unused_self, + // clippy::use_debug, rustdoc::all )] #![cfg_attr(not(test), warn(unused_crate_dependencies))] #![deny(unused_must_use, rust_2018_idioms)] #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] + extern crate alloc; /// Contains functionality related to All Contracts deployed and supported by the Uniswap SDK. diff --git a/src/utils/sorted_insert.rs b/src/utils/sorted_insert.rs index d3e19ec..de386d1 100644 --- a/src/utils/sorted_insert.rs +++ b/src/utils/sorted_insert.rs @@ -2,6 +2,7 @@ use crate::prelude::*; /// Given an array of items sorted by `comparator`, insert an item into its sort index and constrain /// the size to `maxSize` by removing the last item +#[inline] pub fn sorted_insert( items: &mut Vec, add: T, diff --git a/src/utils/sqrt.rs b/src/utils/sqrt.rs index c9b2f08..c54a58e 100644 --- a/src/utils/sqrt.rs +++ b/src/utils/sqrt.rs @@ -8,6 +8,7 @@ use num_traits::Signed; /// * `value`: the value for which to compute the square root, rounded down /// /// returns: BigInt +#[inline] pub fn sqrt(value: &BigInt) -> Result { if value.is_negative() { Err(Error::Invalid) diff --git a/src/utils/validate_and_parse_address.rs b/src/utils/validate_and_parse_address.rs index 1099451..693822e 100644 --- a/src/utils/validate_and_parse_address.rs +++ b/src/utils/validate_and_parse_address.rs @@ -12,6 +12,7 @@ use regex::Regex; /// with only hexadecimal characters after `0x`, returns `Ok(ethereum_address.to_string())`. /// * Otherwise, returns an error message in the form of `Err(format!("{} is not a valid Ethereum /// address.", ethereum_address))`. +#[inline] pub fn check_valid_ethereum_address(ethereum_address: &str) -> Result<&str, String> { let valid_address_regex = Regex::new(r"^0x[0-9a-fA-F]{40}$").unwrap(); if valid_address_regex.is_match(ethereum_address) { @@ -37,6 +38,7 @@ pub fn check_valid_ethereum_address(ethereum_address: &str) -> Result<&str, Stri /// with only hexadecimal characters after `0x`, returns the checksummed address. /// * Otherwise, returns an error message in the form of `Err(format!("{} is not a valid Ethereum /// address.", ethereum_address))`. +#[inline] pub fn validate_and_parse_address(ethereum_address: &str) -> Result { let checksummed_address = eth_checksum::checksum(ethereum_address); check_valid_ethereum_address(&checksummed_address)?; From 41b9fd1ed9e449b8fc9ef0356250a15a22d5a871 Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Thu, 12 Sep 2024 22:58:22 -0400 Subject: [PATCH 2/3] fix(clippy): Enable additional Clippy lints and refactor assertions Re-enable several Clippy lints for cleaner code validation. Refactor `panic!` calls to `assert!` for better consistency and clarity. Update function arguments to use references where appropriate. --- src/entities/fractions/currency_amount.rs | 2 +- src/entities/fractions/fraction.rs | 4 +--- src/entities/token.rs | 4 +--- src/lib.rs | 11 +++-------- src/utils/compute_price_impact.rs | 8 ++++---- 5 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/entities/fractions/currency_amount.rs b/src/entities/fractions/currency_amount.rs index b8c5504..31122fc 100644 --- a/src/entities/fractions/currency_amount.rs +++ b/src/entities/fractions/currency_amount.rs @@ -33,7 +33,7 @@ impl CurrencyAmount { denominator, CurrencyMeta { currency, - decimal_scale: BigUint::from(10u64).pow(exponent as u32), + decimal_scale: BigUint::from(10_u64).pow(exponent as u32), }, )) } diff --git a/src/entities/fractions/fraction.rs b/src/entities/fractions/fraction.rs index a1e3f8e..83460e8 100644 --- a/src/entities/fractions/fraction.rs +++ b/src/entities/fractions/fraction.rs @@ -167,9 +167,7 @@ impl FractionBase for FractionLike { fn new(numerator: impl Into, denominator: impl Into, meta: M) -> Self { let denominator = denominator.into(); // Ensure the denominator is not zero - if denominator.is_zero() { - panic!("denominator is zero"); - } + assert!(!denominator.is_zero(), "denominator is zero"); Self { numerator: numerator.into(), denominator, diff --git a/src/entities/token.rs b/src/entities/token.rs index bc3e618..91c2ebe 100644 --- a/src/entities/token.rs +++ b/src/entities/token.rs @@ -69,9 +69,7 @@ impl Token { buy_fee_bps: Option, sell_fee_bps: Option, ) -> Self { - if chain_id == 0 { - panic!("chain id can't be zero"); - } + assert!(chain_id != 0, "chain id can't be zero"); Self { chain_id, decimals, diff --git a/src/lib.rs b/src/lib.rs index d10bac5..c0a3265 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,16 +9,11 @@ unreachable_pub, clippy::missing_const_for_fn, clippy::missing_inline_in_public_items, - // clippy::needless_pass_by_value, + clippy::needless_pass_by_value, clippy::redundant_clone, - // clippy::explicit_iter_loop, - // clippy::manual_assert, + clippy::manual_assert, // clippy::must_use_candidate, - // clippy::semicolon_if_nothing_returned, - // clippy::suspicious_operation_groupings, - // clippy::unseparated_literal_suffix, - // clippy::unused_self, - // clippy::use_debug, + clippy::unseparated_literal_suffix, rustdoc::all )] #![cfg_attr(not(test), warn(unused_crate_dependencies))] diff --git a/src/utils/compute_price_impact.rs b/src/utils/compute_price_impact.rs index 2587eca..d22531b 100644 --- a/src/utils/compute_price_impact.rs +++ b/src/utils/compute_price_impact.rs @@ -11,7 +11,7 @@ use crate::prelude::*; /// returns: Percent #[inline] pub fn compute_price_impact( - mid_price: Price, + mid_price: &Price, input_amount: &CurrencyAmount, output_amount: &CurrencyAmount, ) -> Result { @@ -42,7 +42,7 @@ mod tests { //is correct for zero assert!( compute_price_impact( - Price::new(Ether::on_chain(1), token.clone(), 10, 100), + &Price::new(Ether::on_chain(1), token.clone(), 10, 100), &CurrencyAmount::from_raw_amount(Ether::on_chain(1), 10).unwrap(), &CurrencyAmount::from_raw_amount(token.clone(), 100).unwrap() ) @@ -53,7 +53,7 @@ mod tests { //is correct for half output assert!( compute_price_impact( - Price::new(token.clone(), token_1.clone(), 10, 100), + &Price::new(token.clone(), token_1.clone(), 10, 100), &CurrencyAmount::from_raw_amount(token.clone(), 10).unwrap(), &CurrencyAmount::from_raw_amount(token_1.clone(), 50).unwrap() ) @@ -64,7 +64,7 @@ mod tests { //is negative for more output assert_eq!( compute_price_impact( - Price::new(token.clone(), token_1.clone(), 10, 100), + &Price::new(token.clone(), token_1.clone(), 10, 100), &CurrencyAmount::from_raw_amount(token.clone(), 10).unwrap(), &CurrencyAmount::from_raw_amount(token_1.clone(), 200).unwrap() ) From 6320be582c96f9234b8ad93c02abb605ee8014e0 Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Thu, 12 Sep 2024 23:01:55 -0400 Subject: [PATCH 3/3] fix(clippy): Enable `clippy::must_use_candidate` lint Uncommented the `clippy::must_use_candidate` lint and annotated essential functions with `#[must_use]` attribute. This increases code reliability by enforcing that critical return values are not ignored. --- src/entities/ether.rs | 2 ++ src/entities/fractions/percent.rs | 1 + src/entities/token.rs | 1 + src/entities/weth9.rs | 3 +++ src/lib.rs | 3 ++- src/utils/compute_price_impact.rs | 4 ++-- 6 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/entities/ether.rs b/src/entities/ether.rs index 5549810..f064a77 100644 --- a/src/entities/ether.rs +++ b/src/entities/ether.rs @@ -32,6 +32,7 @@ impl Currency for Ether { impl Ether { /// Creates a new instance of [`Ether`] with the specified chain ID. #[inline] + #[must_use] pub fn new(chain_id: u64) -> Self { Self { chain_id, @@ -44,6 +45,7 @@ impl Ether { /// Retrieves or creates an [`Ether`] instance for the specified chain ID. #[inline] + #[must_use] pub fn on_chain(chain_id: u64) -> Self { Self::new(chain_id) } diff --git a/src/entities/fractions/percent.rs b/src/entities/fractions/percent.rs index 00e7cee..3cbbdab 100644 --- a/src/entities/fractions/percent.rs +++ b/src/entities/fractions/percent.rs @@ -35,6 +35,7 @@ impl Percent { /// Converts the [`Percent`] to a string with a fixed number of decimal places and rounding /// 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 diff --git a/src/entities/token.rs b/src/entities/token.rs index 91c2ebe..d9aee14 100644 --- a/src/entities/token.rs +++ b/src/entities/token.rs @@ -60,6 +60,7 @@ impl Token { /// /// Panics if `chain_id` is 0. #[inline] + #[must_use] pub const fn new( chain_id: u64, address: Address, diff --git a/src/entities/weth9.rs b/src/entities/weth9.rs index 1200e41..dd5f7ca 100644 --- a/src/entities/weth9.rs +++ b/src/entities/weth9.rs @@ -29,6 +29,7 @@ impl WETH9 { /// /// A new `WETH9` instance with predefined WETH tokens. #[inline] + #[must_use] pub fn new() -> Self { let tokens = FxHashMap::from_iter(vec![ (1, Self::on_chain(1).unwrap()), @@ -56,6 +57,7 @@ impl WETH9 { /// /// Returns: `Some(Token)` if the token exists, `None` otherwise. #[inline] + #[must_use] pub fn on_chain(chain_id: u64) -> Option { match chain_id { 1 => Some(token!( @@ -161,6 +163,7 @@ impl WETH9 { /// /// Returns: `Some(Token)` if the token exists, `None` otherwise. #[inline] + #[must_use] pub fn get(&self, chain_id: u64) -> Option<&Token> { self.tokens.get(&chain_id) } diff --git a/src/lib.rs b/src/lib.rs index c0a3265..21ff8e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ //! //! The Uniswap SDK Core in Rust provides essential functionality for interacting with the Uniswap //! decentralized exchange. + #![cfg_attr(not(any(feature = "std", test)), no_std)] #![warn( missing_copy_implementations, @@ -12,7 +13,7 @@ clippy::needless_pass_by_value, clippy::redundant_clone, clippy::manual_assert, - // clippy::must_use_candidate, + clippy::must_use_candidate, clippy::unseparated_literal_suffix, rustdoc::all )] diff --git a/src/utils/compute_price_impact.rs b/src/utils/compute_price_impact.rs index d22531b..c36df20 100644 --- a/src/utils/compute_price_impact.rs +++ b/src/utils/compute_price_impact.rs @@ -65,8 +65,8 @@ mod tests { assert_eq!( compute_price_impact( &Price::new(token.clone(), token_1.clone(), 10, 100), - &CurrencyAmount::from_raw_amount(token.clone(), 10).unwrap(), - &CurrencyAmount::from_raw_amount(token_1.clone(), 200).unwrap() + &CurrencyAmount::from_raw_amount(token, 10).unwrap(), + &CurrencyAmount::from_raw_amount(token_1, 200).unwrap() ) .unwrap(), Percent::new(-10000, 10000)