Skip to content

Expose lossy conversion#83

Merged
rouzwelt merged 10 commits intomainfrom
2025-07-23-expose-lossy-conversion
Jul 24, 2025
Merged

Expose lossy conversion#83
rouzwelt merged 10 commits intomainfrom
2025-07-23-expose-lossy-conversion

Conversation

@rouzwelt
Copy link
Copy Markdown
Contributor

@rouzwelt rouzwelt commented Jul 23, 2025

Motivation

This PR exposes from/to lossy conversion in Float contract as well as adds rust and wasm bindings for them

Solution

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Summary by CodeRabbit

  • New Features

    • Added lossy fixed-point decimal conversion methods for Float values, enabling both conversion to and from fixed decimals with lossy precision.
    • Exposed new lossy conversion functions in the smart contract interface.
    • Expanded test coverage for lossy and lossless decimal conversions, including new Solidity test contracts.
  • Bug Fixes

    • Improved consistency and error handling across contract and library conversion methods.
  • Documentation

    • Updated documentation to clarify new return values and behaviors for lossy conversion functions.
  • Tests

    • Added comprehensive tests for lossy and lossless decimal conversions, packing, and unpacking logic, ensuring correctness and consistency.
  • Chores

    • Updated gas usage snapshots to reflect new and existing test cases.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 23, 2025

Walkthrough

This change introduces lossy fixed decimal conversion methods to the Float implementation across Rust, Solidity, and WASM JS APIs. It adds new contract functions, Rust library methods, and WASM exports for lossy conversions, updates related test infrastructure, and removes or refactors legacy packing methods. New and updated tests validate the lossy and lossless conversion behaviors.

Changes

File(s) / Path(s) Change Summary
crates/float/src/js_api.rs Removed unused import; replaced pack_lossless_js with new lossy fixed decimal conversion WASM-exported methods.
crates/float/src/lib.rs Added from_fixed_decimal_lossy and to_fixed_decimal_lossy methods; updated packing/unpacking logic and tests.
src/concrete/DecimalFloat.sol Removed packing/unpacking functions; added lossy conversion methods; renamed a lossless conversion function.
.gas-snapshot Updated and expanded gas usage metrics for new and existing DecimalFloat tests.
src/lib/LibDecimalFloat.sol Added @return lossless documentation to a lossy conversion function.
test/src/concrete/DecimalFloat.fromFixedDecimalLossless.t.sol Added tests for fromFixedDecimalLossless function consistency.
test/src/concrete/DecimalFloat.fromFixedDecimalLossy.t.sol Added tests for fromFixedDecimalLossy function consistency.
test/src/concrete/DecimalFloat.toFixedDecimalLossless.t.sol Added tests for toFixedDecimalLossless function consistency.
test/src/concrete/DecimalFloat.toFixedDecimalLossy.t.sol Added tests for toFixedDecimalLossy function consistency.
crates/float/src/evm.rs Added test contract deployment, test call execution, and generalized call logic.
test/concrete/DecimalFloat.packLossless.t.sol Added tests for packLossless function consistency.
test/concrete/TestDecimalFloat.sol Introduced TestDecimalFloat contract exposing internal packing/unpacking for testing.
test/concrete/TestDecimalFloat.unpack.t.sol Added tests for unpacking logic consistency.
test_js/float.test.ts Removed the "should test packLossless" test case.

Sequence Diagram(s)

sequenceDiagram
    participant JS/WASM
    participant Rust Float
    participant EVM
    participant DecimalFloat Contract

    JS/WASM->>Rust Float: from_fixed_decimal_lossy_js(BigInt, decimals)
    Rust Float->>EVM: Call fromFixedDecimalLossy(value, decimals)
    EVM->>DecimalFloat Contract: fromFixedDecimalLossy(value, decimals)
    DecimalFloat Contract-->>EVM: (Float, lossless)
    EVM-->>Rust Float: (Float, lossless)
    Rust Float-->>JS/WASM: Float

    JS/WASM->>Rust Float: to_fixed_decimal_lossy_js(decimals)
    Rust Float->>EVM: Call toFixedDecimalLossy(Float, decimals)
    EVM->>DecimalFloat Contract: toFixedDecimalLossy(Float, decimals)
    DecimalFloat Contract-->>EVM: (uint256, lossless)
    EVM-->>Rust Float: (uint256, lossless)
    Rust Float-->>JS/WASM: BigInt
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • float js/ts bindings #76: Both modify Float JS bindings and conversion methods, but focus on different conversion variants (lossy vs. initial bindings).
  • Expose fromFixedDecimalLosslessPacked #68: This PR extends previous conversion interface work by adding lossy conversion methods, building on the earlier PR.
  • Expose toFixedDecimal #78: Both PRs add fixed decimal conversion methods to Float, but this PR focuses on lossy while the other focuses on lossless variants.

Suggested labels

enhancement

Suggested reviewers

  • hardyjosh
  • findolor
  • thedavidmeister

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between abfe62c and 1e4e308.

📒 Files selected for processing (8)
  • crates/float/src/evm.rs (1 hunks)
  • crates/float/src/js_api.rs (1 hunks)
  • crates/float/src/lib.rs (5 hunks)
  • src/concrete/DecimalFloat.sol (2 hunks)
  • test/concrete/DecimalFloat.packLossless.t.sol (1 hunks)
  • test/concrete/TestDecimalFloat.sol (1 hunks)
  • test/concrete/TestDecimalFloat.unpack.t.sol (1 hunks)
  • test_js/float.test.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • test_js/float.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
crates/float/src/lib.rs (2)
crates/float/src/evm.rs (2)
  • execute_call (51-56)
  • execute_test_call (59-64)
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 (1)
  • from_fixed_decimal_lossy (152-160)
⏰ 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)
  • GitHub Check: git-clean
  • GitHub Check: rainix (ubuntu-latest, test-wasm-build)
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
  • GitHub Check: rainix (macos-latest, rainix-rs-test)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
🔇 Additional comments (17)
test/concrete/TestDecimalFloat.sol (1)

1-27: LGTM! Well-structured test contract for exposing internal functions.

The contract properly exposes LibDecimalFloat.packLossless and LibDecimalFloat.unpack functions for external testing. The implementation is clean and serves its intended purpose as a testing interface for offchain environments.

test/concrete/TestDecimalFloat.unpack.t.sol (1)

1-29: LGTM! Comprehensive test ensuring consistency between library and deployed contract.

The test properly validates that unpack behaves identically whether called directly via the library or through the deployed TestDecimalFloat contract. The try-catch pattern correctly handles both success and revert scenarios, ensuring consistent error behavior.

test/concrete/DecimalFloat.packLossless.t.sol (1)

1-26: LGTM! Thorough test ensuring packLossless consistency across implementations.

The test correctly validates that packLossless produces identical results whether called via the library or the deployed TestDecimalFloat contract. The use of Float.unwrap for comparison and the try-catch pattern for error handling are both appropriate.

crates/float/src/js_api.rs (2)

178-205: LGTM! Proper implementation of lossy conversion from fixed decimal.

The from_fixed_decimal_lossy_js method correctly mirrors the existing lossless version's pattern, with appropriate BigInt to U256 conversion and proper error handling. The documentation and example are clear.


207-237: LGTM! Correct implementation of lossy conversion to fixed decimal.

The to_fixed_decimal_lossy_js method properly implements the lossy conversion pattern, with appropriate error handling and BigInt conversion. The example clearly demonstrates the truncation behavior (12345 with 3 decimals → 1234 with 2 decimals).

crates/float/src/evm.rs (3)

15-25: LGTM! Proper conditional compilation for test infrastructure.

The test-only imports and constants are correctly gated with #[cfg(test)], ensuring they don't affect production builds while providing necessary infrastructure for testing.


38-45: LGTM! Clean conditional deployment of test contract.

The TestDecimalFloat contract deployment is properly isolated to test builds, maintaining separation between production and test infrastructure.


51-99: LGTM! Excellent refactoring that eliminates code duplication.

The introduction of execute_call_at_address as a shared helper function is a clean solution that:

  • Eliminates code duplication between execute_call and execute_test_call
  • Maintains the same error handling logic
  • Uses proper address parameterization
  • Keeps the public APIs unchanged
src/concrete/DecimalFloat.sol (3)

210-210: LGTM - Function renaming aligns with library usage

The function name change from fromFixedDecimalLosslessPacked to fromFixedDecimalLossless maintains consistency with the underlying library call and improves naming clarity.


223-234: LGTM - New lossy conversion function properly implemented

The fromFixedDecimalLossy function correctly exposes the LibDecimalFloat.fromFixedDecimalLossyPacked functionality. The implementation follows established patterns in the codebase, including:

  • Proper parameter naming using float convention
  • Appropriate Slither disable comment for unused-return
  • Correct tuple return signature matching the library function

Based on retrieved learnings, the tuple unpacking pattern is required for Slither compatibility.


236-245: LGTM - Reverse lossy conversion function properly implemented

The toFixedDecimalLossy function correctly exposes the LibDecimalFloat.toFixedDecimalLossy functionality with consistent implementation patterns:

  • Follows established naming conventions
  • Proper Slither disable comment
  • Correct return signature for lossy conversion

The implementation maintains consistency with the companion fromFixedDecimalLossy function.

crates/float/src/lib.rs (6)

9-11: LGTM - Proper test-only code organization

The addition of test-specific imports and the TestDecimalFloat contract binding is correctly guarded with #[cfg(test)] attributes, ensuring these dependencies are only included during testing.

Also applies to: 19-20, 28-33


82-82: LGTM - Contract method call updated consistently

The update from fromFixedDecimalLosslessPackedCall to fromFixedDecimalLosslessCall aligns with the Solidity contract interface changes and maintains the same functionality.


126-160: LGTM - New lossy conversion method properly implemented

The from_fixed_decimal_lossy method follows established patterns:

  • Comprehensive documentation with examples
  • Proper ABI encoding and EVM call execution
  • Correct tuple destructuring to extract the Float value (using ._0)
  • Consistent error handling

The implementation correctly mirrors the lossless version while handling the additional boolean return value.


162-195: LGTM - Reverse lossy conversion method properly implemented

The to_fixed_decimal_lossy method complements the from_fixed_decimal_lossy method with:

  • Consistent documentation and examples
  • Proper ABI encoding and execution
  • Correct extraction of the U256 value from the tuple (using ._0)
  • Matching error handling patterns

The example in the documentation effectively demonstrates the lossy behavior.


223-235: LGTM - Test methods updated to use TestDecimalFloat contract

The migration of pack_lossless and unpack methods to use the TestDecimalFloat contract is appropriate:

  • Methods are correctly marked with #[cfg(test)]
  • unpack method return type updated to (I256, I256) to match the new contract interface
  • Both methods use execute_test_call instead of execute_call for proper contract targeting

This separation allows testing of internal packing/unpacking functionality without exposing it in the main contract API.

Also applies to: 238-250


1687-1737: LGTM - Comprehensive test coverage for lossy conversions

The new test functions provide thorough coverage:

  • test_from_fixed_decimal_lossy: Validates basic conversion cases
  • test_to_fixed_decimal_lossy: Tests reverse conversion with expected precision loss
  • test_from_to_fixed_decimal_lossy_valid_range: Property-based testing across valid input ranges

The tests correctly demonstrate lossy behavior (e.g., precision loss from 9 digits to 8 digits) and use appropriate property-based testing with proptest.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2025-07-23-expose-lossy-conversion

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@rouzwelt rouzwelt self-assigned this Jul 24, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c236a71 and b70e267.

📒 Files selected for processing (1)
  • src/concrete/DecimalFloat.sol (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.
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#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: 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: rouzwelt
PR: rainlanguage/rain.math.float#76
File: test_js/float.test.ts:9-32
Timestamp: 2025-07-17T02:38:44.698Z
Learning: In the rainlanguage/rain.math.float repository, the user rouzwelt accepts non-null assertions in test files because tests should throw and fail immediately when something goes wrong, making it clear where the issue occurred.
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: 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.
src/concrete/DecimalFloat.sol (4)

Learnt from: 0xgleb
PR: #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: #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: #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: #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.

⏰ 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)
  • GitHub Check: git-clean
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
  • GitHub Check: rainix (macos-latest, rainix-rs-test)
  • GitHub Check: rainix (ubuntu-latest, test-wasm-build)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-test)

Comment thread src/concrete/DecimalFloat.sol Outdated
Comment thread src/concrete/DecimalFloat.sol
@hardyjosh hardyjosh requested review from hardyjosh and removed request for thedavidmeister July 24, 2025 15:53
@rouzwelt rouzwelt merged commit c41c899 into main Jul 24, 2025
8 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Sep 16, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants