Handle next trade on fiatsent#421
Conversation
Add next_trade_pubkey and next_trade_index fields to order table, this fields should be sent when the maker of a range order is a buyer, mostrod needs these fields to create the new child order Implement logic of saving next trade fields after fiat-sent action by buyer Code refactoring to optimize some parts of the code Add logic on release to use next trade fields in order when the maker of the range order is the buyer Update mostro-core version
|
Warning Rate limit exceeded@grunch has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request introduces updates to the Mostro project, focusing on trade-related functionality and database management. The changes include updating the Changes
Sequence DiagramsequenceDiagram
participant User
participant App
participant Database
User->>App: Initiate Trade
App->>Database: Retrieve Order
Database-->>App: Order Details
App->>App: Check Trade Conditions
App->>Database: Update Order with Next Trade Info
Database-->>App: Confirmation
App->>User: Send Trade Update Message
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/fiat_sent.rs (1)
100-107: Add a comment explaining the next trade fields usage.While the implementation is correct, adding a comment would help explain why these fields are only updated when the buyer is the maker.
+ // Update next trade fields only when the buyer is the maker of a range order + // These fields will be used to create the next child order in the range if order.creator_pubkey == event.rumor.pubkey.to_string() && next_trade.is_some() {src/app/release.rs (2)
75-91: Enhance error messages for better debugging.While the error handling is improved, the error messages could be more descriptive.
- .ok_or(Error::msg("No order id"))?; + .ok_or(Error::msg("Order ID is required but was not provided"))?; - .ok_or(Error::msg("Order not found"))?; + .ok_or(Error::msg(format!("Order {} not found in database", order_id)))?; - .ok_or(Error::msg("No seller pubkey found"))?; + .ok_or(Error::msg(format!("Seller public key not found for order {}", order_id)))?;
170-205: Add documentation for the handle_child_order function.The new helper function would benefit from documentation explaining its purpose and parameters.
+/// Manages the creation and update of child orders in a range order sequence +/// +/// # Arguments +/// * `child_order` - The child order to be created/updated +/// * `order` - The parent order +/// * `next_trade` - Optional tuple of (pubkey, index) for the next trade +/// * `pool` - Database connection pool +/// * `request_id` - Optional request ID for messaging +/// +/// # Returns +/// Result indicating success or failure of the operation async fn handle_child_order(
📜 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 (5)
Cargo.toml(1 hunks)migrations/20221222153301_orders.sql(1 hunks)src/app/fiat_sent.rs(3 hunks)src/app/release.rs(2 hunks)src/db.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tests
🔇 Additional comments (4)
migrations/20221222153301_orders.sql (1)
40-42: LGTM! Schema changes look good.The new columns
next_trade_pubkeyandnext_trade_indexare well-defined with appropriate types and constraints, consistent with the existing schema design.src/app/fiat_sent.rs (2)
Line range hint
27-33: LGTM! Proper error handling for order retrieval.The mutable order retrieval with proper error handling looks good.
57-60: LGTM! Clean payload extraction.The next trade extraction from payload is well-implemented using pattern matching.
src/db.rs (1)
37-42: LGTM! Improved logging accuracy.Moving the success log message inside the migration success block ensures it only appears when the database is fully initialized.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Add next_trade_pubkey and next_trade_index fields to order table, this fields should be sent when the maker of a range order is a buyer, mostrod needs these fields to create the new child order
Implement logic of saving next trade fields after fiat-sent action by buyer
Code refactoring to optimize some parts of the code
Add logic on release to use next trade fields in order when the maker of the range order is the buyer
Update mostro-core version
Summary by CodeRabbit
Release Notes
Dependencies
mostro-coredependency to version 0.6.25Database
next_trade_pubkeynext_trade_indexImprovements