Remove seller/buyer token#133
Conversation
WalkthroughBumped Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CLI as mostro-cli
participant MC as mostro-core (0.6.49)
participant DB as Local DB
rect rgb(240,248,255)
note over U,CLI: Fetch Direct Messages
U->>CLI: run get-dm
CLI->>MC: get_direct_messages(...)
MC-->>CLI: Messages incl. Payload::Dispute(id, info)
CLI->>CLI: match Dispute (id, info) — token removed
CLI-->>U: print id and optional info
end
rect rgb(245,255,240)
note over U,CLI: Create New Order
U->>CLI: run new-order ...
CLI->>CLI: SmallOrder::new(..., expires_at) %% trailing None args removed
CLI->>DB: INSERT Order (columns exclude buyer_token/seller_token)
DB-->>CLI: OK
CLI-->>U: Order created
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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/db.rs (1)
437-443: Tighten results for trade_keys (filter empties; optional ordering).Not critical, but helps produce a stable, clean list.
- let rows = sqlx::query_as::<_, TradeKeyRow>( - "SELECT DISTINCT trade_keys FROM orders WHERE trade_keys IS NOT NULL", - ) + let rows = sqlx::query_as::<_, TradeKeyRow>( + "SELECT DISTINCT trade_keys FROM orders WHERE trade_keys IS NOT NULL ORDER BY trade_keys", + ) .fetch_all(pool) .await?; - let trade_keys: Vec<String> = rows.into_iter().filter_map(|row| row.trade_keys).collect(); + let trade_keys: Vec<String> = rows + .into_iter() + .filter_map(|row| row.trade_keys) + .filter(|s| !s.trim().is_empty()) + .collect();src/cli/get_dm.rs (1)
68-76: Print Dispute info without Option wrapper.Minor UX tweak: avoid printing
Some(...)by unwrapping when present.- Payload::Dispute(id, info) => { + Payload::Dispute(id, info) => { println!("Action: {}", message.action); println!("Dispute id: {}", id); - if info.is_some() { - println!(); - println!("Dispute info: {:#?}", info); - println!(); - } + if let Some(info) = info { + println!(); + println!("Dispute info: {}", info); + println!(); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.toml(1 hunks)src/cli/get_dm.rs(3 hunks)src/cli/new_order.rs(0 hunks)src/db.rs(3 hunks)
💤 Files with no reviewable changes (1)
- src/cli/new_order.rs
🧰 Additional context used
🧬 Code graph analysis (1)
src/cli/get_dm.rs (1)
src/util.rs (1)
get_direct_messages(281-386)
🔇 Additional comments (4)
Cargo.toml (1)
42-42: Dependency bump to mostro-core 0.6.49 looks good.Matches the Payload::Dispute signature change in code. Ensure Cargo.lock is updated accordingly in the PR if it's tracked.
src/db.rs (1)
353-353: Update statement mapping is consistent.SET columns align with subsequent binds; no token fields remain. Looks correct.
src/cli/get_dm.rs (2)
24-25: Line wrap/readability only.No semantic change; fine as-is.
36-37: Admin path call formatting only.No functional change; looks good.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/db.rs (2)
350-372: Don’t update created_at on UPDATE; keep it immutable.created_at should be write-once. Recommend dropping it from the UPDATE SET/binds.
SET kind = ?, status = ?, amount = ?, fiat_code = ?, min_amount = ?, max_amount = ?, fiat_amount = ?, payment_method = ?, premium = ?, trade_keys = ?, counterparty_pubkey = ?, - is_mine = ?, buyer_invoice = ?, created_at = ?, expires_at = ? + is_mine = ?, buyer_invoice = ?, expires_at = ? WHERE id = ? @@ .bind(&self.counterparty_pubkey) .bind(self.is_mine) .bind(&self.buyer_invoice) - .bind(self.created_at) .bind(self.expires_at) .bind(id)
437-443: Simplify DISTINCT trade_keys fetch with query_scalar + flatten.Removes the ad-hoc row type and one pass over the Vec.
- #[derive(sqlx::FromRow)] - struct TradeKeyRow { - trade_keys: Option<String>, - } - - let rows = sqlx::query_as::<_, TradeKeyRow>( - "SELECT DISTINCT trade_keys FROM orders WHERE trade_keys IS NOT NULL", - ) - .fetch_all(pool) - .await?; - - let trade_keys: Vec<String> = rows.into_iter().filter_map(|row| row.trade_keys).collect(); + let trade_keys: Vec<String> = sqlx::query_scalar::<_, Option<String>>( + "SELECT DISTINCT trade_keys FROM orders WHERE trade_keys IS NOT NULL", + ) + .fetch_all(pool) + .await? + .into_iter() + .flatten() + .collect();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/db.rs(3 hunks)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/db.rs (1)
256-258: Placeholders now match columns (17 = 17).This resolves the earlier runtime bind-count mismatch on INSERT. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/db.rs (1)
487-499: Simplify: fetch DISTINCT trade_keys via query_scalar and drop the ad-hoc row type.Reduces code and avoids Option since the WHERE already excludes NULLs.
- #[derive(sqlx::FromRow)] - struct TradeKeyRow { - trade_keys: Option<String>, - } - - let rows = sqlx::query_as::<_, TradeKeyRow>( - "SELECT DISTINCT trade_keys FROM orders WHERE trade_keys IS NOT NULL", - ) - .fetch_all(pool) - .await?; - - let trade_keys: Vec<String> = rows.into_iter().filter_map(|row| row.trade_keys).collect(); + let trade_keys: Vec<String> = sqlx::query_scalar::<_, String>( + "SELECT DISTINCT trade_keys FROM orders WHERE trade_keys IS NOT NULL", + ) + .fetch_all(pool) + .await?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/db.rs(4 hunks)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/db.rs (4)
67-70: Good: migration invoked only for existing databases.This keeps the fresh-install path clean and avoids unnecessary checks.
313-315: Placeholder count now matches the 17 columns.Previous mismatch is resolved. Looks good.
410-410: Update SET/binds alignment looks correct.14 placeholders before WHERE + 1 for id; 15 binds provided. No issues.
75-127: Add version-aware migration with rebuild fallback for SQLite < 3.35. Python’s sqlite3 (v3.40.1) supports ALTER TABLE…DROP COLUMN, but older SQLite versions require a table rebuild—please apply the suggested diff to include the version check and fallback.
fetch with query_scalar + flatten
1 participant
Summary by CodeRabbit
Refactor
Chores