-
Notifications
You must be signed in to change notification settings - Fork 13
Implementing take orders logic in SDK #2360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2025-12-11-take-orders-candidates
Are you sure you want to change the base?
Implementing take orders logic in SDK #2360
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces a complete take-orders SDK phase 3 implementation with simulation and builder functionality. Adds new raindex_client error variants, a wasm-exported Changes
Sequence DiagramsequenceDiagram
participant Client as Client / WASM
participant RaindexClient
participant Parser
participant Fetcher as SubgraphFetcher
participant Candidates
participant Simulator as Simulation
participant Selector as Selection
participant ConfigBuilder
participant Encoder
Client->>RaindexClient: get_take_orders_calldata(chain_id, sell_token, buy_token, buy_amount, price_cap, min_receive_mode)
rect rgb(200, 220, 255)
Note over Parser: Parse & Validate Input
RaindexClient->>Parser: parse_request(...)
Parser->>Parser: validate addresses, amounts, price_cap
Parser-->>RaindexClient: ParsedTakeOrdersRequest or Error
end
rect rgb(200, 220, 255)
Note over Fetcher: Fetch Orders from Subgraph
RaindexClient->>Fetcher: fetch_orders_for_pair(chain_id, sell_token, buy_token)
Fetcher->>Fetcher: apply directional filters
Fetcher-->>RaindexClient: Vec<RaindexOrder> or NoLiquidity
end
rect rgb(200, 220, 255)
Note over Candidates: Build Candidates & Get RPC Block
RaindexClient->>Candidates: build_take_order_candidates_for_pair(orders, sell_token, buy_token, block_number)
Candidates->>Candidates: fetch quotes, build candidates per order
Candidates-->>RaindexClient: Vec<TakeOrderCandidate>
end
rect rgb(220, 240, 220)
Note over Selector: Select Best Orderbook & Simulate
RaindexClient->>Selector: select_best_orderbook_simulation(candidates, buy_target, price_cap)
Selector->>Selector: group candidates by orderbook
loop For each orderbook group
Selector->>Simulator: simulate_buy_over_candidates(candidates, buy_target, price_cap)
Simulator->>Simulator: sort by ratio (cheapest first)
Simulator->>Simulator: greedily select legs until buy_target exhausted
Simulator-->>Selector: SimulatedBuyResult
end
Selector->>Selector: compare simulations (total buy amount, worst price, address)
Selector-->>RaindexClient: (best_orderbook, best_sim)
end
rect rgb(220, 240, 220)
Note over ConfigBuilder: Build Configuration & Encode Calldata
RaindexClient->>ConfigBuilder: build_take_orders_config_from_buy_simulation(sim, buy_target, price_cap, min_receive_mode)
ConfigBuilder->>ConfigBuilder: validate liquidity (Exact mode)
ConfigBuilder->>ConfigBuilder: set minimumInput, maximumInput, maximumIORatio
ConfigBuilder->>ConfigBuilder: map simulation legs to config orders
ConfigBuilder-->>RaindexClient: BuiltTakeOrdersConfig
RaindexClient->>Encoder: build_calldata_result(orderbook, config, buy_target, price_cap)
Encoder->>Encoder: encode takeOrders3 calldata (ABI)
Encoder->>Encoder: compute effective_price = total_sell / total_buy
Encoder->>Encoder: collect per-leg prices
Encoder-->>RaindexClient: TakeOrdersCalldataResult
end
RaindexClient-->>Client: TakeOrdersCalldataResult { calldata, effective_price, prices, ... }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
|
We reviewed the builder and confirmed takeOrders3 is exact-in on the buy side, not the sell side. The previous exact-sell approach couldn’t enforce a sell budget on-chain. We’ve pivoted to a buy-target + price-cap flow: maximumInput = buyAmount, minimumInput is 0 (partial) or buyAmount (exact), and maximumIORatio = priceCap. Candidates are filtered by the price cap, simulated to reach the buy target, and we return expected sell, per-leg prices, and a worst-case spend cap (buyAmount * priceCap). This aligns the SDK with contract semantics and removes the misleading sell-budget behavior. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
crates/common/src/raindex_client/mod.rs(4 hunks)crates/common/src/raindex_client/take_orders/e2e_tests.rs(1 hunks)crates/common/src/raindex_client/take_orders/mod.rs(1 hunks)crates/common/src/raindex_client/take_orders/request.rs(1 hunks)crates/common/src/raindex_client/take_orders/result.rs(1 hunks)crates/common/src/raindex_client/take_orders/selection.rs(1 hunks)crates/common/src/take_orders/candidates.rs(26 hunks)crates/common/src/take_orders/config.rs(1 hunks)crates/common/src/take_orders/mod.rs(1 hunks)crates/common/src/take_orders/price.rs(1 hunks)crates/common/src/take_orders/simulation.rs(1 hunks)crates/common/src/test_helpers.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
crates/**/*.rs: For Rust crates incrates/*, run lints usingnix develop -c cargo clippy --workspace --all-targets --all-features -D warnings
For Rust crates incrates/*, run tests usingnix develop -c cargo test --workspaceor--package <crate>
Files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/mod.rscrates/common/src/take_orders/simulation.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/config.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/price.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
**/crates/**
📄 CodeRabbit inference engine (AGENTS.md)
Rust workspace organized as
crates/*with subdirectories: cli, common, bindings, js_api, quote, subgraph, settings, math, integration_tests
Files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/mod.rscrates/common/src/take_orders/simulation.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/config.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/price.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Rust: format code withnix develop -c cargo fmt --all
Rust: lint withnix develop -c rainix-rs-static(preconfigured flags included)
Rust: crates and modules usesnake_case; types usePascalCase
Files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/mod.rscrates/common/src/take_orders/simulation.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/config.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/price.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
🧠 Learnings (58)
📓 Common learnings
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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: findolor
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 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.
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/raindex_client/take_orders/selection.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/price.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-07-21T16:34:31.193Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 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/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/take_orders/price.rs
📚 Learning: 2025-09-30T21:18:01.636Z
Learnt from: hardyjosh
Repo: rainlanguage/rain.orderbook PR: 2167
File: crates/virtual-raindex/src/engine/take.rs:131-138
Timestamp: 2025-09-30T21:18:01.636Z
Learning: In the virtual-raindex take order flow (crates/virtual-raindex/src/engine/take.rs), balance diffs are written from the order's perspective where the taker is the counterparty: the order's input column receives taker_output (what the taker provides to the order) and the order's output column receives taker_input (what the taker requests from the order).
Applied to files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/raindex_client/take_orders/selection.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-07-21T16:37:20.599Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 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/common/src/raindex_client/mod.rscrates/common/src/take_orders/mod.rscrates/common/src/take_orders/price.rs
📚 Learning: 2025-06-17T16:46:19.035Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/cli/src/commands/order/calldata.rs:47-57
Timestamp: 2025-06-17T16:46:19.035Z
Learning: In the CLI command `crates/cli/src/commands/order/calldata.rs`, the user prefers to let lower-level errors from `try_into_call()` bubble up when the RPCs list is empty, rather than adding early validation checks with custom error messages.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rs
📚 Learning: 2025-10-06T14:13:18.531Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/trades.rs:133-152
Timestamp: 2025-10-06T14:13:18.531Z
Learning: In the rain.orderbook codebase, the `page` parameter in `RaindexOrder::get_trades_list` method (in crates/common/src/raindex_client/trades.rs) is kept for backwards compatibility with subgraph logic, but the LocalDb fast-path intentionally returns all trades without implementing pagination.
Applied to files:
crates/common/src/raindex_client/mod.rs
📚 Learning: 2025-08-01T09:07:20.383Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 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/mod.rscrates/common/src/raindex_client/take_orders/mod.rs
📚 Learning: 2025-08-01T07:35:13.418Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/errors.rs:39-46
Timestamp: 2025-08-01T07:35:13.418Z
Learning: In the Rain Orderbook project's PersistentFilterStoreError (crates/js_api/src/filters/errors.rs), the Display implementation already provides user-friendly error messages, so using err.to_string() for both msg and readable_msg in the WasmEncodedError conversion is appropriate and doesn't require separate readable message handling.
Applied to files:
crates/common/src/raindex_client/mod.rs
📚 Learning: 2025-06-24T13:36:28.797Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1950
File: crates/common/src/raindex_client/orders.rs:301-301
Timestamp: 2025-06-24T13:36:28.797Z
Learning: In the RaindexClient codebase, when Arc::new(RwLock::new(self.clone())) is used (e.g., in get_orders and get_order_by_hash methods), this creates new Arc/RwLock wrappers around a cloned RaindexClient, but the underlying client data is functionally the same. This pattern is used to provide the correct Arc<RwLock<RaindexClient>> type expected by RaindexOrder::try_from_sg_order() method.
Applied to files:
crates/common/src/raindex_client/mod.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/mod.rscrates/common/src/take_orders/simulation.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/take_orders/config.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/price.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-10-06T11:13:29.956Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2123
File: crates/common/src/raindex_client/local_db/mod.rs:23-29
Timestamp: 2025-10-06T11:13:29.956Z
Learning: In `crates/common/src/raindex_client/local_db/mod.rs`, the `Default` implementation for `LocalDb` that creates an RPC client pointing to `http://localhost:4444` is acceptable because the RPC client must be explicitly configured before actual usage in production scenarios.
Applied to files:
crates/common/src/raindex_client/mod.rs
📚 Learning: 2025-10-18T10:38:41.273Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/selection.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-08-26T14:52:37.000Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2099
File: crates/common/src/hyper_rpc.rs:3-7
Timestamp: 2025-08-26T14:52:37.000Z
Learning: In the rain.orderbook codebase, creating new reqwest::Client instances per request in HyperRpcClient is not considered an issue by the maintainers, despite potential performance benefits of client reuse.
Applied to files:
crates/common/src/raindex_client/mod.rs
📚 Learning: 2025-05-09T05:28:22.089Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/js_api/src/subgraph/order.rs:109-114
Timestamp: 2025-05-09T05:28:22.089Z
Learning: In the rain.orderbook project, SubgraphError contains or converts from OrderbookSubgraphClientError, so using OrderbookSubgraphClientError in error creation followed by a conversion to SubgraphError (via ? operator) is acceptable.
Applied to files:
crates/common/src/raindex_client/mod.rs
📚 Learning: 2025-10-06T11:44:07.888Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Applied to files:
crates/common/src/raindex_client/mod.rs
📚 Learning: 2025-07-04T10:27:22.544Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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/common/src/raindex_client/mod.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-05-17T15:32:28.733Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/simulation.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-03-31T14:36:11.049Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1474
File: crates/js_api/src/yaml/mod.rs:37-44
Timestamp: 2025-03-31T14:36:11.049Z
Learning: The OrderbookYaml implementation in crates/js_api/src/yaml/mod.rs intentionally parses YAML on demand without caching results. This is a deliberate design choice by the author to process YAML only when needed rather than optimizing for repeated calls.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-06-18T19:24:40.518Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/settings/src/yaml/orderbook.rs:185-199
Timestamp: 2025-06-18T19:24:40.518Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers not to refactor get_orderbook_by_network_key to handle multiple orderbooks per network key since their current architecture maintains a one-to-one mapping between orderbooks and networks. They would consider the refactoring if the system evolves to support multiple orderbooks per network in the future.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-10-02T19:17:20.332Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/request.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-10-14T07:51:55.148Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2202
File: crates/common/src/raindex_client/local_db/sync.rs:33-34
Timestamp: 2025-10-14T07:51:55.148Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the hard-coded `DEFAULT_SYNC_CHAIN_ID` constant (set to `SUPPORTED_LOCAL_DB_CHAINS[0]`) will be replaced with proper chain ID handling in downstream PRs as part of the multi-network/orderbook implementation.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/common/src/raindex_client/take_orders/selection.rs
📚 Learning: 2025-07-11T08:37:24.423Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1983
File: crates/settings/src/gui.rs:965-973
Timestamp: 2025-07-11T08:37:24.423Z
Learning: In the rainlanguage/rain.orderbook project, user findolor prefers to keep user-facing error messages generic and user-friendly rather than technically specific. For example, using "a valid number" instead of "a valid u8 (0-255)" in GUI validation error messages to maintain simplicity for end users.
Applied to files:
crates/common/src/raindex_client/mod.rs
📚 Learning: 2025-08-02T02:34:32.237Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 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:
crates/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/simulation.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/config.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/price.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-06-24T08:46:03.368Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Applied to files:
crates/common/src/raindex_client/take_orders/selection.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-07-15T08:01:38.534Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-07-04T09:02:57.301Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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/common/src/raindex_client/take_orders/selection.rs
📚 Learning: 2025-06-17T16:21:24.384Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/config.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/price.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-06-18T18:24:32.049Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/settings/src/yaml/orderbook.rs:180-199
Timestamp: 2025-06-18T18:24:32.049Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers to avoid refactoring duplicate search logic between get_orderbook_by_address and get_orderbook_by_network_key when there are only 2 functions, indicating they would consider it if more similar functions are added in the future.
Applied to files:
crates/common/src/raindex_client/take_orders/selection.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-07-16T14:33:45.887Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2001
File: crates/common/src/raindex_client/vaults.rs:0-0
Timestamp: 2025-07-16T14:33:45.887Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor considers using Address::random() in tests to be acceptable when the mock server doesn't validate the address parameter and returns a fixed response, making the test deterministic regardless of the address value used.
Applied to files:
crates/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-05-20T15:15:01.333Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1876
File: packages/webapp/src/lib/services/resetActiveOrderbookRef.ts:19-20
Timestamp: 2025-05-20T15:15:01.333Z
Learning: In the Rain orderbook project, there is no preferred orderbook when resetting the active orderbook reference, so selecting the first available one (even with non-deterministic object key ordering) is acceptable.
Applied to files:
crates/common/src/raindex_client/take_orders/selection.rs
📚 Learning: 2025-07-31T19:34:11.716Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 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:
crates/common/src/raindex_client/take_orders/selection.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/config.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-07-21T16:34:04.947Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1911
File: crates/common/src/raindex_client/orders.rs:720-720
Timestamp: 2025-07-21T16:34:04.947Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers using `.unwrap()` in test code rather than `.expect()` with descriptive messages, considering the direct unwrap approach acceptable for test contexts where failures should be fast and clear.
Applied to files:
crates/common/src/raindex_client/take_orders/selection.rscrates/common/src/take_orders/config.rscrates/common/src/take_orders/price.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-08-02T03:55:25.215Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: packages/orderbook/test/js_api/filters.test.ts:19-36
Timestamp: 2025-08-02T03:55:25.215Z
Learning: In the rainlanguage/rain.orderbook project's WASM tests, the pattern of chaining `.value!` calls on WASM result types (like from VaultsFilterBuilder methods) is the established and preferred approach for handling WASM results, and should not be refactored into intermediate variables as it would add unnecessary verbosity without improving the code.
Applied to files:
crates/common/src/raindex_client/take_orders/selection.rscrates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/config.rscrates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-05-14T05:13:59.713Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like `#[cfg(target_family = "wasm")]` should not be added to imports or implementations that may be needed for tests.
Applied to files:
crates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/config.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-05-09T05:21:40.234Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/js_api/src/subgraph/vault.rs:19-37
Timestamp: 2025-05-09T05:21:40.234Z
Learning: In the rain.orderbook project, the developer prefers to use tuple structs for WASM wrappers (like VaultCalldataResult, GetVaultsResult) rather than structs with named fields.
Applied to files:
crates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-05-14T05:52:04.270Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1715
File: crates/js_api/src/common/mod.rs:15-22
Timestamp: 2025-05-14T05:52:04.270Z
Learning: The project doesn't require `#[repr(transparent)]` for newtype wrappers in WASM contexts such as `AddOrderCalldata` and `RemoveOrderCalldata` as the current implementation is working as expected without it.
Applied to files:
crates/common/src/raindex_client/take_orders/mod.rscrates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-04-30T09:28:36.960Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using `#[cfg(target_family = "wasm")]` and `#[cfg(not(target_family = "wasm"))]`, and don't require additional `wasm_bindgen_test_configure!(run_in_browser)` directives.
Applied to files:
crates/common/src/raindex_client/take_orders/mod.rscrates/common/src/take_orders/config.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-05-19T13:40:56.080Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1858
File: crates/subgraph/src/orderbook_client/mod.rs:54-58
Timestamp: 2025-05-19T13:40:56.080Z
Learning: The `wasm_bindgen_utils` crate in the Rain Orderbook project handles conditional compilation for `JsValue` and `JsError` internally, allowing `impl From<Error> for JsValue` to work on non-WASM targets without explicit cfg guards.
Applied to files:
crates/common/src/raindex_client/take_orders/mod.rs
📚 Learning: 2025-05-14T04:49:14.621Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1710
File: crates/quote/Cargo.toml:30-30
Timestamp: 2025-05-14T04:49:14.621Z
Learning: In the rain.orderbook repository, wasm-bindgen-utils is intentionally used as an unconditional dependency because non-wasm targets (like tests) sometimes need to use it.
Applied to files:
crates/common/src/raindex_client/take_orders/mod.rs
📚 Learning: 2025-12-03T10:40:25.429Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2344
File: crates/common/src/local_db/pipeline/runner/mod.rs:18-31
Timestamp: 2025-12-03T10:40:25.429Z
Learning: In `crates/common/src/local_db/pipeline/runner/mod.rs`, the `TargetSuccess` struct does not need separate `ob_id` or `orderbook_key` fields because the contained `SyncOutcome` already includes orderbook identification information such as chain_id and orderbook_address. This avoids redundant data duplication.
Applied to files:
crates/common/src/raindex_client/take_orders/result.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-07-23T10:51:12.278Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2012
File: crates/js_api/src/registry.rs:233-252
Timestamp: 2025-07-23T10:51:12.278Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor considers the pattern of creating a partially initialized struct instance within a constructor and then populating it through async operations to be acceptable, as long as the partial state is never exposed to users and the constructor returns Result<T, E> with proper error propagation using the ? operator.
Applied to files:
crates/common/src/take_orders/config.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to crates/integration_tests/**/*.rs : Rust: write tests using `cargo test`; integration tests live in `crates/integration_tests`. Prefer `insta` snapshots and `proptest` where helpful
Applied to files:
crates/common/src/raindex_client/take_orders/e2e_tests.rs
📚 Learning: 2025-05-20T10:20:08.206Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1859
File: crates/quote/src/quote_debug.rs:472-492
Timestamp: 2025-05-20T10:20:08.206Z
Learning: In the Rain Orderbook codebase, the `#[tokio::test(flavor = "multi_thread")]` annotation is specifically needed for tests that use `LocalEvm`, not just for consistency across all async tests.
Applied to files:
crates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/candidates.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/orderbook/**/*.{ts,tsx,js,jsx} : For Orderbook TypeScript (`packages/orderbook`), run tests using `nix develop -c npm run test -w rainlanguage/orderbook`
Applied to files:
crates/common/src/raindex_client/take_orders/e2e_tests.rs
📚 Learning: 2025-06-04T10:21:01.388Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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:
crates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/test_helpers.rs
📚 Learning: 2025-06-16T10:49:47.770Z
Learnt from: thedavidmeister
Repo: rainlanguage/rain.orderbook PR: 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:
crates/common/src/raindex_client/take_orders/e2e_tests.rscrates/common/src/take_orders/candidates.rs
📚 Learning: 2025-05-13T20:06:22.602Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1713
File: crates/settings/src/remote/chains/mod.rs:43-226
Timestamp: 2025-05-13T20:06:22.602Z
Learning: When writing tests for collections of complex objects in Rust, prefer item-by-item comparison over direct vector comparison to get more specific error messages that pinpoint exactly which item and field has a mismatch.
Applied to files:
crates/common/src/take_orders/price.rs
📚 Learning: 2025-08-06T10:52:07.763Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2050
File: packages/webapp/src/lib/services/validateAmount.ts:37-38
Timestamp: 2025-08-06T10:52:07.763Z
Learning: In the rainlanguage/rain.orderbook project, Float comparison methods (like `.lte()`, `.gt()`) return objects with a `.value` property that can include `undefined`. Therefore, boolean coercion with `!!` is necessary when using these values to ensure proper boolean return types.
Applied to files:
crates/common/src/take_orders/price.rs
📚 Learning: 2025-06-07T05:19:46.330Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1872
File: packages/webapp/src/__tests__/handleVaultDeposit.test.ts:20-53
Timestamp: 2025-06-07T05:19:46.330Z
Learning: In the rain.orderbook codebase, simple wrapper/adapter functions that just delegate to other functions (like handleVaultDeposit) don't need extensive edge case testing for missing parameters or error handling - the current test coverage focusing on core functionality is sufficient.
Applied to files:
crates/common/src/take_orders/candidates.rs
📚 Learning: 2025-10-06T11:28:30.692Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql:6-7
Timestamp: 2025-10-06T11:28:30.692Z
Learning: In `crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql`, the orderbook_address is currently hardcoded to '0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB' because the system only supports a single orderbook at the moment. Multiorderbook logic is not yet implemented and will be added in the future.
Applied to files:
crates/common/src/take_orders/candidates.rs
📚 Learning: 2025-07-31T19:03:56.594Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2044
File: tauri-app/src/routes/orders/[chainId]-[orderbook]-[orderHash]/+page.svelte:76-78
Timestamp: 2025-07-31T19:03:56.594Z
Learning: In the rainlanguage/rain.orderbook project, when the Tauri app has issues preventing proper testing, the team prefers to defer Tauri-specific implementations to separate PRs rather than including untested code, especially for financial operations like withdrawals.
Applied to files:
crates/common/src/take_orders/candidates.rs
📚 Learning: 2025-04-04T11:25:21.518Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 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:
crates/common/src/take_orders/candidates.rs
📚 Learning: 2025-11-12T09:42:29.002Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2280
File: crates/common/src/local_db/sync.rs:191-194
Timestamp: 2025-11-12T09:42:29.002Z
Learning: In the rainlanguage/rain.orderbook codebase, `alloy::primitives::Address` has an `is_empty()` method that can be used to check for empty/zero addresses. The code `row.store_address.is_empty()` compiles successfully.
Applied to files:
crates/common/src/take_orders/candidates.rs
📚 Learning: 2025-08-01T07:44:53.910Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/raindex_filter_store.rs:319-336
Timestamp: 2025-08-01T07:44:53.910Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore (crates/js_api/src/filters/raindex_filter_store.rs), the team chose a simplified monolithic approach with hard-coded keys and default auto-save behavior over configurable stores. The update_vaults method intentionally auto-saves to both localStorage and URL after each update as the default behavior, following a design evolution from a previous configurable approach.
Applied to files:
crates/common/src/take_orders/candidates.rs
📚 Learning: 2025-03-31T13:57:59.660Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1474
File: crates/js_api/src/yaml/mod.rs:31-34
Timestamp: 2025-03-31T13:57:59.660Z
Learning: The OrderbookYaml constructor in crates/js_api/src/yaml/mod.rs does not need early YAML validation. The author prefers to validate YAML only when it's actually used rather than during initialization.
Applied to files:
crates/common/src/test_helpers.rs
📚 Learning: 2025-11-25T14:56:29.762Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2334
File: crates/settings/src/yaml/orderbook.rs:18-18
Timestamp: 2025-11-25T14:56:29.762Z
Learning: In crates/settings/src/yaml/orderbook.rs, the `accounts` section is optional in orderbook YAML. The `AccountCfg::parse_all_from_yaml` method returns an empty HashMap when the `accounts` section is missing rather than throwing a Field(Missing(...)) error, so it doesn't need to be wrapped with `to_yaml_string_missing_check` in the `to_yaml_string` method.
Applied to files:
crates/common/src/test_helpers.rs
📚 Learning: 2025-06-20T07:26:50.488Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1941
File: crates/settings/src/yaml/orderbook.rs:112-123
Timestamp: 2025-06-20T07:26:50.488Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers to keep the get_network_by_chain_id implementation simple with O(n) lookup rather than adding caching complexity for performance optimization, even if the method might be called frequently.
Applied to files:
crates/common/src/test_helpers.rs
🧬 Code graph analysis (8)
crates/common/src/raindex_client/take_orders/request.rs (2)
subgraph/src/float.ts (1)
Float(4-4)crates/common/src/withdraw.rs (1)
parse(70-71)
crates/common/src/raindex_client/take_orders/selection.rs (4)
crates/common/src/take_orders/candidates.rs (1)
build_take_order_candidates_for_pair(73-99)crates/common/src/take_orders/price.rs (1)
cmp_float(5-13)crates/common/src/take_orders/simulation.rs (1)
simulate_buy_over_candidates(99-131)crates/common/src/test_helpers.rs (1)
make_candidate(537-550)
crates/common/src/take_orders/simulation.rs (4)
crates/common/src/take_orders/price.rs (1)
cmp_float(5-13)crates/common/src/test_helpers.rs (1)
make_simulation_candidate(552-554)crates/common/src/raindex_client/take_orders/result.rs (1)
high_price_cap(80-82)crates/common/src/take_orders/config.rs (1)
high_price_cap(83-85)
crates/common/src/raindex_client/take_orders/mod.rs (4)
crates/common/src/raindex_client/take_orders/request.rs (1)
parse_request(16-43)crates/common/src/rpc_client.rs (2)
rpc_urls(106-108)new_with_urls(69-82)crates/common/src/raindex_client/take_orders/selection.rs (2)
build_candidates_for_chain(11-24)select_best_orderbook_simulation(42-92)crates/common/src/raindex_client/take_orders/result.rs (1)
build_calldata_result(30-70)
crates/common/src/raindex_client/take_orders/result.rs (5)
crates/common/src/raindex_client/take_orders/selection.rs (2)
select_best_orderbook_simulation(42-92)high_price_cap(99-101)crates/common/src/take_orders/config.rs (2)
build_take_orders_config_from_buy_simulation(25-73)high_price_cap(83-85)crates/common/src/test_helpers.rs (1)
make_candidate(537-550)crates/common/src/raindex_client/take_orders/e2e_tests.rs (1)
high_price_cap(30-32)crates/common/src/take_orders/simulation.rs (1)
high_price_cap(140-142)
crates/common/src/raindex_client/take_orders/e2e_tests.rs (5)
crates/common/src/raindex_client/mod.rs (1)
get_test_yaml(529-593)crates/common/src/raindex_client/take_orders/result.rs (1)
high_price_cap(80-82)crates/common/src/raindex_client/take_orders/selection.rs (1)
high_price_cap(99-101)crates/common/src/take_orders/config.rs (1)
high_price_cap(83-85)crates/common/src/take_orders/simulation.rs (1)
high_price_cap(140-142)
crates/common/src/take_orders/price.rs (1)
subgraph/src/float.ts (1)
Float(4-4)
crates/common/src/test_helpers.rs (3)
crates/test_fixtures/src/lib.rs (1)
add_order(277-303)crates/common/src/add_order.rs (1)
new_from_deployment(98-137)crates/settings/src/spec_version.rs (1)
current(11-13)
⏰ 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: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-browser-test)
- 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-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Deploy-Preview-Push
- GitHub Check: Deploy-Docs-Preview
- GitHub Check: build-tauri (ubuntu-22.04, true)
| #[error("No liquidity available for the requested trade")] | ||
| NoLiquidity, | ||
| #[error("Insufficient liquidity: requested {requested:?}, available {available:?}")] | ||
| InsufficientLiquidity { | ||
| requested: rain_math_float::Float, | ||
| available: rain_math_float::Float, | ||
| }, | ||
| #[error("Buy amount must be greater than zero")] | ||
| NonPositiveBuyAmount, | ||
| #[error("Price cap cannot be negative")] | ||
| NegativePriceCap, | ||
| #[error(transparent)] | ||
| RpcClientError(#[from] RpcClientError), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
New error variants correctly model take-orders failure modes.
The variants cover the key error scenarios: no liquidity, insufficient liquidity, input validation failures, and RPC errors.
One minor observation: InsufficientLiquidity uses {:?} (Debug format) for the Float values. If Float implements Display, consider using {} for more user-friendly output in both the error message and readable message.
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/mod.rs around lines 332 to 344, the
InsufficientLiquidity error message currently formats Float fields with Debug
({:?}); replace the Debug specifiers with Display ({}) so the error text uses
the Float Display implementation for more user-friendly output (update both the
#[error(...)] string and any related human-readable messages that reference
requested/available to use {} instead of {:?}).
| pub(crate) fn parse_request( | ||
| sell_token: &str, | ||
| buy_token: &str, | ||
| buy_amount: &str, | ||
| price_cap: &str, | ||
| min_receive_mode: MinReceiveMode, | ||
| ) -> Result<ParsedTakeOrdersRequest, RaindexError> { | ||
| let sell_token = Address::from_str(sell_token)?; | ||
| let buy_token = Address::from_str(buy_token)?; | ||
| let buy_amount = Float::parse(buy_amount.to_string())?; | ||
| let price_cap = Float::parse(price_cap.to_string())?; | ||
|
|
||
| let zero = Float::zero()?; | ||
| if buy_amount.lte(zero)? { | ||
| return Err(RaindexError::NonPositiveBuyAmount); | ||
| } | ||
| if price_cap.lt(zero)? { | ||
| return Err(RaindexError::NegativePriceCap); | ||
| } | ||
|
|
||
| Ok(ParsedTakeOrdersRequest { | ||
| sell_token, | ||
| buy_token, | ||
| buy_amount, | ||
| price_cap, | ||
| min_receive_mode, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider validating that sell_token and buy_token are different.
The function validates numeric constraints but doesn't check whether sell_token == buy_token. Trading a token for itself is logically invalid and could cause unexpected behavior in downstream candidate selection.
if price_cap.lt(zero)? {
return Err(RaindexError::NegativePriceCap);
}
+ if sell_token == buy_token {
+ return Err(RaindexError::SameTokenPair);
+ }
Ok(ParsedTakeOrdersRequest {This would require adding a new SameTokenPair variant to RaindexError.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/take_orders/request.rs around lines 16 to
43, the parser currently doesn't reject requests where sell_token == buy_token;
add a check after parsing both addresses to compare them and return a new
RaindexError::SameTokenPair when they are equal. Update the RaindexError enum
(in its defining file) to include a SameTokenPair variant and ensure any places
matching RaindexError handle the new variant or fall through, and add a unit
test asserting parse_request returns SameTokenPair for identical token inputs.
| fn matches_direction( | ||
| order: &OrderV4, | ||
| input_index: u32, | ||
| output_index: u32, | ||
| input_token: Address, | ||
| output_token: Address, | ||
| ) -> bool { | ||
| let order_input_token = order.validInputs[input_index as usize].token; | ||
| let order_output_token = order.validOutputs[output_index as usize].token; | ||
| order_input_token == input_token && order_output_token == output_token | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider adding a safety doc comment or debug assertion.
This function performs unchecked array access, relying on callers to validate bounds first via indices_in_bounds. While try_build_candidate does call indices_in_bounds before this function, independent use could panic.
Consider adding documentation or a debug assertion:
+/// # Safety
+/// Caller must ensure indices are in bounds (use `indices_in_bounds` first).
fn matches_direction(
order: &OrderV4,
input_index: u32,
output_index: u32,
input_token: Address,
output_token: Address,
) -> bool {
+ debug_assert!(indices_in_bounds(order, input_index, output_index));
let order_input_token = order.validInputs[input_index as usize].token;🤖 Prompt for AI Agents
In crates/common/src/take_orders/candidates.rs around lines 18 to 28, the
function matches_direction does unchecked indexing into order.validInputs and
order.validOutputs which can panic if called with out-of-range indices; add a
short safety doc comment stating the function expects pre-validated indices AND
add a debug assertion verifying input_index and output_index are within bounds
(compare converted usize to the slices' lengths) before performing the accesses
so debug builds will catch misuse while release performance is unaffected.
| fn filter_candidates_by_price_cap( | ||
| candidates: Vec<TakeOrderCandidate>, | ||
| price_cap: Float, | ||
| ) -> Result<Vec<TakeOrderCandidate>, RaindexError> { | ||
| let mut filtered = Vec::new(); | ||
| for candidate in candidates { | ||
| if candidate.ratio.lte(price_cap)? { | ||
| filtered.push(candidate); | ||
| } | ||
| } | ||
| Ok(filtered) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider using iterator pattern for idiomatic Rust.
The explicit loop works correctly, but could be more idiomatic:
fn filter_candidates_by_price_cap(
candidates: Vec<TakeOrderCandidate>,
price_cap: Float,
) -> Result<Vec<TakeOrderCandidate>, RaindexError> {
- let mut filtered = Vec::new();
- for candidate in candidates {
- if candidate.ratio.lte(price_cap)? {
- filtered.push(candidate);
- }
- }
- Ok(filtered)
+ candidates
+ .into_iter()
+ .filter_map(|c| match c.ratio.lte(price_cap) {
+ Ok(true) => Some(Ok(c)),
+ Ok(false) => None,
+ Err(e) => Some(Err(e)),
+ })
+ .collect()
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/common/src/take_orders/simulation.rs around lines 86 to 97, replace
the explicit for-loop with an iterator-based approach: convert candidates into
an iterator (into_iter), map each candidate to a Result that checks
candidate.ratio.lte(price_cap) and yields Ok(Some(candidate)) when true,
Ok(None) when false, or Err(...) when the lte check fails, then collect that
iterator into a Result<Vec<TakeOrderCandidate>, RaindexError> (collect will
short-circuit on the first Err). This yields idiomatic code without changing
error handling semantics.
| #[cfg(test)] | ||
| #[cfg(not(target_family = "wasm"))] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::test_helpers::candidates::make_simulation_candidate; | ||
| use rain_math_float::Float; | ||
|
|
||
| fn high_price_cap() -> Float { | ||
| Float::parse("1000000".to_string()).unwrap() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Note: high_price_cap() is duplicated across multiple test modules.
This helper function appears in at least 4 locations: simulation.rs, config.rs, result.rs, and selection.rs tests. Consider extracting to a shared test helper module to reduce duplication.
🤖 Prompt for AI Agents
In crates/common/src/take_orders/simulation.rs around lines 133-142 the helper
fn high_price_cap() is duplicated across multiple test modules; extract this
function into a shared test helper (e.g., crates/common/tests/helpers.rs or
crates/common/src/test_helpers/mod.rs) as pub(crate) fn high_price_cap() ->
Float { Float::parse("1000000".to_string()).unwrap() }, update the test modules
(simulation.rs, config.rs, result.rs, selection.rs) to import and call the
shared helper (use crate::test_helpers::high_price_cap or appropriate path), and
remove the duplicated definitions in each file so tests compile with the single
shared implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@findolor, understood! I'll leave the test helper as-is.
🧠 Learnings used
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1975
File: crates/js_api/src/gui/state_management.rs:412-412
Timestamp: 2025-07-09T14:00:12.206Z
Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1983
File: crates/js_api/src/gui/mod.rs:964-1199
Timestamp: 2025-07-11T06:40:49.511Z
Learning: In crates/js_api/src/gui/mod.rs tests, findolor prefers to keep large hardcoded YAML strings (like the validation test YAML) inline in the test file rather than extracting them to separate external files for maintainability purposes.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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: findolor
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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.
| pub async fn deposit_to_orderbook( | ||
| setup: &MultiOrderbookTestSetup, | ||
| orderbook: Address, | ||
| token: Address, | ||
| amount: U256, | ||
| vault_id: B256, | ||
| ) { | ||
| use rain_orderbook_test_fixtures::Orderbook; | ||
|
|
||
| let token_contract = setup | ||
| .local_evm | ||
| .tokens | ||
| .iter() | ||
| .find(|t| *t.address() == token) | ||
| .expect("Token should exist"); | ||
|
|
||
| token_contract | ||
| .approve(orderbook, amount) | ||
| .from(setup.owner) | ||
| .send() | ||
| .await | ||
| .unwrap() | ||
| .get_receipt() | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| let orderbook_instance = Orderbook::new(orderbook, setup.local_evm.provider.clone()); | ||
| let raw_amount = Float::from_fixed_decimal(amount, 18).unwrap().get_inner(); | ||
|
|
||
| orderbook_instance | ||
| .deposit3(token, vault_id, raw_amount, vec![]) | ||
| .from(setup.owner) | ||
| .send() | ||
| .await | ||
| .unwrap() | ||
| .get_receipt() | ||
| .await | ||
| .unwrap(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Hardcoded 18 decimals may cause issues with non-18-decimal tokens.
Line 356 uses Float::from_fixed_decimal(amount, 18) which hardcodes the decimal places. If this helper is used with tokens having different decimals (e.g., USDC with 6 decimals), the deposit amount will be incorrect.
Consider parameterizing decimals or deriving from the token:
pub async fn deposit_to_orderbook(
setup: &MultiOrderbookTestSetup,
orderbook: Address,
token: Address,
amount: U256,
vault_id: B256,
+ decimals: u8,
) {
...
- let raw_amount = Float::from_fixed_decimal(amount, 18).unwrap().get_inner();
+ let raw_amount = Float::from_fixed_decimal(amount, decimals).unwrap().get_inner();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/common/src/test_helpers.rs around lines 329 to 367, the helper
hardcodes 18 decimals when converting the U256 amount
(Float::from_fixed_decimal(amount, 18)), which will mis-handle tokens with
non-18 decimals; update the function to either accept a decimals parameter or
query the token's decimals() from the token_contract and use that value when
calling Float::from_fixed_decimal so the raw_amount is computed according to the
token's actual decimals before calling orderbook_instance.deposit3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is test code it's fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@findolor, understood! I've noted that hardcoded values in test helpers are acceptable for this codebase.
✏️ Learnings added
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2360
File: crates/common/src/test_helpers.rs:329-367
Timestamp: 2025-12-17T13:17:47.475Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor considers hardcoded values (like decimals) acceptable in test helper functions such as deposit_to_orderbook in crates/common/src/test_helpers.rs, as test infrastructure doesn't need the same level of parameterization as production code.
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: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1859
File: crates/quote/src/quote_debug.rs:472-492
Timestamp: 2025-05-20T10:20:08.206Z
Learning: In the Rain Orderbook codebase, the `#[tokio::test(flavor = "multi_thread")]` annotation is specifically needed for tests that use `LocalEvm`, not just for consistency across all async tests.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 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: brusherru
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1911
File: crates/common/src/erc20.rs:336-350
Timestamp: 2025-07-21T16:33:12.129Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers both LocalEvm-based tests and mocked tests to be "unit tests" - they don't distinguish between them as "unit" vs "integration" tests based on their implementation approach.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using `#[cfg(target_family = "wasm")]` and `#[cfg(not(target_family = "wasm"))]`, and don't require additional `wasm_bindgen_test_configure!(run_in_browser)` directives.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like `#[cfg(target_family = "wasm")]` should not be added to imports or implementations that may be needed for tests.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1872
File: packages/webapp/src/__tests__/handleVaultDeposit.test.ts:20-53
Timestamp: 2025-06-07T05:19:46.330Z
Learning: In the rain.orderbook codebase, simple wrapper/adapter functions that just delegate to other functions (like handleVaultDeposit) don't need extensive edge case testing for missing parameters or error handling - the current test coverage focusing on core functionality is sufficient.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: packages/orderbook/test/js_api/filters.test.ts:19-36
Timestamp: 2025-08-02T03:55:25.215Z
Learning: In the rainlanguage/rain.orderbook project's WASM tests, the pattern of chaining `.value!` calls on WASM result types (like from VaultsFilterBuilder methods) is the established and preferred approach for handling WASM results, and should not be refactored into intermediate variables as it would add unnecessary verbosity without improving the code.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2001
File: crates/common/src/raindex_client/order_quotes.rs:62-69
Timestamp: 2025-07-16T14:33:13.457Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor considers hardcoded decimal values (18 and 36) in order quote formatting logic to be acceptable for their use case, even when dynamic token decimals could theoretically provide more accurate formatting for different tokens.
Learnt from: thedavidmeister
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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: thedavidmeister
Repo: rainlanguage/rain.orderbook PR: 2149
File: src/concrete/arb/RouteProcessorOrderBookV5ArbOrderTaker.sol:37-38
Timestamp: 2025-09-16T07:37:53.477Z
Learning: In the rainlanguage/rain.orderbook codebase, user thedavidmeister considers max approval (type(uint256).max) to be acceptable in RouteProcessorOrderBookV5ArbOrderTaker, even when using lossy conversions that might leave dust tokens. The max approval pattern is preferred over exact amount approval in this context.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1706
File: crates/quote/src/js_api/mod.rs:57-61
Timestamp: 2025-04-29T06:19:15.446Z
Learning: In the rain.orderbook codebase, the `get_id` function is designed to use `U256::from_str()` rather than `U256::from_hex()` for parsing order hashes, despite the potential for hex string inputs.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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'.
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1705
File: packages/webapp/src/routes/deploy/page.test.ts:80-81
Timestamp: 2025-04-28T15:38:30.561Z
Learning: For this codebase, explicit `any` type assertions are considered acceptable in test files, as confirmed by the repository owner.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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: findolor
Repo: rainlanguage/rain.orderbook PR: 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.
Chained PR
Motivation
See issues:
Solution
minimumInput,maximumInputandmaximumIORatiovalues based on input parametersChecks
By submitting this for review, I'm confirming I've done the following:
fix #2357
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.