Skip to content

bump int - #391

Merged
thedavidmeister merged 2 commits into
mainfrom
2025-08-06-sqrt
Aug 6, 2025
Merged

bump int#391
thedavidmeister merged 2 commits into
mainfrom
2025-08-06-sqrt

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

Motivation

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

    • Re-enabled the square root (sqrt) operation in the standard operations set.
    • Updated the sqrt operation to use decimal floating point arithmetic for improved precision.
  • Bug Fixes

    • Addressed previous limitations by fully integrating and activating the sqrt operation.
  • Tests

    • Enhanced and modernized test coverage for the sqrt operation, including improved precision and updated input types.

@coderabbitai

coderabbitai Bot commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This change re-enables and refactors the sqrt operation in the standard ops set, updating its implementation from fixed-point to decimal floating-point arithmetic. It updates all relevant opcode integration points, function signatures, and tests to use new operand and floating-point abstractions. The submodule lib/rain.interpreter.interface is also updated.

Changes

Cohort / File(s) Change Summary
Submodule Update
lib/rain.interpreter.interface
Updated submodule commit reference to 0c8a593691cec60714d87887cee0038b908b226f.
Standard Ops Integration
src/lib/op/LibAllStandardOps.sol
Re-enabled LibOpSqrt integration: uncommented imports, metadata, and function pointers; incremented ALL_STANDARD_OPS_LENGTH from 65 to 66.
Sqrt Operation Refactor
src/lib/op/math/LibOpSqrt.sol
Refactored LibOpSqrt to use decimal floating-point (Float), updated function signatures, and added a new referenceFn for testing.
Sqrt Operation Tests
test/src/lib/op/math/LibOpSqrt.t.sol
Updated and uncommented tests to use new operand and floating-point types, improved precision, and added a fork setup for testing.

Sequence Diagram(s)

sequenceDiagram
    participant TestSuite
    participant LibOpSqrt
    participant LibDecimalFloat

    TestSuite->>LibOpSqrt: run(state, operand, stackTop)
    LibOpSqrt->>LibDecimalFloat: Float.sqrt(input, logTableAddr)
    LibDecimalFloat-->>LibOpSqrt: sqrtResult (Float)
    LibOpSqrt-->>TestSuite: stackTop (with sqrtResult)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • e constant #375: Updates the e constant operation implementation and integration, following a similar update pattern as this PR but for a different math op.
  • inv op #374: Updates the inv opcode implementation and integration, mirroring this PR's changes but for the inverse operation.
  • 2025 07 09 div #351: Refactors and re-enables the div operation with floating-point arithmetic, paralleling this PR's approach for the sqrt operation.

Suggested reviewers

  • findolor
  • hardyjosh

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 55e2dc0 and 4f857cf.

⛔ Files ignored due to path filters (3)
  • src/generated/Rainterpreter.pointers.sol is excluded by !**/generated/**
  • src/generated/RainterpreterExpressionDeployer.pointers.sol is excluded by !**/generated/**
  • src/generated/RainterpreterParser.pointers.sol is excluded by !**/generated/**
📒 Files selected for processing (4)
  • lib/rain.interpreter.interface (1 hunks)
  • src/lib/op/LibAllStandardOps.sol (6 hunks)
  • src/lib/op/math/LibOpSqrt.sol (1 hunks)
  • test/src/lib/op/math/LibOpSqrt.t.sol (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:0-0
Timestamp: 2025-07-15T11:31:28.010Z
Learning: In the rainlanguage/rain.interpreter project, forge (Foundry's formatting tool) handles code formatting automatically, so formatting-related suggestions are not actionable.
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:35-36
Timestamp: 2025-07-15T11:31:10.098Z
Learning: In the rain.interpreter codebase, when working with ERC20 tokens that may not implement the optional decimals() function, the preference is to let the call fail explicitly rather than catching errors and defaulting to 18 decimals. This ensures correctness by avoiding potentially incorrect calculations with assumed decimal values.
📚 Learning: in the rainlanguage/rain.interpreter project, forge (foundry's formatting tool) handles code formatt...
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:0-0
Timestamp: 2025-07-15T11:31:28.010Z
Learning: In the rainlanguage/rain.interpreter project, forge (Foundry's formatting tool) handles code formatting automatically, so formatting-related suggestions are not actionable.

Applied to files:

  • lib/rain.interpreter.interface
📚 Learning: in the rain.interpreter codebase, the team uses forge for automatic code formatting, so manual forma...
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:0-0
Timestamp: 2025-07-15T11:31:35.645Z
Learning: In the rain.interpreter codebase, the team uses forge for automatic code formatting, so manual formatting suggestions are not needed as the tool will handle formatting automatically.

Applied to files:

  • lib/rain.interpreter.interface
📚 Learning: in multiplication overflow detection tests like libopuint256multest, when performing sequential mult...
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#368
File: test/src/lib/op/math/uint256/LibOpUint256Mul.t.sol:56-69
Timestamp: 2025-07-17T14:15:14.886Z
Learning: In multiplication overflow detection tests like LibOpUint256MulTest, when performing sequential multiplication (a * b * c * d...), encountering a zero value means the final result will always be zero regardless of subsequent values. Since zero multiplied by any value (including MAX_UINT256) cannot overflow, it's safe and correct to break out of the overflow detection loop early when zero is encountered.

Applied to files:

  • test/src/lib/op/math/LibOpSqrt.t.sol
📚 Learning: in rain interpreter stack operations like libopevery, when the output position (stacktop) is set to ...
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#381
File: src/lib/op/logic/LibOpEvery.sol:24-46
Timestamp: 2025-07-27T22:56:57.928Z
Learning: In Rain interpreter stack operations like LibOpEvery, when the output position (stackTop) is set to coincide with an input item's position on the stack, explicit writing may not be needed if the desired output value is already at that position. The function can return the pointer to that position directly, leveraging the existing stack layout.

Applied to files:

  • src/lib/op/math/LibOpSqrt.sol
📚 Learning: in the rain.interpreter codebase, when working with erc20 tokens that may not implement the optional...
Learnt from: thedavidmeister
PR: rainlanguage/rain.interpreter#360
File: src/lib/op/erc20/LibOpERC20Allowance.sol:35-36
Timestamp: 2025-07-15T11:31:10.098Z
Learning: In the rain.interpreter codebase, when working with ERC20 tokens that may not implement the optional decimals() function, the preference is to let the call fail explicitly rather than catching errors and defaulting to 18 decimals. This ensures correctness by avoiding potentially incorrect calculations with assumed decimal values.

Applied to files:

  • src/lib/op/math/LibOpSqrt.sol
⏰ 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). (10)
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
  • GitHub Check: rainix (ubuntu-latest, test-wasm-build)
  • GitHub Check: rainix (macos-latest, rainix-rs-artifacts)
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-artifacts)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
  • GitHub Check: rainix (macos-latest, rainix-rs-test)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-artifacts)
  • GitHub Check: git-clean
🔇 Additional comments (18)
lib/rain.interpreter.interface (1)

1-1: Confirm the submodule bump is intentional and tracked in CI

The pointer now references commit 0c8a5936…. Please double-check that:

  1. This commit exists on the canonical rain.interpreter.interface repo and is not a WIP/local branch.
  2. CI pipelines (incl. forge test) fetch and compile the submodule at this commit without breaking downstream contracts/tests.

If you already validated this locally, feel free to ignore.

src/lib/op/LibAllStandardOps.sol (6)

102-102: LGTM: Import properly re-enabled.

The LibOpSqrt import is correctly uncommented to support the sqrt operation integration.


115-115: LGTM: Standard ops count correctly incremented.

The ALL_STANDARD_OPS_LENGTH is properly incremented from 65 to 66 to account for the re-enabled sqrt operation.


314-314: LGTM: Authoring metadata properly configured.

The sqrt operation's authoring metadata provides clear documentation with appropriate error handling description for negative inputs.


515-516: LGTM: Operand handler correctly configured.

The sqrt operation properly uses handleOperandDisallowed since it doesn't accept operands, which is consistent with other mathematical operations in this file.


627-627: LGTM: Integrity function pointer properly set.

The sqrt integrity check function pointer correctly references LibOpSqrt.integrity.


738-738: LGTM: Runtime function pointer properly set.

The sqrt execution function pointer correctly references LibOpSqrt.run.

src/lib/op/math/LibOpSqrt.sol (5)

4-9: LGTM: Imports properly updated for floating-point implementation.

The new imports correctly support the transition from fixed-point to decimal floating-point arithmetic using the Float type and associated utilities.


11-14: LGTM: Library documentation and using statement updated.

The documentation correctly reflects the new decimal floating-point implementation, and the using statement enables Float methods.


16-19: LGTM: Integrity function properly updated.

The function signature correctly uses OperandV2 and maintains the correct input/output requirements (1 input, 1 output) for the sqrt operation.


21-34: LGTM: Runtime implementation correctly refactored.

The new implementation properly:

  • Changes from pure to view to access LOG_TABLES_ADDRESS
  • Loads Float value from stack using assembly
  • Computes square root using the new floating-point library
  • Stores result back to the same stack position
  • Returns the stack pointer correctly

36-49: LGTM: Reference function properly implemented.

The new referenceFn correctly:

  • Unwraps StackItem to Float for computation
  • Applies the same sqrt logic as the run function
  • Wraps the result back to StackItem for testing
  • Provides proper interface for test validation
test/src/lib/op/math/LibOpSqrt.t.sol (6)

4-8: LGTM: Imports properly updated for floating-point testing.

The new imports correctly support testing the updated sqrt implementation with OperandV2, Float types, and decimal floating-point utilities.


10-15: LGTM: Fork setup properly configured.

The beforeOpTestConstructor correctly sets up an Ethereum fork, which is necessary for the floating-point operations that require access to LOG_TABLES_ADDRESS.


17-23: LGTM: Integrity test properly updated.

The test function signature correctly uses OperandV2 to match the updated LibOpSqrt.integrity function, and the assertions remain correct (1 input, 1 output).


25-35: LGTM: Runtime test properly refactored.

The test correctly:

  • Uses Float input type instead of fixed-point
  • Applies absolute value to ensure valid sqrt input
  • Wraps Float as StackItem for the test framework
  • Uses the new referenceFn for validation

37-48: LGTM: Evaluation tests use precise floating-point values.

The test values are mathematically correct and use LibDecimalFloat.packLossless for precise representation:

  • sqrt(0.5) ≈ 0.7071067809... (correctly represented with high precision)
  • sqrt(2) ≈ 1.414 and sqrt(2.5) ≈ 1.581 (appropriately precise)

50-68: LGTM: Error handling tests properly maintained.

The bad input/output tests correctly validate:

  • Wrong number of inputs (0 or 2 instead of 1)
  • Wrong number of outputs (0 or 2 instead of 1)
  • Operand usage (which should be disallowed)
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2025-08-06-sqrt

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.
  • 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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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.

@thedavidmeister
thedavidmeister merged commit 70953d0 into main Aug 6, 2025
11 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Aug 7, 2025
4 tasks
@coderabbitai coderabbitai Bot mentioned this pull request Oct 6, 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.

1 participant