revert: roll back Riverpod 3.x migration (#613) and dependent PRs (#624, #625)#628
Conversation
WalkthroughDowngrades Flutter SDK to ~3.32.x, Riverpod to ChangesDependency Downgrade, Transport Simplification, and Test Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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: 44556d6475
ℹ️ 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: 4
🧹 Nitpick comments (1)
test/features/mostro/mostro_nodes_notifier_test.dart (1)
45-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
.thenAnswerfor dynamic state access to avoid re-stubbing.Line 52 uses
.thenReturn(mockSettingsNotifier.state)which captures the state value at stub time. This requires re-stubbing in lines 120-121, 143-144, 162-163, 182-183, and 288-289 whenever the state changes.The integration test (mostro_integration_test.dart:72-77) and performance test use
.thenAnswer((_) => mockSettingsNotifier.state)to dynamically read the current state, eliminating the need for re-stubbing.♻️ Recommended fix for consistency and maintainability
mockSettingsNotifier.state = makeSettings(); - when(mockRef.read(settingsProvider)).thenReturn(mockSettingsNotifier.state); + when(mockRef.read(settingsProvider)) + .thenAnswer((_) => mockSettingsNotifier.state); when(mockRef.read(settingsProvider.notifier)) .thenReturn(mockSettingsNotifier);Then remove the re-stubbing at lines 120-121, 143-144, 162-163, 182-183, and 288-289:
mockSettingsNotifier.state = makeSettings( mostroPublicKey: unknownPubkey, ); - when(mockRef.read(settingsProvider)) - .thenReturn(mockSettingsNotifier.state); final notifier = createNotifier();🤖 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 `@test/features/mostro/mostro_nodes_notifier_test.dart` around lines 45 - 58, In the setUp method, change the stub for mockRef.read(settingsProvider) from using .thenReturn(mockSettingsNotifier.state) to using .thenAnswer((_) => mockSettingsNotifier.state) so it dynamically accesses the current state instead of capturing it at stub time. This dynamic approach will eliminate the need to re-stub this mock in individual test methods whenever the state changes, so you can remove all the re-stubbing occurrences at lines 120-121, 143-144, 162-163, 182-183, and 288-289.
🤖 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 @.github/workflows/release.yml:
- Line 138: Flutter versions are inconsistent across the project configuration
files (.fvmrc, .github/workflows/desktop.yml, .github/workflows/flutter.yml,
.github/workflows/main.yml, and .github/workflows/release.yml), which creates
environment mismatches between local development and CI workflows. Select a
single Flutter version that meets your project's compatibility and testing
requirements, then update the flutter-version property in all workflow files and
the version field in .fvmrc to use this unified version consistently across all
configuration files.
In `@lib/data/repositories/dispute_repository.dart`:
- Around line 55-60: The wrap() method call in the dispute message handling is
missing the masterKey and keyIndex parameters that are needed to respect the
user's privacy mode settings. Update the wrap() call to conditionally pass
masterKey and keyIndex parameters from the session object, similar to how it's
done in mostro_service.dart around lines 366-372. The session object (defined at
line 28) contains the fullPrivacy, masterKey, and keyIndex fields needed for
this conditional logic.
In `@lib/features/relays/relays_notifier.dart`:
- Line 691: The comment containing "🔥 RESET COMPLETO: Limpiar todos los relays
y hacer sync fresco" at line 691 in relays_notifier.dart is written in Spanish
and violates the English-only commenting guideline. Replace this Spanish comment
with an English equivalent that conveys the same intent about performing a
complete reset and fresh synchronization of the relays. Ensure the translated
comment maintains the same meaning and context as the original.
In `@pubspec.yaml`:
- Line 69: Replace the unmaintained lucide_icons package in pubspec.yaml with a
maintained alternative such as iconify_flutter or lucide_icons_flutter. After
updating the dependency version, update all imports and icon references in
custom_drawer_overlay.dart where LucideIcons.user, LucideIcons.settings, and
LucideIcons.info are currently used to match the API of the new package you
choose.
---
Nitpick comments:
In `@test/features/mostro/mostro_nodes_notifier_test.dart`:
- Around line 45-58: In the setUp method, change the stub for
mockRef.read(settingsProvider) from using
.thenReturn(mockSettingsNotifier.state) to using .thenAnswer((_) =>
mockSettingsNotifier.state) so it dynamically accesses the current state instead
of capturing it at stub time. This dynamic approach will eliminate the need to
re-stub this mock in individual test methods whenever the state changes, so you
can remove all the re-stubbing occurrences at lines 120-121, 143-144, 162-163,
182-183, and 288-289.
🪄 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: 223a53a2-fb6f-47ed-b9ce-7b47505b5c70
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
.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/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/settings/settings_notifier.dart
- lib/features/walkthrough/providers/first_run_provider.dart
- lib/features/notifications/providers/notifications_provider.dart
- lib/shared/notifiers/session_notifier.dart
- lib/features/order/notifiers/abstract_mostro_notifier.dart
- lib/services/exchange_service.dart
- lib/features/home/providers/home_order_providers.dart
- lib/features/chat/providers/chat_room_providers.dart
- lib/features/chat/notifiers/chat_rooms_notifier.dart
- assets/data/payment_methods.json
- android/gradle.properties
- test/features/restore/restore_decode_test.dart
- lib/features/disputes/notifiers/dispute_chat_notifier.dart
- lib/features/chat/notifiers/chat_room_notifier.dart
- lib/features/mostro/mostro_nodes_notifier.dart
- lib/features/chat/chat_room_provider.dart
- lib/features/trades/providers/trades_provider.dart
- lib/features/order/providers/payment_methods_provider.dart
- lib/features/notifications/notifiers/notifications_notifier.dart
- lib/shared/providers/exchange_service_provider.dart
- lib/features/community/providers/community_selector_provider.dart
- lib/features/logs/logs_provider.dart
- lib/features/auth/providers/auth_notifier_provider.dart
- lib/features/wallet/providers/nwc_provider.dart
- test/data/mostro_message_nip44_test.dart
Summary
Reverts the Riverpod 3.x migration (#613), which broke the app, and the two PRs merged on top of it (#624, #625). This restores
mainto the exact state of #619.Reverted commits (LIFO order)
feat: add MWK (Malawi Kwacha) payment methodsfeat: add dual send for transport v2 (Phase B)feat: migrate to Riverpod 3.xVerification
git diff fe3a897f HEADis empty).Follow-up
The content of #624 and #625 is re-applied — without any Riverpod 3.x changes — in two separate PRs that stack on top of this one:
feat/transport-v2-phase-b-reapplyfeat/mwk-payment-methods-reapplyMerge this PR first.
Summary by CodeRabbit
Bug Fixes
Chores