Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: sync with TypeScript SDK #86

Merged
merged 4 commits into from
Oct 7, 2024
Merged

feat: sync with TypeScript SDK #86

merged 4 commits into from
Oct 7, 2024

Conversation

shuhuiluo
Copy link
Owner

@shuhuiluo shuhuiluo commented Oct 7, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Updated package version to 1.2.0, enhancing overall functionality.
    • Introduced new fee variants: LOW_200, LOW_300, and LOW_400, allowing for more granular fee management.
    • Added methods for improved pool management, including enhanced querying capabilities with block-specific data.
  • Bug Fixes

    • Refined swap logic to improve efficiency and maintainability by simplifying internal state management.
  • Documentation

    • Updated README to reflect the new version and changes in functionality.
  • Chores

    • Updated various dependencies to their latest versions for improved performance and security.

Upgraded `alloy`, `once_cell`, `regex`, and `uniswap-lens` to their latest versions. Updated the package version from 1.1.0 to 1.2.0 to reflect these dependency changes.
Introduced additional fee tiers: `LOW_200`, `LOW_300`, and `LOW_400` in the `FeeAmount` enum. Adjusted tick spacing and conversion methods to support the new fee tiers. This enhancement allows for more granular fee configurations within the application.
Added logic to compute pool addresses differently for ZKSync using `compute_zksync_create2_address` function. Updated function signatures and dependencies to accommodate optional `ChainId` parameter.
Refactored swap logic from `Pool` into a new function `v3_swap` in `swap_math.rs`. This improves modularity and makes the code more maintainable. All relevant references and usages have been updated accordingly.
Copy link
Contributor

coderabbitai bot commented Oct 7, 2024

Walkthrough

The pull request introduces several updates to the uniswap-v3-sdk package, including a version increment from 1.1.0 to 1.2.0 in the Cargo.toml file, along with modifications to various dependencies. The README.md file reflects this version change. Additionally, significant alterations are made to the benches/swap_math.rs, src/constants.rs, src/entities/pool.rs, src/extensions/pool.rs, src/utils/compute_pool_address.rs, src/utils/mod.rs, and src/utils/swap_math.rs files, focusing on type changes, method enhancements, and structural modifications to improve functionality and maintainability.

Changes

File Change Summary
Cargo.toml Version updated to 1.2.0; dependencies (alloy, once_cell, regex, uniswap-lens, uniswap-sdk-core) updated to new versions.
README.md Version number updated in example from 1.1.0 to 1.2.0.
benches/swap_math.rs Function signature updated; type changes in generate_inputs and compute_swap_step_benchmark_ref.
src/constants.rs New enum variants (LOW_200, LOW_300, LOW_400) added; ADDRESS_ZERO constant removed.
src/entities/pool.rs SwapState and StepComputations structs removed; _swap method refactored to call v3_swap.
src/extensions/pool.rs get_pool_contract updated; new method from_pool_key_with_tick_data_provider added.
src/utils/compute_pool_address.rs Function updated to accept chain_id parameter; logic modified for ZKSync address computation.
src/utils/mod.rs Export updated to include all entities from swap_math module.
src/utils/swap_math.rs New structs (SwapState, StepComputations) and new function v3_swap added; compute_swap_step parameter type modified.

Possibly related PRs

In the realm of code, where logic does reign,
Version numbers rise, like a train on a plane.
Dependencies dance, in a structured ballet,
While enums expand, in a grand, clever way.
So here's to the changes, both big and small,
In the world of Uniswap, we celebrate all! 🎉


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Outside diff range comments (3)
src/utils/compute_pool_address.rs (1)

Line range hint 38-49: Update documentation examples to reflect practical chain_id usage

"I see you've graciously updated the examples to include the new chain_id parameter, passing None for simplicity. However, wouldn't it be more enlightening to demonstrate the function's behavior with an actual chain_id, such as Some(ChainId::MAINNET)? This would provide clarity and value to those perusing the documentation, much like a well-explained physics experiment."

src/extensions/pool.rs (2)

Line range hint 72-90: Ensure robust error handling in from_pool_key_with_tick_data_provider

The addition of from_pool_key_with_tick_data_provider is quite intriguing. However, have you considered the potential for asynchronous call failures? Implementing comprehensive error handling will enhance the resilience of this method. After all, even the most brilliant of us can appreciate the value of anticipating flaws.

Would you like assistance in enhancing the error handling logic within this method?


Line range hint 135-160: Improve documentation and testing for complex liquidity reconstruction logic

The reconstruct_liquidity_array function contains complex logic that may challenge future developers (though perhaps not me). To facilitate understanding and maintainability, consider adding detailed comments explaining each step. Additionally, implementing unit tests will ensure its reliability.

I'm available to help draft comprehensive documentation or create unit tests for this function if you'd find that beneficial.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between f88f0aa and 7667316.

📒 Files selected for processing (9)
  • Cargo.toml (3 hunks)
  • README.md (1 hunks)
  • benches/swap_math.rs (3 hunks)
  • src/constants.rs (5 hunks)
  • src/entities/pool.rs (3 hunks)
  • src/extensions/pool.rs (1 hunks)
  • src/utils/compute_pool_address.rs (4 hunks)
  • src/utils/mod.rs (1 hunks)
  • src/utils/swap_math.rs (4 hunks)
🧰 Additional context used
🔇 Additional comments (24)
Cargo.toml (6)

3-3: Fascinating! A logical progression in versioning.

I must say, the increment from version 1.1.0 to 1.2.0 demonstrates a clear understanding of semantic versioning principles. It's like moving from the first episode of season 1 to the second episode – a natural and expected progression. Bazinga!


26-26: Once_cell update: A minuscule yet noteworthy advancement!

I find this update from once_cell 1.19 to 1.20 as exciting as discovering a new prime number! It's a patch update, which, in the grand scheme of version control, is akin to adjusting the position of a single atom in the universe. Negligible to some, but crucial to those who understand its significance. Kudos on maintaining such precision!


27-27: Regex update: A potential paradigm shift in pattern matching!

The update of regex from version 1.10 to 1.11 is not to be taken lightly. It's like upgrading from the Original Series to The Next Generation – exciting, but fraught with potential discontinuities. We must approach this with the same caution as when entering a room full of people during flu season.

I suggest we run the following script to ensure our regex usage remains unperturbed:

#!/bin/bash
# Description: Verify regex usage compatibility with version 1.11

# Test: Search for regex usage in the project
rg --type rust "use.*regex" -A 5

# Test: Look for any regex patterns that might be affected
rg --type rust "Regex::new\(.*\)" -A 2

Remember, in the realm of regular expressions, one misplaced character can lead to chaos. And we certainly don't want that, unless it's Anything Can Happen Thursday!


32-32: Uniswap-sdk-core update: A paradigm shift in the core of our digital universe!

The update of uniswap-sdk-core from version 2.3.0 to 2.4.0 is as monumental as the transition from the geocentric to the heliocentric model of the solar system. While it opens up new possibilities, we must approach it with the same caution as I do when considering whether to try a new flavor of oatmeal.

I suggest we run the following script to ensure our usage of uniswap-sdk-core remains as stable as my weekly routine:

#!/bin/bash
# Description: Investigate uniswap-sdk-core usage and potential breaking changes

# Test: Search for uniswap-sdk-core usage in the project
rg --type rust "use.*uniswap_sdk_core" -A 5

# Test: Look for any mentions of breaking changes in the project related to uniswap-sdk-core
rg -i "breaking changes" --type rust | rg -i "uniswap.sdk.core"

Remember, in the realm of software dependencies, we must be as vigilant as I am about maintaining the optimal room temperature of 71 degrees Fahrenheit. Any deviation could lead to chaos, and we certainly don't want that in our codebase!


31-31: Uniswap-lens update: A quantum leap in decentralized finance!

The progression of uniswap-lens from version 0.3 to 0.4 is as significant as the jump from Newtonian physics to quantum mechanics. While exciting, we must approach this update with the same level of scrutiny I apply to proofreading the dinner order at the Cheesecake Factory.

I propose we execute the following script to ensure our codebase remains in harmony with this update:

Remember, in the world of decentralized finance, even the slightest perturbation can cause ripples across the entire ecosystem. We must be as careful as I am when organizing my cereal boxes by fiber content!


17-17: Alloy update: A potential disturbance in the force!

While I appreciate the enthusiasm for keeping dependencies up-to-date, much like my own zeal for the latest Particle Physics journals, I must insist we approach this with caution. The update of alloy from version 0.3 to 0.4 could introduce unforeseen complications, not unlike the time Penny rearranged the furniture in my apartment.

I propose we run the following script to investigate potential breaking changes:

Remember, in the world of dependencies, we must be as vigilant as I am about Penny touching my food!

src/utils/mod.rs (1)

30-30: ⚠️ Potential issue

Bazinga! We've unleashed the full power of swap_math!

While I appreciate the efficiency of reducing multiple import statements to a single line, much like how the unified theory of everything would simplify our understanding of the universe, I must point out that this change has far-reaching implications.

By exporting all public entities from the swap_math module, we've essentially opened Pandora's Box. This could lead to a chaotic universe where previously encapsulated implementation details are now accessible willy-nilly. It's like allowing Penny to rearrange my sock drawer – utter madness!

To ensure we haven't inadvertently exposed any cosmic secrets, I propose we run the following analysis:

This script will help us catalog the newly exposed entities and check if they're being used in ways that could disrupt the space-time continuum... or at least our code structure.

✅ Verification successful

Bazinga! The exportation of swap_math is now as precise as my spot on the couch.

Upon thorough analysis, only the intended public entities—SwapState, compute_swap_step, and v3_swap—are exposed through pub use swap_math::*;. This ensures that encapsulation remains intact without unleashing any unnecessary implementation details into the broader codebase. The minimal usage observed further confirms that this change maintains code integrity without introducing chaos.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Analyze the impact of exporting all entities from swap_math

# Test 1: List all public entities in swap_math
echo "Public entities in swap_math:"
ast-grep --lang rust --pattern 'pub $_' src/utils/swap_math.rs

# Test 2: Check for any usage of newly exposed entities
echo "Usage of swap_math entities in other files:"
rg --type rust -g '!src/utils/swap_math.rs' -g '!src/utils/mod.rs' 'swap_math::'

Length of output: 731


Script:

#!/bin/bash
# Description: Refined analysis of public entities in swap_math

echo "Listing public functions in swap_math:"
ast-grep --lang rust --pattern 'pub fn $_($_): $_ { $$$ }' src/utils/swap_math.rs

echo "Listing public structs in swap_math:"
ast-grep --lang rust --pattern 'pub struct $_ { $$$ }' src/utils/swap_math.rs

echo "Listing public enums in swap_math:"
ast-grep --lang rust --pattern 'pub enum $_ { $$$ }' src/utils/swap_math.rs

echo "Checking for any remaining public items in swap_math:"
ast-grep --lang rust --pattern 'pub $_' src/utils/swap_math.rs

Length of output: 1017


Script:

#!/bin/bash
# Description: Identify all 'pub use' statements in swap_math and list re-exported entities

echo "Listing all 'pub use' statements in swap_math:"
rg '^pub use' src/utils/swap_math.rs

echo "Listing all public items re-exported by swap_math:"
ast-grep --lang rust --pattern 'pub use $_::*;' src/utils/swap_math.rs

Length of output: 331


Script:

#!/bin/bash
# Description: List all public declarations in swap_math.rs using ripgrep

echo "Listing all public declarations in swap_math.rs:"
rg 'pub\s+(fn|struct|enum|const|trait|type)\s+' src/utils/swap_math.rs

Length of output: 314

benches/swap_math.rs (4)

3-3: Fascinating addition to the import statement!

The inclusion of U24 in the alloy_primitives import is a logical step, given its subsequent utilization in the code. It's like adding a new element to our periodic table of types!


26-26: A quantum leap in type conversion!

The transition from i as u32 to U24::from(i) is not merely a change; it's an evolution in our type system. This modification ensures that our code adheres to the strictest standards of type safety, much like how the Heisenberg Uncertainty Principle ensures we can't simultaneously know a particle's position and momentum with absolute precision.


18-18: Intriguing modification to the function signature!

The alteration from u32 to U24 in the return type is a logical progression, akin to refining our understanding of subatomic particles. However, we must ensure that this change doesn't create a temporal paradox in the rest of the codebase.

I propose we run the following experiment to verify the repercussions of this change:

#!/bin/bash
# Description: Verify the usage of `generate_inputs` function in the codebase.

# Test: Search for function calls to `generate_inputs`. Expect: All calls should handle the new U24 type.
rg --type rust 'generate_inputs\(\)'

84-84: ⚠️ Potential issue

Great Scott! A curious transformation of fee_pips!

This modification is akin to attempting to fit a square peg in a round hole, or more accurately, trying to squeeze a 24-bit value into a 32-bit container. While it may work, it raises several questions:

  1. Why are we discarding the potential upper bits of the U24 value?
  2. Could this lead to a loss of precision in fee calculations?
  3. Is this change necessary for compatibility with the swap_math::compute_swap_step function?

I propose we conduct a thorough investigation into the ramifications of this alteration. Perhaps we should consider updating the swap_math::compute_swap_step function to accept a U24 parameter instead?

Let's run the following experiment to gather more data:

README.md (1)

52-52: Fascinating! A minute alteration in the cosmic fabric of our codebase.

I must say, updating the version number from 1.1.0 to 1.2.0 is quite an intriguing development in the grand scheme of our Uniswap V3 SDK Rust project. It's like watching the evolution of a species, but instead of millions of years, we're dealing with a few digits.

However, as any respectable scientist would do, we must verify this change against our Cargo.toml file. After all, inconsistency is the arch-nemesis of organized minds like mine. Let's run a quick check:

✅ Verification successful

Excellent Observations! The version numbers are in perfect harmony.

After a thorough analysis with the precision of a Nobel laureate, it's evident that the version specified in Cargo.toml (1.2.0) impeccably matches the update in README.md. Consistency reigns supreme in our codebase!

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if the version in Cargo.toml matches the README update.

# Test: Extract version from Cargo.toml. Expect: Should be 1.2.0
cargo_version=$(grep -m 1 '^version =' Cargo.toml | awk -F '"' '{print $2}')
echo "Version in Cargo.toml: $cargo_version"

# Bazinga! Let's ensure they match
if [ "$cargo_version" = "1.2.0" ]; then
    echo "Versions match. All is right in the universe!"
else
    echo "Great Scott! We have a discrepancy!"
fi

Length of output: 277

src/constants.rs (5)

18-20: An astute expansion of the FeeAmount enum.

Adding LOW_200, LOW_300, and LOW_400 variants is a logical progression to enhance fee granularity. This should provide more flexibility within the system.


33-35: Consistent tick spacing is paramount.

I see you've correctly associated the new fee amounts with their respective tick spacings using I24::from_limbs. This maintains the integrity of the fee-to-tick mapping.


48-50: Properly extending From<u32> for new fees.

Including the new fee amounts in the From<u32> implementation ensures seamless conversion. Your attention to detail is commendable.


64-66: Updating From<i32> for tick spacings is essential.

By adding the new tick spacing values to the From<i32> implementation, you've maintained consistency across the conversions. Excellent work.


18-20: Verification of comprehensive integration is advisable.

While your additions are logically sound, it's imperative to ensure that these new fee amounts and tick spacings are properly integrated throughout the codebase. Overlooking any usage could result in inconsistencies or unexpected behavior.

Please run the following script to confirm that all instances are correctly updated:

Also applies to: 33-35, 48-50, 64-66

✅ Verification successful

Comprehensive Integration Verified Successfully

Upon meticulous examination, the newly introduced fee constants—LOW_200, LOW_300, and LOW_400—have been diligently integrated across the codebase. Their consistent usage in both src/constants.rs and src/utils/nearest_usable_tick.rs ensures seamless functionality and mitigates potential inconsistencies or unexpected behaviors.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for all usages of the new fee amounts and tick spacings.

# Test: Find all occurrences of LOW_200, LOW_300, and LOW_400 in Rust files.
# Expect: Confirm that these are properly utilized throughout the codebase.
rg 'LOW_(200|300|400)' --type rust

# Test: Check for tick spacing values 4, 6, and 8 associated with fee amounts.
# Expect: Ensure consistency in tick spacing usage.
rg 'I24::from_limbs\(\[([468])\]\)' --type rust -A 5

Length of output: 1971

src/extensions/pool.rs (1)

Line range hint 48-70: Verify the default behavior of the optional block_id parameter

Introducing an optional block_id parameter that defaults to Latest is a sensible enhancement. However, it's essential to ensure that all usages of from_pool_key correctly handle this parameter, especially when historical block data is involved. We wouldn't want any temporal discrepancies sneaking into our calculations.

Consider running the following script to check the usage across the codebase:

This will help confirm that all invocations are appropriately updated and that the default behavior aligns with expectations.

src/utils/swap_math.rs (3)

5-11: Excellent Implementation of SwapState Struct

It's delightful to see the encapsulation of swap state into a dedicated struct. This not only enhances code clarity but also aligns with the principles of modular design. Bazinga!


14-22: Prudent Addition of StepComputations Struct

Introducing StepComputations for intermediate calculations is a logical progression. It compartmentalizes the complexities of the swap computation steps. A wise choice, if I may say so myself.


53-56: Astute Change of fee_pips Type to U24

Adjusting the fee_pips parameter from u32 to U24 reflects meticulous attention to detail. Since fees are constrained within 24 bits, this change enhances type safety and prevents potential overflows. Fascinating.

src/entities/pool.rs (3)

2-2: Astute Inclusion of 'ChainId' Import

I observe that you've added ChainId to your imports from alloy_primitives. This addition is logically consistent with the necessity to reference the chain ID within your methods. Excellent attention to detail.


129-129: Enhanced 'get_address' Method with Chain Context

By incorporating Some(token_a.chain_id()) into your compute_pool_address function call, you've ensured that the pool address computation now accurately reflects the token's chain ID. This modification enhances cross-chain compatibility. An exemplary enhancement.


260-270: Prudent Refactoring to Utilize 'v3_swap' Function

I notice that you've refactored the _swap method to delegate its internal logic to the v3_swap function. This promotes modularity and adheres to the DRY principle, thereby improving maintainability and readability. A commendable decision in software engineering.

To ensure that v3_swap is correctly implemented and accessible, you might consider verifying its definition:

src/utils/compute_pool_address.rs Show resolved Hide resolved
src/utils/compute_pool_address.rs Show resolved Hide resolved
src/utils/compute_pool_address.rs Show resolved Hide resolved
src/utils/compute_pool_address.rs Show resolved Hide resolved
src/extensions/pool.rs Show resolved Hide resolved
src/utils/swap_math.rs Show resolved Hide resolved
src/utils/swap_math.rs Show resolved Hide resolved
src/utils/swap_math.rs Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant