fix: clear cached transactions when switching networks#741
fix: clear cached transactions when switching networks#741bfoss765 merged 5 commits intofeature/piggycardsfrom
Conversation
When switching between testnet and mainnet, the HomeViewModel singleton was retaining cached transaction data from the previous network. This caused testnet transactions to appear on mainnet (and vice versa) until the app was restarted. Added an observer for DWCurrentNetworkDidChangeNotification that clears all cached transaction data (txItems, txByHash, crowdNodeTxSet, coinJoinTxSets) and reloads fresh data from the new network's wallet. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the WalkthroughAdds PiggyCards gift-card provider support across database, models, services, caching, token refresh (actor), ordering flow, barcode scanning, UI and metadata; includes a DB migration to add a nullable Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as POIDetailsView
participant VM as DashSpendPayViewModel
participant TokenSvc as PiggyCardsTokenService
participant Repo as PiggyCardsRepository
participant API as PiggyCardsAPI
participant Cache as PiggyCardsCache
participant Coins as SendCoinsService
participant DB as GiftCardsDAO
User->>UI: Tap "Buy Gift Card"
UI->>VM: purchaseGiftCardAndPay()
VM->>TokenSvc: refreshTokenIfNeeded()
alt token refresh required
TokenSvc->>API: performAutoLogin()
API-->>TokenSvc: accessToken
end
VM->>Repo: orderGiftCard(merchantId, amount)
Repo->>Cache: getGiftCards(forMerchant)
alt cache miss
Repo->>API: getGiftCards(...)
API-->>Repo: [PiggyCardsGiftcard]
Repo->>Cache: storeGiftCards(...)
end
Repo->>Repo: selectGiftCard(...)
Repo->>API: createOrder(PiggyCardsOrderRequest)
API-->>Repo: PiggyCardsOrderResponse (payTo, orderId)
Repo-->>VM: GiftCardInfo
VM->>Coins: sendCoins(address, amount)
Coins->>VM: authenticate() (main actor)
VM-->>Coins: signed tx
Coins->>DB: saveGiftCard(provider: "piggycards")
DB-->>Coins: saved
Coins-->>VM: tx confirmed
VM->>UI: show success and gift card details
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift (2)
182-191: SQL injection vulnerability: Use parameterized queries.The
merchantIdis interpolated directly into the SQL string. While it likely comes from a trusted source (the database), parameterized queries are safer and more consistent. Line 639 correctly uses parameterized queries withdb.prepare(providersQuery, merchant.merchantId).#if PIGGYCARDS_ENABLED let providersQuery = """ - SELECT provider, savingsPercentage, denominationsType, sourceId FROM gift_card_providers - WHERE merchantId = '\(merchant.merchantId)' + SELECT provider, savingsPercentage, denominationsType, sourceId FROM gift_card_providers + WHERE merchantId = ? """ #else let providersQuery = """ SELECT provider, savingsPercentage, denominationsType FROM gift_card_providers - WHERE merchantId = '\(merchant.merchantId)' AND provider = 'CTX' + WHERE merchantId = ? AND provider = 'CTX' """ #endif // Then use: try db.prepare(providersQuery, merchant.merchantId)
423-433: Same SQL injection issue in second query location.This is the same issue as lines 182-191 - string interpolation instead of parameterized query.
🧹 Nitpick comments (39)
DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)
66-75: Address SwiftLint warnings.Two linting issues flagged:
@MainActorattribute should be on its own lineusedBiometricsclosure parameter is unused- @MainActor func authenticate() async -> Bool { + @MainActor + func authenticate() async -> Bool { return await withCheckedContinuation { continuation in DSAuthenticationManager.sharedInstance().authenticate( withPrompt: nil, usingBiometricAuthentication: DWGlobalOptions.sharedInstance().biometricAuthEnabled, alertIfLockout: true - ) { authenticatedOrSuccess, usedBiometrics, cancelled in + ) { authenticatedOrSuccess, _, cancelled in continuation.resume(returning: authenticatedOrSuccess && !cancelled) } } }DashWallet/Sources/UI/Home/Views/HomeViewModel.swift (1)
102-118: LGTM with architectural note.The integration of
observeNetworkChange()into the initialization flow is well-placed and consistent with the existing observer pattern.However, per coding guidelines, ViewModels should use
@MainActorclass decorator. This class currently uses a background queue (self.queue) for heavy operations and manually dispatches to main thread, which may be why@MainActorwas omitted. Consider whether the threading model should be refactored to align with the guideline, or document why this ViewModel requires a different pattern.Based on coding guidelines:
**/*ViewModel.swift: Use @mainactor class decorator for ViewModels to ensure UI updates on main threadDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift (2)
195-213: PiggyCards token refresh before pay screen looks sound.The Task‑based flow with a PiggyCards‑only token refresh,
await MainActor.runfor navigation, and early return on failure is coherent and avoids pushing the pay screen when the session is invalid. Optional: if you anticipate multiple callers needing this pattern, consider centralizing the “refresh‑then‑show‑pay‑screen” logic in a shared helper to keep behavior consistent across entry points.
319-327: CTX and PiggyCards token refresh handling is correct; consider deduplicating dialog text.
tryRefreshCtxToken()andtryRefreshPiggyCardsToken()both surface a consistent “session expired” flow and correctly logout ontokenRefreshFailed/unauthorized, while treating other errors as transient and preserving the session.- UI work (
logout, dialogs) is dispatched to the main actor where needed, and non‑fatal errors are logged viaDSLogger.You might want to DRY up the repeated session‑expired dialog (heading + body + button text) into a small helper to keep capitalization/copy synchronized between CTX and PiggyCards paths.
Also applies to: 329-356
DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (1)
58-71: Provider column wiring is correct; align multi‑line parameters to satisfy SwiftLint.
- Insertion now persists both
noteandprovider, and the two update helpers reconstructGiftCardinstances while preservingprovider(andnotewhere appropriate). This keeps the in‑memory cache consistent with the DB and matches the newprovidercolumn added in migrations.- SwiftLint warns about
vertical_parameter_alignment_on_callfor the multi‑lineinsert/updatecalls; you can tweak the indentation to silence it without behavior change, e.g.:- let insert = GiftCard.table.insert(or: .replace, - GiftCard.txId <- dto.txId, - GiftCard.merchantName <- dto.merchantName, - GiftCard.merchantUrl <- dto.merchantUrl, - GiftCard.price <- dto.price.description, - GiftCard.number <- dto.number, - GiftCard.pin <- dto.pin, - GiftCard.barcodeValue <- dto.barcodeValue, - GiftCard.barcodeFormat <- dto.barcodeFormat, - GiftCard.note <- dto.note, - GiftCard.provider <- dto.provider) + let insert = GiftCard.table.insert( + or: .replace, + GiftCard.txId <- dto.txId, + GiftCard.merchantName <- dto.merchantName, + GiftCard.merchantUrl <- dto.merchantUrl, + GiftCard.price <- dto.price.description, + GiftCard.number <- dto.number, + GiftCard.pin <- dto.pin, + GiftCard.barcodeValue <- dto.barcodeValue, + GiftCard.barcodeFormat <- dto.barcodeFormat, + GiftCard.note <- dto.note, + GiftCard.provider <- dto.provider + )(and similarly for the two
updatecalls).Also applies to: 115-126, 147-157
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift (1)
291-291: Remove empty line after opening brace.Per SwiftLint's
vertical_whitespace_opening_bracesrule.private func purchaseGiftCard() { - Task {DashWallet/Sources/Infrastructure/Database/Migrations/AddProviderToGiftCardsTable.swift (1)
28-28: Add trailing newline.Per SwiftLint's
trailing_newlinerule, files should end with a single newline character.DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsTokenService.swift (2)
94-123: Avoid force unwrap onrefreshTaskat line 122.While the force unwrap is likely safe within this actor context (since
refreshTaskis set immediately before at line 103), this triggers SwiftLint and could be avoided with optional binding for defensive coding.Apply this diff:
- _ = try await refreshTask!.value + guard let task = refreshTask else { return } + _ = try await task.valueAlternatively, you could capture the task directly:
// Start new refresh task - refreshTask = Task { + let task = Task { defer { refreshTask = nil isRefreshing = false } isRefreshing = true let success = try await performRefresh() if success { DSLogger.log("PiggyCards: Token refresh completed successfully") } else { DSLogger.log("PiggyCards: Token refresh failed") throw DashSpendError.tokenRefreshFailed } return success } + refreshTask = task - _ = try await refreshTask!.value + _ = try await task.value
91-92: Note:isRefreshingflag appears unused.The
isRefreshingproperty is set in the actor but never read. If it's intended for future use or debugging, consider adding a comment; otherwise, it can be removed to reduce dead code.DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift (2)
156-163: UseisEmptycheck instead of comparing count to zero.Per SwiftLint, prefer checking existence rather than counting rows.
- let checkQuery = "SELECT COUNT(*) FROM merchant WHERE merchantId = '\(testMerchantId)'" - let count = try db.scalar(checkQuery) as? Int64 ?? 0 - - if count > 0 { + let checkQuery = "SELECT 1 FROM merchant WHERE merchantId = '\(testMerchantId)' LIMIT 1" + let exists = try db.scalar(checkQuery) != nil + + if exists {
252-256: Remove vertical whitespace before closing brace.SwiftLint flagged an empty line before the closing brace at line 253.
print("✅ PiggyCards test merchant added successfully") - } catch {DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (2)
79-82: Consider usingguard letinstead of explicit nil comparison.This is more idiomatic Swift.
default: - let token = PiggyCardsTokenService.shared.accessToken - if token == nil { + guard PiggyCardsTokenService.shared.accessToken != nil else { throw DashSpendError.unauthorized } }
45-47: Generic catch-and-rethrow is redundant.The
catch { throw error }block simply rethrows any unhandled error, which is the default behavior. It can be removed unless there's a future plan to add error transformation here.} catch HTTPClientError.statusCode(let r) where r.statusCode == 400 { if target.path.contains("/verify-otp") { throw DashSpendError.invalidCode } throw HTTPClientError.statusCode(r) - } catch { - throw error }DashWallet/Sources/Utils/PercentageFormatter.swift (2)
33-42: Simplify the decimal detection logic.The
hasDecimalvariable at lines 34-35 includes a check that's repeated in line 38, making the logic harder to follow. The condition(percent * 10).truncatingRemainder(dividingBy: 1) != 0doesn't add clarity since it would only differ from the simpler check for values with sub-decimal precision that Double can't represent accurately anyway.- // Check if the value has a meaningful decimal component - let hasDecimal = (percent * 10).truncatingRemainder(dividingBy: 1) != 0 || - percent.truncatingRemainder(dividingBy: 1) != 0 - - // Use 1 decimal place if there's a fractional part, otherwise use whole numbers - if hasDecimal && percent.truncatingRemainder(dividingBy: 1) != 0 { + // Use 1 decimal place if there's a fractional part, otherwise use whole numbers + if percent.truncatingRemainder(dividingBy: 1) != 0 { return String(format: "\(sign)%.1f\(percentSymbol)", percent) } else { return String(format: "\(sign)%.0f\(percentSymbol)", percent) }
21-22: Remove empty line after opening brace.Per SwiftLint, avoid vertical whitespace after opening braces.
enum PercentageFormatter { - /// Format a percentage value with appropriate decimal placesDashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (4)
34-36: Consider making the disabled gift cards list configurable.The hardcoded
disabledGiftCardsdictionary for brandId 174 may need updates over time. Consider loading this from a configuration file or remote config to avoid code changes when the disabled list changes.
62-78: Exchange rate cache lacks expiration mechanism.Exchange rates are volatile and cached indefinitely. While the relevant code snippet from
PiggyCardsRepository.swiftshows cache-first retrieval, there's no TTL to prevent stale rates from being used for extended periods.Consider adding timestamp tracking and a TTL check:
// Cache for exchange rates -private var exchangeRateCache: [String: PiggyCardsExchangeRateResult] = [:] +private var exchangeRateCache: [String: (rate: PiggyCardsExchangeRateResult, timestamp: Date)] = [:] private let exchangeRateQueue = DispatchQueue(label: "piggyCardsCache.exchangeRate", attributes: .concurrent) +private let exchangeRateTTL: TimeInterval = 300 // 5 minutes
84-126: Gift card selection logic is well-structured but has repeated string processing.The priority-based selection is clear, but
priceType.trimmingCharacters(in: .whitespaces).lowercased()is repeated 4 times. Consider pre-processing once per card for efficiency.func selectGiftCard(from cards: [PiggyCardsGiftcard], forAmount amount: Double) -> PiggyCardsGiftcard? { // Filter out disabled cards let enabledCards = cards.filter { card in guard let disabledNames = disabledGiftCards[card.brandId] else { return true } return !disabledNames.contains { card.name.contains($0) } }.filter { $0.quantity > 0 } // Only available cards + + // Pre-compute normalized price types for efficiency + let cardsWithNormalizedType = enabledCards.map { card in + (card: card, normalizedPriceType: card.priceType.trimmingCharacters(in: .whitespaces).lowercased()) + } // Priority 1: Instant delivery fixed cards with exact denomination - if let instantCard = enabledCards.first(where: { card in - card.priceType.trimmingCharacters(in: .whitespaces).lowercased() == PiggyCardsPriceType.fixed.rawValue && + if let instantCard = cardsWithNormalizedType.first(where: { item in + item.normalizedPriceType == PiggyCardsPriceType.fixed.rawValue && // ... rest of conditions
133-134: Add trailing newline.Static analysis indicates a missing trailing newline at end of file.
func clearAllCaches() { clearGiftCardCache() clearExchangeRateCache() } } +DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (1)
120-122: Consider using os_log for error logging.Per coding guidelines, production-safe logging should use
os_logframework instead of silently swallowing errors. Adding debug logging would help diagnose barcode scanning failures.+import os.log + +private let logger = Logger(subsystem: "org.dash.wallet", category: "BarcodeScanner") + // In downloadAndScan: } catch { + #if DEBUG + logger.debug("🎨 Failed to download barcode image: \(error.localizedDescription)") + #endif return nil }DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift (2)
199-202: Remove unusedrowCountvariable.The
rowCountvariable is declared and incremented but never used. This appears to be debug code that was left behind.-var rowCount = 0 for row in rows { - rowCount += 1 if let providerId = row[0] as? String,Also applies to: 442-445, 641-644
658-676: Extra fallback logic for sourceId is defensive but verbose.This third instance of sourceId handling includes additional fallback with
String(describing:)and null string checks. While more robust, the inconsistency with other locations suggests this should be unified in a helper function.DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsView.swift (4)
153-163: Fix trailing closure syntax as flagged by SwiftLint.Button with
actionclosure followed by trailing closure for content should use explicitlabel:parameter.- Button(action: { - if let url = URL(string: claimLink) { - UIApplication.shared.open(url) - } - }) { + Button(action: { + if let url = URL(string: claimLink) { + UIApplication.shared.open(url) + } + }, label: { Text(NSLocalizedString("View your gift card details", comment: "DashSpend")) .font(.subheadline) .fontWeight(.medium) .foregroundColor(.dashBlue) .frame(maxWidth: .infinity, alignment: .center) - } + })
202-211: Fix trailing closure syntax for copy button.Same SwiftLint warning applies here. Use explicit
label:parameter.- Button(action: { - UIPasteboard.general.string = cardNumber - // TODO: Show toast - }) { + Button(action: { + UIPasteboard.general.string = cardNumber + // TODO: Show toast + }, label: { Image("icon_copy_outline") .resizable() .scaledToFit() .tint(.primaryText) .frame(width: 14, height: 14) - } + })
232-241: Fix trailing closure syntax for PIN copy button.Same SwiftLint warning applies here.
- Button(action: { - UIPasteboard.general.string = cardPin - // TODO: Show toast - }) { + Button(action: { + UIPasteboard.general.string = cardPin + // TODO: Show toast + }, label: { Image("icon_copy_outline") .resizable() .scaledToFit() .tint(.primaryText) .frame(width: 14, height: 14) - } + })
399-403: DateFormatter created on every access is inefficient.This computed property creates a new
DateFormatterinstance every time it's accessed. Consider making it a static constant or lazy property since the format is fixed.- private var dateFormatter: DateFormatter { - let formatter = DateFormatter() - formatter.dateFormat = "MMMM dd, yyyy 'at' h:mm a" - return formatter - } + private static let dateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.dateFormat = "MMMM dd, yyyy 'at' h:mm a" + return formatter + }()Then update line 76 to use
Self.dateFormatter.DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (4)
34-36: Remove redundant optional initialization.SwiftLint correctly flags that initializing an optional with
nilis redundant since that's the default.var isClaimLink: Bool = false var hasBeenPollingForLongTime: Bool = false - var provider: String? = nil + var provider: String? }
185-190: Replace unused optional binding with nil check.SwiftLint flags this pattern. Using
!= nilis clearer and more idiomatic.private func fetchGiftCardInfo() async { guard let giftCard = await giftCardsDAO.get(byTxId: txId), - let _ = giftCard.note else { + giftCard.note != nil else { stopTicker() return }
213-219: Same unused binding pattern in CTX fetch.Apply the same fix here.
private func fetchCTXGiftCardInfo() async { guard let giftCard = await giftCardsDAO.get(byTxId: txId), - let _ = giftCard.note, + giftCard.note != nil, ctxSpendRepository.isUserSignedIn else { stopTicker() return }
274-364: Consider extracting helper methods to reduce cyclomatic complexity.SwiftLint flags complexity of 11 (limit is 10). The nested conditionals for
claimCodevsclaimLinkhandling could be extracted into separate helper methods.For example, extract the claim code processing:
private func processClaimCode(_ claimCode: String, pin: String?, barcodeLink: String?) async { await giftCardsDAO.updateCardDetails(txId: txId, number: claimCode, pin: pin) if let barcodeLink = barcodeLink, !barcodeLink.isEmpty, let result = await BarcodeScanner.downloadAndScan(from: barcodeLink) { let cleanValue = result.value.replacingOccurrences(of: " ", with: "") .replacingOccurrences(of: "-", with: "") await giftCardsDAO.updateBarcode(txId: txId, value: cleanValue, format: result.format.rawValue) } else { let cleanCode = claimCode.replacingOccurrences(of: " ", with: "") .replacingOccurrences(of: "-", with: "") await giftCardsDAO.updateBarcode(txId: txId, value: cleanCode, format: "CODE128") } }DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift (2)
129-133: Remove redundant string enum value.SwiftLint correctly flags that
case userId = "userId"is redundant since the raw value matches the case name.struct PiggyCardsSignupResponse: Codable { let userId: String? enum CodingKeys: String, CodingKey { - case userId = "userId" + case userId } }
325-329: Remove redundant string enum values.All three cases have raw values matching their case names.
enum PiggyCardsPriceType: String { - case fixed = "fixed" - case range = "range" - case option = "option" + case fixed + case range + case option }DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (2)
165-166: Remove empty else block.This empty else block serves no purpose and should be removed.
} - } else { }
241-272: PiggyCards payment flow uses hardcoded satoshi conversion.The conversion
giftCardInfo.amount * 100_000_000assumesamountis in DASH. This is correct but consider using a named constant for clarity and to prevent magic number issues.+ let satoshisPerDash: Double = 100_000_000 // Convert DASH amount to satoshis (1 DASH = 100,000,000 satoshis) - let dashAmountInSatoshis = UInt64(giftCardInfo.amount * 100_000_000) + let dashAmountInSatoshis = UInt64(giftCardInfo.amount * satoshisPerDash)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (3)
246-259: Remove empty debug logging blocks.These empty if blocks for JSON logging serve no purpose in the current state.
- // Log the JSON request for debugging - if let jsonData = try? JSONEncoder().encode(orderRequest), - let jsonString = String(data: jsonData, encoding: .utf8) { - } do { let orderResponse: PiggyCardsOrderResponse = try await PiggyCardsAPI.shared.request(.createOrder(orderRequest)) return try await processOrderResponse(orderResponse, selectedCard: selectedCard, giftCards: giftCards, fiatCurrency: fiatCurrency) } catch let error as HTTPClientError { - // Log the raw error response for debugging - if case .statusCode(let response) = error { - if let errorString = String(data: response.data, encoding: .utf8) { - } - } throw try parseError(from: error, context: "create order")
353-356: Another empty for loop to remove.Same debug artifact pattern.
guard let cards = response.data else { throw DashSpendError.merchantUnavailable } - // Log details of each card - for (index, card) in cards.enumerated() { - } // Cache the cards for later use in order creation PiggyCardsCache.shared.storeGiftCards(cards, forMerchant: merchantId)
268-271: Fix vertical parameter alignment as flagged by SwiftLint.- private func processOrderResponse(_ orderResponse: PiggyCardsOrderResponse, - selectedCard: PiggyCardsGiftcard, - giftCards: [PiggyCardsGiftcard], - fiatCurrency: String) async throws -> GiftCardInfo { + private func processOrderResponse( + _ orderResponse: PiggyCardsOrderResponse, + selectedCard: PiggyCardsGiftcard, + giftCards: [PiggyCardsGiftcard], + fiatCurrency: String + ) async throws -> GiftCardInfo {CLAUDE.md (2)
1571-1571: Fix markdown linting violation: bare URL should use link syntax.Line 1571 contains a bare URL which violates markdown linting rules (MD034).
- Based on real issues encountered and resolved: + Based on [real issues encountered and resolved](relevant_link):Or if not linking to external resource, simply ensure URLs are wrapped in markdown link syntax where appropriate.
1589-1598: Add language specifier to fenced code block.The code block starting at line 1589 is missing the language identifier. Add
bashto the opening fence for proper syntax highlighting:-``` +```bash # Find the most recent explore.db for your running simulator find ~/Library/Developer/CoreSimulator/Devices/*/data/Containers/Data/Application/*/Documents/explore.db -type f -exec stat -f '%Sm %N' -t '%Y-%m-%d %H:%M:%S' {} \; | sort -r | head -1 -``` +```
- MerchantDAO: Extract duplicated sourceId coercion logic into extractSourceId() helper method that handles String, Int64, Int types and nil/empty values - ExploreDatabaseConnection: Wrap FTS trigger drop/create operations in a transaction for atomicity and rollback on error - GiftCardDetailsViewModel: Lower longPollingThreshold from 40 to 27 so "code is being generated" message shows after ~40s instead of only on the final retry - HomeViewModel: Fix thread safety in clearCachedData() by dispatching cache clearing to self.queue where those properties are accessed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added explicit documentation that permission to commit/push applies only to the specific changes discussed, not to subsequent changes. Includes example scenario and common mistake to avoid. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (1)
129-157: EnsureisClaimLinkis reset when number stops being a URLIn
loadGiftCard,isClaimLinkis only ever set totrue:if let number = card.number, number.starts(with: "http") { self.uiState.isClaimLink = true }If a PiggyCards order were to move from a link (
claimLink) to a plain code (claimCode) on a later refresh, this flag would staytrueand the UI could render the wrong state. Consider explicitly resetting it in the non‑URL case:- if let number = card.number, number.starts(with: "http") { - self.uiState.isClaimLink = true - } + if let number = card.number, number.starts(with: "http") { + self.uiState.isClaimLink = true + } else { + self.uiState.isClaimLink = false + }This keeps the state derived purely from the current
card.number.
♻️ Duplicate comments (2)
DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift (1)
188-217: Fix inconsistentdenominationsTypecasing andsavingsPercentagevalues.Two issues based on past learnings remain unaddressed:
Casing mismatch: Line 190 uses
'Fixed'(uppercase) but line 213 uses'fixed'(lowercase). Downstream filtering expects lowercase values.savingsPercentage mismatch: The merchant table has
1000(basis points = 10%) but gift_card_providers has10(which would be 0.1%). Both tables must use consistent basis point values.Apply this diff:
1000, - 'Fixed', + 'fixed', 'online',And for the gift_card_providers insert:
'PiggyCards', '177', - 10, + 1000, 'fixed',Based on learnings, denominationsType values must use lowercase and savingsPercentage values must be consistent across both tables in basis points.
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (1)
240-245: Unify barcode format string ("CODE128"vs"CODE_128") and centralize itLine 243 uses
"CODE128":format: "CODE128"while the PiggyCards fallbacks at lines 318 and 328 use
"CODE_128":format: "CODE_128"This inconsistency can cause downstream issues if any code relies on the exact format string (e.g., when interpreting
barcodeFormatfrom the DB). It’s also been flagged in a previous review.To keep things consistent, consider:
- Standardizing on a single canonical string (e.g.
"CODE128"to match the CTX path), and- Extracting it to a shared constant or enum raw value.
For example:
private enum BarcodeFormatString { static let code128 = "CODE128" }and then:
- format: "CODE128" + format: BarcodeFormatString.code128- format: "CODE_128" + format: BarcodeFormatString.code128This avoids subtle mismatches across providers.
Also applies to: 313-329
🧹 Nitpick comments (8)
DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift (2)
156-163: Consider usingEXISTScheck instead ofCOUNT(*)for efficiency.Using
COUNT(*)retrieves the total count when you only need to know if at least one row exists. AnEXISTSorLIMIT 1pattern is more efficient.Apply this diff:
- let checkQuery = "SELECT COUNT(*) FROM merchant WHERE merchantId = '\(testMerchantId)'" - let count = try db.scalar(checkQuery) as? Int64 ?? 0 + let checkQuery = "SELECT 1 FROM merchant WHERE merchantId = '\(testMerchantId)' LIMIT 1" + let exists = try db.scalar(checkQuery) != nil - if count > 0 { + if exists { print("🎯 PiggyCards test merchant already exists, skipping") return }
254-259: Minor: Remove empty line before closing brace.SwiftLint flags vertical whitespace before closing braces.
Apply this diff:
} print("✅ PiggyCards test merchant added successfully") - } catch { print("🎯 Error adding PiggyCards test merchant: \(error)") }DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift (2)
207-216: Consider using parameterized queries consistently for merchantId.The
allLocationsmethod (lines 633-644) correctly uses parameterized queries with?placeholders, but these provider queries still use string interpolation. While merchantId originates from the database (low injection risk), using parameterized queries consistently is a defensive best practice.Example for the first occurrence (apply similarly to line 441):
#if PIGGYCARDS_ENABLED let providersQuery = """ SELECT provider, savingsPercentage, denominationsType, sourceId FROM gift_card_providers - WHERE merchantId = '\(merchant.merchantId)' + WHERE merchantId = ? """ +let statement = try db.prepare(providersQuery, merchant.merchantId) #else let providersQuery = """ SELECT provider, savingsPercentage, denominationsType FROM gift_card_providers - WHERE merchantId = '\(merchant.merchantId)' AND provider = 'CTX' + WHERE merchantId = ? AND provider = 'CTX' """ +let statement = try db.prepare(providersQuery, merchant.merchantId) #endifAlso applies to: 439-448
224-227: Remove unusedrowCountvariable.The
rowCountvariable is declared and incremented but never used. If it's not needed for debugging, remove it to avoid dead code.Apply this diff at each location:
-var rowCount = 0 - for row in rows { - rowCount += 1 if let providerId = row[0] as? String,Also applies to: 457-460, 646-649
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (4)
34-36: UI state additions make sense; minor optional-init style nit
isClaimLink,hasBeenPollingForLongTime, andprovidercleanly capture the new UI requirements. SwiftLint flagsvar provider: String? = nilas redundant; you can drop the= niland rely on the default optional initialization.
49-51: AlignretryCountsemantics with polling duration commentsRight now
retryCountis only incremented in thecatchblocks offetchCTXGiftCardInfoandfetchPiggyCardsGiftCardInfo, so:
hasBeenPollingForLongTimeonly flips totrueafter many errors, not after ~40 seconds of polling as the comments suggest.maxRetrieseffectively caps consecutive error retries, not overall polling duration (successful-but-non‑fulfilled responses can continue indefinitely).Given the comments tying
maxRetriesandlongPollingThresholddirectly to 1.5s polling intervals, it may be clearer to:
- Increment
retryCountonce per polling attempt infetchGiftCardInfo, and- Remove the extra increments from the
catchblocks, usingretryCountas a general “attempt count”.Example adjustment:
private func fetchGiftCardInfo() async { - guard let giftCard = await giftCardsDAO.get(byTxId: txId), + retryCount += 1 + + guard let giftCard = await giftCardsDAO.get(byTxId: txId), let _ = giftCard.note else { stopTicker() return } - // Check if we've been polling for more than 60 seconds + // Check if we've been polling for more than 60 seconds if retryCount >= longPollingThreshold { await MainActor.run { self.uiState.hasBeenPollingForLongTime = true } }} catch { - retryCount += 1 if retryCount >= maxRetries { await MainActor.run { self.uiState.loadingError = error } stopTicker() }(and same pattern in
fetchPiggyCardsGiftCardInfo).
stopTicker()already resetsretryCountandhasBeenPollingForLongTime, which is good and works with this model. Please double‑check the intended behavior and adjust ifretryCountis meant to be “attempts” rather than “error retries”.Also applies to: 192-211, 262-269, 355-363
185-191: Replacelet _ = giftCard.notewith an explicit non-nil checkSwiftLint flags the
guardbindings usinglet _ = giftCard.noteas unused optional bindings. For clarity and to satisfyunused_optional_binding, you can change both guards to:- guard let giftCard = await giftCardsDAO.get(byTxId: txId), - let _ = giftCard.note else { + guard let giftCard = await giftCardsDAO.get(byTxId: txId), + giftCard.note != nil else { stopTicker() return }and similarly in
fetchCTXGiftCardInfo.Also applies to: 213-219
274-364: PiggyCards polling logic looks correct; consider extracting helpers to reduce complexityThe PiggyCards flow correctly:
- Guards on
orderIdand sign‑in status.- Handles
"complete"/"completed"by updating card details and barcode (with barcodeLink scan + claimCode fallbacks).- Handles link-based redemption via
claimLink.- Maps
"failed" / "rejected" / "cancelled"to a user‑friendly error.- Stops polling or keeps polling appropriately.
SwiftLint is flagging cyclomatic complexity (11), and there’s some repeated cleanup/normalization logic (e.g., the “cleanCode” fallbacks and the error‑timeout block). You could modestly simplify by extracting small helpers, for example:
private func updateBarcodeFromClaimCode(_ claimCode: String) asyncprivate func handlePiggyCardsTerminalError() asyncand reuse those for the two fallback branches and the error path. That should drop the complexity without changing behavior and also clean up the vertical whitespace up around
if let firstCard = cards.first {that SwiftLint warns about.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CLAUDE.md(4 hunks)DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift(8 hunks)DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift(5 hunks)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift(4 hunks)DashWallet/Sources/UI/Home/Views/HomeViewModel.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- DashWallet/Sources/UI/Home/Views/HomeViewModel.swift
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{swift,swift.gyb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SwiftFormat/SwiftLint for Swift code formatting and linting (configurations in .swiftformat and .swiftlint.yml)
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
**/*.{h,m,mm,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and maintain 180-character line limit (100 recommended)
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Avoid @objcMembers wholesale exposure (enforced by custom SwiftLint rule)
Always use async/await for promises in Swift code, not completion handlers
Never force unwrap latitude/longitude coordinates; always use guard statements
Use guard statements and avoid force unwrapping optional properties like lastBounds
Add explicit return type annotations to compactMap closures when compiler inference fails
Use computed properties instead of inline #if conditional compilation statements in SwiftUI ViewBuilder
Use closure-based initialization for dictionaries with conditional compilation instead of inline conditionals
Wrap debug print statements in #if DEBUG blocks to prevent performance issues in release builds
Remove empty switch cases when cleaning up debug code; add break statement or remove case entirely
Use @StateObject private var viewModel in SwiftUI views for proper lifecycle management
Use NavigationStack for new SwiftUI navigation; use UIHostingController for legacy UIKit integration
Use Combine @published properties for reactive data flow between services and SwiftUI views
iOS Swift files should declare conformance using 'extension ClassName: ProtocolName' syntax, not inline
Use os_log framework for production-safe logging instead of print() statements
Implement fallback chains for optional API response fields with explicit unwrapping
Always validate CLLocationCoordinate2D.isValid before using coordinates
Use emoji debug markers in logs for easy filtering: 🎯 for goals, 🌐 for network, 💾 for database, 🎨 for UI
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
DashWallet/Sources/UI/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
All new UI components MUST be built with SwiftUI, not UIKit ViewControllers or Storyboards
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
**/*ViewModel.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*ViewModel.swift: Use @mainactor class decorator for ViewModels to ensure UI updates on main thread
Implement @published properties in ViewModels for reactive data binding with SwiftUI
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
DashWallet/Sources/Models/**/*DAO.swift
📄 CodeRabbit inference engine (CLAUDE.md)
DashWallet/Sources/Models/**/*DAO.swift: Apply both rectangular bounds filtering (SQL optimization) AND circular distance filtering (accuracy) for location-based queries
Expand rectangular bounds by 50% when used for circular radius searches to ensure consistency
Filter merchants by provider when fetching details to avoid duplicate/incorrect data from multi-provider database
Always query git_card_providers table (not merchant table) for denomination type and provider-specific data
Files:
DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to **/CTX*.swift : Use blockchain transaction ID (txid), not gift card UUID, when fetching gift cards from CTX API
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/Models/**/*DAO.swift : Always query git_card_providers table (not merchant table) for denomination type and provider-specific data
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 737
File: DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift:200-235
Timestamp: 2025-11-25T20:39:25.847Z
Learning: In DashWallet iOS gift card implementation, discount display rules vary by provider and magnitude: PiggyCards and CTX handle discounts differently, and discounts less than 1% display to 1 decimal place while others display as whole numbers. This intentional differentiation exists in PiggyCardsRepository.calculateDisplayDiscount and DashSpendPayViewModel.updateMerchantInfo.
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/Models/**/*DAO.swift : Filter merchants by provider when fetching details to avoid duplicate/incorrect data from multi-provider database
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 setup, savingsPercentage values in both merchant and gift_card_providers tables must be consistent and expressed in basis points (e.g., 1000 = 10%). The values are divided by 10000 in DashSpendPayViewModel, so both tables should use the same basis point value for the same merchant to avoid discount calculation mismatches.
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.
📚 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:
CLAUDE.mdDashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to **/CTX*.swift : Use blockchain transaction ID (txid), not gift card UUID, when fetching gift cards from CTX API
Applied to files:
CLAUDE.mdDashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: NEVER commit or push changes without explicit user permission in Git workflow
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-05T04:46:12.717Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 728
File: DashWallet.xcodeproj/project.pbxproj:1595-1599
Timestamp: 2025-09-05T04:46:12.717Z
Learning: In iOS projects, the `DashWallet.xcodeproj/project.pbxproj` file is automatically generated and managed by Xcode. Manual changes to this file should not be made, and the changes shown in diffs are typically the result of Xcode updating project configuration, dependencies, or build settings.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/Models/Uphold/DWUpholdMainnetConstants.m : Don't commit DWUpholdMainnetConstants.m if it only has whitespace changes from clang-format script; restore it instead
Applied to files:
CLAUDE.mdDashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-08-25T21:01:26.493Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 723
File: DashWallet/da.lproj/Localizable.strings:2987-2987
Timestamp: 2025-08-25T21:01:26.493Z
Learning: The DashWallet project uses Transifex for translation management rather than direct manual edits to .lproj/Localizable.strings files.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/UI/**/*.swift : All new UI components MUST be built with SwiftUI, not UIKit ViewControllers or Storyboards
Applied to files:
CLAUDE.mdDashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: MCP servers must be configured in ~/Library/Application Support/Claude/claude_desktop_config.json and require Claude Code restart
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Figma Desktop App must be running with Dev Mode enabled (Shift+D) for Figma MCP server to work
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-25T20:39:25.847Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 737
File: DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift:200-235
Timestamp: 2025-11-25T20:39:25.847Z
Learning: In DashWallet iOS gift card implementation, discount display rules vary by provider and magnitude: PiggyCards and CTX handle discounts differently, and discounts less than 1% display to 1 decimal place while others display as whole numbers. This intentional differentiation exists in PiggyCardsRepository.calculateDisplayDiscount and DashSpendPayViewModel.updateMerchantInfo.
Applied to files:
CLAUDE.mdDashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to **/CTX*.swift : CTX gift card fetch endpoint returns different response structures: direct object in production, paginated in staging
Applied to files:
CLAUDE.mdDashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/Models/**/*DAO.swift : Always query git_card_providers table (not merchant table) for denomination type and provider-specific data
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.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 setup, savingsPercentage values in both merchant and gift_card_providers tables must be consistent and expressed in basis points (e.g., 1000 = 10%). The values are divided by 10000 in DashSpendPayViewModel, so both tables should use the same basis point value for the same merchant to avoid discount calculation mismatches.
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/UI/**/HomeViewController*.swift : Shortcut bar displays different button combinations based on wallet state: four different states based on balance and passphrase verification
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
📚 Learning: 2025-09-27T23:33:40.458Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift:25-32
Timestamp: 2025-09-27T23:33:40.458Z
Learning: PiggyCards API getGiftCard endpoint uses an invalid path "gift-cards/\(txid)" that doesn't exist on the server (marked with TODO comment). The correct path should likely be "orders" with query parameters following the CTX API pattern, since PiggyCards uses "orders" for purchases while CTX uses "gift-cards" for both purchase and retrieval operations.
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/Models/**/*DAO.swift : Filter merchants by provider when fetching details to avoid duplicate/incorrect data from multi-provider database
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.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 setup, denominationsType values must use lowercase ('fixed', 'range', 'option') in both merchant and gift_card_providers INSERT statements to match downstream filtering logic. Using uppercase values like 'Fixed' will cause merchants to fail denomination type filtering.
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to **/CTX*.swift : Handle CTX field name differences: staging uses 'userDiscount', production uses 'savingsPercentage' with fallback chain
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/Models/**/*DAO.swift : Expand rectangular bounds by 50% when used for circular radius searches to ensure consistency
Applied to files:
DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWallet/Sources/Models/**/*DAO.swift : Apply both rectangular bounds filtering (SQL optimization) AND circular distance filtering (accuracy) for location-based queries
Applied to files:
DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.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/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
📚 Learning: 2025-11-26T19:32:01.721Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T19:32:01.721Z
Learning: Applies to DashWalletTests/**/*.swift : Test coordinate edge cases thoroughly in unit tests for location-based features
Applied to files:
DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
🧬 Code graph analysis (1)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (3)
DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (2)
from(39-66)downloadAndScan(100-123)DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (3)
get(81-94)updateCardDetails(105-135)updateBarcode(137-166)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (1)
getOrderStatus(302-314)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
1582-1582: Bare URL used
(MD034, no-bare-urls)
1600-1600: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 SwiftLint (0.57.0)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
[Warning] 36-36: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 187-187: Prefer != nil over let _ =
(unused_optional_binding)
[Warning] 215-215: Prefer != nil over let _ =
(unused_optional_binding)
[Warning] 274-274: Function should have complexity 10 or less; currently complexity is 11
(cyclomatic_complexity)
[Warning] 291-291: Don't include vertical whitespace (empty line) after opening braces
(vertical_whitespace_opening_braces)
DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift
[Warning] 231-231: Don't include vertical whitespace (empty line) after opening braces
(vertical_whitespace_opening_braces)
[Warning] 464-464: Don't include vertical whitespace (empty line) after opening braces
(vertical_whitespace_opening_braces)
[Warning] 658-658: Don't include vertical whitespace (empty line) after opening braces
(vertical_whitespace_opening_braces)
DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
[Error] 160-160: Prefer checking isEmpty over comparing count to zero
(empty_count)
[Warning] 256-256: Don't include vertical whitespace (empty line) before closing braces
(vertical_whitespace_closing_braces)
🔇 Additional comments (6)
CLAUDE.md (1)
1-2650:⚠️ Critical Inconsistency: PR Objectives Don't Match File ContentThe PR objectives describe a transaction caching fix (HomeViewModel.shared, DWCurrentNetworkDidChangeNotification, clearCachedData), but this file is extensive documentation covering PiggyCards integration, gift card barcode handling, and other features.
This represents either:
- The wrong file being reviewed
- Mismatched PR objectives
- Multiple unrelated changes bundled together
Before proceeding with review, please clarify:
- Is CLAUDE.md the correct file for the "clear cached transactions when switching networks" PR?
- If yes, should extensive PiggyCards/gift card documentation be in the same commit as the transaction caching fix?
DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift (1)
168-253: Transaction wrapping for FTS triggers is correctly implemented.The FTS trigger operations (DROP and CREATE) are now properly wrapped in
db.transaction { }ensuring atomicity. If any operation fails, the entire transaction rolls back, preventing the database from being left without FTS triggers.DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift (2)
38-61: Well-implemented helper function for sourceId extraction.The
extractSourceIdhelper correctly handles multiple type scenarios (String, Int64, Int) and edge cases (nil, empty, ""). This addresses the DRY principle violation from the past review.
632-664: Good use of parameterized query in allLocations.The
allLocationsmethod correctly uses parameterized queries (WHERE merchantId = ?) withdb.prepare(providersQuery, merchant.merchantId), preventing SQL injection. This pattern should be applied to the other provider queries for consistency.DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (2)
41-45: PiggyCards repository wiring is consistent with existing CTX patternUsing
PiggyCardsRepository.sharedalongsideCTXSpendRepository.sharedkeeps the dependency style consistent with the rest of this view model. No issues here.
177-183: Good: stopping ticker now also clears long-polling UI stateResetting
hasBeenPollingForLongTimeinstopTicker()avoids leaking the “code is being generated” state into subsequent polls, which matches user expectations.
Replace hardcoded string "DWCurrentNetworkDidChangeNotification" with the bridged Objective-C constant NSNotification.Name.DWCurrentNetworkDidChange to ensure the observer actually receives the notification. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
HashEngineering
left a comment
HashEngineering
left a comment
There was a problem hiding this comment.
LGTM, though the base branch is set to master instead of the piggycards branch
2 participants
Summary
HomeViewModelthat clears all cached transaction data when switching networksRoot Cause
The
HomeViewModel.sharedsingleton cached transaction data (txItems,txByHash,crowdNodeTxSet,coinJoinTxSets) but never cleared it when the network changed. AlthoughDWCurrentNetworkDidChangeNotificationwas being posted during network switches,HomeViewModeldidn't observe it.Changes
observeNetworkChange()method to subscribe toDWCurrentNetworkDidChangeNotificationclearCachedData()method that:txItems(transaction groups displayed in UI)txByHash(transaction lookup map)crowdNodeTxSetcoinJoinTxSetsreloadTxsAndShortcuts()to fetch fresh data from the new network's walletTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.