Implement wasm macro to js api subgraph functions and add tests by findolor · Pull Request #1744 · rainlanguage/raindex

findolor · 2025-05-06T18:24:57Z

Motivation

The sdk functions for subgraph were using the old wasm bindgen logic and they were missing rust tests. This PR adds both of these things.

See issue: #1634

Solution

Use the new wasm macro for all of the functions.
Update UI with the latest changes.
Add and update existing 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 #1634

Summary by CodeRabbit

New Features
- Added new API functions for fetching trade data, including trade lists, trade details, and trade counts, with improved error handling and TypeScript bindings.
- Introduced strongly typed result wrappers for all major API functions, replacing generic return types for better type safety and WASM interop.
Refactor
- Unified and simplified error handling across the app and UI, now returning results in a { value, error } structure instead of throwing exceptions.
- Consolidated multiple calldata types into a single VaultCalldataResult type for deposit, withdraw, and approval operations.
- Updated all UI and test code to use the new result structure and types, ensuring consistent error and data handling.
Bug Fixes
- Improved error propagation and display in UI components, ensuring users receive clear feedback when API errors occur.
Tests
- Expanded and improved test coverage for all API functions, including comprehensive integration and unit tests for order, vault, transaction, and trade data.
Chores
- Updated internal dependencies and TypeScript bindings for improved maintainability and cross-platform compatibility.

coderabbitai · 2025-05-06T18:25:04Z

Walkthrough

This change refactors the JS API crate and its consumers to return strongly typed Rust structs wrapped in a unified result type, replacing generic JsValue results and exception-based error handling. It unifies error types, introduces explicit error property checking in TypeScript/JavaScript consumers, consolidates calldata types, and adds comprehensive Rust and JS/TS tests for subgraph APIs.

Changes

File(s)	Change Summary
`crates/js_api/src/subgraph/order.rs`, `crates/js_api/src/subgraph/add_order.rs`, `crates/js_api/src/subgraph/remove_order.rs`, `crates/js_api/src/subgraph/transaction.rs`, `crates/js_api/src/subgraph/vault.rs`	Refactored all subgraph-related API functions to return strongly typed Rust structs in a unified result wrapper, replacing `JsValue` and exception-based error handling. Unified error types to `SubgraphError`. Replaced `wasm_bindgen` with `wasm_export`. Added/updated wrapper result structs, removed redundant serialization, and added comprehensive async tests using HTTP mocks. Consolidated calldata types in vault APIs.
`crates/js_api/src/subgraph/mod.rs`, `crates/js_api/src/subgraph/trades.rs`	Added new `trades` submodule with trade-related API functions and result wrappers. Extended `SubgraphError` with `UrlParseError` and conversion to `WasmEncodedError`.
`crates/js_api/Cargo.toml`	Added `url` crate with `serde` feature and `serde_json` as dev-dependency.
`crates/js_api/src/lib.rs`	Changed conditional compilation for `subgraph` module to include test builds.
`crates/subgraph/src/multi_orderbook_client.rs`, `crates/subgraph/src/pagination.rs`, `crates/subgraph/src/types/common.rs`	Made wasm trait derivations and implementations unconditional for various argument and result structs, and made fields in `MultiSubgraphArgs` public.
`crates/subgraph/src/orderbook_client.rs`	Made `From<OrderbookSubgraphClientError> for JsValue` unconditional and updated imports for error types.
`packages/orderbook/test/js_api/*.test.ts`	Refactored all JS API tests to check for explicit `error` property on result objects instead of relying on exceptions. Updated assertions to use `value` property.
`packages/ui-components/src/__tests__/*.test.ts`	Updated all mocks and assertions to expect API results as `{ value: ... }` or `{ error: ... }` objects, matching new result structure.
`packages/ui-components/src/lib/components/*/.svelte`	Updated all query functions to explicitly check for `error` property in API results, throw on error, and return `value` property. Simplified type imports for vault balance changes.
`packages/ui-components/src/lib/components/CodeMirrorRainlang.svelte`	Refactored to handle explicit error property from `extendOrder` result, displaying error message in template if present.
`packages/ui-components/src/lib/services/awaitTransactionIndexing.ts`	Updated to expect `WasmEncodedResult` wrapper for all subgraph fetches, adjusting polling and result extraction logic.
`packages/ui-components/src/lib/stores/transactionStore.ts`	Consolidated calldata types to use `VaultCalldataResult` in transaction argument types.
`packages/webapp/src/__tests__/DepositOrWithdrawModal.test.ts`, `packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte`	Updated mocks, assertions, and logic to use unified `VaultCalldataResult` type and explicit error property checks for calldata APIs.

Sequence Diagram(s)

sequenceDiagram
    participant JS/TS UI
    participant WASM API
    participant Rust Subgraph Client

    JS/TS UI->>WASM API: call getOrderByHash(args)
    WASM API->>Rust Subgraph Client: fetch order by hash
    Rust Subgraph Client-->>WASM API: SgOrder or SubgraphError
    WASM API-->>JS/TS UI: { value: SgOrder } or { error: SubgraphError }

    Note over JS/TS UI: UI checks for 'error' property, throws or uses 'value'

sequenceDiagram
    participant JS/TS UI
    participant WASM API

    JS/TS UI->>WASM API: call getVaultDepositCalldata(vault, amount)
    WASM API-->>JS/TS UI: { value: VaultCalldataResult } or { error: SubgraphError }

    Note over JS/TS UI: UI checks for 'error', uses 'value' if no error

Assessment against linked issues

Objective	Addressed	Explanation
Add missing tests for JS API crate (#1634)	✅

Possibly related PRs

test common::subgraph #1742: Both PRs modify subgraph client instantiation and related API call sites, indicating a direct code-level connection.
Move common js_api file to main js_api directory and add tests #1715: Both PRs introduce and test strongly typed WASM-exported functions for order-related calldata and subgraph interactions.
Fix transaction store and subgraph await #1670: Both PRs refactor subgraph API function signatures, error types, and their usage in transaction indexing logic.

Suggested labels

js/wasm

Suggested reviewers

hardyjosh

📜 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 60672d7 and 792b061.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

crates/js_api/src/lib.rs (1 hunks)
crates/subgraph/src/types/common.rs (4 hunks)

🧰 Additional context used

🧠 Learnings (1)

crates/js_api/src/lib.rs (2)

Learnt from: findolor
PR: rainlanguage/rain.orderbook#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
PR: rainlanguage/rain.orderbook#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.

⏰ Context from checks skipped due to timeout of 90000ms (14)

GitHub Check: test
GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
GitHub Check: test
GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
GitHub Check: Deploy-Preview
GitHub Check: build-tauri (ubuntu-22.04, true)
GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)

🔇 Additional comments (5)

crates/js_api/src/lib.rs (1)

7-8: Enable subgraph module during tests
Broadening the cfg to any(target_family = "wasm", test) ensures that the subgraph APIs are compiled in non-WASM test targets, which is necessary for the newly added Rust tests to import and exercise this module. This aligns with the project’s approach of making WASM dependencies available in tests without adding extra conditional guards.

crates/subgraph/src/types/common.rs (4)

20-20: LGTM: Aligns with migration from conditional to unconditional wasm trait implementations.

This change improves the codebase by moving the wasm trait implementation for SgOrdersListFilterArgs out of the conditional compilation block, making it consistent with the new pattern being implemented throughout the codebase.

110-110: LGTM: Migrating wasm trait implementation for better type safety.

Moving the wasm trait implementation for SgOrderWithSubgraphName out of the conditional module allows for more consistent usage in the JS API refactoring. This supports the goal of returning strongly typed Rust structs instead of generic JsValue results.

135-135: LGTM: Consistent approach to wasm trait implementation.

Making the SgVaultsListFilterArgs wasm trait implementation unconditional aligns with the pattern being established in this PR. This change simplifies the code structure and ensures these traits are consistently available.

356-356: LGTM: Improving type safety for JS API interactions.

Adding an unconditional wasm trait implementation for SgAddOrderWithOrder supports the PR's goal of replacing generic JsValue with strongly typed Rust structs in the JS API. This change enables more direct and safer use of this struct in the JS API functions.

✨ Finishing Touches

📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 24

🔭 Outside diff range comments (7)

packages/orderbook/test/js_api/order.test.ts (1)
693-724: 🧹 Nitpick (assertive)

Updated order fetching by hash

The tests for fetching orders by hash are correctly updated to handle the new result wrapper pattern.

Consider using template literals in both instances:
-const result = await getOrderByHash(mockServer.url + '/sg1', order1.orderHash);
+const result = await getOrderByHash(`${mockServer.url}/sg1`, order1.orderHash);
🧰 Tools

🪛 Biome (1.9.4)

[error] 693-693: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

[error] 722-722: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)
packages/orderbook/test/js_api/vault.test.ts (1)
85-107: ⚠️ Potential issue

Test assumes a deterministic ordering that the implementation may not guarantee

getVaults aggregates results coming from two different subgraph calls executed in parallel.
Unless the underlying implementation explicitly preserves the original subgraphs array order, vaults[0] could refer to either network.

Using find or Array.prototype.some instead of relying on index positions will make the test robust:
-assert.equal(vaults[0].vault.id, vault1.id);
-assert.equal(vaults[0].subgraphName, 'network-one');
-assert.equal(vaults[1].vault.id, vault2.id);
-assert.equal(vaults[1].subgraphName, 'network-two');
+const one = vaults.find(v => v.subgraphName === 'network-one')!;
+const two = vaults.find(v => v.subgraphName === 'network-two')!;
+assert.strictEqual(one.vault.id, vault1.id);
+assert.strictEqual(two.vault.id, vault2.id);
This avoids sporadic test failures when execution order changes.

🧰 Tools

🪛 Biome (1.9.4)

[error] 88-88: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

[error] 89-89: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)
packages/ui-components/src/lib/services/awaitTransactionIndexing.ts (1)
126-147: 🛠️ Refactor suggestion

Polling loop ignores explicit error responses – may hang until timeout

If the subgraph replies with { error: … }, the current logic just treats it as
“not successful” and continues polling until maxAttempts, potentially hiding
real problems and wasting time.

A minimal fix:
-const data = await fetchData(subgraphUrl, txHash);
-if (data.value && isSuccess(data.value)) {
+const data = await fetchData(subgraphUrl, txHash);
+if (data.error) {
+  clearInterval(checkInterval);
+  resolve({ error: data.error.msg ?? data.error.toString() });
+  return;
+}
+if (data.value && isSuccess(data.value)) {
That way callers get immediate feedback instead of a generic timeout.
crates/subgraph/src/types/common.rs (1)

169-188: ⚠️ Potential issue

Duplicate impl_wasm_traits! risk

impl_wasm_traits!(SgVault); appears here, while a second invocation for the same type exists in the impls module gated to wasm (lines 691-707).
If the outer call is now unconditional, compiling with --target wasm32-unknown-unknown will produce duplicate trait impl errors.

Please remove one of the two invocations or guard them consistently.
crates/js_api/src/subgraph/order.rs (1)
46-77: 🧹 Nitpick (assertive)

Use BTreeMap to guarantee deterministic vault ordering in tests & WASM output

sort_vaults uses a HashMap; key iteration order is non-deterministic, which:

Makes serialised JSON order unstable

Can break snapshot / golden tests

Causes flickering UI diffs when iterating keys in JS

Replacing with BTreeMap<String, Vec<SgVault>> preserves insertion order by key and incurs negligible overhead for the three keys involved.
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeMap, HashSet};

-fn sort_vaults(order: &SgOrder) -> HashMap<String, Vec<SgVault>> {
-    let mut sorted_vaults: HashMap<String, Vec<SgVault>> = HashMap::new();
+fn sort_vaults(order: &SgOrder) -> BTreeMap<String, Vec<SgVault>> {
+    let mut sorted_vaults: BTreeMap<String, Vec<SgVault>> = BTreeMap::new();
crates/js_api/src/subgraph/vault.rs (2)
139-148: ⚠️ Potential issue

Allowance comparison should be >=, not >

If allowance == deposit_amount, no approval is needed, yet the current logic will generate approval calldata because the > guard passes.
-    if allowance > deposit_amount {
+    if allowance >= deposit_amount {
         return Err(SubgraphError::InvalidAmount);
     }
Adjust the accompanying tests that expect an error for the equality case instead of the “less than” case.

170-176: 🧹 Nitpick (assertive)

Early-exit on negative / malformed amounts

U256::from_str will happily parse negative-looking strings like “-1” as Err, but the error message bubbles up from U256 and may confuse JS callers.

Wrap with a domain-specific error:
let amount = U256::from_str(amount).map_err(|_| SubgraphError::InvalidAmount)?;
This keeps the error surface uniform.

♻️ Duplicate comments (1)

packages/ui-components/src/lib/services/awaitTransactionIndexing.ts (1)
199-200: Keep TransactionConfig.fetchData consistent with awaitSubgraphIndexing

See comment above – duplicate but still present here. Defining a single helper
type such as
export type Encoded<T> = WasmEncodedResult<T | null | undefined>;
and re-using it in both places avoids future drift.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c9827d and bec5259.

⛔ Files ignored due to path filters (2)

Cargo.lock is excluded by !**/*.lock
tauri-app/src-tauri/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (42)

Cargo.toml (1 hunks)
crates/js_api/Cargo.toml (1 hunks)
crates/js_api/src/lib.rs (1 hunks)
crates/js_api/src/subgraph/add_order.rs (1 hunks)
crates/js_api/src/subgraph/mod.rs (2 hunks)
crates/js_api/src/subgraph/order.rs (3 hunks)
crates/js_api/src/subgraph/remove_order.rs (1 hunks)
crates/js_api/src/subgraph/trades.rs (1 hunks)
crates/js_api/src/subgraph/transaction.rs (1 hunks)
crates/js_api/src/subgraph/vault.rs (5 hunks)
crates/subgraph/Cargo.toml (0 hunks)
crates/subgraph/src/multi_orderbook_client.rs (1 hunks)
crates/subgraph/src/orderbook_client.rs (2 hunks)
crates/subgraph/src/pagination.rs (1 hunks)
crates/subgraph/src/types/common.rs (8 hunks)
packages/orderbook/test/js_api/addOrder.test.ts (1 hunks)
packages/orderbook/test/js_api/order.test.ts (7 hunks)
packages/orderbook/test/js_api/removeOrder.test.ts (1 hunks)
packages/orderbook/test/js_api/transaction.test.ts (1 hunks)
packages/orderbook/test/js_api/vault.test.ts (8 hunks)
packages/ui-components/src/__tests__/OrderAPY.test.ts (1 hunks)
packages/ui-components/src/__tests__/OrderDetail.test.ts (3 hunks)
packages/ui-components/src/__tests__/OrderVaultsVolTable.test.ts (1 hunks)
packages/ui-components/src/__tests__/VaultBalanceChangesTable.test.ts (3 hunks)
packages/ui-components/src/__tests__/VaultDetail.test.ts (2 hunks)
packages/ui-components/src/__tests__/awaitTransactionIndexing.test.ts (5 hunks)
packages/ui-components/src/__tests__/transactionStore.test.ts (2 hunks)
packages/ui-components/src/lib/components/CodeMirrorRainlang.svelte (2 hunks)
packages/ui-components/src/lib/components/charts/OrderTradesChart.svelte (1 hunks)
packages/ui-components/src/lib/components/charts/VaultBalanceChart.svelte (2 hunks)
packages/ui-components/src/lib/components/detail/OrderDetail.svelte (1 hunks)
packages/ui-components/src/lib/components/detail/VaultDetail.svelte (1 hunks)
packages/ui-components/src/lib/components/tables/OrderAPY.svelte (2 hunks)
packages/ui-components/src/lib/components/tables/OrderTradesListTable.svelte (2 hunks)
packages/ui-components/src/lib/components/tables/OrderVaultsVolTable.svelte (1 hunks)
packages/ui-components/src/lib/components/tables/OrdersListTable.svelte (2 hunks)
packages/ui-components/src/lib/components/tables/VaultBalanceChangesTable.svelte (3 hunks)
packages/ui-components/src/lib/components/tables/VaultsListTable.svelte (1 hunks)
packages/ui-components/src/lib/services/awaitTransactionIndexing.ts (5 hunks)
packages/ui-components/src/lib/stores/transactionStore.ts (2 hunks)
packages/webapp/src/__tests__/DepositOrWithdrawModal.test.ts (5 hunks)
packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte (3 hunks)

💤 Files with no reviewable changes (1)

crates/subgraph/Cargo.toml

🧰 Additional context used

🧬 Code Graph Analysis (2)

packages/ui-components/src/__tests__/OrderDetail.test.ts (1)

packages/ui-components/src/lib/__fixtures__/orderDetail.ts (1)

mockOrder (68-210)

packages/ui-components/src/lib/services/awaitTransactionIndexing.ts (1)

tauri-app/src/lib/stores/settings.ts (2)

subgraphUrl (83-87)

orderHash (268-273)

🪛 Biome (1.9.4)

packages/orderbook/test/js_api/addOrder.test.ts

[error] 371-371: Template literals are preferred over string concatenation.