refactor: clean up dependencies and re-exports #92
Conversation
`lazy_static` was removed from the common prelude module to avoid unnecessary dependencies in files that don't require it. Explicit imports of `lazy_static` were added only where it is actually used, improving clarity and reducing potential overhead.
Reorganized module layout to improve maintainability and simplify imports. Removed standalone `prelude.rs` by integrating its functionality directly into `lib.rs`. Adjusted tests, visibility modifiers, and re-export strategy across modules for cleaner and more efficient usage.
Replaced unused or unnecessary imports with more relevant ones to optimize dependencies. Updated type usage for better clarity and correctness, including replacing `vec![]` with `[]` for `FxHashMap::from_iter` calls. Simplified assertions and improved error handling for better maintainability.
Updated the README to improve documentation order, clarify `no_std` usage, and enhance example formatting. Refactored type exports in the library to use type aliases for better readability and maintainability.
WalkthroughThe pull request introduces several updates to the Uniswap SDK Core in Rust, focusing on dependency management, module reorganization, and minor improvements across various files. Key changes include updating package versions, modifying dependency constraints, enhancing type visibility, and streamlining code initialization. The modifications span multiple files, with particular attention to address handling, static variable definitions, and module re-exports. Changes
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/examples/token_example.rs (1)
4-4: Clarify the rationale for converting this example to a test.Attaching
#[test]to what was previously an examplemain()function can cause confusion regarding the intended usage. Typically, examples remain asmain()functions, while tests use descriptive function names indicating the scenario being tested.Consider changing the function name to something like
test_token_basics()or reverting to a standalone example if the intent is to demonstrate usage rather than test logic.-#[test] -fn main() { +#[test] +fn test_token_basics() {src/lib.rs (1)
46-56: Consider documenting re-export strategyConsider adding a comment or documentation section that explains the re-export strategy and criteria for including items in the prelude. This will help maintain consistency as the codebase grows and guide future contributors.
Reference: The strategy should align with the guidelines from lurklurk.org mentioned in the PR description.
src/constants.rs (1)
3-3: Consideronce_cellfor lazily initialized constants.
Usinglazy_staticis fine, but modern Rust often suggests theonce_cellcrate’sLazyorOnceCelltypes for less macro-based, more idiomatic single initialization. This is a minor preference and not a blocker.src/entities/ether.rs (1)
2-2: UseToStringcarefully for potential performance overhead.Consider whether the
to_string()calls might be invoked frequently in performance-critical paths. Although usingToStringis convenient, double-check high-frequency code paths to ensure that converting toStringrepeatedly will not introduce performance bottlenecks.src/entities/fractions/percent.rs (1)
Line range hint
14-28: Multiplying byONE_HUNDREDinto_significantandto_fixed.These methods multiply the internal fraction by 100 before formatting output, ensuring intuitive representation for percentages. This is a clear, readable approach. Just keep an eye on rounding edge cases, especially when the fraction is near boundary conditions (like
0.999999).src/entities/currency.rs (1)
Line range hint
101-102: Avoid potential confusion betweenTOKEN0andTOKEN1.Because these constants are fairly generic, new contributors might confuse them if multiple tokens are introduced. Adding clarifying commentary or more descriptive naming (e.g.,
TEST_TOKEN_ZERO,TEST_TOKEN_ONE) could prevent confusion in larger test suites.src/entities/weth9.rs (1)
34-34: Adoption ofFxHashMap::from_iteris concise and idiomatic.
If this set of predefined tokens expands significantly, consider storing them in an external config or constant data structure for improved maintainability and clarity.README.md (1)
71-72: Neat closure of example block.Including code examples in collapsible sections is a good user experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
Cargo.toml(1 hunks)README.md(4 hunks)src/addresses.rs(7 hunks)src/constants.rs(1 hunks)src/entities/currency.rs(1 hunks)src/entities/ether.rs(1 hunks)src/entities/fractions/currency_amount.rs(2 hunks)src/entities/fractions/fraction.rs(2 hunks)src/entities/fractions/mod.rs(1 hunks)src/entities/fractions/percent.rs(1 hunks)src/entities/fractions/price.rs(1 hunks)src/entities/mod.rs(1 hunks)src/entities/token.rs(3 hunks)src/entities/weth9.rs(2 hunks)src/examples/mod.rs(1 hunks)src/examples/token_example.rs(1 hunks)src/lib.rs(1 hunks)src/prelude.rs(0 hunks)src/utils/mod.rs(1 hunks)src/utils/sorted_insert.rs(1 hunks)src/utils/sqrt.rs(2 hunks)
💤 Files with no reviewable changes (1)
- src/prelude.rs
✅ Files skipped from review due to trivial changes (2)
- src/examples/mod.rs
- src/entities/fractions/mod.rs
🔇 Additional comments (39)
src/examples/token_example.rs (1)
5-5: Consider relocating this code block into an actual test module.If the primary goal is to adopt Rust's testing conventions, placing this function in a dedicated test module or file under
tests/orsrc/lib.rswith#[cfg(test)]might provide better separation of concerns between examples and tests.Here is a shell script you could use to look for existing test modules and confirm if there's a better place to move this test:
src/lib.rs (3)
39-40: Well-documented utils module addition!The new
utilsmodule is properly documented and follows the consistent documentation style of other modules. This addition aligns well with the PR's objective of improving module organization.
59-60: Good practice: Making examples test-only!Converting the examples module to be private and test-only is a good practice. This ensures that example code doesn't become part of the public API while still maintaining its value for testing and documentation.
46-56: Verify the scope of utils re-exportsThe prelude organization is clean and follows Rust's best practices. However, let's verify that all items from
utilsshould be publicly re-exported.✅ Verification successful
Re-exports from utils module are appropriate and well-scoped
The utils module exports are well-structured and appropriate for public use:
- Each utility function serves a specific, well-defined purpose
- All public functions have clear, domain-specific functionality:
compute_price_impact: Price calculationscompute_zksync_create2_address: ZkSync address computationsorted_insert: Generic sorted collection manipulationsqrt: Mathematical operation with BigInt supportvalidate_and_parse_address: Ethereum address validation (feature-gated)The re-exports in the prelude are intentional and follow good API design practices, making commonly used utilities easily accessible while maintaining proper encapsulation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the contents of the utils module to ensure all items are meant to be public # Search for public items in utils module ast-grep --pattern 'pub $$$' src/utils/Length of output: 41
Script:
#!/bin/bash # Let's try a different approach to check the utils module content # First, let's check if utils.rs exists in src directory fd "utils.rs" src/ # Then use ripgrep to look for public items in utils rg "^pub" -A 2 src/utils.rs || rg "^pub" -A 2 src/utils/mod.rs # Also check if there's a utils directory with multiple files fd . src/utils/Length of output: 703
Script:
#!/bin/bash # Let's check the contents of each utility function to verify their public interfaces # Check compute_price_impact rg "^pub" -A 2 src/utils/compute_price_impact.rs # Check compute_zksync_create2_address rg "^pub" -A 2 src/utils/compute_zksync_create2_address.rs # Check sorted_insert rg "^pub" -A 2 src/utils/sorted_insert.rs # Check sqrt rg "^pub" -A 2 src/utils/sqrt.rs # Check validate_and_parse_address rg "^pub" -A 2 src/utils/validate_and_parse_address.rsLength of output: 1113
src/entities/mod.rs (1)
9-15: Good job consolidating entity imports via re-exports.
Re-exporting these modules and types (base_currency, currency, Ether, fractions, native_currency, token, and WETH9) from a single location simplifies the import paths for consumers. This approach improves code organization and readability. Keep an eye on potential naming collisions, but this design aligns well with recommended Rust patterns.src/utils/mod.rs (1)
6-9: Straightforward re-exports enhance utility discoverability.
Exposing compute_price_impact, compute_zksync_create2_address, sorted_insert, and sqrt at theutilsmodule level is a great way to streamline access to these functions. This change helps reduce excessive module qualification throughout the codebase.src/utils/sqrt.rs (2)
12-12: Simplified negative check is more readable.
Replacingis_negative()with a direct comparison againstBigInt::ZEROmakes the code more intuitive to follow. The error handling remains consistent.
22-22: Test module uses minimal dependencies effectively.
Theuse core::str::FromStr;is a clear choice for parsing numeric test inputs without pulling in extra crates. This keeps the test suite self-contained and efficient.src/entities/ether.rs (1)
Line range hint
29-44: NewEtherconstructors look clean.The new
pub fn newandpub fn on_chainmethods provide clarity in creating and reusingEtherinstances. Great job centralizing the logic for instantiation with defaults. Please ensure that other parts of the code do not assume the old or alternate creation patterns forEther.src/entities/fractions/percent.rs (1)
2-5: Appropriate usage oflazy_staticforONE_HUNDRED.Centralizing the representation of 100 as a
Fractioninside a static reference helps eliminate magic numbers and ensures consistency across the codebase. Good approach!src/utils/sorted_insert.rs (2)
1-2: Imports updated forVecandOrdering.Good choice to rely on
alloc::vec::Vecandcore::cmp::Orderingdirectly. This makes dependencies explicit and clarifies the function’s underlying containers and comparison approach.
Line range hint
13-32: Efficient boundary checks for a sorted insertion.The updated logic correctly returns the inserted item when it would naturally sort after the last element, preventing unnecessary shifting. Nice work on handling the potential pop of the last element if the vector is at maximum size.
src/entities/currency.rs (1)
95-95: Use oflazy_staticforTOKEN0andTOKEN1.Defining these tokens as global static references is a neat approach for test reusability and efficiency. Consider verifying that the address and chain ID remain consistent if you intend to reuse these test tokens in other contexts (e.g., multiple test modules).
src/entities/weth9.rs (1)
2-2: Import ofToStringtrait looks fine.
This addition will assist with string-based operations and aligns with Rust's standard library usage guidelines.src/entities/fractions/price.rs (1)
145-145: Use oflazy_static!for test tokens is consistent with your approach elsewhere.
This ensures easy setup and reuse of static values in multiple tests.src/entities/token.rs (3)
134-134: Switching toalloy_primitives::address!in the no-symbol macro arm.
This is a neat consolidation that aligns internal address parsing under the same utility.
156-156: Use ofalloy_primitives::address!in the single-symbol macro arm.
Maintaining consistent address parsing methods is beneficial for reliability.
178-178: Migration toalloy_primitives::address!in the full signature macro arm.
This unifies address creation logic and reduces custom parsing overhead.src/entities/fractions/currency_amount.rs (3)
2-2: Import ofToString
Facilitates straightforward string conversions, which can be especially handy in methods liketo_exact().
4-4: Import ofIntegertrait
Adds versatility for integer-based arithmetic within the fraction handling logic.
159-159: Introduction oflazy_static!for shared test tokens
This eliminates duplication across tests and improves maintainability.src/entities/fractions/fraction.rs (2)
2-2: Good addition of imports for fraction functionality.These imports (
ToString,Ordering,NonZeroU64, andInteger) provide clearer code (e.g., string conversion, refined comparisons, integer operations, etc.). This looks appropriate for enhancing fraction calculations.Also applies to: 4-4, 6-6, 10-10
169-169: Improved zero-denominator assertion.Replacing
assert!(!denominator.is_zero(), ...)withassert_ne!(denominator, BigInt::ZERO, ...)is more explicit and consistent. This is clearer and maintains the same logic.src/addresses.rs (10)
2-2: New import ofaddressmacro.Using
use alloy_primitives::address;is a good choice for more concise and reliable address definitions throughout the codebase.
5-5: ExposingAddressMappublicly.Making
pub type AddressMap = FxHashMap<u64, Address>;public seems appropriate to ensure address maps are available beyond this module.
56-56: Consistent address map initialization for V2_FACTORY_ADDRESSES.
AddressMap::from_iter([...])aligns with the broader approach in this file. Good job keeping it consistent.
105-105: Consistent address map initialization for V2_ROUTER_ADDRESSES.Same pattern as above. Maintaining a standard code style improves clarity.
385-385: Lazy-initialized global CHAIN_TO_ADDRESSES_MAP.Using
FxHashMap::from_iteris consistent with your approach. This helps keep the code tidy and more easily extensible.
451-451: Adding GOVERNANCE_ALPHA_V1_ADDRESSES with from_iter.This aligns with the rest of the code; each chain is mapped systematically.
459-459: Adding GOVERNANCE_BRAVO_ADDRESSES.Similar pattern as the alpha addresses. Looks correct.
471-471: Adding MERKLE_DISTRIBUTOR_ADDRESS.Well-structured approach for mainnet addresses. Matches your consistent usage pattern.
478-478: Adding ARGENT_WALLET_DETECTOR_ADDRESS.This initialization is straightforward and keeps the code consistent.
510-510: Adding SOCKS_CONTROLLER_ADDRESSES.Same pattern here. The
AddressMap::from_iter([...])usage is clean and consistent.Cargo.toml (2)
3-3: Version bumped to 3.3.0.Increasing the package version indicates a new release with possibly breaking or feature enhancements. Ensure you update documentation and release notes accordingly.
10-10: Updated alloy-primitives dependency.Switching to
{ version = ">=0.8.5", features = ["map-fxhash"] }reflects the new address usage and presumably improved performance of fxhash. Good move.README.md (4)
16-16: Dependency version updated to 3.3.0.Consistent with the Cargo.toml bump. Ensures users referencing the readme see the correct version.
25-28: Reintroducedno_stdclarification.Highlighting that the library can run without the standard library is useful for embedded or WASM contexts. Great clarity.
31-33: Expanded example withtoken!macro.The collapsible code block is helpful for quick reference. This approach fosters clarity on how to create tokens.
104-105: Switched from V2 to V4 in “Used by” list.This properly reflects usage of the newer Uniswap V4 SDK in Rust. Keeps the readme up-to-date.
Following the guidelines here.
https://www.lurklurk.org/effective-rust/re-export.html
Summary by CodeRabbit
Based on the comprehensive changes across multiple files, here are the release notes:
Release Notes v3.3.0
New Features
Improvements
Changes
num-traitsandrustc-hashdependenciesalloy-primitivesDocumentation
no_stdlibrary support