You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Code review — backport of 3 hotfixes (already merged)
Gather context (diff vs base, read changed files and callers)
Review for correctness bugs, security issues, regressions
Post review summary
Note: I can't post formal inline review comments (GitHub App limitation), so all findings are collected here with file/line references.
Summary
I reviewed all three backported commits and found no bugs, security issues, or regressions. Both fixes are correct and I verified the surrounding call sites. Two minor, non-blocking observations below.
✅ lib/core/wallet/data/datasources/lwk_facade.dart — File → Directory fix is correct
The LWK cache path (documents/<hexId>) is a directory, so the old File(dbPath).exists() always returned false and delete() unconditionally threw WalletError.notFound — meaning the UpdateOnDifferentStatus self-heal in lwk_wallet_datasource.dart:63-66 and :94-96 never actually wiped the corrupt cache. The fix restores that heal path. I also verified the recursive delete is safely scoped: BDK uses a distinct <hexId>_bdk_dart path (bdk_facade.dart:196) and nothing else in the app writes under documents/<hexId>.
✅ lib/core/wallet/domain/entities/wallet_transaction.dart — destinationOutput null-safety fix is correct
Previously outputs.firstWhere(...) without orElse threw a StateError when ownership flags disagreed with the direction heuristic, which crashed CSV export. Now matches.firstOrNull degrades to null, and all consumers already handle it: csv_transaction_export_formatter.dart:249 (tx.toAddress ?? '') and transaction_details_table.dart:128 (if (toAddress != null)).
✅ ios/Runner/Info.plist — purpose string addition is fine
NSLocationAlwaysAndWhenInUseUsageDescription mirrors the existing when-in-use string; no duplicate keys, and declaring the key alone doesn't grant or request always-on location.
Minor observations (non-blocking)
Unused dev dependency — path_provider_platform_interface was added to pubspec.yaml:159 as a direct dev dependency, but nothing in lib/ or test/ imports it (on this branch or on main). It looks like a leftover from a test that never landed with the lwk_facade fix. Consider removing it or adding the test that needed it. Fix this →
Heal path can mask the original error — in lwk_wallet_datasource.dart:65 and :95, await delete(wallet: wallet) runs inside the catch before throw e.msg. If the cache directory is already gone (e.g., two concurrent syncs both hit UpdateOnDifferentStatus and the first one wins), delete throws WalletError.notFound, which propagates instead of the original UpdateOnDifferentStatus message. Wrapping the heal-delete in its own try/catch would keep the original error surfaced. This behavior predates this PR and is only now reachable because delete works, so it's fine as a follow-up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.