Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughThis PR establishes a build-time ABI resolution system for the Rain math float crate by adding a Rust build script that resolves Solidity contract ABI JSON paths from environment variables, updating the library code to consume those environment-provided paths, and adding a Nix derivation to build and expose the ABI artifacts. ChangesBuild-Time ABI Resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/float/build.rs`:
- Around line 1-3: The file build.rs is missing the required SPDX license
header; add the repository's SPDX header comment as the first lines of
crates/float/build.rs (above the existing use statements) per REUSE.toml policy,
ensuring it matches the project's standard header format (e.g.,
SPDX-License-Identifier and copyright/owner if required) so the file complies
with the mandated SPDX header rules.
In `@crates/float/src/lib.rs`:
- Around line 24-35: Add the required SPDX license header to the top of
crates/float/src/lib.rs (above the first sol! macro invocation) using the exact
SPDX identifier specified in the repository's REUSE.toml; ensure the header is a
single-line comment like "SPDX-License-Identifier: <identifier>" and appears
before the DecimalFloat and TestDecimalFloat sol! macros so the file complies
with the mandated SPDX header policy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d8fb2ff7-8599-4af2-8a4f-6c889fa77313
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
crates/float/build.rscrates/float/src/lib.rsflake.nix
| sol!( | ||
| #![sol(all_derives)] | ||
| DecimalFloat, | ||
| "../../out/DecimalFloat.sol/DecimalFloat.json" | ||
| env!("RAIN_MATH_FLOAT_DECIMAL_FLOAT_ABI") | ||
| ); | ||
|
|
||
| #[cfg(test)] | ||
| sol!( | ||
| #![sol(all_derives)] | ||
| TestDecimalFloat, | ||
| "../../out/TestDecimalFloat.sol/TestDecimalFloat.json" | ||
| env!("RAIN_MATH_FLOAT_TEST_DECIMAL_FLOAT_ABI") | ||
| ); |
There was a problem hiding this comment.
This touched Rust file also needs the mandatory SPDX header.
crates/float/src/lib.rs does not include the required SPDX license header. Please add it at the top of the file per REUSE.toml.
As per coding guidelines, **/*.{sol,rs,ts,tsx,js,jsx}: All source files must include SPDX license headers per REUSE.toml specification.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/float/src/lib.rs` around lines 24 - 35, Add the required SPDX license
header to the top of crates/float/src/lib.rs (above the first sol! macro
invocation) using the exact SPDX identifier specified in the repository's
REUSE.toml; ensure the header is a single-line comment like
"SPDX-License-Identifier: <identifier>" and appears before the DecimalFloat and
TestDecimalFloat sol! macros so the file complies with the mandated SPDX header
policy.
There was a problem hiding this comment.
REUSE.toml already covers crates/**/
There was a problem hiding this comment.
🧩 Analysis chain
Let me verify the REUSE.toml configuration.
🏁 Script executed:
#!/bin/bash
cat REUSE.toml 2>/dev/null || find . -name "REUSE.toml" -exec cat {} \;
Length of output: 612
@0xgleb, you're right — crates/**/ is explicitly listed in REUSE.toml, which covers all files under that directory including crates/float/src/lib.rs. No per-file SPDX header is needed. My comment was incorrect, sorry for the noise!
✏️ Learnings added
Learnt from: 0xgleb
Repo: rainlanguage/rain.math.float PR: 187
File: crates/float/src/lib.rs:24-35
Timestamp: 2026-05-06T10:07:02.630Z
Learning: In the rain.math.float repository, REUSE.toml uses glob-based annotations to cover entire directory trees (e.g., `crates/**/`, `flake.nix`, etc.) with `SPDX-License-Identifier = "LicenseRef-DCL-1.0"`. Files under these covered paths do NOT require individual in-file SPDX headers; the REUSE.toml annotation satisfies the license requirement for the whole tree.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
0xgleb
force-pushed
the
feat/abi-flake-output
branch
from
abed8bd to
816a579
Compare
|
@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:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=M |
3 participants
Closes RAI-329
Motivation
The ABI paths for
DecimalFloatandTestDecimalFloatwere hardcoded as relative file paths, making the build brittle and tightly coupled to a specific directory layout. This prevented reproducible builds in Nix environments where output paths are managed differently.Solution
A Nix derivation (
decimal-float-abi) is introduced to compile the Solidity contracts and produce the ABI JSON files as a proper build output. The hardcoded paths inlib.rsare replaced with environment variables (RAIN_MATH_FLOAT_DECIMAL_FLOAT_ABIandRAIN_MATH_FLOAT_TEST_DECIMAL_FLOAT_ABI) resolved at compile time viaenv!(). These variables are automatically set in the dev shell to point to the local build output, and the Nix derivation sets them to the compiled artifacts during CI/package builds. A newrain-math-float-wasmpackage entry is also added to the lockfile.