Emit additional meta during order deployment #2331
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds optional metadata support across the stack: extends AddOrderArgs with Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Deployment UI
participant Handler as handleAddOrder
participant TxMgr as TransactionManager
participant Common as AddOrderArgs
participant Metaboard as Metaboard
User->>UI: Start deployment
UI->>Handler: request handleAddOrder
Handler->>Common: build AddOrderArgs (may include additional_meta)
Common-->>Handler: AddOrderArgs (with optional emit_meta_call)
Handler->>Handler: detect emit_meta_call
alt emit_meta_call present
Handler->>UI: show "Publishing metadata" modal
UI-->>Handler: confirm
Handler->>TxMgr: createMetaTransaction(chainId, calldata, txHash)
TxMgr->>Metaboard: submit emitMeta calldata
Metaboard-->>TxMgr: result
TxMgr-->>Handler: success/failure
end
Handler->>UI: show "Deploying your order" modal
UI-->>Handler: confirm
Handler->>Metaboard: submit AddOrder transaction
Metaboard-->>Handler: deployment result
Handler->>UI: completion
sequenceDiagram
participant App
participant Raindex as RaindexOrder
participant Parser as ParsedMeta
participant Metaboard as MetaboardSubgraphClient
App->>Raindex: get_order_by_hash(hash)
Raindex->>Parser: parse meta bytes -> ParsedMeta list
Raindex-->>App: order with parsed_meta
alt parsed_meta lacks dotrain_source
Raindex->>Metaboard: fetch_dotrain_source() [async]
Metaboard-->>Raindex: dotrain meta items
Raindex->>Parser: parse new meta -> update parsed_meta
end
Raindex-->>App: order with dotrain_source/dotrain_gui_state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (60)📓 Common learnings📚 Learning: 2025-10-02T19:17:20.332ZApplied to files:
📚 Learning: 2025-06-24T08:46:03.368ZApplied to files:
📚 Learning: 2025-10-06T14:13:18.531ZApplied to files:
📚 Learning: 2025-07-30T07:41:39.271ZApplied to files:
📚 Learning: 2025-06-18T12:54:47.807ZApplied to files:
📚 Learning: 2025-07-21T16:37:20.599ZApplied to files:
📚 Learning: 2025-10-18T10:38:41.273ZApplied to files:
📚 Learning: 2025-07-18T10:31:05.498ZApplied to files:
📚 Learning: 2025-07-04T10:24:56.163ZApplied to files:
📚 Learning: 2025-07-21T16:34:31.193ZApplied to files:
📚 Learning: 2025-10-06T11:28:30.692ZApplied to files:
📚 Learning: 2025-10-13T12:57:05.900ZApplied to files:
📚 Learning: 2025-07-04T10:27:22.544ZApplied to files:
📚 Learning: 2025-07-15T08:01:38.534ZApplied to files:
📚 Learning: 2025-06-24T13:36:28.797ZApplied to files:
📚 Learning: 2025-06-17T16:32:04.554ZApplied to files:
📚 Learning: 2025-05-09T05:28:22.089ZApplied to files:
📚 Learning: 2025-08-01T09:07:20.383ZApplied to files:
📚 Learning: 2025-06-06T16:38:04.182ZApplied to files:
📚 Learning: 2025-08-02T02:34:32.237ZApplied to files:
📚 Learning: 2025-07-16T10:40:05.717ZApplied to files:
📚 Learning: 2025-07-31T19:34:11.716ZApplied to files:
📚 Learning: 2025-05-14T05:13:59.713ZApplied to files:
📚 Learning: 2025-06-18T19:23:33.747ZApplied to files:
📚 Learning: 2025-09-01T13:35:33.688ZApplied to files:
📚 Learning: 2025-04-29T06:19:15.446ZApplied to files:
📚 Learning: 2025-09-24T10:59:25.666ZApplied to files:
📚 Learning: 2025-07-04T09:02:57.301ZApplied to files:
📚 Learning: 2025-06-17T16:21:24.384ZApplied to files:
📚 Learning: 2025-10-14T07:51:55.148ZApplied to files:
📚 Learning: 2025-07-04T09:13:39.020ZApplied to files:
📚 Learning: 2025-05-19T18:24:17.608ZApplied to files:
📚 Learning: 2025-05-21T21:24:53.708ZApplied to files:
📚 Learning: 2025-05-21T22:14:22.646ZApplied to files:
📚 Learning: 2025-05-21T21:25:47.873ZApplied to files:
📚 Learning: 2025-05-21T22:14:22.646ZApplied to files:
📚 Learning: 2025-05-21T21:26:31.566ZApplied to files:
📚 Learning: 2025-05-21T22:35:26.448ZApplied to files:
📚 Learning: 2025-05-21T22:46:08.530ZApplied to files:
📚 Learning: 2025-10-06T11:13:29.956ZApplied to files:
📚 Learning: 2025-08-01T07:44:53.910ZApplied to files:
📚 Learning: 2025-11-04T09:20:06.368ZApplied to files:
📚 Learning: 2025-05-09T05:21:40.234ZApplied to files:
📚 Learning: 2025-06-18T19:24:40.518ZApplied to files:
📚 Learning: 2025-03-31T14:36:11.049ZApplied to files:
📚 Learning: 2025-08-26T14:52:37.000ZApplied to files:
📚 Learning: 2025-06-18T18:18:44.330ZApplied to files:
📚 Learning: 2025-06-17T16:54:43.176ZApplied to files:
📚 Learning: 2025-07-04T10:26:24.289ZApplied to files:
📚 Learning: 2025-07-23T10:51:12.278ZApplied to files:
📚 Learning: 2025-07-04T10:24:29.750ZApplied to files:
📚 Learning: 2025-06-07T05:19:04.767ZApplied to files:
📚 Learning: 2025-04-30T09:28:36.960ZApplied to files:
📚 Learning: 2025-05-14T04:49:14.621ZApplied to files:
📚 Learning: 2025-08-02T03:55:25.215ZApplied to files:
📚 Learning: 2025-05-20T10:20:08.206ZApplied to files:
📚 Learning: 2025-06-08T18:43:51.842ZApplied to files:
📚 Learning: 2025-04-08T12:56:03.272ZApplied to files:
📚 Learning: 2025-07-21T16:34:04.947ZApplied to files:
🧬 Code graph analysis (2)crates/common/src/raindex_client/add_orders.rs (1)
crates/common/src/raindex_client/orders.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
🔇 Additional comments (6)
Comment |
4 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/ui-components/src/lib/components/detail/OrderDetail.svelte (1)
79-99: Fix query key and invalidation key mismatch—refresh will not work.TanStack Query's fuzzy query key matching only works for array prefixes starting from the first element: if your key is ['getWidgetAdmin', some, more, filters], you can fuzzily invalidate with ['getWidgetAdmin'], not with elements from the middle.
Your queryKey is
[QKEY_ORDER, orderHash](line 79) but both the polling interval (line 88) and manual refresh (line 97) callinvalidateTanstackQueries(queryClient, [orderHash]). Since[orderHash]doesn't match the prefix[QKEY_ORDER, orderHash], the query will never be invalidated or refetched.Align the invalidation key to match:
await invalidateTanstackQueries(queryClient, [QKEY_ORDER, orderHash]);Also update the test expectation at
packages/ui-components/src/__tests__/OrderDetail.test.ts:270to expect the corrected key.packages/orderbook/test/js_api/gui.test.ts (1)
139-463: Metaboard fixtures and mocks align with new metadata workflow
- The various
dotrain*YAML fixtures now declaremetaboardsentries pointing athttp://localhost:8085/metaboard, and the deployment-args test sets up/metaboardPOST mocks formetaV1SandmetaBoards. This matches the expected GraphQL access pattern for locating a metaboard instance.- The
getDeploymentTransactionArgstest asserts that:
deploymentCalldatalength increases when deposits are present and shrinks when cleared.emitMetaCallis always present, targets the zero address returned from the mock, and encodes non-zero subject + metadata bytes whose decoded text includes#handle-add-order.- Together these checks tightly couple JS API behavior to the new Rust-side metadata emission, which is appropriate regression coverage for the meta feature.
If you find these tests becoming brittle over time due to minor calldata layout changes, consider relaxing length checks (e.g., “greater than baseline” plus structural assertions) while keeping the
emitMetaCalldecoding assertions as the primary signal.Also applies to: 1782-1897
crates/js_api/src/gui/order_operations.rs (2)
87-108: Expose and documentemit_meta_callin the deployment args APIThe new
ExternalCalltype andemit_meta_call: Option<ExternalCall>field onDeploymentTransactionArgsare wired correctly for wasm/TS (camelCase+ explicittsify(type = "ExternalCall | undefined")), andget_deployment_transaction_argsnow fills this when a meta call should be emitted. The doc comment forget_deployment_transaction_argsand the “Transaction Package” section, however, still list only approvals, deploymentCalldata, orderbookAddress, and chainId.Consider updating the documentation to explicitly mention the optional
emitMetaCallpayload so consumers know to handle metadata publication.Also applies to: 731-763
802-855: Metaboard integration makes metaboard config effectively mandatory—breaks deployments without itThe implementation at line 862 calls
orderbook_yaml.get_metaboard(&deployment.deployment.order.network.key)?without handling missing config errors, causingget_deployment_transaction_argsto fail entirely when metaboard is not configured. This differs fromcrates/common/src/raindex_client/orders.rs:300-304, which explicitly catchesYamlError::KeyNotFound | YamlError::NotFoundand returnsOk(None).If existing orderbook YAMLs omit metaboard configuration, this change prevents order deployment via the GUI without adding metaboard config. To preserve backward compatibility and align with the Raindex pattern, handle missing metaboard gracefully in
should_emit_meta_call()orget_metaboard_client()by returningOk(false)orNoneinstead of propagating the error.crates/js_api/src/gui/state_management.rs (1)
354-421: State serialization / restoration and select-token tests are robust
encode_statemirrors theserialize_statepipeline (bincode → gzip → base64-url) and is used only in tests, keeping production logic single-sourced.- The new tests around
new_from_state:
- enforce that unknown
select_tokenskeys yieldGuiError::TokenNotInSelectTokens,- ensure an existing token record is removed then replaced from serialized state,
- and verify failure on missing field presets, which is important for preventing silent state corruption.
- Error messages for dotrain mismatch remain unchanged and are now validated against the new hashing method with
DotrainSourceV1.Overall this suite gives good coverage of the new metadata-related state paths.
Also applies to: 541-547, 700-799
crates/common/src/dotrain_order.rs (1)
871-891: Rainlang normalization and deployment-generation tests nicely stabilize expectations
normalize_rainlangeliminates trailing whitespace differences line-by-line, which makes the Rainlang-related tests resilient to minor formatting changes in the composer while still checking semantic structure.split_frontmatter_and_body,get_root_hash, andassert_no_vault_idsprovide focused helpers that make thegenerate_dotrain_for_deployment_*tests readable and precise.- Tests assert that:
- tokens are intentionally omitted from the pruned frontmatter,
- all
vault-idfields are removed from inputs/outputs,- only the referenced orderbook/subgraph/gui deployment entries are retained, and
- a missing deployment key surfaces as a
YamlError::Field { Missing(..), location: "deployments" }viaDotrainOrderError::YamlError, which matches existing error conventions.- The updated Rainlang equality assertions now compare normalized strings, reducing brittleness elsewhere in the suite as well.
Also applies to: 941-991, 1000-1123, 1125-1293, 1971-2044
crates/common/src/raindex_client/orders.rs (1)
9-79: ParsedMeta field and getters are wired correctly (minor clone nit)
RaindexOrdernow carriesparsed_meta: Vec<ParsedMeta>and both WASM and non‑WASM impls expose:
parsed_meta()returning a clonedVec<ParsedMeta>.dotrain_source()selecting the firstDotrainSourceV1.dotrain_gui_state()JSON‑encoding the firstDotrainGuiStateV1.This is functionally sound and matches how the tests exercise these accessors. If you care about micro‑allocations,
dotrain_source/dotrain_gui_statecould iterate over&self.parsed_metainstead of callingself.parsed_meta()to avoid an extra clone.
Comment on lines
+294
to
+306
| #[wasm_export(skip)] | ||
| pub fn get_metaboard_client(&self) -> Result<Option<MetaboardSubgraphClient>, RaindexError> { | ||
| let raindex_client = self.get_raindex_client(); | ||
| let network = raindex_client | ||
| .orderbook_yaml | ||
| .get_network_by_chain_id(self.chain_id)?; | ||
| let metaboard = match raindex_client.orderbook_yaml.get_metaboard(&network.key) { | ||
| Ok(metaboard) => metaboard, | ||
| Err(YamlError::KeyNotFound(_) | YamlError::NotFound(_)) => return Ok(None), | ||
| Err(err) => return Err(err.into()), | ||
| }; | ||
| Ok(Some(MetaboardSubgraphClient::new(metaboard.url.clone()))) | ||
| } |
Contributor
There was a problem hiding this comment.
Missing YamlError import in get_metaboard_client (compile error)
get_metaboard_client matches on YamlError::KeyNotFound and YamlError::NotFound, but YamlError is not imported in this module. This will fail to compile.
Add the import at the top of this file:
use crate::raindex_client::vaults_list::RaindexVaultsList;
+use rain_orderbook_app_settings::yaml::YamlError;The rest of the logic (treating “metaboard missing” as Ok(None) and only propagating other YAML errors) looks good.
@@
-use crate::raindex_client::vaults_list::RaindexVaultsList;
+use crate::raindex_client::vaults_list::RaindexVaultsList;
+use rain_orderbook_app_settings::yaml::YamlError;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/orders.rs around lines 294-306, the match
arm references YamlError::KeyNotFound and YamlError::NotFound but YamlError is
not imported; add an import for YamlError at the top of the file (for example:
use crate::yaml::YamlError; or import from the actual module that defines
YamlError) so the pattern match compiles, then re-run cargo build to verify.
Collaborator
Author
There was a problem hiding this comment.
this is not a compilation error
Contributor
There was a problem hiding this comment.
@findolor, you're absolutely right—I apologize for the confusion. If the code compiles successfully, then YamlError is already accessible in scope, most likely through the use super::*; wildcard import at the top of the file. The suggested import is unnecessary. Thank you for the correction!
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/js_api/src/gui/order_operations.rs:124-134
Timestamp: 2025-06-17T16:32:04.554Z
Learning: In the rain.orderbook codebase, RPC lists are typically small (2 items maximum), so performance optimizations around cloning and converting small Vec<Url> collections are generally unnecessary.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/js_api/src/raindex/mod.rs:92-99
Timestamp: 2025-06-18T18:18:44.330Z
Learning: In crates/js_api/src/raindex/mod.rs, the get_subgraph_url_for_chain method and get_multi_subgraph_args method intentionally duplicate lookup logic because they serve different purposes: get_subgraph_url_for_chain returns only the URL string, while get_multi_subgraph_args returns MultiSubgraphArgs structures containing both URL and network information (name/label). This duplication is acceptable and by design.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2099
File: crates/common/src/hyper_rpc.rs:3-7
Timestamp: 2025-08-26T14:52:37.000Z
Learning: In the rain.orderbook codebase, creating new reqwest::Client instances per request in HyperRpcClient is not considered an issue by the maintainers, despite potential performance benefits of client reuse.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/trades.rs:133-152
Timestamp: 2025-10-06T14:13:18.531Z
Learning: In the rain.orderbook codebase, the `page` parameter in `RaindexOrder::get_trades_list` method (in crates/common/src/raindex_client/trades.rs) is kept for backwards compatibility with subgraph logic, but the LocalDb fast-path intentionally returns all trades without implementing pagination.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/js_api/src/subgraph/order.rs:109-114
Timestamp: 2025-05-09T05:28:22.089Z
Learning: In the rain.orderbook project, SubgraphError contains or converts from OrderbookSubgraphClientError, so using OrderbookSubgraphClientError in error creation followed by a conversion to SubgraphError (via ? operator) is acceptable.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1950
File: crates/common/src/raindex_client/orders.rs:301-301
Timestamp: 2025-06-24T13:36:28.797Z
Learning: In the RaindexClient codebase, when Arc::new(RwLock::new(self.clone())) is used (e.g., in get_orders and get_order_by_hash methods), this creates new Arc/RwLock wrappers around a cloned RaindexClient, but the underlying client data is functionally the same. This pattern is used to provide the correct Arc<RwLock<RaindexClient>> type expected by RaindexOrder::try_from_sg_order() method.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1858
File: crates/subgraph/src/cynic_client.rs:27-29
Timestamp: 2025-05-19T12:25:02.658Z
Learning: In the Rain Orderbook codebase, when passing a URL from `self.get_base_url()` to `reqwest::Client::post()`, the `.clone()` is necessary because `get_base_url()` returns a reference (`&Url`), but an owned `Url` is needed for the reqwest client in the async context to avoid lifetime issues.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/common/src/deposit.rs:63-64
Timestamp: 2025-06-17T16:54:43.176Z
Learning: In the rain.orderbook codebase, the maintainer findolor prefers to keep the current implementation that clones the rpcs vector when calling ReadableClient::new_from_urls rather than optimizing it to avoid the clone or adding early validation for empty rpcs. Performance micro-optimizations like avoiding vector clones in this context are not a priority.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/settings/src/yaml/mod.rs:176-178
Timestamp: 2025-06-18T19:23:33.747Z
Learning: In crates/settings/src/yaml/mod.rs, the YamlError enum has two distinct error variants: `KeyNotFound(String)` for when a specific YAML key is not found in a hash/map, and `NotFound(String)` for when other types of entities (like networks, orderbooks, etc.) are not found in the configuration. These serve different purposes and should not be consolidated.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like `#[cfg(not(target_family = "wasm"))] use super::*;` in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2116
File: crates/settings/src/orderbook.rs:569-0
Timestamp: 2025-09-01T13:35:33.688Z
Learning: In crates/settings/src/orderbook.rs, the deployment-block field is required in orderbook YAML configuration and should cause a parsing error when missing, not default to any value.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1913
File: Cargo.toml:55-55
Timestamp: 2025-06-06T16:38:04.182Z
Learning: In rain.orderbook repository, during PR chains involving dependency updates, wasm-bindgen-utils may temporarily point to git commits that don't contain the full required functionality, with the understanding that subsequent PRs in the chain will update it to the correct commit.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-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
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:35:26.448Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very last part of any comment, after any learning used section or other content.
Comment on lines
801
to
+926
| impl RaindexOrder { | ||
| pub async fn fetch_dotrain_source(&mut self) -> Result<(), RaindexError> { | ||
| if self | ||
| .parsed_meta | ||
| .iter() | ||
| .any(|meta| matches!(meta, ParsedMeta::DotrainSourceV1(_))) | ||
| { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let dotrain_gui_state = match self.parsed_meta.iter().find_map(|meta| { | ||
| if let ParsedMeta::DotrainGuiStateV1(state) = meta { | ||
| Some(state.clone()) | ||
| } else { | ||
| None | ||
| } | ||
| }) { | ||
| Some(state) => state, | ||
| None => return Ok(()), | ||
| }; | ||
|
|
||
| let client = match self.get_metaboard_client()? { | ||
| Some(client) => client, | ||
| None => return Ok(()), | ||
| }; | ||
|
|
||
| let subject_hash = dotrain_gui_state.dotrain_hash(); | ||
|
|
||
| let metabytes = match client | ||
| .get_metabytes_by_subject(&MetaBigInt(alloy::hex::encode_prefixed(subject_hash))) | ||
| .await | ||
| { | ||
| Ok(bytes) => bytes, | ||
| Err(_) => return Ok(()), | ||
| }; | ||
|
|
||
| for meta_bytes in metabytes { | ||
| let documents = match RainMetaDocumentV1Item::cbor_decode(&meta_bytes) { | ||
| Ok(documents) => documents, | ||
| Err(_) => continue, | ||
| }; | ||
|
|
||
| for document in documents { | ||
| if document.magic == KnownMagic::DotrainSourceV1 { | ||
| if let Ok(source) = DotrainSourceV1::try_from(document) { | ||
| self.parsed_meta.push(ParsedMeta::DotrainSourceV1(source)); | ||
| return Ok(()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
fetch_dotrain_source best-effort behavior is reasonable; verify MetaBigInt subject format
fetch_dotrain_source:
- No‑ops if a
DotrainSourceV1is already present or if noDotrainGuiStateV1is found. - Attempts to get a metaboard client; missing/NotFound metaboards degrade to Ok(()).
- Queries metabytes by subject hash and quietly skips on subgraph/CBOR/try_from errors, enriching
parsed_metawith the first validDotrainSourceV1only.
This “best‑effort metadata enrichment” avoids breaking order fetching when metadata infra is flaky, which is a good UX trade‑off and is covered by the new tests.
One thing to double‑check: MetaBigInt(alloy::hex::encode_prefixed(subject_hash)) assumes the metaboard subgraph expects the subject as a hex string with 0x prefix. Please confirm this matches the expected encoding for rain_metaboard_subgraph::types::metas::BigInt; if it expects decimal strings instead, subjects might not match.
🌐 Web query:
What string format does `rain_metaboard_subgraph::types::metas::BigInt` expect for GraphQL BigInt inputs—hex (e.g. "0x…") or decimal?
💡 Result:
Use decimal strings. The Graph's BigInt scalar is passed as a quoted decimal (e.g. "1000000000000000000"), not "0x…" hex. [1][2]
Sources:
[1] The Graph — built-in scalar types / BigInt example.
[2] The Graph (graph-ts) — BigInt.fromString / toString and context examples showing BigInt as quoted decimal.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for rain_metaboard references or documentation in the codebase
rg -i "rain.metaboard|rain_metaboard" --type=toml --type=rs -C2
Length of output: 26
🌐 Web query:
rain_metaboard_subgraph BigInt format GraphQL schema
💡 Result:
Short answer: Use the BigInt scalar in the schema and declare fields with it (e.g. "someValue: BigInt!" or "id: BigInt!"). Most subgraph frameworks (The Graph / related indexers) expose a scalar named BigInt to represent arbitrary-size integers (up to uint256/int256); when returned to JavaScript clients large BigInt values are often serialized as strings to preserve precision. [1][2][3]
Examples:
- schema line: scalar BigInt (implicit in many subgraph toolchains). [1]
- entity field: type Event @entity { id: ID! value: BigInt! timestamp: BigInt! } . [2][3]
Sources:
[1] The Graph — Built‑in scalars and BigInt usage.
[2] Fantom / Opera GraphQL schema notes on scalar BigInt (input/output formats).
[3] Envio / HyperIndex schema docs showing BigInt as an entity scalar.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for MetaBigInt usage and subject_hash patterns in the repository
rg -n "MetaBigInt|subject_hash" --type rust -C3
Length of output: 3902
Fix MetaBigInt subject encoding: use decimal format, not hex
Line 902 in crates/common/src/raindex_client/orders.rs:
The Graph's BigInt scalar expects decimal strings (e.g., "1000000000000000000"), not hex with "0x" prefix. The code passes alloy::hex::encode_prefixed(subject_hash) (hex format), which will not match the metaboard subgraph's expectations. This causes subject lookups to fail silently—metabytes queries return empty, and DotrainSourceV1 metadata is never enriched.
Convert subject_hash to a decimal string before passing to MetaBigInt. Note: a similar issue exists at line 872 in crates/js_api/src/gui/order_operations.rs.
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/orders.rs around lines 873 to 926, the code
currently encodes subject_hash as hex (alloy::hex::encode_prefixed) before
wrapping it in MetaBigInt, but the Graph BigInt scalar expects a decimal string;
change the conversion so MetaBigInt receives the decimal representation of the
subject_hash (e.g., parse the subject_hash bytes as an unsigned big integer and
format to decimal string) instead of hex, then pass that decimal string into
MetaBigInt; apply the analogous fix in crates/js_api/src/gui/order_operations.rs
at the referenced line as well.
Collaborator
Author
There was a problem hiding this comment.
Actually, we're not really using the BigInt directly but we're using the string inside this wrapper struct, so I don't think this is an issue
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find MetaBigInt definition and usage
rg -n "struct MetaBigInt|pub struct MetaBigInt|type MetaBigInt" --type rust -A5
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find get_metabytes_by_subject implementation and usage
rg -n "get_metabytes_by_subject|fn get_metabytes_by_subject" --type rust -A10 -B2
Length of output: 2076
🏁 Script executed:
#!/bin/bash
# Look for other examples of subject hash encoding in the codebase
rg -n "subject.*encode|encode.*subject|MetaBigInt.*subject" --type rust -C3
Length of output: 2582
🏁 Script executed:
#!/bin/bash
# Check the metaboard subgraph client query construction
fd -e rs metaboard_client.rs --exec cat {}
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for rain_metaboard_subgraph imports and MetaBigInt
rg -n "use.*rain_metaboard_subgraph|from.*rain_metaboard_subgraph|rain_metaboard_subgraph::" --type rust -C2
Length of output: 1657
🏁 Script executed:
#!/bin.bash
# Look for GraphQL query templates or query strings with metaV1S
rg -n "metaV1S|query.*subject|subject.*BigInt" --type rust -C3
Length of output: 6037
🏁 Script executed:
#!/bin/bash
# Search in Cargo.toml files for rain-metaboard-subgraph dependency
fd -e toml Cargo.toml --exec grep -H "rain-metaboard-subgraph" {}
Length of output: 255
🏁 Script executed:
#!/bin/bash
# Look for tests that use get_metabytes_by_subject to see working examples
rg -n "get_metabytes_by_subject" --type rust -B5 -A15
Length of output: 3438
🏁 Script executed:
#!/bin/bash
# Look at the metaboard client source implementation
cat lib/rain.interpreter/lib/rain.metadata/crates/metaboard/src/metaboard_client.rs 2>/dev/null || echo "File not found"
Length of output: 14
🏁 Script executed:
#!/bin/bash
# Check what dotrain_hash returns and how it's used in tests
rg -n "dotrain_hash\(\)" --type rust -B2 -A2
Length of output: 2409
🏁 Script executed:
#!/bin/bash
# Look at the full test that mocks metaboard response with subject
rg -n "gui_state.dotrain_hash" --type rust -B10 -A10
Length of output: 9228
🏁 Script executed:
#!/bin/bash
# Check the types module in metaboard crate
cat lib/rain.interpreter/lib/rain.metadata/crates/metaboard/src/types/metas.rs 2>/dev/null || echo "File not found"
Length of output: 14
🏁 Script executed:
#!/bin/bash
# Find DotrainGuiStateV1 and dotrain_hash definition
rg -n "fn dotrain_hash|dotrain_hash.*->" --type rust -B3 -A5
Length of output: 680
🏁 Script executed:
#!/bin/bash
# Look in the metadata crate for DotrainGuiStateV1
find lib/rain.interpreter/lib/rain.metadata -name "*.rs" -type f | head -20
Length of output: 73
🏁 Script executed:
#!/bin/bash
# Check if there are any rust files in the metaboard submodule
find lib/rain.interpreter/lib/rain.metadata/crates/metaboard -name "*.rs" -type f 2>/dev/null | head -10
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for FixedBytes or B256 Display implementations that show hex encoding
rg -n "impl.*Display.*for.*FixedBytes|impl.*ToString.*FixedBytes" --type rust -A5
Length of output: 0
@findolor, you're absolutely right! Looking at the test evidence, I can see that the mock at lines 1652 and 1708 uses gui_state.dotrain_hash().to_string() as the subject value, and the tests pass successfully. This confirms that the metaboard subgraph expects subjects as hex-encoded strings (with "0x" prefix), not decimal BigInt values.
The MetaBigInt type is indeed just a newtype wrapper around String for type safety, and in this specific subgraph implementation, it stores hex strings rather than following The Graph's typical decimal BigInt convention. The current implementation using alloy::hex::encode_prefixed(subject_hash) is correct.
Thank you for the clarification!
🧬 Code Graph Analysis Results
[Relevant code snippets for handling the user comment]
- File: crates/common/src/raindex_client/orders.rs
-
Snippet: RaindexOrder wasm getter for timestamp_added (BigInt conversion)
-
Description: Getter that returns the timestamp as a JavaScript BigInt by converting the internal U256 timestamp_added to a string and then parsing it with BigInt. Includes error mapping to RaindexError::JsError on parse failure.
-
Exact snippet:
#[wasm_bindgen(getter, timestampAdded)]
pub fn timestamp_added(&self) -> Result<BigInt, RaindexError> {
BigInt::from_str(&self.timestamp_added.to_string())
.map_err(|e| RaindexError::JsError(e.to_string().into()))
} -
File: crates/common/src/raindex_client/orders.rs
-
Snippet: RaindexOrder non-WASM timestamp_added (U256)
-
Description: Non-WASM path returning the U256 timestamp directly (no BigInt parsing). Context: provides an alternate access pattern when not targeting wasm.
-
Exact snippet:
pub fn timestamp_added(&self) -> U256 {
self.timestamp_added
}
-
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2012
File: crates/js_api/src/registry.rs:485-512
Timestamp: 2025-07-30T07:41:39.271Z
Learning: In crates/js_api/src/registry.rs, findolor considers the current concurrent fetching of order files using futures::future::join_all without concurrency limits to be acceptable, preferring the simple approach over adding concurrency limiting mechanisms for the DotrainRegistry implementation.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/trades.rs:133-152
Timestamp: 2025-10-06T14:13:18.531Z
Learning: In the rain.orderbook codebase, the `page` parameter in `RaindexOrder::get_trades_list` method (in crates/common/src/raindex_client/trades.rs) is kept for backwards compatibility with subgraph logic, but the LocalDb fast-path intentionally returns all trades without implementing pagination.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1706
File: crates/quote/src/js_api/mod.rs:57-61
Timestamp: 2025-04-29T06:19:15.446Z
Learning: In the rain.orderbook codebase, the `get_id` function is designed to use `U256::from_str()` rather than `U256::from_hex()` for parsing order hashes, despite the potential for hex string inputs.
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/js_api/src/gui/state_management.rs:196-203
Timestamp: 2025-09-24T10:59:25.666Z
Learning: In crates/js_api/src/gui/state_management.rs, the RainMetaDocumentV1Item::hash(false) method returns a slice, so wrapping the result with FixedBytes() is necessary to convert it to the expected FixedBytes type for DotrainGuiStateV1.dotrain_hash field.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1831
File: packages/ui-components/src/lib/types/transaction.ts:71-77
Timestamp: 2025-05-19T10:39:19.157Z
Learning: In the Rain Orderbook project, `orderHash` is intentionally typed as `string` (not `Hex`) in the `InternalTransactionArgs` interface because it matches the return type of the `getRemoveOrderCalldata` WASM function which returns `Promise<string>`. String operations like `.slice()` are performed on `orderHash` in the code. Meanwhile, `txHash` is typed as `Hex` for blockchain transaction hashes.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/settings/src/yaml/orderbook.rs:371-377
Timestamp: 2025-06-17T16:21:24.384Z
Learning: In crates/settings/src/yaml/orderbook.rs tests, the user findolor considers RPC ordering in Vec<Url> assertions to be intentional and not a test brittleness issue. The ordering of RPCs in tests should be preserved as specified.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2202
File: crates/common/src/raindex_client/local_db/sync.rs:33-34
Timestamp: 2025-10-14T07:51:55.148Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the hard-coded `DEFAULT_SYNC_CHAIN_ID` constant (set to `SUPPORTED_LOCAL_DB_CHAINS[0]`) will be replaced with proper chain ID handling in downstream PRs as part of the multi-network/orderbook implementation.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1913
File: Cargo.toml:55-55
Timestamp: 2025-06-06T16:38:04.182Z
Learning: In rain.orderbook repository, during PR chains involving dependency updates, wasm-bindgen-utils may temporarily point to git commits that don't contain the full required functionality, with the understanding that subsequent PRs in the chain will update it to the correct commit.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-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
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:35:26.448Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very last part of any comment, after any learning used section or other content.
Comment on lines
395
to
415
| async fn prepare_add_order_args( | ||
| &mut self, | ||
| deployment: &GuiDeploymentCfg, | ||
| ) -> Result<AddOrderArgs, GuiError> { | ||
| let dotrain_gui_state_instance_v1 = self.generate_dotrain_gui_state_instance_v1()?; | ||
| let dotrain_gui_state_meta = | ||
| RainMetaDocumentV1Item::try_from(dotrain_gui_state_instance_v1.clone())?; | ||
|
|
||
| let dotrain_for_deployment = self | ||
| .dotrain_order | ||
| .generate_dotrain_for_deployment(&deployment.deployment.key)?; | ||
|
|
||
| let add_order_args = AddOrderArgs::new_from_deployment( | ||
| dotrain_for_deployment, | ||
| deployment.deployment.as_ref().clone(), | ||
| Some(vec![dotrain_gui_state_meta]), | ||
| ) | ||
| .await?; | ||
|
|
||
| Ok(add_order_args) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
prepare_add_order_args correctly injects GUI meta, minor clone nit
prepare_add_order_args now:
- Builds a
DotrainGuiStateV1instance from current GUI state. - Converts it into a
RainMetaDocumentV1Itemand passes it asadditional_metatoAddOrderArgs::new_from_deploymentusing the trimmed deployment dotrain. - Is reused by
generate_add_order_calldata,generate_deposit_and_add_order_calldatas, andget_deployment_transaction_args, which keeps meta injection consistent across all code paths.
This is functionally sound and well‑covered by test_prepare_add_order_args_injects_gui_meta. As a small clean‑up, dotrain_gui_state_instance_v1.clone() appears unnecessary since the original value isn’t reused after try_from; you could move it into try_from to avoid an extra allocation.
Also applies to: 520-535, 564-585
🤖 Prompt for AI Agents
In crates/js_api/src/gui/order_operations.rs around lines 395 to 415 (also apply
same change at lines 520-535 and 564-585), remove the unnecessary clone of
dotrain_gui_state_instance_v1 when converting to RainMetaDocumentV1Item; instead
pass the owned value into RainMetaDocumentV1Item::try_from (consume the
variable) so you avoid an extra allocation—i.e., stop calling .clone() and let
try_from take dotrain_gui_state_instance_v1 by value.
Comment on lines
+207
to
+209
| if let Some(select_tokens) = GuiCfg::parse_select_tokens(self.documents.clone(), key)? { | ||
| context.add_select_tokens(select_tokens.iter().map(|st| st.key.clone()).collect()); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting duplicate select-tokens logic.
This block duplicates the select-tokens parsing logic from get_order_for_gui_deployment (lines 174-178). Consider extracting this into a helper method to reduce code duplication:
fn add_select_tokens_to_context(
&self,
context: &mut Context,
deployment_key: &str,
) -> Result<(), YamlError> {
if let Some(select_tokens) = GuiCfg::parse_select_tokens(self.documents.clone(), deployment_key)? {
context.add_select_tokens(select_tokens.iter().map(|st| st.key.clone()).collect());
}
Ok(())
}Then call self.add_select_tokens_to_context(&mut context, key)?; in both methods.
🤖 Prompt for AI Agents
In crates/settings/src/yaml/dotrain.rs around lines 207-209, the select-tokens
parsing and context population is duplicated (same logic appears at ~174-178);
extract that logic into a private helper method on the impl (e.g.,
add_select_tokens_to_context(&self, context: &mut Context, deployment_key: &str)
-> Result<(), YamlError>) that calls GuiCfg::parse_select_tokens(...), maps to
keys and calls context.add_select_tokens(...), returns Ok(()), and then replace
both original blocks with calls to self.add_select_tokens_to_context(&mut
context, key)?; to remove duplication and propagate errors consistently.
Contributor
There was a problem hiding this comment.
@findolor, understood! I'll skip this refactoring suggestion.
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/cli/src/commands/order/calldata.rs:47-48
Timestamp: 2025-09-24T10:54:42.060Z
Learning: In crates/cli/src/commands/order/calldata.rs, the config_deployment variable is used both as a parameter to AddOrderArgs::new_from_deployment() and later to extract RPC information, requiring the clone() to maintain access after ownership transfer.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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
Repo: rainlanguage/rain.orderbook 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: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-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
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:35:26.448Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very last part of any comment, after any learning used section or other content.
Comment on lines
+113
to
+120
| const formatGuiState = (guiState: string) => { | ||
| try { | ||
| return JSON.stringify(JSON.parse(guiState), null, 2); | ||
| } catch { | ||
| errToast('Failed to parse GUI state'); | ||
| return 'Invalid GUI state'; | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
GUI state formatting is solid; minor optional tweak around <pre> whitespace.
The formatGuiState helper (Lines 113–120) is a good defensive wrapper: it pretty-prints valid JSON and surfaces a toast plus a fallback string for invalid data. That’s a nice UX for debugging GUI meta.
One minor optional improvement: because the JSON is rendered inside a <pre> with indented Svelte markup (Lines 293–295), the leading newline/indentation from the template will be preserved and can shift the JSON to the right. If you care about exact formatting, you can inline the expression to avoid extra whitespace:
<pre class="text-sm" data-testid="gui-state-json">{formatGuiState(data.dotrainGuiState)}</pre>This doesn’t affect correctness, just presentation, so feel free to leave as-is if it matches existing patterns.
Also applies to: 289-295
🤖 Prompt for AI Agents
In packages/ui-components/src/lib/components/detail/OrderDetail.svelte around
lines 113-120 and 289-295, the formatGuiState helper is fine but the JSON output
inside an indented multi-line <pre> picks up template whitespace; to fix, render
the formatted JSON inline on the same line as the <pre> tag to avoid the leading
newline/indentation (replace the multi-line <pre> block with a single-line <pre
class="text-sm"
data-testid="gui-state-json">{formatGuiState(data.dotrainGuiState)}</pre>),
leaving formatGuiState unchanged.
Contributor
There was a problem hiding this comment.
@findolor, sounds good! I'll keep the formatting as-is.
🧠 Learnings used
Learnt from: hardyjosh
Repo: rainlanguage/rain.orderbook PR: 2035
File: packages/ui-components/src/lib/components/tables/OrdersListTable.svelte:167-174
Timestamp: 2025-07-26T06:56:28.207Z
Learning: In OrdersListTable.svelte, maintainer hardyjosh considers keyed each-blocks unnecessary for item.inputs and item.outputs arrays because the entire query result updates at once during refresh, making DOM diffing optimizations negligible when the whole dataset is replaced rather than individual items being modified.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1911
File: crates/subgraph/src/vault_balance_changes_query.rs:133-135
Timestamp: 2025-07-23T13:03:40.669Z
Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1975
File: crates/js_api/src/gui/state_management.rs:412-412
Timestamp: 2025-07-09T14:00:12.206Z
Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T23:09:27.578Z
Learning: For PR #1884 in rainlanguage/rain.orderbook repository, I must always reassess the PR size after each new commit, calculating the total changes up to the very latest commit and ensuring accuracy of the report. The calculation must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and report in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number".
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-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
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1907
File: packages/orderbook/test/common/test.test.ts:75-77
Timestamp: 2025-06-04T10:21:01.388Z
Learning: The DotrainOrder.create API in packages/orderbook/test/common/test.test.ts is internal and not used directly in consumer applications, so API changes here don't require external breaking change documentation.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:35:26.448Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very last part of any comment, after any learning used section or other content.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/webapp/src/__tests__/handleAddOrder.test.ts (1)
140-361: Approval + metadata + deployment flows are well covered; stubbing sequence is slightly non-obviousThe “one approval”, “multiple approvals”, and “different txHashes” tests now:
- enforce the modal ordering (approvals → metadata → deployment),
- assert correct modal titles and args, including the metadata step,
- and explicitly call each
onConfirmwith distinct hashes, proving that each transaction creator uses thetxHashargument passed intoonConfirm.One minor readability nit: in the multiple-approvals test (Lines 273-278 and 281-283), the comment mentions “2 approvals + metadata + deployment”, but there are only three
mockResolvedValueOncecalls and the fourth modal relies on the default resolver frombeforeEach. This works correctly but may surprise future readers; adding a fourth explicitmockResolvedValueOnceor tweaking the comment would make the intent clearer. Given everything is passing, this is strictly optional.Also applies to: 407-476
crates/js_api/src/gui/order_operations.rs (2)
520-595: Add‑order calldata and multicall paths now consistently use meta‑awareAddOrderArgsRouting both
generateAddOrderCalldataandgenerateDepositAndAddOrderCalldatasthroughprepare_add_order_args+try_into_callavoids drift between the simple and multicall flows and guarantees GUI meta is always attached when generating add‑order calls.If you later want to trim a bit of work, you could consider letting
generate_deposit_calldatasaccept a pre‑preparedGuiDeploymentCfgto avoid re‑runningprepare_calldata_generation, but it’s not necessary given the expected small config sizes. Based on learnings, this repo generally prefers simplicity over micro‑optimizations.
802-879: Metaboard subgraph now sits on the deployment critical path – confirm this trade‑off
get_deployment_transaction_argsnow always goes throughshould_emit_meta_calland thenget_metaboard_client/get_metaboard_addresseswhen the GUI state indicates we should emit meta. Any YAML misconfig, missing metaboard entry, subgraph error, or “no address” response will surface as aGuiErrorand block order deployment, even though metadata is conceptually optional.If that’s intentional (i.e. deployments must only happen when metaboard infra is healthy and configured), this is fine; otherwise you may want to catch these errors and fall back to
emit_meta_call: Noneso users can still deploy orders without metadata during metaboard outages or when no metaboard is configured.crates/common/src/raindex_client/orders.rs (2)
74-78: parsed_meta field and dotrain getters are wired correctly; minor clone overhead only
parsed_meta: Vec<ParsedMeta>is properly initialized in all constructors and exposed via wasm/non‑wasm getters, and thedotrain_source/dotrain_gui_statehelpers correctly project out specific variants and JSON‑encode the GUI state.The only downside is that
dotrain_source/dotrain_gui_statecallparsed_meta()and clone the whole vector before searching; if this becomes hot code, you could switch those two to iterate over&self.parsed_metadirectly to avoid the extra allocation, while keeping the existingparsed_meta()getter for callers that really need an ownedVec.Also applies to: 128-149, 208-225
928-995: parsed_meta initialization from hex meta looks correct; double‑decode is the only minor costBoth
try_from_sg_orderandtry_from_local_dbnow:
- Preserve the existing
metahex string (viaBytes::from_str).- Independently hex‑decode the same string and run
ParsedMeta::parse_from_bytes, defaulting to an empty vec on error.- Keep the previous rainlang decoding behavior intact.
Functionally this is sound and aligns parsed metadata with the raw
metafield. The only inefficiency is decoding the same hex twice (once intoBytes, once intoVec<u8>for parsing); if meta blobs get large or frequent, you could refactor to decode once and reuse the bytes for bothBytesandParsedMetaconstruction, but it’s not critical.Also applies to: 1030-1094
♻️ Duplicate comments (1)
crates/common/src/raindex_client/orders.rs (1)
874-926: fetch_dotrain_source implements safe, best‑effort enrichment; consider minimal diagnosticsThe method:
- No‑ops if a
DotrainSourceV1is already present or if noDotrainGuiStateV1is found.- Obtains a metaboard client if configured; otherwise it returns
Ok(()).- Queries metabytes by subject hash and silently skips on subgraph, decode, or
DotrainSourceV1::try_fromerrors, adding the first valid source document and then returning.This is a sensible “don’t break order fetching for metadata issues” strategy. The only trade‑off is that failures in the metaboard call or CBOR decoding are completely silent; if debugging such issues becomes important, adding a low‑level log (behind whatever logging infra you use) when these error branches are hit would make troubleshooting easier without changing behavior.
Comment on lines
+262
to
+293
| pub fn try_into_emit_meta_call(&self) -> Result<Option<emitMetaCall>, AddOrderArgsError> { | ||
| match self.additional_meta.as_ref() { | ||
| Some(meta_docs) => { | ||
| match meta_docs | ||
| .iter() | ||
| .find(|document| document.magic == KnownMagic::DotrainGuiStateV1) | ||
| { | ||
| Some(doc) => { | ||
| let gui_state = DotrainGuiStateV1::try_from(doc.clone())?; | ||
| let subject_hash = gui_state.dotrain_hash(); | ||
| let meta_document = RainMetaDocumentV1Item { | ||
| payload: ByteBuf::from(self.dotrain.as_bytes()), | ||
| magic: KnownMagic::DotrainSourceV1, | ||
| content_type: ContentType::OctetStream, | ||
| content_encoding: ContentEncoding::None, | ||
| content_language: ContentLanguage::None, | ||
| }; | ||
| let meta = RainMetaDocumentV1Item::cbor_encode_seq( | ||
| &vec![meta_document.clone()], | ||
| KnownMagic::RainMetaDocumentV1, | ||
| )?; | ||
| Ok(Some(emitMetaCall { | ||
| subject: subject_hash, | ||
| meta: Bytes::copy_from_slice(&meta), | ||
| })) | ||
| } | ||
| None => Ok(None), | ||
| } | ||
| } | ||
| None => Ok(None), | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
try_into_emit_meta_call behavior matches GUI‑state‑driven emit‑meta design, tiny allocation nit
This function cleanly:
- Finds a
DotrainGuiStateV1inadditional_meta. - Derives the subject from its
dotrain_hash. - Builds a single‑item
RainMetaDocumentV1containing aDotrainSourceV1forself.dotrain. - Returns
Ok(Some(emitMetaCall)), orOk(None)when no GUI state is present, or propagates decode errors viaRainMetaError.
The accompanying tests cover both the happy path and invalid GUI state payloads. As a minor micro‑refactor, you could avoid allocating a temporary Vec by using a slice:
- let meta = RainMetaDocumentV1Item::cbor_encode_seq(
- &vec![meta_document.clone()],
- KnownMagic::RainMetaDocumentV1,
- )?;
+ let meta = RainMetaDocumentV1Item::cbor_encode_seq(
+ std::slice::from_ref(&meta_document),
+ KnownMagic::RainMetaDocumentV1,
+ )?;Behavior stays the same, with slightly less allocation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn try_into_emit_meta_call(&self) -> Result<Option<emitMetaCall>, AddOrderArgsError> { | |
| match self.additional_meta.as_ref() { | |
| Some(meta_docs) => { | |
| match meta_docs | |
| .iter() | |
| .find(|document| document.magic == KnownMagic::DotrainGuiStateV1) | |
| { | |
| Some(doc) => { | |
| let gui_state = DotrainGuiStateV1::try_from(doc.clone())?; | |
| let subject_hash = gui_state.dotrain_hash(); | |
| let meta_document = RainMetaDocumentV1Item { | |
| payload: ByteBuf::from(self.dotrain.as_bytes()), | |
| magic: KnownMagic::DotrainSourceV1, | |
| content_type: ContentType::OctetStream, | |
| content_encoding: ContentEncoding::None, | |
| content_language: ContentLanguage::None, | |
| }; | |
| let meta = RainMetaDocumentV1Item::cbor_encode_seq( | |
| &vec![meta_document.clone()], | |
| KnownMagic::RainMetaDocumentV1, | |
| )?; | |
| Ok(Some(emitMetaCall { | |
| subject: subject_hash, | |
| meta: Bytes::copy_from_slice(&meta), | |
| })) | |
| } | |
| None => Ok(None), | |
| } | |
| } | |
| None => Ok(None), | |
| } | |
| } | |
| pub fn try_into_emit_meta_call(&self) -> Result<Option<emitMetaCall>, AddOrderArgsError> { | |
| match self.additional_meta.as_ref() { | |
| Some(meta_docs) => { | |
| match meta_docs | |
| .iter() | |
| .find(|document| document.magic == KnownMagic::DotrainGuiStateV1) | |
| { | |
| Some(doc) => { | |
| let gui_state = DotrainGuiStateV1::try_from(doc.clone())?; | |
| let subject_hash = gui_state.dotrain_hash(); | |
| let meta_document = RainMetaDocumentV1Item { | |
| payload: ByteBuf::from(self.dotrain.as_bytes()), | |
| magic: KnownMagic::DotrainSourceV1, | |
| content_type: ContentType::OctetStream, | |
| content_encoding: ContentEncoding::None, | |
| content_language: ContentLanguage::None, | |
| }; | |
| let meta = RainMetaDocumentV1Item::cbor_encode_seq( | |
| std::slice::from_ref(&meta_document), | |
| KnownMagic::RainMetaDocumentV1, | |
| )?; | |
| Ok(Some(emitMetaCall { | |
| subject: subject_hash, | |
| meta: Bytes::copy_from_slice(&meta), | |
| })) | |
| } | |
| None => Ok(None), | |
| } | |
| } | |
| None => Ok(None), | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/common/src/add_order.rs around lines 262 to 293, replace the temporary
Vec allocation used to pass a single RainMetaDocumentV1Item into cbor_encode_seq
with a fixed-size array or slice to avoid the extra allocation and the
unnecessary clone: create the meta_document as before, then pass a slice
reference (e.g. an array containing that one item) to cbor_encode_seq instead of
Vec::from or vec![meta_document.clone()], and remove the clone from the call so
the single item is borrowed/moved directly into the encoder.
Contributor
There was a problem hiding this comment.
@findolor, understood! I'll leave the current implementation as-is.
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/cli/src/commands/order/calldata.rs:47-57
Timestamp: 2025-06-17T16:46:19.035Z
Learning: In the CLI command `crates/cli/src/commands/order/calldata.rs`, the user prefers to let lower-level errors from `try_into_call()` bubble up when the RPCs list is empty, rather than adding early validation checks with custom error messages.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/js_api/src/gui/state_management.rs:196-203
Timestamp: 2025-09-24T10:59:25.666Z
Learning: In crates/js_api/src/gui/state_management.rs, the RainMetaDocumentV1Item::hash(false) method returns a slice, so wrapping the result with FixedBytes() is necessary to convert it to the expected FixedBytes type for DotrainGuiStateV1.dotrain_hash field.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/raindex_client/add_orders.rs:132-132
Timestamp: 2025-07-04T10:24:29.750Z
Learning: In the rain.orderbook codebase, findolor prefers to use `unwrap_or("")` for optional frontmatter in dotrain parsing, allowing empty strings to be handled by downstream validation in `NewConfig::try_from_yaml()` rather than adding explicit early validation checks at the extraction point.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-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
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:35:26.448Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very last part of any comment, after any learning used section or other content.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dependent PR
Motivation
See issues:
Solution
We now emit additional meta during the order deployment. During the order listing and single order fetching we parse the meta field and show original dotrain and gui state.
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.