Phase 7 - #56
Merged
Merged
Conversation
Rust: - orders.rs: add take_order(order_id, role, fiat_amount) with OrderNotFound/OrderAlreadyTaken/OutOfRange validation; returns mock TradeInfo with initial BuyerStep::OrderTaken or SellerStep::TakerFound Dart: - TakeOrderScreen: 5 info cards (amount/premium, payment method, creation date, order ID with copy, creator reputation), countdown timer with circular progress, Close/Buy|Sell bottom bar - RangeAmountModal: centered dialog with numeric input, min/max helper text, validation, Cancel/Submit buttons - Navigation wiring: take_sell → buyer flow (add_invoice); take_buy → seller flow (pay_invoice); OrderAlreadyTaken → snackbar + return to home - app_routes: wired TakeOrderScreen at /take_sell/:orderId and /take_buy/:orderId replacing stubs
reactive order lookup, store selected amount Rust: - orders.rs: require fiat_amount for range orders (FiatAmountRequired error); validate positive/finite before range check; validate role matches order.kind (Buy→Seller, Sell→Buyer) with InvalidRole error Dart: - take_order_screen: replace _order getter (ref.read) with ref.watch in build() for reactive updates; use ref.read in imperative handlers; store range modal amount in _selectedAmount for Phase 8+ bridge call
Contributor
WalkthroughReplaces stub routes with a new TakeOrderScreen, adds a range-amount modal for fiat-range orders, and implements a Rust Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as TakeOrderScreen
participant Modal as RangeAmountModal
participant API as Rust<br/>take_order()
participant Cache as Order Cache
User->>UI: Open TakeOrderScreen(orderId)
UI->>Cache: Fetch order details
Cache-->>UI: Return order
UI->>User: Display order + countdown
User->>UI: Tap Take
alt Range order
UI->>Modal: Show range amount modal
User->>Modal: Enter amount
Modal->>Modal: Validate (min/max)
Modal-->>UI: Return amount
end
UI->>API: take_order(orderId, role, fiat_amount?)
API->>Cache: Load order
Cache-->>API: Order
API->>API: Validate status == Pending
API->>API: Validate role matches kind
alt Range order
API->>API: Validate fiat_amount finite & positive & within [min,max]
end
API-->>UI: Return TradeInfo
UI->>User: Navigate to add/pay invoice
alt OrderAlreadyTaken
API-->>UI: OrderAlreadyTaken error
UI->>User: Show snackbar + navigate home
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Around line 100-112: Replace the dummy delay with an actual call to the Rust
bridge take_order(orderId, _selectedAmount) (the Rust API is in
rust/src/api/orders.rs) and await its result before navigating; only call
context.push(AppRoute.addInvoicePath(widget.orderId)) or
AppRoute.payInvoicePath(...) after take_order succeeds. Handle and surface the
Rust errors (OrderAlreadyTaken, OutOfRange, InvalidRole) by catching the bridge
exception, mapping each to an appropriate UI error/snackbar/dialog, and return
early on failure so the UI does not navigate. Ensure you reference the widget
fields _selectedAmount and widget.orderId when calling take_order and keep the
mounted check before navigation.
- Around line 54-56: The code uses IterableExtensions.firstOrNull (calls to
firstOrNull in _startCountdown and other places) but the dart:collection import
is missing; add import 'dart:collection'; at the top of the file so firstOrNull
is in scope and the analyzer errors on those calls (firstOrNull references in
_startCountdown and the other occurrences) are resolved.
- Around line 288-290: The CircularProgressIndicator is being normalized against
a hard-coded 24h causing incorrect progress; compute the order lifetime using
order.expiresAt.difference(order.createdAt).inSeconds (or inMilliseconds for
finer granularity) and use that as the denominator when calculating value (e.g.,
_remaining.inSeconds / lifetimeSeconds), guarding against zero/negative
lifetimes and clamping the result to 0.0–1.0 before passing it to
CircularProgressIndicator to avoid NaN/infinite and out-of-range values.
In `@lib/features/order/widgets/range_amount_modal.dart`:
- Around line 43-57: The amount input currently blocks decimal entry and uses
double.tryParse without ensuring the keyboard allows decimals; update the
TextField/TextFormField that uses _controller to set keyboardType:
TextInputType.numberWithOptions(decimal: true) so users can enter decimals, and
keep the existing _parsed getter and _validate logic (which use double.tryParse)
but consider swapping to a locale-aware parser from the intl package if you need
locale-specific decimal separators; apply the same keyboardType change wherever
the amount field is defined (the other occurrence noted around the range amount
widget lines that build the input).
In `@rust/src/api/orders.rs`:
- Around line 217-290: take_order currently fabricates a TradeInfo without
invoking the Mostro "take" helpers, so wire it to call the proper helper in
rust/src/mostro/actions.rs: after validating the order and computing
initial_step/TradeInfo fields, call the appropriate helper (e.g.
mostro::actions::take_buy or mostro::actions::take_sell based on order.kind) and
pass the required params (order, role, fiat_amount, caller identity/context).
Use the helper's Result to update/persist the TradeInfo (or return its error)
instead of returning the mock TradeInfo; keep the existing validations in
take_order and ensure you import the action functions and propagate errors from
mostro::actions into the take_order Result.
- Around line 229-236: The take_order flow must reject self-takes: after
fetching the order via order_book().get_order(&order_id) in take_order(), add a
guard that checks the order.is_mine flag (and/or the maker_id vs current user
id) and return an error (e.g., Err(anyhow::anyhow!("CannotTakeOwnOrder"))) if
true, before proceeding with the existing OrderStatus::Pending check and trade
creation; update any tests that assume create_order() sets is_mine so
take_order() refuses to process locally created orders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d234c762-5322-401a-989f-c46d02bbb11c
📒 Files selected for processing (5)
lib/core/app_routes.dartlib/features/order/screens/take_order_screen.dartlib/features/order/widgets/range_amount_modal.dartrust/src/api/orders.rsspecs/004-mostro-p2p-client/tasks.md
Comment on lines
+54
to
+56
| void _startCountdown() { | ||
| final orders = ref.read(orderBookProvider); | ||
| final order = orders.where((o) => o.id == widget.orderId).firstOrNull; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -60 lib/features/order/screens/take_order_screen.dartRepository: MostroP2P/app
Length of output: 1850
🌐 Web query:
firstOrNull Dart 3.x availability dart:core or dart:collection
💡 Result:
firstOrNull is available in Dart 3.x as an extension method on Iterable<T> in the dart:collection library (IterableExtensions extension), marked @Since("3.0"). It is not in dart:core. Usage: import 'dart:collection'; then iterable.firstOrNull returns the first element or null if empty.
Citations:
- 1: https://api.dart.dev/dart-collection/IterableExtensions/firstOrNull.html
- 2: https://api.dart.dev/dev/latest/dart-collection/IterableExtensions.html
- 3: https://api.dart.dev/stable/3.10.7/dart-collection/IterableExtensions/firstOrNull.html
- 4: https://api.dart.dev/stable/dart-collection/IterableExtensions/firstOrNull.html
🏁 Script executed:
sed -n '80,90p; 130,140p' lib/features/order/screens/take_order_screen.dartRepository: MostroP2P/app
Length of output: 966
firstOrNull is not in scope — missing dart:collection import.
The method firstOrNull on Iterable<T> is provided by IterableExtensions in dart:collection, not in dart:core (which is auto-imported). Lines 56, 83, and 134 all call firstOrNull without this import, causing analysis failures. Add import 'dart:collection'; at the top of the file to resolve.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/order/screens/take_order_screen.dart` around lines 54 - 56, The
code uses IterableExtensions.firstOrNull (calls to firstOrNull in
_startCountdown and other places) but the dart:collection import is missing; add
import 'dart:collection'; at the top of the file so firstOrNull is in scope and
the analyzer errors on those calls (firstOrNull references in _startCountdown
and the other occurrences) are resolved.
Comment on lines
+100
to
+112
| try { | ||
| // TODO (Phase 8+): Call take_order(orderId, _selectedAmount) via Rust bridge. | ||
| await Future.delayed(const Duration(milliseconds: 500)); | ||
|
|
||
| if (!mounted) return; | ||
|
|
||
| // Navigate based on role: | ||
| // Buyer → add invoice screen; Seller → pay invoice screen. | ||
| if (widget.isBuying) { | ||
| context.push(AppRoute.addInvoicePath(widget.orderId)); | ||
| } else { | ||
| context.push(AppRoute.payInvoicePath(widget.orderId)); | ||
| } |
Contributor
There was a problem hiding this comment.
The primary action never actually takes the order.
Lines 101-112 wait 500 ms and navigate unconditionally; rust/src/api/orders.rs::take_order is never invoked. That bypasses the new Rust validations (OrderAlreadyTaken, OutOfRange, InvalidRole) and lets the UI advance even when nothing was published.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/order/screens/take_order_screen.dart` around lines 100 - 112,
Replace the dummy delay with an actual call to the Rust bridge
take_order(orderId, _selectedAmount) (the Rust API is in rust/src/api/orders.rs)
and await its result before navigating; only call
context.push(AppRoute.addInvoicePath(widget.orderId)) or
AppRoute.payInvoicePath(...) after take_order succeeds. Handle and surface the
Rust errors (OrderAlreadyTaken, OutOfRange, InvalidRole) by catching the bridge
exception, mapping each to an appropriate UI error/snackbar/dialog, and return
early on failure so the UI does not navigate. Ensure you reference the widget
fields _selectedAmount and widget.orderId when calling take_order and keep the
mounted check before navigation.
Comment on lines
+217
to
+290
| /// Take an existing order, starting a trade. | ||
| /// | ||
| /// Sends a `take-buy` or `take-sell` MostroMessage via NIP-59. | ||
| /// Returns a `TradeInfo` with the initial trade state. | ||
| /// | ||
| /// TODO: Wire to actual Rust bridge identity + relay pool in Phase 8+. | ||
| /// Currently validates params and returns a mock TradeInfo. | ||
| pub async fn take_order( | ||
| order_id: String, | ||
| role: crate::api::types::TradeRole, | ||
| fiat_amount: Option<f64>, | ||
| ) -> Result<crate::api::types::TradeInfo> { | ||
| let order = order_book() | ||
| .get_order(&order_id) | ||
| .await | ||
| .ok_or_else(|| anyhow::anyhow!("OrderNotFound"))?; | ||
|
|
||
| if order.status != OrderStatus::Pending { | ||
| return Err(anyhow::anyhow!("OrderAlreadyTaken")); | ||
| } | ||
|
|
||
| // Validate range amount. | ||
| let is_range = order.fiat_amount_min.is_some() && order.fiat_amount_max.is_some(); | ||
| if is_range { | ||
| let amt = fiat_amount.ok_or_else(|| anyhow::anyhow!("FiatAmountRequired"))?; | ||
| if !amt.is_finite() || amt <= 0.0 { | ||
| return Err(anyhow::anyhow!("fiat_amount must be positive and finite")); | ||
| } | ||
| let min = order.fiat_amount_min.unwrap(); | ||
| let max = order.fiat_amount_max.unwrap(); | ||
| if amt < min || amt > max { | ||
| return Err(anyhow::anyhow!("OutOfRange")); | ||
| } | ||
| } | ||
|
|
||
| use crate::api::types::*; | ||
|
|
||
| // Validate role matches order kind. | ||
| let expected_role = match order.kind { | ||
| OrderKind::Buy => TradeRole::Seller, | ||
| OrderKind::Sell => TradeRole::Buyer, | ||
| }; | ||
| if role != expected_role { | ||
| return Err(anyhow::anyhow!("InvalidRole")); | ||
| } | ||
|
|
||
| let now = std::time::SystemTime::now() | ||
| .duration_since(std::time::UNIX_EPOCH) | ||
| .unwrap_or_default() | ||
| .as_secs() as i64; | ||
|
|
||
| let initial_step = match role { | ||
| TradeRole::Buyer => TradeStep::Buyer(BuyerStep::OrderTaken), | ||
| TradeRole::Seller => TradeStep::Seller(SellerStep::TakerFound), | ||
| }; | ||
|
|
||
| let trade = TradeInfo { | ||
| id: uuid::Uuid::new_v4().to_string(), | ||
| order: order.clone(), | ||
| role, | ||
| counterparty_pubkey: order.creator_pubkey.clone(), | ||
| current_step: initial_step, | ||
| hold_invoice: None, | ||
| buyer_invoice: None, | ||
| trade_key_index: 0, | ||
| cooperative_cancel_state: None, | ||
| timeout_at: Some(now + 900), // 15 min default | ||
| started_at: now, | ||
| completed_at: None, | ||
| outcome: None, | ||
| }; | ||
|
|
||
| Ok(trade) | ||
| } |
Contributor
There was a problem hiding this comment.
take_order() is still a no-op.
This path only reads from the cache and fabricates a TradeInfo; it never calls the existing rust/src/mostro/actions.rs take helpers. Callers can therefore get a success path without publishing any take-buy / take-sell message or actually reserving the order.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/src/api/orders.rs` around lines 217 - 290, take_order currently
fabricates a TradeInfo without invoking the Mostro "take" helpers, so wire it to
call the proper helper in rust/src/mostro/actions.rs: after validating the order
and computing initial_step/TradeInfo fields, call the appropriate helper (e.g.
mostro::actions::take_buy or mostro::actions::take_sell based on order.kind) and
pass the required params (order, role, fiat_amount, caller identity/context).
Use the helper's Result to update/persist the TradeInfo (or return its error)
instead of returning the mock TradeInfo; keep the existing validations in
take_order and ensure you import the action functions and propagate errors from
mostro::actions into the take_order Result.
Default relays: wss://relay.mostro.network, wss://nos.lol Default Mostro pubkey: 82fa8cb9... Adds rust/src/config.rs constants + seeds.rs for first-launch DB seeding. Updates contracts/settings.md with default configuration section.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specs/004-mostro-p2p-client/tasks.md`:
- Around line 54-62: The Phase 2b section has contradictory task sequencing:
T012b–T012e are marked incomplete but the "Blocks: T012, T029" line references
tasks already completed; update the spec to resolve this by either (A) moving
Phase 2b before T012 and changing T012’s status to incomplete (or renumber Phase
2b to T125–T128) if these steps are prerequisites for T012/T029, (B) change the
"Blocks" wording to "Extends" or "Complements" T012 and T029 if Phase 2b is
post-completion enhancement, or (C) remove the "Blocks" claim and renumber Phase
2b sequentially if it is independent; make the change consistently across the
Phase 2b header and task lines (referencing Phase 2b, T012b–T012e, T012, and
T029) so the dependency graph and statuses are coherent.
- Around line 63-76: Update the seeding logic to use the actual RelayInfo
fields: when populating DEFAULT_RELAYS in rust/src/db/seeds.rs create RelayInfo
instances with is_default: true and source: RelaySource::Default (instead of
user_added), and keep enabled: true; also ensure DEFAULT_RELAYS,
DEFAULT_MOSTRO_PUBKEY and DEFAULT_MOSTRO_NAME are exported from config.rs and
that when no active Mostro node exists you seed settings.active_mostro_pubkey
with DEFAULT_MOSTRO_PUBKEY and store/display DEFAULT_MOSTRO_NAME as the node
name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8670d7de-50b3-467c-8b57-f9433ab3a885
📒 Files selected for processing (2)
specs/004-mostro-p2p-client/contracts/settings.mdspecs/004-mostro-p2p-client/tasks.md
✅ Files skipped from review due to trivial changes (1)
- specs/004-mostro-p2p-client/contracts/settings.md
Comment on lines
+54
to
+62
| ## Phase 2b: Default Configuration (Relays + Mostro Node) | ||
|
|
||
| **Purpose**: Seed the app with default relay URLs and the default Mostro node | ||
| pubkey so it can connect to the network on first launch without any user | ||
| configuration. | ||
|
|
||
| **Depends on**: Phase 2 (T008, T009) | ||
| **Blocks**: T012 (relay pool initialization), T029 (Nostr relay API) | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all references to T012 to understand dependency structure
rg -n 'T012[^b-e]' specs/004-mostro-p2p-client/tasks.mdRepository: MostroP2P/app
Length of output: 872
🏁 Script executed:
sed -n '44,80p' specs/004-mostro-p2p-client/tasks.mdRepository: MostroP2P/app
Length of output: 3466
🏁 Script executed:
sed -n '127,140p' specs/004-mostro-p2p-client/tasks.mdRepository: MostroP2P/app
Length of output: 3819
Clarify task numbering and resolve the "Blocks" contradiction.
Phase 2b tasks T012b–T012e are marked incomplete [ ] and describe prerequisite setup work (creating config constants, seeding relays and Mostro node, wiring bootstrap). However, the section states "Blocks: T012" and "T029", but both T012 and T029 are already marked complete [x] in earlier phases. This creates a logical impossibility—Phase 2b cannot block tasks that are already finished.
Clarify the intent:
- If Phase 2b work is genuinely required before T012 initialization completes, then T012's completion status
[x]should be reconsidered or moved to[ ], and these tasks should be renumbered asT125–T128with proper sequencing. - If T012 and T029 were completed without this default-seeding setup, then revise the "Blocks" statement to reflect the actual relationship—e.g., "Extends T012 and T029 with default relay and node configuration on first launch" or place Phase 2b within Phase 2 before the T012 checkpoint.
- If Phase 2b is independent supplementary work, renumber sequentially and remove the "Blocks" claim, replacing it with "Complements" or "Extends".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/004-mostro-p2p-client/tasks.md` around lines 54 - 62, The Phase 2b
section has contradictory task sequencing: T012b–T012e are marked incomplete but
the "Blocks: T012, T029" line references tasks already completed; update the
spec to resolve this by either (A) moving Phase 2b before T012 and changing
T012’s status to incomplete (or renumber Phase 2b to T125–T128) if these steps
are prerequisites for T012/T029, (B) change the "Blocks" wording to "Extends" or
"Complements" T012 and T029 if Phase 2b is post-completion enhancement, or (C)
remove the "Blocks" claim and renumber Phase 2b sequentially if it is
independent; make the change consistently across the Phase 2b header and task
lines (referencing Phase 2b, T012b–T012e, T012, and T029) so the dependency
graph and statuses are coherent.
Comment on lines
+63
to
+76
| - [ ] T012b Create `rust/src/config.rs` with hardcoded seed constants: | ||
| - `DEFAULT_RELAYS: &[&str]` = `["wss://relay.mostro.network", "wss://nos.lol"]` | ||
| - `DEFAULT_MOSTRO_PUBKEY: &str` = `"82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390"` | ||
| - `DEFAULT_MOSTRO_NAME: &str` = `"Mostro"` | ||
| - Export from `lib.rs` | ||
|
|
||
| - [ ] T012c On first launch (no relays in DB), seed DB with `DEFAULT_RELAYS` as | ||
| `RelayInfo { url, user_added: false, enabled: true }`. Default relays are NOT | ||
| deletable from the UI (only disable allowed). Implement in `rust/src/db/seeds.rs`. | ||
|
|
||
| - [ ] T012d On first launch (no Mostro node selected), seed the active Mostro node | ||
| with `DEFAULT_MOSTRO_PUBKEY` + `DEFAULT_MOSTRO_NAME`. Store in settings as | ||
| `active_mostro_pubkey`. Implement in same `rust/src/db/seeds.rs`. | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify RelayInfo fields and check for relay URL documentation
# Check RelayInfo struct definition
ast-grep --pattern $'struct RelayInfo {
$$$
}'
# Search for nos.lol references to see if the change is documented
rg -i 'nos\.lol'
# Search for relay.damus.io to understand current usage
rg -i 'relay\.damus\.io'Repository: MostroP2P/app
Length of output: 415
🏁 Script executed:
# Search for RelayInfo struct definition
rg 'struct RelayInfo' -A 15
# Check the settings.md for full relay list context
sed -n '1,100p' specs/004-mostro-p2p-client/contracts/settings.md | rg -i 'relay' -B 2 -A 2
# Check current hardcoded relays in rust/src/api/nostr.rs
rg -B 3 -A 3 'wss://relay.damus.io' rust/src/api/nostr.rsRepository: MostroP2P/app
Length of output: 1288
🏁 Script executed:
# Check how default relays are currently created/seeded in the codebase
rg -B 5 -A 10 'fn default_relays\|DEFAULT_RELAYS' rust/src
# Look for RelaySource enum definition
rg 'enum RelaySource' -A 5
# Check the settings.md file more completely to understand the relay seeding requirements
cat specs/004-mostro-p2p-client/contracts/settings.md | head -80Repository: MostroP2P/app
Length of output: 2870
🏁 Script executed:
# Check if is_default and source are both used or if one is redundant
rg 'is_default.*true|source.*Default' rust/src -B 2 -A 2 | head -50
# Look for any existing seed functions to see how RelayInfo is constructed
rg -B 3 -A 10 'fn.*seed.*relay|RelayInfo \{' rust/src/db/ --type rustRepository: MostroP2P/app
Length of output: 383
🏁 Script executed:
# Verify the relay URL change is intentional by checking git history or comments
rg -B 5 -A 5 'nos\.lol' specs/004-mostro-p2p-client/contracts/settings.md
# Check if there's a reason for the change in any comments or documentation
rg -B 3 -A 3 'relay.damus.io|nos.lol' --type markdown specs/
# Confirm the current code still uses the old relay list
cat rust/src/api/nostr.rs | grep -A 5 'fn default_relays'Repository: MostroP2P/app
Length of output: 2655
Correct field names in task description to match actual struct definition.
The task specifies seeding relays with user_added: false, but the actual RelayInfo struct uses is_default: bool and source: RelaySource instead. When seeding default relays, use is_default: true and source: RelaySource::Default.
The relay URL change from relay.damus.io to nos.lol is documented in contracts/settings.md as an intentional upgrade to "General Nostr relay (fallback)" and requires no clarification.
Suggested fix for field names
- [ ] T012c On first launch (no relays in DB), seed DB with `DEFAULT_RELAYS` as
- `RelayInfo { url, user_added: false, enabled: true }`. Default relays are NOT
- deletable from the UI (only disable allowed). Implement in `rust/src/db/seeds.rs`.
+ [ ] T012c On first launch (no relays in DB), seed DB with `DEFAULT_RELAYS` as
+ `RelayInfo { url, is_default: true, is_active: true, source: RelaySource::Default, ... }`. Default relays are NOT
+ deletable from the UI (only disable allowed). Implement in `rust/src/db/seeds.rs`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/004-mostro-p2p-client/tasks.md` around lines 63 - 76, Update the
seeding logic to use the actual RelayInfo fields: when populating DEFAULT_RELAYS
in rust/src/db/seeds.rs create RelayInfo instances with is_default: true and
source: RelaySource::Default (instead of user_added), and keep enabled: true;
also ensure DEFAULT_RELAYS, DEFAULT_MOSTRO_PUBKEY and DEFAULT_MOSTRO_NAME are
exported from config.rs and that when no active Mostro node exists you seed
settings.active_mostro_pubkey with DEFAULT_MOSTRO_PUBKEY and store/display
DEFAULT_MOSTRO_NAME as the node name.
accuracy, decimal keyboard Rust: - orders.rs: reject self-takes with CannotTakeOwnOrder error (is_mine check before status check) Dart: - take_order_screen: compute countdown progress from actual order lifetime (expiresAt - createdAt) instead of hardcoded 24h - range_amount_modal: use numberWithOptions(decimal: true) for fiat amount input
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
lib/features/order/screens/take_order_screen.dart (2)
54-57:⚠️ Potential issue | 🔴 Critical
firstOrNullis out of scope withoutdart:collection.Line 56, Line 83, and Line 134 use
firstOrNull, but this file does not importdart:collection, so analysis/build will fail.Suggested fix
import 'dart:async'; +import 'dart:collection';#!/bin/bash set -euo pipefail file="lib/features/order/screens/take_order_screen.dart" echo "== firstOrNull usages ==" rg -n '\bfirstOrNull\b' "$file" echo "== dart:collection import ==" rg -n "import 'dart:collection';" "$file" || trueAlso applies to: 82-84, 133-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/take_order_screen.dart` around lines 54 - 57, The file uses Iterable.firstOrNull (seen in _startCountdown and the other occurrences) but doesn't import dart:collection, causing build errors; fix it by adding the missing import for dart:collection at the top of the file so firstOrNull is in scope, then re-run analysis to ensure references (e.g., the uses in _startCountdown and the other firstOrNull usages) resolve correctly.
100-112:⚠️ Potential issue | 🔴 CriticalPrimary action still does not execute
take_order.Line 101-Line 112 still uses a delay placeholder and then navigates. This bypasses the Rust validations (
CannotTakeOwnOrder,OrderAlreadyTaken,OutOfRange, role checks) and can move users forward without an actual take.Wire this to the bridge call and only navigate on success; map backend errors to user-facing messages and return early on failure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/take_order_screen.dart` around lines 100 - 112, Replace the placeholder delay in the try block with a real call to the Rust bridge function take_order(widget.orderId, _selectedAmount), await its result, and only perform navigation (context.push to AppRoute.addInvoicePath or AppRoute.payInvoicePath depending on widget.isBuying) when the bridge returns success and the widget is still mounted; catch and map bridge errors like CannotTakeOwnOrder, OrderAlreadyTaken, OutOfRange and role-check failures to user-facing messages (show a Snackbar/Alert or set local error state) and return early on failure instead of navigating, preserving existing mounted checks and error logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Around line 54-57: The file uses Iterable.firstOrNull (seen in _startCountdown
and the other occurrences) but doesn't import dart:collection, causing build
errors; fix it by adding the missing import for dart:collection at the top of
the file so firstOrNull is in scope, then re-run analysis to ensure references
(e.g., the uses in _startCountdown and the other firstOrNull usages) resolve
correctly.
- Around line 100-112: Replace the placeholder delay in the try block with a
real call to the Rust bridge function take_order(widget.orderId,
_selectedAmount), await its result, and only perform navigation (context.push to
AppRoute.addInvoicePath or AppRoute.payInvoicePath depending on widget.isBuying)
when the bridge returns success and the widget is still mounted; catch and map
bridge errors like CannotTakeOwnOrder, OrderAlreadyTaken, OutOfRange and
role-check failures to user-facing messages (show a Snackbar/Alert or set local
error state) and return early on failure instead of navigating, preserving
existing mounted checks and error logging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83eafc4c-5855-4ff1-8141-8e5c31a7e3a5
📒 Files selected for processing (3)
lib/features/order/screens/take_order_screen.dartlib/features/order/widgets/range_amount_modal.dartrust/src/api/orders.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/features/order/widgets/range_amount_modal.dart
- rust/src/api/orders.rs
This was referenced Mar 30, 2026
Merged
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rust:
OrderNotFound/OrderAlreadyTaken/OutOfRange validation; returns mock
TradeInfo with initial BuyerStep::OrderTaken or SellerStep::TakerFound
Dart:
creation date, order ID with copy, creator reputation), countdown
timer with circular progress, Close/Buy|Sell bottom bar
helper text, validation, Cancel/Submit buttons
take_buy → seller flow (pay_invoice); OrderAlreadyTaken → snackbar
/take_buy/:orderId replacing stubs
Summary by CodeRabbit
New Features
Documentation