Improve the vault id related sdk functions#1985
Conversation
|
""" WalkthroughThe codebase was refactored to change how vault IDs are managed throughout the SDK and related UI/test layers. Vault IDs are now keyed by token strings and vault type enums instead of positional indices and booleans. This affects method signatures, internal data structures, and all related usage in tests and UI components, aligning all vault ID handling to a token-keyed approach. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant JSAPI as DotrainOrderGui
participant Settings as OrderCfg
UI->>JSAPI: setVaultId(VaultType, token, vaultId)
JSAPI->>Settings: update_vault_id(VaultType, token, vaultId)
Settings-->>JSAPI: Result
JSAPI-->>UI: Result
UI->>JSAPI: getVaultIds()
JSAPI->>Settings: parse_vault_ids(...)
Settings-->>JSAPI: Map<VaultType, Map<token, vaultId>>
JSAPI-->>UI: Map<VaultType, Map<token, vaultId>>
Estimated code review effort3 (~45 minutes) Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
||
| if let Some(idx) = item_index { | ||
| if let Some(item) = vec.get_mut(idx) { | ||
| if let StrictYaml::Hash(ref mut item_hash) = item { |
There was a problem hiding this comment.
something like item_map would be a clearer name for the variable. item_hash is a bit confusing
| @@ -398,8 +436,8 @@ impl OrderCfg { | |||
| documents: Vec<Arc<RwLock<StrictYaml>>>, | |||
| order_key: &str, | |||
| is_input: bool, | |||
There was a problem hiding this comment.
should we use the new enum here too?
There was a problem hiding this comment.
you are right updating it
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
packages/orderbook/test/js_api/gui.test.ts(4 hunks)packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte(1 hunks)packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts(3 hunks)
🧠 Learnings (4)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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
PR: rainlanguage/rain.orderbook#1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22: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: 0xgleb
PR: rainlanguage/rain.orderbook#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: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:34:42.328Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must use the format:
TOTAL:number
ADDITIONS:number
DELETIONS:number
with no extra text, formatting blocks, or explanations, and must still exclude lock files and reassess after each new commit.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: packages/ui-components/src/lib/components/ButtonVaultLink.svelte:18-18
Timestamp: 2025-07-04T09:13:39.020Z
Learning: In the RaindexVault type, the vaultId field is a bigint, so bigintToHex should be used instead of bigintStringToHex when converting vault IDs to hex format in UI components.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1870
File: packages/webapp/src/lib/components/WithdrawModal.svelte:31-31
Timestamp: 2025-05-20T12:03:18.032Z
Learning: The VaultActionArgs type no longer contains an action property after refactoring the deposit/withdraw modals into separate components.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1872
File: packages/webapp/src/lib/services/handleVaultWithdraw.ts:74-76
Timestamp: 2025-06-07T05:18:31.404Z
Learning: In vault withdrawal error handling, prefer user-friendly generic error messages over exposing technical error details from getVaultWithdrawCalldata, as the underlying errors are not end user friendly.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1882
File: packages/webapp/src/lib/services/handleVaultDeposit.ts:96-125
Timestamp: 2025-06-07T09:07:53.733Z
Learning: In the vault deposit flow, vault.token.address is already validated before reaching the approval transaction creation, so casting it to Hex type is safe and doesn't require additional validation.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1925
File: crates/settings/src/remote_tokens.rs:44-62
Timestamp: 2025-07-11T08:45:43.961Z
Learning: findolor prefers to avoid performance micro-optimizations like changing `.values()` to `.into_values()` in HashMap iterations in the rainlanguage/rain.orderbook codebase, even when they would eliminate unnecessary cloning.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1903
File: crates/js_api/src/subgraph/vault.rs:129-138
Timestamp: 2025-06-17T16:33:58.549Z
Learning: findolor prefers downstream error handling over early validation checks for empty RPC lists in vault functions, allowing errors to bubble up from provider creation logic naturally.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1916
File: packages/webapp/src/routes/+layout.ts:26-26
Timestamp: 2025-06-10T12:01:07.418Z
Learning: The user findolor prefers to keep string concatenation using the plus operator over template literals in the rainlanguage/rain.orderbook project, even when static analysis tools suggest the change.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1975
File: crates/js_api/src/config.rs:44-45
Timestamp: 2025-07-09T07:42:48.458Z
Learning: User findolor prefers to defer code improvements and refactoring suggestions to future PRs when they are not directly related to the current PR's scope, even when the suggestions are valid improvements.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1947
File: crates/common/src/raindex_client/orders.rs:462-1643
Timestamp: 2025-06-24T08:45:10.971Z
Learning: User findolor prefers to keep tests simple and avoid refactoring for maintainability when the code is stable and viewed as "one time thing" that won't require frequent changes.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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.
packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte (12)
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/errors/DeploymentStepsError.ts:16-16
Timestamp: 2025-07-10T12:52:47.468Z
Learning: In packages/ui-components/src/lib/errors/DeploymentStepsError.ts, the error codes NO_SELECT_TOKENS ('Error loading tokens') and NO_AVAILABLE_TOKENS ('Error loading available tokens') represent different failure scenarios in the token loading workflow and should remain as separate error codes.
Learnt from: findolor
PR: #1974
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: findolor
PR: #2004
File: packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte:10-12
Timestamp: 2025-07-17T13:33:08.771Z
Learning: In packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte, maintainer findolor is comfortable with mixed terminology during the strategy-to-order rename, where user-facing text is updated to "orders" but internal identifiers like test IDs ("invalid-strategies") and prop names ("strategiesWithErrors") can remain using "strategy" terminology.
Learnt from: hardingjam
PR: #1493
File: packages/ui-components/src/tests/DeployButton.test.ts:36-40
Timestamp: 2025-03-28T10:22:11.771Z
Learning: The DeployButton component in packages/ui-components/src/lib/components/deployment/DeployButton.svelte doesn't have an explicit disabled prop. Instead, its disabled state is internally controlled by the checkingDeployment variable, which is set to true during deployment checks.
Learnt from: hardingjam
PR: #1493
File: packages/ui-components/src/lib/components/deployment/DeployButton.svelte:0-0
Timestamp: 2025-03-31T14:01:19.067Z
Learning: The DeployButton component in packages/ui-components/src/lib/components/deployment/DeployButton.svelte is designed to be generic, dispatching clickDeploy events to be handled at the page level, where validation of event details (result, networkKey, subgraphUrl) occurs.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte:87-104
Timestamp: 2025-06-18T16:42:40.608Z
Learning: In the orderbook UI components, changing deployments or networks requires page navigation, which automatically remounts components and refreshes data like available tokens. No additional refresh logic is needed for deployment/network changes.
Learnt from: hardingjam
PR: #1594
File: packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte:0-0
Timestamp: 2025-04-05T13:16:58.971Z
Learning: In the DeploymentSteps component, the checkingDeployment state variable should be reset to false both after successful operations and error cases to ensure the deploy button returns to its normal state.
Learnt from: hardingjam
PR: #1870
File: packages/webapp/src/lib/components/WithdrawModal.svelte:31-31
Timestamp: 2025-05-20T12:03:18.032Z
Learning: The VaultActionArgs type no longer contains an action property after refactoring the deposit/withdraw modals into separate components.
Learnt from: findolor
PR: #2004
File: packages/ui-components/src/lib/components/input/InputRegistryUrl.svelte:29-33
Timestamp: 2025-07-17T13:33:29.619Z
Learning: In packages/ui-components/src/lib/components/input/InputRegistryUrl.svelte, findolor is comfortable keeping the input element id as "strategy-url" even when the placeholder text has been updated to reference "order", indicating acceptance of mixed terminology during the strategy-to-order rename transition.
Learnt from: hardingjam
PR: #1504
File: tauri-app/src/routes/orders/[network]-[orderHash]/+page.svelte:33-37
Timestamp: 2025-04-08T16:35:15.127Z
Learning: In the Rain OrderBook project, the onDeposit and onWithdraw functions in page components are kept simple (just calling modal handlers and revalidating queries) because error handling for deposit and withdraw actions (including user cancellations and failed transactions) is handled within the modal components themselves.
Learnt from: findolor
PR: #1744
File: packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte:73-76
Timestamp: 2025-05-09T05:30:02.520Z
Learning: In Rain.orderbook, VaultCalldataResult is a wrapper type that contains both error and value properties. When using this in the DepositOrWithdrawModal component, only the inner value field is passed to the handleTransaction function, not the complete wrapper.
packages/orderbook/test/js_api/gui.test.ts (26)
Learnt from: findolor
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: findolor
PR: #1996
File: packages/ui-components/src/tests/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/tests/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like VaultIdInformationComponentProps in test files, even when static analysis tools flag this as discouraged practice.
Learnt from: hardingjam
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
PR: #1469
File: packages/ui-components/src/tests/CodeMirrorDotrain.test.ts:75-98
Timestamp: 2025-03-31T10:16:53.544Z
Learning: In the rainlanguage/rain.orderbook project, direct manipulation of Svelte's internal state (component.$$.ctx) in tests is an acceptable approach for testing component behavior, as demonstrated in the CodeMirrorDotrain tests.
Learnt from: findolor
PR: #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
PR: #1936
File: packages/ui-components/vite.config.ts:21-23
Timestamp: 2025-06-17T14:55:22.914Z
Learning: In the rain.orderbook project, the Vite configuration uses 'import.meta.vitest': 'undefined' (as a string) combined with conditional if (import.meta.vitest) checks for in-source testing. The mock files are excluded from test execution using exclude: ['src/lib/__mocks__/**/*.ts']. This configuration successfully allows dev builds to work without vi undefined errors, despite the theoretical expectation that the string "undefined" would be truthy and cause issues.
Learnt from: hardingjam
PR: #1724
File: packages/ui-components/src/tests/ButtonDarkMode.test.ts:1-54
Timestamp: 2025-05-03T10:29:04.009Z
Learning: JSDoc comments are not considered necessary for test files in the rainlanguage/rain.orderbook repository. Test descriptions and assertions are sufficient documentation.
Learnt from: findolor
PR: #1974
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: hardingjam
PR: #1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:37-37
Timestamp: 2025-03-26T16:16:51.943Z
Learning: For Rain Orderbook projects, in test files, the preference is to use "as any" type assertions with per-line ESLint disable comments rather than creating dedicated types for test parameters.
Learnt from: findolor
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 to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Learnt from: findolor
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
PR: #1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:303-303
Timestamp: 2025-07-17T10:35:09.329Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is fine with hardcoded 2-second delays using setTimeout in test cases for waiting after token selection, preferring this approach over deterministic waiting patterns.
Learnt from: findolor
PR: #1917
File: tauri-app/src/routes/orders/add/+page.svelte:45-47
Timestamp: 2025-06-11T11:39:15.239Z
Learning: In the rain.orderbook codebase, every instance of Config (returned by helpers such as parseDotrainAndYaml) is guaranteed to include the dotrainOrder property; it is never undefined.
Learnt from: findolor
PR: #1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:172-172
Timestamp: 2025-07-17T10:35:53.182Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is comfortable with hardcoded 2-second delays using setTimeout after token selection elements appear, preferring this approach over deterministic waiting patterns with waitFor.
Learnt from: findolor
PR: #1975
File: packages/orderbook/test/js_api/gui.test.ts:958-960
Timestamp: 2025-07-09T14:00:09.618Z
Learning: In test files for the rain.orderbook project, maintainer findolor prefers to keep hard-coded serialized state strings (like gzip/base64 blobs) rather than replacing them with semantic assertions or round-trip testing, even when the hard-coded approach may be more brittle to changes.
Learnt from: findolor
PR: #1956
File: packages/ui-components/src/tests/VaultDetail.test.ts:177-181
Timestamp: 2025-07-04T09:14:35.748Z
Learning: In the rain.orderbook project, maintainer findolor is fine with mutating mock data directly in test files using @ts-expect-error comments, preferring this pragmatic approach over immutable patterns like spread operators for test data updates.
Learnt from: findolor
PR: #1916
File: packages/ui-components/src/lib/fixtures/settings-12-11-24.json:182-195
Timestamp: 2025-06-10T12:04:54.107Z
Learning: In test fixture files like packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json, network configuration inconsistencies (such as matchain using Polygon's RPC, chainId, and currency while having its own network key) are acceptable since they are used for testing purposes only.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/components/deployment/SelectToken.svelte:137-151
Timestamp: 2025-06-18T16:44:14.948Z
Learning: In the SelectToken.svelte component, the SDK validates addresses before making RPC calls, so calling saveTokenSelection on every keystroke in handleInput doesn't result in network calls until there's a full valid address.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/errors/DeploymentStepsError.ts:16-16
Timestamp: 2025-07-10T12:52:47.468Z
Learning: In packages/ui-components/src/lib/errors/DeploymentStepsError.ts, the error codes NO_SELECT_TOKENS ('Error loading tokens') and NO_AVAILABLE_TOKENS ('Error loading available tokens') represent different failure scenarios in the token loading workflow and should remain as separate error codes.
Learnt from: findolor
PR: #1588
File: packages/orderbook/test/js_api/gui.test.ts:2037-2057
Timestamp: 2025-04-08T12:53:12.526Z
Learning: In the remote network test for gui.test.ts, asserting that getCurrentDeployment() succeeds is sufficient validation that remote networks were properly fetched, as this operation would fail if the networks weren't correctly processed.
Learnt from: hardingjam
PR: #1860
File: packages/ui-components/src/lib/stores/transactionStore.ts:130-136
Timestamp: 2025-05-19T18:24:17.608Z
Learning: In the rain.orderbook project, the awaitSubgraphIndexing function has a two-step validation process: it first checks that data.value from WasmEncodedResult exists, and only then applies the isSuccess predicate. This makes a simple truthiness check (!!data) sufficient in the isSuccess predicate since the value has already been pre-validated.
Learnt from: thedavidmeister
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.
Learnt from: findolor
PR: #1721
File: crates/js_api/src/bindings/mod.rs:1-131
Timestamp: 2025-05-01T14:39:53.795Z
Learning: Empty input validation for functions like keccak256 and get_order_hash in the WebAssembly bindings is not required as empty inputs are considered valid in this context.
Learnt from: hardingjam
PR: #1860
File: packages/ui-components/src/lib/stores/transactionStore.ts:130-136
Timestamp: 2025-05-19T18:24:17.608Z
Learning: In the rain.orderbook project, the getTransaction function (and related entity fetch functions) use wasmEncodedResult to validate the returned data, ensuring that only valid data is returned. This makes a simple truthiness check (!!data) sufficient for the isSuccess predicate in awaitSubgraphIndexing.
Learnt from: findolor
PR: #1956
File: packages/orderbook/test/js_api/raindexClient.test.ts:91-99
Timestamp: 2025-07-04T10:23:41.820Z
Learning: In the rain.orderbook project, maintainer findolor is fine with test code patterns that might seem like inverted logic in production contexts, such as the extractWasmEncodedData function in test files that returns undefined values when result.value is undefined.
packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts (18)
Learnt from: findolor
PR: #1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:303-303
Timestamp: 2025-07-17T10:35:09.329Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is fine with hardcoded 2-second delays using setTimeout in test cases for waiting after token selection, preferring this approach over deterministic waiting patterns.
Learnt from: findolor
PR: #1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:172-172
Timestamp: 2025-07-17T10:35:53.182Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is comfortable with hardcoded 2-second delays using setTimeout after token selection elements appear, preferring this approach over deterministic waiting patterns with waitFor.
Learnt from: findolor
PR: #1974
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: findolor
PR: #2004
File: packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte:10-12
Timestamp: 2025-07-17T13:33:08.771Z
Learning: In packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte, maintainer findolor is comfortable with mixed terminology during the strategy-to-order rename, where user-facing text is updated to "orders" but internal identifiers like test IDs ("invalid-strategies") and prop names ("strategiesWithErrors") can remain using "strategy" terminology.
Learnt from: findolor
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: findolor
PR: #1925
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Learnt from: findolor
PR: #1588
File: packages/orderbook/test/js_api/gui.test.ts:2037-2057
Timestamp: 2025-04-08T12:53:12.526Z
Learning: In the remote network test for gui.test.ts, asserting that getCurrentDeployment() succeeds is sufficient validation that remote networks were properly fetched, as this operation would fail if the networks weren't correctly processed.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/errors/DeploymentStepsError.ts:16-16
Timestamp: 2025-07-10T12:52:47.468Z
Learning: In packages/ui-components/src/lib/errors/DeploymentStepsError.ts, the error codes NO_SELECT_TOKENS ('Error loading tokens') and NO_AVAILABLE_TOKENS ('Error loading available tokens') represent different failure scenarios in the token loading workflow and should remain as separate error codes.
Learnt from: findolor
PR: #1996
File: packages/ui-components/src/tests/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/tests/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like VaultIdInformationComponentProps in test files, even when static analysis tools flag this as discouraged practice.
Learnt from: findolor
PR: #1916
File: packages/ui-components/src/lib/fixtures/settings-12-11-24.json:182-195
Timestamp: 2025-06-10T12:04:54.107Z
Learning: In test fixture files like packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json, network configuration inconsistencies (such as matchain using Polygon's RPC, chainId, and currency while having its own network key) are acceptable since they are used for testing purposes only.
Learnt from: findolor
PR: #1687
File: crates/js_api/src/gui/order_operations.rs:470-489
Timestamp: 2025-04-29T11:17:46.178Z
Learning: In the get_deployment_transaction_args method of DotrainOrderGui, approvals are intentionally only checked against output tokens as per the design requirements.
Learnt from: hardingjam
PR: #1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:0-0
Timestamp: 2025-03-26T16:22:50.224Z
Learning: In the Rain Orderbook project, the DotrainOrderGui.getDeploymentDetail method resolves promises with objects that may contain either a value property or an error property, rather than rejecting promises on errors.
Learnt from: hardingjam
PR: #1566
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/+page.svelte:26-31
Timestamp: 2025-04-02T08:11:19.742Z
Learning: In the Rain Orderbook deployment page component, the conditional check if (dotrain && deployment) inside onMount is necessary to prevent GUI initialization with null values, even though there's already a redirect logic outside the function and conditional rendering for the UI message. These serve different purposes in the component's error handling strategy.
Learnt from: thedavidmeister
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.
Learnt from: findolor
PR: #1744
File: packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte:73-76
Timestamp: 2025-05-09T05:30:02.520Z
Learning: In Rain.orderbook, VaultCalldataResult is a wrapper type that contains both error and value properties. When using this in the DepositOrWithdrawModal component, only the inner value field is passed to the handleTransaction function, not the complete wrapper.
Learnt from: hardingjam
PR: #1870
File: packages/webapp/src/lib/components/WithdrawModal.svelte:31-31
Timestamp: 2025-05-20T12:03:18.032Z
Learning: The VaultActionArgs type no longer contains an action property after refactoring the deposit/withdraw modals into separate components.
Learnt from: findolor
PR: #1983
File: crates/js_api/src/gui/deposits.rs:147-156
Timestamp: 2025-07-11T06:41:11.924Z
Learning: In the Rain OrderBook project, token info is already cached in the YAML file which serves as the source of truth, so the get_token_info() async method in the GUI SDK retrieves from pre-loaded/parsed data rather than making expensive network calls.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/components/deployment/SelectToken.svelte:137-151
Timestamp: 2025-06-18T16:44:14.948Z
Learning: In the SelectToken.svelte component, the SDK validates addresses before making RPC calls, so calling saveTokenSelection on every keystroke in handleInput doesn't result in network calls until there's a full valid address.
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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
PR: rainlanguage/rain.orderbook#1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22: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: 0xgleb
PR: rainlanguage/rain.orderbook#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: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:34:42.328Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must use the format:
TOTAL:number
ADDITIONS:number
DELETIONS:number
with no extra text, formatting blocks, or explanations, and must still exclude lock files and reassess after each new commit.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: packages/ui-components/src/lib/components/ButtonVaultLink.svelte:18-18
Timestamp: 2025-07-04T09:13:39.020Z
Learning: In the RaindexVault type, the vaultId field is a bigint, so bigintToHex should be used instead of bigintStringToHex when converting vault IDs to hex format in UI components.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1870
File: packages/webapp/src/lib/components/WithdrawModal.svelte:31-31
Timestamp: 2025-05-20T12:03:18.032Z
Learning: The VaultActionArgs type no longer contains an action property after refactoring the deposit/withdraw modals into separate components.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1872
File: packages/webapp/src/lib/services/handleVaultWithdraw.ts:74-76
Timestamp: 2025-06-07T05:18:31.404Z
Learning: In vault withdrawal error handling, prefer user-friendly generic error messages over exposing technical error details from getVaultWithdrawCalldata, as the underlying errors are not end user friendly.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1882
File: packages/webapp/src/lib/services/handleVaultDeposit.ts:96-125
Timestamp: 2025-06-07T09:07:53.733Z
Learning: In the vault deposit flow, vault.token.address is already validated before reaching the approval transaction creation, so casting it to Hex type is safe and doesn't require additional validation.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1925
File: crates/settings/src/remote_tokens.rs:44-62
Timestamp: 2025-07-11T08:45:43.961Z
Learning: findolor prefers to avoid performance micro-optimizations like changing `.values()` to `.into_values()` in HashMap iterations in the rainlanguage/rain.orderbook codebase, even when they would eliminate unnecessary cloning.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1903
File: crates/js_api/src/subgraph/vault.rs:129-138
Timestamp: 2025-06-17T16:33:58.549Z
Learning: findolor prefers downstream error handling over early validation checks for empty RPC lists in vault functions, allowing errors to bubble up from provider creation logic naturally.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1916
File: packages/webapp/src/routes/+layout.ts:26-26
Timestamp: 2025-06-10T12:01:07.418Z
Learning: The user findolor prefers to keep string concatenation using the plus operator over template literals in the rainlanguage/rain.orderbook project, even when static analysis tools suggest the change.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1975
File: crates/js_api/src/config.rs:44-45
Timestamp: 2025-07-09T07:42:48.458Z
Learning: User findolor prefers to defer code improvements and refactoring suggestions to future PRs when they are not directly related to the current PR's scope, even when the suggestions are valid improvements.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1947
File: crates/common/src/raindex_client/orders.rs:462-1643
Timestamp: 2025-06-24T08:45:10.971Z
Learning: User findolor prefers to keep tests simple and avoid refactoring for maintainability when the code is stable and viewed as "one time thing" that won't require frequent changes.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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.
packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte (12)
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/errors/DeploymentStepsError.ts:16-16
Timestamp: 2025-07-10T12:52:47.468Z
Learning: In packages/ui-components/src/lib/errors/DeploymentStepsError.ts, the error codes NO_SELECT_TOKENS ('Error loading tokens') and NO_AVAILABLE_TOKENS ('Error loading available tokens') represent different failure scenarios in the token loading workflow and should remain as separate error codes.
Learnt from: findolor
PR: #1974
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: findolor
PR: #2004
File: packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte:10-12
Timestamp: 2025-07-17T13:33:08.771Z
Learning: In packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte, maintainer findolor is comfortable with mixed terminology during the strategy-to-order rename, where user-facing text is updated to "orders" but internal identifiers like test IDs ("invalid-strategies") and prop names ("strategiesWithErrors") can remain using "strategy" terminology.
Learnt from: hardingjam
PR: #1493
File: packages/ui-components/src/tests/DeployButton.test.ts:36-40
Timestamp: 2025-03-28T10:22:11.771Z
Learning: The DeployButton component in packages/ui-components/src/lib/components/deployment/DeployButton.svelte doesn't have an explicit disabled prop. Instead, its disabled state is internally controlled by the checkingDeployment variable, which is set to true during deployment checks.
Learnt from: hardingjam
PR: #1493
File: packages/ui-components/src/lib/components/deployment/DeployButton.svelte:0-0
Timestamp: 2025-03-31T14:01:19.067Z
Learning: The DeployButton component in packages/ui-components/src/lib/components/deployment/DeployButton.svelte is designed to be generic, dispatching clickDeploy events to be handled at the page level, where validation of event details (result, networkKey, subgraphUrl) occurs.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte:87-104
Timestamp: 2025-06-18T16:42:40.608Z
Learning: In the orderbook UI components, changing deployments or networks requires page navigation, which automatically remounts components and refreshes data like available tokens. No additional refresh logic is needed for deployment/network changes.
Learnt from: hardingjam
PR: #1594
File: packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte:0-0
Timestamp: 2025-04-05T13:16:58.971Z
Learning: In the DeploymentSteps component, the checkingDeployment state variable should be reset to false both after successful operations and error cases to ensure the deploy button returns to its normal state.
Learnt from: hardingjam
PR: #1870
File: packages/webapp/src/lib/components/WithdrawModal.svelte:31-31
Timestamp: 2025-05-20T12:03:18.032Z
Learning: The VaultActionArgs type no longer contains an action property after refactoring the deposit/withdraw modals into separate components.
Learnt from: findolor
PR: #2004
File: packages/ui-components/src/lib/components/input/InputRegistryUrl.svelte:29-33
Timestamp: 2025-07-17T13:33:29.619Z
Learning: In packages/ui-components/src/lib/components/input/InputRegistryUrl.svelte, findolor is comfortable keeping the input element id as "strategy-url" even when the placeholder text has been updated to reference "order", indicating acceptance of mixed terminology during the strategy-to-order rename transition.
Learnt from: hardingjam
PR: #1504
File: tauri-app/src/routes/orders/[network]-[orderHash]/+page.svelte:33-37
Timestamp: 2025-04-08T16:35:15.127Z
Learning: In the Rain OrderBook project, the onDeposit and onWithdraw functions in page components are kept simple (just calling modal handlers and revalidating queries) because error handling for deposit and withdraw actions (including user cancellations and failed transactions) is handled within the modal components themselves.
Learnt from: findolor
PR: #1744
File: packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte:73-76
Timestamp: 2025-05-09T05:30:02.520Z
Learning: In Rain.orderbook, VaultCalldataResult is a wrapper type that contains both error and value properties. When using this in the DepositOrWithdrawModal component, only the inner value field is passed to the handleTransaction function, not the complete wrapper.
packages/orderbook/test/js_api/gui.test.ts (26)
Learnt from: findolor
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: findolor
PR: #1996
File: packages/ui-components/src/tests/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/tests/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like VaultIdInformationComponentProps in test files, even when static analysis tools flag this as discouraged practice.
Learnt from: hardingjam
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
PR: #1469
File: packages/ui-components/src/tests/CodeMirrorDotrain.test.ts:75-98
Timestamp: 2025-03-31T10:16:53.544Z
Learning: In the rainlanguage/rain.orderbook project, direct manipulation of Svelte's internal state (component.$$.ctx) in tests is an acceptable approach for testing component behavior, as demonstrated in the CodeMirrorDotrain tests.
Learnt from: findolor
PR: #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
PR: #1936
File: packages/ui-components/vite.config.ts:21-23
Timestamp: 2025-06-17T14:55:22.914Z
Learning: In the rain.orderbook project, the Vite configuration uses 'import.meta.vitest': 'undefined' (as a string) combined with conditional if (import.meta.vitest) checks for in-source testing. The mock files are excluded from test execution using exclude: ['src/lib/__mocks__/**/*.ts']. This configuration successfully allows dev builds to work without vi undefined errors, despite the theoretical expectation that the string "undefined" would be truthy and cause issues.
Learnt from: hardingjam
PR: #1724
File: packages/ui-components/src/tests/ButtonDarkMode.test.ts:1-54
Timestamp: 2025-05-03T10:29:04.009Z
Learning: JSDoc comments are not considered necessary for test files in the rainlanguage/rain.orderbook repository. Test descriptions and assertions are sufficient documentation.
Learnt from: findolor
PR: #1974
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: hardingjam
PR: #1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:37-37
Timestamp: 2025-03-26T16:16:51.943Z
Learning: For Rain Orderbook projects, in test files, the preference is to use "as any" type assertions with per-line ESLint disable comments rather than creating dedicated types for test parameters.
Learnt from: findolor
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 to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Learnt from: findolor
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
PR: #1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:303-303
Timestamp: 2025-07-17T10:35:09.329Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is fine with hardcoded 2-second delays using setTimeout in test cases for waiting after token selection, preferring this approach over deterministic waiting patterns.
Learnt from: findolor
PR: #1917
File: tauri-app/src/routes/orders/add/+page.svelte:45-47
Timestamp: 2025-06-11T11:39:15.239Z
Learning: In the rain.orderbook codebase, every instance of Config (returned by helpers such as parseDotrainAndYaml) is guaranteed to include the dotrainOrder property; it is never undefined.
Learnt from: findolor
PR: #1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:172-172
Timestamp: 2025-07-17T10:35:53.182Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is comfortable with hardcoded 2-second delays using setTimeout after token selection elements appear, preferring this approach over deterministic waiting patterns with waitFor.
Learnt from: findolor
PR: #1975
File: packages/orderbook/test/js_api/gui.test.ts:958-960
Timestamp: 2025-07-09T14:00:09.618Z
Learning: In test files for the rain.orderbook project, maintainer findolor prefers to keep hard-coded serialized state strings (like gzip/base64 blobs) rather than replacing them with semantic assertions or round-trip testing, even when the hard-coded approach may be more brittle to changes.
Learnt from: findolor
PR: #1956
File: packages/ui-components/src/tests/VaultDetail.test.ts:177-181
Timestamp: 2025-07-04T09:14:35.748Z
Learning: In the rain.orderbook project, maintainer findolor is fine with mutating mock data directly in test files using @ts-expect-error comments, preferring this pragmatic approach over immutable patterns like spread operators for test data updates.
Learnt from: findolor
PR: #1916
File: packages/ui-components/src/lib/fixtures/settings-12-11-24.json:182-195
Timestamp: 2025-06-10T12:04:54.107Z
Learning: In test fixture files like packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json, network configuration inconsistencies (such as matchain using Polygon's RPC, chainId, and currency while having its own network key) are acceptable since they are used for testing purposes only.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/components/deployment/SelectToken.svelte:137-151
Timestamp: 2025-06-18T16:44:14.948Z
Learning: In the SelectToken.svelte component, the SDK validates addresses before making RPC calls, so calling saveTokenSelection on every keystroke in handleInput doesn't result in network calls until there's a full valid address.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/errors/DeploymentStepsError.ts:16-16
Timestamp: 2025-07-10T12:52:47.468Z
Learning: In packages/ui-components/src/lib/errors/DeploymentStepsError.ts, the error codes NO_SELECT_TOKENS ('Error loading tokens') and NO_AVAILABLE_TOKENS ('Error loading available tokens') represent different failure scenarios in the token loading workflow and should remain as separate error codes.
Learnt from: findolor
PR: #1588
File: packages/orderbook/test/js_api/gui.test.ts:2037-2057
Timestamp: 2025-04-08T12:53:12.526Z
Learning: In the remote network test for gui.test.ts, asserting that getCurrentDeployment() succeeds is sufficient validation that remote networks were properly fetched, as this operation would fail if the networks weren't correctly processed.
Learnt from: hardingjam
PR: #1860
File: packages/ui-components/src/lib/stores/transactionStore.ts:130-136
Timestamp: 2025-05-19T18:24:17.608Z
Learning: In the rain.orderbook project, the awaitSubgraphIndexing function has a two-step validation process: it first checks that data.value from WasmEncodedResult exists, and only then applies the isSuccess predicate. This makes a simple truthiness check (!!data) sufficient in the isSuccess predicate since the value has already been pre-validated.
Learnt from: thedavidmeister
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.
Learnt from: findolor
PR: #1721
File: crates/js_api/src/bindings/mod.rs:1-131
Timestamp: 2025-05-01T14:39:53.795Z
Learning: Empty input validation for functions like keccak256 and get_order_hash in the WebAssembly bindings is not required as empty inputs are considered valid in this context.
Learnt from: hardingjam
PR: #1860
File: packages/ui-components/src/lib/stores/transactionStore.ts:130-136
Timestamp: 2025-05-19T18:24:17.608Z
Learning: In the rain.orderbook project, the getTransaction function (and related entity fetch functions) use wasmEncodedResult to validate the returned data, ensuring that only valid data is returned. This makes a simple truthiness check (!!data) sufficient for the isSuccess predicate in awaitSubgraphIndexing.
Learnt from: findolor
PR: #1956
File: packages/orderbook/test/js_api/raindexClient.test.ts:91-99
Timestamp: 2025-07-04T10:23:41.820Z
Learning: In the rain.orderbook project, maintainer findolor is fine with test code patterns that might seem like inverted logic in production contexts, such as the extractWasmEncodedData function in test files that returns undefined values when result.value is undefined.
packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts (18)
Learnt from: findolor
PR: #1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:303-303
Timestamp: 2025-07-17T10:35:09.329Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is fine with hardcoded 2-second delays using setTimeout in test cases for waiting after token selection, preferring this approach over deterministic waiting patterns.
Learnt from: findolor
PR: #1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:172-172
Timestamp: 2025-07-17T10:35:53.182Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is comfortable with hardcoded 2-second delays using setTimeout after token selection elements appear, preferring this approach over deterministic waiting patterns with waitFor.
Learnt from: findolor
PR: #1974
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: findolor
PR: #2004
File: packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte:10-12
Timestamp: 2025-07-17T13:33:08.771Z
Learning: In packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte, maintainer findolor is comfortable with mixed terminology during the strategy-to-order rename, where user-facing text is updated to "orders" but internal identifiers like test IDs ("invalid-strategies") and prop names ("strategiesWithErrors") can remain using "strategy" terminology.
Learnt from: findolor
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: findolor
PR: #1925
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Learnt from: findolor
PR: #1588
File: packages/orderbook/test/js_api/gui.test.ts:2037-2057
Timestamp: 2025-04-08T12:53:12.526Z
Learning: In the remote network test for gui.test.ts, asserting that getCurrentDeployment() succeeds is sufficient validation that remote networks were properly fetched, as this operation would fail if the networks weren't correctly processed.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/errors/DeploymentStepsError.ts:16-16
Timestamp: 2025-07-10T12:52:47.468Z
Learning: In packages/ui-components/src/lib/errors/DeploymentStepsError.ts, the error codes NO_SELECT_TOKENS ('Error loading tokens') and NO_AVAILABLE_TOKENS ('Error loading available tokens') represent different failure scenarios in the token loading workflow and should remain as separate error codes.
Learnt from: findolor
PR: #1996
File: packages/ui-components/src/tests/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/tests/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like VaultIdInformationComponentProps in test files, even when static analysis tools flag this as discouraged practice.
Learnt from: findolor
PR: #1916
File: packages/ui-components/src/lib/fixtures/settings-12-11-24.json:182-195
Timestamp: 2025-06-10T12:04:54.107Z
Learning: In test fixture files like packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json, network configuration inconsistencies (such as matchain using Polygon's RPC, chainId, and currency while having its own network key) are acceptable since they are used for testing purposes only.
Learnt from: findolor
PR: #1687
File: crates/js_api/src/gui/order_operations.rs:470-489
Timestamp: 2025-04-29T11:17:46.178Z
Learning: In the get_deployment_transaction_args method of DotrainOrderGui, approvals are intentionally only checked against output tokens as per the design requirements.
Learnt from: hardingjam
PR: #1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:0-0
Timestamp: 2025-03-26T16:22:50.224Z
Learning: In the Rain Orderbook project, the DotrainOrderGui.getDeploymentDetail method resolves promises with objects that may contain either a value property or an error property, rather than rejecting promises on errors.
Learnt from: hardingjam
PR: #1566
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/+page.svelte:26-31
Timestamp: 2025-04-02T08:11:19.742Z
Learning: In the Rain Orderbook deployment page component, the conditional check if (dotrain && deployment) inside onMount is necessary to prevent GUI initialization with null values, even though there's already a redirect logic outside the function and conditional rendering for the UI message. These serve different purposes in the component's error handling strategy.
Learnt from: thedavidmeister
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.
Learnt from: findolor
PR: #1744
File: packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte:73-76
Timestamp: 2025-05-09T05:30:02.520Z
Learning: In Rain.orderbook, VaultCalldataResult is a wrapper type that contains both error and value properties. When using this in the DepositOrWithdrawModal component, only the inner value field is passed to the handleTransaction function, not the complete wrapper.
Learnt from: hardingjam
PR: #1870
File: packages/webapp/src/lib/components/WithdrawModal.svelte:31-31
Timestamp: 2025-05-20T12:03:18.032Z
Learning: The VaultActionArgs type no longer contains an action property after refactoring the deposit/withdraw modals into separate components.
Learnt from: findolor
PR: #1983
File: crates/js_api/src/gui/deposits.rs:147-156
Timestamp: 2025-07-11T06:41:11.924Z
Learning: In the Rain OrderBook project, token info is already cached in the YAML file which serves as the source of truth, so the get_token_info() async method in the GUI SDK retrieves from pre-loaded/parsed data rather than making expensive network calls.
Learnt from: findolor
PR: #1925
File: packages/ui-components/src/lib/components/deployment/SelectToken.svelte:137-151
Timestamp: 2025-06-18T16:44:14.948Z
Learning: In the SelectToken.svelte component, the SDK validates addresses before making RPC calls, so calling saveTokenSelection on every keystroke in handleInput doesn't result in network calls until there's a full valid address.
🔇 Additional comments (7)
packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts (3)
217-219: Verify vault-ID ↔ token assignment matches the UI flowIn the UI simulation you first type
0x123for token 2 and0x234for token 1 (lines 193-199).
Here the programmatic setup assigns
'0x123' → token1/outputand'0x234' → token2/input.Double-check the mapping is intentional; otherwise deployment calldata may embed the wrong vault IDs.
374-375: Check vault mapping consistency for Auction strategySame potential swap: UI puts
0x123on output,0x234on input but token keys passed here are'output'/'output'and'input'/'input'.
Confirm tokens & vault types are aligned with the front-end order to avoid mis-configured orders.
520-521: Dynamic spread strategy may have token/vault mix-upUI input order is token1 →
0x234, token2 →0x123; code assigns
'0x123' → token2/output,'0x234' → token1/input.
Please verify this mirrors the intended vault semantics.packages/orderbook/test/js_api/gui.test.ts (4)
965-965: LGTM - Serialized state updated for new vault ID structure.The hardcoded serialized state reflects the underlying changes to the vault ID data structure, transitioning from positional indexing to token-key-based identification.
1005-1006: LGTM - setVaultId method updated to use token-key-based API.The method calls have been correctly updated to use the new API with string-based vault type and token key parameters instead of the previous boolean flag and index approach.
1629-1671: LGTM - getVaultIds correctly implements nested map structure.The method now returns a well-structured nested map (
Map<string, Map<string, string | undefined>>) where the outer map distinguishes between input/output vault types and the inner map associates token keys with their vault IDs. The test assertions correctly validate:
- Setting and retrieving vault IDs by vault type and token key
- Handling undefined values when clearing vault IDs
- Handling empty string values (converted to undefined)
This is a significant improvement in API clarity compared to the previous array-indexed approach.
1676-1679: LGTM - Error messages updated for token-key-based API.The error messages now reference specific token keys ('token1') instead of positional indices, providing more meaningful and user-friendly error information that aligns with the new API structure.
| {#each allTokenOutputs as output} | ||
| <TokenIOInput label="Output" vault={output} /> | ||
| {/each} | ||
|
|
||
| {#each allTokenInputs as input, i} | ||
| <TokenIOInput {i} label="Input" vault={input} /> | ||
| {#each allTokenInputs as input} | ||
| <TokenIOInput label="Input" vault={input} /> | ||
| {/each} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add keyed each blocks to avoid unstable DOM diffing
TokenIOInput items are now rendered without an explicit key.
If allTokenOutputs / allTokenInputs are replaced with new array instances that contain objects with the same identity, Svelte will destroy & recreate every row on every update, losing local component state and hurting performance.
-{#each allTokenOutputs as output}
- <TokenIOInput label="Output" vault={output} />
+{#each allTokenOutputs as output (output.tokenKey)}
+ <TokenIOInput label="Output" vault={output} />
{/each}
-{#each allTokenInputs as input}
- <TokenIOInput label="Input" vault={input} />
+{#each allTokenInputs as input (input.tokenKey)}
+ <TokenIOInput label="Input" vault={input} />
{/each}Use a deterministic unique key (tokenKey, vaultId, etc.) to preserve component instances across reactive updates.
🤖 Prompt for AI Agents
In packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte
around lines 244 to 250, the each blocks rendering TokenIOInput components lack
unique keys, causing Svelte to destroy and recreate components unnecessarily on
updates. Fix this by adding a unique key expression to each each block, such as
using a stable property like tokenKey or vaultId from the output and input
objects, to ensure component instances are preserved across updates and improve
performance.
3 participants
Motivation
See issue: #1931
The two functions we have for the vault ids weren't properly written. The input parameters and return types were hard to work with. This PR aims to improve that.
Solution
These are patterns that we use in the app so the functions follow a better structure now.
Checks
By submitting this for review, I'm confirming I've done the following:
fix #1931
Summary by CodeRabbit
Refactor
Bug Fixes
Tests