Conversation
…nd paymentId Changed GiftCardResponse to be more resilient to API changes: - Required fields: id, status, paymentId (critical for transaction flow) - Optional fields: All other fields including paymentCryptoNetwork, percentDiscount, rate This prevents JSON decoding failures when CTX API doesn't return all expected fields, which was causing issues in the purchase gift card flow. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Version Update: - Updated MARKETING_VERSION from 8.4.2 to 8.4.3 across all build configurations CTX JSON Parsing Fix: - Made GiftCardResponse more resilient to API changes - Required fields: id, status, paymentId (critical for transaction flow) - Optional fields: All other fields including paymentCryptoNetwork, percentDiscount, rate - Prevents JSON decoding failures when CTX API doesn't return all expected fields Documentation Updates: - Added git commit/push policy to CLAUDE.md - Added detailed git workflow guidelines to AI-DEVELOPMENT-GUIDE.md - Ensures AI assistants wait for explicit permission before committing/pushing 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…hants - Fetch real-time merchant availability from CTX API getMerchant endpoint - Display "Temporarily unavailable" and disable buy button when merchant.enabled is false - Make getMerchant endpoint public (no authentication required) - Fix login error handling by adding complete error cases to CTXSpendAPI.request() - Clean up error handling in CTXSpendRepository and CTXSpendTokenService - Add "Temporarily unavailable" localization string Fixes issue where users couldn't see which merchants were disabled before attempting purchase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdded a critical Git commit/push policy in docs; updated CLAUDE guidance; changed Xcode project build-phase scripts to skip a specific file during clang-format; made CTX network baseURI dynamic; updated CTX models, API/endpoint/repository/token handling and error mapping; introduced merchantEnabled state, UI provider helpers, conditional provider filtering, logging updates, and a new localization string. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UI as POIDetailsView
participant VM as POIDetailsViewModel
participant Repo as CTXSpendRepository
participant API as CTXSpendAPI
participant EP as CTXSpendEndpoint
participant Pay as SendCoinsService
U->>UI: select provider
UI->>VM: providerSelected(id)
VM->>Repo: fetchMerchantStatus(merchantId)
Repo->>API: request(.getMerchant)
API->>EP: build request (baseURL = CTXConstants.baseURI)
EP-->>API: request sent (auth: bearer, X-Client-Id)
API-->>Repo: merchantStatus / error
Repo-->>VM: merchantEnabled (true/false)
VM->>UI: update provider rows & button state
U->>UI: tap purchase
UI->>VM: purchaseGiftCardAndPay(...)
VM->>Repo: purchaseGiftCard(...)
Repo->>API: request(.purchaseGiftCard)
API->>EP: send purchase (baseURL dynamic)
alt success
API-->>Repo: GiftCardResponse (includes paymentId, paymentUrls)
Repo-->>VM: GiftCardResponse
VM->>Pay: payWithDashUrl(paymentUrl)
Pay-->>VM: txHash
VM->>UI: persist metadata & show success
else http/error
API-->>Repo: mapped DashSpendError
Repo-->>VM: throw mapped error
VM->>UI: present error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (1)
156-186: Status is not refreshed when switching providers; ordering is nondeterministic
- selectProvider(_) does not refetch status; the initial Task runs only once here. After switching providers, merchantEnabled may be stale.
- Using supportedProviders.keys.first is unordered; the default provider may vary run-to-run.
Apply both fixes:
- Trigger a status fetch inside selectProvider(_:) after observeDashSpendState.
- Pick a deterministic default (e.g., prefer .ctx, else sort by displayName).
func selectProvider(_ provider: GiftCardProvider) { selectedProvider = provider observeDashSpendState(provider: provider) + // Refresh provider-specific status on selection change + Task { await fetchMerchantStatus() } } @@ -// Select the first available provider -selectedProvider = supportedProviders.keys.first +// Prefer CTX if present, else deterministic first by display name +if supportedProviders.keys.contains(.ctx) { + selectedProvider = .ctx +} else { + selectedProvider = supportedProviders.keys.sorted { $0.displayName < $1.displayName }.first +} // Start observing the selected provider if let selectedProvider = selectedProvider { observeDashSpendState(provider: selectedProvider) } // Fetch merchant enabled status from API Task { await fetchMerchantStatus() }DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift (1)
457-465: Fix type error in suggested refactor; selectedProvider is OptionalThe review correctly identifies that
merchantEnabledunconditionally gates all providers when only CTX should require this check. However, the suggested diff has a critical type flaw:viewModel.selectedProviderisGiftCardProvider?(Optional), not non-optional. The comparisonprovider == .ctxwould fail type checking or produce unexpected behavior.The refactor should be:
- return viewModel.merchantEnabled && - viewModel.networkStatus == .online && - viewModel.syncState == .syncDone + let providerAllowsPurchase = (viewModel.selectedProvider == .ctx) ? viewModel.merchantEnabled : true + return providerAllowsPurchase && + viewModel.networkStatus == .online && + viewModel.syncState == .syncDone
🧹 Nitpick comments (8)
DashWallet.xcodeproj/project.pbxproj (1)
12086-12086: Consider centralizing MARKETING_VERSION to reduce duplication.Define MARKETING_VERSION once in a shared .xcconfig (or at project level) and set it as the Base Configuration for all targets/configs; remove target-level overrides to prevent drift in future bumps.
Based on learnings.
CLAUDE.md (1)
5-22: LGTM! Clear workflow policy for AI agents.The Git Workflow Policy provides explicit guidance for AI agents on when to commit/push changes. The examples and exception cases are well-structured.
Minor: Address markdown linting.
Static analysis flagged that the code blocks in the example workflows (lines 16 and 28) should specify a language. Consider adding
textormarkdownidentifiers to satisfy the linter.Based on static analysis hints.
.claude/agents/AI-DEVELOPMENT-GUIDE.md (1)
5-40: LGTM! Comprehensive commit policy with clear examples.The Git Commit and Push Policy aligns well with the parallel section in CLAUDE.md. The four-rule process and workflow examples make the expectations explicit.
Minor: Specify language for code blocks.
Static analysis indicates the code blocks at lines 16 and 28 should declare a language (e.g.,
text). This improves markdown consistency.Based on static analysis hints.
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendTokenService.swift (1)
47-54: LGTM! Improved guard readability.Splitting the compound guard into two separate checks makes the validation logic clearer. Each guard now has a focused purpose, improving maintainability.
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (2)
55-55: Defaulting to “enabled” on errors can allow purchases when a merchant is disabledmerchantEnabled starts as true and is also set to true on fetch failure. This can surface “Buy” even if the merchant is actually disabled or the network is down. Prefer a safer default (false) or a tri-state (nil/loading/Bool) and gate UI accordingly; at minimum, only apply this gate for CTX-backed providers. As per coding guidelines (reliability), avoid partial error masking.
Would you like me to wire a tri-state with a small loading indicator on first fetch?
Also applies to: 196-203
188-204: Make status fetch provider-aware and race-safe
- Downcast to CTXSpendRepository is fine, but only CTX needs this status; for other providers, don’t block.
- Guard against races: if selectedProvider changes mid-await, avoid writing stale state.
- Treat CancellationError separately.
private func fetchMerchantStatus() async { - guard case .merchant(let m) = merchant.category, - m.paymentMethod == .giftCard, - let provider = selectedProvider, - let repository = repositories[provider] as? CTXSpendRepository else { - return - } + guard case .merchant(let m) = merchant.category, + m.paymentMethod == .giftCard, + let provider = selectedProvider else { return } + + // Only CTX supports merchant availability checks; don’t gate others. + guard provider == .ctx, + let repository = repositories[provider] as? CTXSpendRepository else { + merchantEnabled = true + return + } + + let providerAtStart = provider do { let merchantResponse = try await repository.getMerchant(merchantId: m.merchantId) - merchantEnabled = merchantResponse.enabled - } catch { + // Avoid stale write if selection changed during await + guard providerAtStart == selectedProvider else { return } + merchantEnabled = merchantResponse.enabled + } catch is CancellationError { + return + } catch { // On error, default to enabled to avoid blocking legitimate purchases merchantEnabled = true DSLogger.log("Failed to fetch merchant status: \(error)") } }If GiftCardProvider isn’t Equatable, confirm we can compare it (it’s used with == in the view already).
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift (1)
286-290: Sort providers for a stable, predictable orderArray(viewModel.supportedProviders.keys) is unordered; the order may flicker. Sort by displayName (or a defined priority like .ctx first).
-ForEach(Array(viewModel.supportedProviders.keys), id: \.self) { provider in +ForEach(viewModel.supportedProviders.keys.sorted { $0.displayName < $1.displayName }, id: \.self) { provider in providerRow(for: provider) }DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
150-199: Unreachable HTTP error mapping; move 400-detail parsing into CTXSpendAPICTXSpendAPI.request already converts HTTPClientError status codes into DashSpendError. This repository catch for HTTPClientError likely never runs, so specialized parsing for “above/below threshold”, “insufficient funds”, etc., won’t trigger.
Centralize the 400 parsing in CTXSpendAPI (where the status code is still visible) and simplify this method’s catch blocks.
- } catch let error as HTTPClientError { - DSLogger.log("Gift card purchase failed with HTTPClientError: \(error)") - ... - throw DashSpendError.unknown - } catch { - DSLogger.log("Gift card purchase failed with error: \(error)") - throw DashSpendError.networkError - } + } catch let error as DashSpendError { + DSLogger.log("Gift card purchase failed with CTXSpendError: \(error)") + throw error + } catch { + DSLogger.log("Gift card purchase failed with error: \(error)") + throw DashSpendError.networkError + }I can move the detailed 400 mapping (limit exceeded, merchant unavailable, etc.) into CTXSpendAPI with targeted path checks for
.purchaseGiftCardif you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.claude/agents/AI-DEVELOPMENT-GUIDE.md(1 hunks)CLAUDE.md(1 hunks)DashWallet.xcodeproj/project.pbxproj(12 hunks)DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swift(1 hunks)DashWallet/Sources/Models/Explore Dash/Model/Entites/CTXSpendModels.swift(1 hunks)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swift(1 hunks)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift(1 hunks)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift(4 hunks)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendTokenService.swift(1 hunks)DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift(4 hunks)DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift(2 hunks)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift(2 hunks)DashWallet/en.lproj/Localizable.strings(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{swift,h,m}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and keep lines under 180 characters (100 recommended)
Files:
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swiftDashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendTokenService.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swiftDashWallet/Sources/Models/Explore Dash/Model/Entites/CTXSpendModels.swift
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Avoid wholesale use of @objcMembers; expose only needed members (enforced by custom SwiftLint rule)
Avoid conditional compilation inside SwiftUI ViewBuilder expressions; use computed properties to encapsulate conditions
Prefer closure-based initialization for dictionaries when using conditional compilation to avoid syntax errors
Do not place #if conditionals mid-boolean expressions; assign to a conditional variable instead
Never force-unwrap latitude or longitude; use guard-let with error handling for coordinate values
Avoid force-unwraps of runtime properties (e.g., lastBounds!); prefer guard or optional binding with error handling
Use compactMap with explicit return types when creating map annotations to avoid type inference issues
Add explicit closure return type annotations when generic type inference fails
For location-based queries, first filter by rectangular bounds (SQL optimization), then apply precise circular distance filtering in memory
When using rectangular SQL bounds for circular searches, expand bounds by 50% to avoid excluding valid results before applying circular filtering
Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
After removing debug prints, ensure switch cases are not left empty; add break or remove the case
Prefer os.Logger (os_log) for production-safe logging over print
Files:
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swiftDashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendTokenService.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swiftDashWallet/Sources/Models/Explore Dash/Model/Entites/CTXSpendModels.swift
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift: Enforce View-ViewModel separation in SwiftUI: Views remain lightweight and delegate business logic to ViewModels
Use SwiftUI NavigationStack and navigationDestination for new screens
Files:
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift
**/*.{swift,m}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{swift,m}: Set template images to render with .withRenderingMode(.alwaysTemplate) where appropriate (e.g., SVG icons)
Use the kDefaultRadius constant (32000m) instead of hardcoded radius values
Files:
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swiftDashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendTokenService.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swiftDashWallet/Sources/Models/Explore Dash/Model/Entites/CTXSpendModels.swift
🧠 Learnings (2)
📚 Learning: 2025-09-05T04:46:12.717Z
Learnt from: HashEngineering
PR: dashpay/dashwallet-ios#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.mdDashWallet.xcodeproj/project.pbxproj
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
PR: dashpay/dashwallet-ios#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/Info.plist : Use $(MARKETING_VERSION) in Info.plist instead of hardcoded version strings; never edit versions directly in Info.plist
Applied to files:
DashWallet.xcodeproj/project.pbxproj
🧬 Code graph analysis (6)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
getMerchant(206-234)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (4)
DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)
payWithDashUrl(73-89)DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (1)
updateIcon(126-179)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
purchaseGiftCard(135-204)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift (1)
purchaseGiftCard(277-305)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift (1)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (1)
selectProvider(93-96)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (4)
login(67-69)verifyEmail(87-104)refreshToken(129-131)getMerchant(206-234)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendTokenService.swift (2)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (3)
refreshToken(129-131)updateTokens(117-121)clearTokensOnRefreshFailure(123-127)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swift (4)
request(24-45)request(47-68)requestDirectly(71-73)requestDirectly(75-77)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swift (2)
request(24-45)request(47-68)
🪛 markdownlint-cli2 (0.18.1)
.claude/agents/AI-DEVELOPMENT-GUIDE.md
16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (16)
DashWallet.xcodeproj/project.pbxproj (10)
10594-10594: LGTM — consistent MARKETING_VERSION for this configuration.
10768-10768: LGTM — WatchApp target updated to 8.4.3.
10790-10790: LGTM — WatchApp target (alt config) aligned at 8.4.3.
10815-10815: LGTM — WatchKit extension set to 8.4.3.
10842-10842: LGTM — WatchKit extension (alt config) aligned at 8.4.3.
11582-11582: LGTM — app target config shows 8.4.3.
11751-11751: LGTM — WatchApp config reflects 8.4.3.
11776-11776: LGTM — WatchKit extension config reflects 8.4.3.
11893-11893: LGTM — app configuration aligned at 8.4.3.
12061-12061: LGTM — WatchApp configuration aligned at 8.4.3.DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swift (1)
21-33: LGTM! Environment-aware base URL selection.The computed property correctly selects the CTX API base URL based on the current network (mainnet vs testnet). The implementation is clean, with appropriate documentation comments.
DashWallet/en.lproj/Localizable.strings (1)
961-962: LGTM! Localization string added.The "Temporarily unavailable" string supports the merchant status UI updates. Properly formatted and ready for use.
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift (1)
57-64: LGTM! Appropriate authorization exemption for public merchant data.Allowing
getMerchantto be called without bearer authentication makes sense—merchant information is typically public data. This aligns with REST API best practices.DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swift (1)
31-44: LGTM! Comprehensive HTTP error handling.The error mapping provides clear, actionable error types for various failure scenarios:
- 400 responses are distinguished by path (verify vs. other)
- 409/422 map to transaction/amount errors
- 500+ and decoder failures are properly handled
Note: Code duplication is acceptable here.
The error handling is duplicated between the generic and non-generic
request()methods (lines 54-67), but this is necessary given their different return signatures.DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
143-146: LGTM: return concrete GiftCardResponseDecoding and returning GiftCardResponse directly is cleaner and aligns with the updated models.
DashWallet/Sources/Models/Explore Dash/Model/Entites/CTXSpendModels.swift (1)
54-107: Based on the script output analysis and web search, I can now generate the verification findings:Code review findings confirmed safe; no issues identified
The verification found:
paymentId (required): Used safely at two call sites without force-unwraps:
- Line 187: Debug print statement
- Line 211: Direct pass to
saveGiftCardDummy(giftCardId: response.paymentId)No decoding failures expected since the struct requires this field.
paymentCryptoNetwork, percentDiscount, rate (now optional): All correctly defined as optional in the struct (lines 67–69). Script search found no force-unwraps or risky assumptions on these fields.
Call sites and usage: Only two usages of GiftCardResponse found in DashSpendPayViewModel.swift, both safe. No dangerous nil-coalescing or force-unwraps detected.
The struct change is well-formed. No decoding or usage issues were found. The fields are optionalized correctly and call sites properly handle the values.
- Update all remaining MARKETING_VERSION entries to 8.4.3 (8 targets) - Make providerStatusText provider-aware to fix "Temporarily unavailable" labels - Remove 30+ debug print statements from gift card purchase flow - Update CLAUDE.md with version management best practices Changes ensure consistent versioning across all targets and improve provider-specific status handling in POIDetailsView. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
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/Merchants & ATMs/Details/Views/POIDetailsView.swift (1)
460-468: Button enablement should be provider-aware.The
merchantEnabledcheck applies to all gift card merchants, butmerchantEnabledis CTX-specific (as correctly used inproviderStatusTextat line 356). For merchants supporting multiple providers, if a non-CTX provider is selected, the button should remain enabled even whenmerchantEnabledis false.Apply this diff to make the button enablement provider-aware:
private var isButtonEnabled: Bool { guard case .merchant(let m) = merchant.category, m.paymentMethod == .giftCard else { return true } - return viewModel.merchantEnabled && - viewModel.networkStatus == .online && + // Only check merchantEnabled for CTX provider + let isProviderAvailable = viewModel.selectedProvider != .ctx || viewModel.merchantEnabled + + return isProviderAvailable && + viewModel.networkStatus == .online && viewModel.syncState == .syncDone }
🧹 Nitpick comments (5)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (5)
178-199: Remove no-op do-catch; it only rethrows.Tighten flow; let errors propagate naturally.
- do { - let response = try await purchaseGiftCardAPI() + let response = try await purchaseGiftCardAPI() @@ - return transaction.txHashData - } catch { - throw error - } + return transaction.txHashData
182-188: Localize error messages and guard against empty payment URL.Prevents user-facing English strings and empty URL edge case.
- guard let paymentUrls = response.paymentUrls else { - throw DashSpendError.paymentProcessingError("No payment URLs received") - } + guard let paymentUrls = response.paymentUrls else { + throw DashSpendError.paymentProcessingError( + NSLocalizedString("No payment URLs received", comment: "DashSpend")) + } @@ - guard let paymentUrlString = paymentUrls.first?.value else { - throw DashSpendError.paymentProcessingError("No payment URL received") - } + guard let paymentUrlString = paymentUrls.first?.value, !paymentUrlString.isEmpty else { + throw DashSpendError.paymentProcessingError( + NSLocalizedString("No payment URL received", comment: "DashSpend")) + }
303-303: Log what you send.Use the exact string sent to the API for consistency.
- DSLogger.log("Attempting to purchase gift card for merchant \(merchantId) with amount \(amount)") + DSLogger.log("Attempting to purchase gift card for merchant \(merchantId) with amount \(fiatAmountString)")
306-312: Respect injected repository/provider; avoid hard-wiring CTX.Use repository[provider] to enable other providers and ease testing.
Example:
guard let repo = repository[provider] else { throw DashSpendError.unknown } let response = try await repo.purchaseGiftCard( merchantId: merchantId, fiatAmount: fiatAmountString, fiatCurrency: defaultCurrency, cryptoCurrency: "DASH" )Also replace the hard-coded "USD" with defaultCurrency (see next comment). Based on learnings.
309-309: Don’t hardcode fiat currency; pass defaultCurrency.Prevents mismatch if app currency ≠ USD.
- fiatCurrency: "USD", + fiatCurrency: defaultCurrency,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CLAUDE.md(2 hunks)DashWallet.xcodeproj/project.pbxproj(20 hunks)DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift(4 hunks)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{swift,h,m}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and keep lines under 180 characters (100 recommended)
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Avoid wholesale use of @objcMembers; expose only needed members (enforced by custom SwiftLint rule)
Avoid conditional compilation inside SwiftUI ViewBuilder expressions; use computed properties to encapsulate conditions
Prefer closure-based initialization for dictionaries when using conditional compilation to avoid syntax errors
Do not place #if conditionals mid-boolean expressions; assign to a conditional variable instead
Never force-unwrap latitude or longitude; use guard-let with error handling for coordinate values
Avoid force-unwraps of runtime properties (e.g., lastBounds!); prefer guard or optional binding with error handling
Use compactMap with explicit return types when creating map annotations to avoid type inference issues
Add explicit closure return type annotations when generic type inference fails
For location-based queries, first filter by rectangular bounds (SQL optimization), then apply precise circular distance filtering in memory
When using rectangular SQL bounds for circular searches, expand bounds by 50% to avoid excluding valid results before applying circular filtering
Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
After removing debug prints, ensure switch cases are not left empty; add break or remove the case
Prefer os.Logger (os_log) for production-safe logging over print
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift: Enforce View-ViewModel separation in SwiftUI: Views remain lightweight and delegate business logic to ViewModels
Use SwiftUI NavigationStack and navigationDestination for new screens
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift
**/*.{swift,m}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{swift,m}: Set template images to render with .withRenderingMode(.alwaysTemplate) where appropriate (e.g., SVG icons)
Use the kDefaultRadius constant (32000m) instead of hardcoded radius values
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift
🧠 Learnings (3)
📚 Learning: 2025-09-05T04:46:12.717Z
Learnt from: HashEngineering
PR: dashpay/dashwallet-ios#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:
DashWallet.xcodeproj/project.pbxprojCLAUDE.md
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
PR: dashpay/dashwallet-ios#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/Info.plist : Use $(MARKETING_VERSION) in Info.plist instead of hardcoded version strings; never edit versions directly in Info.plist
Applied to files:
DashWallet.xcodeproj/project.pbxprojCLAUDE.md
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
PR: dashpay/dashwallet-ios#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.swift : Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
🧬 Code graph analysis (2)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (4)
DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)
payWithDashUrl(73-89)DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (1)
updateIcon(126-179)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
purchaseGiftCard(135-204)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift (1)
purchaseGiftCard(277-305)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift (1)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (1)
selectProvider(93-96)
🔇 Additional comments (5)
DashWallet.xcodeproj/project.pbxproj (1)
10454-10454: Version bump to 8.4.3 is now complete — well done!All 20 MARKETING_VERSION entries have been consistently updated to 8.4.3 across all targets (dashwallet, dashpay, TodayExtension, WatchApp, WatchApp Extension) and configurations (Debug, Testnet, Release, Testflight). This addresses the previous review concern about stale version entries.
Based on learnings.
Also applies to: 10594-10594, 10768-10768, 10790-10790, 10815-10815, 10842-10842, 10874-10874, 10903-10903, 10953-10953, 11088-11088, 11234-11234, 11378-11378, 11582-11582, 11695-11695, 11751-11751, 11776-11776, 11893-11893, 12006-12006, 12061-12061, 12086-12086
CLAUDE.md (1)
5-21: Clear and well-structured critical policy.The Git Workflow Policy is appropriately positioned and unambiguously stated. The four-step process, example permission phrases, and cautionary note effectively communicate that commits/pushes require explicit user authorization. This is important guidance for AI-assisted workflows.
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift (3)
288-288: LGTM! Clean refactor using the helper view.The extraction of provider row rendering into a dedicated helper improves code organization and maintainability.
305-310: Past review comment addressed successfully.The status text call now correctly passes the provider parameter, making it provider-aware as requested in the previous review.
331-362: Excellent refactor! Past review comments fully addressed.Both helper methods are well-implemented:
providerRow(for:)cleanly extracts row rendering with proper styling and selection handlingproviderStatusText(for:isFixed:)is now correctly provider-aware, checking specifically for.ctxbefore applying themerchantEnabledstatusThis implementation directly addresses the previous review feedback requesting provider-aware status text.
- Fix CLAUDE.md verification command with correct grep logic for MARKETING_VERSION checks - Replace print statements with DSLogger.log in CustomIconMetadataProvider for proper logging - Fix locale-sensitive number formatting using NumberFormatter with en_US_POSIX locale Changes ensure proper version verification, consistent logging patterns, and locale-invariant decimal formatting to prevent rounding issues. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (2)
174-201: LGTM! Clear payment flow with proper guards.The refactored flow correctly:
- Obtains the gift card response via
purchaseGiftCardAPI()- Guards for payment URL existence with descriptive errors
- Processes payment through
sendCoinsService.payWithDashUrl()- Persists transaction metadata, icon, and gift card data on success
- Propagates errors to the caller
Optional: The outer
do-catchblock at lines 178–200 that simply rethrows could be removed since no additional error handling is performed:- do { - let response = try await purchaseGiftCardAPI() + let response = try await purchaseGiftCardAPI() - // Process the payment using the payment URL - guard let paymentUrls = response.paymentUrls else { - throw DashSpendError.paymentProcessingError("No payment URLs received") - } + // Process the payment using the payment URL + guard let paymentUrls = response.paymentUrls else { + throw DashSpendError.paymentProcessingError("No payment URLs received") + } - guard let paymentUrlString = paymentUrls.first?.value else { - throw DashSpendError.paymentProcessingError("No payment URL received") - } + guard let paymentUrlString = paymentUrls.first?.value else { + throw DashSpendError.paymentProcessingError("No payment URL received") + } - let transaction = try await sendCoinsService.payWithDashUrl(url: paymentUrlString) + let transaction = try await sendCoinsService.payWithDashUrl(url: paymentUrlString) - // Payment successful - save gift card information - markGiftCardTransaction(txId: transaction.txHashData) - customIconProvider.updateIcon(txId: transaction.txHashData, iconUrl: merchantIconUrl) - saveGiftCardDummy(txHashData: transaction.txHashData, giftCardId: response.paymentId) + // Payment successful - save gift card information + markGiftCardTransaction(txId: transaction.txHashData) + customIconProvider.updateIcon(txId: transaction.txHashData, iconUrl: merchantIconUrl) + saveGiftCardDummy(txHashData: transaction.txHashData, giftCardId: response.paymentId) - return transaction.txHashData - } catch { - throw error - } + return transaction.txHashData
316-326: Optional: Simplify redundant do-catch wrapper.Similar to the previous method, this
do-catchblock only rethrows without additional handling.- do { - let response = try await ctxSpendRepository.purchaseGiftCard( - merchantId: merchantId, - fiatAmount: fiatAmountString, - fiatCurrency: "USD", - cryptoCurrency: "DASH" - ) - return response - } catch { - throw error - } + return try await ctxSpendRepository.purchaseGiftCard( + merchantId: merchantId, + fiatAmount: fiatAmountString, + fiatCurrency: "USD", + cryptoCurrency: "DASH" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CLAUDE.md(2 hunks)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift(2 hunks)DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CLAUDE.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{swift,h,m}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and keep lines under 180 characters (100 recommended)
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Avoid wholesale use of @objcMembers; expose only needed members (enforced by custom SwiftLint rule)
Avoid conditional compilation inside SwiftUI ViewBuilder expressions; use computed properties to encapsulate conditions
Prefer closure-based initialization for dictionaries when using conditional compilation to avoid syntax errors
Do not place #if conditionals mid-boolean expressions; assign to a conditional variable instead
Never force-unwrap latitude or longitude; use guard-let with error handling for coordinate values
Avoid force-unwraps of runtime properties (e.g., lastBounds!); prefer guard or optional binding with error handling
Use compactMap with explicit return types when creating map annotations to avoid type inference issues
Add explicit closure return type annotations when generic type inference fails
For location-based queries, first filter by rectangular bounds (SQL optimization), then apply precise circular distance filtering in memory
When using rectangular SQL bounds for circular searches, expand bounds by 50% to avoid excluding valid results before applying circular filtering
Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
After removing debug prints, ensure switch cases are not left empty; add break or remove the case
Prefer os.Logger (os_log) for production-safe logging over print
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift: Enforce View-ViewModel separation in SwiftUI: Views remain lightweight and delegate business logic to ViewModels
Use SwiftUI NavigationStack and navigationDestination for new screens
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift
**/*.{swift,m}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{swift,m}: Set template images to render with .withRenderingMode(.alwaysTemplate) where appropriate (e.g., SVG icons)
Use the kDefaultRadius constant (32000m) instead of hardcoded radius values
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift
🧠 Learnings (1)
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
PR: dashpay/dashwallet-ios#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.swift : Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
🧬 Code graph analysis (2)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (4)
DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)
payWithDashUrl(73-89)DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (1)
updateIcon(126-179)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
purchaseGiftCard(135-204)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift (1)
purchaseGiftCard(277-305)
DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (1)
DashWallet/Sources/Categories/Foundation+Bitcoin.swift (1)
hexEncodedString(43-46)
🔇 Additional comments (4)
DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (3)
129-131: LGTM! Production-safe error logging implemented.The invalid URL path now uses
DSLogger.log()with appropriate context (URL and txId), addressing the previous review feedback to remove rawprint()statements.
136-138: LGTM! Consistent error logging for image creation failure.The failed image creation path correctly uses
DSLogger.log()with full context, maintaining consistency with the project's logging pattern.
175-176: LGTM! Comprehensive error logging in catch block.The catch block properly logs fetch failures with full context (URL, txId, and error details) using
DSLogger.log(), completing the production-safe logging improvements for the icon update flow.DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (1)
301-312: LGTM! Locale-invariant formatting correctly implemented.The
NumberFormatterconfiguration addresses the previous review feedback perfectly:
- Uses
en_US_POSIXlocale for consistency- Enforces exactly 2 decimal places
- Sets explicit
"."decimal separator- Disables grouping separators
- Uses
NSDecimalNumberto avoidDoublerounding driftThis ensures the API payload always contains properly formatted decimal amounts regardless of device locale.
HashEngineering
left a comment
HashEngineering
left a comment
There was a problem hiding this comment.
Very good. All matters have been resolved.
- Updated clang-format build scripts to exclude DWUpholdMainnetConstants.m - This file was getting unwanted whitespace changes on every build - Updated CLAUDE.md documentation to explain the root cause - The file will no longer be modified by the automatic formatting scripts Co-Authored-By: Claude <noreply@anthropic.com>
…innet - Removed cached kBaseURL constant that was preventing network switching - baseURL property now computes URL dynamically based on current network - This fixes the issue where switching from testnet to mainnet (or vice versa) required app restart - Updated CLAUDE.md with network switching documentation and best practices The issue occurred because the API endpoint was cached at initialization time. Now it correctly evaluates the network on each request, ensuring: - Testnet uses: http://staging.spend.ctx.com/ - Mainnet uses: https://spend.ctx.com/ Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift (1)
64-68: Consider safer URL initialization for better crash diagnostics.While the force unwrap follows the documented pattern in CLAUDE.md, using a guard statement or fatalError with a descriptive message would make debugging easier if CTXConstants.baseURI ever returns an invalid URL.
Apply this diff to improve crash diagnostics:
- public var baseURL: URL { - // Dynamically compute the base URL based on the current network - // This ensures the correct endpoint is used even when switching between testnet/mainnet - return URL(string: CTXConstants.baseURI)! - } + public var baseURL: URL { + // Dynamically compute the base URL based on the current network + // This ensures the correct endpoint is used even when switching between testnet/mainnet + guard let url = URL(string: CTXConstants.baseURI) else { + fatalError("Invalid CTX base URI: \(CTXConstants.baseURI)") + } + return url + }Note: The same pattern in CLAUDE.md lines 400-406 should also be updated if this change is adopted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CLAUDE.md(4 hunks)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{swift,h,m}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and keep lines under 180 characters (100 recommended)
Files:
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Avoid wholesale use of @objcMembers; expose only needed members (enforced by custom SwiftLint rule)
Avoid conditional compilation inside SwiftUI ViewBuilder expressions; use computed properties to encapsulate conditions
Prefer closure-based initialization for dictionaries when using conditional compilation to avoid syntax errors
Do not place #if conditionals mid-boolean expressions; assign to a conditional variable instead
Never force-unwrap latitude or longitude; use guard-let with error handling for coordinate values
Avoid force-unwraps of runtime properties (e.g., lastBounds!); prefer guard or optional binding with error handling
Use compactMap with explicit return types when creating map annotations to avoid type inference issues
Add explicit closure return type annotations when generic type inference fails
For location-based queries, first filter by rectangular bounds (SQL optimization), then apply precise circular distance filtering in memory
When using rectangular SQL bounds for circular searches, expand bounds by 50% to avoid excluding valid results before applying circular filtering
Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
After removing debug prints, ensure switch cases are not left empty; add break or remove the case
Prefer os.Logger (os_log) for production-safe logging over print
Files:
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift
**/*.{swift,m}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{swift,m}: Set template images to render with .withRenderingMode(.alwaysTemplate) where appropriate (e.g., SVG icons)
Use the kDefaultRadius constant (32000m) instead of hardcoded radius values
Files:
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift
🧠 Learnings (3)
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
PR: dashpay/dashwallet-ios#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to DashWallet/Sources/Models/Uphold/DWUpholdMainnetConstants.m : Avoid committing whitespace-only changes to DWUpholdMainnetConstants.m; restore if only whitespace changed
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-05T04:46:12.717Z
Learnt from: HashEngineering
PR: dashpay/dashwallet-ios#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-10-02T16:19:48.485Z
Learnt from: CR
PR: dashpay/dashwallet-ios#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/Info.plist : Use $(MARKETING_VERSION) in Info.plist instead of hardcoded version strings; never edit versions directly in Info.plist
Applied to files:
CLAUDE.md
🧬 Code graph analysis (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (4)
login(67-69)verifyEmail(87-104)refreshToken(129-131)getMerchant(206-234)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
315-315: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🪛 SwiftLint (0.57.0)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift
[Warning] 67-67: Force unwrapping should be avoided
(force_unwrapping)
🔇 Additional comments (3)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift (1)
57-61: LGTM! Authorization handling correctly updated.The move of
.getMerchantto the nil-token authorization group is correct based on the verification in past review comments confirming this endpoint does not require authorization.CLAUDE.md (2)
5-21: LGTM! Clear policy for AI-assisted development.The Git workflow policy provides clear guidance to prevent accidental commits when using AI code assistants, with explicit permission requirements and example phrases.
341-362: LGTM! Version verification commands are now correct.The verification methods are well-documented and logically sound. Method 1 correctly identifies mismatches by showing entries that don't match the target version, and Method 2 confirms consistency by counting distinct values.
When PIGGYCARDS_ENABLED is not defined, the merchant list was incorrectly showing combined discounts (e.g., 14% for Domino's) instead of CTX-only discounts (6%). This fix: - Adds calculateEffectiveDiscount() method to extract CTX-only discounts - Properly converts basis points to percentages (600 -> 6%) - Filters out PiggyCards discounts when the feature flag is disabled - Ensures merchant list shows the same discount as the details view 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add 'X-Client-Id: dcg_ios' header to all CTX API requests for proper client identification and tracking. This header helps CTX identify traffic from the iOS Dash Wallet application. While production doesn't strictly require this header, it's recommended for analytics and may be required for staging environment access. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix staging URL to use HTTPS instead of HTTP - Add authentication to getMerchant endpoint for staging compatibility - Handle dual field names (savingsPercentage/userDiscount) for API compatibility - Fix provider filtering to exclude PiggyCards data when feature is disabled - Use provider denomination type instead of merchant table for accurate data - Add comprehensive documentation for CTX/DashSpend API integration These changes ensure the app works correctly with both staging and production CTX APIs despite their structural differences. 🤖 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: 3
🧹 Nitpick comments (3)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift (1)
68-71: Consider using guard-let instead of force unwrapping.The force unwrap is flagged by SwiftLint and violates coding guidelines preferring guard or optional binding. While
CTXConstants.baseURIcurrently returns valid URLs, defensive coding would use a guard statement.Apply this diff:
public var baseURL: URL { // Dynamically compute the base URL based on the current network // This ensures the correct endpoint is used even when switching between testnet/mainnet - return URL(string: CTXConstants.baseURI)! + guard let url = URL(string: CTXConstants.baseURI) else { + fatalError("Invalid CTXConstants.baseURI: \(CTXConstants.baseURI)") + } + return url }As per coding guidelines.
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (2)
220-234: Optional: Simplify pattern matching syntax.SwiftLint suggests moving the
letkeyword out of tuples in the DecodingError switch cases. This is a minor style improvement.Apply this diff:
} catch let decodingError as DecodingError { DSLogger.log("🔍 CTXSpendRepository.purchaseGiftCard - JSON Decoding Error: \(decodingError)") switch decodingError { - case .dataCorrupted(let context): + case let .dataCorrupted(context): DSLogger.log("🔍 Data corrupted: \(context)") - case .keyNotFound(let key, let context): + case let .keyNotFound(key, context): DSLogger.log("🔍 Key not found: \(key), context: \(context)") - case .typeMismatch(let type, let context): + case let .typeMismatch(type, context): DSLogger.log("🔍 Type mismatch: \(type), context: \(context)") - case .valueNotFound(let type, let context): + case let .valueNotFound(type, context): DSLogger.log("🔍 Value not found: \(type), context: \(context)") @unknown default: DSLogger.log("🔍 Unknown decoding error") }As per SwiftLint.
245-259: Optional: Use isEmpty instead of count comparison.Line 251 compares count to zero, which SwiftLint suggests should use
isEmptyfor clarity.Apply this diff:
DSLogger.log("🔍 denominations array: \(response.denominations)") DSLogger.log("🔍 denominations count: \(response.denominations.count)") - if response.denominations.count > 0 { + if !response.denominations.isEmpty { DSLogger.log("🔍 First denomination: \(response.denominations[0])") }As per SwiftLint.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CLAUDE.md(5 hunks)DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift(3 hunks)DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swift(1 hunks)DashWallet/Sources/Models/Explore Dash/Model/Entites/CTXSpendModels.swift(2 hunks)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift(2 hunks)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift(4 hunks)DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swift(1 hunks)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- DashWallet/Sources/Models/Explore Dash/Model/Entites/CTXSpendModels.swift
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{swift,h,m}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and keep lines under 180 characters (100 recommended)
Files:
DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swiftDashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Avoid wholesale use of @objcMembers; expose only needed members (enforced by custom SwiftLint rule)
Avoid conditional compilation inside SwiftUI ViewBuilder expressions; use computed properties to encapsulate conditions
Prefer closure-based initialization for dictionaries when using conditional compilation to avoid syntax errors
Do not place #if conditionals mid-boolean expressions; assign to a conditional variable instead
Never force-unwrap latitude or longitude; use guard-let with error handling for coordinate values
Avoid force-unwraps of runtime properties (e.g., lastBounds!); prefer guard or optional binding with error handling
Use compactMap with explicit return types when creating map annotations to avoid type inference issues
Add explicit closure return type annotations when generic type inference fails
For location-based queries, first filter by rectangular bounds (SQL optimization), then apply precise circular distance filtering in memory
When using rectangular SQL bounds for circular searches, expand bounds by 50% to avoid excluding valid results before applying circular filtering
Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
After removing debug prints, ensure switch cases are not left empty; add break or remove the case
Prefer os.Logger (os_log) for production-safe logging over print
Files:
DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swiftDashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
**/*.{swift,m}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{swift,m}: Set template images to render with .withRenderingMode(.alwaysTemplate) where appropriate (e.g., SVG icons)
Use the kDefaultRadius constant (32000m) instead of hardcoded radius values
Files:
DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swiftDashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift: Enforce View-ViewModel separation in SwiftUI: Views remain lightweight and delegate business logic to ViewModels
Use SwiftUI NavigationStack and navigationDestination for new screens
Files:
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
🧠 Learnings (16)
📚 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/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swiftCLAUDE.mdDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.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/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftCLAUDE.mdDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to DashWallet/Sources/Models/Uphold/DWUpholdMainnetConstants.m : Avoid committing whitespace-only changes to DWUpholdMainnetConstants.m; restore if only whitespace changed
Applied to files:
CLAUDE.mdDashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 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-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/Info.plist : Use $(MARKETING_VERSION) in Info.plist instead of hardcoded version strings; never edit versions directly in Info.plist
Applied to files:
CLAUDE.md
📚 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-09-05T04:46:21.894Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 728
File: DashWallet.xcodeproj/project.pbxproj:3174-3180
Timestamp: 2025-09-05T04:46:21.894Z
Learning: Xcode project files (.pbxproj) are automatically generated by Xcode and should not be manually edited. Hash changes in Pod library references (like libPods-dashwallet.a) are normal and occur when CocoaPods updates or when Xcode regenerates the project structure.
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.{swift,h,m} : Use 4-space indentation and keep lines under 180 characters (100 recommended)
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.{h,m} : Follow the NYTimes Objective-C Style Guide for Objective-C code
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to DashWalletTests/**/*.swift : Keep unit tests under DashWalletTests/ using XCTest with testable imports and mocks for isolation
Applied to files:
CLAUDE.mdDashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to {DashWallet/Sources/UI,SwiftUI Components}/**/*.swift : Enforce View-ViewModel separation in SwiftUI: Views remain lightweight and delegate business logic to ViewModels
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to {DashWallet/Sources/UI,SwiftUI Components}/**/*.swift : Use SwiftUI NavigationStack and navigationDestination for new screens
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.swift : Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.swift : Prefer os.Logger (os_log) for production-safe logging over print
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.swift : After removing debug prints, ensure switch cases are not left empty; add break or remove the case
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-09-27T15:37:44.449Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift:94-96
Timestamp: 2025-09-27T15:37:44.449Z
Learning: User bfoss765 prefers to defer fixing force unwrapping warnings until crashes are observed during testing, rather than proactively addressing them.
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
🧬 Code graph analysis (4)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swift (1)
DashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swift (1)
toSavingPercentages(101-103)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (2)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swift (2)
request(24-45)request(47-68)DashWallet/Sources/Categories/Foundation+Bitcoin.swift (1)
hexEncodedString(43-46)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
getMerchant(241-285)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (4)
DashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swift (1)
toSavingsFraction(105-107)DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)
payWithDashUrl(73-89)DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (1)
updateIcon(126-179)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (2)
getMerchant(241-285)purchaseGiftCard(135-239)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
315-315: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🪛 SwiftLint (0.57.0)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift
[Error] 251-251: Prefer checking isEmpty over comparing count to zero
(empty_count)
[Warning] 225-225: Combine multiple pattern matching bindings by moving keywords out of tuples
(pattern_matching_keywords)
[Warning] 225-225: Combine multiple pattern matching bindings by moving keywords out of tuples
(pattern_matching_keywords)
[Warning] 227-227: Combine multiple pattern matching bindings by moving keywords out of tuples
(pattern_matching_keywords)
[Warning] 227-227: Combine multiple pattern matching bindings by moving keywords out of tuples
(pattern_matching_keywords)
[Warning] 229-229: Combine multiple pattern matching bindings by moving keywords out of tuples
(pattern_matching_keywords)
[Warning] 229-229: Combine multiple pattern matching bindings by moving keywords out of tuples
(pattern_matching_keywords)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift
[Warning] 71-71: Force unwrapping should be avoided
(force_unwrapping)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
[Warning] 141-141: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (10)
CLAUDE.md (3)
388-407: Dynamic CTX baseURL guidance aligns with PR objective.The new section documenting dynamic network switching (lines 388–407) correctly captures the critical pattern: computing the baseURL on each request rather than caching it as a constant. This pattern ensures environment changes (testnet ↔ mainnet) are reflected without requiring an app restart.
The example code (lines 398–407) clearly distinguishes the ❌ wrong approach (static cache) from the ✅ correct approach (dynamic computation). This guidance directly supports the PR objective of fixing CTX JSON parsing by ensuring the correct endpoint is targeted.
1002-1246: Verify comprehensive CTX/DashSpend API documentation against implementation.The new section (lines 1002–1246) provides extensive documentation on CTX/DashSpend integration, covering environment differences, field name mappings, authentication asymmetries, response structure variations, common issues, database patterns, and testing guidelines. This is valuable reference material.
To ensure accuracy and completeness:
- Environment URLs (lines 1009–1020): Confirm staging endpoint is indeed
https://staging.spend.ctx.com/(HTTPS, not HTTP).- Field Names (lines 1022–1034): Verify that production uses
savingsPercentageand staging usesuserDiscountfor discounts.- Authentication (lines 1036–1052): Confirm that staging's
getMerchantendpoint requires auth but production doesn't.- Response Structures (lines 1054–1102): Verify gift card response parsing handles both paginated (staging) and direct-object (production) formats.
- Provider Filtering (lines 1179–1201): Ensure the
gift_card_providerstable and filtering pattern accurately reflect current database schema and query behavior.- Alignment with Implementation: Verify that sections like "Issue 1: CTX-Only Discount Display," "Issue 2: Transaction ID vs Gift Card UUID," and "Issue 3: Denomination Type Source" reflect actual problems solved or bugs fixed in this PR.
354-361: Verification commands successfully address previous review feedback.The updated verification methods (lines 354–361) correctly replace the earlier inverted grep logic. Both approaches are now sound:
- Method 1 (line 356): Checks for entries that do NOT match the target version (returns empty if all are correct).
- Method 2 (lines 358–360): Counts distinct version values (should be exactly 1).
The placeholder
X.Y.Zreminds contributors to substitute their target version. This is a good improvement over the prior iteration.DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendEndpoint.swift (1)
59-62: LGTM - getMerchant authorization handling is appropriate.The authorization logic for
getMerchanthas been correctly updated to use.bearerfor both staging and production environments, with a clear explanatory comment. This aligns with the previous review discussion.DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift (1)
180-191: LGTM - Consistent provider filtering pattern.The conditional compilation pattern correctly filters gift card providers based on the
PIGGYCARDS_ENABLEDflag across all three query paths. The implementation is consistent and the comments clearly explain the behavior.Also applies to: 410-421, 605-616
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swift (1)
61-91: LGTM - Provider-aware discount calculation is well-structured.The new
calculateEffectiveDiscounthelper correctly handles provider-specific discounts with proper conditional compilation. The basis point conversion (divide by 100.0) is accurate, and the code gracefully handles missing CTX providers.DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (3)
241-276: LGTM - Improved error handling and transaction flow.The refactored
purchaseGiftCardAndPay()method now has structured error handling with clear validation steps and proper transaction state persistence. The nested do-catch blocks appropriately isolate payment processing errors.
347-387: LGTM - Enhanced merchant info updates with comprehensive logging.The
updateMerchantInfo()method now includes thorough debug logging and correctly usesmerchantInfo.discountto handle multiple discount sources. The guard checks and denomination handling are appropriate.
390-422: LGTM - Correct locale-invariant fiat amount formatting.The
purchaseGiftCardAPI()method correctly usesNumberFormatterwithen_US_POSIXlocale to ensure consistent decimal formatting for API payloads. The use ofNSDecimalNumberavoids floating-point rounding issues.DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
143-219: LGTM - Comprehensive error handling and debug logging.The enhanced error handling in
purchaseGiftCard()provides thorough HTTP status code mapping and detailed logging for troubleshooting. The fallback to hex encoding for non-UTF8 response data (line 171) is a nice touch for debugging binary responses.
- Fixed documentation comment in CTXConstants.swift to correctly show https:// URL for staging - Removed redundant nil initialization in DashSpendPayViewModel.swift 🤖 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swift(1 hunks)DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{swift,h,m}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and keep lines under 180 characters (100 recommended)
Files:
DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Avoid wholesale use of @objcMembers; expose only needed members (enforced by custom SwiftLint rule)
Avoid conditional compilation inside SwiftUI ViewBuilder expressions; use computed properties to encapsulate conditions
Prefer closure-based initialization for dictionaries when using conditional compilation to avoid syntax errors
Do not place #if conditionals mid-boolean expressions; assign to a conditional variable instead
Never force-unwrap latitude or longitude; use guard-let with error handling for coordinate values
Avoid force-unwraps of runtime properties (e.g., lastBounds!); prefer guard or optional binding with error handling
Use compactMap with explicit return types when creating map annotations to avoid type inference issues
Add explicit closure return type annotations when generic type inference fails
For location-based queries, first filter by rectangular bounds (SQL optimization), then apply precise circular distance filtering in memory
When using rectangular SQL bounds for circular searches, expand bounds by 50% to avoid excluding valid results before applying circular filtering
Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
After removing debug prints, ensure switch cases are not left empty; add break or remove the case
Prefer os.Logger (os_log) for production-safe logging over print
Files:
DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
**/*.{swift,m}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{swift,m}: Set template images to render with .withRenderingMode(.alwaysTemplate) where appropriate (e.g., SVG icons)
Use the kDefaultRadius constant (32000m) instead of hardcoded radius values
Files:
DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
{DashWallet/Sources/UI,SwiftUI Components}/**/*.swift: Enforce View-ViewModel separation in SwiftUI: Views remain lightweight and delegate business logic to ViewModels
Use SwiftUI NavigationStack and navigationDestination for new screens
Files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
🧠 Learnings (10)
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to DashWallet/Sources/Models/Uphold/DWUpholdMainnetConstants.m : Avoid committing whitespace-only changes to DWUpholdMainnetConstants.m; restore if only whitespace changed
Applied to files:
DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to DashWalletTests/**/*.swift : Keep unit tests under DashWalletTests/ using XCTest with testable imports and mocks for isolation
Applied to files:
DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to {DashWallet/Sources/UI,SwiftUI Components}/**/*.swift : Enforce View-ViewModel separation in SwiftUI: Views remain lightweight and delegate business logic to ViewModels
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to {DashWallet/Sources/UI,SwiftUI Components}/**/*.swift : Use SwiftUI NavigationStack and navigationDestination for new screens
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.swift : Wrap debug logging in #if DEBUG blocks and prefer a debugLog helper; avoid raw print statements in release paths
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.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/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.swift : Prefer os.Logger (os_log) for production-safe logging over print
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-10-02T16:19:48.485Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T16:19:48.485Z
Learning: Applies to **/*.swift : After removing debug prints, ensure switch cases are not left empty; add break or remove the case
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.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/DashSpendPayViewModel.swift
📚 Learning: 2025-09-27T15:37:44.449Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift:94-96
Timestamp: 2025-09-27T15:37:44.449Z
Learning: User bfoss765 prefers to defer fixing force unwrapping warnings until crashes are observed during testing, rather than proactively addressing them.
Applied to files:
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
🧬 Code graph analysis (1)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (4)
DashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swift (1)
toSavingsFraction(105-107)DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)
payWithDashUrl(73-89)DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (1)
updateIcon(126-179)DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (2)
getMerchant(241-285)purchaseGiftCard(135-239)
🔇 Additional comments (5)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (4)
125-203: Well-structured provider-aware initialization.The initialization logic correctly implements Swift's two-phase initialization pattern by computing values in local variables before
super.init()and assigning them to instance properties afterward. The provider-specific denomination handling and fallback logic are clear and properly separated.The previous redundant optional initialization concern has been addressed—line 141 now correctly declares
var providerDenominationType: String?without the unnecessary= nil.
241-277: Excellent error handling and payment flow sequencing.The payment flow is well-structured with proper validation of
paymentUrls, nested error handling, and correct transaction state persistence. The use ofDSLogger.log()at key points (lines 257, 262, 264) provides appropriate production-safe tracing for this critical payment path.The previous concern about debug print statements has been properly addressed by converting them to DSLogger calls.
347-388: Robust merchant info synchronization with proper denomination handling.The method correctly updates merchant details from the CTX API with appropriate precondition checks (line 348-351) and proper handling of both Range and Fixed denomination types. The use of the
discountproperty (line 368) ensures consistent discount calculation across provider and merchant data sources.
390-422: Proper locale-invariant amount formatting for API payload.The implementation correctly addresses the previous concern about locale-sensitive formatting by using
NumberFormatterwithen_US_POSIXlocale (lines 396-407). The use ofNSDecimalNumber(line 406) avoids Double conversion, preventing potential rounding drift in the amount calculation.The error handling properly propagates exceptions from the repository layer.
DashWallet/Sources/Models/Explore Dash/Model/DashSpend/CTXConstants.swift (1)
18-19: LGTM!The Foundation import is appropriate for this file.
Removed 100+ debug logging statements that were added during development: - Removed all DSLogger.log statements with 🔍 debug markers - Cleaned up debug output from CTXConstants, CTXSpendRepository, POIDetailsViewModel, and DashSpendPayViewModel - Retained only essential error logging for production use This cleanup improves performance and reduces console noise in production builds. 🤖 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: 1
🧹 Nitpick comments (1)
DashWallet.xcodeproj/project.pbxproj (1)
8144-8144: ExcludingDWUpholdMainnetConstants.mfrom clang‑format looks fineThe added
-not -path "*/DWUpholdMainnetConstants.m"filters are valid and safely scope the change to clang‑format runs only. No functional/build impact, and both ObjC format build phases are kept in sync.Minor nit: the hard‑coded path is duplicated in two shell scripts; if this exclusion list grows, consider centralizing the script (or at least the list) to avoid future drift, but this is optional.
Based on learnings
Also applies to: 8423-8423
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.claude/agents/AI-DEVELOPMENT-GUIDE.md(1 hunks)CLAUDE.md(1 hunks)DashWallet.xcodeproj/project.pbxproj(2 hunks)DashWallet/en.lproj/Localizable.strings(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-05T04:46:21.894Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 728
File: DashWallet.xcodeproj/project.pbxproj:3174-3180
Timestamp: 2025-09-05T04:46:21.894Z
Learning: Xcode project files (.pbxproj) are automatically generated by Xcode and should not be manually edited. Hash changes in Pod library references (like libPods-dashwallet.a) are normal and occur when CocoaPods updates or when Xcode regenerates the project structure.
Applied to files:
CLAUDE.mdDashWallet.xcodeproj/project.pbxproj
📚 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:
DashWallet/en.lproj/Localizable.strings.claude/agents/AI-DEVELOPMENT-GUIDE.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:
DashWallet.xcodeproj/project.pbxproj.claude/agents/AI-DEVELOPMENT-GUIDE.md
🪛 markdownlint-cli2 (0.18.1)
.claude/agents/AI-DEVELOPMENT-GUIDE.md
16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
DashWallet/en.lproj/Localizable.strings (1)
961-963: String addition is fine; just ensure it flows through the Transifex pipelineThe new DashSpend
"Temporarily unavailable"key/value looks consistent with neighboring CTX/DashSpend strings. Please just confirm this key is added via the normal Transifex-driven localization workflow (and propagated to other locales there), rather than manually editing translated.lprojfiles. Based on learningsCLAUDE.md (1)
5-22: Git workflow policy section is clear and matches the intended STOP/WAIT behaviorThe new policy block cleanly encodes the “change → show diff → STOP/WAIT → only commit/push on explicit phrases” workflow and aligns with the guidance in
.claude/agents/AI-DEVELOPMENT-GUIDE.md. No changes needed here.
2 participants
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Localization
✏️ Tip: You can customize this high-level summary in your review settings.