Skip to content

Commit

Permalink
perf: refactor Trade methods (#102)
Browse files Browse the repository at this point in the history
* perf: refactor `Trade` methods

Refactored `input_amount` and `output_amount` methods to use `Fraction` for cleaner and more efficient code. Improved heuristic algorithm in functions handling pool exclusions during trade computations. Added `#[repr(u32)]` and `#[allow(non_camel_case_types)]` attributes to `FeeAmount` enum and updated crate version to `2.4.1`.

* Update Cargo.toml

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Refactor trade pool iteration logic

Updated loops in trade.rs to use `enumerate()` for better clarity. Changed pool exclusion to utilize `take` and `skip` for more concise and readable code. These adjustments improve code readability and maintainability.

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
  • Loading branch information
shuhuiluo and coderabbitai[bot] authored Nov 4, 2024
1 parent 29d2da6 commit c20b930
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 16 deletions.
6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "uniswap-v3-sdk"
version = "2.4.0"
version = "2.4.1"
edition = "2021"
authors = ["Shuhui Luo <twitter.com/aureliano_law>"]
description = "Uniswap V3 SDK for Rust"
Expand Down Expand Up @@ -34,8 +34,8 @@ uniswap-sdk-core = "3.0.0"

[features]
default = []
extensions = ["uniswap-lens/std", "alloy", "anyhow", "base64", "regex", "serde_json"]
std = ["thiserror", "uniswap-sdk-core/std"]
extensions = ["alloy", "anyhow", "base64", "regex", "serde_json", "uniswap-lens"]
std = ["alloy?/std", "thiserror", "uniswap-sdk-core/std", "uniswap-lens?/std"]

[dev-dependencies]
alloy-signer = "0.5"
Expand Down
4 changes: 2 additions & 2 deletions src/constants.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(non_camel_case_types)]

use alloy_primitives::{
address,
aliases::{I24, U24},
Expand All @@ -13,6 +11,8 @@ pub const POOL_INIT_CODE_HASH: B256 =

/// The default factory enabled fee amounts, denominated in hundredths of bips.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[repr(u32)]
#[allow(non_camel_case_types)]
pub enum FeeAmount {
LOWEST = 100,
LOW_200 = 200,
Expand Down
41 changes: 30 additions & 11 deletions src/entities/trade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,16 @@ where
/// The input amount for the trade assuming no slippage.
#[inline]
pub fn input_amount(&self) -> Result<CurrencyAmount<TInput>, Error> {
let mut total = CurrencyAmount::from_raw_amount(self.input_currency().clone(), 0)?;
let mut total = Fraction::default();
for Swap { input_amount, .. } in &self.swaps {
total = total.add(input_amount)?;
total = total + input_amount.as_fraction();
}
Ok(total)
CurrencyAmount::from_fractional_amount(
self.input_currency().clone(),
total.numerator,
total.denominator,
)
.map_err(Error::Core)
}

/// The input amount for the trade assuming no slippage.
Expand All @@ -266,11 +271,16 @@ where
/// The output amount for the trade assuming no slippage.
#[inline]
pub fn output_amount(&self) -> Result<CurrencyAmount<TOutput>, Error> {
let mut total = CurrencyAmount::from_raw_amount(self.output_currency().clone(), 0)?;
let mut total = Fraction::default();
for Swap { output_amount, .. } in &self.swaps {
total = total.add(output_amount)?;
total = total + output_amount.as_fraction();
}
Ok(total)
CurrencyAmount::from_fractional_amount(
self.output_currency().clone(),
total.numerator,
total.denominator,
)
.map_err(Error::Core)
}

/// The output amount for the trade assuming no slippage.
Expand Down Expand Up @@ -616,6 +626,11 @@ where
/// Given a list of pools, and a fixed amount in, returns the top `max_num_results` trades that
/// go from an input token amount to an output token, making at most `max_hops` hops.
///
/// ## Note
///
/// This does not consider aggregation, as routes are linear. It's possible a better route
/// exists by splitting the amount in among multiple routes.
///
/// ## Arguments
///
/// * `pools`: The pools to consider in finding the best trade
Expand Down Expand Up @@ -650,7 +665,7 @@ where
None => currency_amount_in.wrapped()?,
};
let token_out = currency_out.wrapped();
for pool in &pools {
for (i, pool) in pools.iter().enumerate() {
// pool irrelevant
if !pool.involves_token(&amount_in.currency) {
continue;
Expand All @@ -677,7 +692,8 @@ where
} else if max_hops > 1 && pools.len() > 1 {
let pools_excluding_this_pool = pools
.iter()
.filter(|&p| p.address(None, None) != pool.address(None, None))
.take(i)
.chain(pools.iter().skip(i + 1))
.cloned()
.collect();
// otherwise, consider all the other paths that lead from this token as long as we
Expand All @@ -704,7 +720,9 @@ where
/// Given a list of pools, and a fixed amount out, returns the top `max_num_results` trades that
/// go from an input token to an output token amount, making at most `max_hops` hops.
///
/// Note this does not consider aggregation, as routes are linear. It's possible a better route
/// ## Note
///
/// This does not consider aggregation, as routes are linear. It's possible a better route
/// exists by splitting the amount in among multiple routes.
///
/// ## Arguments
Expand Down Expand Up @@ -740,7 +758,7 @@ where
None => currency_amount_out.wrapped()?,
};
let token_in = currency_in.wrapped();
for pool in &pools {
for (i, pool) in pools.iter().enumerate() {
// pool irrelevant
if !pool.involves_token(&amount_out.currency) {
continue;
Expand All @@ -767,7 +785,8 @@ where
} else if max_hops > 1 && pools.len() > 1 {
let pools_excluding_this_pool = pools
.iter()
.filter(|&p| p.address(None, None) != pool.address(None, None))
.take(i)
.chain(pools.iter().skip(i + 1))
.cloned()
.collect();
// otherwise, consider all the other paths that arrive at this token as long as we
Expand Down

0 comments on commit c20b930

Please sign in to comment.