feat: implement timeout detection and reversal for waiting orders #215
Conversation
- Implement timeout detection for orders in waiting-buyer-invoice and waiting-payment states - Add timeout reversal persistence for maker orders to maintain pending status after restart - Remove sessions for taker orders that timeout to clean up from My Trades - Enrich synthetic messages with complete order data for full UI functionality - Add race condition protection for concurrent timeout processing - Add cancel button support for timeout-reverted maker orders via Action.timeoutReversal - Add timeouts for critical operations to prevent application hangs Detects when orders timeout from waiting states and handles maker vs taker scenarios: - Maker orders: revert to pending and preserve session for continued trading - Taker orders: clean up session and remove from My Trades view
|
""" WalkthroughThis change introduces a robust timeout detection and handling mechanism for order workflows. It adds new timeout constants, extends enums and message models for timeout reversals, updates order state logic, and enhances notification and localization support for timeout scenarios. Real-time timeout detection is integrated with order event processing and user notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OrderNotifier
participant PublicEventsProvider
participant NotificationNotifier
participant Storage
Note over OrderNotifier: On event or sync, or via public event subscription
OrderNotifier->>PublicEventsProvider: Get latest public event
OrderNotifier->>Storage: Get latest gift wrap message
OrderNotifier->>OrderNotifier: Compare timestamps and order status
alt Timeout detected
alt User is maker
OrderNotifier->>Storage: Persist timeout reversal message
OrderNotifier->>NotificationNotifier: Show maker timeout notification
else User is taker
OrderNotifier->>Storage: Delete session
OrderNotifier->>NotificationNotifier: Show taker timeout notification
end
end
NotificationNotifier->>User: Display localized timeout message
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)lib/**/*.dartInstructions used from: Sources:
**/*.dartInstructions used from: Sources:
🧠 Learnings (2)📓 Common learningslib/features/order/notfiers/order_notifier.dart (17)🔇 Additional comments (8)
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. 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
🧹 Nitpick comments (2)
lib/data/models/mostro_message.dart (1)
32-81: Comprehensive timeout reversal factory constructor with solid implementation.The factory constructor effectively creates complete timeout reversal messages by extracting detailed order information from public events. The logic for determining buyer/seller master keys based on order type is correct, and the defensive programming with null coalescing and default values is appropriate.
However, consider this potential improvement for type safety:
- ) as T, + ) as T, // Note: This cast assumes T is Order type in timeout reversal contextThe cast to
Tassumes the generic type isOrder, which should be true for timeout reversals but could be made more explicit through a type constraint or runtime check if type safety is a concern.lib/features/order/models/order_state.dart (1)
142-142: Translate comment to English for consistency.The codebase uses English for comments. Please translate this Spanish comment to maintain consistency.
- // FIX: Cuando alguien toma una orden, debe cambiar el status inmediatamente + // FIX: When someone takes an order, the status must change immediately
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
lib/core/config.dart(1 hunks)lib/data/models/enums/action.dart(1 hunks)lib/data/models/mostro_message.dart(2 hunks)lib/features/order/models/order_state.dart(7 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(1 hunks)lib/features/order/notfiers/order_notifier.dart(3 hunks)lib/features/trades/widgets/mostro_message_detail_widget.dart(2 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)lib/shared/notifiers/notification_notifier.dart(2 hunks)lib/shared/widgets/notification_listener_widget.dart(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
lib/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
lib/l10n/intl_{en,es,it}.arb
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
lib/l10n/*.arb
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
lib/l10n/**
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (13)
📓 Common learnings
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
lib/features/order/notfiers/abstract_mostro_notifier.dart (5)
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in the NostrEvent extension
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.305Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify `test/mocks.mocks.dart` - it already has file-level ignores
lib/data/models/enums/action.dart (1)
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
lib/features/trades/widgets/mostro_message_detail_widget.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/shared/providers/app_init_provider.dart:78-95
Timestamp: 2025-05-06T16:08:19.352Z
Learning: In the Mostro Mobile codebase, MostroMessage objects have an 'action' member that can be directly accessed (e.g., mostroMessage.action).
lib/core/config.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/main.dart : Locales for the `timeago` package are configured in `main.dart` with `timeago.setLocaleMessages()`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through `services/nostr_service.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in `shared/providers/mostro_database_provider.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
lib/shared/widgets/notification_listener_widget.dart (14)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in the NostrEvent extension
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/main.dart : Locales for the `timeago` package are configured in `main.dart` with `timeago.setLocaleMessages()`
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/core/app_routes.dart : GoRouter navigation is configured in `core/app_routes.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Use Riverpod for all state management
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/models.dart : Models should be exported through `data/models.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in `shared/providers/mostro_database_provider.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.305Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify `test/mocks.mocks.dart` - it already has file-level ignores
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through `services/nostr_service.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Pass BuildContext to methods that need localization
lib/l10n/intl_it.arb (1)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (`intl_en.arb`, `intl_es.arb`, `intl_it.arb`)
lib/shared/notifiers/notification_notifier.dart (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Encapsulate business logic in Notifiers
lib/data/models/mostro_message.dart (13)
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through `services/nostr_service.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/models.dart : Models should be exported through `data/models.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in the NostrEvent extension
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.305Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify `test/mocks.mocks.dart` - it already has file-level ignores
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in `shared/providers/mostro_database_provider.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/repositories/**/*.dart : All data access should go through repository classes in `data/repositories/`
lib/l10n/intl_en.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (`intl_en.arb`, `intl_es.arb`, `intl_it.arb`)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
lib/features/order/models/order_state.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/shared/providers/app_init_provider.dart:78-95
Timestamp: 2025-05-06T16:08:19.352Z
Learning: In the Mostro Mobile codebase, MostroMessage objects have an 'action' member that can be directly accessed (e.g., mostroMessage.action).
lib/features/order/notfiers/order_notifier.dart (12)
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Encapsulate business logic in Notifiers
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through `services/nostr_service.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in the NostrEvent extension
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.
lib/l10n/intl_es.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (`intl_en.arb`, `intl_es.arb`, `intl_it.arb`)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
🔇 Additional comments (26)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)
217-219: LGTM! Well-designed timeout reversal handling.The explicit case for
Action.timeoutReversalwith manual handling inOrderNotifierfollows the established pattern and is appropriately documented. This aligns with the retrieved learning thatAbstractMostroNotifierhandles state updates while specialized notifiers manage specific side effects.lib/data/models/enums/action.dart (1)
41-42: LGTM! Clean enum addition following established conventions.The new
timeoutReversalenum value follows the correct naming pattern (camelCase for enum, kebab-case for string value) and integrates seamlessly with the existingfromStringmethod.lib/features/trades/widgets/mostro_message_detail_widget.dart (2)
43-45: Well-designed conditional display logic.The conditional logic appropriately handles
timeoutReversalby showing only the status rather than concatenating action text, which makes sense since timeout reversal is an internal state transition rather than a user action.
196-197: LGTM! Proper localization handling for timeout reversal.The new case follows the established pattern and correctly uses
S.of(context)!.orderTimeoutMakerfor localized user feedback, adhering to the coding guidelines for user-facing strings.lib/l10n/intl_es.arb (1)
689-694: ✅ Timeout localization keys present in all ARB files
- lib/l10n/intl_en.arb:
orderTimeoutTaker&orderTimeoutMakerconfirmed- lib/l10n/intl_es.arb:
orderTimeoutTaker&orderTimeoutMakerconfirmed- lib/l10n/intl_it.arb:
orderTimeoutTaker&orderTimeoutMakerconfirmedAll timeout messages are now consistently localized across English, Spanish, and Italian. Ready to merge!
lib/core/config.dart (1)
4-40: Excellent configuration improvements.The comment translations to English improve code readability, and the new timeout constants are well-designed:
timeoutDetectionTimeout(8s) andmessageStorageTimeout(5s) provide sensible defaults for the timeout detection mechanism- Proper use of
Durationtypes and descriptive naming- Well-organized under appropriate comment sections
These constants support the timeout handling features described in the PR objectives.
lib/l10n/intl_en.arb (1)
659-662: LGTM! New timeout notification messages properly added.The new localization keys
orderTimeoutTakerandorderTimeoutMakerare appropriately added with clear, user-friendly messages. The grouping under@_comment_timeout_messagesprovides good organization.lib/l10n/intl_it.arb (1)
699-702: LGTM! Consistent localization keys added for Italian.The timeout notification messages are properly translated and maintain consistency with the English version. Good adherence to the coding guideline requiring new localization keys in all three ARB files.
lib/shared/widgets/notification_listener_widget.dart (3)
4-4: LGTM! Proper import for action enum.The import follows the established pattern and provides necessary access to the
Action.timeoutReversalenum value.
18-39: Excellent implementation of custom message handling.The logic elegantly handles both custom messages with specific localization keys (
orderTimeoutTaker,orderTimeoutMaker) and fallback to action-based messages. The switch statement provides clean mapping to localized strings, and the fallback logic fortimeoutReversalaction is appropriate.
43-45: Good explicit duration setting for SnackBar.Setting the duration to 2 seconds provides consistent user experience for informational notifications.
lib/shared/notifiers/notification_notifier.dart (3)
11-11: LGTM! Appropriate addition of customMessage field.The nullable
customMessagefield provides flexibility for custom notification messages while maintaining backward compatibility.
18-19: Good constructor update with proper parameter ordering.The constructor maintains consistency with existing patterns and places the new optional parameter appropriately.
31-35: Excellent implementation of showCustomMessage method.The method follows the established pattern of other notification methods and correctly sets the informational flag. This provides a clean API for displaying custom timeout messages.
lib/data/models/mostro_message.dart (1)
8-12: LGTM! Necessary imports for the new factory constructor.The imports provide access to required types for the timeout reversal functionality.
lib/features/order/models/order_state.dart (3)
85-85: LGTM! Clean logging messages.Removing emojis from logger messages improves consistency and professionalism in the codebase.
Also applies to: 96-97, 122-124
179-181: Correct implementation of timeout reversal status mapping.The timeout reversal action appropriately returns the order to pending status, aligning with the PR's timeout handling objectives.
202-204: Consistent action configuration for timeout-reverted orders.The addition of
timeoutReversalaction withcancelas an allowed action for both buyer and seller roles is appropriate and consistent with other pending order states.Also applies to: 282-284
lib/features/order/notfiers/order_notifier.dart (8)
2-6: Required imports for timeout functionality.All added imports are necessary for the new timeout detection features.
14-22: Well-structured timeout detection initialization.Good implementation with proper subscription lifecycle management and race condition prevention using the
_isProcessingTimeoutflag.
53-87: Robust sync implementation with timeout detection.The sync method properly tracks the latest gift wrap message and integrates timeout checking seamlessly into the synchronization flow.
152-264: Excellent timeout detection and handling implementation.The method comprehensively handles timeout scenarios with proper differentiation between maker and taker cases, state persistence, and robust error handling. The use of timeouts on async operations prevents the app from hanging.
266-281: Clear logic for determining order creator.The method correctly identifies whether the user created (maker) or took (taker) an order based on role and order type matching.
283-339: Robust real-time timeout detection with excellent concurrency control.The subscription implementation properly prevents race conditions and validates state at multiple checkpoints before performing cleanup operations.
361-365: Proper resource cleanup in dispose method.The subscription is correctly disposed to prevent memory leaks.
341-359: Localization keys verified for timeout notifications.Verified that both
orderTimeoutMakerandorderTimeoutTakerare defined in all three ARB files (lib/l10n/intl_en.arb,intl_es.arb,intl_it.arb). No further changes required.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/features/order/notfiers/order_notifier.dart (1)
24-38: Fix method signature and race condition handling.Two issues need addressing:
- The method is marked
asyncbut doesn't returnFuture<void>- The existing comment about missing
awaitonsuper.handleEvent()should be addressedAdditionally, consider adding the
_isProcessingTimeoutflag here to prevent concurrent timeout processing, similar to the subscription handler.- void handleEvent(MostroMessage event) async { + Future<void> handleEvent(MostroMessage event) async { + if (_isProcessingTimeout) return; + + try { + _isProcessingTimeout = true; // First handle the event normally await super.handleEvent(event); // Then check for timeout if we're in waiting states // Only check if we have a valid session (this is a taker scenario) final currentSession = ref.read(sessionNotifierProvider.notifier).getSessionByOrderId(orderId); if (mounted && currentSession != null && (state.status == Status.waitingBuyerInvoice || state.status == Status.waitingPayment)) { final shouldCleanup = await _checkTimeoutAndCleanup(state, event); if (shouldCleanup) { // Session was cleaned up, invalidate this provider ref.invalidateSelf(); } } + } finally { + _isProcessingTimeout = false; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/order/notfiers/order_notifier.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
lib/features/order/notfiers/order_notifier.dart (13)
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Encapsulate business logic in Notifiers
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/**/*.dart : Always check `mounted` before using context after async operations
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through `services/nostr_service.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in the NostrEvent extension
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.
⏰ 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 (6)
lib/features/order/notfiers/order_notifier.dart (6)
1-22: LGTM: Clean imports and property initialization.The new imports and private properties are well-structured and follow Dart conventions. The constructor properly initializes the timeout detection subscription.
152-264: Excellent timeout detection implementation with robust error handling.This method effectively handles the complex timeout detection logic with several strengths:
- Comprehensive error handling with timeouts to prevent hanging
- Proper differentiation between maker and taker scenarios
- Critical persistence of timeout reversal messages for state recovery
- Thorough logging for debugging and monitoring
- Graceful degradation when operations fail
The method is appropriately complex given the business requirements it addresses.
266-281: LGTM: Clear and correct maker/taker determination logic.The helper method correctly implements the business logic to distinguish between makers and takers based on user role and order type.
283-339: Robust real-time timeout detection with excellent race condition protection.This subscription handler demonstrates excellent defensive programming:
- Proper race condition protection with
_isProcessingTimeoutflag- Multiple safety checks for mounted state and valid sessions
- Timeout handling on async operations to prevent hanging
- Comprehensive error handling with proper cleanup in finally block
The implementation ensures reliable real-time timeout detection without side effects.
341-359: LGTM: Proper notification handling with translation keys.The method correctly uses translation keys (
orderTimeoutMaker,orderTimeoutTaker) rather than hardcoded strings, allowing the UI layer to handle localization withS.of(context)where context is available.
361-365: LGTM: Proper resource cleanup in dispose method.The dispose method correctly closes the subscription and calls super.dispose() for proper resource management.
Detects when orders timeout from waiting states and handles maker vs taker scenarios:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes