feat(wallet): prepare 8.6.0 release#777
Open
romchornyi wants to merge 61 commits into
Open
Conversation
…sion to 8.6.0 - Replace storyboard-based Buy & Sell screen with SwiftUI view matching Figma designs - Add Maya as a new service in Buy & Sell portal with dedicated Maya portal screen - Group services into cards: Uphold+Coinbase, Topper with "Powered by Uphold" badge, Maya - Fix navigation: back chevron works for both modal dismiss and push pop - Present Buy & Sell portal full screen instead of sheet overlay - Add shortcut bar customization with banner and selection sheet - Add new shortcut icon SVG assets (ATM, Coinbase, CrowdNode, Topper, Uphold, etc.) - Bump version to 8.6.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace invalid MAYA PBX object IDs with valid 24-char hex - Fix integer division in Color initializer (176/255 → 176.0/255.0) - Change infoButton from IUO to regular optional - Add geoblock check to Coinbase shortcut entry point - Add accessibility label and 44x44 hit target to banner dismiss button - Add comment explaining why MayaPortalViewController wrapper is needed (UIHostingController hides UIKit nav bar when pushed directly) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add operator spacing around division in BuySellPortalView and replace force-unwraps with safe optional chaining in BuySellPortalViewController. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat: Maya entry points, Buy & Sell portal redesign, v8.6.0
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implement coin selection screen for Maya Protocol integration. Users can browse and search Maya-supported cryptocurrencies with live fiat prices fetched from Midgard API, with halted chain indicators. - Add MayaCryptoCurrency model with 19 hardcoded coins - Add MayaEndpoint (Moya) + MayaAPIService for pools and inbound addresses - Add SelectCoinView with search, loading, error, and halted toast states - Add CoinRowView with icon, name, ticker, and fiat price - Wire "Convert Dash" button in MayaPortalView to push coin selection - Bundle 15 coin icon PNGs from crypto-icons inventory - Add all files to both dashwallet and dashpay Xcode targets Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…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>
- Replace force-unwrapped URLs in MayaEndpoint with guard let + preconditionFailure - Pin Maya to end of service list after sorting in ServiceDataProvider - Use explicit label: parameter on Button to satisfy SwiftLint trailing closure rule - Remove halted coin UI/logic (Requirement 8 not yet implemented): remove inbound_addresses API call, halted toast, disabled/greyed coin states - Update MAYA.md: mark Req 1 as Implemented, update Req 2 status, fix coin count Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat: Maya select destination coin (Requirement 1)
- 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>
…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>
feat: Maya Enter Destination Address (Requirement 2)
…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>
feat: add Uphold/Coinbase address sources for Maya and fix Coinbase OAuth
- 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>
…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>
feat: validate destination address for Maya swaps (MO-32)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…address selection
- MayaTransactionSuccessView: success state with icon, title, message - MayaTransactionFailureView: failure state with retry, cancel, and support actions - MayaFeeInfoSheet: bottom-sheet component explaining the Maya swap fee with a Learn more link and Got it dismiss button - All three views registered in both dashwallet and dashpay Xcode targets
…version-order-preview
Why: - keep users on Order Preview when a previous Maya swap is still confirming - align Podfile.lock with the locally installed pod state used on this branch What: - add a dedicated pending-swap DashSpend error and surface it as a system alert in Order Preview - prevent the previous-swap-pending case from navigating to the Maya failure screen - refine AddressSourceView subtitle rendering and sync Podfile.lock to Protobuf 3.29.5 Validation: - attempted xcodebuild for dashwallet - build is currently blocked by CocoaPods/TodayExtension environment issues outside this code path
…lity fix(wallet): hide unavailable account entry points
chore(ui): update PiggyCards translations
feat(ui): polish shortcut sheet and migrate shortcut bar UI
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Maya conversion services and SwiftUI screens, updates Coinbase OAuth and account retrieval, hardens send and swap transaction handling, refreshes Buy/Sell and shortcuts entry points, gates some CrowdNode surfaces, and adds or remaps many asset catalog image manifests. ChangesMaya, Coinbase, and UI integration
Sequence Diagram(s)sequenceDiagram
participant User
participant MayaConvertView
participant MayaConvertViewModel
participant MayaAPIService
participant MayaAPI
User->>MayaConvertView: Enter amount or change currency
MayaConvertView->>MayaConvertViewModel: Update input state
MayaConvertViewModel->>MayaAPIService: fetchPools()
MayaAPIService->>MayaAPI: GET pools
MayaAPI-->>MayaAPIService: pool data
MayaAPIService-->>MayaConvertViewModel: rates
MayaConvertViewModel->>MayaAPIService: fetchQuote(dashSatoshis, toAsset, destination)
MayaAPIService->>MayaAPI: GET quote
MayaAPI-->>MayaAPIService: quote or error
MayaAPIService-->>MayaConvertViewModel: parsed quote/error
MayaConvertViewModel-->>MayaConvertView: receive amount / error / continue state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
DashWallet/Resources/AppAssets.xcassets/maya.coin.arb.imageset/Contents.json (1)
13-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
preserves-vector-representationfrom all Maya coin PNG assets.All seven
maya.coin.*asset catalogs setpreserves-vector-representation: truebut reference PNG files rather than SVG. This property is only meaningful for vector formats and creates misleading configuration. Remove it from all Maya cryptocurrency icon manifests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Resources/AppAssets.xcassets/maya.coin.arb.imageset/Contents.json` at line 13, Remove the "preserves-vector-representation" key from the Contents.json manifests for all maya.coin.* image sets (e.g., maya.coin.arb.imageset) because these assets reference PNGs, not vectors; open each maya.coin.* image set's Contents.json and delete the "preserves-vector-representation" entry so the manifest only describes raster images.Source: Coding guidelines
DashWallet/Sources/UI/Buy Sell/BuySellPortalViewController.swift (1)
76-99:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid force unwrapping
navigationController.Line 82 force-unwraps
navigationController, which could crash if the controller is not in a navigation stack (e.g., if presented modally in an unexpected way). Use safe unwrapping instead.🛡️ Proposed fix
let vc = IntegrationViewController.controller(model: CoinbaseEntryPointModel()) vc.userSignedOutBlock = { [weak self] isNeedToShowSignOutError in guard let self else { return } - self.navigationController!.popToViewController(self, animated: true) + guard let navigationController = self.navigationController else { return } + navigationController.popToViewController(self, animated: true) if isNeedToShowSignOutError {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Buy` Sell/BuySellPortalViewController.swift around lines 76 - 99, The navigateToCoinbase method currently force-unwraps navigationController in the vc.userSignedOutBlock which can crash; update the block to safely unwrap the navigation controller (e.g., guard let nav = navigationController else { return }) and use that local nav to call popToViewController and pushViewController (also ensure other uses of navigationController? in this method use safe optional chaining or the same guard before pushing ServiceOverviewViewController) so no force unwrapping occurs; update references inside vc.userSignedOutBlock and the push logic to use the safely unwrapped navigation controller.Source: Linters/SAST tools
🧹 Nitpick comments (15)
DashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-plus.imageset/Contents.json (1)
23-25: ⚡ Quick winVerify removal of
template-rendering-intentis intentional.The previous version included
template-rendering-intentin the properties, which has been removed. Navigation bar icons are typically generic UI elements that should inherit the system tint color viatemplate-rendering-intent: "template". Without this property, the icon may not tint correctly with the app's theme.Recommended addition
"properties" : { - "localizable" : true + "localizable" : true, + "template-rendering-intent" : "template" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Resources/AppAssets.xcassets/Navigation` bar/navigationbar-plus.imageset/Contents.json around lines 23 - 25, The Contents.json for the navigationbar-plus imageset removed the "template-rendering-intent" property which prevents the navigation icon from being tinted by the system; restore "template-rendering-intent": "template" inside the "properties" object of the navigationbar-plus.imageset Contents.json (alongside "localizable": true) so the asset uses template rendering and inherits tint colors.Source: Learnings
DashWallet/Resources/AppAssets.xcassets/info-icon.imageset/Contents.json (1)
1-23: ⚡ Quick winConsider adding template-rendering-intent for tinting.
The
info-iconis a generic UI icon that would benefit from system tinting. Add apropertiessection withtemplate-rendering-intent: "template"to allow the icon to inherit the system tint color.♻️ Suggested properties addition
], "info" : { "author" : "xcode", "version" : 1 + }, + "properties" : { + "template-rendering-intent" : "template" } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Resources/AppAssets.xcassets/info-icon.imageset/Contents.json` around lines 1 - 23, The asset catalog entry for "info-icon" needs a properties block to enable system tinting; open the info-icon.imageset Contents.json and add a top-level "properties" object containing "template-rendering-intent": "template" (so the asset will render as a template image and inherit tint), leaving the existing "images" and "info" entries intact.Source: Learnings
DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)
163-164: ⚡ Quick winGate swap diagnostics behind
#if DEBUGand apply emoji log prefixes.These logs run on production send paths and currently skip the repository’s debug/log-format conventions.
As per coding guidelines, “Wrap debug print statements in
#ifDEBUG blocks…” and “Use emoji debug markers for easy log filtering…”.Also applies to: 236-242, 273-278, 283-293, 302-306
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/Models/Transactions/SendCoinsService.swift` around lines 163 - 164, Wrap the diagnostic DSLogger.log calls in SendCoinsService (e.g., the memo/byteCount logs and the other logs referenced) inside `#if` DEBUG ... `#endif` blocks and prepend an emoji prefix (like "🔧" or "🧪") to each log message to follow repository conventions; locate usages of DSLogger.log in methods such as sendMayaSwap and surrounding blocks (the calls around memoByteCount and the later DSLogger.log calls) and change them to conditional debug-only logging with the emoji prefix while leaving production behavior unchanged.Source: Coding guidelines
DashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swift (1)
40-50: ⚡ Quick winCache NumberFormatter to avoid repeated allocations.
Creating a new
NumberFormatteron each access of thesymbolcomputed property is inefficient. SinceNumberFormatterinitialization is relatively expensive and this property is accessed during view rendering (line 90), consider using a static cached formatter or moving the formatter creation outside the computed property.⚡ Suggested optimization
+private let currencySymbolFormatter: NumberFormatter = { + let formatter = NumberFormatter() + formatter.numberStyle = .currency + return formatter +}() + enum CurrencyOption: Hashable { case fiat(String) case dash case coin(String) // ... existing code ... var symbol: String? { switch self { case .fiat(let code): - let formatter = NumberFormatter() - formatter.numberStyle = .currency - formatter.currencyCode = code - return formatter.currencySymbol + currencySymbolFormatter.currencyCode = code + return currencySymbolFormatter.currencySymbol case .dash: return nil case .coin(let code): return code } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swift` around lines 40 - 50, The symbol computed property creates a new NumberFormatter on every access (in var symbol for case .fiat(let code)); cache a formatter instead to avoid repeated allocations by adding a static cached NumberFormatter (or a static [String: NumberFormatter] keyed by currency code) in the same type/extension and reuse it inside symbol: set the cached formatter's currencyCode = code (or fetch the cached per-code formatter) and return its currencySymbol for .fiat, leaving .dash and .coin unchanged; update references to use the cached formatter to improve performance during view rendering.DashWallet/Sources/UI/Maya/Convert/MayaConvertHostingController.swift (1)
34-36: 💤 Low valueMark unimplemented initializer as unavailable.
SwiftLint suggests marking the
init(coder:)initializer as unavailable to make the intent explicit and prevent accidental use.📝 Suggested annotation
- required init?(coder: NSCoder) { + `@available`(*, unavailable) + required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertHostingController.swift` around lines 34 - 36, Mark the unimplemented initializer as explicitly unavailable: annotate the required init?(coder: NSCoder) in MayaConvertHostingController with `@available`(*, unavailable) so it cannot be used accidentally (keep the signature required init?(coder: NSCoder) and the existing fatalError or empty body). This makes the intent explicit to SwiftLint and prevents instantiation via storyboard/NIB.Source: Linters/SAST tools
DashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swift (1)
66-67: 💤 Low valueRemove commented code.
The commented lines
.scaleToFitWidth()and.layoutPriority(1)should be removed if they are no longer needed, as they add unnecessary clutter to the codebase.🧹 Cleanup suggestion
amountView .foregroundStyle(Color.primaryText) -// .scaleToFitWidth() -// .layoutPriority(1) if showCurrencyButton {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swift` around lines 66 - 67, Remove the two commented-out legacy layout modifiers (.scaleToFitWidth() and .layoutPriority(1)) from the MayaAmountView SwiftUI view in MayaAmountView.swift; locate the commented lines within the view body (e.g., inside the struct or its body property) and delete them to clean up the file and remove unnecessary clutter.DashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift (2)
316-321: 💤 Low valueConsider providing user feedback on network failures.
Network errors are silently swallowed here, setting
errorMessage = nil. While API-level errors (line 311-312) do get shown to the user, network failures leave users with no feedback about why the quote didn't appear.Consider either showing a generic "Unable to fetch quote, please try again" message, or at minimum adding debug logging per the coding guidelines.
🛠️ Suggested improvement
} catch { guard quoteRequestID == snapshot.id else { return } latestQuote = nil - errorMessage = nil + `#if` DEBUG + DSLogger.log("🌐 Quote fetch failed: \(error.localizedDescription)") + `#endif` + errorMessage = NSLocalizedString("Unable to get a quote", comment: "Maya") receiveAmount = nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift` around lines 316 - 321, The catch block in MayaConvertViewModel (inside the async quote fetch where quoteRequestID and snapshot.id are compared) currently swallows all network errors by clearing latestQuote/errorMessage/receiveAmount; update it to set a user-facing generic errorMessage (e.g., "Unable to fetch quote, please try again") when quoteRequestID == snapshot.id, keep latestQuote/receiveAmount cleared as before, and add a debug log of the caught error (using the project's logging utility) so network failures are visible in logs for debugging.Source: Coding guidelines
395-406: 💤 Low value
trimTrailingZeroscan return empty string for input"0".If
trimTrailingZeros("0")is called, it removes the trailing"0"leaving an empty string. Currently this isn't reachable becausereceiveAmountalways formats with decimals, but the function could silently break if used elsewhere.Consider adding a safeguard:
🛠️ Suggested safeguard
static func trimTrailingZeros(_ s: String) -> String { var result = s while result.hasSuffix("0") { result.removeLast() } if result.hasSuffix(".") { result.removeLast() } + return result.isEmpty ? "0" : result - return result }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift` around lines 395 - 406, The trimTrailingZeros function can return an empty string for input "0"; update MayaInputFormatter.trimTrailingZeros to guard against that by returning "0" when the trimmed result is empty (i.e., after removing trailing zeros and a trailing dot, if result.isEmpty then return "0"). Keep MayaInputFormatter.receiveAmount behavior unchanged but rely on the safe trimTrailingZeros to never produce an empty string.DashWallet/Sources/UI/Buy Sell/BuySellPortalViewController.swift (1)
168-171: 💤 Low valueConsider using
staticinstead ofclassin final class.Since
BuySellPortalViewControlleris markedfinal, the factory method can usestatic funcinstead ofclass funcfor clarity and slight performance benefit.♻️ Proposed refactor
`@objc` -class func controller() -> BuySellPortalViewController { +static func controller() -> BuySellPortalViewController { BuySellPortalViewController() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Buy` Sell/BuySellPortalViewController.swift around lines 168 - 171, The factory method controller() on the final class BuySellPortalViewController is declared as class func but should be static for clarity and minor performance improvement; change the declaration from class func controller() -> BuySellPortalViewController to static func controller() -> BuySellPortalViewController while keeping the implementation body the same.Source: Linters/SAST tools
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift (1)
742-748: ⚖️ Poor tradeoffConsider consolidating the CrowdNode filtering logic.
The CrowdNode state check here duplicates the logic in
ShortcutActionType.customizableActions(ShortcutAction.swift lines 55-58). If one is updated without the other, users could select a CrowdNode shortcut in the picker but have it filtered out during reload, or vice versa.Consider extracting this into a shared helper or computed property on
ShortcutActionType:extension ShortcutActionType { static func shouldShow(_ type: ShortcutActionType) -> Bool { if type == .crowdNode { let state = CrowdNode.shared.signUpState return state == .finished || state == .linkedOnline } return true } }Then use it in both locations:
// In customizableActions if ShortcutActionType.shouldShow(.crowdNode) { actions.append(.crowdNode) } // In reloadShortcuts .filter { ShortcutActionType.shouldShow($0.type) }This ensures consistency and makes the business rule explicit in one place.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Home/Views/HomeViewModel.swift` around lines 742 - 748, Duplicate CrowdNode visibility logic exists in the HomeViewModel reload filter and in ShortcutActionType.customizableActions; extract the rule into a single helper (e.g., add a static method or computed property on ShortcutActionType like shouldShow(_:)) that checks if type == .crowdNode then inspects CrowdNode.shared.signUpState (returning true only for .finished or .linkedOnline) otherwise returns true, then update ShortcutActionType.customizableActions to call that helper when deciding to append .crowdNode and update the HomeViewModel filter block (the closure shown) to use .filter { ShortcutActionType.shouldShow($0.type) } so both places share the same logic.DashWallet/Sources/UI/Home/Views/Shortcuts/ShortcutItemView.swift (2)
24-24: ⚡ Quick winRemove unused
alphaproperty.The
alphaproperty is declared but never used in the view body or anywhere else in the struct. This is dead code that should be removed.🧹 Proposed fix
let title: String let iconName: String - var alpha: CGFloat = 1.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Home/Views/Shortcuts/ShortcutItemView.swift` at line 24, Remove the dead property declaration "var alpha: CGFloat = 1.0" from the ShortcutItemView struct; locate the unused "alpha" property in ShortcutItemView (the var named alpha) and delete it so the struct no longer contains an unused stored property.
75-77: ⚡ Quick winRemove duplicate "Disabled" preview.
The "Disabled" preview is identical to the "Enabled" preview—both render the same
shortcutItemGrid()with no differences. Either apply a meaningful distinction (e.g., disabled appearance, different state) or remove this duplicate preview.🧹 Proposed fix
`#Preview`("Enabled") { shortcutItemGrid() } - - -#Preview("Disabled") { - shortcutItemGrid() -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/UI/Home/Views/Shortcuts/ShortcutItemView.swift` around lines 75 - 77, The "Disabled" SwiftUI preview that calls shortcutItemGrid() is a duplicate of the "Enabled" preview; either remove the duplicate Preview("Disabled") block or modify it to show a true disabled state (e.g., call shortcutItemGrid().disabled(true) or supply a disabled/configured init/state to the view) so the preview differs; update the Preview("Disabled") declaration and its call to shortcutItemGrid() accordingly.DashWallet/Sources/Models/Coinbase/Auth/CBAuth.swift (1)
150-151: ⚡ Quick winConsider extracting the OAuth host to a constant.
The OAuth host
"login.coinbase.com"is hardcoded here and also used inCoinbaseAPIEndpoint.baseURL(line 169). Extract it toCoinbase+Constants.swiftalongside other OAuth constants (callbackURLScheme,redirectUri, etc.) for easier maintenance and consistency.📝 Proposed refactor
In
Coinbase+Constants.swift:+ static let oauthHost = "login.coinbase.com" static let callbackURLScheme = "dashwallet"Then use it here and in
CoinbaseAPIEndpoint.swift:urlComponents.scheme = "https" - urlComponents.host = "login.coinbase.com" + urlComponents.host = Coinbase.oauthHost urlComponents.path = "/oauth2/auth"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/Models/Coinbase/Auth/CBAuth.swift` around lines 150 - 151, Hardcoded OAuth host "login.coinbase.com" is duplicated; extract it to a shared constant (e.g., coinbaseOAuthHost) in Coinbase+Constants (alongside callbackURLScheme/redirectUri) and replace occurrences in CBAuth (where urlComponents.host is set) and in CoinbaseAPIEndpoint.baseURL to reference that constant for consistency and easier maintenance.DashWallet/Sources/Models/Maya/MayaAPIService.swift (1)
75-75: 💤 Low valueConsider adding emoji marker to debug log for easier filtering.
Per coding guidelines, debug logs should use emoji markers (e.g., 🌐 for network-related logs). This helps with log filtering during debugging.
📝 Suggested change
- DSLogger.log("Maya: Address validation request failed: \(error)") + DSLogger.log("🌐 Maya: Address validation request failed: \(error)")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/Models/Maya/MayaAPIService.swift` at line 75, The DSLogger.log call that records failed address validation ("Maya: Address validation request failed: \(error)") should include the recommended emoji marker for network logs for easier filtering; update the log invocation in the Maya API error path (the DSLogger.log call inside the address validation request failure branch, likely within the method handling address validation in MayaAPIService) to prepend an appropriate emoji like "🌐" to the message (e.g., "🌐 Maya: Address validation request failed: ...") so all network-related debug logs are consistently marked.Source: Coding guidelines
DashWallet/Sources/Models/Maya/MayaCoinIconLoader.swift (1)
62-64: 💤 Low valueConsider percent-encoding the coin code for robustness.
While coin codes are typically alphanumeric, adding percent encoding would make the URL construction more robust against unexpected characters.
🔧 Suggested enhancement
- guard let url = URL(string: Self.baseURL + "\(code.lowercased()).png") else { + guard let encoded = code.lowercased().addingPercentEncoding(withAllowedCharacters: .urlPathAllowed), + let url = URL(string: Self.baseURL + "\(encoded).png") else { return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/Models/Maya/MayaCoinIconLoader.swift` around lines 62 - 64, The URL construction using Self.baseURL + "\(code.lowercased()).png" should percent-encode the coin code to handle unexpected characters; before building the URL, call something like let encoded = code.lowercased().addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) and use encoded in place of code.lowercased(), returning nil if encoding fails; update the code in MayaCoinIconLoader (the place that references Self.baseURL and code.lowercased()) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@DashWallet/Resources/AppAssets.xcassets/maya.coin.link.imageset/Contents.json`:
- Around line 12-14: The Contents.json properties block incorrectly sets
"preserves-vector-representation" : true for PNG raster assets; open each
maya.coin.{link,pepe,rune,tgt,usdc,usdt,wbtc,wsteth}.imageset/Contents.json and
either remove the "preserves-vector-representation" property or change it to
false in the "properties" object so the "preserves-vector-representation" key is
not true for images whose filenames are maya.coin.<coin>.png.
In `@DashWallet/Resources/AppAssets.xcassets/Navigation`
bar/navigationbar-info.imageset/Contents.json:
- Around line 4-15: Update the incorrect scale suffixes in the imageset: replace
"controls-info@x1.png", "controls-info@x2.png", and "controls-info@x3.png" with
the iOS-standard "controls-info@1x.png", "controls-info@2x.png", and
"controls-info@3x.png" inside the Navigation bar/navigationbar-info.imageset
Contents.json and also rename the actual PNG files in the same imageset folder
to match those corrected filenames so the scale entries and filenames are
identical.
In
`@DashWallet/Resources/AppAssets.xcassets/Shortcuts/shortcut-bar-send-contact.imageset/Contents.json`:
- Around line 4-14: The imageset folder name shortcut-bar-send-contact.imageset
does not match the contained file names (shortcut-bar-send-account.png / `@2x` /
`@3x`); either rename the files to shortcut-bar-send-contact(.png / `@2x` / `@3x`) or
rename the folder to shortcut-bar-send-account.imageset to make names
consistent, and update any references to the asset in code/IB to the chosen
canonical name (shortcut-bar-send-contact or shortcut-bar-send-account) to avoid
runtime asset lookup issues.
In
`@DashWallet/Sources/Models/Coinbase/Infrastructure/API/CoinbaseAPIEndpoint.swift`:
- Around line 170-172: The .path case currently builds a full URL with
force-unwrapped URL(string:) in baseURL and leaves the path property empty,
risking crashes and breaking the TargetType contract; change baseURL to return
only the host (e.g., "https://api.coinbase.com") for all cases, move the route
string into the path computed property for the .path(let string) case (returning
the decoded/encoded route), and eliminate force-unwrapping by constructing any
full-URL overrides with safe URL(initializers) or URLComponents and handling the
optional (e.g., throw or return a safe fallback) inside the CoinbaseAPIEndpoint
implementation so no URL(string: ...) is force-unwrapped.
In `@DashWallet/Sources/Models/Transactions/SendCoinsService.swift`:
- Around line 130-134: The current authenticate() returns only Bool so callers
in SendCoinsService (uses of authenticate() in the send flow) conflate
user-cancel and hard failure into "Authentication cancelled"; change
authenticate() to surface distinct results (either throw AuthenticationError
with cases like .cancelled and .failed, or return Result<Void,
AuthenticationError>/enum) and update all callers in SendCoinsService (the
authenticate() call sites) to switch on the new result: throw
DashSpendError.paymentProcessingError("Authentication cancelled") only for
.cancelled and throw a different DashSpendError.paymentProcessingError with the
underlying error message for .failed (or rethrow the error) so cancellation vs
real failure are preserved.
- Around line 254-265: The current loop only checks per-input spend status via
account.isInputSpent and can still allow starting a new swap while a previous
swap is unconfirmed; add a wallet-level check using the new unconfirmed-swap API
before selecting/iterating inputs (e.g., call wallet.hasPreviousSwapPending() or
the provided wallet-level unconfirmed-swap method) and if it returns true throw
DashSpendError.previousSwapPending to enforce the new previousSwapPending
semantics alongside the existing per-input guard.
In `@DashWallet/Sources/UI/Buy` Sell/Model/ServiceDataProvider.swift:
- Around line 63-75: The updateServices() method duplicates mayaItems because
mayaItems is appended when building sortedItems (sortedItems = ... + mayaItems)
and then appended again in the handler call (handler?(sortedItems + mayaItems));
fix by returning the collection only once—either remove the + mayaItems when
creating sortedItems and keep handler?(sortedItems + mayaItems), or simpler:
keep sortedItems as currently built and change the handler call to
handler?(sortedItems) so mayaItems are appended only once; update references to
items, nonMayaItems, mayaItems, sortedItems and handler accordingly.
In `@DashWallet/Sources/UI/Home/Views/Shortcuts/ShortcutsView.swift`:
- Line 249: The comment on the frame height in ShortcutsView.swift is outdated:
it reads "68 cell + 8×2 vertical padding" but verticalPadding was changed to 4.0
(variable verticalPadding), so either change the height from 92 to 76 (68 + 4×2)
to match the new padding or update the comment to reflect why 92 is correct
(e.g., different base cell height or other paddings). Locate the .frame(width:
375, height: 92) line and either adjust the numeric height to 76 to match
verticalPadding or revise the inline comment to accurately explain the 92
calculation.
In `@DashWallet/Sources/UI/Maya/EnterAddress/Components/AddressSourceView.swift`:
- Around line 37-43: The switch in the computed property var title (in the
AddressSource/AddressSourceView enum) returns a hardcoded "Clipboard" string;
change that to a localized string. Replace the "Clipboard" literal with a
localization lookup (e.g. NSLocalizedString("clipboard", comment: "Address
source: Clipboard") or your app's L10n/Localization helper) so the case
.clipboard returns the localized value while leaving .uphold and .coinbase
as-is.
---
Outside diff comments:
In
`@DashWallet/Resources/AppAssets.xcassets/maya.coin.arb.imageset/Contents.json`:
- Line 13: Remove the "preserves-vector-representation" key from the
Contents.json manifests for all maya.coin.* image sets (e.g.,
maya.coin.arb.imageset) because these assets reference PNGs, not vectors; open
each maya.coin.* image set's Contents.json and delete the
"preserves-vector-representation" entry so the manifest only describes raster
images.
In `@DashWallet/Sources/UI/Buy` Sell/BuySellPortalViewController.swift:
- Around line 76-99: The navigateToCoinbase method currently force-unwraps
navigationController in the vc.userSignedOutBlock which can crash; update the
block to safely unwrap the navigation controller (e.g., guard let nav =
navigationController else { return }) and use that local nav to call
popToViewController and pushViewController (also ensure other uses of
navigationController? in this method use safe optional chaining or the same
guard before pushing ServiceOverviewViewController) so no force unwrapping
occurs; update references inside vc.userSignedOutBlock and the push logic to use
the safely unwrapped navigation controller.
---
Nitpick comments:
In `@DashWallet/Resources/AppAssets.xcassets/info-icon.imageset/Contents.json`:
- Around line 1-23: The asset catalog entry for "info-icon" needs a properties
block to enable system tinting; open the info-icon.imageset Contents.json and
add a top-level "properties" object containing "template-rendering-intent":
"template" (so the asset will render as a template image and inherit tint),
leaving the existing "images" and "info" entries intact.
In `@DashWallet/Resources/AppAssets.xcassets/Navigation`
bar/navigationbar-plus.imageset/Contents.json:
- Around line 23-25: The Contents.json for the navigationbar-plus imageset
removed the "template-rendering-intent" property which prevents the navigation
icon from being tinted by the system; restore "template-rendering-intent":
"template" inside the "properties" object of the navigationbar-plus.imageset
Contents.json (alongside "localizable": true) so the asset uses template
rendering and inherits tint colors.
In `@DashWallet/Sources/Models/Coinbase/Auth/CBAuth.swift`:
- Around line 150-151: Hardcoded OAuth host "login.coinbase.com" is duplicated;
extract it to a shared constant (e.g., coinbaseOAuthHost) in Coinbase+Constants
(alongside callbackURLScheme/redirectUri) and replace occurrences in CBAuth
(where urlComponents.host is set) and in CoinbaseAPIEndpoint.baseURL to
reference that constant for consistency and easier maintenance.
In `@DashWallet/Sources/Models/Maya/MayaAPIService.swift`:
- Line 75: The DSLogger.log call that records failed address validation ("Maya:
Address validation request failed: \(error)") should include the recommended
emoji marker for network logs for easier filtering; update the log invocation in
the Maya API error path (the DSLogger.log call inside the address validation
request failure branch, likely within the method handling address validation in
MayaAPIService) to prepend an appropriate emoji like "🌐" to the message (e.g.,
"🌐 Maya: Address validation request failed: ...") so all network-related debug
logs are consistently marked.
In `@DashWallet/Sources/Models/Maya/MayaCoinIconLoader.swift`:
- Around line 62-64: The URL construction using Self.baseURL +
"\(code.lowercased()).png" should percent-encode the coin code to handle
unexpected characters; before building the URL, call something like let encoded
= code.lowercased().addingPercentEncoding(withAllowedCharacters:
.urlPathAllowed) and use encoded in place of code.lowercased(), returning nil if
encoding fails; update the code in MayaCoinIconLoader (the place that references
Self.baseURL and code.lowercased()) accordingly.
In `@DashWallet/Sources/Models/Transactions/SendCoinsService.swift`:
- Around line 163-164: Wrap the diagnostic DSLogger.log calls in
SendCoinsService (e.g., the memo/byteCount logs and the other logs referenced)
inside `#if` DEBUG ... `#endif` blocks and prepend an emoji prefix (like "🔧" or
"🧪") to each log message to follow repository conventions; locate usages of
DSLogger.log in methods such as sendMayaSwap and surrounding blocks (the calls
around memoByteCount and the later DSLogger.log calls) and change them to
conditional debug-only logging with the emoji prefix while leaving production
behavior unchanged.
In `@DashWallet/Sources/UI/Buy` Sell/BuySellPortalViewController.swift:
- Around line 168-171: The factory method controller() on the final class
BuySellPortalViewController is declared as class func but should be static for
clarity and minor performance improvement; change the declaration from class
func controller() -> BuySellPortalViewController to static func controller() ->
BuySellPortalViewController while keeping the implementation body the same.
In `@DashWallet/Sources/UI/Home/Views/HomeViewModel.swift`:
- Around line 742-748: Duplicate CrowdNode visibility logic exists in the
HomeViewModel reload filter and in ShortcutActionType.customizableActions;
extract the rule into a single helper (e.g., add a static method or computed
property on ShortcutActionType like shouldShow(_:)) that checks if type ==
.crowdNode then inspects CrowdNode.shared.signUpState (returning true only for
.finished or .linkedOnline) otherwise returns true, then update
ShortcutActionType.customizableActions to call that helper when deciding to
append .crowdNode and update the HomeViewModel filter block (the closure shown)
to use .filter { ShortcutActionType.shouldShow($0.type) } so both places share
the same logic.
In `@DashWallet/Sources/UI/Home/Views/Shortcuts/ShortcutItemView.swift`:
- Line 24: Remove the dead property declaration "var alpha: CGFloat = 1.0" from
the ShortcutItemView struct; locate the unused "alpha" property in
ShortcutItemView (the var named alpha) and delete it so the struct no longer
contains an unused stored property.
- Around line 75-77: The "Disabled" SwiftUI preview that calls
shortcutItemGrid() is a duplicate of the "Enabled" preview; either remove the
duplicate Preview("Disabled") block or modify it to show a true disabled state
(e.g., call shortcutItemGrid().disabled(true) or supply a disabled/configured
init/state to the view) so the preview differs; update the Preview("Disabled")
declaration and its call to shortcutItemGrid() accordingly.
In `@DashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swift`:
- Around line 40-50: The symbol computed property creates a new NumberFormatter
on every access (in var symbol for case .fiat(let code)); cache a formatter
instead to avoid repeated allocations by adding a static cached NumberFormatter
(or a static [String: NumberFormatter] keyed by currency code) in the same
type/extension and reuse it inside symbol: set the cached formatter's
currencyCode = code (or fetch the cached per-code formatter) and return its
currencySymbol for .fiat, leaving .dash and .coin unchanged; update references
to use the cached formatter to improve performance during view rendering.
In `@DashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swift`:
- Around line 66-67: Remove the two commented-out legacy layout modifiers
(.scaleToFitWidth() and .layoutPriority(1)) from the MayaAmountView SwiftUI view
in MayaAmountView.swift; locate the commented lines within the view body (e.g.,
inside the struct or its body property) and delete them to clean up the file and
remove unnecessary clutter.
In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertHostingController.swift`:
- Around line 34-36: Mark the unimplemented initializer as explicitly
unavailable: annotate the required init?(coder: NSCoder) in
MayaConvertHostingController with `@available`(*, unavailable) so it cannot be
used accidentally (keep the signature required init?(coder: NSCoder) and the
existing fatalError or empty body). This makes the intent explicit to SwiftLint
and prevents instantiation via storyboard/NIB.
In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift`:
- Around line 316-321: The catch block in MayaConvertViewModel (inside the async
quote fetch where quoteRequestID and snapshot.id are compared) currently
swallows all network errors by clearing latestQuote/errorMessage/receiveAmount;
update it to set a user-facing generic errorMessage (e.g., "Unable to fetch
quote, please try again") when quoteRequestID == snapshot.id, keep
latestQuote/receiveAmount cleared as before, and add a debug log of the caught
error (using the project's logging utility) so network failures are visible in
logs for debugging.
- Around line 395-406: The trimTrailingZeros function can return an empty string
for input "0"; update MayaInputFormatter.trimTrailingZeros to guard against that
by returning "0" when the trimmed result is empty (i.e., after removing trailing
zeros and a trailing dot, if result.isEmpty then return "0"). Keep
MayaInputFormatter.receiveAmount behavior unchanged but rely on the safe
trimTrailingZeros to never produce an empty string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Why: - reduce English fallbacks in the Maya conversion flow across supported locales What: - update Maya portal and conversion-related strings in 20 localization files - refresh labels such as Convert Dash, Contact Maya Support, Receive amount, Loading, and Log In Validation: - build/tests were not run because the staged diff only changes .strings files
HashEngineering
left a comment
HashEngineering
left a comment
Contributor
There was a problem hiding this comment.
Looks fine. the individual PR's were reviewed previously
Why: - prevent the Maya service card from being appended twice in the Buy & Sell service list What: - return the already-sorted service items without concatenating mayaItems a second time Validation: - build/tests were not run because the staged diff is a one-line logic fix
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Why: - address follow-up issues around swap submission safety and asset catalog consistency What: - add a wallet-level guard to block starting a new swap while a previous swap is still unconfirmed - align navigationbar-info asset filenames with their expected 1x/2x/3x naming in the image set manifest Validation: - build/tests were not run in this session before committing
romchornyi
dismissed
HashEngineering’s stale review
The merge-base changed after approval.
Why: - Backport Maya convert fixes and related UI polish onto the release branch without pulling in risky localization churn. What: - Fix coin-input gross-up and Max behavior on the Maya convert screen. - Polish Maya select coin, order preview, portal, loading spinner, and shortcut item UI. Validation: - Not run.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
DashWallet/Sources/Models/Transactions/MayaSwapPendingGate.swift (1)
83-83: 💤 Low valueAdd emoji prefix to log statement per coding guidelines.
Coding guidelines recommend using emoji debug markers with DSLogger for easier log filtering (e.g.,
🔍for search,🔄for sync operations). Consider using an appropriate emoji prefix here.-DSLogger.log("MayaSwapPendingGate: IS-lock received for \(expected) — gate released") +DSLogger.log("🔄 MayaSwapPendingGate: IS-lock received for \(expected) — gate released")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/Models/Transactions/MayaSwapPendingGate.swift` at line 83, The DSLogger.log statement that logs the IS-lock received message for the gate release is missing an emoji prefix required by coding guidelines. Add an appropriate emoji prefix (such as 🔄 for sync operations or ✅ for completion) to the beginning of the log message string in the DSLogger.log call to align with the project's debug logging conventions and enable easier log filtering.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DashWallet/Sources/Models/Transactions/MayaSwapPendingGate.swift`:
- Around line 54-68: The observer property assignment in the register method
occurs after lock.unlock() is called, creating a race condition where
clearLocked() could be called from the notification handler before observer is
assigned. Move the entire observer registration code (the
NotificationCenter.default.addObserver call) inside the lock block, before
lock.unlock() is called, so that all mutations to the observer property are
atomic and protected by the same lock that clearLocked() expects to be held when
it accesses observer.
In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift`:
- Around line 261-267: In the applyQuoteError method, the ternary operator
evaluating the apiError condition currently returns the same localized string
for both the true and false branches (lines 264-267). This is a logic error—the
condition checks whether the apiError contains "not enough asset to pay for
fees" to provide a specific message, but the else branch should display a
different, more generic error message instead of duplicating the same one.
Update the else branch of the ternary operator to return a generic error message
(for example, the original apiError value or a generic "Conversion failed" type
message) rather than the identical "Amount too small to cover fees" string.
---
Nitpick comments:
In `@DashWallet/Sources/Models/Transactions/MayaSwapPendingGate.swift`:
- Line 83: The DSLogger.log statement that logs the IS-lock received message for
the gate release is missing an emoji prefix required by coding guidelines. Add
an appropriate emoji prefix (such as 🔄 for sync operations or ✅ for completion)
to the beginning of the log message string in the DSLogger.log call to align
with the project's debug logging conventions and enable easier log filtering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46f8434e-47b2-463e-bf45-9598134344e1
⛔ Files ignored due to path filters (3)
DashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/controls-info@1x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/controls-info@2x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/controls-info@3x.pngis excluded by!**/*.png
📒 Files selected for processing (9)
DashWallet.xcodeproj/project.pbxprojDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/Contents.jsonDashWallet/Sources/Models/Explore Dash/Services/DashSpend/DashSpendError.swiftDashWallet/Sources/Models/Transactions/MayaSwapPendingGate.swiftDashWallet/Sources/Models/Transactions/SendCoinsService.swiftDashWallet/Sources/UI/Home/Views/Shortcuts/ShortcutItemView.swiftDashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertView.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift
✅ Files skipped from review due to trivial changes (1)
- DashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/Contents.json
🚧 Files skipped from review as they are similar to previous changes (5)
- DashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swift
- DashWallet/Sources/Models/Explore Dash/Services/DashSpend/DashSpendError.swift
- DashWallet/Sources/UI/Home/Views/Shortcuts/ShortcutItemView.swift
- DashWallet/Sources/Models/Transactions/SendCoinsService.swift
- DashWallet/Sources/UI/Maya/Convert/MayaConvertView.swift
Comment on lines
+261
to
+267
| private func applyQuoteError(_ apiError: String) { | ||
| latestQuote = nil | ||
| receiveAmount = nil | ||
| errorMessage = apiError.contains("not enough asset to pay for fees") | ||
| ? NSLocalizedString("Amount too small to cover fees", comment: "Maya") | ||
| : NSLocalizedString("Amount too small to cover fees", comment: "Maya") | ||
| } |
There was a problem hiding this comment.
Both ternary branches produce identical error messages.
Lines 264-267 return the same localized string regardless of whether the API error contains "not enough asset to pay for fees". This appears to be a copy-paste error—the else branch should likely display a different, more generic error message.
Suggested fix
private func applyQuoteError(_ apiError: String) {
latestQuote = nil
receiveAmount = nil
errorMessage = apiError.contains("not enough asset to pay for fees")
? NSLocalizedString("Amount too small to cover fees", comment: "Maya")
- : NSLocalizedString("Amount too small to cover fees", comment: "Maya")
+ : NSLocalizedString("Unable to get quote", comment: "Maya")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private func applyQuoteError(_ apiError: String) { | |
| latestQuote = nil | |
| receiveAmount = nil | |
| errorMessage = apiError.contains("not enough asset to pay for fees") | |
| ? NSLocalizedString("Amount too small to cover fees", comment: "Maya") | |
| : NSLocalizedString("Amount too small to cover fees", comment: "Maya") | |
| } | |
| private func applyQuoteError(_ apiError: String) { | |
| latestQuote = nil | |
| receiveAmount = nil | |
| errorMessage = apiError.contains("not enough asset to pay for fees") | |
| ? NSLocalizedString("Amount too small to cover fees", comment: "Maya") | |
| : NSLocalizedString("Unable to get quote", comment: "Maya") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift` around lines
261 - 267, In the applyQuoteError method, the ternary operator evaluating the
apiError condition currently returns the same localized string for both the true
and false branches (lines 264-267). This is a logic error—the condition checks
whether the apiError contains "not enough asset to pay for fees" to provide a
specific message, but the else branch should display a different, more generic
error message instead of duplicating the same one. Update the else branch of the
ternary operator to return a generic error message (for example, the original
apiError value or a generic "Conversion failed" type message) rather than the
identical "Amount too small to cover fees" string.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
DashWallet/Sources/Models/Maya/MayaCoinIconLoader.swift (1)
22-29: 💤 Low valueDoc comment is outdated after adding SwapKit CDN source.
The comment at lines 24-25 states icons are fetched from jsupa/crypto-icons, but the primary source is now the SwapKit CDN with jsupa as fallback. Consider updating to reflect the dual-source approach.
📝 Suggested doc update
/// Loads remote coin icons with a two-level cache: memory (`NSCache`) + disk (`URLCache`). /// -/// Icons are fetched from the jsupa/crypto-icons repository using the lowercased coin code, -/// matching the Android implementation. Disk-cached entries survive app restarts and are +/// Icons are fetched primarily from the SwapKit token-list CDN using the full asset identifier, +/// with jsupa/crypto-icons as a fallback using the lowercased coin code. Disk-cached entries survive app restarts and are /// served without re-downloading until the OS purges the cache.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DashWallet/Sources/Models/Maya/MayaCoinIconLoader.swift` around lines 22 - 29, The documentation comment for the MayaCoinIconLoader class incorrectly states that icons are fetched from the jsupa/crypto-icons repository. Update the doc comment to accurately reflect the current dual-source approach where the SwapKit CDN is the primary source for fetching coin icons and jsupa/crypto-icons is used as a fallback. This ensures the documentation accurately describes the actual implementation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@DashWallet/Sources/Models/Maya/MayaCoinIconLoader.swift`:
- Around line 22-29: The documentation comment for the MayaCoinIconLoader class
incorrectly states that icons are fetched from the jsupa/crypto-icons
repository. Update the doc comment to accurately reflect the current dual-source
approach where the SwapKit CDN is the primary source for fetching coin icons and
jsupa/crypto-icons is used as a fallback. This ensures the documentation
accurately describes the actual implementation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 358844d9-0392-49be-b747-c5e41fe58b1e
📒 Files selected for processing (7)
DashWallet.xcodeproj/project.pbxprojDashWallet/Sources/Models/Maya/MayaCoinIconLoader.swiftDashWallet/Sources/Models/Transactions/MayaSwapPendingGate.swiftDashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swiftDashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertView.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift
🚧 Files skipped from review as they are similar to previous changes (4)
- DashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swift
- DashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swift
- DashWallet/Sources/Models/Transactions/MayaSwapPendingGate.swift
- DashWallet/Sources/UI/Maya/Convert/MayaConvertView.swift
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
4 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.
Issue being fixed or feature implemented
This PR merges the
8.6.0release branch intomaster.It bundles the approved changes for the
8.6.0iOS release into a single production-ready branch, including the new Maya conversion flow, shortcut bar improvements, PiggyCards translation updates, CrowdNode restrictions for this release, and the final release version/build updates.What was done?
8.6.0release work into a single branch for final delivery.8.6.0.How Has This Been Tested?
feat/release-8.6.0.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements