feat: Maya Enter Destination Address (Requirement 2)#756
Merged
Conversation
…t 2) Build the address entry screen where users enter/paste/scan the destination wallet address for the selected cryptocurrency. Includes a generic QR scanner since the existing Dash-only scanner rejects non-Dash addresses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a full Maya Protocol integration: asset catalog entries, new Maya data models and API endpoints, SwiftUI screens (SelectCoin, EnterAddress, MayaPortal), UIKit hosting controllers, QR scanner, and Buy & Sell portal wiring with navigation changes and project file updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BuySell as BuySellPortalView
participant MayaPortal as MayaPortalView
participant SelectCoin as SelectCoinView
participant SelectVM as SelectCoinViewModel
participant API as MayaAPIService
participant EnterAddr as EnterAddressView
participant QR as GenericQRScannerController
User->>BuySell: Tap Maya card
BuySell->>MayaPortal: Present Maya portal
User->>MayaPortal: Tap Convert Dash
MayaPortal->>SelectCoin: Push SelectCoinView
SelectCoin->>SelectVM: init/loadCoins()
SelectVM->>API: fetchPools()
API-->>SelectVM: pools
SelectVM->>API: fetchInboundAddresses()
API-->>SelectVM: inbound addresses
SelectVM->>SelectVM: build CoinDisplayItem list (prices, halted)
SelectVM-->>SelectCoin: coins ready
User->>SelectCoin: Select coin
SelectCoin->>EnterAddr: Push EnterAddressView
alt QR flow
User->>EnterAddr: Tap Scan QR
EnterAddr->>QR: Present scanner
QR-->>EnterAddr: onQRCodeScanned(address)
else Paste flow
User->>EnterAddr: Paste clipboard
end
EnterAddr->>EnterAddr: Validate address
EnterAddr->>User: Continue (onAddressConfirmed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
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/Buy Sell/BuySellPortalViewController.swift (1)
244-267:⚠️ Potential issue | 🟠 MajorGeoblock lookup can suspend forever when placemark resolution fails.
nextEmittedPlacemark()only resumes on a non-nil$currentPlacemark. InDWLocationManager.reverseGeocodeLocation, failure paths can leavecurrentPlacemarknil, soawait isGeoblocked()can hang indefinitely after the Coinbase tap. Add a timeout or failure path so the flow always resolves.Run this to inspect the continuation bridge next to
DWLocationManager's placemark publishing. Expect the first snippet to wait for a non-nil placemark only, while the second shows publishing paths that do not guarantee one.#!/bin/bash set -euo pipefail fd BuySellPortalViewController.swift DashWallet -x sed -n '244,267p' {} printf '\n---\n' fd DWLocationManager.swift DashWallet -x rg -n -C3 'currentPlacemark|reverseCurrentLocation|reverseGeocodeLocation|placemark' {}As per coding guidelines, "Always use checked continuations with resumed flag to prevent double-resumption crashes when bridging Vision framework completion handlers to async/await."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Buy` Sell/BuySellPortalViewController.swift around lines 244 - 267, The isGeoblocked() path can hang because nextEmittedPlacemark() only resumes for a non-nil $currentPlacemark while DWLocationManager.reverseGeocodeLocation may never publish a placemark; update nextEmittedPlacemark() to enforce a bounded wait and a safe single-resume pattern (e.g., use withCheckedContinuation or withCheckedThrowingContinuation plus a resumed flag or timeout Task that resumes with a failure/default after X seconds), ensure the AnyCancellable from DWLocationManager.shared.$currentPlacemark is cancelled when the continuation is resumed or timed out, and propagate/handle the timeout error back in isGeoblocked() so the flow always returns (reference nextEmittedPlacemark(), isGeoblocked(), DWLocationManager.shared.$currentPlacemark, and reverseGeocodeLocation).
🧹 Nitpick comments (6)
DashWallet/Sources/UI/Maya/MayaPortalView.swift (1)
33-33: Consider extracting hardcoded color to a named asset.The Maya background color
Color(red: 0.08, green: 0.11, blue: 0.25)could be defined as a named color in the asset catalog for consistency and easier theming.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/MayaPortalView.swift` at line 33, Replace the hardcoded Color(red: 0.08, green: 0.11, blue: 0.25) used in MayaPortalView (e.g., in the view body where .fill(...) is applied) with a named asset color; add a new color asset (e.g., "MayaBackground") to the asset catalog and then use Color("MayaBackground") (or a small Color extension if you prefer) in the .fill call so the background color is centralized for theming and reuse.DashWallet/Sources/UI/Maya/SelectCoinView.swift (1)
118-130: Same trailing closure pattern as noted in EnterAddressView.SwiftLint flags the trailing closure syntax on line 122. Consider using explicit
label:parameter for consistency with linting rules.♻️ Proposed fix
- Button(action: { + Button(action: { Task { await viewModel.loadCoins() } - }) { + }, label: { Text(NSLocalizedString("Retry", comment: "")) .font(.system(size: 14, weight: .semibold)) .foregroundColor(.white) .padding(.horizontal, 24) .padding(.vertical, 10) .background(Color.dashBlue) .cornerRadius(8) - } + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/SelectCoinView.swift` around lines 118 - 130, The Button currently uses a trailing closure for its label which triggers SwiftLint; change the Button to use the explicit label parameter form so the action and label are both passed as named parameters (keep the existing action that calls Task { await viewModel.loadCoins() } and the existing label content with Text/formatting), referencing the Button declaration and viewModel.loadCoins() call so the change is applied to that exact component.DashWallet/Sources/UI/Maya/EnterAddressView.swift (2)
71-77: Address SwiftLint trailing closure warning.SwiftLint flags using trailing closure syntax when passing multiple closure arguments. Consider using explicit
label:parameter for consistency.♻️ Proposed fix
- Button(action: { onScanQR?() }) { + Button(action: { onScanQR?() }, label: { Image("scan-qr.accessory.icon") .resizable() .aspectRatio(contentMode: .fit) .frame(width: 20, height: 20) .foregroundColor(.secondaryText) - } + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/EnterAddressView.swift` around lines 71 - 77, SwiftLint warns about using trailing-closure syntax when multiple closure parameters are present in the Button initializer; update the Button call in EnterAddressView (the Button(action: { onScanQR?() }) that currently uses a trailing closure for the label) to use the explicit label: parameter (i.e., Button(action: { onScanQR?() }, label: { /* Image(...) */ })) so the action and label closures are passed explicitly and the warning is resolved.
50-52: Consider using SwiftUI's@FocusStatefor keyboard dismissal.The current approach using
UIApplication.shared.sendActionworks but is a UIKit-based solution. SwiftUI's@FocusStateprovides a more idiomatic approach for managing keyboard focus.♻️ Alternative approach using `@FocusState`
// Add to struct properties: `@FocusState` private var isAddressFieldFocused: Bool // Update TextField: TextField(viewModel.placeholderText, text: $viewModel.addressText) .focused($isAddressFieldFocused) // ... // Update tap gesture: .onTapGesture { isAddressFieldFocused = false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/EnterAddressView.swift` around lines 50 - 52, Replace the UIKit-based keyboard dismissal in EnterAddressView by adding a SwiftUI `@FocusState` property (e.g., private var isAddressFieldFocused: Bool) to the view struct, attach .focused($isAddressFieldFocused) to the TextField(viewModel.placeholderText, text: $viewModel.addressText) and change the .onTapGesture handler to set isAddressFieldFocused = false instead of calling UIApplication.shared.sendAction; remove the UIKit call so focus is managed idiomatically by SwiftUI.DashWallet/Sources/UI/Maya/EnterAddressViewModel.swift (1)
34-36: Consider adding coin-specific address validation.The current
isAddressValidonly checks for non-empty input. Since Maya supports multiple cryptocurrencies with different address formats, you might want to add basic format validation per coin (e.g., BTC addresses start with specific prefixes, ETH addresses are 42 chars starting with 0x).This could prevent users from proceeding with obviously invalid addresses before the API call.
Would you like me to help design a basic address validation approach for common cryptocurrencies?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/EnterAddressViewModel.swift` around lines 34 - 36, Replace the simplistic non-empty check in isAddressValid with coin-specific format validation: trim addressText and route it to a new helper (e.g., validateAddress(for:coin:)) that switches on the view model's coin identifier (use your existing coin property such as selectedCoin/coinType) and applies lightweight rules (BTC: allowed prefixes and base58/Bech32 patterns, ETH: startsWith "0x" and length 42, etc.); update isAddressValid to return the result of that helper so UI gating uses coin-aware validation while keeping trimming/empty checks.DashWallet/Sources/UI/Maya/SelectCoinViewModel.swift (1)
54-55: Inject the network and FX dependencies into the ViewModel.
MayaAPIService.sharedandCurrencyExchanger.sharedbake global state into the ViewModel, which makes loading/error cases hard to unit-test. An initializer that accepts protocol-backed services would keep this MVVM surface testable.As per coding guidelines, "Use protocol-based design with extensive use of protocols for dependency injection." and "Use service layer with dedicated services for major functionality (CoinJoin, networking, database)."
Also applies to: 115-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/SelectCoinViewModel.swift` around lines 54 - 55, SelectCoinViewModel currently references global singletons (MayaAPIService.shared and CurrencyExchanger.shared) which prevents unit testing; add protocol abstractions (e.g., MayaAPIServiceProtocol and CurrencyExchangerProtocol) and change SelectCoinViewModel to accept these as init dependencies (with default parameters that wrap the current shared singletons for backward compatibility), replace direct calls to MayaAPIService.shared.fetchPools()/fetchInboundAddresses() and CurrencyExchanger.shared usage with the injected protocol instances, and update other similar usages (the other async lets around lines 115-119) to use the injected network service; update tests to inject mock implementations of the protocols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@DashWallet/Sources/UI/Buy` Sell/Model/ServiceDataProvider.swift:
- Around line 43-44: The comment is wrong because updateServices() sorts the
items array and can move the .maya entry; to ensure Maya remains last, modify
updateServices(): remove or exclude the .maya item from the array before
performing the current sort (the block using usageCount and isInUse), perform
the sort on the remaining items, then re-append the .maya item
(items.append(.init(service: .maya, dataProvider: nil))) after sorting;
reference the items array, the .maya service enum case, and the updateServices()
sorting logic so the change targets the correct place.
In `@DashWallet/Sources/UI/Maya/EnterAddressHostingController.swift`:
- Around line 52-55: Remove the plain-text address logging in the onContinue
closure of EnterAddressHostingController: do not call DSLogger.log with the full
destination address; either remove the log entirely or only log a redacted
address (e.g. show first/last chars) and/or wrap logging behind `#if` DEBUG.
Update the block that currently calls DSLogger.log("Maya: Address confirmed for
\(self.coin.code): \(address)") so that sensitive payment data is not emitted,
and keep the onAddressConfirmed(self.coin, address) behavior unchanged.
In `@DashWallet/Sources/UI/Maya/GenericQRScannerController.swift`:
- Around line 216-217: The DSLogger.log call in GenericQRScannerController
exposes raw QR payloads (sensitive addresses/URIs); change the logging so raw
values are never emitted in release builds — either remove the
DSLogger.log("Maya QR Scanner: Scanned value: ...") call entirely or guard it
with a debug-only check (e.g., only log under DEBUG or use a
redaction/safe-logging helper) and ensure onQRCodeScanned?(stringValue) still
fires; update references to DSLogger.log and the QR scanning handler in
GenericQRScannerController accordingly.
- Around line 43-60: Replace direct AVCaptureSession start/stop calls in
viewWillAppear(_:) and viewWillDisappear(_:) and the immediate session start in
setupCamera() with DWCaptureSessionManager usage so authorization is checked
(using DWCaptureSessionManager's authorization handling) and all start/stop
operations are dispatched on its dedicated serial queue to avoid concurrent
startRunning() races; locate calls to
session.startRunning()/session.stopRunning() in viewWillAppear(_:),
viewWillDisappear(_:), and setupCamera() and route them through
DWCaptureSessionManager APIs. Also wrap any raw QR payload logging in the QR
handling code (where the scanned payload is logged) with `#if` DEBUG ... `#endif` to
avoid emitting sensitive data in release builds.
In `@DashWallet/Sources/UI/Maya/SelectCoinViewModel.swift`:
- Around line 70-88: The loop that builds picker items currently only ensures a
pool exists (using poolsByAsset[coin.mayaAsset]) but ignores
MayaPool.isAvailable, so staged/non-available pools still appear; update the
loop in the SelectCoinViewModel where you iterate
MayaCryptoCurrency.supportedCoins and before creating the CoinDisplayItem (the
items.append(...) call), check pool.isAvailable and continue/skip if false
(i.e., only create CoinDisplayItem when pool.isAvailable is true), ensuring
unusable assets are excluded from the picker.
In `@MAYA.md`:
- Around line 52-61: Update the tracker row for "Enter Destination Address"
(Requirement 2 / table row 2, Figma Node 24007-6732) to reflect this PR: set the
Branch to the current PR branch name and change Status from "Not Started" to "In
Progress" (or "Done" if applicable); ensure the table entry for the feature
title "Enter Destination Address" is edited accordingly so the
single-source-of-truth table matches the implemented enter-address screen in
this PR.
---
Outside diff comments:
In `@DashWallet/Sources/UI/Buy` Sell/BuySellPortalViewController.swift:
- Around line 244-267: The isGeoblocked() path can hang because
nextEmittedPlacemark() only resumes for a non-nil $currentPlacemark while
DWLocationManager.reverseGeocodeLocation may never publish a placemark; update
nextEmittedPlacemark() to enforce a bounded wait and a safe single-resume
pattern (e.g., use withCheckedContinuation or withCheckedThrowingContinuation
plus a resumed flag or timeout Task that resumes with a failure/default after X
seconds), ensure the AnyCancellable from
DWLocationManager.shared.$currentPlacemark is cancelled when the continuation is
resumed or timed out, and propagate/handle the timeout error back in
isGeoblocked() so the flow always returns (reference nextEmittedPlacemark(),
isGeoblocked(), DWLocationManager.shared.$currentPlacemark, and
reverseGeocodeLocation).
---
Nitpick comments:
In `@DashWallet/Sources/UI/Maya/EnterAddressView.swift`:
- Around line 71-77: SwiftLint warns about using trailing-closure syntax when
multiple closure parameters are present in the Button initializer; update the
Button call in EnterAddressView (the Button(action: { onScanQR?() }) that
currently uses a trailing closure for the label) to use the explicit label:
parameter (i.e., Button(action: { onScanQR?() }, label: { /* Image(...) */ }))
so the action and label closures are passed explicitly and the warning is
resolved.
- Around line 50-52: Replace the UIKit-based keyboard dismissal in
EnterAddressView by adding a SwiftUI `@FocusState` property (e.g., private var
isAddressFieldFocused: Bool) to the view struct, attach
.focused($isAddressFieldFocused) to the TextField(viewModel.placeholderText,
text: $viewModel.addressText) and change the .onTapGesture handler to set
isAddressFieldFocused = false instead of calling
UIApplication.shared.sendAction; remove the UIKit call so focus is managed
idiomatically by SwiftUI.
In `@DashWallet/Sources/UI/Maya/EnterAddressViewModel.swift`:
- Around line 34-36: Replace the simplistic non-empty check in isAddressValid
with coin-specific format validation: trim addressText and route it to a new
helper (e.g., validateAddress(for:coin:)) that switches on the view model's coin
identifier (use your existing coin property such as selectedCoin/coinType) and
applies lightweight rules (BTC: allowed prefixes and base58/Bech32 patterns,
ETH: startsWith "0x" and length 42, etc.); update isAddressValid to return the
result of that helper so UI gating uses coin-aware validation while keeping
trimming/empty checks.
In `@DashWallet/Sources/UI/Maya/MayaPortalView.swift`:
- Line 33: Replace the hardcoded Color(red: 0.08, green: 0.11, blue: 0.25) used
in MayaPortalView (e.g., in the view body where .fill(...) is applied) with a
named asset color; add a new color asset (e.g., "MayaBackground") to the asset
catalog and then use Color("MayaBackground") (or a small Color extension if you
prefer) in the .fill call so the background color is centralized for theming and
reuse.
In `@DashWallet/Sources/UI/Maya/SelectCoinView.swift`:
- Around line 118-130: The Button currently uses a trailing closure for its
label which triggers SwiftLint; change the Button to use the explicit label
parameter form so the action and label are both passed as named parameters (keep
the existing action that calls Task { await viewModel.loadCoins() } and the
existing label content with Text/formatting), referencing the Button declaration
and viewModel.loadCoins() call so the change is applied to that exact component.
In `@DashWallet/Sources/UI/Maya/SelectCoinViewModel.swift`:
- Around line 54-55: SelectCoinViewModel currently references global singletons
(MayaAPIService.shared and CurrencyExchanger.shared) which prevents unit
testing; add protocol abstractions (e.g., MayaAPIServiceProtocol and
CurrencyExchangerProtocol) and change SelectCoinViewModel to accept these as
init dependencies (with default parameters that wrap the current shared
singletons for backward compatibility), replace direct calls to
MayaAPIService.shared.fetchPools()/fetchInboundAddresses() and
CurrencyExchanger.shared usage with the injected protocol instances, and update
other similar usages (the other async lets around lines 115-119) to use the
injected network service; update tests to inject mock implementations of the
protocols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e4698b6-83d6-4b2c-8276-bbc4a0395542
⛔ Files ignored due to path filters (18)
DashWallet/Resources/AppAssets.xcassets/convert.crypto.imageset/convert.crypto.svgis excluded by!**/*.svgDashWallet/Resources/AppAssets.xcassets/maya.coin.arb.imageset/maya.coin.arb.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.btc.imageset/maya.coin.btc.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.dai.imageset/maya.coin.dai.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.eth.imageset/maya.coin.eth.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.gld.imageset/maya.coin.gld.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.kuji.imageset/maya.coin.kuji.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.leo.imageset/maya.coin.leo.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.link.imageset/maya.coin.link.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.pepe.imageset/maya.coin.pepe.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.rune.imageset/maya.coin.rune.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.tgt.imageset/maya.coin.tgt.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.usdc.imageset/maya.coin.usdc.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.usdt.imageset/maya.coin.usdt.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.wbtc.imageset/maya.coin.wbtc.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.coin.wsteth.imageset/maya.coin.wsteth.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/maya.logo.imageset/maya.logo.svgis excluded by!**/*.svgDashWallet/Resources/AppAssets.xcassets/portal.maya.imageset/portal.maya.svgis excluded by!**/*.svg
📒 Files selected for processing (41)
DashWallet.xcodeproj/project.pbxprojDashWallet/Resources/AppAssets.xcassets/convert.crypto.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.arb.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.btc.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.dai.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.eth.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.gld.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.kuji.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.leo.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.link.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.pepe.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.rune.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.tgt.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.usdc.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.usdt.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.wbtc.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.coin.wsteth.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/maya.logo.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/portal.maya.imageset/Contents.jsonDashWallet/Sources/Models/Maya/MayaAPIService.swiftDashWallet/Sources/Models/Maya/MayaCryptoCurrency.swiftDashWallet/Sources/Models/Maya/MayaEndpoint.swiftDashWallet/Sources/Models/Maya/MayaPool.swiftDashWallet/Sources/UI/Buy Sell/BuySellPortalView.swiftDashWallet/Sources/UI/Buy Sell/BuySellPortalViewController.swiftDashWallet/Sources/UI/Buy Sell/Model/BuySellPortalModel.swiftDashWallet/Sources/UI/Buy Sell/Model/ServiceDataProvider.swiftDashWallet/Sources/UI/Coinbase/ServiceOverview/ServiceEntryPointModel.swiftDashWallet/Sources/UI/Home/HomeViewController+Shortcuts.swiftDashWallet/Sources/UI/Home/Views/Shortcuts/ShortcutCustomizeBannerView.swiftDashWallet/Sources/UI/Maya/CoinRowView.swiftDashWallet/Sources/UI/Maya/EnterAddressHostingController.swiftDashWallet/Sources/UI/Maya/EnterAddressView.swiftDashWallet/Sources/UI/Maya/EnterAddressViewModel.swiftDashWallet/Sources/UI/Maya/GenericQRScannerController.swiftDashWallet/Sources/UI/Maya/MayaPortalView.swiftDashWallet/Sources/UI/Maya/MayaPortalViewController.swiftDashWallet/Sources/UI/Maya/SelectCoinHostingController.swiftDashWallet/Sources/UI/Maya/SelectCoinView.swiftDashWallet/Sources/UI/Maya/SelectCoinViewModel.swiftMAYA.md
- Fix Maya sort order in ServiceDataProvider to keep Maya last - Wrap sensitive address/QR logging in #if DEBUG - Check pool.isAvailable before showing coins in SelectCoinViewModel - Implement two-step clipboard reveal matching Send screen pattern - Fix clipboard paste flash by matching hosting controller background color and animating the transition - Update MAYA.md Requirement 2 status to In Progress Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
DashWallet/Sources/UI/Maya/GenericQRScannerController.swift (2)
216-219:⚠️ Potential issue | 🟠 MajorDon't emit the raw QR payload.
Even in DEBUG, the scanned string can be a wallet address or payment URI. Logging it verbatim leaks sensitive transaction data into shared test/device logs; redact it or remove the payload from the message.
🔒 Minimal fix
`#if` DEBUG - DSLogger.log("Maya QR Scanner: Scanned value: \(stringValue)") + let preview = "\(stringValue.prefix(6))...\(stringValue.suffix(4))" + DSLogger.log("🔍 Maya QR Scanner: Scanned value: \(preview)") `#endif` onQRCodeScanned?(stringValue)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/GenericQRScannerController.swift` around lines 216 - 219, The DEBUG log in GenericQRScannerController.swift emits the raw scanned QR payload (stringValue) which may contain sensitive payment data; update the DEBUG logging inside the scan handler to avoid logging stringValue directly—either remove the payload from the DSLogger.log call or replace it with a redacted/placeholder message like "Maya QR Scanner: Scanned value: <redacted>" while keeping the onQRCodeScanned?(stringValue) call intact so functionality is unchanged.
43-103:⚠️ Potential issue | 🟠 MajorRequest camera access first and serialize session start/stop.
setupCamera()already dispatchesstartRunning(), andviewWillAppear(_:)can queue a secondstartRunning()before the first flipsisRunning. Users with denied/restricted camera access also just land on the scanner chrome with no explicit recovery path. Gate setup on.videoauthorization and move everystartRunning()/stopRunning()call onto a dedicated serial queue.🔐 One way to tighten this up
+ private let sessionQueue = DispatchQueue(label: "org.dashwallet.maya.qr-session") + override func viewWillAppear(_ animated: Bool) { super.viewWillAppear(animated) - - if let session = captureSession, !session.isRunning { - DispatchQueue.global(qos: .userInitiated).async { - session.startRunning() - } - } + guard AVCaptureDevice.authorizationStatus(for: .video) == .authorized, + let session = captureSession, + !session.isRunning else { return } + sessionQueue.async { session.startRunning() } } override func viewWillDisappear(_ animated: Bool) { super.viewWillDisappear(animated) if let session = captureSession, session.isRunning { - DispatchQueue.global(qos: .userInitiated).async { - session.stopRunning() - } + sessionQueue.async { session.stopRunning() } } }Then branch
setupCamera()throughAVCaptureDevice.authorizationStatus(for: .video)/requestAccess(for: .video)before creating the session or preview.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/GenericQRScannerController.swift` around lines 43 - 103, The scanner currently may call startRunning/stopRunning concurrently and proceeds without requesting camera permission; update setupCamera(), viewWillAppear(_:), and viewWillDisappear(_:) to first check AVCaptureDevice.authorizationStatus(for: .video) and call requestAccess(for: .video) when needed, only creating the AVCaptureSession and previewLayer after authorization is granted (show a clear camera-denied alert with recovery actions when denied/restricted), and move all session.startRunning() and session.stopRunning() calls onto a dedicated serial DispatchQueue (e.g., a private DispatchQueue used by this controller) so start/stop are serialized and you avoid racing on captureSession.isRunning; ensure captureSession and previewLayer are only set after successful setup and that viewWillAppear/viewWillDisappear schedule start/stop on that serial queue rather than directly on global queues.DashWallet/Sources/UI/Maya/EnterAddressHostingController.swift (1)
52-57:⚠️ Potential issue | 🟠 MajorRedact the confirmed address before logging.
#if DEBUGnarrows the audience, but this still writes the full destination address into device/QA logs on every successful continue. Please either drop the payload or log only a short preview.🔒 Minimal fix
onContinue: { [weak self] address in guard let self else { return } `#if` DEBUG - DSLogger.log("Maya: Address confirmed for \(self.coin.code): \(address)") + let preview = "\(address.prefix(6))...\(address.suffix(4))" + DSLogger.log("🎯 Maya: Address confirmed for \(self.coin.code): \(preview)") `#endif` self.onAddressConfirmed?(self.coin, address) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/EnterAddressHostingController.swift` around lines 52 - 57, The debug log currently prints the full destination address inside the onContinue closure of EnterAddressHostingController; change the DSLogger.log call so it does not include the full address—either remove the address payload or log a redacted preview (e.g., first 4 and last 4 chars or a fixed-length mask) while retaining the coin code; update the DSLogger.log invocation in the onContinue closure accordingly and leave self.onAddressConfirmed?(self.coin, address) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@DashWallet/Sources/UI/Maya/EnterAddressView.swift`:
- Around line 78-84: The QR scan Button (the one calling onScanQR?() and showing
Image("scan-qr.accessory.icon")) needs an explicit accessibility label and a
larger hit target; update the Button to include .accessibilityLabel("Scan QR
code") (or similar localized string) and enlarge its tappable area to at least
44×44 by adding padding or using .frame(minWidth: 44, minHeight: 44) together
with .contentShape(Rectangle()) so the image remains visually 20×20 but the hit
area meets accessibility guidelines.
In `@DashWallet/Sources/UI/Maya/EnterAddressViewModel.swift`:
- Around line 49-71: Update clipboard handling and normalize pasted addresses:
change checkClipboard() to detect URL items as well (use
UIPasteboard.general.hasStrings || UIPasteboard.general.hasURLs) and update
revealClipboard() to prefer UIPasteboard.general.url?.absoluteString when
available; create a single helper (e.g., normalizeAddress(_:),
extractAddressFromURI(_:), or similar) that mirrors
PiggyCardsRepository.parsePaymentURI() behavior to strip wallet URI schemes and
query params (handle scheme:address?params like bitcoin:addr?amount= or
ethereum:addr?...), then call that normalizer from pasteFromClipboard() and
setAddress(_:) so revealedClipboardContent and addressText store only the
extracted/normalized address instead of raw URIs.
---
Duplicate comments:
In `@DashWallet/Sources/UI/Maya/EnterAddressHostingController.swift`:
- Around line 52-57: The debug log currently prints the full destination address
inside the onContinue closure of EnterAddressHostingController; change the
DSLogger.log call so it does not include the full address—either remove the
address payload or log a redacted preview (e.g., first 4 and last 4 chars or a
fixed-length mask) while retaining the coin code; update the DSLogger.log
invocation in the onContinue closure accordingly and leave
self.onAddressConfirmed?(self.coin, address) unchanged.
In `@DashWallet/Sources/UI/Maya/GenericQRScannerController.swift`:
- Around line 216-219: The DEBUG log in GenericQRScannerController.swift emits
the raw scanned QR payload (stringValue) which may contain sensitive payment
data; update the DEBUG logging inside the scan handler to avoid logging
stringValue directly—either remove the payload from the DSLogger.log call or
replace it with a redacted/placeholder message like "Maya QR Scanner: Scanned
value: <redacted>" while keeping the onQRCodeScanned?(stringValue) call intact
so functionality is unchanged.
- Around line 43-103: The scanner currently may call startRunning/stopRunning
concurrently and proceeds without requesting camera permission; update
setupCamera(), viewWillAppear(_:), and viewWillDisappear(_:) to first check
AVCaptureDevice.authorizationStatus(for: .video) and call requestAccess(for:
.video) when needed, only creating the AVCaptureSession and previewLayer after
authorization is granted (show a clear camera-denied alert with recovery actions
when denied/restricted), and move all session.startRunning() and
session.stopRunning() calls onto a dedicated serial DispatchQueue (e.g., a
private DispatchQueue used by this controller) so start/stop are serialized and
you avoid racing on captureSession.isRunning; ensure captureSession and
previewLayer are only set after successful setup and that
viewWillAppear/viewWillDisappear schedule start/stop on that serial queue rather
than directly on global queues.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8012e80-f04e-4b26-84fe-c1e1ae515f06
📒 Files selected for processing (8)
DashWallet/Sources/UI/Buy Sell/Model/ServiceDataProvider.swiftDashWallet/Sources/UI/Maya/EnterAddressHostingController.swiftDashWallet/Sources/UI/Maya/EnterAddressView.swiftDashWallet/Sources/UI/Maya/EnterAddressViewModel.swiftDashWallet/Sources/UI/Maya/GenericQRScannerController.swiftDashWallet/Sources/UI/Maya/MayaPortalViewController.swiftDashWallet/Sources/UI/Maya/SelectCoinViewModel.swiftMAYA.md
🚧 Files skipped from review as they are similar to previous changes (1)
- DashWallet/Sources/UI/Buy Sell/Model/ServiceDataProvider.swift
…to URI normalization Refactor GenericQRScannerController from 221-line UIKit to thin wrapper hosting new SwiftUI GenericQRScannerView + QRCaptureView UIViewRepresentable. Add 44x44pt hit target and accessibility label to QR scan button. Detect clipboard URLs and normalize crypto URIs (bitcoin:, ethereum:, etc.) by stripping scheme, chain ID, and query params in paste/scan paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GenericQRScannerController) since the existing Dash-only scanner rejects non-Dash addressesNew Files
EnterAddressViewModel.swift— ViewModel with address text, clipboard reading, validationEnterAddressView.swift— SwiftUI screen with address field, QR button, clipboard paste section, Continue buttonEnterAddressHostingController.swift— Thin UIKit wrapper with QR scanner presentationGenericQRScannerController.swift— Reusable QR scanner returning raw strings (AVCaptureSession-based)Modified Files
MayaPortalViewController.swift— AddednavigateToEnterAddress(for:)wiring coin selection to address entryproject.pbxproj— Added new files to both dashwallet and dashpay targetsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit