Add missing cantdo reasons on cantdo msgs #407
Conversation
Bug fixes bumps mostro core version
WalkthroughThis pull request introduces a series of updates across multiple files in the Rust project, primarily focusing on enhancing error handling by incorporating the Changes
Possibly related PRs
Poem
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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/app/take_buy.rs (1)
41-47: Evaluate theInvalidPubkeyreason for taking one’s own order.
UsingCantDoReason::InvalidPubkeyto handle the maker’s attempt to take their own order might be semantically imprecise. Consider introducing a more fitting variant (e.g.,CantDoReason::OwnOrder) if you want clearer semantics in your code.src/app/take_sell.rs (1)
42-48: Consider a more specificCantDoReason.
As with the take-buy flow, returningCantDoReason::InvalidPubkeyfor an attempt to take one’s own order may be misleading. A dedicated reason (e.g.,CantDoReason::OwnOrder) could improve clarity.src/util.rs (1)
640-640: Improve test coverage for the new error reason.This is a good replacement of
CantDoReason::InvalidPeerwithCantDoReason::InvalidPubkey, which clarifies the condition being violated. As a follow-up, consider adding or expanding unit/integration tests to ensure that the code properly handles and reports this newly specified reason.
📜 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 (12)
Cargo.toml(1 hunks)src/app.rs(1 hunks)src/app/admin_add_solver.rs(2 hunks)src/app/admin_take_dispute.rs(2 hunks)src/app/cancel.rs(3 hunks)src/app/dispute.rs(2 hunks)src/app/fiat_sent.rs(2 hunks)src/app/order.rs(3 hunks)src/app/rate_user.rs(1 hunks)src/app/take_buy.rs(2 hunks)src/app/take_sell.rs(1 hunks)src/util.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Cargo.toml
🔇 Additional comments (17)
src/app/admin_add_solver.rs (2)
5-5: New import looks good.
The newly introduced CantDoReason import aligns well with the subsequent usage in send_cant_do_msg.
38-44: Consider verifying the optional parameters for send_cant_do_msg.
Currently, you're passing None for the order ID, and Some(CantDoReason::InvalidPubkey) for the reason. While this makes sense for an invalid key scenario, double-check if you want to include additional context (e.g., an order or trade index) to facilitate debugging or tracking the error downstream.
src/app/fiat_sent.rs (2)
4-4: Import usage verified.
The addition of CantDoReason is consistent with the approach for providing detailed error messaging.
49-55: Check reason alignment with business requirements.
The use of CantDoReason::InvalidPubkey accurately flags the discrepancy between the buyer’s actual pubkey and the one tied to the order. Ensure this is the intended reason for blocking the action.
src/app/take_buy.rs (1)
7-7: Import statement is coherent.
Adding CantDoReason matches your need to provide more descriptive error messaging.
src/app/admin_take_dispute.rs (2)
7-7: Good import for enhanced error reporting.
Importing CantDoReason here neatly paves the way for more descriptive error messages, boosting usability and debuggability.
77-83: Specific error reason adds clarity.
By specifying CantDoReason::InvalidPubkey instead of a more generic reason or None, the error handling becomes clearer and more informative for users. This is a beneficial step for improved maintainability and quick troubleshooting.
src/app/rate_user.rs (1)
83-89: Concise and informative error message.
Providing CantDoReason::InvalidOrderStatus gives a clear indication of why the rate operation fails. This direct mapping between status checks and error messages is helpful for both developers and end users.
src/app/dispute.rs (2)
14-14: Import aligns with overall improvements.
Including the CantDoReason import strongly complements the broader pattern of more descriptive error handling introduced throughout the codebase.
187-193: Specific invalid pubkey handling.
Using CantDoReason::InvalidPubkey precisely informs users and maintainers about the cause of the dispute initiation failure. This is a great improvement in clarity relative to more generic error feedback.
src/app.rs (1)
140-140: New reason clarifies user creation failures.
Replacing a less specific reason with CantDoReason::CantCreateUser is a good move toward transparent error reporting, as it accurately conveys the root cause of the failure scenario when creating new users.
src/app/order.rs (3)
5-5: Explicit import of CantDoReason.
Including CantDoReason in the import statement aligns it with other code sections that require detailed reasons for non-executable operations. This is consistent and clear.
52-58: Appropriate use of CantDoReason::InvalidAmount.
Sending a CantDoReason::InvalidAmount message when min >= max is a clearer error explanation, preventing confusion for the end user. This is a good improvement for error reporting logic.
81-88: Validate contradiction between premium and fiat_amount.
Rejecting orders that incorrectly specify both premium and a fiat_amount ensures the logic remains consistent. However, if you anticipate a scenario where both might coexist in the future, consider documenting that or revisiting the validation approach as requirements evolve.
src/app/cancel.rs (3)
9-9: Add CantDoReason to improve explanatory error messages.
Importing CantDoReason here creates consistent and informative error handling across the application.
43-55: Enhanced flow for already-canceled orders.
Proactively detecting when an order is already canceled and communicating that via send_cant_do_msg prevents confusing re-cancellation attempts. This is a neat improvement in user feedback.
92-92: Restrict cancellation condition to WaitingBuyerInvoice.
The added check ensures that the cancel_add_invoice routine is only executed under the correct status, mitigating edge cases. This approach further refines logic flow for sellers.
Bug fixes
bumps mostro core version
Summary by CodeRabbit
New Features
Bug Fixes
Documentation