Update localdb id range order #104
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces several modifications across different files in the mostro-cli project. The changes primarily focus on enhancing message handling, error reporting, and key generation functionality. In Changes
Possibly Related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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
🧹 Nitpick comments (2)
src/cli/send_msg.rs (2)
45-45: Consider using a full 128-bit identifier for uniqueness
Truncating UUID to 64 bits is typically fine, but consider using the entire UUID to further reduce collision risk where it’s critical to guarantee request uniqueness.
67-114: Refactor logic to handle matching response and order creation more cleanly
This block uses a multi-step chain withfind_mapreturning(Option<SmallOrder>, Option<i64>). If code complexity grows or more response types need handling, consider refactoring into dedicated helper methods that:
- Match the request ID.
- Validate the presence of a
NewOrdermessage.- Conditionally create a DB order and update the user’s trade index.
Separating these steps into smaller functions can reduce future maintenance overhead and clarify the control flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.toml(2 hunks)src/cli/send_msg.rs(3 hunks)src/db.rs(1 hunks)src/util.rs(0 hunks)
💤 Files with no reviewable changes (1)
- src/util.rs
✅ Files skipped from review due to trivial changes (1)
- Cargo.toml
🔇 Additional comments (2)
src/cli/send_msg.rs (2)
48-48: Message creation looks good
No issues found—this is a clean way to include the newly generated request ID in your order message.
57-57: Synchronous message sending call appears correct
The code handles potential errors via ?, and the usage of send_message_sync is clearly structured.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/cli/get_dm.rs (1)
68-72: Good error messaging forCantDopayload.
Printing thecant_do_reasonhelps with debugging. Consider adding user-friendly guidance or next steps for the user if applicable.src/cli/new_order.rs (1)
147-160: Refactor repeatedAction::CantDohandling
The approach here is clean for handling specific error reasons withAction::CantDo. However, since similar logic is repeated in other CLI modules, consider extracting a helper function to reduce duplication and maintain consistent behavior across the codebase.src/cli/take_buy.rs (1)
68-81: Consistent error case checks
You're correctly switching oncant_do_reasonfor more granular error reporting. For maintainability, consider refactoring similar checks across modules into a shared function.src/cli/take_sell.rs (1)
111-125: Extract shared code for user updates
This block for updating user data and trade indexes appears in multiple places. Centralizing it into a helper function would improve readability and reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
src/cli.rs(1 hunks)src/cli/get_dm.rs(2 hunks)src/cli/new_order.rs(2 hunks)src/cli/take_buy.rs(2 hunks)src/cli/take_sell.rs(3 hunks)src/db.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/db.rs
🔇 Additional comments (8)
src/cli/get_dm.rs (3)
5-9: Imports look good, providing new functionality for message matching.
It appears you're now using Action, Message, and Payload from mostro_core::message. This is appropriate for the extended match logic below.
16-16: Consider validating or documenting the new trade_index parameter.
You’ve introduced trade_index to the function signature without explicit validation or explanation of acceptable ranges. A quick check (e.g., ensuring it’s non-negative) or clarifying in documentation could help avoid edge-case errors.
73-91: Check database error for “order not found” vs. other errors.
Currently, is_err() is used to judge whether to insert a new Order. However, a database connectivity error or other failure also satisfies is_err(), which might lead you to erroneously create a duplicate order. Distinguishing “not found” from “DB error” could improve robustness.
src/cli.rs (1)
359-359: Ensure consistent usage of the added trade_index parameter.
Passing trade_index to execute_get_dm aligns the call site with the updated function signature. Double-check that no other parts of the codebase rely on the old signature.
src/cli/new_order.rs (1)
2-2: Good use of import for structured error handling
Bringing in CantDoReason from mostro_core::message helps keep your code consistent with the extended error handling approach across the codebase.
src/cli/take_buy.rs (1)
2-2: New import aligns with updated error structure
Importing CantDoReason neatly supports handling customized error reasons.
src/cli/take_sell.rs (2)
3-3: Import for extended error handling
Including CantDoReason is consistent with the newly introduced Action::CantDo pattern.
87-100: Specialized handling for CantDo
The structure here properly distinguishes known reasons (e.g., out-of-range amounts) from unknown ones. It mirrors the approach elsewhere, helping unify error handling.
trade index and trade key on child order
Summary by CodeRabbit
New Features
Bug Fixes
Chores