feat: validate destination address for Maya swaps (MO-32)#761
Merged
Conversation
…r Maya swaps Add exchange address sources to the Maya Enter Address screen so users can retrieve deposit addresses from their Uphold and Coinbase accounts. - Add AddressSourceView, MayaExchangeAddressProvider, and address source state management to EnterAddressViewModel - Uphold: fetch cards, create network-specific crypto addresses via API, with session caching and proper network key lookup - Coinbase: direct account lookup by currency code with account-list fallback, address creation via POST, session caching - Add login flows for both exchanges from the Enter Address screen - Re-check authorization after failed fetches to show "Log In" instead of "Not available" when sessions expire Fix Coinbase OAuth which migrated to new endpoints: - Auth URL: login.coinbase.com/oauth2/auth (was coinbase.com/oauth/authorize) - Token URL: login.coinbase.com/oauth2/token (was api.coinbase.com/oauth/token) - Redirect URI: dashwallet://brokers/coinbase/connect (was authhub://oauth-callback) - Token exchange encoding: application/x-www-form-urlencoded (was JSON) - Remove deprecated account and meta[send_limit_*] OAuth parameters Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add chain-specific address validation with two levels: 1. Local regex validation (real-time) - validates format as user types: - BTC: Legacy Base58 (1.../3...) and Bech32 SegWit/Taproot (bc1q.../bc1p...) - ETH/ARB: 0x + 40 hex characters - KUJI: kujira1 + 38 bech32 characters - THOR: thor1 + 38 bech32 characters 2. Maya API validation (on Continue) - calls /mayachain/quote/swap to verify checksum and chain-level validity. Shows API error message on failure, or temp SUCCESS dialog on success. UI updates to match Figma MO-32 design: - Red border and light red background on invalid address field - Error message displayed below the input field - Continue button disabled for format-invalid addresses Co-Authored-By: Claude Opus 4.6 (1M context) <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:
✨ 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: 5
🧹 Nitpick comments (5)
DashWalletTests/MayaAddressValidatorTests.swift (1)
20-21: Sort imports alphabetically.SwiftLint flags that imports should be sorted. The
@testable import dashwalletshould come beforeimport XCTestalphabetically.Proposed fix
-import XCTest `@testable` import dashwallet +import XCTest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWalletTests/MayaAddressValidatorTests.swift` around lines 20 - 21, Sort the import statements so they are alphabetical: move "@testable import dashwallet" before "import XCTest" to satisfy SwiftLint's import ordering rule; update the import block in MayaAddressValidatorTests.swift accordingly.DashWallet/Sources/Models/Maya/MayaEndpoint.swift (1)
54-69: Clean up pattern matching by extractingletkeyword.SwiftLint recommends combining multiple pattern matching bindings by moving the
letkeyword out of the tuple.Proposed fix
var task: Moya.Task { switch self { - case .quoteSwap(let fromAsset, let toAsset, let amount, let destination): + case let .quoteSwap(fromAsset, toAsset, amount, destination): return .requestParameters( parameters: [ "from_asset": fromAsset, "to_asset": toAsset, "amount": amount, "destination": destination, ], encoding: URLEncoding.queryString ) default: return .requestPlain } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/Models/Maya/MayaEndpoint.swift` around lines 54 - 69, In the var task switch, clean up pattern matching by moving the let binding outside the tuple: replace the current case .quoteSwap(let fromAsset, let toAsset, let amount, let destination) with a single let on the case (e.g., case let .quoteSwap(fromAsset, toAsset, amount, destination)) so the switch in MayaEndpoint.swift (specifically the var task implementation and the .quoteSwap pattern) uses a combined let binding as recommended by SwiftLint.DashWallet/Sources/UI/Maya/AddressSourceView.swift (1)
56-159: Well-structured SwiftUI component.The view correctly handles all state cases with appropriate disabled states and loading indicators. Minor SwiftLint style notes:
@ViewBuilderattributes on lines 115 and 129 should be on their own lines per project conventions, and a trailing newline is missing at line 160.,
🧹 Optional: Fix SwiftLint attribute placement
- `@ViewBuilder` - private var icon: some View { + `@ViewBuilder` + private var icon: some View {And add a trailing newline at the end of the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/UI/Maya/AddressSourceView.swift` around lines 56 - 159, Move the `@ViewBuilder` attribute declarations onto their own lines for the computed view properties `private var icon: some View` and `private var subtitle: some View` (i.e., place `@ViewBuilder` on a separate line immediately above the property declarations) to satisfy SwiftLint style, and ensure the file ends with a single trailing newline; update the declarations for `icon` and `subtitle` and add the missing newline at EOF.DashWallet/Sources/Models/Maya/MayaExchangeAddressProvider.swift (1)
352-357:firstAddressreturns arbitrary dictionary value.Swift dictionary iteration order is not guaranteed.
address?.values.firstmay return different values across runs. Since the code now uses network-specific lookups (line 85), this property appears unused. Consider removing it or documenting that it's for fallback/debugging only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/Models/Maya/MayaExchangeAddressProvider.swift` around lines 352 - 357, The computed property firstAddress in MayaExchangeAddressProvider currently returns an arbitrary dictionary value via address?.values.first which is non-deterministic; either remove firstAddress (and any callers) to avoid flaky behavior or explicitly document it as a non-deterministic fallback/debug-only helper, and if kept, make it deterministic by selecting a specific network key (e.g., .mainnet or .testnet) or sorting keys first; update or remove references to firstAddress accordingly and ensure any production lookups use the network-specific access already implemented elsewhere (see the address property and network-specific lookup at line ~85).DashWallet/Sources/Models/Coinbase/Infrastructure/API/CoinbaseAPIEndpoint.swift (1)
166-176: OAuth endpoints are correctly configured.The OAuth2 base URL
https://login.coinbase.comand paths (/oauth2/token,/oauth2/revoke,/oauth2/auth) match current Coinbase OAuth2 documentation. TheURLEncoding.httpBodyencoding is appropriate for OAuth2 token requests.Regarding the force unwrapping on hardcoded URLs (lines 169, 172): while compile-time constant URLs are low-risk, consider adding an explicit SwiftLint suppression (
// swiftlint:disable force_unwrapping) to document this is intentional, or refactor to a safer pattern for consistency with the codebase's optional handling practices.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DashWallet/Sources/Models/Coinbase/Infrastructure/API/CoinbaseAPIEndpoint.swift` around lines 166 - 176, The baseURL computed property currently force-unwraps hardcoded OAuth URLs in the cases for .getToken, .refreshToken, .revokeToken and the .path case; either add an explicit SwiftLint suppression comment (e.g., a single-line // swiftlint:disable force_unwrapping placed immediately above the URL(...)! usages) to document the intentional force-unwrap, or refactor the property to safely construct URLs (for example, use guard/if-let and fallback to kBaseURL) inside the baseURL computed var so the .getToken/.refreshToken/.revokeToken and .path branches no longer use force-unwrapping.
🤖 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/Models/Coinbase/Accounts/AccountRepository.swift`:
- Around line 88-110: The returned accounts from allIncludingEmpty() can expose
CoinbaseUserAccountData.balanceFormatted which currently force-unwraps
balance.amount.decimal() and the formatter result; update the balanceFormatted
computed property in CoinbaseUserAccountData to defensively unwrap and handle
failures (use guard/if-let to safely get balance.amount.decimal() and the
NumberFormatter result and return a safe fallback like "0" or the raw decimal
string instead of force-unwrapping), mirroring the safer pattern used in
fiatBalanceFormatted so accessing balanceFormatted on accounts from
allIncludingEmpty() cannot crash.
In `@DashWallet/Sources/Models/Maya/MayaAPIService.swift`:
- Around line 46-56: The catch block in validateAddress currently swallows
network/unknown errors and returns nil (treated as valid); change it so network
failures return a generic error string instead of nil: inside the catch for
validateAddress, after attempting to decode MayaSwapQuote from
HTTPClientError.statusCode, log the full error via DSLogger and return a generic
validation error message (e.g. "Address validation unavailable — network error")
so callers can distinguish "unknown/error" from a valid address; alternatively,
if you prefer stronger typing, change validateAddress's return to a Result/enum
(e.g. .valid/.invalid(error)/.unknown) and update callers accordingly.
In `@DashWallet/Sources/Models/Maya/MayaExchangeAddressProvider.swift`:
- Around line 176-185: In createUpholdCard the URLSession.data call ignores the
HTTPURLResponse status; update the method to capture the response (let (data,
response) = try await URLSession.shared.data(for: request)), cast to
HTTPURLResponse, and validate that response.statusCode is in the 200...299 range
before attempting JSONDecoder().decode(UpholdCard.self, from: data); if the
status is not successful, log the status and response body (or error) and return
nil — mirror the status-checking behavior used in fetchUpholdCards to ensure
non-2xx responses are handled safely.
In `@DashWallet/Sources/UI/Maya/EnterAddressHostingController.swift`:
- Around line 171-174: The presentationAnchor(for:) implementation in
EnterAddressHostingController returns a new empty ASPresentationAnchor() when
view.window is nil, which can cause unexpected behavior; update
presentationAnchor(for:) to robustly obtain a valid anchor (for example: prefer
view.window, then the app's key window or a window from connected scenes) and
handle the nil case explicitly (e.g., log an error or assertion and return the
best-available window) so ASWebAuthenticationSession always receives a real
anchor; reference the EnterAddressHostingController type and its
presentationAnchor(for:) method when making this change.
- Around line 133-153: The ASWebAuthenticationSession callback in
presentUpholdLogin uses an unused `error` parameter and relies on a fragile
string contains check; change the callback signature to use `_` for the unused
error, and replace the `callbackURL.absoluteString.contains("uphold")` check
with an explicit URL component match (e.g., verify callbackURL.scheme and/or
callbackURL.host against the expected redirect scheme/host for your
DWUpholdClient flow) before calling
DWUpholdClient.sharedInstance().completeAuthRoutine(with:); ensure you still
guard for a non-nil callbackURL and handle failure paths accordingly.
---
Nitpick comments:
In
`@DashWallet/Sources/Models/Coinbase/Infrastructure/API/CoinbaseAPIEndpoint.swift`:
- Around line 166-176: The baseURL computed property currently force-unwraps
hardcoded OAuth URLs in the cases for .getToken, .refreshToken, .revokeToken and
the .path case; either add an explicit SwiftLint suppression comment (e.g., a
single-line // swiftlint:disable force_unwrapping placed immediately above the
URL(...)! usages) to document the intentional force-unwrap, or refactor the
property to safely construct URLs (for example, use guard/if-let and fallback to
kBaseURL) inside the baseURL computed var so the
.getToken/.refreshToken/.revokeToken and .path branches no longer use
force-unwrapping.
In `@DashWallet/Sources/Models/Maya/MayaEndpoint.swift`:
- Around line 54-69: In the var task switch, clean up pattern matching by moving
the let binding outside the tuple: replace the current case .quoteSwap(let
fromAsset, let toAsset, let amount, let destination) with a single let on the
case (e.g., case let .quoteSwap(fromAsset, toAsset, amount, destination)) so the
switch in MayaEndpoint.swift (specifically the var task implementation and the
.quoteSwap pattern) uses a combined let binding as recommended by SwiftLint.
In `@DashWallet/Sources/Models/Maya/MayaExchangeAddressProvider.swift`:
- Around line 352-357: The computed property firstAddress in
MayaExchangeAddressProvider currently returns an arbitrary dictionary value via
address?.values.first which is non-deterministic; either remove firstAddress
(and any callers) to avoid flaky behavior or explicitly document it as a
non-deterministic fallback/debug-only helper, and if kept, make it deterministic
by selecting a specific network key (e.g., .mainnet or .testnet) or sorting keys
first; update or remove references to firstAddress accordingly and ensure any
production lookups use the network-specific access already implemented elsewhere
(see the address property and network-specific lookup at line ~85).
In `@DashWallet/Sources/UI/Maya/AddressSourceView.swift`:
- Around line 56-159: Move the `@ViewBuilder` attribute declarations onto their
own lines for the computed view properties `private var icon: some View` and
`private var subtitle: some View` (i.e., place `@ViewBuilder` on a separate line
immediately above the property declarations) to satisfy SwiftLint style, and
ensure the file ends with a single trailing newline; update the declarations for
`icon` and `subtitle` and add the missing newline at EOF.
In `@DashWalletTests/MayaAddressValidatorTests.swift`:
- Around line 20-21: Sort the import statements so they are alphabetical: move
"@testable import dashwallet" before "import XCTest" to satisfy SwiftLint's
import ordering rule; update the import block in MayaAddressValidatorTests.swift
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e27559c-0f60-42c6-b998-46e39b745d9f
⛔ Files ignored due to path filters (2)
DashWallet/Resources/AppAssets.xcassets/Maya/maya.coinbase.logo.imageset/coinbase-logo.svgis excluded by!**/*.svgDashWallet/Resources/AppAssets.xcassets/Maya/maya.uphold.logo.imageset/uphold-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (18)
DashWallet.xcodeproj/project.pbxprojDashWallet/Resources/AppAssets.xcassets/Maya/maya.coinbase.logo.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/Maya/maya.uphold.logo.imageset/Contents.jsonDashWallet/Sources/Models/Coinbase/Accounts/AccountRepository.swiftDashWallet/Sources/Models/Coinbase/Accounts/AccountService.swiftDashWallet/Sources/Models/Coinbase/Auth/CBAuth.swiftDashWallet/Sources/Models/Coinbase/Coinbase+Constants.swiftDashWallet/Sources/Models/Coinbase/Coinbase.swiftDashWallet/Sources/Models/Coinbase/Infrastructure/API/CoinbaseAPIEndpoint.swiftDashWallet/Sources/Models/Maya/MayaAPIService.swiftDashWallet/Sources/Models/Maya/MayaAddressValidator.swiftDashWallet/Sources/Models/Maya/MayaEndpoint.swiftDashWallet/Sources/Models/Maya/MayaExchangeAddressProvider.swiftDashWallet/Sources/UI/Maya/AddressSourceView.swiftDashWallet/Sources/UI/Maya/EnterAddressHostingController.swiftDashWallet/Sources/UI/Maya/EnterAddressView.swiftDashWallet/Sources/UI/Maya/EnterAddressViewModel.swiftDashWalletTests/MayaAddressValidatorTests.swift
- Remove force unwraps in CoinbaseUserAccountData.balanceFormatted, using guard/let with raw string fallback matching fiatBalanceFormatted - Return user-facing error on network failure in validateAddress instead of nil (which was incorrectly treated as valid) - Check HTTP status code in createUpholdCard before decoding response Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HashEngineering
approved these changes
Mar 24, 2026
HashEngineering
left a comment
HashEngineering
left a comment
Contributor
There was a problem hiding this comment.
LGTM
In a future PR, we need to add new text items for the Maya integration to the English source localization file so that the new text can be translated in Transfix.
…efore deallocation AVCaptureMetadataOutput holds an unretained reference to its delegate. When the scanner dismisses, stopSession() stops the capture session asynchronously, but the Coordinator can be deallocated before the session fully stops. If a QR code is detected in that window, the callback fires on a dangling pointer, causing EXC_BAD_ACCESS. Fix: nil out the metadata output delegate in both stopSession() and deinit before the coordinator can be deallocated. Also remove the unnecessary DispatchQueue.main.async hop in the delegate callback (already dispatched on main), and replace force cast with guard. Co-Authored-By: Claude Opus 4.6 (1M context) <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
/mayachain/quote/swapendpoint to verify checksum and chain-level validityMayaAddressValidatorTests)Changes
MayaAddressValidator.swiftMayaAddressValidatorTests.swiftEnterAddressViewModel.swiftisAddressValidnow calls validator; addedshowAddressErrorand error messageEnterAddressView.swiftEnterAddressHostingController.swiftMayaEndpoint.swiftquoteSwapendpointMayaAPIService.swiftvalidateAddress()andMayaSwapQuotemodelTest plan
12mVndRNjYZmTVr37ixEThhyS6MfKXB(valid format, bad checksum) → Continue enabled, press Continue → API error shown🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests