Polling Bug

[dewabisma]

Summary

Fixed the bug of polling visited accounts even though not active.

---

Note

Medium Risk
Changes when balances and history refresh across multi-account wallets; non-active views may stay stale until opened, while fixing incorrect background polling of inactive accounts.

Overview
Background and manual history polling no longer refresh every wallet account or the global pagination controller. Polling is centralized in polling_refresh_scope, which limits routine updates to the active account (silent refresh and balance invalidation only when the filtered pagination provider is already alive).

Event-driven paths still target the right accounts: pending-tx discovery and reconciliation refresh the active account plus senders/receivers involved in pending txs, without fan-out to unrelated accounts. Reconciliation now matches pending items against per-account filtered pagination cache instead of the aggregate allTransactions stream.

Reversible transfer monitoring watches the active account’s transaction list only, updates pagination for affected from/to accounts on execution, and drops wallet-wide controller updates. Account switch triggers a dedicated refresh (silent vs loading based on whether chain data was loaded).

Unit tests cover refresh target deduplication and reconciliation account scoping.

^{Reviewed by Cursor Bugbot for commit 62e6719. Bugbot is set up for automated code reviews on this repo. Configure here.}

[n13]

I have enough context. Here's my review:

PR #502 Review: Polling Bug Fix

The refactor is a solid direction — narrowing polling to the active account avoids the wasteful fan-out across every visited wallet, and the new polling_refresh_scope.dart cleanly centralizes the scoping logic. The unit tests on accountRefreshTargets / reconciliationAccountIds are a nice touch. That said, there are a few real issues that should be addressed before merging.

Critical: Double-scheduled timer on activeId == null

Cursor Bugbot's finding is correct, and now actually reachable.

    try {
      final activeId = activeAccountId(_ref);
      if (activeId == null) {
        _scheduleNextPoll();   // ← inner schedule
        return;
      }
      ...
    } catch (e) {
      ...
    } finally {
      if (_isPolling) {
        _scheduleNextPoll();   // ← finally also schedules → orphans the first timer
      }
    }

The finally block runs on return, so when activeId == null you assign _pollingTimer twice; the first Timer is lost but still fires, effectively doubling poll cadence each time it triggers. The same shape existed before with the accountsState empty check, but that branch was guarded by the accountsProvider listener that calls stopPolling(). The new activeId == null branch is much more reachable (account switching, transient activeAccountProvider loading), so what was latent is now an actual bug.

Fix: drop the inner call; the finally already handles it.

if (activeId == null) return;

Concern: receive notifications won't fire for non-active accounts

notification_integration_service.dart listens to the wallet-wide paginationControllerProvider to detect newly received transfers and fire _notifyTokenReceived:

    _ref.listen(paginationControllerProvider, (previous, next) {
      if (previous != null && previous.otherTransfers.isNotEmpty) {
        ...
        final newReceiveTxs = newTxs.where((tx) {
          if (tx is! TransferEvent) return false;
          return accountIds.contains(tx.to) && !accountIds.contains(tx.from);
        });

        for (final receiveTokenTx in newReceiveTxs) {
          _notifyTokenReceived(receiveTokenTx as TransferEvent);
        }
      }
    });

The PR removes every paginationControllerProvider.silentRefresh() call from polling, so this global controller will no longer be ticked by the timer. That means an incoming transfer to an account that isn't currently selected won't drive this listener — i.e. no "you received X" notification — until the user switches to that account. Worth confirming whether that's acceptable, or whether paginationControllerProvider still needs a periodic silent refresh (or whether the notification listener should move to a different signal).

Concern: reconciliation can silently skip the active account

In the reconciliation rewrite:

for (final accountId in accountIds) {
  await refreshAccountsPagination(
    _ref,
    accountIds: [accountId],
    action: (notifier) => notifier.silentRefresh(),
    onlyIfAlive: accountId == activeId,
  );
}
final confirmedTransactions = _loadConfirmedTransactions(txService, accountIds);

…and:

final pagination = _ref.read(filteredPaginationControllerProviderFamily(params));
if (!pagination.hasLoadedChainData) continue;

Two things compound:

1. For the active account we use onlyIfAlive: true. If the user is on a screen that doesn't currently keep that filtered pagination provider alive (e.g., settings, send flow), the refresh is skipped — and then _loadConfirmedTransactions reads the freshly instantiated provider, sees hasLoadedChainData == false, and silently skips it. So the active-account pending tx won't be reconciled until that screen is reopened. The previous code used allTransactionsProvider, which is referenced by other consumers and was reliably kept up to date.
2. hasLoadedChainData is otherTransfers.isNotEmpty || scheduledReversibleTransfers.isNotEmpty — for a brand-new wallet account with no history, it's permanently false. Not a real regression (nothing to match anyway), just be aware the per-account skip can mask "no data yet".

Suggested fix: don't gate the active account on onlyIfAlive, or perform a forced silentRefresh on the active account regardless before calling _loadConfirmedTransactions.

Minor: triggerSilentHistoryRefresh no longer adds the new tx to the global pagination controller

Old code did mainController.addTransactionToHistory(newTransaction); mainController.silentRefresh(); against the wallet-wide controller. The new code only updates per-account filtered controllers. Combined with the notification-service point above, anything still reading paginationControllerProvider directly will be stale until something else refreshes it.

Minor: DRY between refreshAccountsPagination and updatePaginationFiltersFor

Both iterate over filters and invoke a callback per (accountIds, filter) notifier. The new helper adds onlyIfAlive and async support; the old one is still sync with Reader. The filters param added to updatePaginationFiltersFor doesn't appear to have any callers passing it. Consider either consolidating into a single API or dropping the unused filters param to keep this from drifting.

Nits

• print('Performing global history poll for active account...') — most of the controller code already uses quantusDebugPrint. Mixed-mode logging is preexisting, but worth noting.
• refreshAccountsPagination is missing a doc comment while its peers have them.
• In the activeAccountProvider listener you skip when previousId == null, which means the first account assignment after app launch never triggers refreshActiveAccountOnSwitch. Likely intentional (handled by the initial bring-up), but worth a short comment explaining why.

Positives

• Active-account-only scoping is a meaningful efficiency win.
• Awaiting reconcilePendingTransactions() and the manual refresh now produces sane ordering.
• ProviderSubscription lifecycle in ReversibleTransferMonitoringService is properly closed on lifecycle changes — good resource hygiene.
• accountRefreshTargets correctly dedupes active vs affected.
• Switching the reversible monitor to listen to activeAccountPaginationProvider(TransactionFilter.all) is a clean fix.

Recommendation

Request changes — primarily for the timer double-scheduling bug (small fix) and the reconciliation/notification regressions (need a decision on whether paginationControllerProvider still needs to be ticked, or whether the notification listener should move).

PR link: Polling Bug #502

[dewabisma]

Concern: receive notifications won't fire for non-active accounts

We already have remote notification. So this is intentional to not send notification for non active account.

Concern: reconciliation can silently skip the active account

This is nonsense. We don't skip. We don't do recon if there is nothing to recon.

[cursor]

Cursor Bugbot has reviewed your changes and found 3 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 62e6719. Configure here.}

[cursor]

Reconciliation onlyIfAlive condition is inverted

Medium Severity

The onlyIfAlive: accountId == activeId condition is inverted. This sets onlyIfAlive to true for the active account (potentially skipping it) and false for non-active accounts (unconditionally creating their providers). The intent is the opposite: the active account's provider is the one that's definitely alive and always worth refreshing, while non-active accounts involved in pending transactions are the ones that might not have providers alive and shouldn't have them created. The condition likely needs to be accountId != activeId to align with the PR's goal of not polling non-active accounts.

 

^{Reviewed by Cursor Bugbot for commit 62e6719. Configure here.}

[cursor]

Subscription not closed in dispose method

Medium Severity

The new _txSubscription field is closed in the lifecycle handler and in _listenToTransactions, but dispose() does not close it. When the provider is torn down via ref.onDispose(service.dispose), the ProviderSubscription will remain open, leaking the listener and preventing proper garbage collection.

 

^{Reviewed by Cursor Bugbot for commit 62e6719. Configure here.}

[cursor]

Silent refresh triggered twice for active account

Low Severity

triggerSilentRefresh calls silentRefreshActiveAccount(_ref) directly, and then _reversibleMonitor.forceCheckAllMonitoredTransfers() also calls silentRefreshActiveAccount(_ref) internally when there are active execution pollers. This results in the active account's pagination being refreshed twice redundantly during a single silent refresh cycle.

Additional Locations (1)

• mobile-app/lib/services/reversible_transfer_monitoring_service.dart#L180-L184

 

^{Reviewed by Cursor Bugbot for commit 62e6719. Configure here.}