[NWC] Phase 5: Payment notifications and enhanced UX#472
Conversation
- Subscribe to NWC notification events (kind 23196) for real-time payment_received and payment_sent updates - Add NwcNotificationListener at app level for in-app snackbar notifications on payment events - Add pre-flight balance check before payment attempts - Add payment receipt widget with amount breakdown, fees, preimage, and timestamp - Add connection health monitoring with periodic checks (30s) - Add auto-reconnect with exponential backoff on connection drops - Add periodic balance refresh (60s) - Add lookup_invoice verification after successful payments - Add connection health indicator (green/orange) in wallet settings - Add NwcConnectionStatusIndicator reusable widget - Add low balance and connection unstable warnings in payment flow - Add 15 new localization strings (EN, ES, IT) - Add NWC_PHASE5_IMPLEMENTATION.md documentation Closes #460
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughImplements NWC Phase 5, adding real-time payment notifications (NIP-47), connection health monitoring with auto-reconnect, pre-flight balance verification, and enhanced payment receipts. Extends NwcClient with notification streaming, introduces three new UI widgets for status display and notifications, and integrates comprehensive lifecycle management into NwcNotifier. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Mobile Client
participant Notifier as NwcNotifier
participant Service as NwcClient
participant Relay as Nostr Relay
participant Wallet as NWC Wallet
Note over Client,Wallet: Connection & Initialization
Client->>Notifier: connect(uri)
Notifier->>Service: connect(uri)
Service->>Relay: Connect WebSocket
Relay-->>Service: Connected
Service->>Relay: Subscribe to kind 23196 (notifications)
Relay-->>Service: Subscribed
Service->>Service: _startHealthChecks()
Service->>Service: _startBalanceRefresh()
Service-->>Notifier: Connected
Notifier-->>Client: State updated (connected)
Note over Client,Wallet: Real-time Notification Flow
Wallet->>Relay: Send NWC notification (kind 23196)
Relay->>Service: Relay notification event
Service->>Service: Decrypt & parse notification
Service->>Service: Emit NwcNotification via stream
Notifier->>Service: Listen to notifications stream
Notifier->>Notifier: Update connectionHealthy
Notifier->>Notifier: Update lastSuccessfulContact
Notifier->>Client: Trigger UI update
Client->>Client: NwcNotificationListener shows SnackBar
Note over Client,Wallet: Health Check Loop
Service->>Service: _checkHealth() periodic timer
Service->>Service: preFlightBalanceCheck()
Service->>Wallet: Request balance via NWC
Wallet-->>Service: Return balance
Service->>Notifier: Update connectionHealthy
Service->>Client: UI reflects health status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🤖 Fix all issues with AI agents
In `@docs/NWC_PHASE5_IMPLEMENTATION.md`:
- Around line 168-186: The localization table declares 15 new strings but only
lists 14; add the missing keys (at minimum nwcConnectionError,
nwcPaymentSuccess, and nwcPreimageLabel) to the EN/ES/IT localization entries so
the table actually contains all 15 items, include appropriate translations for
each language, update the table header/count to match 15, and verify references
in nwc_connection_status_indicator.dart and nwc_payment_receipt_widget.dart
still match the exact key names.
In `@lib/features/wallet/providers/nwc_provider.dart`:
- Around line 286-310: _handleConnectionDrop currently cancels timers via
connect->_cleanup and can be re-entered concurrently causing orphaned timers and
double-incremented _reconnectAttempts; fix by adding a reentrancy guard (e.g. a
private bool _isReconnecting) at the top of _handleConnectionDrop to return
early if already reconnecting, then convert _handleConnectionDrop into an
internal retry loop that increments _reconnectAttempts once per attempted
reconnect and performs exponential backoff internally (use Duration(seconds: 1
<< _reconnectAttempts)); to avoid _cleanup cancelling timers on failed attempts,
either add a connect parameter (e.g. skipCleanup: true) or implement a private
connect variant that does not call _cleanup so that failed attempts do not stop
the health/balance timers — only on final success or when giving up should
normal cleanup/reset logic run; ensure you reference and update
_reconnectAttempts, _maxReconnectAttempts, mounted, state.status,
_healthCheckTimer and _balanceRefreshTimer accordingly.
In `@lib/services/nwc/nwc_client.dart`:
- Around line 203-216: In _subscribeToNotifications add a since parameter to the
NostrFilter used in the NostrRequest so the subscription only fetches new events
(prevent replay of historical kind 23196 notifications); update the NostrFilter
(the one with kinds: const [23196], authors: [connection.walletPubkey]) to
include since: DateTime.now().millisecondsSinceEpoch (or convert to the expected
unix-timestamp type) when constructing the NostrRequest in order to subscribe
from "now" onward.
In `@lib/shared/widgets/nwc_notification_listener.dart`:
- Around line 41-44: _subscribe currently captures a single notifier instance
via ref.read(nwcProvider.notifier) so the _subscription will point to a stale
stream if the nwcProvider is invalidated; change the logic to react to provider
changes by using ref.listen on the provider (or on a dedicated notifications
stream provider) and re-subscribe to the new notifier's notifications stream,
cancelling the previous _subscription before assigning the new one (keep using
_onNotification as the listener); ensure the resubscribe happens whenever
nwcProvider.notifier changes so the listener always attaches to the live stream.
In `@lib/shared/widgets/nwc_payment_widget.dart`:
- Around line 109-113: The code fires _verifyPayment() without awaiting and then
immediately calls widget.onPaymentSuccess, which can lead to
ref.read(nwcProvider.notifier) running after the ConsumerState is disposed;
update the flow so _verifyPayment is either awaited before calling
widget.onPaymentSuccess OR add a mounted guard at the start of _verifyPayment to
return early if the state is no longer mounted, and additionally check mounted
immediately before any use of ref (e.g., before calling
ref.read(nwcProvider.notifier)) to avoid accessing ref on a disposed
ConsumerState.
🧹 Nitpick comments (5)
lib/shared/widgets/nwc_notification_listener.dart (1)
110-117:_formatSatsis duplicated across four files.The identical helper appears in
nwc_notification_listener.dart,nwc_connection_status_indicator.dart,nwc_payment_receipt_widget.dart, andnwc_payment_widget.dart. Extract it to a shared utility (e.g.,lib/shared/utils/format_utils.dart).lib/services/nwc/nwc_client.dart (2)
74-76:_notificationControlleris never closed.The broadcast
StreamControlleris created in the constructor but not closed indisconnect(). If theNwcClientis discarded after disconnect, this leaks. If the client is meant to be reconnectable (reconnect after disconnect), closing it indisconnect()would break that. Consider adding adispose()method for final teardown, or closing it indisconnect()and recreating onconnect().
11-23:NwcNotificationis a simple data class — consider making it immutable with@immutableannotation.Minor nit: the class is effectively immutable already (both fields are
final), but adding@immutableannotation documents the intent and enables analyzer checks.lib/shared/widgets/nwc_payment_receipt_widget.dart (1)
215-218: Timestamp format is hardcoded and not locale-aware.
_formatTimestampalways producesYYYY-MM-DD HH:mmregardless of the user's locale. For a payment receipt this is generally acceptable, but consider usingDateFormatfromintlfor locale-sensitive formatting if the project already depends on it.lib/features/wallet/providers/nwc_provider.dart (1)
261-283: Non-timeout health-check failures leave connection marked unhealthy with no recovery path.When
_checkHealthcatches a non-timeout exception (line 279), it setsconnectionHealthy: falsebut does not call_handleConnectionDrop. If the error persists, the health timer keeps firing and marking the connection unhealthy, but no reconnect is ever attempted. Consider whether persistent non-timeout failures (e.g., decryption errors, unexpected wallet responses) should also eventually trigger a reconnect.
- Add 'since: DateTime.now()' filter to notification subscription to prevent replaying historical events on connect - Add reentrancy guard (_isReconnecting) to _handleConnectionDrop and convert to internal retry loop to prevent orphaned timers and double-incremented reconnect attempts - Re-subscribe to notification stream when NWC provider reconnects to prevent stale subscription after disconnect/reconnect cycle - Add mounted guard in _verifyPayment() to prevent accessing ref after widget disposal - Fix documentation: correct localization string count (14 new, not 15) and note that nwcConnectionError, nwcPaymentSuccess, nwcPreimageLabel were added in earlier phases
2 participants
Summary
Final phase of NWC integration: real-time payment notifications via NIP-47 kind 23196 events, enhanced payment UX with pre-flight balance checks and payment receipts, connection resilience with auto-reconnect, and wallet health monitoring.
Closes #460
Changes
Real-time Notifications (NIP-47 kind 23196)
payment_receivedandpayment_sentnotification events from the walletNwcNotificationListenerat app level shows floating snackbar notifications for payment eventsptagEnhanced Payment UX
pay_invoice, verify sufficient balance with a freshget_balancecallConnection Resilience
get_balanceto verify connection is aliveWallet Balance
New Files
lib/shared/widgets/nwc_connection_status_indicator.dart— Compact connection health widgetlib/shared/widgets/nwc_payment_receipt_widget.dart— Payment receipt with amount breakdownlib/shared/widgets/nwc_notification_listener.dart— App-level notification snackbar listenerdocs/NWC_PHASE5_IMPLEMENTATION.md— Detailed implementation documentationModified Files
lib/services/nwc/nwc_client.dart— Notification subscription (kind 23196),NwcNotificationmodellib/features/wallet/providers/nwc_provider.dart— Auto-reconnect, health checks, balance refresh timers, notification forwarding,preFlightBalanceCheck(),lookupInvoice()lib/shared/widgets/nwc_payment_widget.dart— Pre-flight check, receipt, health/balance warningslib/features/wallet/screens/wallet_settings_screen.dart— Health indicator (green/orange)lib/core/app.dart—NwcNotificationListenerintegrationlib/l10n/intl_en.arb,intl_es.arb,intl_it.arb— 15 new stringsLocalization
All 15 new strings added in EN, ES, and IT.
References
Summary by CodeRabbit
Release Notes
New Features
Improvements