chore: update derive_more and fastnum
#106
Conversation
Upgraded `derive_more` to v2 with new features and `fastnum` to v0.1.11, adjusting included features. Increased package version to 4.0.0-rc4 to reflect these updates.
WalkthroughThe pull request updates dependency configurations in the Changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
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 (
|
… with `HashMap` Updated dependencies to use non-default features and introduced `default = []` in features. Replaced `FxHashMap` with standard `HashMap` for better compatibility. Refactored documentation and examples for clarity, aligning the library's setup with best practices for no-std support.
![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 (1)
README.md (1)
60-66: Refine macro invocation syntax.
Consider removing the extra spacing in the macro calls (e.g., changeprintln ! (toprintln!() to adhere to idiomatic Rust syntax. This small style refinement will improve the readability of the example.- Ok(true) => println ! ("DAI sorts before ETH"), - Ok(false) => println ! ("DAI does not sort before ETH"), - Err(e) => println ! ("Error comparing tokens: {:?}", e), + Ok(true) => println!("DAI sorts before ETH"), + Ok(false) => println!("DAI does not sort before ETH"), + Err(e) => println!("Error comparing tokens: {:?}", e),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Cargo.toml(2 hunks)README.md(3 hunks)src/addresses.rs(2 hunks)src/entities/weth9.rs(2 hunks)src/lib.rs(4 hunks)src/utils/sorted_insert.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/utils/sorted_insert.rs
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~86-~86: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 2495 characters long)
Context: ... Contribution Contributions are welcome! If you find a bug or have suggestions f...
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (18)
Cargo.toml (3)
3-3: Version bump looks appropriate.The version increment to
4.0.0-rc4aligns with the dependency updates in this PR.
28-28: Good update to std feature dependencies.The std feature now correctly includes all necessary standard library support for the dependencies.
15-23: Verify compatibility with derive_more v2.The update to
derive_morev2 is a major version change that could introduce breaking changes. Please ensure all derived traits continue to work as expected.Dependencies are well-configured for no_std support.
Good practice setting
default-features = falsefor the dependencies to support no_std environments.Consider the impact of feature changes.
The removal of the
libmfeature fromfastnumand its addition tonum-traitssuggests a refactor of floating-point operations. Ensure this change doesn't affect numerical precision or behavior.src/lib.rs (3)
1-1: Good documentation practice.Including the README.md content in the crate documentation improves visibility and maintainability.
21-21: Verify num_traits usage.The addition of
num_traitsimport with_suggests it's used for its traits implementation. Ensure this doesn't conflict with existing numeric trait implementations.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for num_traits usage rg "impl.*num_traits::" -A 5Length of output: 30
🏁 Script executed:
#!/bin/bash echo "Searching for all occurrences of 'num_traits' in the codebase (with file names and line numbers):" rg "num_traits" -n echo "" echo "Searching for 'num_traits' occurrences in Cargo.toml (to verify dependency setup):" rg "num_traits" Cargo.tomlLength of output: 483
NUM_TRAITS Usage Verified
The only reference to
num_traitsis insrc/lib.rsasuse num_traits as _;, which is a common pattern for importing traits solely for their implementations without binding them to an explicit name. No conflicting implementations or additional usage ofnum_traitswere found in the codebase or in the dependency configuration (Cargo.toml), and there is no indication that this import interferes with other numeric trait implementations.
46-50: Consider performance implications of HashMap switch.The change from
FxHashMaptoHashMapmight impact performance asFxHashMapuses a faster hashing algorithm. Consider benchmarking if this is performance-critical code.✅ Verification successful
🌐 Web query:
What are the performance differences between HashMap and FxHashMap in Rust?Length of output: 5363
Performance Benchmarking Recommendation
The switch from using an optimized
FxHashMap(which uses the fast FxHash algorithm) to the defaultHashMap(which uses SipHash for improved security) can indeed have performance implications. Specifically:
Speed vs. Security:
FxHashMaptypically outperforms the defaultHashMapin many scenarios (up to 2–3× faster in benchmarks with integer keys) because FxHash is optimized for speed.- The default
HashMap, using SipHash, provides robust collision protection, which may incur additional overhead.Workload Considerations:
- If the affected code is performance-critical and handles large numbers of entries or relies on fast hash operations (especially with primitive types), you may see a noticeable performance impact.
- For use cases with untrusted input or where collision resistance is paramount, the performance trade-off for security is acceptable.
Recommendation:
- Benchmark the relevant code paths (using tools like Criterion) to measure if the slower default
HashMapaffects your application's performance in a critical way.- If the performance hit is unacceptable and the security trade-offs allow it (e.g., with trusted data), consider reverting to or conditionally using
FxHashMap.Based on the web research and documented performance differences, the review comment’s suggestion is valid.
src/entities/weth9.rs (1)
9-9: Consistent HashMap usage.The change from
FxHashMaptoHashMapaligns with the changes in lib.rs. Since this is a static mapping of chain IDs to tokens, the performance difference should be negligible.src/addresses.rs (1)
5-5: Consistent HashMap usage across address mappings.The change from
FxHashMaptoHashMapfor address mappings is consistent with other files. Since these are mostly static mappings used for lookups, the performance impact should be minimal.Also applies to: 384-385
README.md (10)
15-18: Ensure README version consistency.
The Cargo.toml dependency snippet showsuniswap-sdk-core = "4.0.0", but the PR objectives indicate the package version was bumped to4.0.0-rc4. Please confirm if this discrepancy is intentional or update the snippet to reflect the correct version.
22-24: Usage code snippet looks good.
The Rust snippet correctly imports the prelude usinguse uniswap_sdk_core::prelude::*;and is clearly formatted.
32-34: Improved example description.
The explanation now clearly states that the example demonstrates creating a newTokeninstance using thetoken!macro, enhancing clarity for users.
36-36: Enhanced details tag for improved UX.
The updated<summary>tag (“Click to expand”) improves content accessibility by providing a clear action for users to reveal the example details.
39-41: Clear import documentation.
The updated comment (// Import necessary preludes and token macro) alongside the import statement makes it clear that both the prelude modules and the token macro are being imported.
42-48: Well-documented token configuration.
The constants defining the chain ID, token address, decimals, symbol, and name are clearly set up with descriptive inline comments. This provides an excellent example for users on how to configure a token.
49-51: Effective demonstration of macro usage.
Using thetoken!macro to create a newTokeninstance is shown clearly, reinforcing how the macro should be utilized in user code.
52-55: Good demonstration of token method usage.
The snippet illustrating the use of methods likeaddress()andis_native()is well presented and helps users understand how to interact with the token instance.
56-59: Clear example of token comparison.
The example showing token equality using two instances of the same token is clear and reinforces that two tokens instantiated identically are considered equal.
91-91: Improved license reference.
Changing the license link to[LICENSE](./LICENSE)ensures that users can reliably locate the license file from the repository root.
Upgraded
derive_moreto v2 with new features andfastnumto v0.1.11, adjusting included features. Increased package version to 4.0.0-rc4 to reflect these updates.Summary by CodeRabbit
4.0.0-rc4for improved system reliability.FxHashMaptoHashMapfor better performance consistency across various components.