fix: resolve provider init and node-switch restore connectivity races - #631
Conversation
|
Warning Review limit reached
More reviews will be available in 52 minutes and 12 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR downgrades the Flutter SDK (~3.44 → ~3.32), Android build toolchain (Gradle 8.14→8.9, AGP 8.11→8.6, Kotlin 2.2→2.1), and Riverpod from 3.3.2 to 2.6.1 while replacing ChangesToolchain and dependency downgrade
Transport simplification, restore improvements, and functional fixes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a830acc9ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/shared/widgets/bottom_nav_bar.dart (1)
8-9: 🎯 Functional Correctness | 🟡 MinorVerify that notification counters are written to somewhere in the codebase.
The
chatCountProviderandorderBookNotificationCountProviderare initialized to0but have no writers anywhere in the codebase—they remain dead counters that never trigger the notification dots (line 108). Additionally,orderBookNotificationCountProvideris wired to the myTrades nav item (index 1), not the order book item (index 0), suggesting either the name is misleading or the wiring is incorrect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/shared/widgets/bottom_nav_bar.dart` around lines 8 - 9, The chatCountProvider and orderBookNotificationCountProvider are initialized but lack any writers in the codebase to update their values, making them dead code that never triggers the notification dots at line 108. Additionally, orderBookNotificationCountProvider is wired to the myTrades navigation item (index 1) instead of the orderBook item (index 0), indicating either the provider name is misleading or the wiring is incorrect. Add code that writes to these providers whenever chat messages or relevant notifications are received from your backend or data sources, and verify that the provider wiring in the BottomNavigationBar construction matches the intended navigation items—either rename orderBookNotificationCountProvider to reflect its actual use with myTrades, or rewire it to the correct navigation index if the name is correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.fvmrc:
- Line 2: The Flutter SDK version in .fvmrc is pinned to 3.35.7 while CI
workflows (flutter.yml, desktop.yml, main.yml, and release.yml) use inconsistent
versions like 3.32.5 and 3.32.2, causing divergence between local development
and CI environments. Choose a single Flutter version and update the flutter
field in .fvmrc and all occurrences in the CI workflow files to use that same
version consistently across all configurations.
In `@lib/data/repositories/dispute_repository.dart`:
- Around line 55-57: The `wrap()` method call on `disputeMessage` at line 55 is
missing the `masterKey` and `keyIndex` parameters that are present in the
standard publish path. Add these missing parameters to the `wrap()` call to
ensure that identity and trade-index semantics are properly preserved for
non-full-privacy sessions. Include both parameters with their appropriate values
to align with how wrap is called elsewhere in the publish flow.
In `@lib/features/relays/relays_notifier.dart`:
- Line 691: The inline comment marked with "// 🔥 RESET COMPLETO: Limpiar todos
los relays y hacer sync fresco" is written in Spanish and violates the coding
guideline requiring all comments to be in English. Translate this comment to
English while preserving its meaning about performing a complete reset, cleaning
all relays, and performing a fresh synchronization.
---
Outside diff comments:
In `@lib/shared/widgets/bottom_nav_bar.dart`:
- Around line 8-9: The chatCountProvider and orderBookNotificationCountProvider
are initialized but lack any writers in the codebase to update their values,
making them dead code that never triggers the notification dots at line 108.
Additionally, orderBookNotificationCountProvider is wired to the myTrades
navigation item (index 1) instead of the orderBook item (index 0), indicating
either the provider name is misleading or the wiring is incorrect. Add code that
writes to these providers whenever chat messages or relevant notifications are
received from your backend or data sources, and verify that the provider wiring
in the BottomNavigationBar construction matches the intended navigation
items—either rename orderBookNotificationCountProvider to reflect its actual use
with myTrades, or rewire it to the correct navigation index if the name is
correct.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da40a1bd-d05e-4cb1-b19d-7ba6eab7a6c0
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (85)
.fvmrc.github/workflows/desktop.yml.github/workflows/flutter.yml.github/workflows/main.yml.github/workflows/release.ymlandroid/app/build.gradleandroid/gradle.propertiesandroid/gradle/wrapper/gradle-wrapper.propertiesandroid/settings.gradleassets/data/payment_methods.jsondocs/architecture/SESSION_RECOVERY_ARCHITECTURE.mddocs/architecture/TRANSPORT_V2_MIGRATION.mdlib/data/models/mostro_message.dartlib/data/repositories/dispute_repository.dartlib/data/repositories/open_orders_repository.dartlib/features/auth/notifiers/auth_notifier.dartlib/features/auth/providers/auth_notifier_provider.dartlib/features/chat/chat_room_provider.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/notifiers/chat_rooms_notifier.dartlib/features/chat/providers/active_chat_screens_provider.dartlib/features/chat/providers/chat_room_providers.dartlib/features/chat/providers/chat_tab_provider.dartlib/features/chat/widgets/chat_messages_list.dartlib/features/community/providers/community_selector_provider.dartlib/features/disputes/notifiers/dispute_chat_notifier.dartlib/features/disputes/providers/dispute_read_status_provider.dartlib/features/home/providers/home_order_providers.dartlib/features/key_manager/key_management_screen.dartlib/features/logs/logs_provider.dartlib/features/mostro/mostro_instance_provider.dartlib/features/mostro/mostro_nodes_notifier.dartlib/features/mostro/mostro_nodes_provider.dartlib/features/notifications/notifiers/notifications_notifier.dartlib/features/notifications/providers/backup_reminder_provider.dartlib/features/notifications/providers/notifications_provider.dartlib/features/order/notifiers/abstract_mostro_notifier.dartlib/features/order/providers/order_notifier_provider.dartlib/features/order/providers/payment_methods_provider.dartlib/features/relays/relays_notifier.dartlib/features/relays/relays_provider.dartlib/features/restore/restore_manager.dartlib/features/restore/restore_mode_provider.dartlib/features/restore/restore_progress_notifier.dartlib/features/settings/about_screen.dartlib/features/settings/notification_settings_screen.dartlib/features/settings/settings_notifier.dartlib/features/settings/settings_provider.dartlib/features/settings/settings_screen.dartlib/features/trades/providers/trades_provider.dartlib/features/trades/widgets/status_filter_widget.dartlib/features/walkthrough/providers/first_run_provider.dartlib/features/wallet/providers/nwc_provider.dartlib/features/wallet/screens/connect_wallet_screen.dartlib/features/wallet/screens/wallet_settings_screen.dartlib/features/wallet/widgets/wallet_balance_widget.dartlib/features/wallet/widgets/wallet_status_card.dartlib/services/event_bus.dartlib/services/exchange_service.dartlib/services/mostro_service.dartlib/shared/notifiers/locale_notifier.dartlib/shared/notifiers/navigation_notifier.dartlib/shared/notifiers/session_notifier.dartlib/shared/providers/drawer_provider.dartlib/shared/providers/exchange_service_provider.dartlib/shared/providers/navigation_notifier_provider.dartlib/shared/providers/session_notifier_provider.dartlib/shared/widgets/bottom_nav_bar.dartlib/shared/widgets/custom_drawer_overlay.dartlib/shared/widgets/ln_address_confirmation_widget.dartlib/shared/widgets/nwc_connection_status_indicator.dartlib/shared/widgets/nwc_invoice_widget.dartlib/shared/widgets/nwc_notification_listener.dartlib/shared/widgets/nwc_payment_receipt_widget.dartlib/shared/widgets/nwc_payment_widget.dartpubspec.yamltest/data/mostro_message_nip44_test.darttest/features/mostro/mostro_integration_test.darttest/features/mostro/mostro_nodes_notifier_test.darttest/features/mostro/mostro_nodes_performance_test.darttest/features/restore/restore_decode_test.darttest/mocks.darttest/notifiers/add_order_notifier_test.darttest/notifiers/take_order_notifier_test.darttest/services/mostro_service_test.dart
💤 Files with no reviewable changes (25)
- lib/features/order/providers/payment_methods_provider.dart
- lib/features/settings/settings_notifier.dart
- lib/features/notifications/notifiers/notifications_notifier.dart
- lib/features/chat/chat_room_provider.dart
- lib/features/chat/providers/chat_room_providers.dart
- lib/features/notifications/providers/notifications_provider.dart
- lib/features/wallet/providers/nwc_provider.dart
- lib/shared/notifiers/session_notifier.dart
- lib/features/home/providers/home_order_providers.dart
- lib/features/order/notifiers/abstract_mostro_notifier.dart
- lib/features/auth/providers/auth_notifier_provider.dart
- lib/features/mostro/mostro_nodes_notifier.dart
- lib/features/chat/notifiers/chat_rooms_notifier.dart
- lib/services/exchange_service.dart
- assets/data/payment_methods.json
- lib/features/chat/notifiers/chat_room_notifier.dart
- lib/features/logs/logs_provider.dart
- android/gradle.properties
- lib/features/community/providers/community_selector_provider.dart
- test/features/restore/restore_decode_test.dart
- lib/features/disputes/notifiers/dispute_chat_notifier.dart
- lib/features/trades/providers/trades_provider.dart
- lib/shared/providers/exchange_service_provider.dart
- lib/features/walkthrough/providers/first_run_provider.dart
- test/data/mostro_message_nip44_test.dart
…ests - Add _waitForNodeConnectivity to ensure the app is connected to the selected node before restore - Engage bootstrap relays immediately and wait for kind 38385 info event to prove connectivity - Fall back to best-effort attempt if info event not received within discovery window - Add _initRetryTimer to OpenOrdersRepository to poll for NostrService readiness - Defer order subscription until NostrService.isInitialized to avoid
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
82c5612 to
f3ab613
Compare
Summary
Fixes two pre-existing race conditions that surface after the Riverpod 3.x rollback (#628), both caused by code that the revert does not touch:
Exception: Nostr is not initialized. Call init() first.during app start, logged as "Failed to init mostro instance listener".TimeoutException: Stage RestoreStage.gettingRestoreData timed out after 10swhen switching Mostro nodes.Neither is introduced by the revert; the Riverpod 2.x timing change merely exposes them.
Root causes
1. Order repository init race
OpenOrdersRepository's constructor eagerly callssubscribeToEvents(), which throws whenNostrServiceis not yet initialized. The transport Phase A listener (SubscriptionManager._initMostroInstanceListener) forces the first build oforderRepositoryProviderduringSubscriptionManagerconstruction, which can run beforeinit()completes. The thrown error is cached on the provider, poisoning every later read of the order repository.2. Node-switch restore timeout
Since the move to bootstrap relay discovery (#610), no relays are seeded and the selected node's relays are rediscovered asynchronously via kind 10002. On node switch,
initRestoreProcess()sends the restore request immediately, while thenew node's relays take up to ~5s (periodic resync) plus discovery to connect, so the request races connectivity and times out before any relay can route it.
Changes
OpenOrdersRepository._subscribeToOrders(): defer the subscription with a bounded retry whenNostrServiceis not initialized, instead of throwing and poisoning the provider.OpenOrdersRepository.updateSettings(): reset_mostroInstanceon instance change so stale node info is not reported for the new instance.RestoreService: add_waitForNodeConnectivity(), called before sending the restore request. It engages the bootstrap relays immediately (instead of waiting for the periodic watchdog) and waits for the node's info event (kind 38385) within the discovery window, falling back to a best-effort attempt with a live-relay floor.Verification
flutter analyze: no issues.flutter test: all tests pass (473).nodes, and the order book loads cleanly on cold start.
Notes
revert/riverpod-3x-rollback(or rebase ontomainonce revert: roll back Riverpod 3.x migration (#613) and dependent PRs (#624, #625) #628 is merged).Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Chores