Restore orders feature #355
Conversation
Protocol Compliance
- Update MostroMessage serialization to use correct wrapper key:
* "restore" key for Action.restore messages
* "order" key for all other actions (backward compatible)
- Implement EmptyPayload class for null payload serialization
- Support full privacy mode in restore requests:
* Reputation mode: includes master key signature
* Full privacy mode: omits master key, uses only trade key
## Restore Session Flow
- Implement RestoreService with initial session restore workflow:
1. Clear existing data (sessions, storage, notifications)
2. Create temporary subscription on trade key index 1
3. Send restore-session request to Mostro
4. Handle restore response with orders and disputes data
- Add RestoreOverlay widget with multi-stage progress indicator - Add ImportMnemonicDialog for mnemonic phrase input and validation - Implement RestoreProgressNotifier for state management
WalkthroughAdds a multi-stage restore workflow: new payload types/models, restore orchestration and progress state/providers, a visible restore overlay, mnemonic import UI and validator, message filtering and restore-mode gating, small repo/test adjustments, settings integration, and localized strings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant KeyScreen as Key Management Screen
participant Dialog as ImportMnemonicDialog
participant RestoreSvc as RestoreService
participant Progress as RestoreProgressNotifier
participant Overlay as RestoreOverlay
participant Relay as Relay/Mostro
User->>KeyScreen: Tap Import
KeyScreen->>Dialog: showDialog(...)
Dialog->>Dialog: validateMnemonic (BIP39)
Dialog-->>KeyScreen: return mnemonic
KeyScreen->>RestoreSvc: importMnemonicAndRestore(mnemonic)
RestoreSvc->>Progress: startRestore()
Progress->>Overlay: set visible (requesting)
RestoreSvc->>Relay: send Action.restore (wrapper: "restore")
Relay-->>RestoreSvc: restore_data (orders & disputes)
RestoreSvc->>Progress: updateStep(receivingOrders)
RestoreSvc->>Relay: send Action.orders (wrapper: "restore" / "orders")
Relay-->>RestoreSvc: orders details
RestoreSvc->>Progress: updateStep(loadingDetails)
RestoreSvc->>Relay: send Action.lastTradeIndex
Relay-->>RestoreSvc: last-trade-index
RestoreSvc->>Progress: updateStep(finalizing)
RestoreSvc->>Progress: completeRestore()
Progress->>Overlay: show(completed) then hide()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas needing extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)lib/features/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.dart📄 CodeRabbit inference engine (AGENTS.md)
Files:
lib/**/*.dart📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-10-21T21:47:03.451ZApplied to files:
📚 Learning: 2025-10-22T12:29:26.971ZApplied to files:
📚 Learning: 2025-10-22T12:29:26.971ZApplied to files:
📚 Learning: 2025-10-22T12:29:26.971ZApplied to files:
📚 Learning: 2025-06-04T19:35:20.209ZApplied to files:
⏰ 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)
🔇 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
lib/data/models/orders_request.dart (1)
11-15: Add defensive checks in fromJson for robustness.The fromJson factory doesn't validate that the 'ids' key exists or that it's a List, which could cause runtime errors if the JSON is malformed or unexpected.
Apply this diff to add defensive checks:
factory OrdersPayload.fromJson(Map<String, dynamic> json) { + final idsList = json['ids']; + if (idsList == null) { + return const OrdersPayload(ids: []); + } + if (idsList is! List) { + throw FormatException('Expected List for "ids" but got ${idsList.runtimeType}'); + } return OrdersPayload( - ids: (json['ids'] as List<dynamic>).map((e) => e as String).toList(), + ids: idsList.map((e) => e as String).toList(), ); }lib/features/restore/restore_progress_state.dart (1)
42-45: Consider adding bounds checking to progressPercentage.The progressPercentage getter doesn't validate that currentProgress is within [0, totalProgress], which could return values outside the expected [0.0, 1.0] range if the state is updated incorrectly.
Apply this diff to ensure the percentage stays within bounds:
double get progressPercentage { if (totalProgress == 0) return 0.0; - return currentProgress / totalProgress; + final percentage = currentProgress / totalProgress; + return percentage.clamp(0.0, 1.0); }lib/services/mostro_service.dart (1)
44-45: Clarify comment about trade index 1.The comment states "The app always use trade index 1 for restore-related messages" but this invariant isn't enforced in code. Consider:
- If this is a critical assumption, add validation that rejects messages with other trade indices
- If this is merely informational, clarify that this is a convention, not an enforced rule
- Add a reference to where trade index 1 is assigned for restore sessions
lib/data/models/mostro_message.dart (1)
101-111: Consider extracting wrapper key selection logic.The wrapper key selection logic (
action == Action.restore || action == Action.lastTradeIndex ? 'restore' : 'order') is duplicated in bothsign()(line 103) andserialize()(line 115). Consider extracting to a helper:String _getWrapperKey() { return (action == Action.restore || action == Action.lastTradeIndex) ? 'restore' : 'order'; }Then use in both methods to ensure consistency and simplify future maintenance.
lib/data/models/orders_response.dart (2)
12-19: Consider simplifying nullable list handling.The nullable list handling is correct but could be more concise.
Apply this diff for a more idiomatic pattern:
factory OrdersResponse.fromJson(Map<String, dynamic> json) { return OrdersResponse( - orders: (json['orders'] as List<dynamic>?) - ?.map((o) => OrderDetail.fromJson(o as Map<String, dynamic>)) - .toList() ?? - [], + orders: (json['orders'] as List<dynamic>? ?? []) + .map((o) => OrderDetail.fromJson(o as Map<String, dynamic>)) + .toList(), ); }
60-77: Simplify nullable field parsing.The nullable field handling is overly verbose. Dart's type system can handle this more elegantly.
Apply this diff to simplify the factory:
factory OrderDetail.fromJson(Map<String, dynamic> json) { return OrderDetail( id: json['id'] as String, kind: json['kind'] as String, status: json['status'] as String, amount: json['amount'] as int, fiatCode: json['fiat_code'] as String, - minAmount: json['min_amount'] != null ? json['min_amount'] as int : null, - maxAmount: json['max_amount'] != null ? json['max_amount'] as int : null, + minAmount: json['min_amount'] as int?, + maxAmount: json['max_amount'] as int?, fiatAmount: json['fiat_amount'] as int, paymentMethod: json['payment_method'] as String, premium: json['premium'] as int, - buyerTradePubkey: json['buyer_trade_pubkey'] != null ? json['buyer_trade_pubkey'] as String : null, - sellerTradePubkey: json['seller_trade_pubkey'] != null ? json['seller_trade_pubkey'] as String : null, - createdAt: json['created_at'] != null ? json['created_at'] as int : null, - expiresAt: json['expires_at'] != null ? json['expires_at'] as int : null, + buyerTradePubkey: json['buyer_trade_pubkey'] as String?, + sellerTradePubkey: json['seller_trade_pubkey'] as String?, + createdAt: json['created_at'] as int?, + expiresAt: json['expires_at'] as int?, ); }lib/features/key_manager/key_management_screen.dart (3)
175-188: Consider adjusting button width ratios.The 7:3 flex ratio between Import and Refresh buttons may make the Refresh button too narrow on smaller screens, especially since it only contains an icon. Consider testing on various screen sizes or adjusting to 8:2 or making both buttons equal width with the refresh icon and text.
781-786: Consider adding mounted check after dialog dismissal.After dismissing the dialog, you immediately access
ref.read()to trigger the restore process. While the timing is tight, it's a best practice to checkmountedafter async gaps to prevent accessing disposed widgets.Apply this diff:
onPressed: () async { Navigator.of(dialogContext).pop(); - - final restoreService = ref.read(restoreServiceProvider); - await restoreService.initRestoreProcess(); + + if (mounted) { + final restoreService = ref.read(restoreServiceProvider); + await restoreService.initRestoreProcess(); + } },
609-625: Consider adding accessibility support to icon-only button.The Refresh button uses only an icon without text. For better accessibility, consider adding a tooltip or semantic label so screen readers can announce the button's purpose.
Apply this diff:
Widget _buildRefreshUserButton(BuildContext context) { - return OutlinedButton( + return Tooltip( + message: S.of(context)!.refreshUser, + child: OutlinedButton( - onPressed: () => _showRefreshUserDialog(context), - style: OutlinedButton.styleFrom( - side: const BorderSide(color: AppTheme.activeColor), - shape: RoundedRectangleBorder( - borderRadius: BorderRadius.circular(8), + onPressed: () => _showRefreshUserDialog(context), + style: OutlinedButton.styleFrom( + side: const BorderSide(color: AppTheme.activeColor), + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(8), + ), + padding: const EdgeInsets.symmetric(vertical: 16), ), - padding: const EdgeInsets.symmetric(vertical: 16), + child: const Icon( + LucideIcons.refreshCw, + size: 20, + color: AppTheme.activeColor, + ), ), - child: const Icon( - LucideIcons.refreshCw, - size: 20, - color: AppTheme.activeColor, - ), ); }lib/features/restore/restore_manager.dart (1)
499-502: Consider extracting magic number to constant.The 8-second delay is hardcoded and depends on relay timing characteristics. Consider extracting this to a named constant at the class level for better maintainability and potential future configuration.
Apply this diff:
class RestoreService { final Ref ref; final Logger _logger = Logger(); + static const Duration _historicalMessageWaitDuration = Duration(seconds: 8); StreamSubscription<NostrEvent>? _tempSubscription; ... // Later in the restore method: - _logger.i('Restore: waiting 8 seconds for historical messages to be saved...'); + _logger.i('Restore: waiting ${_historicalMessageWaitDuration.inSeconds} seconds for historical messages to be saved...'); //WARNING: It is very important to wait here to ensure all historical messages arrive before rebuilding state // Relays could send them with delay - await Future.delayed(const Duration(seconds: 8)); + await Future.delayed(_historicalMessageWaitDuration);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
lib/core/app.dart(2 hunks)lib/data/models/enums/action.dart(1 hunks)lib/data/models/last_trade_index_response.dart(1 hunks)lib/data/models/mostro_message.dart(3 hunks)lib/data/models/orders_request.dart(1 hunks)lib/data/models/orders_response.dart(1 hunks)lib/data/models/payload.dart(1 hunks)lib/data/models/restore_response.dart(1 hunks)lib/data/repositories/open_orders_repository.dart(1 hunks)lib/features/key_manager/import_mnemonic_dialog.dart(1 hunks)lib/features/key_manager/key_management_screen.dart(5 hunks)lib/features/notifications/utils/notification_message_mapper.dart(2 hunks)lib/features/notifications/widgets/notification_item.dart(1 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(3 hunks)lib/features/order/notfiers/order_notifier.dart(1 hunks)lib/features/restore/restore_manager.dart(1 hunks)lib/features/restore/restore_overlay.dart(1 hunks)lib/features/restore/restore_progress_notifier.dart(1 hunks)lib/features/restore/restore_progress_state.dart(1 hunks)lib/features/settings/settings_screen.dart(2 hunks)lib/l10n/intl_en.arb(3 hunks)lib/l10n/intl_es.arb(3 hunks)lib/l10n/intl_it.arb(3 hunks)lib/services/mostro_service.dart(3 hunks)lib/shared/utils/mnemonic_validator.dart(1 hunks)test/mocks.mocks.dart(2 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Store persistence and API code under lib/data/
Files:
lib/data/repositories/open_orders_repository.dartlib/data/models/last_trade_index_response.dartlib/data/models/orders_request.dartlib/data/models/payload.dartlib/data/models/orders_response.dartlib/data/models/restore_response.dartlib/data/models/mostro_message.dartlib/data/models/enums/action.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores
Files:
lib/data/repositories/open_orders_repository.dartlib/data/models/last_trade_index_response.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/data/models/orders_request.dartlib/features/settings/settings_screen.dartlib/data/models/payload.dartlib/services/mostro_service.dartlib/shared/utils/mnemonic_validator.dartlib/features/order/notfiers/order_notifier.dartlib/data/models/orders_response.dartlib/data/models/restore_response.dartlib/features/restore/restore_progress_state.dartlib/data/models/mostro_message.darttest/mocks.mocks.dartlib/features/key_manager/import_mnemonic_dialog.dartlib/features/notifications/utils/notification_message_mapper.dartlib/core/app.dartlib/features/restore/restore_overlay.dartlib/features/notifications/widgets/notification_item.dartlib/features/restore/restore_progress_notifier.dartlib/data/models/enums/action.dartlib/features/key_manager/key_management_screen.dartlib/features/restore/restore_manager.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier
lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible
Files:
lib/data/repositories/open_orders_repository.dartlib/data/models/last_trade_index_response.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/data/models/orders_request.dartlib/features/settings/settings_screen.dartlib/data/models/payload.dartlib/services/mostro_service.dartlib/shared/utils/mnemonic_validator.dartlib/features/order/notfiers/order_notifier.dartlib/data/models/orders_response.dartlib/data/models/restore_response.dartlib/features/restore/restore_progress_state.dartlib/data/models/mostro_message.dartlib/features/key_manager/import_mnemonic_dialog.dartlib/features/notifications/utils/notification_message_mapper.dartlib/core/app.dartlib/features/restore/restore_overlay.dartlib/features/notifications/widgets/notification_item.dartlib/features/restore/restore_progress_notifier.dartlib/data/models/enums/action.dartlib/features/key_manager/key_management_screen.dartlib/features/restore/restore_manager.dart
lib/data/repositories/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use the Repository pattern for all data access (repositories under data/repositories)
Files:
lib/data/repositories/open_orders_repository.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/settings/settings_screen.dartlib/features/order/notfiers/order_notifier.dartlib/features/restore/restore_progress_state.dartlib/features/key_manager/import_mnemonic_dialog.dartlib/features/notifications/utils/notification_message_mapper.dartlib/features/restore/restore_overlay.dartlib/features/notifications/widgets/notification_item.dartlib/features/restore/restore_progress_notifier.dartlib/features/key_manager/key_management_screen.dartlib/features/restore/restore_manager.dart
lib/services/**
📄 CodeRabbit inference engine (AGENTS.md)
Place service-layer code in lib/services/
Files:
lib/services/mostro_service.dart
lib/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Put shared utilities in lib/shared/
Files:
lib/shared/utils/mnemonic_validator.dart
test/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*.dart: Mirror feature layout for tests under test/
Prefer Mockito fakes generated via build_runner in tests
test/**/*.dart: Unit tests belong in test/; integration tests in integration_test/
Mocks are generated via Mockito; regenerate with dart run build_runner build -d after mock changes
Files:
test/mocks.mocks.dart
test/mocks.mocks.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Do not manually edit test/mocks.mocks.dart (Mockito-generated)
Files:
test/mocks.mocks.dart
**/*.mocks.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Do not manually edit any *.mocks.dart generated Mockito files
Files:
test/mocks.mocks.dart
**/*.{g.dart,mocks.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
Never add per-item ignore comments to generated files; rely on file-level ignores or regenerate
Files:
test/mocks.mocks.dart
lib/core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep dependency wiring in lib/core/
Files:
lib/core/app.dart
lib/l10n/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
lib/l10n/*.arb: Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)
When adding a new localization key, update all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
After modifying ARB files, run dart run build_runner build -d to regenerate localization
Files:
lib/l10n/intl_es.arblib/l10n/intl_en.arblib/l10n/intl_it.arb
🧠 Learnings (39)
📓 Common learnings
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/data/repositories/**/*.dart : Use the Repository pattern for all data access (repositories under data/repositories)
Applied to files:
lib/data/repositories/open_orders_repository.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : SubscriptionManager manages subscriptions, including relay list stream; ensure proper initialization and cleanup
Applied to files:
lib/data/repositories/open_orders_repository.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/services/mostro_service.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/data/models.dart : Export models through lib/data/models.dart
Applied to files:
lib/data/models/last_trade_index_response.dartlib/data/models/orders_request.dartlib/features/settings/settings_screen.dartlib/data/models/restore_response.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/relays/relays_notifier.dart : Implement relay operations: syncWithMostroInstance, removeRelayWithBlacklist, addRelayWithSmartValidation, _handleMostroRelayListUpdate
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/settings/settings_screen.dartlib/features/order/notfiers/order_notifier.dart
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/services/mostro_service.dartlib/features/order/notfiers/order_notifier.darttest/mocks.mocks.dartlib/core/app.dartlib/features/restore/restore_overlay.dartlib/features/notifications/widgets/notification_item.dartlib/features/restore/restore_progress_notifier.dartlib/features/restore/restore_manager.dart
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/services/mostro_service.dartlib/features/order/notfiers/order_notifier.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/mostro_fsm.dart : MostroFSM manages order state transitions (core/mostro_fsm.dart)
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/settings/settings_screen.dartlib/features/order/notfiers/order_notifier.dartlib/features/restore/restore_progress_state.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dartlib/features/notifications/utils/notification_message_mapper.dartlib/features/notifications/widgets/notification_item.dartlib/features/restore/restore_progress_notifier.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/services/mostro_service.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/settings/settings_screen.dartlib/services/mostro_service.dartlib/core/app.dart
📚 Learning: 2025-05-06T15:49:55.079Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
Applied to files:
lib/features/settings/settings_screen.dartlib/core/app.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/settings/settings.dart : Settings model must include blacklistedRelays and maintain backward-compatible serialization
Applied to files:
lib/features/settings/settings_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Initialize Sembast database in shared/providers/mostro_database_provider.dart
Applied to files:
lib/features/settings/settings_screen.dartlib/core/app.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/relays/relay.dart : Relay model must track source (user, mostro, defaultConfig) and expose canDelete/canBlacklist semantics
Applied to files:
lib/features/settings/settings_screen.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/services/mostro_service.darttest/mocks.mocks.dartlib/data/models/enums/action.dart
📚 Learning: 2025-05-08T16:31:29.582Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/models/relay_list_event.dart : Parse NIP-65 kind 10002 events, validate WS URLs, and handle timestamps/null-safety
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-07-08T05:40:47.527Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
Applied to files:
lib/data/models/mostro_message.dartlib/features/notifications/utils/notification_message_mapper.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to test/mocks.mocks.dart : Do not manually edit test/mocks.mocks.dart (Mockito-generated)
Applied to files:
test/mocks.mocks.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to test/**/*.dart : Mocks are generated via Mockito; regenerate with dart run build_runner build -d after mock changes
Applied to files:
test/mocks.mocks.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to **/*.mocks.dart : Do not manually edit any *.mocks.dart generated Mockito files
Applied to files:
test/mocks.mocks.dart
📚 Learning: 2025-10-07T17:31:12.117Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T17:31:12.117Z
Learning: Applies to test/**/*.dart : Prefer Mockito fakes generated via build_runner in tests
Applied to files:
test/mocks.mocks.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/generated/l10n.dart : Generated localization entrypoint file is lib/generated/l10n.dart
Applied to files:
lib/features/key_manager/import_mnemonic_dialog.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.
Applied to files:
lib/features/notifications/utils/notification_message_mapper.dartlib/core/app.dartlib/features/restore/restore_overlay.dartlib/features/notifications/widgets/notification_item.dartlib/features/restore/restore_progress_notifier.dartlib/features/key_manager/key_management_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/notifications/notification_service.dart : Notification service configuration and custom icons should be defined in notification_service.dart
Applied to files:
lib/features/notifications/utils/notification_message_mapper.dartlib/features/notifications/widgets/notification_item.dartlib/features/key_manager/key_management_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/**/*.dart : Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Applied to files:
lib/features/notifications/utils/notification_message_mapper.dartlib/features/key_manager/key_management_screen.dart
📚 Learning: 2025-05-06T16:08:19.352Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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).
Applied to files:
lib/features/notifications/utils/notification_message_mapper.dartlib/features/notifications/widgets/notification_item.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/app_routes.dart : Use GoRouter for navigation configured in core/app_routes.dart
Applied to files:
lib/core/app.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/core/app.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/main.dart : Configure timeago locales and app initialization in main.dart
Applied to files:
lib/core/app.dart
📚 Learning: 2025-10-07T17:31:12.117Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T17:31:12.117Z
Learning: Applies to lib/**/*.dart : Name Riverpod providers as <Feature>Provider or <Feature>Notifier
Applied to files:
lib/core/app.dartlib/features/restore/restore_progress_notifier.dart
📚 Learning: 2025-09-22T19:42:55.332Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 311
File: lib/services/mostro_service.dart:171-182
Timestamp: 2025-09-22T19:42:55.332Z
Learning: NextTrade payload is supported by both fiatSent and release actions in the Mostro protocol. For fiatSent action, it's specifically used when the maker is the buyer on a range order, as documented in docs/protocol/fiatsent.md with the payload format: { "next_trade": ["<trade pubkey>", <trade index>] }
Applied to files:
lib/features/notifications/widgets/notification_item.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/l10n/*.arb : Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)
Applied to files:
lib/l10n/intl_es.arblib/l10n/intl_en.arblib/l10n/intl_it.arb
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/l10n/*.arb : When adding a new localization key, update all three ARB files (en, es, it)
Applied to files:
lib/l10n/intl_es.arblib/l10n/intl_en.arblib/l10n/intl_it.arb
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/**/screens/**/*.dart : Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)
Applied to files:
lib/features/key_manager/key_management_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
Applied to files:
lib/l10n/intl_en.arb
⏰ 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 (23)
lib/data/repositories/open_orders_repository.dart (1)
141-146: LGTM! Clean cache-clearing implementation.The method correctly clears the in-memory cache and resubscribes to reload orders from relays. The logging is appropriate for debugging restore operations.
lib/shared/utils/mnemonic_validator.dart (1)
7-22: LGTM! Solid mnemonic validation utility.The implementation properly delegates to the bip39 library with appropriate defensive checks for empty input and exception handling.
lib/data/models/enums/action.dart (1)
41-44: LGTM! Enum expansion is correctly implemented.The new Action values for the restore feature integrate cleanly with the existing fromString/toString logic via the _valueMap.
lib/core/app.dart (1)
167-174: LGTM! Clean overlay integration.The Stack-based approach correctly overlays the RestoreOverlay on top of all routed content, ensuring the restore progress UI is always visible when active. The const constructor for RestoreOverlay is efficient.
lib/data/models/payload.dart (1)
40-49: Verify whether restore/lastTradeIndex responses can have empty payloads in JSON.The original concern is technically valid: if incoming responses for the
restoreorlastTradeIndexactions contain"payload": {}(empty object), the code will throwUnsupportedErrorbecauseEmptyPayloadis not handled in thePayload.fromJsonfactory (lines 15-37 oflib/data/models/payload.dart).However, verification is inconclusive:
EmptyPayload()is only used for outgoing messages, andMostroMessage.fromJsonskips deserialization ifpayloadis null. Whether incoming responses actually have empty payloads depends on the Mostro protocol specification, which couldn't be confirmed from the codebase alone.Action required: Check the Mostro protocol specification to confirm whether
restoreandlastTradeIndexresponse messages can have empty payloads. If they can, addEmptyPayloadsupport toPayload.fromJsonby checking forjson.isEmptyand returningEmptyPayload().lib/features/notifications/widgets/notification_item.dart (1)
124-126: LGTM! New action cases properly handled.The three new action cases (restore, orders, lastTradeIndex) are appropriately added with break statements, aligning with the restore feature introduced in this PR. The no-op behavior is consistent with other cases in this switch that don't require navigation.
lib/features/order/notfiers/order_notifier.dart (1)
142-154: LGTM! Restore helper methods properly implemented.The new
updateStateFromMessageandupdateDisputemethods are well-designed for use during restore flows:
- Both check
mountedbefore updating state (appropriate for StateNotifier per project learnings)- Simple, focused implementations that delegate to existing state update logic
- Public visibility enables external orchestration by RestoreService
lib/features/notifications/utils/notification_message_mapper.dart (2)
87-90: Clarify notification requirements for new actions.The TODO placeholder strings ('TODO: implement title key if needed') would be displayed to users if notifications are created for restore, orders, or lastTradeIndex actions.
Clarify the notification strategy:
- If these actions don't create user-facing notifications: Consider throwing an exception or returning a sentinel value instead of a TODO string to catch misuse during development.
- If these actions do create notifications: Complete the localization by adding proper keys to the
.arbfiles before release.
183-186: Same concern: clarify notification requirements for message keys.Same issue as the title keys above - TODO placeholders would be visible to users. Ensure these actions either don't create notifications, or complete the localization before release.
lib/services/mostro_service.dart (1)
121-124: LGTM! Proper filtering of restore payloads.The skip logic correctly prevents restore-specific historical events from being processed as normal order events, avoiding interference during subscription setup per the PR objectives.
lib/data/models/mostro_message.dart (2)
40-40: LGTM! Proper EmptyPayload serialization.Serializing
EmptyPayloadasnullaligns with the protocol specification and properly handles the no-data payload variant introduced in this PR.
47-47: LGTM! Flexible deserialization for restore messages.Accepting 'order', 'restore', or 'cant-do' keys provides backward compatibility while supporting the new restore protocol, with appropriate priority ordering.
lib/data/models/last_trade_index_response.dart (1)
1-21: LGTM! Clean payload model implementation.The
LastTradeIndexResponsemodel follows the standard payload pattern:
- Implements
Payloadinterface correctly- Provides proper JSON serialization/deserialization
- Uses const constructor for efficiency
- Type identifier matches the action name convention ('last-trade-index')
test/mocks.mocks.dart (2)
849-856: Generated mock looks correct.The
clearCache()mock forMockOpenOrdersRepositoryis properly generated and corresponds to the new method added to the repository. No manual edits needed.
2930-2947: Generated mocks look correct.The
updateStateFromMessage()andupdateDispute()mocks forMockOrderNotifierare properly generated and correspond to the new methods inOrderNotifier. Standard noSuchMethod implementations are appropriate for these void return methods.lib/features/settings/settings_screen.dart (1)
464-477: Errors are not silently ignored—they are logged and surfaced via RestoreOverlay. The review comment's concerns are unfounded.The verification confirms that the restore error handling works as intended:
Errors are logged:
initRestoreProcess()catches all exceptions at line 705 and logs them via_logger.e()with full stack trace.Errors are surfaced to UI: At line 707, caught errors update the
restoreProgressProviderviashowError(), which setsstep: RestoreStep.error. TheRestoreOverlaywatches this provider and renders the error state.RestoreOverlay is integrated: It's part of the main widget tree (
lib/core/app.dart:171), so error states are displayed to users.The try-catch in settings_screen.dart is defensive: Since
initRestoreProcess()internally handles all exceptions and never re-throws, this catch block is redundant but harmless. The misleading comment should ideally be clarified or removed.The code structure is correct. No changes needed.
lib/features/restore/restore_overlay.dart (1)
8-172: LGTM!The RestoreOverlay implementation is well-structured with:
- Proper use of latest Flutter APIs (
withValuesfor color transparency)- Good separation of concerns with helper methods
- Responsive design considerations
- Correct localization usage via
S.of(context)- Clean state management via Riverpod
lib/features/key_manager/import_mnemonic_dialog.dart (1)
6-227: LGTM!The ImportMnemonicDialog implementation is well-designed:
- Proper validation using external utility
- Correct mounted check before navigation (Line 46)
- Good error handling and user feedback
- Clean UI structure with info bullets
- Proper resource disposal in dispose()
lib/features/restore/restore_progress_notifier.dart (1)
6-123: LGTM!The RestoreProgressNotifier implementation follows best practices:
- Proper mounted checks before state updates in async callbacks (Lines 63, 80, 95)
- Correct timer cleanup in dispose() (Lines 114-117)
- Good timeout handling with configurable duration
- Clean state transitions
lib/features/restore/restore_manager.dart (2)
368-400: TODO acknowledged - dispute creator determination.The TODO on Line 398 is documented in the PR objectives as a known limitation. The current implementation uses trade index comparison with pubkey verification, which is a reasonable best-effort approach. Future protocol changes may enable more definitive detection.
454-456: Good coordination with restore mode flag.The static flag
AbstractMostroNotifier.setRestoring()creates intentional coordination between restore service and message processing. The implementation correctly:
- Enables the flag before state reconstruction
- Disables it after completion
- Ensures cleanup in error cases via the catch block (Lines 608-612)
This pattern prevents race conditions during restoration as documented in the PR objectives.
Also applies to: 604-606, 608-612
lib/l10n/intl_en.arb (1)
462-914: Localization keys properly maintained across all ARB files.Verification confirms that new localization keys for import/restore features are present in all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb). Tested keys across all categories returned consistent results across all languages. Ensure
dart run build_runner build -dhas been run to regenerate localizations after ARB modifications.lib/features/key_manager/key_management_screen.dart (1)
809-822: Add mounted check after async dialog.After
showDialogcompletes, the widget may have been disposed. You should checkmountedbefore accessingref.read()and performing further operations.Apply this diff:
Future<void> _showImportMnemonicDialog(BuildContext context) async { final mnemonic = await showDialog<String>( context: context, builder: (BuildContext dialogContext) { return const ImportMnemonicDialog(); }, ); - if (mnemonic != null && mnemonic.isNotEmpty) { + if (mounted && mnemonic != null && mnemonic.isNotEmpty) { final restoreService = ref.read(restoreServiceProvider); await restoreService.importMnemonicAndRestore(mnemonic); } }⛔ Skipped due to learnings
Learnt from: CR Repo: MostroP2P/mobile PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-10-22T12:29:26.971Z Learning: Applies to lib/**/*.dart : After async operations, check mounted before using BuildContextLearnt from: Catrya Repo: MostroP2P/mobile PR: 327 File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182 Timestamp: 2025-10-21T21:47:03.451Z Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
lib/services/mostro_service.dart (2)
46-96: Defensive coding improvements address previous concerns.The defensive type checking and null-safety improvements (lines 51-53, 56-58, 78, 85) successfully address the concerns raised in the previous review. The method now safely validates all nested structures before accessing or casting them, and returns false early on any unexpected type.
Consider adding debug logging when a restore payload is detected to aid troubleshooting:
// RestoreData: has 'restore_data' wrapper with 'orders' and 'disputes' arrays if (payload.containsKey('restore_data')) { + _logger.d('Detected RestoreData payload, skipping'); return true; }
139-142: Add logging when skipping restore payloads for debugging consistency.For consistency with the dispute message skip logic (line 135-136) and to aid debugging, consider logging when restore payloads are skipped:
// Skip restore-specific payloads that arrive as historical events due to temporary subscription if (result[0] is Map && _isRestorePayload(result[0] as Map<String, dynamic>)) { + _logger.i('Skipping restore-specific payload (handled by RestoreManager)'); return; }lib/features/restore/restore_manager.dart (1)
368-400: TODO: Incomplete dispute initiation detection.Line 398 contains a TODO indicating that the current method of determining dispute initiators may need improvement if the protocol changes. The current logic relies on comparing trade indexes, which may not be robust for future protocol versions.
Would you like me to open an issue to track improving this dispute detection logic when the Mostro protocol provides better restoration data?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/data/models/restore_response.dart(1 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(4 hunks)lib/features/restore/restore_manager.dart(1 hunks)lib/features/restore/restore_mode_provider.dart(1 hunks)lib/services/mostro_service.dart(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/data/models/restore_response.dart
- lib/features/order/notfiers/abstract_mostro_notifier.dart
🧰 Additional context used
📓 Path-based instructions (4)
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/restore/restore_mode_provider.dartlib/features/restore/restore_manager.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores
Files:
lib/features/restore/restore_mode_provider.dartlib/features/restore/restore_manager.dartlib/services/mostro_service.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier
lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible
Files:
lib/features/restore/restore_mode_provider.dartlib/features/restore/restore_manager.dartlib/services/mostro_service.dart
lib/services/**
📄 CodeRabbit inference engine (AGENTS.md)
Place service-layer code in lib/services/
Files:
lib/services/mostro_service.dart
🧠 Learnings (12)
📓 Common learnings
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/*/providers/**/*.dart : Organize Riverpod providers by feature under features/{feature}/providers/
Applied to files:
lib/features/restore/restore_mode_provider.dart
📚 Learning: 2025-10-07T17:31:12.117Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T17:31:12.117Z
Learning: Applies to lib/**/*.dart : Name Riverpod providers as <Feature>Provider or <Feature>Notifier
Applied to files:
lib/features/restore/restore_mode_provider.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/features/restore/restore_mode_provider.dart
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Applied to files:
lib/features/restore/restore_mode_provider.dartlib/features/restore/restore_manager.dartlib/services/mostro_service.dart
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : SubscriptionManager manages subscriptions, including relay list stream; ensure proper initialization and cleanup
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-05-08T16:31:29.582Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/models/relay_list_event.dart : Parse NIP-65 kind 10002 events, validate WS URLs, and handle timestamps/null-safety
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/services/mostro_service.dart
⏰ 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 (7)
lib/services/mostro_service.dart (1)
127-131: LGTM: Good defensive validation.The validation correctly prevents index access on empty or invalid payloads and provides helpful logging.
lib/features/restore/restore_mode_provider.dart (1)
1-9: LGTM! Clean Riverpod state management.The provider is well-documented and correctly implements restore mode state using Riverpod's StateProvider. The transition from a static mutable flag to proper state management is a good architectural improvement.
lib/features/restore/restore_manager.dart (5)
57-74: Good provider invalidation pattern.The invalidation and re-initialization of the keyManagerProvider ensures clean state after mnemonic import. The sequencing of operations is correct.
604-610: Excellent cleanup pattern for restore mode flag.The restore mode flag is properly disabled in both the success path (line 605) and error path (line 610), ensuring state consistency even when errors occur.
708-722: Robust cleanup in finally block.The finally block ensures critical cleanup operations (subscription cancellation, key clearing) always execute, and the conditional check prevents calling
completeRestore()when already in error state. This is excellent defensive programming.
726-728: Provider follows naming conventions.The provider is correctly named and structured according to the Riverpod conventions for this codebase.
Based on learnings
499-502: Consider making the message wait time configurable.The hardcoded 8-second delay assumes all historical messages will arrive within this timeframe. Network conditions and relay response times can vary significantly, potentially causing:
- Race conditions if messages arrive after 8 seconds
- Unnecessary waiting if all messages arrive quickly
Consider making this configurable or implementing a more robust synchronization mechanism:
// In class fields: static const Duration _messageWaitDuration = Duration(seconds: 8); // Or make it configurable via settings: final waitDuration = ref.read(settingsProvider).restoreWaitDuration ?? const Duration(seconds: 8); await Future.delayed(waitDuration);Alternatively, consider implementing a completion detection mechanism that monitors message arrival rather than relying solely on a fixed delay.
⛔ Skipped due to learnings
Learnt from: Catrya Repo: MostroP2P/mobile PR: 327 File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182 Timestamp: 2025-10-21T21:47:03.451Z Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.Learnt from: CR Repo: MostroP2P/mobile PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-10-22T12:29:26.971Z Learning: Applies to lib/**/*.dart : After async operations, check mounted before using BuildContext
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
lib/features/restore/restore_manager.dart (3)
185-186: Past review comment not addressed: Add bounds check before array access.Lines 185-186 access
contentList[0]without verifying the list is non-empty. This was flagged in a previous review but has not been fixed. If Mostro returns an empty array, this will throw aRangeError.Apply this diff to add bounds checking:
final contentList = jsonDecode(rumor.content!) as List<dynamic>; + if (contentList.isEmpty) { + _logger.w('Restore: empty content list received'); + return (ordersMap: <String, int>{}, disputes: <RestoredDispute>[]); + } final messageData = contentList[0] as Map<String, dynamic>;
274-279: Past review comment not addressed: Add bounds check and null-safe map access.Multiple safety issues flagged in previous review remain unaddressed:
- Lines 274-275: Access
contentList[0]without checking if the list is non-empty- Lines 278-279: Access map keys
'order'and'payload'without null checksApply this diff:
final contentList = jsonDecode(rumor.content!) as List<dynamic>; + if (contentList.isEmpty) { + throw Exception('Empty content list in orders details response'); + } final messageData = contentList[0] as Map<String, dynamic>; // Extract payload from order wrapper - final orderWrapper = messageData['order'] as Map<String, dynamic>; - final payload = orderWrapper['payload'] as Map<String, dynamic>; + final orderWrapper = messageData['order'] as Map<String, dynamic>?; + if (orderWrapper == null) { + throw Exception('No order wrapper found in response'); + } + final payload = orderWrapper['payload'] as Map<String, dynamic>?; + if (payload == null) { + throw Exception('No payload found in order wrapper'); + }
332-333: Past review comment not addressed: Add bounds check for array access.Lines 332-333 access
contentList[0]without verifying the list is non-empty. This was flagged in a previous review but remains unaddressed.Apply this diff:
final contentList = jsonDecode(rumor.content!) as List<dynamic>; + if (contentList.isEmpty) { + _logger.w('Restore: empty content list for last trade index, defaulting to 0'); + return LastTradeIndexResponse(tradeIndex: 0); + } final messageData = contentList[0] as Map<String, dynamic>;
🧹 Nitpick comments (2)
lib/features/restore/restore_manager.dart (2)
398-398: Consider addressing the TODO for improved dispute detection.The TODO comment indicates that the current dispute initiation detection method may need improvement if the protocol changes. While the current double-verification approach is sound, consider documenting the protocol dependency more explicitly or creating a follow-up issue to track potential protocol changes.
Would you like me to open an issue to track this protocol-dependent logic for future improvement?
514-518: Consider making the delay configurable.The hard-coded 8-second delay (line 518) is necessary to wait for historical messages to arrive, as noted in the comment. However, consider making this duration configurable (e.g., via a constant or settings parameter) to allow easier tuning if relay latency characteristics change.
For example, define a constant at the class level:
static const Duration _historicalMessagesDelay = Duration(seconds: 8);Then use it:
await Future.delayed(_historicalMessagesDelay);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/restore/restore_manager.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/restore/restore_manager.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores
Files:
lib/features/restore/restore_manager.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier
lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible
Files:
lib/features/restore/restore_manager.dart
🧠 Learnings (3)
📓 Common learnings
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Applied to files:
lib/features/restore/restore_manager.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/features/restore/restore_manager.dart
🔇 Additional comments (1)
lib/features/restore/restore_manager.dart (1)
57-74: LGTM! Good use of provider invalidation.The pattern of invalidating and re-reading the keyManagerProvider after importing the mnemonic ensures that all dependent providers get fresh instances with the new key. This is the correct approach for refreshing provider state.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/features/restore/restore_manager.dart (1)
515-518: Extract hardcoded delay to a named constant.The 8-second delay is a magic number that should be extracted to a named constant for better maintainability and self-documentation. This would make it easier to adjust the timing if needed and clarify the purpose of the delay.
Apply this diff to extract the constant:
+ // Delay duration to wait for historical messages from relays during restore + static const Duration _historicalMessagesDelay = Duration(seconds: 8); + Future<void> restore(Map<String, int> ordersIds, int lastTradeIndex, OrdersResponse ordersResponse, List<RestoredDispute> disputes) async { try { ... // Wait for historical messages to arrive and be saved to storage _logger.i('Restore: waiting 8 seconds for historical messages to be saved...'); //WARNING: It is very important to wait here to ensure all historical messages arrive before rebuilding state // Relays could send them with delay - await Future.delayed(const Duration(seconds: 8)); + await Future.delayed(_historicalMessagesDelay);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/restore/restore_manager.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/restore/restore_manager.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores
Files:
lib/features/restore/restore_manager.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier
lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible
Files:
lib/features/restore/restore_manager.dart
🧠 Learnings (4)
📓 Common learnings
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Applied to files:
lib/features/restore/restore_manager.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.
Applied to files:
lib/features/restore/restore_manager.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/mostro_fsm.dart : MostroFSM manages order state transitions (core/mostro_fsm.dart)
Applied to files:
lib/features/restore/restore_manager.dart
⏰ 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 (2)
lib/features/restore/restore_manager.dart (2)
633-740: Well-structured multi-stage restore workflow.The orchestration logic correctly implements the 4-stage restore process described in the PR objectives:
- Stage 1: Getting restore data (orders map and disputes)
- Stage 2: Getting order details
- Stage 3: Getting last trade index
- Stage 4: Processing and restoring sessions
The error handling, cleanup in the finally block, and temporary subscription management are all properly implemented. The special handling for empty orders (lines 681-689) ensures the key index is still updated even when no orders exist.
368-400: Good security verification for dispute initiation detection.The method implements proper security checks by:
- Verifying the session's trade pubkey matches the order's buyer/seller pubkey for the given role
- Comparing trade indexes to determine who initiated the dispute
- Defaulting to peer-initiated (false) if verification fails
This defensive approach prevents incorrect dispute attribution due to session mismatches.
Ready for ReviewNotes
|
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
Me ha funcionado bastante bien, pero hubo una vez que me salió error en la app al intentar importar el usuario, estos fueron los logs de mostrod:
2025-11-19T00:16:40.990977Z INFO mostrod::app::restore_session: Starting background restore session for master key: 858cb808efb31edc6b8c63259d8655515ab47b4a0fd340e79140cde5dd150aca
2025-11-19T00:16:40.993441Z INFO mostrod::db: Background restore session completed with 2 orders, 0 disputes
2025-11-19T00:16:40.993519Z INFO mostrod::app::restore_session: Restore session response sent to user 620ea3eebbd2d7bfca0fbe37b6e1b5a55dbe8075aa5fb9cbb0bde371ee456419
2025-11-19T00:16:41.155040Z INFO mostrod::util: sender key 82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390 - receiver key 620ea3eebbd2d7bfca0fbe37b6e1b5a55dbe8075aa5fb9cbb0bde371ee456419
2025-11-19T00:16:41.155872Z INFO mostrod::util: Sending DM, Event ID: df50fd5f2030cbc2e9ca681efbf6e70207e6ffe3618731af1aa5b467ad60dce0 to 620ea3eebbd2d7bfca0fbe37b6e1b5a55dbe8075aa5fb9cbb0bde371ee456419 with payload: "{\"restore\":{\"version\":1,\"request_id\":null,\"trade_index\":null,\"action\":\"restore-session\",\"payload\":{\"restore_data\":{\"orders\":[{\"order_id\":\"16a40294-919f-4d57-92e5-80cfd2376290\",\"trade_index\":2,\"status\":\"pending\"},{\"order_id\":\"d4950e52-cc57-41b9-bb69-fc58228f05d9\",\"trade_index\":1,\"status\":\"active\"}],\"disputes\":[]}}}}"
2025-11-19T00:16:41.613018Z WARN mostrod::app: Error unwrapping gift: malformed signature at line 1 column 1422Pero al volver a intentarlo me funcionó bien, ese mensaje de error pasa a veces en mostrod pero igual lo aviso aqui por si caso
There was a problem hiding this comment.
Intenté restaurar un usuario antiguo que tengo con mostro-cli, mostrod envía que hay 8 órdenes en total y aunque la app lo recibe, solo muestra 2 órdenes en la UI.
Este es el mensaje que envia mostrod:
2025-11-19T00:51:22.892070Z INFO mostrod::db: Background restore session completed with 8 orders, 0 disputes
2025-11-19T00:51:22.892199Z INFO mostrod::app::restore_session: Restore session response sent to user 519c20f0dc60797d3a8b344123d035bd2f8e80d838fd9729a6cc2774d3de97bf
2025-11-19T00:51:23.075859Z INFO mostrod::util: sender key 82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390 - receiver key 519c20f0dc60797d3a8b344123d035bd2f8e80d838fd9729a6cc2774d3de97bf
2025-11-19T00:51:23.076892Z INFO mostrod::util: Sending DM, Event ID: 591055211d95706748533330d3f61db97af3a8cc86034c95329e80e6aeb160b4 to 519c20f0dc60797d3a8b344123d035bd2f8e80d838fd9729a6cc2774d3de97bf with payload: "{\"restore\":{\"version\":1,\"request_id\":null,\"trade_index\":null,\"action\":\"restore-session\",\"payload\":{\"restore_data\":{\"orders\":[{\"order_id\":\"d4950e52-cc57-41b9-bb69-fc58228f05d9\",\"trade_index\":51,\"status\":\"active\"},{\"order_id\":\"1400e742-7db4-4363-b315-03b6b6918111\",\"trade_index\":33,\"status\":\"active\"},{\"order_id\":\"8221930d-bf6b-40bb-b2da-a8f4e9315dfb\",\"trade_index\":34,\"status\":\"active\"},{\"order_id\":\"e186ee33-a264-4503-a25c-06e4397a82cf\",\"trade_index\":37,\"status\":\"active\"},{\"order_id\":\"f6ea4b1d-f92c-49b9-a777-d3b472a6e036\",\"trade_index\":38,\"status\":\"active\"},{\"order_id\":\"abfa3fb4-01fb-469a-b763-9f8e3ff7de1e\",\"trade_index\":40,\"status\":\"active\"},{\"order_id\":\"2dfd3da6-a189-445e-9fdc-a11c7c6a1f24\",\"trade_index\":43,\"status\":\"fiat-sent\"},{\"order_id\":\"45c02db6-5d21-4c43-919c-eb194c76ca0f\",\"trade_index\":48,\"status\":\"pending\"}],\"disputes\":[]}}}}"
2025-11-19T00:51:23.434725Z INFO mostrod::util: sender key 82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390 - receiver key 519c20f0dc60797d3a8b344123d035bd2f8e80d838fd9729a6cc2774d3de97bf
2025-11-19T00:51:23.436443Z INFO mostrod::util: Sending DM, Event ID: 59406aae5651422cccf7bc63eddeae80870b309d2de58f305fb73720820b179b to 519c20f0dc60797d3a8b344123d035bd2f8e80d838fd9729a6cc2774d3de97bf with payload: "{\"order\":{\"version\":1,\"request_id\":1763513483241,\"trade_index\":null,\"action\":\"orders\",\"payload\":{\"orders\":[{\"id\":\"d4950e52-cc57-41b9-bb69-fc58228f05d9\",\"kind\":\"sell\",\"status\":\"active\",\"amount\":903,\"fiat_code\":\"ARS\",\"min_amount\":null,\"max_amount\":null,\"fiat_amount\":1234,\"payment_method\":\"MODO\",\"premium\":0,\"buyer_trade_pubkey\":\"9b708fc76cdfa7556166bdfe1949f51890dc83036d98ba6ec23b5382d40ffa1b\",\"seller_trade_pubkey\":\"620ea3eebbd2d7bfca0fbe37b6e1b5a55dbe8075aa5fb9cbb0bde371ee456419\",\"buyer_invoice\":\"spiralboot92@walletofsatoshi.com\",\"created_at\":null,\"expires_at\":null},{\"id\":\"1400e742-7db4-4363-b315-03b6b6918111\",\"kind\":\"buy\",\"status\":\"active\",\"amount\":656,\"fiat_code\":\"CUP\",\"min_amount\":300,\"max_amount\":1000,\"fiat_amount\":300,\"payment_method\":\"Transfermovil\",\"premium\":0,\"buyer_trade_pubkey\":\"f0518eb155b46e633fa5ad52f2636a266c49958b150597dc8032943d0501b9e8\",\"seller_trade_pubkey\":\"a36641acae07b0f9f2f25a3369492f16fdbb9e3294e028ef300e89775a30dd46\",\"buyer_invoice\":\"lnbc6540n1p53yq72pp5ca39ncfqwe7ea5vmthpg8cc5f78vye88vrt6qr3tqdc9vw9gu8kqdqqcqzzsxqyz5vqsp5nj0ha0k3mhz5c95taecwtd7uwhswmmryyjvxyulffmj3kqp7d64s9qxpqysgq7p6qe9qcankt8x035kh3g3t3s2p5jautxjyz3elk29vf7tzzedrkc4dr384urfc7exzjynelr0eag93gvgsjsx5422m3nmr4l3jxmfgpqtnphx\",\"created_at\":null,\"expires_at\":null},{\"id\":\"8221930d-bf6b-40bb-b2da-a8f4e9315dfb\",\"kind\":\"buy\",\"status\":\"active\",\"amount\":895,\"fiat_code\":\"MLC\",\"min_amount\":2,\"max_amount\":10,\"fiat_amount\":2,\"payment_method\":\"Transferencia Bancaria,Efectivo\",\"premium\":0,\"buyer_trade_pubkey\":\"d138f22eaef17793cf4e544b4206c8b7916c8fac93a936d88e6921c42458d1b1\",\"seller_trade_pubkey\":\"d8b2dbaa02889fa7f0d81d5fdd684182a063eec6e83f32c63dd517bd539cc4fc\",\"buyer_invoice\":\"spiralboot92@walletofsatoshi.com\",\"created_at\":null,\"expires_at\":null},{\"id\":\"e186ee33-a264-4503-a25c-06e4397a82cf\",\"kind\":\"buy\",\"status\":\"active\",\"amount\":644,\"fiat_code\":\"CUP\",\"min_amount\":300,\"max_amount\":1000,\"fiat_amount\":300,\"payment_method\":\"EnZona\",\"premium\":0,\"buyer_trade_pubkey\":\"4ef51a823e6b6b79244abe28d7b777fc912aba7e52b65fdb9dfd4ce64d051b7a\",\"seller_trade_pubkey\":\"a640eaa86053198a0d6079da122aaa4327b4efe6d7370faf6ebef3a0e11a976e\",\"buyer_invoice\":\"spiralboot92@walletofsatoshi.com\",\"created_at\":null,\"expires_at\":null},{\"id\":\"f6ea4b1d-f92c-49b9-a777-d3b472a6e036\",\"kind\":\"buy\",\"status\":\"active\",\"amount\":881,\"fiat_code\":\"MLC\",\"min_amount\":null,\"max_amount\":null,\"fiat_amount\":2,\"payment_method\":\"Transferencia Bancaria\",\"premium\":0,\"buyer_trade_pubkey\":\"32b89ba2bc14d9a93ae39cf51ca5a24a9cb3eb9eaba672fee6bc19d318eca034\",\"seller_trade_pubkey\":\"cd0bba88405fadb96aa481b34f9acdffbfe9a6ec09f620f833d883a077d59dad\",\"buyer_invoice\":\"spiralboot92@walletofsatoshi.com\",\"created_at\":null,\"expires_at\":null},{\"id\":\"abfa3fb4-01fb-469a-b763-9f8e3ff7de1e\",\"kind\":\"buy\",\"status\":\"active\",\"amount\":856,\"fiat_code\":\"CUP\",\"min_amount\":null,\"max_amount\":null,\"fiat_amount\":400,\"payment_method\":\"EnZona\",\"premium\":0,\"buyer_trade_pubkey\":\"0aaa43d21584548f8929cf01f0b93e4e15b22c158a7af14661390cc036ccbf64\",\"seller_trade_pubkey\":\"8c20092c30445ca76f1c1a7b0e6e796d04436517b641baccc1763229bbcf4e5f\",\"buyer_invoice\":\"spiralboot92@walletofsatoshi.com\",\"created_at\":null,\"expires_at\":null},{\"id\":\"2dfd3da6-a189-445e-9fdc-a11c7c6a1f24\",\"kind\":\"buy\",\"status\":\"fiat-sent\",\"amount\":645,\"fiat_code\":\"CUP\",\"min_amount\":null,\"max_amount\":null,\"fiat_amount\":300,\"payment_method\":\"Transfermovil\",\"premium\":0,\"buyer_trade_pubkey\":\"ad593988f36a07dc004ac3e12fad465acf8d34b649b65babe0c49b9a9a3f1ff0\",\"seller_trade_pubkey\":\"b016c0fdedd78975a8fa39f397fe4fdd8a89b6cbfd51a107ec39c3fcfdeb4616\",\"buyer_invoice\":\"spiralboot92@walletofsatoshi.com\",\"created_at\":null,\"expires_at\":null},{\"id\":\"45c02db6-5d21-4c43-919c-eb194c76ca0f\",\"kind\":\"sell\",\"status\":\"pending\",\"amount\":0,\"fiat_code\":\"CUP\",\"min_amount\":null,\"max_amount\":null,\"fiat_amount\":500,\"payment_method\":\"test\",\"premium\":0,\"seller_trade_pubkey\":\"de66228dbb847eb3f04a720e6e63573c42c6767cee0e40b48740ff69c38f3f2e\",\"created_at\":null,\"expires_at\":null}]}}}"
2025-11-19T00:51:23.785434Z INFO mostrod::app::last_trade_index: User with pubkey: d1ebe4b36e251d5f30da171e6df700b616030775929eb0bc1e32b830ac82640e requested last trade index
2025-11-19T00:51:23.785710Z INFO mostrod::app::last_trade_index: Last trade index: 51
2025-11-19T00:51:23.785804Z INFO mostrod::util: sender key 82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390 - receiver key 519c20f0dc60797d3a8b344123d035bd2f8e80d838fd9729a6cc2774d3de97bf
2025-11-19T00:51:23.786363Z INFO mostrod::util: Sending DM, Event ID: 4494a94cb215108b21ecea5440956e6246aeaf18a2dfa158cdcef0c17efde444 to 519c20f0dc60797d3a8b344123d035bd2f8e80d838fd9729a6cc2774d3de97bf with payload: "{\"restore\":{\"version\":1,\"request_id\":null,\"trade_index\":51,\"action\":\"last-trade-index\",\"payload\":null}}"pero la app solo muestra 2 ordenes
estos son los logs de la app en 2 momentos diferentes cuando intenté resataurar ese usuario
1er intento:
logs.txt
en este me llama la atención esto, que no salió la 2da vez que lo intenté:
[ERROR:flutter/runtime/dart_vm_initializer.cc(40)] Unhandled Exception: Invalid argument(s): Trade key does not match derived key
#0 SessionStorage._decodeSession (package:mostro_mobile/data/repositories/session_storage.dart:40:7)
#1 SessionStorage.fromDbMap (package:mostro_mobile/data/repositories/session_storage.dart:25:12)
#2 BaseStorage.find.<anonymous closure> (package:mostro_mobile/data/repositories/base_storage.dart:75:23)
#3 MappedListIterable.elementAt (dart:_internal/iterable.dart:442:31)
#4 ListIterator.moveNext (dart:_internal/iterable.dart:371:26)
#5 new _List._ofEfficientLengthIterable (dart:core-patch/array.dart:170:27)
#6 new _List.of (dart:core-patch/array.dart:134:20)
#7 new List.of (dart:core-patch/array_patch.dart:39:50)
#8 ListIterable.toList (dart:_internal/iterable.dart:224:7)
#9 BaseStorage.find (package:mostro_mobile/data/repositories/base_storage.dart:76:10)
<asynchronous suspension>
#10 SessionNotifier._cleanup (package:mostro_mobile/shared/notifiers/session_notifier.dart:73:29)
<asynchronous suspension>
2do intento:
logs1.txt
Estas son las 8 ordenes, las que muestra la UI son la de trade index 51 y 48
1. Order ID: d4950e52-cc57-41b9-bb69-fc58228f05d9
Trade Index: 51
Status: "active"
2. Order ID: 1400e742-7db4-4363-b315-03b6b6918111
Trade Index: 33
Status: "active"
3. Order ID: 8221930d-bf6b-40bb-b2da-a8f4e9315dfb
Trade Index: 34
Status: "active"
4. Order ID: e186ee33-a264-4503-a25c-06e4397a82cf
Trade Index: 37
Status: "active"
5. Order ID: f6ea4b1d-f92c-49b9-a777-d3b472a6e036
Trade Index: 38
Status: "active"
6. Order ID: abfa3fb4-01fb-469a-b763-9f8e3ff7de1e
Trade Index: 40
Status: "active"
7. Order ID: 2dfd3da6-a189-445e-9fdc-a11c7c6a1f24
Trade Index: 43
Status: "fiat-sent"
8. Order ID: 45c02db6-5d21-4c43-919c-eb194c76ca0f
Trade Index: 48
Status: "pending"
Excelente review, voy a fusionar el PR, cuando puedas @Catrya crea un issue en ingles con esta info así lo tenemos pendiente |
|
@BraCR10 great job!!! congratulations on your first Mostro contribution 🥳 Please add a lightning invoice with amount of 400k sats as a comment of this PR |
|
Thks sir! @grunch
|
Payment hash: 6e3a67b9421b6deffb80d3726860d127a8113ce63572cafbbe5615a3aa970896 |
3 participants
Restore Feature Implementation
This PR introduces the restore feature to regenerate order states.
Below are the key details and implementation notes:
1. Main Logic
The core logic for the restore process is located in
mobile/lib/features/restore/restore_manager.dart.The method
initRestoreProcess()can be triggered in three different ways:mobile/lib/features/key_manager/key_management_screen.dart2. Clearing Current State
Before starting the restore process, the current state is cleared to ensure consistency:
3. Blocking Old Events During Restore
A new flag was added in
mobile/lib/features/order/notifiers/abstract_mostro_notifier.dartto block processing of old incoming events during the restoration process.
This prevents outdated data from interfering with the new restored state.
4. Restore Delay
An 8-second delay is introduced before updating the order state with the data received from Mostro’s restore responses.
This delay helps avoid overwriting the restored state with old events that may still be arriving during subscription setup.
5. TODO: Dispute Creator Determination
There’s a pending
TODOin_determineIfUserInitiatedDisputesince it’s still unclear how to determine the dispute creator with the current information provided by Mostro during restoration events.6. Mostro Message Updates
Minor updates were made to
mobile/lib/data/models/mostro_message.dartto support restore request messages, in addition to the existing order messages.
7. Mnemonic Validation Utility
A new file was created:
mobile/lib/shared/utils/mnemonic_validator.dartThis handles mnemonic word validation to ensure input accuracy before triggering restore operations.
Feedback
If you have any feedback or suggestions for improvement, please let me know. 🙏
Summary by CodeRabbit
New Features
Bug Fixes
Documentation