Conversation
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests
Comment |
![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: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
crates/float/src/js_api.rs(1 hunks)crates/float/src/lib.rs(4 hunks)test_js/float.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 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: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:35-39
Timestamp: 2025-08-11T14:30:35.264Z
Learning: In Foundry/Forge test framework, the `bound()` function is pure - it performs mathematical clamping operations without reading contract state, so test functions using `bound()` can be marked as pure if all other operations in the function are also pure.
📚 Learning: 2025-06-16T13:19:32.009Z
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.
Applied to files:
test_js/float.test.ts
📚 Learning: 2025-04-25T03:58:01.307Z
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.
Applied to files:
test_js/float.test.ts
📚 Learning: 2025-06-18T09:10:41.740Z
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.
Applied to files:
crates/float/src/lib.rs
📚 Learning: 2025-06-21T11:07:19.190Z
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.
Applied to files:
crates/float/src/lib.rs
🧬 Code graph analysis (2)
crates/float/src/lib.rs (2)
crates/float/src/evm.rs (1)
execute_call(51-56)crates/float/src/error.rs (3)
from(79-84)from(88-90)from(94-96)
crates/float/src/js_api.rs (1)
crates/float/src/lib.rs (4)
max_positive_value(362-369)min_positive_value(394-401)max_negative_value(426-433)min_negative_value(459-466)
⏰ 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). (9)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: git-clean
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
🔇 Additional comments (4)
crates/float/src/lib.rs (2)
2-2: Import reorder is fine.No functional change; compiles cleanly.
4-4: Import update looks correct.
fixed_bytesmacro andU256are used; no issues.crates/float/src/js_api.rs (2)
293-380: JS wrappers for constants look correct and consistent.Signatures, naming, and error handling align with existing API shape.
293-380: No duplicate JS exports foundI’ve searched the entire repository and confirmed that each of
maxPositiveValue,minPositiveValue,maxNegativeValue, andminNegativeValueis exported exactly once via#[wasm_export(js_name = "...")]. There are no conflicting bindings.
Comment on lines
+338
to
+369
| /// Returns the maximum positive value that can be represented as a `Float`. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Ok(Float)` - The maximum positive value. | ||
| /// * `Err(FloatError)` - If the EVM call fails. | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ``` | ||
| /// use rain_math_float::Float; | ||
| /// | ||
| /// let max_pos = Float::max_positive_value()?; | ||
| /// let zero = Float::parse("0".to_string())?; | ||
| /// | ||
| /// // Max positive is greater than zero | ||
| /// assert!(max_pos.gt(zero)?); | ||
| /// | ||
| /// // Max positive is greater than any normal large number | ||
| /// let big_number = Float::parse("999999999999999999999".to_string())?; | ||
| /// assert!(max_pos.gt(big_number)?); | ||
| /// | ||
| /// anyhow::Ok(()) | ||
| /// ``` | ||
| pub fn max_positive_value() -> Result<Self, FloatError> { | ||
| let calldata = DecimalFloat::maxPositiveValueCall {}.abi_encode(); | ||
|
|
||
| execute_call(Bytes::from(calldata), |output| { | ||
| let decoded = DecimalFloat::maxPositiveValueCall::abi_decode_returns(output.as_ref())?; | ||
| Ok(Float(decoded)) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider caching the constant to avoid repeated EVM calls.
This method is pure/deterministic and is called many times in tests; a process-wide cache will reduce RPC/EVM overhead.
Apply caching via OnceLock:
@@
- pub fn max_positive_value() -> Result<Self, FloatError> {
- let calldata = DecimalFloat::maxPositiveValueCall {}.abi_encode();
-
- execute_call(Bytes::from(calldata), |output| {
- let decoded = DecimalFloat::maxPositiveValueCall::abi_decode_returns(output.as_ref())?;
- Ok(Float(decoded))
- })
- }
+ pub fn max_positive_value() -> Result<Self, FloatError> {
+ static MAX_POS: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
+ MAX_POS
+ .get_or_try_init(|| {
+ let calldata = DecimalFloat::maxPositiveValueCall {}.abi_encode();
+ execute_call(Bytes::from(calldata), |output| {
+ let decoded = DecimalFloat::maxPositiveValueCall::abi_decode_returns(output.as_ref())?;
+ Ok(Float(decoded))
+ })
+ })
+ .copied()
+ }Outside this hunk, add once near the other imports:
use std::sync::OnceLock; // if you prefer a single import; not required by the inline static above
Contributor
Author
There was a problem hiding this comment.
we don't have performance issues, so no need to optimize this for now
Contributor
There was a problem hiding this comment.
@0xgleb Understood! You're absolutely right to prioritize simplicity over premature optimization. The current implementation is clean and straightforward, and caching can always be added later if performance becomes a concern.
🧠 Learnings used
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: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
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.
Comment on lines
+371
to
+401
| /// Returns the minimum positive value that can be represented as a `Float`. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Ok(Float)` - The minimum positive value. | ||
| /// * `Err(FloatError)` - If the EVM call fails. | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ``` | ||
| /// use rain_math_float::Float; | ||
| /// | ||
| /// let min_pos = Float::min_positive_value()?; | ||
| /// let zero = Float::parse("0".to_string())?; | ||
| /// | ||
| /// // Min positive is greater than zero but smaller than any other positive number | ||
| /// assert!(min_pos.gt(zero)?); | ||
| /// | ||
| /// let small_number = Float::parse("0.000000000000000001".to_string())?; | ||
| /// assert!(min_pos.lt(small_number)?); | ||
| /// | ||
| /// anyhow::Ok(()) | ||
| /// ``` | ||
| pub fn min_positive_value() -> Result<Self, FloatError> { | ||
| let calldata = DecimalFloat::minPositiveValueCall {}.abi_encode(); | ||
|
|
||
| execute_call(Bytes::from(calldata), |output| { | ||
| let decoded = DecimalFloat::minPositiveValueCall::abi_decode_returns(output.as_ref())?; | ||
| Ok(Float(decoded)) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mirror caching for min_positive_value.
Same rationale as max_positive_value.
@@
- pub fn min_positive_value() -> Result<Self, FloatError> {
- let calldata = DecimalFloat::minPositiveValueCall {}.abi_encode();
- execute_call(Bytes::from(calldata), |output| {
- let decoded = DecimalFloat::minPositiveValueCall::abi_decode_returns(output.as_ref())?;
- Ok(Float(decoded))
- })
- }
+ pub fn min_positive_value() -> Result<Self, FloatError> {
+ static MIN_POS: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
+ MIN_POS
+ .get_or_try_init(|| {
+ let calldata = DecimalFloat::minPositiveValueCall {}.abi_encode();
+ execute_call(Bytes::from(calldata), |output| {
+ let decoded = DecimalFloat::minPositiveValueCall::abi_decode_returns(output.as_ref())?;
+ Ok(Float(decoded))
+ })
+ })
+ .copied()
+ }📝 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
| /// Returns the minimum positive value that can be represented as a `Float`. | |
| /// | |
| /// # Returns | |
| /// | |
| /// * `Ok(Float)` - The minimum positive value. | |
| /// * `Err(FloatError)` - If the EVM call fails. | |
| /// | |
| /// # Example | |
| /// | |
| /// ``` | |
| /// use rain_math_float::Float; | |
| /// | |
| /// let min_pos = Float::min_positive_value()?; | |
| /// let zero = Float::parse("0".to_string())?; | |
| /// | |
| /// // Min positive is greater than zero but smaller than any other positive number | |
| /// assert!(min_pos.gt(zero)?); | |
| /// | |
| /// let small_number = Float::parse("0.000000000000000001".to_string())?; | |
| /// assert!(min_pos.lt(small_number)?); | |
| /// | |
| /// anyhow::Ok(()) | |
| /// ``` | |
| pub fn min_positive_value() -> Result<Self, FloatError> { | |
| let calldata = DecimalFloat::minPositiveValueCall {}.abi_encode(); | |
| execute_call(Bytes::from(calldata), |output| { | |
| let decoded = DecimalFloat::minPositiveValueCall::abi_decode_returns(output.as_ref())?; | |
| Ok(Float(decoded)) | |
| }) | |
| } | |
| pub fn min_positive_value() -> Result<Self, FloatError> { | |
| static MIN_POS: std::sync::OnceLock<Float> = std::sync::OnceLock::new(); | |
| MIN_POS | |
| .get_or_try_init(|| { | |
| let calldata = DecimalFloat::minPositiveValueCall {}.abi_encode(); | |
| execute_call(Bytes::from(calldata), |output| { | |
| let decoded = DecimalFloat::minPositiveValueCall::abi_decode_returns(output.as_ref())?; | |
| Ok(Float(decoded)) | |
| }) | |
| }) | |
| .copied() | |
| } |
🤖 Prompt for AI Agents
In crates/float/src/lib.rs around lines 371 to 401, the min_positive_value()
function must use the same mirror caching pattern as max_positive_value: before
making the on-chain execute_call, check the mirror cache for a stored
min_positive_value and return it if present; if not present, perform the
execute_call, decode into a Float, store that Float into the mirror cache under
min_positive_value for future calls, and then return it. Ensure you follow the
exact cache access and storage API used by max_positive_value (same MIRROR/cache
struct and field names) and preserve error handling and return types.
Contributor
There was a problem hiding this comment.
@0xgleb Thank you for confirming! I've provided the implementation with std::sync::OnceLock caching in my previous suggestion for min_positive_value(). This will mirror the same optimization pattern used in max_positive_value() to avoid redundant EVM calls for these constant values.
The caching approach ensures that after the first call, subsequent calls to min_positive_value() will return the cached result immediately without additional blockchain execution overhead.
🧠 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#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.
Comment on lines
+403
to
+433
| /// Returns the maximum negative value that can be represented as a `Float`. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Ok(Float)` - The maximum negative value (closest to zero). | ||
| /// * `Err(FloatError)` - If the EVM call fails. | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ``` | ||
| /// use rain_math_float::Float; | ||
| /// | ||
| /// let max_neg = Float::max_negative_value()?; | ||
| /// let zero = Float::parse("0".to_string())?; | ||
| /// | ||
| /// // Max negative is less than zero but greater than any other negative number | ||
| /// assert!(max_neg.lt(zero)?); | ||
| /// | ||
| /// let small_negative = Float::parse("-0.000000000000000001".to_string())?; | ||
| /// assert!(max_neg.gt(small_negative)?); | ||
| /// | ||
| /// anyhow::Ok(()) | ||
| /// ``` | ||
| pub fn max_negative_value() -> Result<Self, FloatError> { | ||
| let calldata = DecimalFloat::maxNegativeValueCall {}.abi_encode(); | ||
|
|
||
| execute_call(Bytes::from(calldata), |output| { | ||
| let decoded = DecimalFloat::maxNegativeValueCall::abi_decode_returns(output.as_ref())?; | ||
| Ok(Float(decoded)) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mirror caching for max_negative_value.
@@
- pub fn max_negative_value() -> Result<Self, FloatError> {
- let calldata = DecimalFloat::maxNegativeValueCall {}.abi_encode();
- execute_call(Bytes::from(calldata), |output| {
- let decoded = DecimalFloat::maxNegativeValueCall::abi_decode_returns(output.as_ref())?;
- Ok(Float(decoded))
- })
- }
+ pub fn max_negative_value() -> Result<Self, FloatError> {
+ static MAX_NEG: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
+ MAX_NEG
+ .get_or_try_init(|| {
+ let calldata = DecimalFloat::maxNegativeValueCall {}.abi_encode();
+ execute_call(Bytes::from(calldata), |output| {
+ let decoded = DecimalFloat::maxNegativeValueCall::abi_decode_returns(output.as_ref())?;
+ Ok(Float(decoded))
+ })
+ })
+ .copied()
+ }🤖 Prompt for AI Agents
In crates/float/src/lib.rs around lines 403 to 433, max_negative_value()
currently performs an EVM call on every invocation and lacks the mirror (cached)
value pattern used elsewhere; change it to use a thread-safe lazy cache (e.g.
OnceCell/OnceLock or lazy_static) to store the computed Float on first
successful call and return a clone of the cached value on subsequent calls;
ensure the cached type is suitable (store Result<Float, FloatError> or only the
Float on success consistent with repo patterns), make the cache static, handle
synchronization, and preserve existing error propagation semantics.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
I understand you'd like the same caching approach applied to max_negative_value(). Let me check the current implementation and provide a comprehensive caching solution for all the constant methods.
@0xgleb I'll implement the same caching pattern for max_negative_value() and the other constant methods. Here's the implementation using std::sync::OnceLock for thread-safe lazy initialization:
/// Returns the maximum positive value that can be represented as a `Float`.
///
/// # Returns
///
/// * `Ok(Float)` - The maximum positive value.
/// * `Err(FloatError)` - If the EVM call fails.
///
/// # Example
///
/// ```
/// use rain_math_float::Float;
///
/// let max_pos = Float::max_positive_value()?;
/// let zero = Float::parse("0".to_string())?;
///
/// // Max positive is greater than zero
/// assert!(max_pos.gt(zero)?);
///
/// // Max positive is greater than any normal large number
/// let big_number = Float::parse("999999999999999999999".to_string())?;
/// assert!(max_pos.gt(big_number)?);
///
/// anyhow::Ok(())
/// ```
pub fn max_positive_value() -> Result<Self, FloatError> {
static MAX_POS: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MAX_POS
.get_or_try_init(|| {
let calldata = DecimalFloat::maxPositiveValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::maxPositiveValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}
/// Returns the minimum positive value that can be represented as a `Float`.
///
/// # Returns
///
/// * `Ok(Float)` - The minimum positive value.
/// * `Err(FloatError)` - If the EVM call fails.
///
/// # Example
///
/// ```
/// use rain_math_float::Float;
///
/// let min_pos = Float::min_positive_value()?;
/// let zero = Float::parse("0".to_string())?;
///
/// // Min positive is greater than zero but smaller than any other positive number
/// assert!(min_pos.gt(zero)?);
///
/// let small_number = Float::parse("0.000000000000000001".to_string())?;
/// assert!(min_pos.lt(small_number)?);
///
/// anyhow::Ok(())
/// ```
pub fn min_positive_value() -> Result<Self, FloatError> {
static MIN_POS: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MIN_POS
.get_or_try_init(|| {
let calldata = DecimalFloat::minPositiveValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::minPositiveValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}
/// Returns the maximum negative value that can be represented as a `Float`.
///
/// # Returns
///
/// * `Ok(Float)` - The maximum negative value (closest to zero).
/// * `Err(FloatError)` - If the EVM call fails.
///
/// # Example
///
/// ```
/// use rain_math_float::Float;
///
/// let max_neg = Float::max_negative_value()?;
/// let zero = Float::parse("0".to_string())?;
///
/// // Max negative is less than zero but greater than any other negative number
/// assert!(max_neg.lt(zero)?);
///
/// let small_negative = Float::parse("-0.000000000000000001".to_string())?;
/// assert!(max_neg.gt(small_negative)?);
///
/// anyhow::Ok(())
/// ```
pub fn max_negative_value() -> Result<Self, FloatError> {
static MAX_NEG: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MAX_NEG
.get_or_try_init(|| {
let calldata = DecimalFloat::maxNegativeValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::maxNegativeValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}
/// Returns the minimum negative value that can be represented as a `Float`.
///
/// # Returns
///
/// * `Ok(Float)` - The minimum negative value (furthest from zero).
/// * `Err(FloatError)` - If the EVM call fails.
///
/// # Example
///
/// ```
/// use rain_math_float::Float;
///
/// let min_neg = Float::min_negative_value()?;
/// let zero = Float::parse("0".to_string())?;
///
/// // Min negative is less than zero
/// assert!(min_neg.lt(zero)?);
///
/// // Min negative is less than any normal negative number
/// let big_negative = Float::parse("-999999999999999999999".to_string())?;
/// assert!(min_neg.lt(big_negative)?);
///
/// anyhow::Ok(())
/// ```
pub fn min_negative_value() -> Result<Self, FloatError> {
static MIN_NEG: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MIN_NEG
.get_or_try_init(|| {
let calldata = DecimalFloat::minNegativeValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::minNegativeValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}This caching approach:
- Uses
std::sync::OnceLock<Float>for thread-safe lazy initialization - Performs the expensive EVM call only once per constant
- Returns cached values on subsequent calls
- Preserves the existing error propagation semantics
- Uses
.copied()sinceFloatimplementsCopy
🐰
╱| |╲
( ◕ ◕ )
╲ ▾ ╱
╱╲ ╱╲
╱ ╲
🏁 Script executed:
#!/bin/bash
# Check if any of the constant methods already implement caching
rg -nA 10 -B 2 "OnceLock|lazy_static|Lazy" crates/float/src/lib.rs
# Also check for any existing caching patterns in the codebase
rg -nA 5 -B 2 "static.*:" crates/float/src/lib.rs
# Look for all the constant value methods to see what needs caching
rg -nA 15 "pub fn.*_value.*Result<Self, FloatError>" crates/float/src/lib.rs
Length of output: 2751
Perfect! I can see that none of the constant methods currently implement caching, and all 4 methods (max_positive_value, min_positive_value, max_negative_value, and min_negative_value) would benefit from it. Here's the complete caching implementation for all constant methods:
/// Returns the maximum positive value that can be represented as a `Float`.
///
/// # Returns
///
/// * `Ok(Float)` - The maximum positive value.
/// * `Err(FloatError)` - If the EVM call fails.
///
/// # Example
///
/// ```
/// use rain_math_float::Float;
///
/// let max_pos = Float::max_positive_value()?;
/// let zero = Float::parse("0".to_string())?;
///
/// // Max positive is greater than zero
/// assert!(max_pos.gt(zero)?);
///
/// // Max positive is greater than any normal large number
/// let big_number = Float::parse("999999999999999999999".to_string())?;
/// assert!(max_pos.gt(big_number)?);
///
/// anyhow::Ok(())
/// ```
pub fn max_positive_value() -> Result<Self, FloatError> {
static MAX_POS: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MAX_POS
.get_or_try_init(|| {
let calldata = DecimalFloat::maxPositiveValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::maxPositiveValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}
/// Returns the minimum positive value that can be represented as a `Float`.
///
/// # Returns
///
/// * `Ok(Float)` - The minimum positive value.
/// * `Err(FloatError)` - If the EVM call fails.
///
/// # Example
///
/// ```
/// use rain_math_float::Float;
///
/// let min_pos = Float::min_positive_value()?;
/// let zero = Float::parse("0".to_string())?;
///
/// // Min positive is greater than zero but smaller than any other positive number
/// assert!(min_pos.gt(zero)?);
///
/// let small_number = Float::parse("0.000000000000000001".to_string())?;
/// assert!(min_pos.lt(small_number)?);
///
/// anyhow::Ok(())
/// ```
pub fn min_positive_value() -> Result<Self, FloatError> {
static MIN_POS: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MIN_POS
.get_or_try_init(|| {
let calldata = DecimalFloat::minPositiveValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::minPositiveValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}
/// Returns the maximum negative value that can be represented as a `Float`.
///
/// # Returns
///
/// * `Ok(Float)` - The maximum negative value (closest to zero).
/// * `Err(FloatError)` - If the EVM call fails.
///
/// # Example
///
/// ```
/// use rain_math_float::Float;
///
/// let max_neg = Float::max_negative_value()?;
/// let zero = Float::parse("0".to_string())?;
///
/// // Max negative is less than zero but greater than any other negative number
/// assert!(max_neg.lt(zero)?);
///
/// let small_negative = Float::parse("-0.000000000000000001".to_string())?;
/// assert!(max_neg.gt(small_negative)?);
///
/// anyhow::Ok(())
/// ```
pub fn max_negative_value() -> Result<Self, FloatError> {
static MAX_NEG: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MAX_NEG
.get_or_try_init(|| {
let calldata = DecimalFloat::maxNegativeValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::maxNegativeValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}
/// Returns the minimum negative value that can be represented as a `Float`.
///
/// # Returns
///
/// * `Ok(Float)` - The minimum negative value (furthest from zero).
/// * `Err(FloatError)` - If the EVM call fails.
///
/// # Example
///
/// ```
/// use rain_math_float::Float;
///
/// let min_neg = Float::min_negative_value()?;
/// let zero = Float::parse("0".to_string())?;
///
/// // Min negative is less than zero
/// assert!(min_neg.lt(zero)?);
///
/// // Min negative is less than any normal negative number
/// let big_negative = Float::parse("-999999999999999999999".to_string())?;
/// assert!(min_neg.lt(big_negative)?);
///
/// anyhow::Ok(())
/// ```
pub fn min_negative_value() -> Result<Self, FloatError> {
static MIN_NEG: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MIN_NEG
.get_or_try_init(|| {
let calldata = DecimalFloat::minNegativeValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::minNegativeValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}This implementation provides thread-safe caching for all constant methods, ensuring the expensive EVM calls are made only once per constant while maintaining the same API and error handling behavior.
🐰
╱| |╲
( ◐ ◐ )
╲ ω ╱
╱╲ ╱╲
╱ ╲
🧠 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#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.
Comment on lines
+435
to
+466
| /// Returns the minimum negative value that can be represented as a `Float`. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Ok(Float)` - The minimum negative value (furthest from zero). | ||
| /// * `Err(FloatError)` - If the EVM call fails. | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ``` | ||
| /// use rain_math_float::Float; | ||
| /// | ||
| /// let min_neg = Float::min_negative_value()?; | ||
| /// let zero = Float::parse("0".to_string())?; | ||
| /// | ||
| /// // Min negative is less than zero | ||
| /// assert!(min_neg.lt(zero)?); | ||
| /// | ||
| /// // Min negative is less than any normal negative number | ||
| /// let big_negative = Float::parse("-999999999999999999999".to_string())?; | ||
| /// assert!(min_neg.lt(big_negative)?); | ||
| /// | ||
| /// anyhow::Ok(()) | ||
| /// ``` | ||
| pub fn min_negative_value() -> Result<Self, FloatError> { | ||
| let calldata = DecimalFloat::minNegativeValueCall {}.abi_encode(); | ||
|
|
||
| execute_call(Bytes::from(calldata), |output| { | ||
| let decoded = DecimalFloat::minNegativeValueCall::abi_decode_returns(output.as_ref())?; | ||
| Ok(Float(decoded)) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mirror caching for min_negative_value.
@@
- pub fn min_negative_value() -> Result<Self, FloatError> {
- let calldata = DecimalFloat::minNegativeValueCall {}.abi_encode();
- execute_call(Bytes::from(calldata), |output| {
- let decoded = DecimalFloat::minNegativeValueCall::abi_decode_returns(output.as_ref())?;
- Ok(Float(decoded))
- })
- }
+ pub fn min_negative_value() -> Result<Self, FloatError> {
+ static MIN_NEG: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
+ MIN_NEG
+ .get_or_try_init(|| {
+ let calldata = DecimalFloat::minNegativeValueCall {}.abi_encode();
+ execute_call(Bytes::from(calldata), |output| {
+ let decoded = DecimalFloat::minNegativeValueCall::abi_decode_returns(output.as_ref())?;
+ Ok(Float(decoded))
+ })
+ })
+ .copied()
+ }Committable suggestion skipped: line range outside the PR's diff.
Contributor
There was a problem hiding this comment.
@0xgleb I'll apply the same caching pattern to min_negative_value():
pub fn min_negative_value() -> Result<Self, FloatError> {
static MIN_NEG: std::sync::OnceLock<Float> = std::sync::OnceLock::new();
MIN_NEG
.get_or_try_init(|| {
let calldata = DecimalFloat::minNegativeValueCall {}.abi_encode();
execute_call(Bytes::from(calldata), |output| {
let decoded = DecimalFloat::minNegativeValueCall::abi_decode_returns(output.as_ref())?;
Ok(Float(decoded))
})
})
.copied()
}This completes the caching optimization for all four constant methods, ensuring they only make the expensive EVM call once per program execution.
/| /|
( :v: )
|(_)|
🧠 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#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.
Comment on lines
+1194
to
+1231
| #[test] | ||
| fn test_float_constants() { | ||
| // Test that all constant methods return valid floats | ||
| let max_pos = Float::max_positive_value().unwrap(); | ||
| let min_pos = Float::min_positive_value().unwrap(); | ||
| let max_neg = Float::max_negative_value().unwrap(); | ||
| let min_neg = Float::min_negative_value().unwrap(); | ||
|
|
||
| // Verify they return expected hex values (these are extreme values that may not format well) | ||
| assert_eq!( | ||
| max_pos.as_hex(), | ||
| "0x7fffffff7fffffffffffffffffffffffffffffffffffffffffffffffffffffff" | ||
| ); | ||
| assert_eq!( | ||
| min_pos.as_hex(), | ||
| "0x8000000000000000000000000000000000000000000000000000000000000001" | ||
| ); | ||
| assert_eq!( | ||
| max_neg.as_hex(), | ||
| "0x80000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff" | ||
| ); | ||
| assert_eq!( | ||
| min_neg.as_hex(), | ||
| "0x7fffffff80000000000000000000000000000000000000000000000000000000" | ||
| ); | ||
|
|
||
| // Verify logical relationships between constants | ||
| let zero = Float::parse("0".to_string()).unwrap(); | ||
|
|
||
| // max_pos > min_pos (but these extreme values might not compare properly) | ||
| // Instead just verify they are different values | ||
| assert_ne!(max_pos.as_hex(), min_pos.as_hex()); | ||
| assert_ne!(max_neg.as_hex(), min_neg.as_hex()); | ||
|
|
||
| // Test that we can use these constants in basic operations | ||
| assert!(min_pos.gt(zero).unwrap()); // min positive should be > 0 | ||
| assert!(max_neg.lt(zero).unwrap()); // max negative should be < 0 | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Hex-equality tests are fine; consider guarding against upstream encoding changes.
If the on-chain encoding changes, these will break noisily. Optionally gate the exact-hex asserts behind a feature flag and keep the ordering asserts unconditional.
🤖 Prompt for AI Agents
In crates/float/src/lib.rs around lines 1194-1231, the test's strict
hex-equality asserts will break if upstream encoding changes; gate the exact-hex
asserts behind a cargo feature (e.g. "exact-hex-consts") while leaving the
ordering/difference and comparison asserts unconditional. Concretely, wrap only
the four assert_eq!(... .as_hex(), "...") blocks in a cfg(feature =
"exact-hex-consts") conditional and keep the assert_ne! and gt/lt checks as-is;
also add the new feature to Cargo.toml (optional default = false) so CI can opt
into exact-hex checks.
Comment on lines
+1911
to
+1974
| proptest! { | ||
| #[test] | ||
| fn test_constants_relationships(float in reasonable_float()) { | ||
| let max_pos = Float::max_positive_value().unwrap(); | ||
| let min_pos = Float::min_positive_value().unwrap(); | ||
| let max_neg = Float::max_negative_value().unwrap(); | ||
| let min_neg = Float::min_negative_value().unwrap(); | ||
| let zero = Float::parse("0".to_string()).unwrap(); | ||
|
|
||
| // Test that constants are the extremes | ||
| // Any reasonable positive float should be <= max_positive and >= min_positive | ||
| if float.gt(zero).unwrap() { | ||
| prop_assert!(float.lte(max_pos).unwrap()); | ||
| prop_assert!(float.gte(min_pos).unwrap()); | ||
| } | ||
|
|
||
| // Any reasonable negative float should be <= max_negative and >= min_negative | ||
| // (max_negative is closest to zero, min_negative is furthest from zero) | ||
| if float.lt(zero).unwrap() { | ||
| prop_assert!(float.lte(max_neg).unwrap()); | ||
| prop_assert!(float.gte(min_neg).unwrap()); | ||
| } | ||
|
|
||
| // Constants should be consistent regardless of arbitrary float | ||
| prop_assert!(max_pos.gt(zero).unwrap()); | ||
| prop_assert!(min_pos.gt(zero).unwrap()); | ||
| prop_assert!(max_neg.lt(zero).unwrap()); | ||
| prop_assert!(min_neg.lt(zero).unwrap()); | ||
|
|
||
| // Verify constants maintain their ordering | ||
| prop_assert!(min_pos.lt(max_pos).unwrap()); | ||
| prop_assert!(min_neg.lt(max_neg).unwrap()); | ||
| prop_assert!(max_neg.lt(zero).unwrap()); | ||
| prop_assert!(min_pos.gt(zero).unwrap()); | ||
| } | ||
| } | ||
|
|
||
| proptest! { | ||
| #[test] | ||
| fn test_constants_edge_cases(float in arb_float()) { | ||
| let max_pos = Float::max_positive_value().unwrap(); | ||
| let min_pos = Float::min_positive_value().unwrap(); | ||
| let max_neg = Float::max_negative_value().unwrap(); | ||
| let min_neg = Float::min_negative_value().unwrap(); | ||
|
|
||
| // Constants should always be distinct | ||
| prop_assert!(!max_pos.eq(min_pos).unwrap()); | ||
| prop_assert!(!max_neg.eq(min_neg).unwrap()); | ||
| prop_assert!(!max_pos.eq(max_neg).unwrap()); | ||
| prop_assert!(!min_pos.eq(min_neg).unwrap()); | ||
|
|
||
| // Test that constants are at the boundaries | ||
| // (Note: We can't test arithmetic operations that would overflow/underflow | ||
| // since those would fail, but we can test comparisons) | ||
|
|
||
| // No arbitrary float should be greater than max_pos or less than min_neg | ||
| if !float.eq(max_pos).unwrap() { | ||
| prop_assert!(!float.gt(max_pos).unwrap()); | ||
| } | ||
| if !float.eq(min_neg).unwrap() { | ||
| prop_assert!(!float.lt(min_neg).unwrap()); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Reduce proptest overhead by fetching constants once.
Each case re-calls four EVM endpoints. Cache once per process to speed up the suite.
Example for the test module (outside this hunk):
use std::sync::OnceLock;
static CONSTANTS: OnceLock<(Float, Float, Float, Float)> = OnceLock::new();
fn constants() -> (Float, Float, Float, Float) {
*CONSTANTS.get_or_init(|| {
(
Float::max_positive_value().unwrap(),
Float::min_positive_value().unwrap(),
Float::max_negative_value().unwrap(),
Float::min_negative_value().unwrap(),
)
})
}Then inside the property tests:
- let max_pos = Float::max_positive_value().unwrap();
- let min_pos = Float::min_positive_value().unwrap();
- let max_neg = Float::max_negative_value().unwrap();
- let min_neg = Float::min_negative_value().unwrap();
+ let (max_pos, min_pos, max_neg, min_neg) = constants();
Contributor
Author
There was a problem hiding this comment.
we don't have performance issues, so no need to optimize this for now
Contributor
There was a problem hiding this comment.
@0xgleb Understood! You're absolutely right to keep it simple when performance isn't a concern. The current approach is clear and straightforward. Thanks for the clarification!
🐰
🧠 Learnings used
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#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#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.
findolor
approved these changes
Aug 28, 2025
hardyjosh
approved these changes
Sep 8, 2025
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll analyze the PR to assess its size classification. Let me examine the changes made in this pull request. 🏁 Script executed: Length of output: 16814 Looking at the PR changes, I can see this involves:
This represents a focused feature addition that touches multiple files, requires domain expertise to review properly, includes comprehensive test coverage, and would take approximately 15-30 minutes to review thoroughly. SIZE=M |
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
See #93
Solution
Expose and test Solidity methods for offchain use
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Documentation
Tests