Handle next trade data on release #420
Conversation
|
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 48 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 (1)
WalkthroughThis pull request introduces changes to the Mostro trading platform, focusing on modifications in dependency management and order processing logic. The Changes
Possibly related PRs
Poem
Finishing Touches
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
🔭 Outside diff range comments (1)
src/app/release.rs (1)
Line range hint
255-261: PotentialSendissue with&mut orderin spawned async taskPassing
&mut orderintopayment_successwithin anasync moveblock spawned bytokio::spawnmay lead to a compile-time error because&mut orderdoes not have a'staticlifetime, andOrdermay not implementSend.Consider cloning the
orderbefore the async block or restructuring the code to avoid moving a mutable reference into the spawned task:- tokio::spawn(payment); + let order_clone = order.clone(); + tokio::spawn(async move { + // Use order_clone inside the async block + // ... + let _ = payment_success( + &mut order_clone, + &buyer_pubkey, + &seller_pubkey, + &my_keys, + request_id, + ) + .await; + // ... + });Ensure that
OrderimplementsCloneandSendtraits.
🧹 Nitpick comments (1)
src/app/release.rs (1)
149-149: Avoid shadowing variablenext_trade_pubkeyThe variable
next_trade_pubkeyis first defined as aStringand later shadowed as aPublicKey. This can lead to confusion or errors.Consider renaming the second variable to avoid shadowing:
// At line 157, rename the variable to avoid shadowing - let next_trade_pubkey = PublicKey::from_str(&next_trade_pubkey)?; + let next_trade_public_key = PublicKey::from_str(&next_trade_pubkey)?;Also applies to: 157-157
📜 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 (3)
Cargo.toml(1 hunks)src/app/admin_settle.rs(0 hunks)src/app/release.rs(4 hunks)
💤 Files with no reviewable changes (1)
- src/app/admin_settle.rs
✅ 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 (1)
src/app/release.rs (1)
70-73: Efficient extraction ofnext_tradeinformationThe code correctly extracts the
next_tradeinformation from the message payload using pattern matching.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/release.rs (1)
156-159:⚠️ Potential issueCritical: Inconsistency between intention and implementation
The comment on line 156 indicates the intention to create a new order, but the code on line 158 updates the existing child_order instead. This matches a previously identified issue and could lead to data inconsistency.
Fix this by creating the new order instead of updating:
- child_order.update(&pool).await?; + new_order.create(&pool).await?;
🧹 Nitpick comments (1)
src/app/release.rs (1)
148-154: Consider optimizing clone operationsMultiple clone operations and type conversions could be optimized:
- let mut child_order = child_order.clone(); - let next_trade_pubkey = next_trade_pubkey.to_string(); - child_order.seller_pubkey = Some(next_trade_pubkey.clone()); - child_order.creator_pubkey = next_trade_pubkey.clone(); - let next_trade_index = Some(next_trade_index as i64); - child_order.trade_index_seller = next_trade_index; + let mut new_order = child_order.as_new_order(); + new_order.seller_pubkey = Some(next_trade_pubkey.to_string()); + new_order.creator_pubkey = next_trade_pubkey.to_string(); + new_order.trade_index_seller = Some(next_trade_index as i64);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/release.rs(4 hunks)
🔇 Additional comments (2)
src/app/release.rs (2)
70-73: Clean implementation of next trade data handling!The pattern matching implementation is type-safe and handles the optional nature of next trade data appropriately.
257-257: Well-structured payment success handling!The simplified payment success logic maintains proper error handling while ensuring all necessary notifications and updates are performed.
Summary by CodeRabbit
Release Notes
Dependency Update
mostro-coredependency to version 0.6.24Order Processing Improvements
Bug Fixes
The release focuses on refining order management and trade processing capabilities.