expose zero #129
Merged
expose zero #129
Conversation
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
Contributor
WalkthroughAdds a Float.zero() Rust method that performs an EVM call to obtain the zero value, a wasm-exported Float.zero() JS binding, and corresponding tests. Several insertions are duplicated: duplicate Rust implementations, duplicate wasm-exported methods, and duplicate JS test blocks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor JSCaller as "JS caller"
participant JSBinding as "Float (wasm JS export)"
participant RustImpl as "Float::zero() (Rust)"
participant EVM as "EVM (eth_call)"
participant Decoder as "ABI Decoder"
JSCaller->>JSBinding: Float.zero() (wasm)
JSBinding->>RustImpl: call Float::zero()
RustImpl->>EVM: DecimalFloat::zeroCall() (eth_call)
EVM-->>RustImpl: encoded return data
RustImpl->>Decoder: ABI decode returned data
Decoder-->>RustImpl: decoded zero value
RustImpl-->>JSBinding: Ok(Float(0))
JSBinding-->>JSCaller: Ok(Float(0))
note over JSBinding: `zero` exported via wasm (duplicate definitions present)
note over RustImpl: `zero()` implemented (duplicate insertion present)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-06-18T09:10:41.740ZApplied to files:
📚 Learning: 2025-07-03T11:20:50.456ZApplied to files:
📚 Learning: 2025-06-17T10:17:56.205ZApplied to files:
📚 Learning: 2025-06-17T10:11:32.740ZApplied to files:
📚 Learning: 2025-08-26T15:50:31.262ZApplied to files:
🧬 Code graph analysis (1)crates/float/src/lib.rs (1)
⏰ 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). (8)
🔇 Additional comments (1)
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
crates/float/src/js_api.rs(1 hunks)crates/float/src/lib.rs(2 hunks)test_js/float.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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
🧬 Code graph analysis (3)
crates/float/src/lib.rs (1)
crates/float/src/evm.rs (1)
execute_call(51-56)
test_js/float.test.ts (1)
crates/float/src/lib.rs (1)
zero(490-497)
crates/float/src/js_api.rs (1)
crates/float/src/lib.rs (1)
zero(490-497)
⏰ 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). (6)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
🔇 Additional comments (1)
crates/float/src/lib.rs (1)
1133-1146: Rust test for zero(): LGTMCovers is_zero, formatting, equality to parsed zero and Default.
Comment on lines
+381
to
+403
| /// Returns the zero value of a `Float` in its maximized representation. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Ok(Float)` - The zero value. | ||
| /// * `Err(FloatError)` - If the EVM call fails. | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ```typescript | ||
| /// const zeroResult = Float.zero(); | ||
| /// if (zeroResult.error) { | ||
| /// console.error(zeroResult.error); | ||
| /// } | ||
| /// const zero = zeroResult.value; | ||
| /// assert(zero.isZero().value); | ||
| /// assert(zero.format().value === "0"); | ||
| /// ``` | ||
| #[wasm_export(js_name = "zero", preserve_js_class)] | ||
| pub fn zero_js() -> Result<Float, FloatError> { | ||
| Self::zero() | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
JS export zero(): good; fix example to match Result-wrapped API
Docs show direct property access; other examples use .value/.error. Make this consistent to avoid confusion.
- /// const zeroResult = Float.zero();
- /// if (zeroResult.error) {
- /// console.error(zeroResult.error);
- /// }
- /// const zero = zeroResult.value;
- /// assert(zero.isZero().value);
- /// assert(zero.format().value === "0");
+ /// const zeroResult = Float.zero();
+ /// if (zeroResult.error) {
+ /// console.error(zeroResult.error);
+ /// }
+ /// const zero = zeroResult.value!;
+ /// assert(zero.isZero()?.value!);
+ /// assert(zero.format()?.value! === "0");📝 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 zero value of a `Float` in its maximized representation. | |
| /// | |
| /// # Returns | |
| /// | |
| /// * `Ok(Float)` - The zero value. | |
| /// * `Err(FloatError)` - If the EVM call fails. | |
| /// | |
| /// # Example | |
| /// | |
| /// ```typescript | |
| /// const zeroResult = Float.zero(); | |
| /// if (zeroResult.error) { | |
| /// console.error(zeroResult.error); | |
| /// } | |
| /// const zero = zeroResult.value; | |
| /// assert(zero.isZero().value); | |
| /// assert(zero.format().value === "0"); | |
| /// ``` | |
| #[wasm_export(js_name = "zero", preserve_js_class)] | |
| pub fn zero_js() -> Result<Float, FloatError> { | |
| Self::zero() | |
| } | |
| /// Returns the zero value of a `Float` in its maximized representation. | |
| /// | |
| /// # Returns | |
| /// | |
| /// * `Ok(Float)` - The zero value. | |
| /// * `Err(FloatError)` - If the EVM call fails. | |
| /// | |
| /// # Example | |
| /// | |
| /// |
🤖 Prompt for AI Agents
In crates/float/src/js_api.rs around lines 381 to 403, the TypeScript example
for Float.zero() uses direct property access instead of the Result-wrapped API
used elsewhere; update the example to handle the returned Result object (check
zeroResult.error and use zeroResult.value) and then call methods on the
unwrapped value using their `.value` fields (e.g., const zeroResult =
Float.zero(); if (zeroResult.error) { console.error(zeroResult.error); } const
zero = zeroResult.value; assert(zero.isZero().value); assert(zero.format().value
=== "0");) so the snippet consistently reflects the .value/.error pattern.
Contributor
Author
There was a problem hiding this comment.
It still uses .value/.error, so it's fine as is
Contributor
There was a problem hiding this comment.
@0xgleb You're absolutely right - the example already properly demonstrates the Result pattern with .value/.error handling. The current documentation is clear and consistent with the API. Thanks for the clarification!
🐰💭
🧠 Learnings used
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.
Comment on lines
+468
to
+498
| /// Returns the zero value of a `Float` in its maximized representation. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Ok(Float)` - The zero value. | ||
| /// * `Err(FloatError)` - If the EVM call fails. | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ``` | ||
| /// use rain_math_float::Float; | ||
| /// | ||
| /// let zero = Float::zero()?; | ||
| /// assert!(zero.is_zero()?); | ||
| /// assert_eq!(zero.format()?, "0"); | ||
| /// | ||
| /// // Should be equal to parsed zero | ||
| /// let parsed_zero = Float::parse("0".to_string())?; | ||
| /// assert!(zero.eq(parsed_zero)?); | ||
| /// | ||
| /// anyhow::Ok(()) | ||
| /// ``` | ||
| pub fn zero() -> Result<Self, FloatError> { | ||
| let calldata = DecimalFloat::zeroCall {}.abi_encode(); | ||
|
|
||
| execute_call(Bytes::from(calldata), |output| { | ||
| let decoded = DecimalFloat::zeroCall::abi_decode_returns(output.as_ref())?; | ||
| Ok(Float(decoded)) | ||
| }) | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
zero(): correct and consistent; document Default equivalence and consider caching
Implementation mirrors other constants. To align with prior preference for standard traits, explicitly note equivalence to Default and (optionally) cache the value to avoid repeated EVM calls in-process.
Doc tweak within this block:
- /// Returns the zero value of a `Float` in its maximized representation.
+ /// Returns the zero value of a `Float` in its maximized representation.
+ ///
+ /// Note: This is equivalent to `Default::default()` for `Float`. It is provided to mirror the
+ /// on-chain API and other constant constructors.
@@
- /// anyhow::Ok(())
+ /// // Equal to Rust default as well
+ /// assert!(zero.eq(Float::default())?);
+ ///
+ /// anyhow::Ok(())Optional caching (outside this hunk): use a OnceLock to memoize the decoded zero. Happy to draft it if you want it in this PR.
📝 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 zero value of a `Float` in its maximized representation. | |
| /// | |
| /// # Returns | |
| /// | |
| /// * `Ok(Float)` - The zero value. | |
| /// * `Err(FloatError)` - If the EVM call fails. | |
| /// | |
| /// # Example | |
| /// | |
| /// ``` | |
| /// use rain_math_float::Float; | |
| /// | |
| /// let zero = Float::zero()?; | |
| /// assert!(zero.is_zero()?); | |
| /// assert_eq!(zero.format()?, "0"); | |
| /// | |
| /// // Should be equal to parsed zero | |
| /// let parsed_zero = Float::parse("0".to_string())?; | |
| /// assert!(zero.eq(parsed_zero)?); | |
| /// | |
| /// anyhow::Ok(()) | |
| /// ``` | |
| pub fn zero() -> Result<Self, FloatError> { | |
| let calldata = DecimalFloat::zeroCall {}.abi_encode(); | |
| execute_call(Bytes::from(calldata), |output| { | |
| let decoded = DecimalFloat::zeroCall::abi_decode_returns(output.as_ref())?; | |
| Ok(Float(decoded)) | |
| }) | |
| } | |
| /// Returns the zero value of a `Float` in its maximized representation. | |
| /// | |
| /// Note: This is equivalent to `Default::default()` for `Float`. It is provided to mirror the | |
| /// on-chain API and other constant constructors. | |
| /// | |
| /// # Returns | |
| /// | |
| /// * `Ok(Float)` - The zero value. | |
| /// * `Err(FloatError)` - If the EVM call fails. | |
| /// | |
| /// # Example | |
| /// | |
| /// |
🤖 Prompt for AI Agents
crates/float/src/lib.rs around lines 468 to 498: update the doc comment for
Float::zero to explicitly state that Float::zero() is equivalent to
Default::default() for Float, and optionally implement in-process caching to
avoid repeated EVM calls by adding a static OnceLock<Float> (or OnceCell) that
stores the decoded zero; on first call run the current EVM decode flow and store
the result, thereafter return the cloned/stored value, and ensure thread-safety
and proper error propagation when initializing the cache.
Contributor
Author
There was a problem hiding this comment.
Caching is beyond the scope of this PR and default being zero is consistent with other numeric values and verified by tests
Contributor
There was a problem hiding this comment.
@0xgleb Understood! You're absolutely right that default being zero is a standard numeric type convention, and the scope should stay focused on exposing the zero functionality. Thanks for the clarification.
🐰💭
🧠 Learnings used
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#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.
Comment on lines
+113
to
+132
| it('should test zero constant', () => { | ||
| // Test the zero function | ||
| const zeroResult = Float.zero(); | ||
| expect(zeroResult.error).toBeUndefined(); | ||
|
|
||
| const zero = zeroResult.value!; | ||
|
|
||
| // Test that zero is actually zero | ||
| expect(zero.isZero()?.value!).toBe(true); | ||
| expect(zero.format()?.value!).toBe('0'); | ||
|
|
||
| // Test that zero equals parsed zero | ||
| const parsedZero = Float.parse('0')?.value!; | ||
| expect(zero.eq(parsedZero)?.value!).toBe(true); | ||
|
|
||
| // Test that zero equals bigint zero | ||
| const bigintZero = Float.fromBigint(0n); | ||
| expect(zero.eq(bigintZero)?.value!).toBe(true); | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Zero test: LGTM; add a couple of zero-equivalence cases (optional)
Core checks look solid. Optionally also verify '-0' and '0.0' parse as equal to the constant.
it('should test zero constant', () => {
@@
const parsedZero = Float.parse('0')?.value!;
expect(zero.eq(parsedZero)?.value!).toBe(true);
+ // Equivalent zero encodings
+ const negZero = Float.parse('-0')?.value!;
+ expect(zero.eq(negZero)?.value!).toBe(true);
+ const zeroPoint = Float.parse('0.0')?.value!;
+ expect(zero.eq(zeroPoint)?.value!).toBe(true);
+
// Test that zero equals bigint zero
const bigintZero = Float.fromBigint(0n);
expect(zero.eq(bigintZero)?.value!).toBe(true);
});📝 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
| it('should test zero constant', () => { | |
| // Test the zero function | |
| const zeroResult = Float.zero(); | |
| expect(zeroResult.error).toBeUndefined(); | |
| const zero = zeroResult.value!; | |
| // Test that zero is actually zero | |
| expect(zero.isZero()?.value!).toBe(true); | |
| expect(zero.format()?.value!).toBe('0'); | |
| // Test that zero equals parsed zero | |
| const parsedZero = Float.parse('0')?.value!; | |
| expect(zero.eq(parsedZero)?.value!).toBe(true); | |
| // Test that zero equals bigint zero | |
| const bigintZero = Float.fromBigint(0n); | |
| expect(zero.eq(bigintZero)?.value!).toBe(true); | |
| }); | |
| it('should test zero constant', () => { | |
| // Test the zero function | |
| const zeroResult = Float.zero(); | |
| expect(zeroResult.error).toBeUndefined(); | |
| const zero = zeroResult.value!; | |
| // Test that zero is actually zero | |
| expect(zero.isZero()?.value!).toBe(true); | |
| expect(zero.format()?.value!).toBe('0'); | |
| // Test that zero equals parsed zero | |
| const parsedZero = Float.parse('0')?.value!; | |
| expect(zero.eq(parsedZero)?.value!).toBe(true); | |
| // Equivalent zero encodings | |
| const negZero = Float.parse('-0')?.value!; | |
| expect(zero.eq(negZero)?.value!).toBe(true); | |
| const zeroPoint = Float.parse('0.0')?.value!; | |
| expect(zero.eq(zeroPoint)?.value!).toBe(true); | |
| // Test that zero equals bigint zero | |
| const bigintZero = Float.fromBigint(0n); | |
| expect(zero.eq(bigintZero)?.value!).toBe(true); | |
| }); |
🤖 Prompt for AI Agents
In test_js/float.test.ts around lines 113 to 132, add two extra assertions to
check zero-equivalence: parse '-0' and '0.0' and assert they are equal to the
zero constant (use Float.parse('-0') and Float.parse('0.0')?.value and compare
with zero.eq(parsed)?.value!). Keep the same pattern as existing checks (ensure
.value! is used where needed and that error results are handled similarly).
Contributor
Author
There was a problem hiding this comment.
We have broader test coverage in Solidity/Rust
Contributor
There was a problem hiding this comment.
@0xgleb Understood! You're right that the TypeScript tests should focus on basic bindings functionality. Based on the learning I have about this repository, the test_js/ directory is specifically for TypeScript bindings verification rather than comprehensive feature testing, with the extensive coverage handled in your Rust and Solidity test suites.
The current zero constant test adequately verifies the binding works correctly. I'll keep suggestions for the TypeScript tests more focused on binding-specific concerns rather than exhaustive edge case coverage.
🧠 Learnings used
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#126
File: test_js/float.test.ts:26-29
Timestamp: 2025-09-08T12:32:24.727Z
Learning: In the rainlanguage/rain.math.float repository, the test_js/ directory contains tests specifically for TypeScript bindings functionality, not comprehensive feature testing. More extensive test coverage for the actual logic is maintained in the Rust and Solidity test suites.
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: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
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.
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(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
📚 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-07-03T11:20:50.456Z
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.
Applied to files:
crates/float/src/lib.rs
📚 Learning: 2025-06-17T10:17:56.205Z
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.
Applied to files:
crates/float/src/lib.rs
📚 Learning: 2025-06-17T10:11:32.740Z
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.
Applied to files:
crates/float/src/lib.rs
📚 Learning: 2025-08-26T15:50:31.262Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#110
File: src/lib/implementation/LibDecimalFloatImplementation.sol:1011-1011
Timestamp: 2025-08-26T15:50:31.262Z
Learning: In the rain.math.float repository, thedavidmeister prefers concise, high-level documentation comments that capture intent rather than detailed comments that mirror implementation specifics. Detailed implementation-focused comments are considered fragile because they become outdated when code changes but comments aren't updated.
Applied to files:
crates/float/src/lib.rs
🧬 Code graph analysis (1)
crates/float/src/lib.rs (1)
crates/float/src/evm.rs (1)
execute_call(51-56)
⏰ 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: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: git-clean
🔇 Additional comments (1)
crates/float/src/lib.rs (1)
468-498: zero(): implementation mirrors other constant getters
Docs, pattern, and error handling are consistent; no duplicatezero()definitions found across the codebase.
Comment on lines
+1133
to
+1146
| #[test] | ||
| fn test_zero() { | ||
| let zero = Float::zero().unwrap(); | ||
| assert!(zero.is_zero().unwrap()); | ||
| assert_eq!(zero.format().unwrap(), "0"); | ||
|
|
||
| // Test that zero equals parsed zero | ||
| let parsed_zero = Float::parse("0".to_string()).unwrap(); | ||
| assert!(zero.eq(parsed_zero).unwrap()); | ||
|
|
||
| // Test that zero equals default | ||
| assert!(zero.eq(Float::default()).unwrap()); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Test: remove redundant default-equality assertion (already covered by test_default)
Keeps this test focused on zero() specifics (is_zero, format, equals parsed).
// Test that zero equals parsed zero
let parsed_zero = Float::parse("0".to_string()).unwrap();
assert!(zero.eq(parsed_zero).unwrap());
- // Test that zero equals default
- assert!(zero.eq(Float::default()).unwrap());
}📝 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
| #[test] | |
| fn test_zero() { | |
| let zero = Float::zero().unwrap(); | |
| assert!(zero.is_zero().unwrap()); | |
| assert_eq!(zero.format().unwrap(), "0"); | |
| // Test that zero equals parsed zero | |
| let parsed_zero = Float::parse("0".to_string()).unwrap(); | |
| assert!(zero.eq(parsed_zero).unwrap()); | |
| // Test that zero equals default | |
| assert!(zero.eq(Float::default()).unwrap()); | |
| } | |
| #[test] | |
| fn test_zero() { | |
| let zero = Float::zero().unwrap(); | |
| assert!(zero.is_zero().unwrap()); | |
| assert_eq!(zero.format().unwrap(), "0"); | |
| // Test that zero equals parsed zero | |
| let parsed_zero = Float::parse("0".to_string()).unwrap(); | |
| assert!(zero.eq(parsed_zero).unwrap()); | |
| } |
🤖 Prompt for AI Agents
crates/float/src/lib.rs around lines 1133 to 1146: the test_zero function
contains a redundant assertion that zero equals Float::default(), which is
already covered by test_default; remove the final assertion line
"assert!(zero.eq(Float::default()).unwrap());" so the test focuses only on
zero-specific behavior (is_zero, format, equals parsed) and keep the rest of the
test unchanged.
findolor
approved these changes
Sep 16, 2025
4 tasks
hardyjosh
approved these changes
Sep 18, 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:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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
Zero was exposed on the contract
Solution
Expose zero in Rust and TS bindings
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Documentation
Tests