fix: resolve transaction list not updating during sync#744
Conversation
This fixes an issue where users were not seeing recently sent or received transactions while the app was syncing with the chain. Root causes identified and fixed: - isResyncingWallet flag was blocking transaction reload during normal sync - Initial txByHash cache was empty when incremental updates arrived - Race condition between full reload and incremental transaction updates - Sync state changes triggered excessive reloads without debouncing - Balance changes only reloaded shortcuts, not the transaction list - Duplicate CrowdNode check in reloadTxDataSource - Transaction date changes (unconfirmed->confirmed) weren't moving txs between groups - Transactions absorbed by CoinJoin/CrowdNode groups weren't removed from individual list Changes: - Always reload transactions when sync starts (removed isResyncingWallet check) - Added hasCompletedInitialLoad flag to trigger full reload if cache is empty - Added isReloading flag to prevent race conditions during full reload - Added 0.5s debouncing to sync state change handling - Balance changes now trigger full transaction reload - Removed duplicate CrowdNode tryInclude() call - Handle transaction date group changes by moving between groups - Remove individual transactions when absorbed by CoinJoin/CrowdNode groups - Added bounds checking for array access safety - Added diagnostic logging for troubleshooting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds full-reload state tracking and a debounce for sync-state driven reloads; expands reload behavior to include full transaction list reloads on balance or sync events; improves transaction-status handling for date-group moves and group absorption; adds logging and resets state when clearing or switching networks. (48 words) Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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[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)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift (2)
287-294: Consider safe unwrapping foritems.first.While
Dictionary(grouping:by:)won't create empty groups, usingcompactMapwith a guard adds defensive safety and silences the SwiftLint warning.🔎 Proposed fix
- let array = groupedItems.map { key, items in - TransactionGroup(id: key, date: items.first!.date, items: items) - }.sorted { $0.date > $1.date } + let array = groupedItems.compactMap { key, items -> TransactionGroup? in + guard let firstItem = items.first else { return nil } + return TransactionGroup(id: key, date: firstItem.date, items: items) + }.sorted { $0.date > $1.date }
386-397: Redundant optional initialization.Swift optionals default to
nil; explicit initialization is unnecessary.🔎 Proposed fix
- var oldGroupIndex: Int? = nil - var oldItemIndex: Int? = nil - var oldDateKey: String? = nil + var oldGroupIndex: Int? + var oldItemIndex: Int? + var oldDateKey: String?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
🧰 Additional context used
📓 Path-based instructions (3)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Use SwiftUI for all new UI components. Do NOT create new UIKit ViewControllers, XIB files, or Storyboards. Use thin UIViewController wrappers only when integrating with existing UIKit navigation.
Always use guard statements instead of force unwrapping for optional coordinates (latitude, longitude). Never force unwraplatitude!orlongitude!- use guard with error handling.
For generic type inference in Swift closures, add explicit return type annotations (e.g.,{ merchant -> MerchantAnnotation? in }) when compilation fails with generic parameter errors.
In SwiftUI conditional compilation, use computed properties instead of inline#ifstatements to avoid buildExpression errors. Move complex conditions into dedicated computed properties.
For dictionary initialization with conditional compilation, use closure-based initialization instead of inline conditionals to avoid syntax errors.
When removing debug print statements, ensure switch cases remain valid (addbreakif case becomes empty). Never leave empty switch cases.
Conditional compilation in Swift: use#if DASHPAYfor DashPay features and#if PIGGYCARDS_ENABLEDfor PiggyCards features. Wrap all provider-specific code in appropriate flags.
Always use debug flags like#if DEBUGto wrap debug print statements. Remove debug logging before release commits. Search forprint("statements before any release.
Use emoji prefixes in debug logs for easy filtering: 🔍 for search, 📍 for location, 🗺️ for map, 🎯 for targeted debugging. This improves ability to filter console output.
Always validate CLLocationCoordinate2D.isValid before using coordinates. Implement fallback chains for CTX API data. Test coordinate edge cases thoroughly in unit tests.
Use os_log framework for production-safe logging instead of print statements. os_log messages are only logged in debug builds and stripped in release builds.
Files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
**/*.{swift,m,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and 180-character line limit (100 recommended). Follow NYTimes Objective-C Style Guide for ObjC code. Use SwiftFormat/SwiftLint for Swift code formatting.
Files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
**/*ViewModel.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*ViewModel.swift: Format discounts with 1 decimal place for values < 1% (e.g., -0.5%), and whole numbers for 1% and above (e.g., -10%). Use.doubleValue, not.intValue, to preserve decimal precision.
For claim link (online redemption) gift cards, store the URL in the card 'number' field and display as tappable link instead of barcode. Detect URLs by checking if number starts with 'http'.
After updating barcode in database, always callawait loadGiftCard()to reload UI. Don't rely on automatic updates - explicitly reload after database changes.
MVVM Pattern: Use ViewModels for SwiftUI components and modern controllers. ViewModels should contain business logic, Views should be lightweight display only.
Files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
🧠 Learnings (4)
📚 Learning: 2025-12-17T21:42:15.776Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T21:42:15.776Z
Learning: Applies to **/*ViewModel.swift : After updating barcode in database, always call `await loadGiftCard()` to reload UI. Don't rely on automatic updates - explicitly reload after database changes.
Applied to files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
📚 Learning: 2025-12-17T21:42:15.776Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T21:42:15.776Z
Learning: Applies to DashWallet/Sources/Models/Uphold/DWUpholdMainnetConstants.m : The DWUpholdMainnetConstants.m file gets automatically formatted by the clang-format build phase script, adding unwanted blank lines. Restore this file before committing if it only has whitespace changes.
Applied to files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
📚 Learning: 2025-12-17T21:42:15.776Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T21:42:15.776Z
Learning: Applies to **/*ViewModel.swift : MVVM Pattern: Use ViewModels for SwiftUI components and modern controllers. ViewModels should contain business logic, Views should be lightweight display only.
Applied to files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
📚 Learning: 2025-11-24T15:33:27.646Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 739
File: DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift:152-251
Timestamp: 2025-11-24T15:33:27.646Z
Learning: In ExploreDatabaseConnection.swift test merchant insertion (DEBUG/Testflight builds), FTS trigger operations (DROP TRIGGER and CREATE TRIGGER statements) must be wrapped in a single SQLite transaction using `db.transaction { }` to ensure atomicity. If any operation fails between dropping and recreating triggers, the database would be left without FTS triggers for the session.
Applied to files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
🪛 SwiftLint (0.57.0)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
[Warning] 293-293: Force unwrapping should be avoided
(force_unwrapping)
[Warning] 386-386: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 387-387: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 388-388: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (5)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift (5)
50-59: Well-documented state tracking flags.The new flags (
isReloading,hasCompletedInitialLoad) and debounce mechanism are clearly documented with comments explaining their purpose and which fix they address. This aids maintainability.
155-158: LGTM!Properly resetting load tracking flags on network change ensures the new network's data will load correctly without stale state.
192-218: Appropriate fixes for transaction visibility during sync.Fix #1 (always reload on sync start) and Fix #5 (balance changes trigger full reload) correctly address the root causes where transactions weren't appearing during normal sync operations.
556-573: Well-implemented debounce pattern.The debounce correctly cancels pending work items before scheduling new ones, uses weak self to prevent retain cycles, and the 0.5s interval is reasonable for throttling rapid sync state changes.
390-397: Verify thread safety fortxItemsaccess across queues.
txItemsis accessed onself.queue(lines 366-367, 390-391, 436-439) but modified on the main queue. While Swift arrays are value types and the captured values should be consistent within a single read, this pattern could lead to data races if the main queue modifiestxItemsduring iteration. Consider whether accessingtxItemsshould be consolidated to one queue for safer concurrent access.
Move index discovery and removal operations to main thread to prevent race conditions. Add comprehensive bounds validation before array removals to avoid index-out-of-bounds crashes when txItems changes between index capture and removal execution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift (2)
292-294: Consider optional binding to avoid force unwrap.While the force unwrap on
items.first!is safe here (Dictionary grouping guarantees non-empty arrays), using optional binding orcompactMapwould be more defensive and satisfy SwiftLint.🔎 Proposed fix
let array = groupedItems.map { key, items in - TransactionGroup(id: key, date: items.first!.date, items: items) + TransactionGroup(id: key, date: items.first?.date ?? Date(), items: items) }.sorted { $0.date > $1.date }Or use
compactMapwithguard:let array = groupedItems.compactMap { key, items -> TransactionGroup? in guard let firstDate = items.first?.date else { return nil } return TransactionGroup(id: key, date: firstDate, items: items) }.sorted { $0.date > $1.date }
400-402: Remove redundant nil initialization.In Swift, optional variables are implicitly initialized to
nil. These explicit initializations are unnecessary.🔎 Proposed fix
- var oldGroupIndex: Int? = nil - var oldItemIndex: Int? = nil - var oldDateKey: String? = nil + var oldGroupIndex: Int? + var oldItemIndex: Int? + var oldDateKey: String?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
🧰 Additional context used
📓 Path-based instructions (3)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Use SwiftUI for all new UI components. Do NOT create new UIKit ViewControllers, XIB files, or Storyboards. Use thin UIViewController wrappers only when integrating with existing UIKit navigation.
Always use guard statements instead of force unwrapping for optional coordinates (latitude, longitude). Never force unwraplatitude!orlongitude!- use guard with error handling.
For generic type inference in Swift closures, add explicit return type annotations (e.g.,{ merchant -> MerchantAnnotation? in }) when compilation fails with generic parameter errors.
In SwiftUI conditional compilation, use computed properties instead of inline#ifstatements to avoid buildExpression errors. Move complex conditions into dedicated computed properties.
For dictionary initialization with conditional compilation, use closure-based initialization instead of inline conditionals to avoid syntax errors.
When removing debug print statements, ensure switch cases remain valid (addbreakif case becomes empty). Never leave empty switch cases.
Conditional compilation in Swift: use#if DASHPAYfor DashPay features and#if PIGGYCARDS_ENABLEDfor PiggyCards features. Wrap all provider-specific code in appropriate flags.
Always use debug flags like#if DEBUGto wrap debug print statements. Remove debug logging before release commits. Search forprint("statements before any release.
Use emoji prefixes in debug logs for easy filtering: 🔍 for search, 📍 for location, 🗺️ for map, 🎯 for targeted debugging. This improves ability to filter console output.
Always validate CLLocationCoordinate2D.isValid before using coordinates. Implement fallback chains for CTX API data. Test coordinate edge cases thoroughly in unit tests.
Use os_log framework for production-safe logging instead of print statements. os_log messages are only logged in debug builds and stripped in release builds.
Files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
**/*.{swift,m,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and 180-character line limit (100 recommended). Follow NYTimes Objective-C Style Guide for ObjC code. Use SwiftFormat/SwiftLint for Swift code formatting.
Files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
**/*ViewModel.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*ViewModel.swift: Format discounts with 1 decimal place for values < 1% (e.g., -0.5%), and whole numbers for 1% and above (e.g., -10%). Use.doubleValue, not.intValue, to preserve decimal precision.
For claim link (online redemption) gift cards, store the URL in the card 'number' field and display as tappable link instead of barcode. Detect URLs by checking if number starts with 'http'.
After updating barcode in database, always callawait loadGiftCard()to reload UI. Don't rely on automatic updates - explicitly reload after database changes.
MVVM Pattern: Use ViewModels for SwiftUI components and modern controllers. ViewModels should contain business logic, Views should be lightweight display only.
Files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
🧠 Learnings (4)
📚 Learning: 2025-12-17T21:42:15.776Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T21:42:15.776Z
Learning: Applies to **/*ViewModel.swift : After updating barcode in database, always call `await loadGiftCard()` to reload UI. Don't rely on automatic updates - explicitly reload after database changes.
Applied to files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
📚 Learning: 2025-09-27T15:36:10.803Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift:21-23
Timestamp: 2025-09-27T15:36:10.803Z
Learning: In PiggyCardsRepository.swift, there's an ACL mismatch (public static let shared in an internal class) and potential thread safety concerns that will be addressed during dedicated PiggyCards development rather than in the current UI improvements PR.
Applied to files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
📚 Learning: 2025-11-24T15:33:27.646Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 739
File: DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift:152-251
Timestamp: 2025-11-24T15:33:27.646Z
Learning: In ExploreDatabaseConnection.swift test merchant insertion (DEBUG/Testflight builds), FTS trigger operations (DROP TRIGGER and CREATE TRIGGER statements) must be wrapped in a single SQLite transaction using `db.transaction { }` to ensure atomicity. If any operation fails between dropping and recreating triggers, the database would be left without FTS triggers for the session.
Applied to files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
📚 Learning: 2025-12-17T21:42:15.776Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T21:42:15.776Z
Learning: Applies to **/*ViewModel.swift : MVVM Pattern: Use ViewModels for SwiftUI components and modern controllers. ViewModels should contain business logic, Views should be lightweight display only.
Applied to files:
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
🧬 Code graph analysis (1)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift (2)
DashWallet/Sources/Models/CoinJoin/CoinJoinMixingTxSet.swift (1)
tryInclude(71-122)DashWallet/Sources/Models/CrowdNode/TxFilters/FullCrowdNodeSignUpTxSet.swift (1)
tryInclude(91-140)
🪛 SwiftLint (0.57.0)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
[Warning] 293-293: Force unwrapping should be avoided
(force_unwrapping)
[Warning] 400-400: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 401-401: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 402-402: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (7)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift (7)
50-59: LGTM! Well-documented state tracking properties.The new properties (
isReloading,hasCompletedInitialLoad, and debounce-related members) are clearly documented with their purpose tied to specific fix numbers. The queue-based access pattern appears consistent with how they're used throughout the file.
155-158: LGTM! Proper state reset on network change.Resetting
hasCompletedInitialLoadandisReloadingwhen switching networks ensures the new network's data loads correctly without stale state interference.
192-218: LGTM! Correctly addresses root causes for missing transactions.Fix #5 ensures balance changes trigger full transaction reloads (not just shortcuts), and Fix #1 removes the
isResyncingWalletconditional that was blocking reloads during normal sync. Both changes directly address the documented root causes.
234-305: LGTM! Reload state tracking properly implemented.The
isReloadingflag correctly brackets the full reload operation, preventing race conditions with incremental updates. The flag is set before processing begins and cleared after completion, along withhasCompletedInitialLoad.
362-391: Past review concern addressed - iteration moved to main thread with proper bounds checks.The implementation now correctly performs the index discovery and removal entirely on the main thread, avoiding the race condition where indices could become stale. The bounds checks at lines 370, 374-375, and 380 ensure safe array access.
423-455: Past review concern addressed - comprehensive bounds validation before removal.The implementation now validates both
oldGIdxandoldIIdxbounds (lines 427-433) before performing array removal, addressing the previously flagged incomplete bounds check. The check properly ensures both indices are within valid range.
579-596: LGTM! Debounce correctly implemented.The debounce pattern properly cancels pending work items before scheduling new ones, preventing excessive reloads during rapid sync state changes. The 0.5s interval is reasonable, and the use of
[weak self]prevents retain cycles.
2 participants
Summary
This PR fixes an issue where users were not seeing recently sent or received transactions while the app was syncing with the chain. The issue was traced to several problems introduced during the December 2024 refactoring of transaction loading in HomeViewModel.
Root Causes Identified and Fixed:
isResyncingWalletflag blocking sync reload - Normal sync operations weren't refreshing the transaction listtxByHashcache empty during sync - Incremental updates failed when cache wasn't populatedChanges:
isResyncingWalletconditional)hasCompletedInitialLoadflag to trigger full reload if cache is emptyisReloadingflag to prevent race conditions during full reloadtryInclude()callTest plan
HomeViewModel:log entries to verify fix is active🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Performance
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.