Skip to content

Convert account balance from U256 to Float#2117

Merged
findolor merged 4 commits intomainfrom
account-balance-float
Oct 3, 2025
Merged

Convert account balance from U256 to Float#2117
findolor merged 4 commits intomainfrom
account-balance-float

Conversation

@findolor
Copy link
Copy Markdown
Collaborator

@findolor findolor commented Sep 2, 2025

Motivation

See issue: #2067

The existing vault account balance system was using U256 integers to represent token balances, which required
complex conversion logic when interfacing with the Float math system used throughout the application. This
created inconsistencies and required error-prone conversions between fixed-point integers and floating-point
representations, particularly when displaying formatted balances to users.

Solution

  • Refactored the AccountBalance struct to use Float directly instead of U256
  • Update UI and tests

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)

fix #2067

Summary by CodeRabbit

  • New Features
    • Balances now use a fixed-decimal Float representation for consistent decimal handling across web, UI, and desktop.
  • Bug Fixes
    • Improved accuracy and consistency of balance display, formatting, and validation in deposit/withdraw flows.
  • Refactor
    • Unified balance representation and getters across environments to Float, simplifying usage and formatting paths.
  • Tests
    • Updated tests and mocks to use Float-based balances and adjusted assertions.
  • Chores
    • Exported Float for consumers to parse/format balances.

@findolor findolor self-assigned this Sep 2, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 2, 2025

Walkthrough

Replaces bigint/U256 account balance representation with rain_math_float::Float across Rust core, JS API, Svelte components, Tauri, webapp, and tests. Constructors, getters, initializations, formatting, validation, and mocks updated so AccountBalance.balance is a Float and on-chain U256 values are converted via Float::from_fixed_decimal.

Changes

Cohort / File(s) Summary
Core balance model (Rust)
crates/common/src/raindex_client/vaults.rs
Change AccountBalance.balance type U256 → Float; remove PartialEq derive; update constructor and wasm/non-wasm getters to return Float; convert on-chain U256 via Float::from_fixed_decimal; remove format_amount_u256 usage.
JS API balance conversion
crates/js_api/src/gui/select_tokens.rs
Convert on-chain U256 → Float (Float::from_fixed_decimal), build AccountBalance::new(Float, formatted) and use Float::format() for formatting.
Orderbook JS API tests
packages/orderbook/test/js_api/gui.test.ts, packages/orderbook/test/js_api/raindexClient.test.ts
Update assertions to inspect Float fixed-decimal values (.toFixedDecimal(...).value) instead of bigint; minor whitespace edits.
UI components - production
packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte
Import Float; initialize AccountBalance.balance with Float.parse('0').value and reset to same on error.
UI components - tests
packages/ui-components/src/__tests__/*
packages/ui-components/src/__tests__/SelectToken.test.ts, .../TokenBalance.test.ts, .../TokenIOInput.test.ts, .../VaultIdInformation.test.ts
Import Float; add minimal mock wrappers for @rainlanguage/orderbook where needed; replace BigInt literals with Float.parse(...).value in test data and expectations.
Webapp - components
packages/webapp/src/lib/components/DepositModal.svelte, packages/webapp/src/lib/components/WithdrawModal.svelte
Initialize userBalance.balance as Float (via Float.parse('0').value); use raw userBalance.balance for validation and InputTokenAmount.maxValue instead of converting from fixed-decimal.
Webapp - tests
packages/webapp/src/__tests__/DepositModal.test.ts, packages/webapp/src/__tests__/WithdrawModal.test.ts
Replace BigInt-based mocked balances with Float.parse(...).value in getOwnerBalance mocks; formattedBalance strings unchanged.
Tauri app - component
tauri-app/src/lib/components/ModalVaultDeposit.svelte
Initialize userBalance.balance with Float.parse('0').value; use userBalance.balance directly for maxValue instead of fixed-decimal conversion.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor UI as UI
  participant JSAPI as JS API (wasm)
  participant Core as raindex_client (Rust)
  participant Chain as On-chain RPC

  UI->>JSAPI: get_account_balance(vault, owner)
  JSAPI->>Core: request owner balance
  Core->>Chain: fetch U256 raw balance
  Chain-->>Core: U256 balance
  Core->>Core: Float::from_fixed_decimal(U256, decimals)
  Core-->>JSAPI: AccountBalance{ balance: Float, formatted: String }
  JSAPI-->>UI: AccountBalance (Float)

  note over UI,JSAPI: Consumers use Float directly for validation/formatting/max values
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

rust, js/wasm, webapp, test

Suggested reviewers

  • hardyjosh
  • 0xgleb

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Convert account balance from U256 to Float” directly and concisely summarizes the primary change of switching the AccountBalance type throughout the codebase from U256 to Float without extraneous detail. It is specific enough to signal the core refactor to reviewers and avoids unnecessary noise.
Linked Issues Check ✅ Passed The changes fully implement the goals of issue #2067 by replacing U256 with Float for the AccountBalance struct, removing intermediate U256-to-Float conversions, and updating all downstream consumers—including UI components, JavaScript API, and tests—to use the Float representation for balance values. This ensures consistency and meets the linked issue’s requirements.
Out of Scope Changes Check ✅ Passed All modifications are directly related to the Float migration objective, encompassing updates to the common library, JS API, Svelte components, and test suites; no unrelated features or refactors are present, so the changes remain within the intended scope.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch account-balance-float

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a3f8ce and 37d0238.

📒 Files selected for processing (4)
  • crates/common/src/raindex_client/vaults.rs (4 hunks)
  • crates/js_api/src/gui/select_tokens.rs (2 hunks)
  • packages/orderbook/test/js_api/gui.test.ts (3 hunks)
  • packages/orderbook/test/js_api/raindexClient.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
crates/**/{src,tests,benches,examples}/**/*.rs

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

crates/**/{src,tests,benches,examples}/**/*.rs: Rust crates: lint with cargo clippy --workspace --all-targets --all-features -D warnings
Rust crates: ensure cargo test --workspace passes
Rust crates: build with cargo build during development/readiness checks

Files:

  • crates/js_api/src/gui/select_tokens.rs
  • crates/common/src/raindex_client/vaults.rs
packages/orderbook/**/*.{ts,tsx}

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

packages/orderbook/**/*.{ts,tsx}: Orderbook package: build with npm run build:orderbook
Orderbook package: type/lint check with npm run check -w @rainlanguage/orderbook
Orderbook package: run tests with npm run test -w @rainlanguage/orderbook

Files:

  • packages/orderbook/test/js_api/gui.test.ts
  • packages/orderbook/test/js_api/raindexClient.test.ts
🧠 Learnings (28)
📓 Common learnings
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: 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.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2117
File: packages/webapp/src/lib/components/DepositModal.svelte:25-28
Timestamp: 2025-09-02T08:05:07.646Z
Learning: In packages/webapp/src/lib/components/DepositModal.svelte, findolor considers the double-cast pattern `as unknown as AccountBalance` safe for initializing userBalance with Float.parse('0').value and formattedBalance: '0'.
📚 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:

  • crates/js_api/src/gui/select_tokens.rs
  • packages/orderbook/test/js_api/raindexClient.test.ts
  • crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-07-16T05:52:05.576Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
  • crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-09-02T08:05:07.646Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2117
File: packages/webapp/src/lib/components/DepositModal.svelte:25-28
Timestamp: 2025-09-02T08:05:07.646Z
Learning: In packages/webapp/src/lib/components/DepositModal.svelte, findolor considers the double-cast pattern `as unknown as AccountBalance` safe for initializing userBalance with Float.parse('0').value and formattedBalance: '0'.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
  • crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-07-21T16:34:31.193Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
  • crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-07-23T13:03:40.669Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/vault_balance_changes_query.rs:133-135
Timestamp: 2025-07-23T13:03:40.669Z
Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
  • packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-09-02T08:04:34.729Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 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:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-09T07:42:48.458Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1975
File: crates/js_api/src/config.rs:44-45
Timestamp: 2025-07-09T07:42:48.458Z
Learning: User findolor prefers to defer code improvements and refactoring suggestions to future PRs when they are not directly related to the current PR's scope, even when the suggestions are valid improvements.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-06-17T16:21:24.384Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1903
File: crates/settings/src/yaml/orderbook.rs:371-377
Timestamp: 2025-06-17T16:21:24.384Z
Learning: In crates/settings/src/yaml/orderbook.rs tests, the user findolor considers RPC ordering in Vec<Url> assertions to be intentional and not a test brittleness issue. The ordering of RPCs in tests should be preserved as specified.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-06-24T08:45:10.971Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1947
File: crates/common/src/raindex_client/orders.rs:462-1643
Timestamp: 2025-06-24T08:45:10.971Z
Learning: User findolor prefers to keep tests simple and avoid refactoring for maintainability when the code is stable and viewed as "one time thing" that won't require frequent changes.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-06-18T12:56:44.290Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1938
File: crates/js_api/src/raindex/mod.rs:102-118
Timestamp: 2025-06-18T12:56:44.290Z
Learning: In the rainlanguage/rain.orderbook codebase, it's acceptable to scaffold unused enum variants in initial implementation PRs when they will be implemented in future PRs, as confirmed by findolor.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-15T08:01:38.534Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1994
File: crates/common/src/raindex_client/vaults.rs:282-292
Timestamp: 2025-07-15T08:01:38.534Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor prefers to avoid concurrency optimizations like using `futures::future::try_join_all` for parallel processing of balance changes, considering such optimizations "not that critical at the moment" when the performance impact is minimal.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-04T10:27:22.544Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: crates/common/src/raindex_client/orders.rs:609-609
Timestamp: 2025-07-04T10:27:22.544Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor prefers not to implement overflow protection for trades count casting (usize to u16) at this time, considering it unnecessary for the current scope since the practical risk of orders having 65,535+ trades is extremely low.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like `#[cfg(not(target_family = "wasm"))] use super::*;` in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-21T16:37:20.599Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/utils/mod.rs:1-1
Timestamp: 2025-07-21T16:37:20.599Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers explicit module imports over re-exporting symbols at higher levels. Specifically for the float constants in crates/subgraph/src/utils/float.rs, they prefer using `utils::float::*` rather than re-exporting with `pub use float::*` in the utils module, as the explicit import makes it clearer what is being imported and why it's needed.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-08-04T09:07:00.160Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-06-04T10:21:01.388Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1907
File: packages/orderbook/test/common/test.test.ts:75-77
Timestamp: 2025-06-04T10:21:01.388Z
Learning: The DotrainOrder.create API in packages/orderbook/test/common/test.test.ts is internal and not used directly in consumer applications, so API changes here don't require external breaking change documentation.

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-09-06T07:12:25.751Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-06T07:12:25.751Z
Learning: Applies to packages/orderbook/**/*.{ts,tsx} : Orderbook package: run tests with `npm run test -w rainlanguage/orderbook`

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-07-16T14:33:03.454Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2001
File: packages/ui-components/src/lib/services/historicalOrderCharts.ts:18-21
Timestamp: 2025-07-16T14:33:03.454Z
Learning: User findolor is comfortable with potential precision loss when converting formatted decimal strings to JavaScript Number type in historicalOrderCharts.ts, preferring the simpler implementation over using decimal arithmetic libraries like decimal.js.

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-04-02T08:11:19.742Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1566
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/+page.svelte:26-31
Timestamp: 2025-04-02T08:11:19.742Z
Learning: In the Rain Orderbook deployment page component, the conditional check `if (dotrain && deployment)` inside `onMount` is necessary to prevent GUI initialization with null values, even though there's already a redirect logic outside the function and conditional rendering for the UI message. These serve different purposes in the component's error handling strategy.

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-03-26T16:22:50.224Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:0-0
Timestamp: 2025-03-26T16:22:50.224Z
Learning: In the Rain Orderbook project, the DotrainOrderGui.getDeploymentDetail method resolves promises with objects that may contain either a value property or an error property, rather than rejecting promises on errors.

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-05-03T10:29:04.009Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1724
File: packages/ui-components/src/__tests__/ButtonDarkMode.test.ts:1-54
Timestamp: 2025-05-03T10:29:04.009Z
Learning: JSDoc comments are not considered necessary for test files in the rainlanguage/rain.orderbook repository. Test descriptions and assertions are sufficient documentation.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-08-02T02:34:32.237Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2036
File: crates/js_api/src/filters/raindex_filter_store.rs:446-533
Timestamp: 2025-08-02T02:34:32.237Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore WASM tests (crates/js_api/src/filters/raindex_filter_store.rs), brusherru decided to focus on testing only methods without side effects (that don't use web_sys) due to difficulties with mocking localStorage and window APIs in the WASM test environment. This pragmatic approach tests pure logic separately from browser integration.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 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:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-04-04T11:25:21.518Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1559
File: packages/ui-components/src/__tests__/OrderOrVaultHash.test.ts:94-94
Timestamp: 2025-04-04T11:25:21.518Z
Learning: In the rain.orderbook project, minimal test fixtures are preferred over complete mocks that implement the entire interface. Type casting (e.g., `as unknown as SgVault`) is an acceptable approach to maintain both minimal fixtures and TypeScript type compatibility.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-03-26T16:16:51.943Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:37-37
Timestamp: 2025-03-26T16:16:51.943Z
Learning: For Rain Orderbook projects, in test files, the preference is to use "as any" type assertions with per-line ESLint disable comments rather than creating dedicated types for test parameters.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-08-01T09:07:20.383Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2044
File: packages/orderbook/README.md:186-189
Timestamp: 2025-08-01T09:07:20.383Z
Learning: In the rainlanguage/rain.orderbook project, Rust methods on structs like RaindexVaultsList are exported as JavaScript getters in WASM bindings using #[wasm_bindgen(getter)]. This means while the Rust code uses method calls like items(), the JavaScript/WASM API exposes them as property access like .items. The README.md correctly documents the JavaScript API surface, not the Rust implementation details.

Applied to files:

  • crates/common/src/raindex_client/vaults.rs
🧬 Code graph analysis (2)
crates/js_api/src/gui/select_tokens.rs (1)
crates/common/src/raindex_client/vaults.rs (6)
  • vaults (995-1011)
  • balance (102-104)
  • balance (144-146)
  • balance (183-185)
  • balance (193-195)
  • new (172-177)
crates/common/src/raindex_client/vaults.rs (1)
subgraph/src/float.ts (1)
  • Float (4-4)
⏰ 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). (18)
  • GitHub Check: test
  • GitHub Check: copilot-setup-steps
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
  • GitHub Check: test
  • GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
  • 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-wasm-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
  • GitHub Check: git-clean
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
  • GitHub Check: Deploy-Preview-Push
  • GitHub Check: build-tauri (ubuntu-22.04, true)
  • GitHub Check: Deploy-Docs-Preview
  • GitHub Check: test
🔇 Additional comments (2)
crates/js_api/src/gui/select_tokens.rs (1)

423-424: Update non‑wasm test to construct AccountBalance with Float.

AccountBalance::new now expects a Float, but test_get_account_balance still passes a U256. This no longer type-checks and will break the non-wasm test build. Please derive the Float in the test (using Float::from_fixed_decimal(..)) before calling AccountBalance::new.

Apply this diff:

@@
-        use crate::gui::{DotrainOrderGui, GuiError};
-        use alloy::primitives::{Address, U256};
+        use crate::gui::{DotrainOrderGui, GuiError};
+        use alloy::primitives::{Address, U256};
+        use rain_math_float::Float;
@@
-            assert_eq!(
-                balance,
-                AccountBalance::new(U256::from(1000), "0.000000000000001".to_string(),)
-            );
+            assert_eq!(
+                balance,
+                AccountBalance::new(
+                    Float::from_fixed_decimal(U256::from(1000), 18).unwrap(),
+                    "0.000000000000001".to_string(),
+                )
+            );

Also applies to: 1063-1066

packages/orderbook/test/js_api/raindexClient.test.ts (1)

1953-1953: Float balance assertion looks solid

The test now validates the raw 18‑decimal fixed value obtained from the Float, which correctly stays at 1000 wei-style units for this fixture.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

@findolor findolor requested review from 0xgleb and hardyjosh September 2, 2025 07:54
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

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

⚠️ Outside diff range comments (8)
packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte (1)

131-137: Deduplicate zero balance literal.

Create a single ZERO_ACCOUNT_BALANCE to reuse.

+	const ZERO_ACCOUNT_BALANCE = { balance: Float.parse('0').value as Float, formattedBalance: '0' } as AccountBalance;
 ...
-		balances.set(tokenInfo.key, {
-			value: { balance: Float.parse('0').value, formattedBalance: '0' } as AccountBalance,
+		balances.set(tokenInfo.key, {
+			value: ZERO_ACCOUNT_BALANCE,
 			loading: true,
 			error: ''
 		});
tauri-app/src/lib/components/ModalVaultDeposit.svelte (2)

55-61: Bug: parsing hex allowance with Float.parse() will fail. Use fromFixedDecimal.

vault.getAllowance() returns a hex U256; parse it with token decimals.

-      let allowanceFloat = Float.parse(allowance.value);
-      if (allowanceFloat.error) {
-        throw new Error(allowanceFloat.error.readableMsg);
-      }
+      const allowanceFloatRes = Float.fromFixedDecimal(allowance.value, vault.token.decimals);
+      if (allowanceFloatRes.error) {
+        throw new Error(allowanceFloatRes.error.readableMsg);
+      }
+      const allowanceFloat = allowanceFloatRes.value;

20-33: Optional: initialize amount to zero to avoid undefined on first render.

Prevents transient disabled-state logic from relying on falsy checks.

-  let amount: Float;
+  let amount: Float = Float.parse('0').value as Float;
crates/common/src/raindex_client/vaults.rs (5)

327-336: Amount validation works; minor micro-optimization optional.

Parsing "0" each call is fine; consider a small helper zero() if available to avoid parse overhead.

-        let zero_float = Float::parse("0".to_string())?;
+        // If available: prefer a zero constructor/const to avoid parsing every call.
+        let zero_float = Float::parse("0".to_string())?;

355-367: Docs: update examples to pass Float, not string.

The JS examples still pass string amounts; the API expects a Float value now.

-/// const result = await vault.getDepositCalldata(vault, "10.5");
+/// const result = await vault.getDepositCalldata(Float.parse("10.5").value);

386-394: Docs: update withdraw example to Float.

-/// const result = await vault.getWithdrawCalldata("55.2");
+/// const result = await vault.getWithdrawCalldata(Float.parse("55.2").value);

451-459: Docs: update approval example to Float.

-/// const result = await vault.getApprovalCalldata("20.75");
+/// const result = await vault.getApprovalCalldata(Float.parse("20.75").value);

172-178: Fix remaining AccountBalance::new call to use Float
In crates/js_api/src/gui/select_tokens.rs:1064, the test still calls

AccountBalance::new(U256::from(1000), "0.000000000000001".to_string())

—update it to pass a Float (e.g. via Float::from_fixed_decimal) and the preformatted string.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5f21269 and 6a3f8ce.

📒 Files selected for processing (14)
  • crates/common/src/raindex_client/vaults.rs (4 hunks)
  • crates/js_api/src/gui/select_tokens.rs (2 hunks)
  • packages/orderbook/test/js_api/gui.test.ts (3 hunks)
  • packages/orderbook/test/js_api/raindexClient.test.ts (1 hunks)
  • packages/ui-components/src/__tests__/SelectToken.test.ts (6 hunks)
  • packages/ui-components/src/__tests__/TokenBalance.test.ts (7 hunks)
  • packages/ui-components/src/__tests__/TokenIOInput.test.ts (4 hunks)
  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts (3 hunks)
  • packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte (3 hunks)
  • packages/webapp/src/__tests__/DepositModal.test.ts (6 hunks)
  • packages/webapp/src/__tests__/WithdrawModal.test.ts (1 hunks)
  • packages/webapp/src/lib/components/DepositModal.svelte (3 hunks)
  • packages/webapp/src/lib/components/WithdrawModal.svelte (1 hunks)
  • tauri-app/src/lib/components/ModalVaultDeposit.svelte (2 hunks)
🧰 Additional context used
🧠 Learnings (36)
📓 Common learnings
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: 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.
📚 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:

  • packages/orderbook/test/js_api/raindexClient.test.ts
  • crates/js_api/src/gui/select_tokens.rs
  • crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-06-04T10:21:01.388Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1907
File: packages/orderbook/test/common/test.test.ts:75-77
Timestamp: 2025-06-04T10:21:01.388Z
Learning: The DotrainOrder.create API in packages/orderbook/test/common/test.test.ts is internal and not used directly in consumer applications, so API changes here don't require external breaking change documentation.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
  • packages/orderbook/test/js_api/gui.test.ts
  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
📚 Learning: 2025-08-02T02:34:32.237Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2036
File: crates/js_api/src/filters/raindex_filter_store.rs:446-533
Timestamp: 2025-08-02T02:34:32.237Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore WASM tests (crates/js_api/src/filters/raindex_filter_store.rs), brusherru decided to focus on testing only methods without side effects (that don't use web_sys) due to difficulties with mocking localStorage and window APIs in the WASM test environment. This pragmatic approach tests pure logic separately from browser integration.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-03-26T16:16:51.943Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:37-37
Timestamp: 2025-03-26T16:16:51.943Z
Learning: For Rain Orderbook projects, in test files, the preference is to use "as any" type assertions with per-line ESLint disable comments rather than creating dedicated types for test parameters.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-05-03T10:29:04.009Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1724
File: packages/ui-components/src/__tests__/ButtonDarkMode.test.ts:1-54
Timestamp: 2025-05-03T10:29:04.009Z
Learning: JSDoc comments are not considered necessary for test files in the rainlanguage/rain.orderbook repository. Test descriptions and assertions are sufficient documentation.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 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:

  • packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-04-04T11:25:21.518Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1559
File: packages/ui-components/src/__tests__/OrderOrVaultHash.test.ts:94-94
Timestamp: 2025-04-04T11:25:21.518Z
Learning: In the rain.orderbook project, minimal test fixtures are preferred over complete mocks that implement the entire interface. Type casting (e.g., `as unknown as SgVault`) is an acceptable approach to maintain both minimal fixtures and TypeScript type compatibility.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts
📚 Learning: 2025-08-04T09:07:00.160Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
  • packages/webapp/src/__tests__/WithdrawModal.test.ts
  • packages/webapp/src/lib/components/DepositModal.svelte
  • packages/webapp/src/lib/components/WithdrawModal.svelte
  • packages/orderbook/test/js_api/gui.test.ts
  • packages/webapp/src/__tests__/DepositModal.test.ts
  • tauri-app/src/lib/components/ModalVaultDeposit.svelte
  • packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte
  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
  • packages/ui-components/src/__tests__/TokenBalance.test.ts
  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts
📚 Learning: 2025-07-21T16:34:31.193Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
  • crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-07-04T09:14:35.748Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: packages/ui-components/src/__tests__/VaultDetail.test.ts:177-181
Timestamp: 2025-07-04T09:14:35.748Z
Learning: In the rain.orderbook project, maintainer findolor is fine with mutating mock data directly in test files using ts-expect-error comments, preferring this pragmatic approach over immutable patterns like spread operators for test data updates.

Applied to files:

  • packages/orderbook/test/js_api/raindexClient.test.ts
  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
📚 Learning: 2025-05-20T12:08:13.760Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1870
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:74-97
Timestamp: 2025-05-20T12:08:13.760Z
Learning: In the WithdrawModal test suite, testing that transactions are correctly passed to the transactionStore is considered sufficient coverage, without needing explicit tests for callback execution.

Applied to files:

  • packages/webapp/src/__tests__/WithdrawModal.test.ts
📚 Learning: 2025-05-20T12:05:48.570Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1870
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:291-303
Timestamp: 2025-05-20T12:05:48.570Z
Learning: The modal closing behavior in the WithdrawModal component is handled by Flowbite's native implementation rather than through component props, making it unnecessary to test the open state changes directly.

Applied to files:

  • packages/webapp/src/__tests__/WithdrawModal.test.ts
📚 Learning: 2025-04-08T16:35:15.127Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1504
File: tauri-app/src/routes/orders/[network]-[orderHash]/+page.svelte:33-37
Timestamp: 2025-04-08T16:35:15.127Z
Learning: In the Rain OrderBook project, the onDeposit and onWithdraw functions in page components are kept simple (just calling modal handlers and revalidating queries) because error handling for deposit and withdraw actions (including user cancellations and failed transactions) is handled within the modal components themselves.

Applied to files:

  • packages/webapp/src/lib/components/DepositModal.svelte
  • packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte
📚 Learning: 2025-05-20T12:03:18.032Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1870
File: packages/webapp/src/lib/components/WithdrawModal.svelte:31-31
Timestamp: 2025-05-20T12:03:18.032Z
Learning: The VaultActionArgs type no longer contains an action property after refactoring the deposit/withdraw modals into separate components.

Applied to files:

  • packages/webapp/src/lib/components/DepositModal.svelte
  • tauri-app/src/lib/components/ModalVaultDeposit.svelte
📚 Learning: 2025-07-23T13:03:40.669Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/vault_balance_changes_query.rs:133-135
Timestamp: 2025-07-23T13:03:40.669Z
Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-04-02T08:11:19.742Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1566
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/+page.svelte:26-31
Timestamp: 2025-04-02T08:11:19.742Z
Learning: In the Rain Orderbook deployment page component, the conditional check `if (dotrain && deployment)` inside `onMount` is necessary to prevent GUI initialization with null values, even though there's already a redirect logic outside the function and conditional rendering for the UI message. These serve different purposes in the component's error handling strategy.

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-03-26T16:22:50.224Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:0-0
Timestamp: 2025-03-26T16:22:50.224Z
Learning: In the Rain Orderbook project, the DotrainOrderGui.getDeploymentDetail method resolves promises with objects that may contain either a value property or an error property, rather than rejecting promises on errors.

Applied to files:

  • packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-07-16T05:52:05.576Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-15T08:01:38.534Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1994
File: crates/common/src/raindex_client/vaults.rs:282-292
Timestamp: 2025-07-15T08:01:38.534Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor prefers to avoid concurrency optimizations like using `futures::future::try_join_all` for parallel processing of balance changes, considering such optimizations "not that critical at the moment" when the performance impact is minimal.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-04T10:27:22.544Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: crates/common/src/raindex_client/orders.rs:609-609
Timestamp: 2025-07-04T10:27:22.544Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor prefers not to implement overflow protection for trades count casting (usize to u16) at this time, considering it unnecessary for the current scope since the practical risk of orders having 65,535+ trades is extremely low.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like `#[cfg(not(target_family = "wasm"))] use super::*;` in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-07-21T16:37:20.599Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/utils/mod.rs:1-1
Timestamp: 2025-07-21T16:37:20.599Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers explicit module imports over re-exporting symbols at higher levels. Specifically for the float constants in crates/subgraph/src/utils/float.rs, they prefer using `utils::float::*` rather than re-exporting with `pub use float::*` in the utils module, as the explicit import makes it clearer what is being imported and why it's needed.

Applied to files:

  • crates/js_api/src/gui/select_tokens.rs
📚 Learning: 2025-04-04T09:56:04.949Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1493
File: packages/ui-components/src/lib/utils/handleDeployment.ts:10-43
Timestamp: 2025-04-04T09:56:04.949Z
Learning: In the rain.orderbook project, errors from the handleDeployment utility function are thrown to DeploymentSteps.svelte, which handles displaying the errors to the user through the DeploymentStepsError utility.

Applied to files:

  • packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte
📚 Learning: 2025-03-31T10:16:53.544Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1469
File: packages/ui-components/src/__tests__/CodeMirrorDotrain.test.ts:75-98
Timestamp: 2025-03-31T10:16:53.544Z
Learning: In the rainlanguage/rain.orderbook project, direct manipulation of Svelte's internal state (component.$$.ctx) in tests is an acceptable approach for testing component behavior, as demonstrated in the CodeMirrorDotrain tests.

Applied to files:

  • packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte
  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts
📚 Learning: 2025-06-07T05:19:04.767Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1872
File: packages/webapp/src/lib/services/handleVaultWithdraw.ts:50-54
Timestamp: 2025-06-07T05:19:04.767Z
Learning: In the rainlanguage/rain.orderbook codebase, the team prefers pragmatic code approaches over strict TypeScript patterns when the current implementation is sufficient for their use case, such as using `if (result.error)` instead of `if ('error' in result)` for discriminated union type checking.

Applied to files:

  • packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte
📚 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:

  • packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte
📚 Learning: 2025-07-09T12:35:45.699Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1974
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.

Applied to files:

  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
  • packages/ui-components/src/__tests__/TokenBalance.test.ts
  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts
📚 Learning: 2025-06-30T14:17:16.626Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1926
File: packages/ui-components/src/lib/__mocks__/stores.ts:13-17
Timestamp: 2025-06-30T14:17:16.626Z
Learning: User findolor reports that vi.mock(import('rainlanguage/orderbook'), async (importOriginal) => { ... }) syntax works in their testing environment, despite official Vitest documentation indicating the first argument should be a string. This suggests there may be specific Vitest versions or configurations that support dynamic import() as the first argument to vi.mock().

Applied to files:

  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts
📚 Learning: 2025-08-14T18:29:32.933Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2083
File: packages/ui-components/src/__tests__/VaultsListTable.test.ts:16-19
Timestamp: 2025-08-14T18:29:32.933Z
Learning: In the rain.orderbook project's UI components tests, mocking hooks like useToasts is often required as infrastructure even when not directly asserting on their calls, because components internally depend on these hooks. Removing such mocks would break component rendering in tests and require more complex test setup with providers.

Applied to files:

  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
📚 Learning: 2025-06-17T14:55:22.914Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1936
File: packages/ui-components/vite.config.ts:21-23
Timestamp: 2025-06-17T14:55:22.914Z
Learning: In the rain.orderbook project, the Vite configuration uses `'import.meta.vitest': 'undefined'` (as a string) combined with conditional `if (import.meta.vitest)` checks for in-source testing. The mock files are excluded from test execution using `exclude: ['src/lib/__mocks__/**/*.ts']`. This configuration successfully allows dev builds to work without `vi` undefined errors, despite the theoretical expectation that the string "undefined" would be truthy and cause issues.

Applied to files:

  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts
📚 Learning: 2025-04-08T09:18:46.653Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1597
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:120-120
Timestamp: 2025-04-08T09:18:46.653Z
Learning: In test files for the Rain Orderbook project, it's acceptable to bypass TypeScript's strict typing using constructs like `as unknown as [Type]` to create simplified mock objects that don't need to implement the entire interface.

Applied to files:

  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
📚 Learning: 2025-06-08T18:43:51.842Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1891
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/page.test.ts:66-80
Timestamp: 2025-06-08T18:43:51.842Z
Learning: In the rain.orderbook webapp test files, when mocking objects like the transaction manager, it's acceptable to use simple empty objects with ts-expect-error comments rather than providing complete mock implementations with all properties and methods.

Applied to files:

  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
📚 Learning: 2025-03-26T15:00:17.997Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1516
File: packages/webapp/src/routes/deploy/[strategyName]/layout.test.ts:0-0
Timestamp: 2025-03-26T15:00:17.997Z
Learning: For Rain Orderbook projects, there's a preference not to include tests for SvelteKit methods (like parent function rejections) in test files, as these are considered framework responsibilities rather than application code that needs testing.

Applied to files:

  • packages/ui-components/src/__tests__/TokenIOInput.test.ts
  • packages/ui-components/src/__tests__/SelectToken.test.ts
📚 Learning: 2025-07-17T10:36:02.846Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1996
File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like `VaultIdInformationComponentProps` in test files, even when static analysis tools flag this as discouraged practice.

Applied to files:

  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts
📚 Learning: 2025-07-31T19:34:11.716Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2044
File: crates/common/src/raindex_client/vaults_list.rs:363-423
Timestamp: 2025-07-31T19:34:11.716Z
Learning: In the rainlanguage/rain.orderbook project, for WASM-exposed functionality like VaultsList, the team prefers to keep comprehensive tests in the non-WASM environment due to the complexity of recreating objects like RaindexVaults in WASM. WASM tests focus on basic functionality and error cases since the WASM code reuses the already-tested non-WASM implementation.

Applied to files:

  • packages/ui-components/src/__tests__/VaultIdInformation.test.ts
🧬 Code graph analysis (4)
packages/webapp/src/__tests__/WithdrawModal.test.ts (1)
subgraph/src/float.ts (1)
  • Float (4-4)
packages/webapp/src/__tests__/DepositModal.test.ts (1)
subgraph/src/float.ts (1)
  • Float (4-4)
crates/js_api/src/gui/select_tokens.rs (1)
crates/common/src/raindex_client/vaults.rs (6)
  • vaults (995-1011)
  • balance (102-104)
  • balance (144-146)
  • balance (183-185)
  • balance (193-195)
  • new (172-177)
packages/ui-components/src/__tests__/TokenBalance.test.ts (1)
packages/ui-components/src/lib/types/tokenBalance.ts (1)
  • TokenBalance (3-7)
⏰ 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: standard-tests (ubuntu-latest, rainix-sol-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
  • 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-wasm-test)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
  • GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
  • GitHub Check: test
  • GitHub Check: build-tauri (ubuntu-22.04, true)
  • GitHub Check: git-clean
  • GitHub Check: test
  • GitHub Check: Deploy-Docs-Preview
  • GitHub Check: Deploy-Preview-Push
🔇 Additional comments (34)
packages/webapp/src/lib/components/WithdrawModal.svelte (1)

25-28: Prefer precise typing over double-cast.

Avoid as unknown as AccountBalance; cast the inner value and then the object.

-	let userBalance: AccountBalance = {
-		balance: Float.parse('0').value,
-		formattedBalance: '0'
-	} as unknown as AccountBalance;
+	let userBalance: AccountBalance = {
+		balance: Float.parse('0').value as Float,
+		formattedBalance: '0'
+	} as AccountBalance;
⛔ Skipped due to learnings
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2003
File: packages/webapp/src/lib/components/WithdrawModal.svelte:25-28
Timestamp: 2025-07-17T07:49:52.476Z
Learning: In the rainlanguage/rain.orderbook project, findolor considers the double-cast pattern `as unknown as AccountBalance` acceptable for initializing variables when needed, even when static analysis suggests alternatives for type safety.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1597
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:120-120
Timestamp: 2025-04-08T09:18:46.653Z
Learning: In test files for the Rain Orderbook project, it's acceptable to bypass TypeScript's strict typing using constructs like `as unknown as [Type]` to create simplified mock objects that don't need to implement the entire interface.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2001
File: packages/ui-components/src/lib/services/historicalOrderCharts.ts:18-21
Timestamp: 2025-07-16T14:33:03.454Z
Learning: User findolor is comfortable with potential precision loss when converting formatted decimal strings to JavaScript Number type in historicalOrderCharts.ts, preferring the simpler implementation over using decimal arithmetic libraries like decimal.js.
packages/orderbook/test/js_api/raindexClient.test.ts (1)

1950-1953: LGTM: assertion updated for Float-based balance.

Validates the fixed-decimal path cleanly.

packages/webapp/src/__tests__/WithdrawModal.test.ts (1)

29-33: LGTM: mock uses Float for balance.

Matches project preference to unwrap .value in tests.

packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte (1)

14-16: LGTM: importing Float and AccountBalance here is correct.

tauri-app/src/lib/components/ModalVaultDeposit.svelte (2)

24-27: LGTM: default AccountBalance now uses Float.


156-159: LGTM: maxValue now uses Float wallet balance directly.

packages/webapp/src/lib/components/DepositModal.svelte (1)

52-52: validateAmount and maxValue accept Float as expected
Both validateAmount(amount: Float, balance: Float) and InputTokenAmount’s export let maxValue: Float | undefined are correctly typed to accept Float; no changes needed.

packages/orderbook/test/js_api/gui.test.ts (3)

1145-1145: Whitespace-only change.

No action needed.


1613-1613: Whitespace-only change.

No action needed.


2045-2046: Assertion updated to Float fixed-decimal — correct.

Comparing toFixedDecimal(18).value to the raw base-unit BigInt aligns with the mocked 18 decimals and formatted human string.

packages/webapp/src/__tests__/DepositModal.test.ts (1)

29-31: Mocks migrated to Float — good coverage.

Using Float.parse(...).value across tests keeps fixtures consistent with the new API.

Also applies to: 81-83, 116-118, 137-139, 162-164, 195-197

packages/ui-components/src/__tests__/SelectToken.test.ts (3)

6-6: Importing Float for tests — OK.


44-49: Module mock wrapper preserves original exports — OK.

Matches prior project preference for this pattern.


334-336: Balance fixtures switched to Float — consistent with type migration.

Also applies to: 369-371, 404-406, 439-441

packages/ui-components/src/__tests__/TokenBalance.test.ts (3)

6-6: Importing Float and AccountBalance — OK.


10-15: Module mock wrapper — OK.

Keeps real exports available while allowing overrides.


18-19: Fixtures use Float.parse(...).value — looks good.

Optional: add a tiny helper to reduce repetition in tests.

+const f = (s: string) => Float.parse(s).value;
- tokenBalance: createMockTokenBalance(Float.parse('1000').value, '1000', false, '')
+ tokenBalance: createMockTokenBalance(f('1000'), '1000', false, '')

Also applies to: 35-36, 46-47, 57-63, 74-75, 86-86, 96-101

⛔ Skipped due to learnings
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1559
File: packages/ui-components/src/__tests__/OrderOrVaultHash.test.ts:94-94
Timestamp: 2025-04-04T11:25:21.518Z
Learning: In the rain.orderbook project, minimal test fixtures are preferred over complete mocks that implement the entire interface. Type casting (e.g., `as unknown as SgVault`) is an acceptable approach to maintain both minimal fixtures and TypeScript type compatibility.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1974
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1916
File: packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json:182-195
Timestamp: 2025-06-10T12:04:54.107Z
Learning: In test fixture files like `packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json`, network configuration inconsistencies (such as matchain using Polygon's RPC, chainId, and currency while having its own network key) are acceptable since they are used for testing purposes only.
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/vault_balance_changes_query.rs:133-135
Timestamp: 2025-07-23T13:03:40.669Z
Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.
packages/ui-components/src/__tests__/TokenIOInput.test.ts (5)

5-5: Importing Float is correct and aligns tests with the new AccountBalance shape.


9-14: Module mock pattern looks good and follows project conventions.

Merging the real module and overriding DotrainOrderGui with vi.fn() matches prior learnings and avoids breaking other exports.


131-137: Good migration to Float-based balance in test fixtures.

Using Float.parse('1').value is consistent with project preferences to unwrap in tests.


153-159: LGTM: zero balance via Float.parse('0').value.


175-181: LGTM: error-state fixture uses Float.parse('0').value.

packages/ui-components/src/__tests__/VaultIdInformation.test.ts (5)

5-5: Correctly importing Float for the new AccountBalance representation.


21-27: Fixture updated to Float is correct; assertions remain valid.


42-48: LGTM: loading-state uses Float.parse('0').value as per repo norms.


60-66: LGTM: error-state uses Float.parse('0').value.


2-2: Ensure vi is available or import it explicitly.

If Vitest globals are disabled in this workspace, this file will fail due to vi.mock usage without import.

-import { describe, it, expect } from 'vitest';
+import { describe, it, expect, vi } from 'vitest';
crates/common/src/raindex_client/vaults.rs (5)

164-170: AccountBalance: migration to Float is correct and consistent.

Fields and derives look right for (de)serialization and wasm exposure.


183-185: WASM getter returns Float by value — acceptable if Float is wasm-bindgen compatible.

Assuming rain_math_float::Float already exposes wasm bindings (used elsewhere), this is fine.


193-199: Non-WASM getter returning Float matches struct change.


539-543: Formatting consistency: format() vs format18().

Here formatted_balance uses format(), while vault and balance change paths use format18(). If intentional (ERC20 decimals vs canonical 18), add a brief comment to prevent regressions; otherwise unify.

-        let account_balance = AccountBalance {
-            balance: float_balance,
-            formatted_balance: float_balance.format()?,
-        };
+        // NOTE: Using token decimals here; other paths use format18() for subgraph canonicals.
+        let account_balance = AccountBalance {
+            balance: float_balance,
+            formatted_balance: float_balance.format()?,
+        };

1181-1183: Subgraph conversion to Float via from_hex and format18() looks correct.

Matches how subgraph encodes Float values; tests confirm with F* constants.

crates/js_api/src/gui/select_tokens.rs (2)

3-3: Import Float for balance migration — OK

Necessary for the U256 → Float refactor.


8-8: AccountBalance import — OK

Matches the updated constructor/signature in common.

Comment thread crates/js_api/src/gui/select_tokens.rs
Comment on lines +9 to +13
vi.mock('@rainlanguage/orderbook', async (importOriginal) => {
return {
...(await importOriginal())
};
});
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Sep 2, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

No-op passthrough mock is harmless but optional.

You can drop this block unless you plan to override exports here. Keeping it is fine if it matches your test scaffolding style.

-vi.mock('@rainlanguage/orderbook', async (importOriginal) => {
-	return {
-		...(await importOriginal())
-	};
-});
+// Passthrough mock intentionally omitted (no overrides needed)
📝 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
vi.mock('@rainlanguage/orderbook', async (importOriginal) => {
return {
...(await importOriginal())
};
});
// Passthrough mock intentionally omitted (no overrides needed)
🤖 Prompt for AI Agents
In packages/ui-components/src/__tests__/VaultIdInformation.test.ts around lines
9 to 13 there is a no-op passthrough mock for '@rainlanguage/orderbook' that is
unnecessary; remove the vi.mock(...) block entirely to simplify the test unless
you intend to override exports from that module, in which case replace it with a
targeted mock that only overrides the specific exports you need; after editing,
run the test suite to confirm nothing else depends on that mock.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is needed for tests to run

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@findolor, understood! I'll note that this passthrough mock is required in your testing environment.


✏️ Learnings added
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2117
File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:9-13
Timestamp: 2025-09-02T08:04:44.805Z
Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts and similar test files in the rain.orderbook project, the passthrough vi.mock('rainlanguage/orderbook', async (importOriginal) => { return { ...(await importOriginal()) }; }); block is required for tests to run properly, even when not overriding any exports. This is needed due to the specific Vitest configuration or test environment setup in the project.

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.


🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1996
File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like `VaultIdInformationComponentProps` in test files, even when static analysis tools flag this as discouraged practice.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1974
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.

Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1559
File: packages/ui-components/src/__tests__/OrderOrVaultHash.test.ts:94-94
Timestamp: 2025-04-04T11:25:21.518Z
Learning: In the rain.orderbook project, minimal test fixtures are preferred over complete mocks that implement the entire interface. Type casting (e.g., `as unknown as SgVault`) is an acceptable approach to maintain both minimal fixtures and TypeScript type compatibility.

Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1494
File: packages/ui-components/src/__tests__/WalletProvider.test.ts:18-28
Timestamp: 2025-03-24T12:27:07.862Z
Learning: In the WalletProvider tests, verifying that setAccountContext is called with the correct store is sufficient. Additional testing of Svelte's store implementation (like subscribing to verify the store value) is unnecessary as it would just be testing the framework itself.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1926
File: packages/ui-components/src/lib/__mocks__/stores.ts:13-17
Timestamp: 2025-06-30T14:17:16.626Z
Learning: User findolor reports that vi.mock(import('rainlanguage/orderbook'), async (importOriginal) => { ... }) syntax works in their testing environment, despite official Vitest documentation indicating the first argument should be a string. This suggests there may be specific Vitest versions or configurations that support dynamic import() as the first argument to vi.mock().

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1469
File: packages/ui-components/src/__tests__/CodeMirrorDotrain.test.ts:75-98
Timestamp: 2025-03-31T10:16:53.544Z
Learning: In the rainlanguage/rain.orderbook project, direct manipulation of Svelte's internal state (component.$$.ctx) in tests is an acceptable approach for testing component behavior, as demonstrated in the CodeMirrorDotrain tests.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: packages/ui-components/src/__tests__/VaultDetail.test.ts:177-181
Timestamp: 2025-07-04T09:14:35.748Z
Learning: In the rain.orderbook project, maintainer findolor is fine with mutating mock data directly in test files using ts-expect-error comments, preferring this pragmatic approach over immutable patterns like spread operators for test data updates.

Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2083
File: packages/ui-components/src/__tests__/VaultsListTable.test.ts:16-19
Timestamp: 2025-08-14T18:29:32.933Z
Learning: In the rain.orderbook project's UI components tests, mocking hooks like useToasts is often required as infrastructure even when not directly asserting on their calls, because components internally depend on these hooks. Removing such mocks would break component rendering in tests and require more complex test setup with providers.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1891
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/page.test.ts:66-80
Timestamp: 2025-06-08T18:43:51.842Z
Learning: In the rain.orderbook webapp test files, when mocking objects like the transaction manager, it's acceptable to use simple empty objects with ts-expect-error comments rather than providing complete mock implementations with all properties and methods.

Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1597
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:120-120
Timestamp: 2025-04-08T09:18:46.653Z
Learning: In test files for the Rain Orderbook project, it's acceptable to bypass TypeScript's strict typing using constructs like `as unknown as [Type]` to create simplified mock objects that don't need to implement the entire interface.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1936
File: packages/ui-components/vite.config.ts:21-23
Timestamp: 2025-06-17T14:55:22.914Z
Learning: In the rain.orderbook project, the Vite configuration uses `'import.meta.vitest': 'undefined'` (as a string) combined with conditional `if (import.meta.vitest)` checks for in-source testing. The mock files are excluded from test execution using `exclude: ['src/lib/__mocks__/**/*.ts']`. This configuration successfully allows dev builds to work without `vi` undefined errors, despite the theoretical expectation that the string "undefined" would be truthy and cause issues.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1907
File: packages/orderbook/test/common/test.test.ts:75-77
Timestamp: 2025-06-04T10:21:01.388Z
Learning: The DotrainOrder.create API in packages/orderbook/test/common/test.test.ts is internal and not used directly in consumer applications, so API changes here don't require external breaking change documentation.

Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2044
File: crates/common/src/raindex_client/vaults_list.rs:363-423
Timestamp: 2025-07-31T19:34:11.716Z
Learning: In the rainlanguage/rain.orderbook project, for WASM-exposed functionality like VaultsList, the team prefers to keep comprehensive tests in the non-WASM environment due to the complexity of recreating objects like RaindexVaults in WASM. WASM tests focus on basic functionality and error cases since the WASM code reuses the already-tested non-WASM implementation.

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

Comment on lines 25 to 28
let userBalance: AccountBalance = {
balance: BigInt(0),
balance: Float.parse('0').value,
formattedBalance: '0'
} as unknown as AccountBalance;
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Sep 2, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unsafe cast on userBalance initialization.

Avoid as unknown as AccountBalance; keep types precise.

Apply:

-	let userBalance: AccountBalance = {
-		balance: Float.parse('0').value,
-		formattedBalance: '0'
-	} as unknown as AccountBalance;
+	let userBalance: AccountBalance = {
+		balance: Float.parse('0').value as Float,
+		formattedBalance: '0'
+	};
📝 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
let userBalance: AccountBalance = {
balance: BigInt(0),
balance: Float.parse('0').value,
formattedBalance: '0'
} as unknown as AccountBalance;
let userBalance: AccountBalance = {
balance: Float.parse('0').value as Float,
formattedBalance: '0'
};
🤖 Prompt for AI Agents
In packages/webapp/src/lib/components/DepositModal.svelte around lines 25–28,
the userBalance initialization uses an unsafe double-cast "as unknown as
AccountBalance"; replace this by constructing a literal that matches the
AccountBalance shape so the compiler can verify types (e.g., set balance to the
correct Float/number value expected by AccountBalance and formattedBalance to
'0'), remove the "as unknown as" cast, and adjust imports/types if needed so the
assigned object exactly matches AccountBalance without forced casting.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is safe to do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@findolor Understood! I'll note that you consider this casting pattern safe for the AccountBalance initialization.


✏️ Learnings added
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2117
File: packages/webapp/src/lib/components/DepositModal.svelte:25-28
Timestamp: 2025-09-02T08:05:07.614Z
Learning: In packages/webapp/src/lib/components/DepositModal.svelte, findolor considers the double-cast pattern `as unknown as AccountBalance` safe for initializing userBalance with Float.parse('0').value and formattedBalance: '0'.

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.


🧠 Learnings used
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1504
File: tauri-app/src/routes/orders/[network]-[orderHash]/+page.svelte:33-37
Timestamp: 2025-04-08T16:35:15.127Z
Learning: In the Rain OrderBook project, the onDeposit and onWithdraw functions in page components are kept simple (just calling modal handlers and revalidating queries) because error handling for deposit and withdraw actions (including user cancellations and failed transactions) is handled within the modal components themselves.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.

Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1504
File: packages/webapp/src/routes/orders/[network]-[orderHash]/+page.svelte:50-74
Timestamp: 2025-04-03T11:29:00.839Z
Learning: In the Rain OrderBook app, error handling for deposit and withdraw actions (including user cancellations and failed transactions) is implemented in the transaction modal component, not in the page-level action handlers.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#2003
File: packages/webapp/src/lib/components/WithdrawModal.svelte:25-28
Timestamp: 2025-07-17T07:49:52.476Z
Learning: In the rainlanguage/rain.orderbook project, findolor considers the double-cast pattern `as unknown as AccountBalance` acceptable for initializing variables when needed, even when static analysis suggests alternatives for type safety.

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

@findolor findolor merged commit 16fd538 into main Oct 3, 2025
19 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 3, 2025

@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
Copy Markdown
Contributor

coderabbitai Bot commented Oct 3, 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: 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.

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.

Update user balance amount type to be float instead of u256

3 participants