Conversation
WalkthroughThis set of changes introduces new configuration and source files for a Rust/Solidity project, enhances CI workflows, and updates development environment settings. It adds a Rust crate interfacing with a Solidity contract via an in-memory EVM, expands GitHub Actions workflows with concurrency and platform matrix improvements, and introduces Changes
Sequence Diagram(s)sequenceDiagram
participant RustClient as Rust Calculator
participant EVM as In-Memory EVM
participant Contract as DecimalFloat Solidity Contract
RustClient->>EVM: Deploy DecimalFloat bytecode at fixed address
RustClient->>EVM: Encode call (parse/format/add/sub) and send calldata
EVM->>Contract: Execute contract method with calldata
Contract-->>EVM: Return result or error (ABI encoded)
EVM-->>RustClient: Return execution result (success or revert)
RustClient->>RustClient: Decode output or map error selector
sequenceDiagram
participant GitHubActions
participant Workflow as rainix.yaml Workflow
participant Matrix as Job Matrix (Ubuntu/macOS)
participant Nix as Nix Shell
GitHubActions->>Workflow: Trigger workflow on push/pull_request
Workflow->>Workflow: Apply concurrency control (keyed by ref)
Workflow->>Nix: Run `nix develop -c rainix-sol-prelude`
Workflow->>Matrix: Run matrix jobs (rainix-rs-test on Ubuntu/macOS, others on Ubuntu)
Matrix->>Nix: Run test/build commands per job
sequenceDiagram
participant DevEnv as Developer Environment
participant Direnv as .envrc
participant Nix as Nix Flake
DevEnv->>Direnv: Load .envrc
Direnv->>Direnv: Check nix-direnv version, source script if needed
Direnv->>Direnv: Watch flake.lock and flake.nix for changes
Direnv->>Nix: Activate flake with verbose and trace options
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockflake.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.cargo/config.toml(1 hunks).envrc(1 hunks).github/workflows/git-clean.yaml(2 hunks).github/workflows/rainix.yaml(2 hunks).gitignore(1 hunks).vscode/settings.json(1 hunks)Cargo.toml(1 hunks)crates/float/Cargo.toml(1 hunks)crates/float/proptest-regressions/lib.txt(1 hunks)crates/float/src/lib.rs(1 hunks)src/lib/LibDecimalFloat.sol(1 hunks)
🧰 Additional context used
🪛 LanguageTool
crates/float/proptest-regressions/lib.txt
[uncategorized] ~2-~2: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... in the past. It is # automatically read and these particular cases re-run before an...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (21)
src/lib/LibDecimalFloat.sol (1)
74-74: Doc typo fixed correctly
Correction from "WTBC" to "WBTC" in the inline comment improves clarity..vscode/settings.json (1)
2-3: VSCode Solidity settings align with contract version
Using remote compiler v0.8.25 and Forge formatter matches the codebase..cargo/config.toml (1)
1-3: Set wasm32 Rust flags for getrandom
Configuration enablesrandcrate onwasm32-unknown-unknownby settinggetrandom_backend="wasm_js", matching crate feature requirements..github/workflows/git-clean.yaml (2)
4-7: Add concurrency control
Introducingconcurrencywith branch-specific groups and cancellation of stale runs helps reduce duplicate workflows. Configuration aligns withrainix.yamlupdates.
38-38: Verify workspace cleanliness
Re-runninggit diff --exit-codeafter formatting and pointer generation ensures the workspace is clean.crates/float/Cargo.toml (3)
6-10: Workspace dependencies look correct
Dependencies (alloy,revm,thiserror) are properly referenced as workspace members.
11-13: Verify WebAssemblygetrandomfeatures
Using features["js", "js-sys"]seems appropriate forwasm32-unknown-unknown. Please confirm this aligns with your.cargo/config.tomlgetrandom_backendsetting.
14-16: Dev-dependency setup is correct
proptest.workspace = trueensures consistent versioning for property-based tests across the workspace.crates/float/proptest-regressions/lib.txt (1)
7-7: Seed entry format looks correct
The regression seed is present and properly formatted for re-running failure cases..envrc (3)
1-5: Environment bootstrap logic is sound
Version check and sourcing ofnix-direnvat 3.0.6 with the specified hash ensures reproducible setups.
7-9: Automatic reload triggers are correct
Watchingflake.lockandflake.nixwill refresh the environment on flake changes.
10-10: Flake activation command is valid
use flake . --verbose --show-traceprovides the intended verbose and trace-enabled Nix environment.Cargo.toml (3)
1-4: Workspace configuration is accurate
Definingmembers = ["crates/*"]andresolver = "2"aligns with multi-crate management and Cargo's latest resolver.
5-10: Workspace package metadata is defined
Edition, version, license, and homepage are set appropriately for the workspace.
11-16: Workspace dependencies align with member crates
Versions foralloy,revm,thiserror, andproptestmatch the crate-level requirements..github/workflows/rainix.yaml (5)
4-7: Concurrency settings are well configured
Usinggithub.refto key concurrency and cancel in-progress runs for non-main branches optimizes CI usage.
13-15: Verify YAML syntax formatrix.task
Ensure the inline list undermatrix.taskis correctly parsed as an array by GitHub Actions.
39-39: Add preparatory Nix step
Runningnix develop -c rainix-sol-preludebefore matrix tasks sets up required environment hooks.
45-46: Confirm emptyDEPLOY_BROADCASTandDEPLOY_VERIFIERusage
Setting these to""may override defaults. Verify this matches intended deployment logic.
51-51: Dynamic task execution is correct
run: nix develop -c ${{ matrix.task }}will execute each task variant in the matrix as desired.crates/float/src/lib.rs (1)
244-252:Details
❓ Verification inconclusive
Investigate commented-out test case.
There's a commented-out test case for parsing large numbers with many decimals. The comment suggests uncertainty about expected behavior. This should be clarified and either:
- Fixed if it's a bug, or
- Documented with the actual limits
Would you like me to help investigate the precision limits of the DecimalFloat contract?
Investigate and document DecimalFloat precision bounds
The commented-out test in crates/float/src/lib.rs (lines 244–252) indicates that parsing
"1341234234625468391.1341234234625468391"
succeeds instead of returning an error. Please:
- Determine and document the maximum supported integer magnitude and decimal precision for
DecimalFloat::parse.- If this behavior is unintended, restore the test and assert that an error is returned for inputs beyond those limits.
- If it’s expected, update the crate’s documentation to specify these bounds and remove or adapt the commented test case accordingly.
| [package] | ||
| name = "rain-math-float" | ||
| version.workspace = true | ||
| edition.workspace = true | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure crate metadata includes licensing and repository details
The manifest inherits version and edition from the workspace but omits license, repository, and description. Explicitly declaring these fields is recommended for crates.io publishing.
🤖 Prompt for AI Agents
In crates/float/Cargo.toml at lines 1 to 5, add explicit fields for license,
repository, and description to the [package] section. This involves specifying
the license type (e.g., "MIT OR Apache-2.0"), the URL of the repository, and a
brief description of the crate to ensure proper metadata for crates.io
publishing.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.toml(1 hunks)crates/float/proptest-regressions/lib.txt(1 hunks)crates/float/src/lib.rs(1 hunks)src/concrete/DecimalFloat.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
crates/float/proptest-regressions/lib.txt (1)
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.
crates/float/src/lib.rs (1)
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#46
File: crates/float/src/lib.rs:112-153
Timestamp: 2025-06-11T09:07:00.591Z
Learning: Alloy generates helpers to decode full ABI-encoded error payloads, but it doesn’t generate utilities to map raw 4-byte error selectors to error types; manual mapping is required when a contract returns only the selector.
🪛 LanguageTool
crates/float/proptest-regressions/lib.txt
[uncategorized] ~2-~2: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... in the past. It is # automatically read and these particular cases re-run before an...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (2)
crates/float/proptest-regressions/lib.txt (1)
1-9: Generated by proptest – no manual edits.🧰 Tools
🪛 LanguageTool
[uncategorized] ~2-~2: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... in the past. It is # automatically read and these particular cases re-run before an...(COMMA_COMPOUND_SENTENCE_2)
crates/float/src/lib.rs (1)
257-264: Clarify expected behaviour for invalid decimalsThe test asserts that
"1.2.3"parses as"1.2". If this lenient parse is intentional, add a doc comment inparse()and an explanatory//note here; if not, tighten validation and expect an error.
| prop_compose! { | ||
| fn reasonable_float()( | ||
| int_part in -10i128.pow(18)..10i128.pow(18), | ||
| decimal_places in 0u8..18u8, | ||
| decimal_part in 0u128..10u128.pow(18u32) | ||
| ) -> Float { | ||
| let mut calculator = Calculator::new().unwrap(); | ||
|
|
||
| let num_str = if decimal_places == 0 { | ||
| format!("{int_part}") | ||
| } else { | ||
| let decimal_str = format!("{decimal_part:0width$}", width = decimal_places as usize); | ||
| format!("{int_part}.{decimal_str}") | ||
| }; | ||
|
|
||
| calculator.parse(num_str).unwrap() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
decimal_part range can overrun decimal_places
decimal_part is always drawn from 0..10^18, even when decimal_places is, say, 2.
For decimal_places = 2 this can yield "123456" → "1.123456" (6 decimals).
Either limit decimal_part to 10^decimal_places or truncate/pad intentionally.
🤖 Prompt for AI Agents
In crates/float/src/lib.rs between lines 221 and 238, the decimal_part range is
fixed to 0..10^18 regardless of decimal_places, which can cause the generated
decimal part to have more digits than decimal_places. To fix this, adjust the
decimal_part range dynamically to 0..10^decimal_places so that the number of
decimal digits matches decimal_places exactly, or alternatively truncate or pad
decimal_part to ensure it has the correct length corresponding to
decimal_places.
There was a problem hiding this comment.
See the format! expression on line 232
There was a problem hiding this comment.
@0xgleb
I did look at that format! call:
let decimal_str = format!("{decimal_part:0width$}", width = decimal_places as usize);The width in a Rust format specifier is a minimum width, not a maximum.
If the decimal_part already has more than decimal_places digits, the formatter leaves it intact—it never truncates:
decimal_places = 2 // width = 2
decimal_part = 123456
format!("{decimal_part:02}") -> "123456" // 6 digits remain, no truncation
So with the current property generator we can still get numbers like "1.123456" when we intended only two decimal places. The earlier comment was pointing out that edge case.
Two easy fixes:
- Draw
decimal_partfrom0 .. 10u128.pow(decimal_places as u32)so the value never has too many digits. - Keep the wide range but explicitly truncate/pad:
let decimal_str = format!("{decimal_part:0width$}", width = decimal_places as usize) .chars() .take(decimal_places as usize) .collect::<String>();
Either way guarantees that the generated decimal strings respect decimal_places. Let me know if you’d like a patch for this.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
crates/float/src/lib.rs (1)
221-233: Duplicate of earlier comment: decimal string generator can emit more decimals than intended because the formatter only sets a minimum width.Cargo.toml (1)
6-6: 🧹 Nitpick (assertive)Declare
rust-versionto lock the MSRV
Addrust-version = "1.78"under[workspace.package]so downstream crates and CI know the minimum supported toolchain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.toml(1 hunks)crates/float/Cargo.toml(1 hunks)crates/float/src/lib.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
crates/float/src/lib.rs (1)
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#46
File: crates/float/src/lib.rs:112-153
Timestamp: 2025-06-11T09:07:00.591Z
Learning: Alloy generates helpers to decode full ABI-encoded error payloads, but it doesn’t generate utilities to map raw 4-byte error selectors to error types; manual mapping is required when a contract returns only the selector.
🔇 Additional comments (3)
Cargo.toml (3)
1-4: Workspace configuration is correctly set
Usingmembers = ["crates/*"]with resolver version 2 enables workspace-level dependency management and modern Cargo features.
8-8: Verify SPDX license identifier
Ensure that"CAL-1.0"is a recognized SPDX expression to prevent crates.io publishing errors.
11-17: Pinned workspace dependencies look solid
The specified versions and feature flags foralloyandrevmalign with your EVM integration requirements, andthiserror = "2.0.12"is up-to-date.
crates/float/src/lib.rs
Outdated
| pub fn parse(&mut self, str: String) -> Result<Float, CalculatorError> { | ||
| let calldata = DecimalFloat::parseCall { str }.abi_encode(); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Accept &str instead of String to avoid needless allocation
parse only reads the input; taking ownership forces every caller to allocate a new String.
-pub fn parse(&mut self, str: String) -> Result<Float, CalculatorError> {
+pub fn parse(&mut self, s: &str) -> Result<Float, CalculatorError> {
let calldata = DecimalFloat::parseCall { str }.abi_encode();(You can keep the ABI encode call by passing s.to_owned() inside the struct literal.)
Small win, but this method sits on the hot-path.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/float/src/lib.rs around lines 154 to 156, the parse function currently
takes ownership of a String, causing unnecessary allocations. Change the
function signature to accept a &str instead of String, and inside the function,
pass s.to_owned() when constructing the struct for the ABI encode call to
maintain functionality without forcing callers to allocate a new String.
There was a problem hiding this comment.
parseCall takes a String, so allocation is needed regardless
There was a problem hiding this comment.
@0xgleb Good point—because parseCall’s generated ABI struct owns a String, the contract invocation needs the allocation no matter what. The question is just where that allocation happens:
• Current signature (String)
– Zero-cost when the caller already has an owned String.
– Forces callers holding an &str to allocate up-front via .to_string().
• Suggested signature (&str)
– Avoids the up-front allocation for &str callers, but incurs one inside parse (s.to_owned()).
– Adds a clone if the caller already has a String.
If we want the best of both worlds, a common Rust idiom is to accept an impl Into<String> (or more explicitly S: Into<String>):
pub fn parse<S: Into<String>>(&mut self, s: S) -> Result<Float, CalculatorError> {
let calldata = DecimalFloat::parseCall { str: s.into() }.abi_encode();
…
}• Callers with String pass it by value—no extra clone.
• Callers with &str (or &String) pass it and pay exactly one allocation.
• The generic adds negligible monomorphisation overhead for a hot path like this.
Totally fine to keep the current API if the extra generic isn’t worth it to you; just wanted to surface the trade-off.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
crates/float/Cargo.toml (1)
1-7: Add missing crate metadata (description & repository).The manifest still lacks a
descriptionandrepositoryfield. These are strongly recommended for crates.io publication and were pointed out in an earlier review.[package] name = "rain-math-float" version.workspace = true edition.workspace = true license.workspace = true homepage.workspace = true +description = "Rust bindings that run the DecimalFloat Solidity library inside an in-memory EVM." +repository = "https://github.com/rainlanguage/rain.math.float"crates/float/src/lib.rs (2)
154-156: Accept anyInto<String>to avoid unnecessary caller allocations.A generic signature keeps zero-cost for
Stringcallers while sparing&strcallers an extra.to_string()outside the API.-pub fn parse(str: String) -> Result<Self, FloatError> { - let calldata = DecimalFloat::parseCall { str }.abi_encode(); +pub fn parse<S: Into<String>>(s: S) -> Result<Self, FloatError> { + let calldata = DecimalFloat::parseCall { str: s.into() }.abi_encode();
228-236: Generator can emit more decimals than requested.
format!("{decimal_part:0width$}")sets a minimum width, not a cap.
Ifdecimal_partalready has >decimal_placesdigits the string is left intact (e.g. width = 2, value = 123456 →"123456").
Restrict the range or truncate to guarantee the decimal length matchesdecimal_places.-decimal_part in 0u128..10u128.pow(18u32) +decimal_part in 0u128..10u128.pow(decimal_places as u32)(or explicitly
take(decimal_places as usize)after formatting).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
crates/float/Cargo.toml(1 hunks)crates/float/src/lib.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
crates/float/src/lib.rs (1)
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#46
File: crates/float/src/lib.rs:112-153
Timestamp: 2025-06-11T09:07:00.591Z
Learning: Alloy generates helpers to decode full ABI-encoded error payloads, but it doesn’t generate utilities to map raw 4-byte error selectors to error types; manual mapping is required when a contract returns only the selector.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: git-clean
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
Cargo.toml (1)
5-9: Consider declaringrust-versionfor clearer MSRV signalling.Even though Cargo doesn’t require it, adding the field prevents downstream users on older toolchains from accidentally pulling the crate and improves CI/tooling ergonomics.
crates/float/Cargo.toml (1)
1-6: 🛠️ Refactor suggestionAdd missing crate metadata (
descriptionandrepository).
Even when inheriting fields from the workspace, crates published on crates.io should explicitly declare adescriptionandrepositoryfor proper discoverability and linking.Apply this diff:
[package] name = "rain-math-float" version.workspace = true edition.workspace = true license.workspace = true homepage.workspace = true +description.workspace = true +repository.workspace = true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.toml(1 hunks)crates/float/Cargo.toml(1 hunks)crates/float/src/lib.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
crates/float/src/lib.rs (1)
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#46
File: crates/float/src/lib.rs:112-153
Timestamp: 2025-06-11T09:07:00.591Z
Learning: Alloy generates helpers to decode full ABI-encoded error payloads, but it doesn’t generate utilities to map raw 4-byte error selectors to error types; manual mapping is required when a contract returns only the selector.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
🔇 Additional comments (4)
crates/float/Cargo.toml (4)
8-10: Workspace dependencies are correctly declared.
Usingworkspace = trueforalloyandthiserrorcleanly inherits versions and keeps the manifest DRY.
12-19: Non-WASMrevmdependency correctly disables default features.
Thedefault-features = falseflag here prevents the heavy default stack from being pulled in, aligning with workspace-level opt-out.
20-27: WASM target dependencies look good.
Default features forrevmare disabled andgetrandomis specified with JS bindings for proper WebAssembly support.
29-31: Development dependencies are appropriately scoped.
alloywith testing features andproptestviaworkspace = trueensures the crate has necessary test harnesses without extra version pinning.
2 participants
Motivation
We need to be able to work with
Floatvalues in Rust but don't want to duplicate the logicSolution
Use
revmto deployDecimalFloatcontract and use the isolated environment to provide bindings to its methods. This PR starts withparse,format,add, andsub.Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Chores
.gitignoreto exclude.direnvandtargetdirectories..envrcfor environment management.Documentation