Conversation
Contributor
## Walkthrough
The `Float` struct now derives the `Hash` trait and `Tsify` for TypeScript bindings, with wasm-bindgen traits implemented for JavaScript interoperability. Methods `as_hex` and `from_hex` were added for hex string conversion, and `format18` for truncated decimal formatting. The `FloatError` enum gained an `InvalidHex` variant. Tests were updated and expanded.
## Changes
| File(s) | Change Summary |
|-----------------------------|-------------------------------------------------------------------------------------------------------------|
| crates/float/src/lib.rs | Derived `Hash` and `Tsify` for `Float`; implemented wasm-bindgen traits; added `as_hex`, `from_hex`, and `format18` methods; clarified `format` docs; renamed and added unit/property tests. |
| crates/float/src/error.rs | Added `InvalidHex(String)` variant to `FloatError` enum for invalid hex string errors. |
| crates/float/Cargo.toml | Added new dependency `wasm-bindgen-utils = "0.0.10"`. |
## Possibly related PRs
- rainlanguage/rain.math.float#46: Initial Rust `Float` library implementation interfacing with Solidity contracts; this PR builds upon that foundation.
## Suggested reviewers
- hardyjosh
- findolor
- thedavidmeisterWarning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Clippy (1.86.0)warning: failed to write cache, path: /usr/local/registry/index/index.crates.io-1949cf8c6b5b557f/.cache/al/lo/alloy, error: Permission denied (os error 13) Caused by: 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningscrates/float/src/lib.rs (15)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (7)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
crates/float/src/lib.rs(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:233-242
Timestamp: 2025-06-17T10:17:56.205Z
Learning: In the rainlanguage/rain.math.float repository, the maintainer 0xgleb prefers to handle documentation additions and improvements in separate issues rather than inline with feature PRs.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: src/concrete/DecimalFloat.sol:175-182
Timestamp: 2025-06-16T13:17:28.513Z
Learning: In the rainlanguage/rain.math.float codebase, there's an established naming convention where functions accepting a `Float` type parameter consistently use `float` as the parameter name, even though it shadows the type name. This pattern is used throughout `LibDecimalFloat.sol` and should be maintained for consistency in related contracts like `DecimalFloat.sol`.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#70
File: crates/float/src/evm.rs:38-43
Timestamp: 2025-07-03T11:20:50.456Z
Learning: In the rainlanguage/rain.math.float codebase, the user 0xgleb prefers not to add explanatory comments for well-established Rust idioms like the double `?` pattern, as these are self-explanatory to experienced Rust developers and don't need over-commenting.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:447-461
Timestamp: 2025-06-17T10:11:32.740Z
Learning: In the rainlanguage/rain.math.float repository, the user 0xgleb prefers using .unwrap() over ? in tests because it provides clearer stack traces showing the exact line where panics occurred, making debugging easier.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#30
File: test/src/lib/LibDecimalFloat.gt.t.sol:33-36
Timestamp: 2025-04-25T03:58:01.307Z
Learning: In the rain.math.float library, all values of `Float` (which is a type alias for bytes32) are considered valid and can be safely used with methods like gt(), lt(), or eq() without causing reverts.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#57
File: crates/float/src/lib.rs:316-328
Timestamp: 2025-06-18T09:10:41.740Z
Learning: In the rain.math.float codebase, the user prefers implementing standard Rust traits (like Neg) rather than creating redundant public methods when the trait already provides the needed functionality. Float implements Copy, so reference usage with operators is not a concern.
crates/float/src/lib.rs (10)
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#57
File: crates/float/src/lib.rs:316-328
Timestamp: 2025-06-18T09:10:41.740Z
Learning: In the rain.math.float codebase, the user prefers implementing standard Rust traits (like Neg) rather than creating redundant public methods when the trait already provides the needed functionality. Float implements Copy, so reference usage with operators is not a concern.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: src/concrete/DecimalFloat.sol:175-182
Timestamp: 2025-06-16T13:17:28.513Z
Learning: In the rainlanguage/rain.math.float codebase, there's an established naming convention where functions accepting a `Float` type parameter consistently use `float` as the parameter name, even though it shadows the type name. This pattern is used throughout `LibDecimalFloat.sol` and should be maintained for consistency in related contracts like `DecimalFloat.sol`.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#70
File: crates/float/src/evm.rs:38-43
Timestamp: 2025-07-03T11:20:50.456Z
Learning: In the rainlanguage/rain.math.float codebase, the user 0xgleb prefers not to add explanatory comments for well-established Rust idioms like the double `?` pattern, as these are self-explanatory to experienced Rust developers and don't need over-commenting.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#46
File: crates/float/proptest-regressions/lib.txt:1-3
Timestamp: 2025-06-11T09:03:20.771Z
Learning: The file `crates/float/proptest-regressions/lib.txt` is automatically generated by proptest and should not be manually edited.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#30
File: test/src/lib/LibDecimalFloat.gt.t.sol:33-36
Timestamp: 2025-04-25T03:58:01.307Z
Learning: In the rain.math.float library, all values of `Float` (which is a type alias for bytes32) are considered valid and can be safely used with methods like gt(), lt(), or eq() without causing reverts.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#68
File: crates/float/src/lib.rs:134-135
Timestamp: 2025-06-21T11:07:19.190Z
Learning: In Alloy primitives library, B256 is a type alias for FixedBytes<32> (pub type B256 = FixedBytes<32>), making them completely interchangeable in type usage. Changing between these types is not a breaking change.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:447-461
Timestamp: 2025-06-17T10:11:32.740Z
Learning: In the rainlanguage/rain.math.float repository, the user 0xgleb prefers using .unwrap() over ? in tests because it provides clearer stack traces showing the exact line where panics occurred, making debugging easier.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: crates/float/src/lib.rs:382-401
Timestamp: 2025-06-16T13:19:32.009Z
Learning: In the rainlanguage/rain.math.float codebase, variable shadowing in property tests like test_lt_eq_gt_with_add is acceptable when it represents a logical progression of the same conceptual value being tested through different states.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
🔇 Additional comments (4)
crates/float/src/lib.rs (4)
21-21: LGTM: Hash trait addition is appropriate.Adding the Hash trait to Float is a useful enhancement that enables using Float values as keys in HashMap or elements in HashSet collections.
164-165: LGTM: Documentation clarification is helpful.The updated documentation clearly explains the 18 decimal place limitation and failure behavior, which addresses the core issue mentioned in the PR objective.
810-815: LGTM: Test rename and equality method update improve clarity.The rename to
test_format_parsebetter reflects the test flow, and using theeqmethod for comparison is more appropriate than direct field comparison.
817-835: LGTM: Comprehensive test coverage for format18.The tests effectively cover both the truncation behavior (with specific examples) and round-trip compatibility (with property testing). The test cases demonstrate the method works correctly for both exact 18-decimal inputs and inputs requiring truncation.
Comment on lines
+192
to
+216
| /// Formats the float as a decimal string. Gets truncated to 18 decimal | ||
| /// places if it has more than that. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Ok(String)` - The formatted string. | ||
| /// * `Err(FloatError)` - If formatting fails. | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ``` | ||
| /// use rain_math_float::Float; | ||
| /// | ||
| /// let float = Float::parse("2.5".to_string())?; | ||
| /// assert_eq!(float.format18()?, "2.5"); | ||
| /// | ||
| /// anyhow::Ok(()) | ||
| /// ``` | ||
| pub fn format18(self) -> Result<String, FloatError> { | ||
| let ten_to_18 = Self::from_fixed_decimal(U256::from(10u64).pow(U256::from(18)), 0)?; | ||
| let multiplied = (self * ten_to_18)?; | ||
| let floored = multiplied.floor()?; | ||
| let divided = (floored / ten_to_18)?; | ||
| divided.format() | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM: format18 implementation is mathematically sound.
The implementation correctly truncates to 18 decimal places using the multiply-floor-divide approach. The method provides the safer alternative to format as intended by the PR objective.
Consider caching the ten_to_18 constant to avoid recreating it on each call:
+ const TEN_TO_18: Float = Float(B256::from_be_bytes([/* pre-computed value */]));
+
pub fn format18(self) -> Result<String, FloatError> {
- let ten_to_18 = Self::from_fixed_decimal(U256::from(10u64).pow(U256::from(18)), 0)?;
- let multiplied = (self * ten_to_18)?;
+ let multiplied = (self * Self::TEN_TO_18)?;
let floored = multiplied.floor()?;
- let divided = (floored / ten_to_18)?;
+ let divided = (floored / Self::TEN_TO_18)?;
divided.format()
}This would improve performance by avoiding the conversion calculation on each call.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Formats the float as a decimal string. Gets truncated to 18 decimal | |
| /// places if it has more than that. | |
| /// | |
| /// # Returns | |
| /// | |
| /// * `Ok(String)` - The formatted string. | |
| /// * `Err(FloatError)` - If formatting fails. | |
| /// | |
| /// # Example | |
| /// | |
| /// ``` | |
| /// use rain_math_float::Float; | |
| /// | |
| /// let float = Float::parse("2.5".to_string())?; | |
| /// assert_eq!(float.format18()?, "2.5"); | |
| /// | |
| /// anyhow::Ok(()) | |
| /// ``` | |
| pub fn format18(self) -> Result<String, FloatError> { | |
| let ten_to_18 = Self::from_fixed_decimal(U256::from(10u64).pow(U256::from(18)), 0)?; | |
| let multiplied = (self * ten_to_18)?; | |
| let floored = multiplied.floor()?; | |
| let divided = (floored / ten_to_18)?; | |
| divided.format() | |
| } | |
| /// Formats the float as a decimal string. Gets truncated to 18 decimal | |
| /// places if it has more than that. | |
| /// | |
| /// # Returns | |
| /// | |
| /// * `Ok(String)` - The formatted string. | |
| /// * `Err(FloatError)` - If formatting fails. | |
| /// | |
| /// # Example | |
| /// | |
| /// |
🤖 Prompt for AI Agents
In crates/float/src/lib.rs around lines 192 to 216, the format18 method
recreates the constant ten_to_18 on every call, which is inefficient. To fix
this, define ten_to_18 as a static or lazy-initialized constant outside the
method so it is computed only once and reused on subsequent calls, improving
performance by avoiding repeated calculations.
Contributor
Author
There was a problem hiding this comment.
We don't need to optimize this for now
Contributor
There was a problem hiding this comment.
@0xgleb Understood! No optimization needed at this time. The current implementation is clear and functional as-is.
🧠 Learnings used
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#57
File: crates/float/src/lib.rs:316-328
Timestamp: 2025-06-18T09:10:41.740Z
Learning: In the rain.math.float codebase, the user prefers implementing standard Rust traits (like Neg) rather than creating redundant public methods when the trait already provides the needed functionality. Float implements Copy, so reference usage with operators is not a concern.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#70
File: crates/float/src/evm.rs:38-43
Timestamp: 2025-07-03T11:20:50.456Z
Learning: In the rainlanguage/rain.math.float codebase, the user 0xgleb prefers not to add explanatory comments for well-established Rust idioms like the double `?` pattern, as these are self-explanatory to experienced Rust developers and don't need over-commenting.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: src/concrete/DecimalFloat.sol:175-182
Timestamp: 2025-06-16T13:17:28.513Z
Learning: In the rainlanguage/rain.math.float codebase, there's an established naming convention where functions accepting a `Float` type parameter consistently use `float` as the parameter name, even though it shadows the type name. This pattern is used throughout `LibDecimalFloat.sol` and should be maintained for consistency in related contracts like `DecimalFloat.sol`.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:447-461
Timestamp: 2025-06-17T10:11:32.740Z
Learning: In the rainlanguage/rain.math.float repository, the user 0xgleb prefers using .unwrap() over ? in tests because it provides clearer stack traces showing the exact line where panics occurred, making debugging easier.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: crates/float/src/lib.rs:201-232
Timestamp: 2025-06-16T13:14:38.431Z
Learning: In the rain.math.float Rust crate, the `execute_call` function already provides sufficient abstraction for EVM contract calls by handling execution boilerplate, error handling, and result processing. Individual methods like `lt`, `eq`, `gt` only need to handle their specific call encoding and result decoding, making further abstraction unnecessary.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:233-242
Timestamp: 2025-06-17T10:17:56.205Z
Learning: In the rainlanguage/rain.math.float repository, the maintainer 0xgleb prefers to handle documentation additions and improvements in separate issues rather than inline with feature PRs.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#30
File: test/src/lib/LibDecimalFloat.gt.t.sol:33-36
Timestamp: 2025-04-25T03:58:01.307Z
Learning: In the rain.math.float library, all values of `Float` (which is a type alias for bytes32) are considered valid and can be safely used with methods like gt(), lt(), or eq() without causing reverts.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#46
File: crates/float/proptest-regressions/lib.txt:1-3
Timestamp: 2025-06-11T09:03:20.771Z
Learning: The file `crates/float/proptest-regressions/lib.txt` is automatically generated by proptest and should not be manually edited.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.
4 tasks
findolor
approved these changes
Jul 14, 2025
hardyjosh
approved these changes
Jul 15, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Regular
formatreverts when a float has more than 18 decimal places, which might be the case for IO ratiosSolution
Add
format18that truncates the float before formattingChecks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests