Skip to content

Conversation

@thedavidmeister
Copy link
Contributor

@thedavidmeister thedavidmeister commented Oct 9, 2025

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

  • Chores

    • Updated external registry and settings URLs for strategies.
    • Updated internal library dependencies and submodule pointers.
    • Refined numeric formatting display in test outputs.
  • Tests

    • Adjusted test expectations for read/write operations and number display formats.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

This PR migrates the LibTOFUTokenDecimals library from a local contract file to an external submodule at lib/rain.tofu.erc20-decimals, updates all contract and test imports accordingly, refactors token decimals handling to eliminate per-contract state caching, and adjusts test expectations to reflect reduced VM access patterns. Build configuration files are updated to support the new dependency.

Changes

Cohort / File(s) Summary
Submodule and Configuration
.gitmodules, foundry.toml, slither.config.json, REUSE.toml
Added new submodule entry for lib/rain.tofu.erc20-decimals with path and URL in .gitmodules. Added remapping in foundry.toml for library resolution. Expanded slither.config.json filter paths and added foundry.lock to REUSE.toml annotations.
Submodule Pointer Updates
lib/rain.interpreter, lib/rain.orderbook.interface, lib/rain.tofu.erc20-decimals
Updated lib/rain.interpreter commit pointer; updated lib/rain.orderbook.interface commit pointer; added new submodule lib/rain.tofu.erc20-decimals.
Library Migration and Removal
src/lib/LibTOFUTokenDecimals.sol
Removed entire local library including TOFUOutcome enum, TOFUTokenDecimals struct, error definitions, and LibTOFUTokenDecimals functions.
Contract Decimals Refactoring
src/abstract/OrderBookV5ArbOrderTaker.sol, src/abstract/OrderBookV5FlashBorrower.sol, src/concrete/ob/OrderBook.sol
Removed sTOFUTokenDecimals state mapping; updated imports to use external rain.tofu.erc20-decimals library; refactored function signatures to invoke LibTOFUTokenDecimals.safeDecimalsForToken(token) directly without mapping parameter. Introduced checkTokenSelfTrade() function in OrderBook.sol to centralize self-trade validation.
Test Fixture and Deployment Updates
test/util/abstract/ArbTest.sol, test/util/abstract/OrderBookExternalMockTest.sol, test/util/abstract/OrderBookExternalRealTest.sol, crates/test_fixtures/src/lib.rs
Added imports for TOFUTokenDecimals and LibTOFUTokenDecimals; etched TOFUTokenDecimals deployment code to enable runtime calls in tests; added TOFU_TOKEN_DECIMALS_ADDRESS constant in Rust fixture.
Test VM Access Count Adjustments
test/concrete/ob/OrderBook.deposit.entask.t.sol, test/concrete/ob/OrderBook.deposit.t.sol, test/concrete/ob/OrderBook.withdraw.entask.t.sol, test/concrete/ob/OrderBook.withdraw.t.sol
Updated expected VM reads/writes counts in reentrancy checks and deposit/withdraw test assertions to reflect optimized access patterns after refactoring. Updated decimal formatting expectations.
Build Script Updates
pointers.sh, prep-all.sh
Added build steps for lib/rain.tofu.erc20-decimals with forge build invocation and directory-specific setup.
Rust Test Formatting Updates
crates/common/src/types/order_takes_list_flattened.rs, crates/common/src/types/token_vault_flattened.rs, crates/common/src/types/vault_balance_change_flattened.rs
Updated test assertions for balance/amount formatting: switched between scientific notation and decimal representation (e.g., -0.98765432-9.8765432e-1, 987659.87650000000000000000001e4).
Selector and Mock Updates
packages/orderbook/test/js_api/gui.test.ts, packages/orderbook/test/js_api/raindexClient.test.ts, crates/quote/src/rpc.rs, crates/cli/src/commands/order/calldata.rs
Updated RPC mock selectors from 0xf0cfdd37, 0xc19423bc, 0x24376855 to 0x56fb83e9, 0x251ac32e, 0xf79693f4. Updated test data for unknown error selectors. Updated comments referencing renamed endpoints.
Enum Variant Renaming
crates/common/src/fuzz/impls.rs
Renamed public enum variants DeployerISP::iInterpreterCallDeployerISP::I_INTERPRETERCall and DeployerISP::iStoreCallDeployerISP::I_STORECall with updated usages.
Constants and Registry Updates
packages/webapp/src/lib/constants.ts, tauri-app/src/lib/services/loadRemoteSettings.ts, tauri-app/src-tauri/src/commands/order_take.rs, flake.nix
Updated REGISTRY_URL and REMOTE_SETTINGS_URL to point to new commit hash in rain.strategies. Updated CSV test fixture formatting. Changed flake.nix to use old-pkgs for sentry-cli dependency.

Sequence Diagram(s)

sequenceDiagram
    participant Contract as OrderBook Contract
    participant Local as Local sTOFUTokenDecimals
    participant ExtLib as External LibTOFUTokenDecimals

    Note over Contract,Local: Before (Removed)
    Contract->>Local: Read sTOFUTokenDecimals mapping
    Local-->>Contract: Return cached decimals

    Note over Contract,ExtLib: After (New)
    Contract->>ExtLib: safeDecimalsForToken(token)
    ExtLib->>ExtLib: staticcall token.decimals()
    ExtLib-->>Contract: Return uint8 decimals
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • High-density logic areas:

    • src/concrete/ob/OrderBook.sol — substantial refactoring with new checkTokenSelfTrade() function, state removal, and multiple function signature updates requiring verification that all call sites are correctly updated
    • src/abstract/OrderBookV5ArbOrderTaker.sol and src/abstract/OrderBookV5FlashBorrower.sol — contract-layer decimals handling changes need careful validation
  • Systematic/repetitive changes:

    • Test VM access count adjustments across multiple test files (repetitive pattern but each requires verification against actual behavior)
    • Selector and formatting updates in multiple test/mock files (straightforward but numerous)
    • Enum renaming in Rust code (mechanical but wide-scoped)
  • Risk areas:

    • Confirm all references to removed LibTOFUTokenDecimals.sol have been migrated to external library
    • Verify test read/write counts accurately reflect optimized contract behavior post-refactoring
    • Ensure TOFUTokenDecimals deployment in test fixtures is correctly wired and doesn't cause unexpected test behavior

Possibly related PRs

  • update deps #2154 — Updates submodule pointers for lib/rain.interpreter and lib/rain.orderbook.interface alongside this PR's submodule changes
  • ob int #2139 — Also updates the same submodule pointers (lib/rain.interpreter and lib/rain.orderbook.interface)
  • update settings commit #2201 — Updates REGISTRY_URL and REMOTE_SETTINGS_URL constants in packages/webapp/src/lib/constants.ts to different commit hashes

Suggested labels

rust, test, refactor, dependencies

Suggested reviewers

  • hardyjosh
  • findolor

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The pull request title "2025 10 08 int" appears to be a date stamp with an abbreviated term, which does not convey meaningful information about the changeset. The actual changes comprise a significant refactoring that extracts token decimals functionality to an external submodule (lib/rain.tofu.erc20-decimals), removes internal implementations, updates multiple source files and tests to use the external library, and modifies various configuration files. However, the title gives no indication of these changes, using a vague and generic format that leaves a teammate scanning history unable to understand the primary purpose or scope of the PR without examining the detailed changeset. The title should be revised to clearly describe the main change. A more descriptive title might be something like "Extract token decimals to external rain.tofu.erc20-decimals submodule" or "Refactor to use external LibTOFUTokenDecimals library" to clearly communicate the primary refactoring effort and help reviewers quickly understand the PR's intent.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2025-10-08-int

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
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: 6

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d92844 and 7538faf.

⛔ Files ignored due to path filters (2)
  • flake.lock is excluded by !**/*.lock
  • foundry.lock is excluded by !**/*.lock
📒 Files selected for processing (17)
  • .gitmodules (1 hunks)
  • foundry.toml (1 hunks)
  • lib/rain.interpreter (1 hunks)
  • lib/rain.orderbook.interface (1 hunks)
  • lib/rain.tofu.erc20-decimals (1 hunks)
  • src/abstract/OrderBookV4FlashLenderDisabled.sol (1 hunks)
  • src/abstract/OrderBookV5ArbOrderTaker.sol (2 hunks)
  • src/abstract/OrderBookV5FlashBorrower.sol (2 hunks)
  • src/concrete/ob/OrderBook.sol (11 hunks)
  • src/lib/LibTOFUTokenDecimals.sol (0 hunks)
  • test/concrete/ob/OrderBook.deposit.entask.t.sol (1 hunks)
  • test/concrete/ob/OrderBook.deposit.t.sol (1 hunks)
  • test/concrete/ob/OrderBook.withdraw.entask.t.sol (1 hunks)
  • test/concrete/ob/OrderBook.withdraw.t.sol (1 hunks)
  • test/util/abstract/ArbTest.sol (2 hunks)
  • test/util/abstract/OrderBookExternalMockTest.sol (2 hunks)
  • test/util/abstract/OrderBookExternalRealTest.sol (2 hunks)
💤 Files with no reviewable changes (1)
  • src/lib/LibTOFUTokenDecimals.sol
🧰 Additional context used
📓 Path-based instructions (4)
**/*.sol

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Solidity contracts must build and have passing Forge tests (forge build, forge test)

**/*.sol: Format Solidity with forge fmt
For Solidity, add fuzz/property tests where relevant

Files:

  • test/concrete/ob/OrderBook.withdraw.entask.t.sol
  • test/concrete/ob/OrderBook.deposit.t.sol
  • test/concrete/ob/OrderBook.withdraw.t.sol
  • test/concrete/ob/OrderBook.deposit.entask.t.sol
  • src/abstract/OrderBookV5FlashBorrower.sol
  • test/util/abstract/ArbTest.sol
  • src/concrete/ob/OrderBook.sol
  • test/util/abstract/OrderBookExternalMockTest.sol
  • src/abstract/OrderBookV4FlashLenderDisabled.sol
  • test/util/abstract/OrderBookExternalRealTest.sol
  • src/abstract/OrderBookV5ArbOrderTaker.sol
test/**/*.sol

📄 CodeRabbit inference engine (AGENTS.md)

Place Solidity tests under test/

Files:

  • test/concrete/ob/OrderBook.withdraw.entask.t.sol
  • test/concrete/ob/OrderBook.deposit.t.sol
  • test/concrete/ob/OrderBook.withdraw.t.sol
  • test/concrete/ob/OrderBook.deposit.entask.t.sol
  • test/util/abstract/ArbTest.sol
  • test/util/abstract/OrderBookExternalMockTest.sol
  • test/util/abstract/OrderBookExternalRealTest.sol
src/**/*.sol

📄 CodeRabbit inference engine (AGENTS.md)

Place Solidity contract source files under src/

Files:

  • src/abstract/OrderBookV5FlashBorrower.sol
  • src/concrete/ob/OrderBook.sol
  • src/abstract/OrderBookV4FlashLenderDisabled.sol
  • src/abstract/OrderBookV5ArbOrderTaker.sol
foundry.toml

📄 CodeRabbit inference engine (AGENTS.md)

Use solc 0.8.25 as configured in foundry.toml

Files:

  • foundry.toml
🧠 Learnings (6)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.orderbook#2149
File: src/concrete/arb/RouteProcessorOrderBookV5ArbOrderTaker.sol:37-38
Timestamp: 2025-09-16T07:37:24.455Z
Learning: In the rainlanguage/rain.orderbook codebase, caching token decimals is considered dangerous unless handled with TOFU (Trust On First Use) pattern and expectation checks on every use. The additional validation required eliminates any gas savings from caching, so calling IERC20Metadata(...).decimals() on each use is the preferred safe pattern, as explained by thedavidmeister.
📚 Learning: 2025-07-04T09:02:57.301Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.

Applied to files:

  • lib/rain.orderbook.interface
📚 Learning: 2025-09-16T07:37:24.455Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.orderbook#2149
File: src/concrete/arb/RouteProcessorOrderBookV5ArbOrderTaker.sol:37-38
Timestamp: 2025-09-16T07:37:24.455Z
Learning: In the rainlanguage/rain.orderbook codebase, caching token decimals is considered dangerous unless handled with TOFU (Trust On First Use) pattern and expectation checks on every use. The additional validation required eliminates any gas savings from caching, so calling IERC20Metadata(...).decimals() on each use is the preferred safe pattern, as explained by thedavidmeister.

Applied to files:

  • src/abstract/OrderBookV5FlashBorrower.sol
  • test/util/abstract/ArbTest.sol
  • src/concrete/ob/OrderBook.sol
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.

Applied to files:

  • src/abstract/OrderBookV5FlashBorrower.sol
📚 Learning: 2025-08-15T15:57:45.830Z
Learnt from: Siddharth2207
PR: rainlanguage/rain.orderbook#2089
File: test/concrete/ob/OrderBook.withdraw.t.rounding.sol:7-7
Timestamp: 2025-08-15T15:57:45.830Z
Learning: In the rainlanguage/rain.orderbook codebase, user Siddharth2207 considers Math import from OpenZeppelin contracts as required in test files like test/concrete/ob/OrderBook.withdraw.t.rounding.sol, even when direct usage is not immediately visible in the code.

Applied to files:

  • src/concrete/ob/OrderBook.sol
  • test/util/abstract/OrderBookExternalMockTest.sol
  • test/util/abstract/OrderBookExternalRealTest.sol
📚 Learning: 2025-06-16T10:49:47.770Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.orderbook#1926
File: test/concrete/ob/OrderBook.clear.zeroAmount.t.sol:24-32
Timestamp: 2025-06-16T10:49:47.770Z
Learning: LibTestAddOrder.conformConfig() in test/util/lib/LibTestAddOrder.sol automatically constrains OrderConfigV3 to prevent common test failures by ensuring validInputs[0].token != validOutputs[0].token, setting them to address(0) and address(1) respectively if they're equal. This prevents TokenSelfTrade errors in fuzz tests.

Applied to files:

  • src/concrete/ob/OrderBook.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). (17)
  • GitHub Check: test
  • GitHub Check: test
  • GitHub Check: build-tauri (ubuntu-22.04, true)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
  • GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
  • GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
  • GitHub Check: git-clean
  • GitHub Check: test
  • GitHub Check: Deploy-Docs-Preview
  • GitHub Check: Deploy-Preview-Push
🔇 Additional comments (12)
.gitmodules (1)

10-12: New submodule for TOFU ERC20 decimals — LGTM

Path/URL look correct and match the intended remap.

test/util/abstract/OrderBookExternalRealTest.sol (1)

29-29: TOFU decimals integration — LGTM

Importing TOFUTokenDecimals/LibTOFUTokenDecimals aligns with the safe “no caching decimals” guidance and centralizes TOFU usage. Based on learnings

test/concrete/ob/OrderBook.withdraw.entask.t.sol (1)

117-117: Reentrancy RW expectation tightened — LGTM

Using 6 reads and 3 writes matches the updated access pattern.

test/util/abstract/OrderBookExternalMockTest.sol (1)

45-47: LGTM! Consistent test infrastructure setup.

The TOFU decimals deployment etching is correctly placed before other test setup (interpreter, store, orderbook) to ensure decimals calls succeed during test execution. This pattern aligns with other test utilities (ArbTest, OrderBookExternalRealTest).

test/util/abstract/ArbTest.sol (1)

54-56: LGTM! TOFU decimals setup correctly ordered.

The etching is correctly placed before interpreter and store initialization to ensure decimals are available when needed during arb operations.

src/abstract/OrderBookV5FlashBorrower.sol (1)

161-162: LGTM! Correctly uses direct decimals retrieval.

The change from cached sTOFUTokenDecimals to direct LibTOFUTokenDecimals.safeDecimalsForToken(token) calls aligns with the established pattern that caching token decimals is dangerous without TOFU pattern validation on every use. Direct calls are the preferred safe pattern.

Based on learnings.

src/abstract/OrderBookV5ArbOrderTaker.sol (1)

82-84: LGTM! Consistent with decimals retrieval pattern.

Direct calls to LibTOFUTokenDecimals.safeDecimalsForToken(token) maintain consistency with the broader refactoring away from cached decimals storage.

src/concrete/ob/OrderBook.sol (3)

427-428: LGTM! Token address caching optimizes repeated access.

Caching orderInputToken and orderOutputToken reduces redundant array accesses throughout the takeOrders3 function, improving both readability and gas efficiency.


678-679: LGTM! Direct decimals retrieval aligns with safe pattern.

The change from cached sTOFUTokenDecimals storage to direct LibTOFUTokenDecimals.decimalsForTokenReadOnly(token) calls follows the established safe pattern. While this increases gas costs slightly, it eliminates the risk of using stale decimals without proper TOFU validation.

Based on learnings.

Also applies to: 697-698


940-940: LGTM! Consistent decimals retrieval for token transfers.

The pullTokens and pushTokens functions now use direct LibTOFUTokenDecimals.decimalsForToken(token) calls, maintaining consistency with the broader refactoring and ensuring fresh decimals for each token transfer.

Also applies to: 962-962

test/concrete/ob/OrderBook.withdraw.t.sol (1)

47-48: Verify updated storage access counts
The new assertions (reads: 6, writes: 3) should accurately reflect storage operations for the empty vault withdrawal after removing internal decimals caching. Please run the relevant test locally to confirm.

test/concrete/ob/OrderBook.deposit.entask.t.sol (1)

38-44: Manual verification required for storage access patterns

The sandbox environment doesn’t have Forge installed. Please run the deposit entask test locally with:

forge test --match-contract OrderBookDepositEnactTest -vvv

and confirm that reads.length is 5 and writes.length is 3.

// - vault balance floats x2
// - token decimals x2
assertTrue(reads.length == 6 || reads.length == 8, "reads");
assertTrue(reads.length == 5, "reads");
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Comments vs. assertion mismatch on read-count

Block comments list 7 reads (3 reentrancy, 2 vault floats, 2 token decimals) but assertion enforces 5. Update comments to reflect the current 5-read path or adjust the assertion if 5 is unintentional.

🤖 Prompt for AI Agents
In test/concrete/ob/OrderBook.deposit.t.sol around line 166, the block comment
above asserts enumerates seven external reads but the actual assertion checks
for 5 reads; update the comment to accurately list the current five reads (or,
if the test should exercise seven reads, adjust the assertion to expect 7 and
modify the test flow to include the two missing reads). Ensure comment and
assertion are consistent: either reduce the comment to describe the five read
sources exactly, or change the assertion and test setup so that all seven reads
occur.

Comment on lines +42 to +45
// Put the TOFU decimals contract in place so that any calls to it
// succeed. This is because we don't have zoltu here.
vm.etch(address(LibTOFUTokenDecimals.TOFU_DECIMALS_DEPLOYMENT), type(TOFUTokenDecimals).runtimeCode);

Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Guard the etch to avoid stomping existing code (optional)

Add a code.length check before etching to be resilient if code is already present at the deployment address.

-        vm.etch(address(LibTOFUTokenDecimals.TOFU_DECIMALS_DEPLOYMENT), type(TOFUTokenDecimals).runtimeCode);
+        if (address(LibTOFUTokenDecimals.TOFU_DECIMALS_DEPLOYMENT).code.length == 0) {
+            vm.etch(
+                address(LibTOFUTokenDecimals.TOFU_DECIMALS_DEPLOYMENT),
+                type(TOFUTokenDecimals).runtimeCode
+            );
+        }
📝 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
// Put the TOFU decimals contract in place so that any calls to it
// succeed. This is because we don't have zoltu here.
vm.etch(address(LibTOFUTokenDecimals.TOFU_DECIMALS_DEPLOYMENT), type(TOFUTokenDecimals).runtimeCode);
// Put the TOFU decimals contract in place so that any calls to it
// succeed. This is because we don't have zoltu here.
if (address(LibTOFUTokenDecimals.TOFU_DECIMALS_DEPLOYMENT).code.length == 0) {
vm.etch(
address(LibTOFUTokenDecimals.TOFU_DECIMALS_DEPLOYMENT),
type(TOFUTokenDecimals).runtimeCode
);
}
🤖 Prompt for AI Agents
In test/util/abstract/OrderBookExternalRealTest.sol around lines 42 to 45, the
test unconditionally etches TOFU decimals bytecode to the deployment address
which can overwrite existing code; guard the vm.etch by first checking that
address(LibTOFUTokenDecimals.TOFU_DECIMALS_DEPLOYMENT).code.length == 0 (or use
address(...).code.length > 0 as appropriate) and only call vm.etch when there is
no code present at that address so you don't stomp existing deployments.

Copy link
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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7538faf and d2d6371.

⛔ Files ignored due to path filters (1)
  • foundry.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • lib/rain.interpreter (1 hunks)
  • lib/rain.orderbook.interface (1 hunks)
⏰ 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). (17)
  • GitHub Check: build-tauri (ubuntu-22.04, true)
  • GitHub Check: test
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
  • GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
  • GitHub Check: test
  • GitHub Check: test
  • GitHub Check: git-clean
  • GitHub Check: Deploy-Preview-Push
  • GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (1)
lib/rain.interpreter (1)

1-1: Submodule bump OK; confirm remappings and imports. .gitmodules defines lib/rain.interpreter, and foundry.toml maps rain.interpreter/lib/rain.interpreter/src/. No direct Solidity imports were detected in the CI sandbox—verify locally that all import paths resolve and that the updated interpreter API matches its consumers (run git submodule update --init && forge test).

@@ -1 +1 @@
Subproject commit 698588adcf35117864fe56fef8b83f436b6db2b1
Subproject commit dae86d795b32cb88005d9c47cbbbff9a5eef940a
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Submodule pointer update looks fine; please verify the commit exists upstream.

No interface code change here, only the gitlink. Confirm the target commit is present on the submodule remote.


🏁 Script executed:

#!/bin/bash
set -euo pipefail

path="lib/rain.orderbook.interface"

# Gitlink commit recorded in the superproject
commit="$(git ls-tree HEAD "$path" | awk '{print $3}')"
echo "Gitlink for $path: $commit"

# Resolve submodule URL from .gitmodules
url="$(git config -f .gitmodules --get "submodule.$path.url" || true)"
if [ -z "${url}" ]; then
  echo "ERROR: Submodule URL not found for $path in .gitmodules" >&2
  exit 1
fi
echo "Remote URL: $url"

# Verify the commit exists on the remote
echo "Verifying commit exists upstream..."
if git ls-remote "$url" "$commit" | grep -q "$commit"; then
  echo "OK: Commit found on remote."
else
  echo "ERROR: Commit not found on remote for $url" >&2
  exit 2
fi

# Best-effort: list a few heads/tags for context
echo "Recent heads:"
git ls-remote --heads "$url" | head -n 10 || true
echo "Recent tags:"
git ls-remote --tags "$url" | tail -n 10 || true

Length of output: 282


Submodule pointer invalid: commit dae86d795b32cb88005d9c47cbbbff9a5eef940a not found on the remote https://github.com/rainlanguage/rain.orderbook.interface. Push the missing commit to the submodule’s remote or update the pointer to a valid commit.

🤖 Prompt for AI Agents
In lib/rain.orderbook.interface around lines 1 to 1, the submodule pointer
references commit dae86d795b32cb88005d9c47cbbbff9a5eef940a which does not exist
on the submodule remote; either push that missing commit to
https://github.com/rainlanguage/rain.orderbook.interface from the submodule
repo, or update the pointer in the parent repo to a valid commit by entering the
submodule, checking out the intended commit (or fetch and create it), then in
the parent repo run git add lib/rain.orderbook.interface and commit the updated
submodule reference (or update .gitmodules if the URL is wrong), and push the
parent repo change.

@thedavidmeister thedavidmeister self-assigned this Oct 10, 2025
Copy link
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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2d6371 and c4ee0e3.

📒 Files selected for processing (2)
  • test/concrete/ob/OrderBook.deposit.entask.t.sol (2 hunks)
  • test/concrete/ob/OrderBook.withdraw.entask.t.sol (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sol

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Solidity contracts must build and have passing Forge tests (forge build, forge test)

**/*.sol: Format Solidity with forge fmt
For Solidity, add fuzz/property tests where relevant

Files:

  • test/concrete/ob/OrderBook.withdraw.entask.t.sol
  • test/concrete/ob/OrderBook.deposit.entask.t.sol
test/**/*.sol

📄 CodeRabbit inference engine (AGENTS.md)

Place Solidity tests under test/

Files:

  • test/concrete/ob/OrderBook.withdraw.entask.t.sol
  • test/concrete/ob/OrderBook.deposit.entask.t.sol
🧠 Learnings (1)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.orderbook#2149
File: src/concrete/arb/RouteProcessorOrderBookV5ArbOrderTaker.sol:37-38
Timestamp: 2025-09-16T07:37:24.455Z
Learning: In the rainlanguage/rain.orderbook codebase, caching token decimals is considered dangerous unless handled with TOFU (Trust On First Use) pattern and expectation checks on every use. The additional validation required eliminates any gas savings from caching, so calling IERC20Metadata(...).decimals() on each use is the preferred safe pattern, as explained by thedavidmeister.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
⏰ 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). (17)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
  • GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
  • GitHub Check: test
  • GitHub Check: test
  • GitHub Check: Deploy-Docs-Preview
  • GitHub Check: test
  • GitHub Check: git-clean
  • GitHub Check: Deploy-Preview-Push
  • GitHub Check: build-tauri (ubuntu-22.04, true)
🔇 Additional comments (3)
test/concrete/ob/OrderBook.deposit.entask.t.sol (1)

225-233: LGTM: toDecimalString(false) aligns with TOFU migration.

The changes to use toDecimalString(false) for formatting vault balance assertions are consistent with the broader migration to the TOFU (Trust On First Use) pattern for token decimals. Both the pre-deposit and post-deposit balance checks correctly use the same formatting approach.

test/concrete/ob/OrderBook.withdraw.entask.t.sol (2)

380-380: Verify toDecimalString(false) semantics and usage
Confirm the boolean false produces correct 6-decimal formatting and that no legacy integer-parameter calls remain across tests or contracts.


117-117: Approve read count reduction

The drop from 7→6 reads reflects removal of local decimals storage in OrderBook.sol and the shift to LibTOFUTokenDecimals.

Comment on lines 26 to 33
// 3 reads for reentrancy guard.
// 5 reads for deposit.
assertEq(reads.length, isFirstDeposit ? 8 : 6);
assertEq(reads.length, 5);
assertEq(reads[0], bytes32(uint256(0)));
assertEq(reads[1], bytes32(uint256(0)));
assertEq(reads[reads.length - 1], bytes32(uint256(0)));
// 2 writes for reentrancy guard.
// 2 write for deposit.
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix misleading comments describing read/write breakdown.

The comments don't correctly describe the breakdown of reads and writes:

  • Line 27 states "5 reads for deposit" which is the total, not a component. It should say "2 reads for deposit" to complete the breakdown (3 for reentrancy + 2 for deposit = 5 total).
  • Line 33 states "2 write for deposit" but the assertion shows 3 total writes. It should say "1 write for deposit" to match the breakdown (2 for reentrancy + 1 for deposit = 3 total).
  • Line 33 has a grammar issue: "write" should be "write" (singular) if count is 1.

Apply this diff to clarify the breakdown:

     // 3 reads for reentrancy guard.
-    // 5 reads for deposit.
+    // 2 reads for deposit.
     assertEq(reads.length, 5);
     assertEq(reads[0], bytes32(uint256(0)));
     assertEq(reads[1], bytes32(uint256(0)));
     assertEq(reads[reads.length - 1], bytes32(uint256(0)));
     // 2 writes for reentrancy guard.
-    // 2 write for deposit.
+    // 1 write for deposit.
     assertEq(writes.length, 3);
📝 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
// 3 reads for reentrancy guard.
// 5 reads for deposit.
assertEq(reads.length, isFirstDeposit ? 8 : 6);
assertEq(reads.length, 5);
assertEq(reads[0], bytes32(uint256(0)));
assertEq(reads[1], bytes32(uint256(0)));
assertEq(reads[reads.length - 1], bytes32(uint256(0)));
// 2 writes for reentrancy guard.
// 2 write for deposit.
// 3 reads for reentrancy guard.
// 2 reads for deposit.
assertEq(reads.length, 5);
assertEq(reads[0], bytes32(uint256(0)));
assertEq(reads[1], bytes32(uint256(0)));
assertEq(reads[reads.length - 1], bytes32(uint256(0)));
// 2 writes for reentrancy guard.
// 1 write for deposit.
assertEq(writes.length, 3);
🤖 Prompt for AI Agents
In test/concrete/ob/OrderBook.deposit.entask.t.sol around lines 26 to 33, the
inline comments incorrectly describe the read/write breakdown; update the reads
comment from "5 reads for deposit" to "2 reads for deposit" so the breakdown
reads "3 reads for reentrancy guard + 2 reads for deposit = 5", and update the
writes comment from "2 write for deposit" to "1 write for deposit" (use singular
"write") so the breakdown reads "2 writes for reentrancy guard + 1 write for
deposit = 3".

Copy link
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: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 546fae5 and 539c82e.

📒 Files selected for processing (4)
  • crates/test_fixtures/src/lib.rs (6 hunks)
  • pointers.sh (1 hunks)
  • prep-all.sh (1 hunks)
  • tauri-app/src-tauri/src/commands/order_take.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
prep-all.sh

📄 CodeRabbit inference engine (AGENTS.md)

Use ./prep-all.sh to bootstrap (install deps and build workspaces)

Files:

  • prep-all.sh
tauri-app/**

📄 CodeRabbit inference engine (AGENTS.md)

Desktop app code is in tauri-app (Rust + Svelte); src-tauri is not part of the Cargo workspace

Files:

  • tauri-app/src-tauri/src/commands/order_take.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust with nix develop -c cargo fmt --all
Lint Rust with nix develop -c rainix-rs-static (preconfigured flags)
Use snake_case for Rust crates/modules and PascalCase for types

Files:

  • tauri-app/src-tauri/src/commands/order_take.rs
  • crates/test_fixtures/src/lib.rs
crates/*/{src,tests,benches,examples}/**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Rust code must pass clippy with all warnings denied (cargo clippy --workspace --all-targets --all-features -D warnings)

Files:

  • crates/test_fixtures/src/lib.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Rust code lives in the workspace under crates/* (e.g., cli, common, bindings, js_api, quote, subgraph, settings, math, integration_tests)

Files:

  • crates/test_fixtures/src/lib.rs
🧠 Learnings (8)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.orderbook#2149
File: src/concrete/arb/RouteProcessorOrderBookV5ArbOrderTaker.sol:37-38
Timestamp: 2025-09-16T07:37:24.455Z
Learning: In the rainlanguage/rain.orderbook codebase, caching token decimals is considered dangerous unless handled with TOFU (Trust On First Use) pattern and expectation checks on every use. The additional validation required eliminates any gas savings from caching, so calling IERC20Metadata(...).decimals() on each use is the preferred safe pattern, as explained by thedavidmeister.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Solidity: build with nix develop -c forge build; test with nix develop -c forge test

Applied to files:

  • prep-all.sh
  • pointers.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Quick preflight before PR: nix develop -c npm run lint-format-check:all && nix develop -c rainix-rs-static

Applied to files:

  • prep-all.sh
  • pointers.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Rust: build with nix develop -c cargo build --workspace; test with nix develop -c cargo test

Applied to files:

  • prep-all.sh
  • pointers.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: WASM bundle via nix develop -c rainix-wasm-artifacts

Applied to files:

  • prep-all.sh
  • pointers.sh
📚 Learning: 2025-10-06T08:05:02.438Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-06T08:05:02.438Z
Learning: Always run all commands via Nix using: nix develop -c <command>

Applied to files:

  • pointers.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Run all commands via nix develop -c <cmd>; avoid interactive Nix shells (use shell attrs like nix develop .#tauri-shell when needed)

Applied to files:

  • pointers.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: JS workspaces: nix develop -c npm run test/build commands at top-level

Applied to files:

  • pointers.sh
🔇 Additional comments (4)
tauri-app/src-tauri/src/commands/order_take.rs (1)

236-236: Approve scientific notation change
No internal CSV readers detected; external CSV tools support “-5e-1”. Consider noting this formatting update in the CHANGELOG.

crates/test_fixtures/src/lib.rs (3)

6-6: LGTM!

The import changes correctly add address macro for the new address constants and remove the now-unused FromStr import.


136-141: LGTM!

The refactoring of MULTICALL3_ADDRESS from a string literal to a typed Address constant improves type safety and follows Rust best practices. The usage is correctly updated throughout the code.

Also applies to: 351-351


112-118: Confirm TOFUTokenDecimals address and DEPLOYED_BYTECODE

  • No other .anvil_set_code entries found in the repo.
  • Verify 0x4f1C29FAAB7EDdF8D7794695d8259996734Cc665 is the intended deployment address.
  • Confirm TOFUTokenDecimals::DEPLOYED_BYTECODE matches the compiled runtime bytecode.

Comment on lines +37 to +40
sol!(
#![sol(all_derives = true, rpc = true)]
TOFUTokenDecimals, "../../lib/rain.tofu.erc20-decimals/out/TOFUTokenDecimals.sol/TOFUTokenDecimals.json"
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the external submodule path and artifact.

The sol! binding references an external submodule artifact at ../../lib/rain.tofu.erc20-decimals/out/TOFUTokenDecimals.sol/TOFUTokenDecimals.json. Ensure that:

  1. The submodule is properly initialized and built before this code runs
  2. The artifact path is correct and matches the Forge output structure
  3. The contract ABI and bytecode are compatible with the expected interface

🏁 Script executed:

#!/bin/bash
# Verify the TOFUTokenDecimals artifact exists and is valid

# Check if submodule directory exists
if [ -d "lib/rain.tofu.erc20-decimals" ]; then
    echo "✓ Submodule directory exists"
else
    echo "✗ Submodule directory not found"
    exit 1
fi

# Check if the artifact file exists
ARTIFACT="lib/rain.tofu.erc20-decimals/out/TOFUTokenDecimals.sol/TOFUTokenDecimals.json"
if [ -f "$ARTIFACT" ]; then
    echo "✓ Artifact file exists"
    # Verify it's valid JSON and contains expected fields
    if jq -e '.abi and .bytecode' "$ARTIFACT" > /dev/null 2>&1; then
        echo "✓ Artifact contains abi and bytecode"
    else
        echo "✗ Artifact missing required fields"
        exit 1
    fi
else
    echo "✗ Artifact file not found at: $ARTIFACT"
    echo "Run 'cd lib/rain.tofu.erc20-decimals && nix develop -c forge build' to build it"
    exit 1
fi

Length of output: 216


Ensure the TOFUTokenDecimals artifact exists and binding path is correct

  • The submodule directory exists, but lib/rain.tofu.erc20-decimals/out/TOFUTokenDecimals.sol/TOFUTokenDecimals.json was not found.
  • Run cd lib/rain.tofu.erc20-decimals && nix develop -c forge build (or forge build) to generate the JSON, then confirm it resides at out/.../TOFUTokenDecimals.json or adjust the ../../lib/... path in crates/test_fixtures/src/lib.rs.
🤖 Prompt for AI Agents
In crates/test_fixtures/src/lib.rs around lines 37 to 40, the sol! binding
references
../../lib/rain.tofu.erc20-decimals/out/TOFUTokenDecimals.sol/TOFUTokenDecimals.json
which does not exist; either generate the artifact by running `cd
lib/rain.tofu.erc20-decimals && nix develop -c forge build` (or `forge build`)
so the JSON appears under out/.../TOFUTokenDecimals.json, or correct the
relative path in the sol! macro to point to the actual location of
TOFUTokenDecimals.json after confirming where the submodule’s build output is
placed.

const MULTICALL3_ADDRESS: Address = address!("cA11bde05977b3631167028862bE2a173976CA11");
const MULTICALL3_BYTECODE: &str = "0x6080604052600436106100f35760003560e01c80634d2301cc1161008a578063a8b0574e11610059578063a8b0574e1461025a578063bce38bd714610275578063c3077fa914610288578063ee82ac5e1461029b57600080fd5b80634d2301cc146101ec57806372425d9d1461022157806382ad56cb1461023457806386d516e81461024757600080fd5b80633408e470116100c65780633408e47014610191578063399542e9146101a45780633e64a696146101c657806342cbb15c146101d957600080fd5b80630f28c97d146100f8578063174dea711461011a578063252dba421461013a57806327e86d6e1461015b575b600080fd5b34801561010457600080fd5b50425b6040519081526020015b60405180910390f35b61012d610128366004610a85565b6102ba565b6040516101119190610bbe565b61014d610148366004610a85565b6104ef565b604051610111929190610bd8565b34801561016757600080fd5b50437fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0140610107565b34801561019d57600080fd5b5046610107565b6101b76101b2366004610c60565b610690565b60405161011193929190610cba565b3480156101d257600080fd5b5048610107565b3480156101e557600080fd5b5043610107565b3480156101f857600080fd5b50610107610207366004610ce2565b73ffffffffffffffffffffffffffffffffffffffff163190565b34801561022d57600080fd5b5044610107565b61012d610242366004610a85565b6106ab565b34801561025357600080fd5b5045610107565b34801561026657600080fd5b50604051418152602001610111565b61012d610283366004610c60565b61085a565b6101b7610296366004610a85565b610a1a565b3480156102a757600080fd5b506101076102b6366004610d18565b4090565b60606000828067ffffffffffffffff8111156102d8576102d8610d31565b60405190808252806020026020018201604052801561031e57816020015b6040805180820190915260008152606060208201528152602001906001900390816102f65790505b5092503660005b8281101561047757600085828151811061034157610341610d60565b6020026020010151905087878381811061035d5761035d610d60565b905060200281019061036f9190610d8f565b6040810135958601959093506103886020850185610ce2565b73ffffffffffffffffffffffffffffffffffffffff16816103ac6060870187610dcd565b6040516103ba929190610e32565b60006040518083038185875af1925050503d80600081146103f7576040519150601f19603f3d011682016040523d82523d6000602084013e6103fc565b606091505b50602080850191909152901515808452908501351761046d577f08c379a000000000000000000000000000000000000000000000000000000000600052602060045260176024527f4d756c746963616c6c333a2063616c6c206661696c656400000000000000000060445260846000fd5b5050600101610325565b508234146104e6576040517f08c379a000000000000000000000000000000000000000000000000000000000815260206004820152601a60248201527f4d756c746963616c6c333a2076616c7565206d69736d6174636800000000000060448201526064015b60405180910390fd5b50505092915050565b436060828067ffffffffffffffff81111561050c5761050c610d31565b60405190808252806020026020018201604052801561053f57816020015b606081526020019060019003908161052a5790505b5091503660005b8281101561068657600087878381811061056257610562610d60565b90506020028101906105749190610e42565b92506105836020840184610ce2565b73ffffffffffffffffffffffffffffffffffffffff166105a66020850185610dcd565b6040516105b4929190610e32565b6000604051808303816000865af19150503d80600081146105f1576040519150601f19603f3d011682016040523d82523d6000602084013e6105f6565b606091505b5086848151811061060957610609610d60565b602090810291909101015290508061067d576040517f08c379a000000000000000000000000000000000000000000000000000000000815260206004820152601760248201527f4d756c746963616c6c333a2063616c6c206661696c656400000000000000000060448201526064016104dd565b50600101610546565b5050509250929050565b43804060606106a086868661085a565b905093509350939050565b6060818067ffffffffffffffff8111156106c7576106c7610d31565b60405190808252806020026020018201604052801561070d57816020015b6040805180820190915260008152606060208201528152602001906001900390816106e55790505b5091503660005b828110156104e657600084828151811061073057610730610d60565b6020026020010151905086868381811061074c5761074c610d60565b905060200281019061075e9190610e76565b925061076d6020840184610ce2565b73ffffffffffffffffffffffffffffffffffffffff166107906040850185610dcd565b60405161079e929190610e32565b6000604051808303816000865af19150503d80600081146107db576040519150601f19603f3d011682016040523d82523d6000602084013e6107e0565b606091505b506020808401919091529015158083529084013517610851577f08c379a000000000000000000000000000000000000000000000000000000000600052602060045260176024527f4d756c746963616c6c333a2063616c6c206661696c656400000000000000000060445260646000fd5b50600101610714565b6060818067ffffffffffffffff81111561087657610876610d31565b6040519080825280602002602001820160405280156108bc57816020015b6040805180820190915260008152606060208201528152602001906001900390816108945790505b5091503660005b82811015610a105760008482815181106108df576108df610d60565b602002602001015190508686838181106108fb576108fb610d60565b905060200281019061090d9190610e42565b925061091c6020840184610ce2565b73ffffffffffffffffffffffffffffffffffffffff1661093f6020850185610dcd565b60405161094d929190610e32565b6000604051808303816000865af19150503d806000811461098a576040519150601f19603f3d011682016040523d82523d6000602084013e61098f565b606091505b506020830152151581528715610a07578051610a07576040517f08c379a000000000000000000000000000000000000000000000000000000000815260206004820152601760248201527f4d756c746963616c6c333a2063616c6c206661696c656400000000000000000060448201526064016104dd565b506001016108c3565b5050509392505050565b6000806060610a2b60018686610690565b919790965090945092505050565b60008083601f840112610a4b57600080fd5b50813567ffffffffffffffff811115610a6357600080fd5b6020830191508360208260051b8501011115610a7e57600080fd5b9250929050565b60008060208385031215610a9857600080fd5b823567ffffffffffffffff811115610aaf57600080fd5b610abb85828601610a39565b90969095509350505050565b6000815180845260005b81811015610aed57602081850181015186830182015201610ad1565b81811115610aff576000602083870101525b50601f017fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0169290920160200192915050565b600082825180855260208086019550808260051b84010181860160005b84811015610bb1578583037fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe001895281518051151584528401516040858501819052610b9d81860183610ac7565b9a86019a9450505090830190600101610b4f565b5090979650505050505050565b602081526000610bd16020830184610b32565b9392505050565b600060408201848352602060408185015281855180845260608601915060608160051b870101935082870160005b82811015610c52577fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa0888703018452610c40868351610ac7565b95509284019290840190600101610c06565b509398975050505050505050565b600080600060408486031215610c7557600080fd5b83358015158114610c8557600080fd5b9250602084013567ffffffffffffffff811115610ca157600080fd5b610cad86828701610a39565b9497909650939450505050565b838152826020820152606060408201526000610cd96060830184610b32565b95945050505050565b600060208284031215610cf457600080fd5b813573ffffffffffffffffffffffffffffffffffffffff81168114610bd157600080fd5b600060208284031215610d2a57600080fd5b5035919050565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b7f4e487b7100000000000000000000000000000000000000000000000000000000600052603260045260246000fd5b600082357fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff81833603018112610dc357600080fd5b9190910192915050565b60008083357fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe1843603018112610e0257600080fd5b83018035915067ffffffffffffffff821115610e1d57600080fd5b602001915036819003821315610a7e57600080fd5b8183823760009101908152919050565b600082357fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffc1833603018112610dc357600080fd5b600082357fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa1833603018112610dc357600080fdfea2646970667358221220bb2b5c71a328032f97c676ae39a1ec2148d3e5d6f73d95e9b17910152d61f16264736f6c634300080c0033";

const TOFU_TOKEN_DECIMALS_ADDRESS: Address = address!("4f1C29FAAB7EDdF8D7794695d8259996734Cc665");
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Document the address derivation.

The constant TOFU_TOKEN_DECIMALS_ADDRESS uses a specific address but lacks documentation explaining:

  1. How this address was derived (e.g., CREATE2 with specific salt, deterministic deployment)
  2. Why this specific address is used
  3. Whether this address is consistent across different environments

Consider adding a documentation comment:

+/// The canonical deployment address for TOFUTokenDecimals.
+/// This address is used in tests to etch the contract runtime code.
+/// Source: [explain derivation or reference]
 const TOFU_TOKEN_DECIMALS_ADDRESS: Address = address!("4f1C29FAAB7EDdF8D7794695d8259996734Cc665");
📝 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
const TOFU_TOKEN_DECIMALS_ADDRESS: Address = address!("4f1C29FAAB7EDdF8D7794695d8259996734Cc665");
/// The canonical deployment address for TOFUTokenDecimals.
/// This address is used in tests to etch the contract runtime code.
/// Source: [explain derivation or reference]
const TOFU_TOKEN_DECIMALS_ADDRESS: Address = address!("4f1C29FAAB7EDdF8D7794695d8259996734Cc665");
🤖 Prompt for AI Agents
In crates/test_fixtures/src/lib.rs at line 354, the constant
TOFU_TOKEN_DECIMALS_ADDRESS needs an explanatory doc comment: state exactly how
the address was derived (e.g., produced by CREATE2 with the specific salt and
init bytecode, or copied from a deterministic deployment script), why this
particular address is used in tests (e.g., to guarantee a stable token decimals
contract for fixtures), and whether it is expected to be identical across
environments; include a reference/link to the script/command that generates the
address (or the deployment tx/hash) and add/mention a small assertion or test
that recomputes the address from the same inputs to ensure consistency.

(cd lib/rain.interpreter && nix develop -c i9r-prelude)
(cd lib/rain.interpreter/lib/rain.metadata && nix develop -c rainix-sol-prelude)
(cd lib/rain.interpreter/lib/rain.metadata && nix develop -c rainix-rs-prelude)
(cd lib/rain.tofu.erc20-decimals && nix develop -c forge build)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify submodule initialization before build.

The forge build command for rain.tofu.erc20-decimals assumes the submodule is already initialized and its dependencies are ready. Ensure that:

  1. The submodule is initialized (via git submodule update --init --recursive or similar)
  2. Any submodule dependencies are properly set up before this build step
  3. The build order is correct (this runs after rain.math.float and rain.interpreter setup)

Based on the related changes in prep-all.sh, it appears the submodule initialization might be handled by nix develop -c forge install (line 30 in prep-all.sh). Verify that this is sufficient:


🏁 Script executed:

#!/bin/bash
# Verify the build prerequisites for rain.tofu.erc20-decimals

# Check if the submodule is registered
if git config --file .gitmodules --get-regexp path | grep -q "rain.tofu.erc20-decimals"; then
    echo "✓ Submodule registered in .gitmodules"
else
    echo "✗ Submodule not found in .gitmodules"
    exit 1
fi

# Check if the submodule directory exists and is initialized
if [ -d "lib/rain.tofu.erc20-decimals/.git" ] || [ -f "lib/rain.tofu.erc20-decimals/.git" ]; then
    echo "✓ Submodule initialized"
else
    echo "✗ Submodule not initialized"
    echo "Run: git submodule update --init --recursive"
    exit 1
fi

# Check if forge can be run in that directory
if [ -f "lib/rain.tofu.erc20-decimals/foundry.toml" ]; then
    echo "✓ Foundry project detected"
else
    echo "⚠ No foundry.toml found - may not be a Forge project"
fi

Length of output: 110


Ensure submodule is initialized before forge build
Line 12 of pointers.sh runs forge build in rain.tofu.erc20-decimals, but this submodule isn’t initialized on a fresh clone. Prepend

git submodule update --init --recursive lib/rain.tofu.erc20-decimals

(or confirm prep-all.sh covers it) before invoking the build.

🤖 Prompt for AI Agents
In pointers.sh around line 12, the script runs a forge build inside
lib/rain.tofu.erc20-decimals but the submodule may be uninitialized on a fresh
clone; ensure the submodule is initialized before the build by running git
submodule update --init --recursive lib/rain.tofu.erc20-decimals (or, if
prep-all.sh already handles this, call or source prep-all.sh beforehand), then
proceed to (cd lib/rain.tofu.erc20-decimals && nix develop -c forge build).

Comment on lines +38 to +39
echo "Setting up rain.tofu.erc20-decimals..."
(cd lib/rain.tofu.erc20-decimals && nix develop -c forge build)
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider consistent environment variable handling.

The new setup step for rain.tofu.erc20-decimals uses a simpler execution pattern compared to other submodule setups:

(cd lib/rain.tofu.erc20-decimals && nix develop -c forge build)

While other steps use:

nix develop -i ${keep[@]} -c bash -c '(cd lib/... && command)'

This inconsistency means the new step won't preserve environment variables (CI_DEPLOY_SEPOLIA_RPC_URL, etc.) that might be needed. If the forge build for this submodule doesn't require these variables, the current approach is acceptable. However, for consistency and future-proofing, consider aligning with the established pattern.

Apply this diff to align with the existing pattern:

 echo "Setting up rain.tofu.erc20-decimals..."
-(cd lib/rain.tofu.erc20-decimals && nix develop -c forge build)
+nix develop -i ${keep[@]} -c bash -c '(cd lib/rain.tofu.erc20-decimals && forge build)'

This ensures consistent environment handling across all setup steps.

🤖 Prompt for AI Agents
In prep-all.sh around lines 38 to 39, the new submodule setup uses "(cd
lib/rain.tofu.erc20-decimals && nix develop -c forge build)" which doesn't
preserve the same environment handling as other steps; replace it with the
established pattern using "nix develop -i ${keep[@]} -c bash -c '(cd
lib/rain.tofu.erc20-decimals && nix develop -c forge build)'" so the keep array
and environment variables are preserved and the command runs in a subshell with
consistent quoting and behavior.

Copy link
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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 08b8ebb and c9e48ab.

⛔ Files ignored due to path filters (1)
  • flake.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • flake.nix (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.orderbook#2149
File: src/concrete/arb/RouteProcessorOrderBookV5ArbOrderTaker.sol:37-38
Timestamp: 2025-09-16T07:37:24.455Z
Learning: In the rainlanguage/rain.orderbook codebase, caching token decimals is considered dangerous unless handled with TOFU (Trust On First Use) pattern and expectation checks on every use. The additional validation required eliminates any gas savings from caching, so calling IERC20Metadata(...).decimals() on each use is the preferred safe pattern, as explained by thedavidmeister.
⏰ 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). (17)
  • GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
  • GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
  • GitHub Check: git-clean
  • GitHub Check: test
  • GitHub Check: build-tauri (ubuntu-22.04, true)
  • GitHub Check: test
  • GitHub Check: test
  • GitHub Check: Deploy-Docs-Preview
  • GitHub Check: Deploy-Preview-Push
🔇 Additional comments (2)
flake.nix (2)

1-304: Inconsistency: PR summary does not match file changes.

The AI-generated summary mentions TOFU token decimals, submodule additions, and Solidity contract changes, but this file contains only Nix flake configuration updates. The PR description is also empty with no motivation or solution provided.

Please update the PR description to:

  1. Explain why the rainix dependency is being pinned to this specific commit
  2. Clarify why libsoup_2_4 is being added to the tauri-shell environment
  3. If this PR includes other files beyond flake.nix that implement the TOFU decimals changes, ensure those files are included in the review

5-5: Document rationale for pinned rainix commit

The pinned commit (f2a4edac01a17368ffdbae9fb42a110139acebde) exists; please update the PR description to explain why this specific commit was chosen and outline a plan for future updates.

@rouzwelt rouzwelt self-assigned this Oct 29, 2025
@rouzwelt rouzwelt marked this pull request as ready for review October 29, 2025 18:55
Copy link
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
prep-all.sh (1)

56-57: Quote array expansions to avoid word-splitting (unaddressed from prior review).

Lines 56–57 have unquoted array expansions that trigger shellcheck SC2068. If any element in keep[@] contains whitespace, it will be mis-split when passed to nix.

Apply this diff:

-nix develop -i ${keep[@]} .#tauri-shell -c ob-tauri-prelude
-nix develop -i ${keep[@]} .#tauri-shell -c ob-ui-components-prelude
+nix develop -i "${keep[@]}" .#tauri-shell -c ob-tauri-prelude
+nix develop -i "${keep[@]}" .#tauri-shell -c ob-ui-components-prelude
♻️ Duplicate comments (1)
prep-all.sh (1)

38-39: Environment variables not preserved in rain.tofu.erc20-decimals setup and unquoted array expansion throughout script.

Two issues identified:

  1. Lines 45–46 (rain.tofu.erc20-decimals setup): Does not preserve environment variables via the keep[@] array, unlike the established pattern used elsewhere (lines 40–42, 49–50, 54–55, 58–64). This is inconsistent and risks losing CI variables needed for the build.

  2. Multiple lines (40, 42, 49, 50, 54, 55, 58–64): Unquoted array expansion ${keep[@]} triggers shellcheck SC2068. Should use "${keep[@]}" to prevent word-splitting if any element contains spaces (though currently all elements are single-word flags, this is a defensive practice).

Apply this diff for lines 45–46:

 echo "Setting up rain.tofu.erc20-decimals..."
-(cd lib/rain.tofu.erc20-decimals && nix develop -c forge build)
+nix develop -i "${keep[@]}" -c bash -c '(cd lib/rain.tofu.erc20-decimals && forge build)'

Additionally, quote all ${keep[@]} expansions throughout the script to "${keep[@]}" (lines 40, 42, 49, 50, 54, 55, 58–64).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 139b458 and 8549030.

⛔ Files ignored due to path filters (2)
  • flake.lock is excluded by !**/*.lock
  • foundry.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • flake.nix (2 hunks)
  • packages/webapp/src/lib/constants.ts (1 hunks)
  • prep-all.sh (1 hunks)
  • tauri-app/src/lib/services/loadRemoteSettings.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
packages/webapp/**/*.{svelte,ts}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Webapp must pass Svelte lint/format checks (npm run svelte-lint-format-check -w @rainlanguage/webapp)

Files:

  • packages/webapp/src/lib/constants.ts
packages/{webapp,ui-components}/**/*.{svelte,ts}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

If you modify frontend code, you must include a screenshot of the built webapp reflecting the change

Files:

  • packages/webapp/src/lib/constants.ts
packages/**

📄 CodeRabbit inference engine (AGENTS.md)

JavaScript/Svelte projects live under packages/* (webapp, ui-components, orderbook)

Files:

  • packages/webapp/src/lib/constants.ts
tauri-app/**

📄 CodeRabbit inference engine (AGENTS.md)

Desktop app code is in tauri-app (Rust + Svelte); src-tauri is not part of the Cargo workspace

Files:

  • tauri-app/src/lib/services/loadRemoteSettings.ts
prep-all.sh

📄 CodeRabbit inference engine (AGENTS.md)

Use ./prep-all.sh to bootstrap (install deps and build workspaces)

Files:

  • prep-all.sh
🧠 Learnings (25)
📓 Common learnings
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:46:08.530Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report should be the only content in the comment - no text before it, no text after it, no formatting blocks, just the raw report in the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:34:42.328Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must use the format:
TOTAL:number
ADDITIONS:number
DELETIONS:number
with no extra text, formatting blocks, or explanations, and must still exclude lock files and reassess after each new commit.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:09:46.344Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must reassess the PR size after each new commit to ensure the report includes the latest changes, and calculate the total size up to the very latest commit.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: thedavidmeister
PR: rainlanguage/rain.orderbook#2149
File: src/concrete/arb/RouteProcessorOrderBookV5ArbOrderTaker.sol:37-38
Timestamp: 2025-09-16T07:37:24.455Z
Learning: In the rainlanguage/rain.orderbook codebase, caching token decimals is considered dangerous unless handled with TOFU (Trust On First Use) pattern and expectation checks on every use. The additional validation required eliminates any gas savings from caching, so calling IERC20Metadata(...).decimals() on each use is the preferred safe pattern, as explained by thedavidmeister.
Learnt from: Siddharth2207
PR: rainlanguage/rain.orderbook#2089
File: test/concrete/ob/OrderBook.withdraw.t.rounding.sol:7-7
Timestamp: 2025-08-15T15:57:45.830Z
Learning: In the rainlanguage/rain.orderbook codebase, user Siddharth2207 considers Math import from OpenZeppelin contracts as required in test files like test/concrete/ob/OrderBook.withdraw.t.rounding.sol, even when direct usage is not immediately visible in the code.
📚 Learning: 2025-10-19T20:24:08.629Z
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#2246
File: packages/webapp/src/hooks.client.ts:2-2
Timestamp: 2025-10-19T20:24:08.629Z
Learning: In the rain.orderbook repository, the import `rainlanguage/orderbook/esm` in packages/webapp/src/hooks.client.ts is intentional and should not be changed to the root package path, as it's specific to the webapp build configuration.

Applied to files:

  • packages/webapp/src/lib/constants.ts
📚 Learning: 2025-04-09T12:58:03.399Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/lib/services/loadRegistryUrl.ts:5-19
Timestamp: 2025-04-09T12:58:03.399Z
Learning: In the rain.orderbook application, URL validation for registry URLs is handled at the frontend/component level before the `loadRegistryUrl` function is called, so additional validation within this function is not necessary.

Applied to files:

  • packages/webapp/src/lib/constants.ts
📚 Learning: 2025-04-09T13:00:24.640Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/__tests__/registryManager.test.ts:227-238
Timestamp: 2025-04-09T13:00:24.640Z
Learning: In the rain.orderbook project, error handling was added to the RegistryManager to handle localStorage exceptions, addressing part of the verification agent's suggestions. The team should also consider testing extremely long URLs that might approach browser limits.

Applied to files:

  • packages/webapp/src/lib/constants.ts
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Quick preflight before PR: nix develop -c npm run lint-format-check:all && nix develop -c rainix-rs-static

Applied to files:

  • prep-all.sh
  • flake.nix
📚 Learning: 2025-05-21T23:09:27.578Z
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:09:27.578Z
Learning: For PR #1884 in rainlanguage/rain.orderbook repository, I must always reassess the PR size after each new commit, calculating the total changes up to the very latest commit and ensuring accuracy of the report. The calculation must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and report in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number".

Applied to files:

  • prep-all.sh
  • flake.nix
📚 Learning: 2025-05-21T21:26:31.566Z
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Solidity: build with nix develop -c forge build; test with nix develop -c forge test

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Run all commands via nix develop -c <cmd>; avoid interactive Nix shells (use shell attrs like nix develop .#tauri-shell when needed)

Applied to files:

  • prep-all.sh
📚 Learning: 2025-05-14T17:02:38.389Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1776
File: .envrc:10-11
Timestamp: 2025-05-14T17:02:38.389Z
Learning: Multiple `use flake` commands in a single `.envrc` file is a valid pattern when using `nix-direnv`, as it supports loading multiple development shells for different purposes.

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Applies to **/*.rs : Lint Rust with nix develop -c rainix-rs-static (preconfigured flags)

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Rust: build with nix develop -c cargo build --workspace; test with nix develop -c cargo test

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Tauri: nix develop .#tauri-shell --command cargo tauri dev

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: WASM bundle via nix develop -c rainix-wasm-artifacts

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:02.438Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-06T08:05:02.438Z
Learning: Always run all commands via Nix using: nix develop -c <command>

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: JS workspaces: nix develop -c npm run test/build commands at top-level

Applied to files:

  • prep-all.sh
📚 Learning: 2025-10-06T08:05:50.452Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: AGENTS.md:0-0
Timestamp: 2025-10-06T08:05:50.452Z
Learning: Applies to **/*.rs : Format Rust with nix develop -c cargo fmt --all

Applied to files:

  • prep-all.sh
📚 Learning: 2025-05-21T22:14:22.646Z
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.

Applied to files:

  • flake.nix
📚 Learning: 2025-05-21T22:14:22.646Z
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.

Applied to files:

  • flake.nix
📚 Learning: 2025-07-11T12:30:09.245Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1971
File: .github/workflows/vercel-docs-prod.yaml:32-34
Timestamp: 2025-07-11T12:30:09.245Z
Learning: findolor prefers not to pin Node versions explicitly in GitHub Actions workflows when using Nix with webapp-shell, considering the current setup sufficient for deterministic builds in the rainlanguage/rain.orderbook project.

Applied to files:

  • flake.nix
📚 Learning: 2025-06-06T16:38:04.182Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1913
File: Cargo.toml:55-55
Timestamp: 2025-06-06T16:38:04.182Z
Learning: In rain.orderbook repository, during PR chains involving dependency updates, wasm-bindgen-utils may temporarily point to git commits that don't contain the full required functionality, with the understanding that subsequent PRs in the chain will update it to the correct commit.

Applied to files:

  • flake.nix
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.

Applied to files:

  • flake.nix
📚 Learning: 2025-05-21T21:25:47.873Z
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).

Applied to files:

  • flake.nix
📚 Learning: 2025-05-21T21:25:47.873Z
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". This report should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).

Applied to files:

  • flake.nix
📚 Learning: 2025-05-21T21:24:53.708Z
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.

Applied to files:

  • flake.nix
⏰ 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: standard-tests (ubuntu-latest, test-js-bindings)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
  • GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
  • GitHub Check: build-tauri (ubuntu-22.04, true)
  • GitHub Check: test
  • GitHub Check: test
  • GitHub Check: Deploy-Preview-Push
  • GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (2)
packages/webapp/src/lib/constants.ts (1)

2-4: Both URLs are accessible and return valid content.

Verification confirms that both REGISTRY_URL and REMOTE_SETTINGS_URL are accessible at commit d57c3aa77b63f90535957947d15146c0182367dc with HTTP 200 responses. The registry file contains valid strategy entries and settings.yaml contains proper configuration data.

tauri-app/src/lib/services/loadRemoteSettings.ts (1)

2-2: No issues detected. URL is accessible and contains valid YAML content.

The HTTP 200 status confirms the URL is accessible, and the retrieved YAML content shows valid configuration with expected structure (version: 3, networks). The commit d57c3aa77b63f90535957947d15146c0182367dc exists and the settings.yaml file is properly accessible.

@thedavidmeister thedavidmeister merged commit 438bd7a into main Oct 29, 2025
19 checks passed
@github-actions
Copy link
Contributor

@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:

  • Simple bug fixes, typos, or minor refactoring
  • Single-purpose changes affecting 1-2 files
  • Documentation updates
  • Configuration tweaks
  • Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

  • Fix typo in variable name
  • Update README with new instructions
  • Adjust configuration values
  • Simple one-line bug fixes
  • Import statement cleanup

Medium (M)

Characteristics:

  • Feature additions or enhancements
  • Refactoring that touches multiple files but maintains existing behavior
  • Breaking changes with backward compatibility
  • Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

  • Add new feature or component
  • Refactor common utility functions
  • Update dependencies with minor breaking changes
  • Add new component with tests
  • Performance optimizations
  • More complex bug fixes

Large (L)

Characteristics:

  • Major feature implementations
  • Breaking changes or API redesigns
  • Complex refactoring across multiple modules
  • New architectural patterns or significant design changes
  • Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

  • Complete new feature with frontend/backend changes
  • Protocol upgrades or breaking changes
  • Major architectural refactoring
  • Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

  • Test coverage impact: More comprehensive test changes lean toward larger classification
  • Risk level: Changes to critical systems bump up a size category
  • Team familiarity: Novel patterns or technologies increase complexity

Notes:

  • the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
  • the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
  • do not include any additional text, only the size classification
  • your assessment comment must not include tips or additional sections
  • do NOT tag me or anyone else on your comment

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

Tip

For best results, initiate chat on the files or code changes.

SIZE=L


🧠 Learnings used
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: For PR #1884, after each commit review, I should provide a new comment with a PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding irrelevant files such as lock files (package-lock.json, cargo.lock, etc.).

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: After each commit in PR #1884, I should provide a new comment with the PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`. The report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". This report should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:46:08.530Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report should be the only content in the comment - no text before it, no text after it, no formatting blocks, just the raw report in the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1971
File: .github/workflows/vercel-docs-prod.yaml:32-34
Timestamp: 2025-07-11T12:30:09.245Z
Learning: findolor prefers not to pin Node versions explicitly in GitHub Actions workflows when using Nix with webapp-shell, considering the current setup sufficient for deterministic builds in the rainlanguage/rain.orderbook project.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1971
File: .github/workflows/vercel-docs-prod.yaml:10-14
Timestamp: 2025-07-11T12:29:49.284Z
Learning: In GitHub Actions workflows for the rainlanguage/rain.orderbook project, findolor is comfortable with job names that may seem misleading (like "Deploy-Docs-Preview" in a production workflow) when there's only one job in the workflow, as the context makes it clear enough.

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:34:42.328Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must use the format:
TOTAL:number
ADDITIONS:number
DELETIONS:number
with no extra text, formatting blocks, or explanations, and must still exclude lock files and reassess after each new commit.

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:09:46.344Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must reassess the PR size after each new commit to ensure the report includes the latest changes, and calculate the total size up to the very latest commit.

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.

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.

5 participants