fix: flush balance, transaction history, and pubkey watchers on auth event#3168
fix: flush balance, transaction history, and pubkey watchers on auth event#3168takenagain wants to merge 2 commits intodevfrom
Conversation
WalkthroughUpdates introduce private dependency fields and logging in CoinAddressesBloc, add an auth-change event, adjust CoinsBloc to await async cache flush, overhaul activation flow and cache flush in CoinsRepo with refined logging and error handling, and add auth-aware subscription reset/start logic in TransactionHistoryBloc with a new auth-change event. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI
participant CoinsBloc
participant CoinsRepo
participant SDK
User->>UI: Activate assets
UI->>CoinsBloc: activate(assets)
CoinsBloc->>CoinsRepo: activate(assets)
loop For each asset
CoinsRepo->>CoinsRepo: emit "activating"
CoinsRepo->>SDK: sdk.activate(asset)
alt Activation coordination conflict
CoinsRepo->>CoinsRepo: log info (conflict)
CoinsRepo->>CoinsRepo: emit state (expected)
else Success
CoinsRepo->>CoinsRepo: log success\n(parent if applicable)
CoinsRepo->>CoinsRepo: emit "active"
else Error
CoinsRepo->>CoinsRepo: log warning/severe
CoinsRepo->>CoinsRepo: emit "suspended"
end
end
note over CoinsRepo: Async flushCache cancels watchers\nand clears caches with logging
sequenceDiagram
autonumber
participant Auth as AuthService
participant TH as TransactionHistoryBloc
participant SDK
Auth-->>TH: onUserChanged(user/wallet)
TH->>TH: add(TransactionHistoryAuthStateChanged)
TH->>TH: cancel history & new-tx subscriptions\nclear processed IDs\nreset to initial
TH->>SDK: streamHistoricalTx()
SDK-->>TH: historical batches
SDK-->>TH: onDone
TH->>TH: _subscribeToNewTransactions()
SDK-->>TH: new tx stream
TH-->>TH: map to state updates / errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
Visit the preview URL for this PR (updated for commit 8cf4d94): https://walletrc--pull-3168-merge-d4261r31.web.app (expires Fri, 10 Oct 2025 16:55:54 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
CoinAddressesBloc is created at lower levels within buttons etc, so the auth state replay causes issues with the pubkey loading (initial state + current state = clear listeners while starting listeners)
|
@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: 2
🧹 Nitpick comments (2)
lib/bloc/coin_addresses/bloc/coin_addresses_bloc.dart (1)
194-201: Optional: guard fine/finest logs to avoid building message strings when disabledWrap with isLoggable to skip interpolation cost at lower levels.
- _log.fine('Starting pubkey watching for asset ${asset.id.id}'); + if (_log.isLoggable(Level.FINE)) { + _log.fine('Starting pubkey watching for asset ${asset.id.id}'); + } @@ - _log.finest( - 'Received pubkey update for asset ${asset.id.id}: ${assetPubkeys.keys.length} addresses', - ); + if (_log.isLoggable(Level.FINEST)) { + _log.finest( + 'Received pubkey update for asset ${asset.id.id}: ${assetPubkeys.keys.length} addresses', + ); + } @@ - _log.fine( - 'Pubkey watching started successfully for asset ${asset.id.id}', - ); + if (_log.isLoggable(Level.FINE)) { + _log.fine( + 'Pubkey watching started successfully for asset ${asset.id.id}', + ); + } @@ - _log.severe( - 'Failed to start pubkey watching for asset ${asset.id.id}: $e', - ); + _log.severe( + 'Failed to start pubkey watching for asset ${asset.id.id}: $e', + );Based on learnings
Also applies to: 206-209, 222-224, 226-228
lib/bloc/transaction_history/transaction_history_bloc.dart (1)
219-226: Fix comparator: comparing to DateTime.now() is incorrectSimplify to pure descending timestamp sort.
-int _sortTransactions(Transaction tx1, Transaction tx2) { - if (tx2.timestamp == DateTime.now()) { - return 1; - } else if (tx1.timestamp == DateTime.now()) { - return -1; - } - return tx2.timestamp.compareTo(tx1.timestamp); -} +int _sortTransactions(Transaction a, Transaction b) => + b.timestamp.compareTo(a.timestamp);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/bloc/coin_addresses/bloc/coin_addresses_bloc.dart(8 hunks)lib/bloc/coin_addresses/bloc/coin_addresses_event.dart(2 hunks)lib/bloc/coins_bloc/coins_bloc.dart(2 hunks)lib/bloc/coins_bloc/coins_repo.dart(5 hunks)lib/bloc/transaction_history/transaction_history_bloc.dart(6 hunks)lib/bloc/transaction_history/transaction_history_event.dart(1 hunks)
🔇 Additional comments (5)
lib/bloc/transaction_history/transaction_history_event.dart (1)
28-31: New auth-change event looks goodConst, sealed hierarchy preserved. OK.
lib/bloc/coin_addresses/bloc/coin_addresses_event.dart (1)
2-3: Auth-change event for CoinAddresses is well-formedImport + Equatable props are correct. LGTM.
Also applies to: 51-59
lib/bloc/coin_addresses/bloc/coin_addresses_bloc.dart (1)
51-76: General: pre-cache + cancel-before-start is solidPrecaching pubkeys, cancelling prior sub, and re-requesting after address creation is correct and improves UX. Nice.
lib/bloc/transaction_history/transaction_history_bloc.dart (2)
27-33: Auth-change handling LGTMAuth listener + clearing history/new subscriptions and resetting state is correct and matches the PR goal.
Also applies to: 43-49, 201-216
81-84: Address set is captured once; verify behavior when new addresses are created latermyAddresses is built before subscriptions and not refreshed. If users create new addresses, sanitize() may misclassify later txs. Consider re-fetching pubkeys periodically or subscribing to pubkey updates to refresh the set.
Also applies to: 146-178
| Future<void> flushCache() async { | ||
| // Intentionally avoid flushing the prices cache - prices are independent | ||
| // of the user's session and should be updated on a regular basis. | ||
| _addressCache.clear(); | ||
|
|
||
| enabledAssetsChanges.close(); | ||
| _log.info('Clearing ${_balanceWatchers.length} balance watchers'); | ||
| final cancelFutures = _balanceWatchers.values.map((sub) => sub.cancel()); | ||
| await Future.wait(cancelFutures); | ||
| _balanceWatchers.clear(); | ||
| } |
There was a problem hiding this comment.
Also clear _balancesCache when flushing on auth changes
flushCache() now runs on every auth transition, but it only clears _addressCache and cancels watchers. _balancesCache still holds the previous wallet’s balances, so subsequent _assetToCoinWithoutAddress calls (e.g., during the next login’s pre-population) will surface stale balances from the prior session. That’s a correctness issue: the newly signed-in wallet briefly displays someone else’s balances until fresh updates arrive. Please clear _balancesCache alongside the watcher teardown.
Apply this diff:
_addressCache.clear();
+ _balancesCache.clear();🤖 Prompt for AI Agents
In lib/bloc/coins_bloc/coins_repo.dart around lines 153 to 162, flushCache()
currently clears _addressCache and cancels balance watchers but fails to clear
_balancesCache, which allows stale balances from the previous session to be
shown after auth transitions; update flushCache() to also clear _balancesCache
(e.g., call _balancesCache.clear()) alongside _addressCache and the watcher
teardown so any cached balances are removed when auth changes.
|
Closing as unnecessary. This PR refactors existing logic, and does not fix a bug or improve performance. The root cause was found and fixed in the SDK (linked PR), and the duplicate activation issue was resolved for me on current dev |
Summary by CodeRabbit
New Features
Bug Fixes
Performance and Stability