feat(wallet): restore MAYACHAIN swap routes with OP_RETURN memo deposits - #916
Merged
Conversation
…scription `viewModel.$error` publishes `Error?`, and `Optional` has no `localizedDescription`, so both error sinks failed to compile. The dashpay scheme did not build at all on this branch until this was fixed. Unrelated to the Maya work that surfaced it — split out so it can be reviewed and landed against the base branch on its own.
Maya was disabled during the DashSync unlink because SwiftDashSDK could not build an OP_RETURN output. With the SDK-side controls in place, Maya returns as routes inside Dash DEX — the standalone Maya portal stays hidden. Send path: - SwiftDashSDKTransactionSender.buildAndSignSwapDeposit builds VOUT0=vault, VOUT1=zero-value OP_RETURN memo, VOUT2=change back to VIN0, then decodes the signed bytes and refuses to return unless the shape, the memo payload, the VOUT2==VIN0 script and a >= 1 duff/byte fee all check out - WalletSendService.sendSwapDeposit reuses the existing sync/online/authorize plumbing; the standard send signature is untouched - SendCoinsService.sendSwapKitSwap routes memo-bearing deposits to it and keeps memo-less NEAR routes on the plain send Routing: - MAYACHAIN and MAYACHAIN_STREAMING are separate SwapKit providers with different token lists (31 vs 18); classification unions both, otherwise Maya-routable assets such as KUJI.KUJI and XRD.XRD are invisible - NEAR stays preferred; Maya is requested only for assets NEAR cannot route - mayaOnly assets are no longer hidden from the Sell picker Guards (both kept — a memo-less deposit orphans the funds at the vault): - the provider and OrderPreviewViewModel each refuse a memo over the 80-byte OP_RETURN limit, reporting a specific error rather than "coin unavailable" - Maya's 10k-duff dust floor is enforced, keyed on the memo rather than on the execution-network display label Measured live: real Maya memos run 72-80 bytes, and for one route the length varies with the amount (79/80/79/79 at 0.1/1/10/50 DASH), so the ceiling is reachable in production and the copy names both the amount and the address.
The picker labels a coin routable by both protocols "Multiple networks", but the Sell quote only ever asked NEAR, so the label promised a choice the code never made. Offer both and let SwapKit's route ranking decide. The provider list stays explicit rather than nil: no filter would also admit THORChain, Chainflip and the rest, which neither the classification nor the deposit path accounts for. Maya-only coins keep the Maya providers, NEAR-only keep NEAR, and an unusable classification still falls back to NEAR. A MAYACHAIN route can now win for a coin that previously always deposited memo-less, so the 80-byte memo ceiling and Maya's dust floor apply to dual-routable coins too. Both guards run on the fresh pre-commit quote.
|
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesMaya swap deposit flow
Error observer cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant OrderPreviewViewModel
participant SendCoinsService
participant WalletSendService
participant SwiftDashSDKTransactionSender
participant DashNetwork
OrderPreviewViewModel->>SendCoinsService: Submit swap with memo
SendCoinsService->>WalletSendService: sendSwapDeposit(vaultAddress, amount, memo)
WalletSendService->>SwiftDashSDKTransactionSender: Build and sign MAYACHAIN deposit
SwiftDashSDKTransactionSender-->>WalletSendService: Signed transaction and txHash
WalletSendService->>DashNetwork: Broadcast transaction
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DashWallet/Sources/Models/SwapKit/SwapKitSwapProvider.swift (1)
416-438: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject a MAYACHAIN route that has no memo.
When
best.providerscontainsMAYACHAINorMAYACHAIN_STREAMING, an absent or whitespace-onlyswapResponse.memobecomesnil.SendCoinsService.sendSwapKitSwapthen uses the plain-send path. The vault receives DASH without the required OP_RETURN instruction.Require a non-empty memo for Maya routes before creating
SwapQuoteResult.Proposed fix
let memo = swapResponse.memo?.trimmingCharacters(in: .whitespacesAndNewlines) +let isMayaRoute = best.providers.contains { + SwapKitConstants.mayaProviders.contains($0.uppercased()) +} +guard !isMayaRoute || memo?.isEmpty == false else { + return errorResult(NSLocalizedString( + "MAYACHAIN returned no deposit memo. Please refresh and try again.", + comment: "SwapKit" + )) +} if let memo, !memo.isEmpty, memo.utf8.count > Constants.maxMemoBytes {🤖 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/SwapKit/SwapKitSwapProvider.swift` around lines 416 - 438, Before creating SwapQuoteResult in the neutral-result mapping, detect whether best.providers contains MAYACHAIN or MAYACHAIN_STREAMING and require swapResponse.memo to be non-empty after whitespace trimming. Return the existing appropriate errorResult when a Maya route lacks a valid memo; preserve the current over-length rejection and non-Maya behavior.
🧹 Nitpick comments (1)
DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKTransactionSender.swift (1)
124-178: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftAdd automated tests for the swap-deposit transaction shape.
Test an 80-byte UTF-8 memo, an 81-byte UTF-8 memo, VOUT ordering, zero-value OP_RETURN output, change output, and invalid shape rejection. This flow sends funds and depends on exact serialized-output semantics.
Also applies to: 573-617
🤖 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/Infrastructure/SwiftDashSDK/SwiftDashSDKTransactionSender.swift` around lines 124 - 178, Add automated coverage for buildAndSignSwapDeposit and assert the serialized transaction shape: accept an 80-byte UTF-8 memo, reject an 81-byte memo, preserve vault/payment at VOUT0 and zero-value OP_RETURN at VOUT1, return change when applicable, and reject transactions failing assertSwapDepositShape. Use test fixtures or mocks that avoid broadcasting while exercising the exact output serialization and validation paths.
🤖 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.
Outside diff comments:
In `@DashWallet/Sources/Models/SwapKit/SwapKitSwapProvider.swift`:
- Around line 416-438: Before creating SwapQuoteResult in the neutral-result
mapping, detect whether best.providers contains MAYACHAIN or MAYACHAIN_STREAMING
and require swapResponse.memo to be non-empty after whitespace trimming. Return
the existing appropriate errorResult when a Maya route lacks a valid memo;
preserve the current over-length rejection and non-Maya behavior.
---
Nitpick comments:
In
`@DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKTransactionSender.swift`:
- Around line 124-178: Add automated coverage for buildAndSignSwapDeposit and
assert the serialized transaction shape: accept an 80-byte UTF-8 memo, reject an
81-byte memo, preserve vault/payment at VOUT0 and zero-value OP_RETURN at VOUT1,
return change when applicable, and reject transactions failing
assertSwapDepositShape. Use test fixtures or mocks that avoid broadcasting while
exercising the exact output serialization and validation paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4322d5d9-afd3-433b-8198-c0dd559b691e
📒 Files selected for processing (13)
DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKTransactionSender.swiftDashWallet/Sources/Models/Swap/SwapExecutionData.swiftDashWallet/Sources/Models/Swap/SwapKitErrorCopy.swiftDashWallet/Sources/Models/Swap/SwapTrackingService.swiftDashWallet/Sources/Models/SwapKit/SwapKitConstants.swiftDashWallet/Sources/Models/SwapKit/SwapKitSwapProvider.swiftDashWallet/Sources/Models/Transactions/SendCoinsService.swiftDashWallet/Sources/Models/Transactions/WalletSendService.swiftDashWallet/Sources/UI/Buy Sell/BuySellPortalView.swiftDashWallet/Sources/UI/CrowdNode/Online/OnlineAccountEmailController.swiftDashWallet/Sources/UI/CrowdNode/Portal/CrowdNodePortalViewController.swiftDashWallet/Sources/UI/Swap/OrderPreview/OrderPreviewViewModel.swiftDashWallet/Sources/UI/Swap/SelectCoin/SelectCoinViewModel.swift
…ble"
QA hit this picking a Uphold destination address in the Dash DEX flow.
`EnterAddressViewModel.resolveSourceState` distinguishes "this coin isn't
supported" (.notAvailable) from "your session ended" (.loggedOut) purely by
`DWUpholdClient.isAuthorized`. That flag only asks whether a token is stored,
never whether Uphold still accepts it — and an expired token stays in the
keychain. So the whole .loggedOut branch was unreachable for the most common
way to lose a session, and the user saw "Not available" with no hint that
re-authorizing would fix it:
Fetch cards failed (HTTP 401): access token has expired
Got 0 cards: [], looking for SOL on solana
No card for SOL, creating card and address
Create card failed with status 401 for SOL
A 401 now clears the stored token via the new
`DWUpholdClient.invalidateRejectedSession`, so `isAuthorized` reports NO and
the existing UI offers re-login. It deliberately skips the revoke call `logOut`
makes: Uphold has already rejected the token, so revoking it would just fail
again. Note the 401 also made the code read "no cards" as "user has no card"
and go on to create one — that second request is now not reached.
Also stop asking Uphold to mint an address on a network its address endpoint
does not accept. Naming a network in `upholdNetwork(for:)` is not the same as
Uphold being able to create one there, so selecting USDC/USDT on Arbitrum
produced a guaranteed 400 on every tap:
No 'arbitrum' address on card 0b046a36…, creating one
Address creation failed (HTTP 400).
{"code":"validation_failed","errors":{"network":[{"code":"invalid",…}]}}
The UI outcome was already correct — nil address plus a live session maps to
.notAvailable — so this removes the pointless round trip and the misleading
error log, not a user-visible bug. Addresses already present on the card are
still used for any network.
A QA report of the "Conversion failed" screen could not be diagnosed from an exported log: the whole swap flow left no trace of why it failed. That screen renders the `default` branch of `SwapKitErrorCopy.message` — an error code the mapper does not recognise — and every path to it discarded the raw reason on the way: - `SwapKitSwapProvider.errorResult` wrapped the message into a result and returned it unlogged, and it is the funnel for every quote and swap failure - `OrderPreviewViewModel.setFailure` mapped straight to user-facing copy - `SwapConvertViewModel.applyQuoteError` rewrote the API error into UI strings - `SwapKitErrorCopy` collapsed anything unmapped into "something went wrong" So a screenshot was the only evidence that existed, and it says nothing. Log at each of those points, before the message is rewritten, plus one line on the success side when the deposit is broadcast — without it a log cannot distinguish "the deposit never went out" from "it went out and the swap failed afterwards". An unmapped code is now visible in the logs, which is also how we find out which SwapKit errors still deserve their own copy.
Contributor
Author
|
Status update for review — both SDK dependencies have landed since this PR was opened:
Two things worth a reviewer's attention, both stated in the description but easy to miss:
Unrelated fixes riding along, kept as their own commits so they can be split if preferred: the CrowdNode |
QuantumExplorer
added a commit
that referenced
this pull request
Aug 6, 2026
…he merge All four fixes are to upstream code that no longer compiled at swift-sdk-integration HEAD (the maya-swift-sdk merge, PR #916, landed after the finalizeAtomic refactor, PR #920, without rebasing over it): - CrowdNode error observers (OnlineAccountEmailController, CrowdNodePortalViewController): drop the `if let` on a value the preceding `compactMap { $0 }` already unwrapped — a non-optional binding is a compile error. - MAYA swap deposit (SwiftDashSDKTransactionSender.buildAndSignSwapDeposit): migrate from the removed split setFunding/buildSigned surface to finalizeAtomic, returning FinalizedCoreTransaction like every other send path; assertSwapDepositShape takes the serialized bytes + fee since the finalized handle exposes no raw `.data`. - WalletSendService.buildPreparedSwapDeposit: serialize via `serializedData()` (the PreparedStandardSend initializer now holds a FinalizedCoreTransaction). Candidate for cherry-pick to swift-sdk-integration — the base branch does not build without these. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
added a commit
that referenced
this pull request
Aug 7, 2026
…-funds prompt (#858) * feat(coinjoin): offer BIP44 or Shielded destination in post-sync move-funds prompt The first-sync "Move your mixed coins" popup previously always swept the leftover CoinJoin balance into the BIP44 spendable balance. When the balance is large enough to be worth shielding (>= 2x the shield pool fee + asset-lock base cost + L1 send-fee reserve, and the shielded sub-wallet is bound), a destination-choice sheet now offers: - Dash Wallet balance: the existing one-hop sweep. - Shielded balance: sweep CoinJoin -> own BIP44 receive address, wait for the swept outputs to become spendable, then asset-lock the net amount into the shielded pool (Type 18), with a step checklist. The user authenticates once: the sweep leg runs the spend gate and the asset lock skips its own via a new explicit alreadyAuthorized parameter on ShieldedTransferCoordinator.performAssetLock. Failure posture: the sweep-leg result is kept on the ViewModel, so "Try again" never re-sweeps an emptied CoinJoin account - it resumes from the UTXO wait, or resumes a committed asset lock on its exact outpoint (mirroring the transfer confirm sheet); a stuck lock that survives the session is picked up by the home tx list's ShieldedRecoverySheet. Every failure mode leaves the funds spendable in the BIP44 balance. Small balances keep the BIP44-only dialog, and the availability check fails closed to it. Plumbing: sweepCoinJoin(to:) now also returns the net swept duffs; the CrowdNode UTXO-wait helper is promoted for reuse (waitForFunds); ShieldedTransferStepList gains a positional (labels/currentIndex) init for stages the coordinator phases can't express; the wallet source's onMain trampoline is promoted for the availability check's host reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(coinjoin): shield mixed coins via a direct CoinJoin-drain asset lock Replaces the two-hop shielded destination (sweep CoinJoin -> BIP44, wait, asset-lock) with a single CoinJoin-funded drain asset lock: every mixed-coin UTXO funds the Type 18 lock directly (lock value = sum(inputs) - L1 fee, computed SDK-side) and the shielded pool receives lock_value - pool_fee. The mixed coins never hop through a transparent BIP44 address, and the flow is one transaction with one PIN prompt. - ShieldedTransferCoordinator.performAssetLock gains an AssetLockFundingSource (.bip44(amountDuffs:) | .coinJoinDrain) routed to the new SDK wrapper shieldedFundFromCoinJoinDrain; the historical amountDuffs entry point delegates unchanged. The alreadyAuthorized seam is removed - the coordinator's own gate is the flow's single prompt again. Resume-by-outpoint covers drain locks identically. - CoinJoinMoveFundsSheet: the shielded path is one coordinator run with the same step checklist as the internal transfer; auth-cancel returns to the destination choice; the CoinJoin balance is re-tallied on completion so the popup/Settings surfaces self-clear. - The dead two-hop plumbing is removed (sweepCoinJoinForShielding, waitForSweptCoinJoinFunds, the sweep net-amount return, waitForFunds, and their error strings). Consumes SwiftDashSDK's shieldedFundFromCoinJoinDrain (platform feat/coinjoin-asset-lock-funding, on rust-dashcore key-wallet drain support) - rebuild DashSDKFFI.xcframework after pulling platform. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(build): repair swift-sdk-integration compile breaks surfaced by the merge All four fixes are to upstream code that no longer compiled at swift-sdk-integration HEAD (the maya-swift-sdk merge, PR #916, landed after the finalizeAtomic refactor, PR #920, without rebasing over it): - CrowdNode error observers (OnlineAccountEmailController, CrowdNodePortalViewController): drop the `if let` on a value the preceding `compactMap { $0 }` already unwrapped — a non-optional binding is a compile error. - MAYA swap deposit (SwiftDashSDKTransactionSender.buildAndSignSwapDeposit): migrate from the removed split setFunding/buildSigned surface to finalizeAtomic, returning FinalizedCoreTransaction like every other send path; assertSwapDepositShape takes the serialized bytes + fee since the finalized handle exposes no raw `.data`. - WalletSendService.buildPreparedSwapDeposit: serialize via `serializedData()` (the PreparedStandardSend initializer now holds a FinalizedCoreTransaction). Candidate for cherry-pick to swift-sdk-integration — the base branch does not build without these. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 7, 2026
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
3 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
Maya swaps were disabled during the DashSync unlink because SwiftDashSDK could
not build an
OP_RETURNoutput, and every MAYACHAIN route needs the swap memoencoded on-chain. The workaround forced NEAR-intents routing on every Sell quote
and hid Maya-only coins from the picker, which left a set of destination assets
unswappable.
With the SDK-side controls in place (dashpay/platform#4286, dashpay/rust-dashcore#922),
Maya comes back as routes inside Dash DEX. The standalone Maya portal stays
hidden —
ServiceDataProvider.shouldShow(.maya)is unchanged.What was done?
Send path
SwiftDashSDKTransactionSender.buildAndSignSwapDepositbuilds the MAYACHAINdeposit shape:
VOUT0vault payment,VOUT1zero-valueOP_RETURNmemo,VOUT2change back to theVIN0address, with BIP-69 output sorting off.It then decodes the signed bytes and refuses to return unless the output
shape, the memo payload, the
VOUT2==VIN0script and a ≥ 1 duff/byte feeall check out.
WalletSendService.sendSwapDepositreuses the existing sync/online/authorizeplumbing; the standard
sendsignature is untouched so no other caller cansilently drop a memo.
SendCoinsService.sendSwapKitSwapgained amemoparameter and routesmemo-bearing deposits to the new path, leaving memo-less NEAR routes on the
plain send.
Routing
MAYACHAINandMAYACHAIN_STREAMINGare separate SwapKit providers withdifferent
/tokenslists (31 vs 18). Classification now unions both —building it from the streaming list alone hid Maya-routable assets such as
KUJI.KUJIandXRD.XRD.SwapKit's ranking decides, which is what the picker's "Multiple networks"
label already promised. The provider list stays explicit so routing cannot
widen to THORChain/Chainflip.
Guards (both kept — a memo-less deposit orphans the funds at the vault)
SwapKitSwapProviderandOrderPreviewViewModel.resolveExecutionDataeachrefuse a memo over the 80-byte
OP_RETURNlimit, surfacing a specific messageinstead of a generic "coin unavailable". Measured against the live API, real
Maya memos run 72–80 bytes and the length varies with the amount as well
as the address (79/80/79/79 at 0.1/1/10/50 DASH on one route), so the copy
names both levers.
rather than on the execution-network display label.
Incidental
fix(crowdnode):viewModel.$errorpublishesError?, andOptionalhas nolocalizedDescription, so both CrowdNode error sinks failed to compile — thedashpayscheme did not build at all on this branch before it. Unrelated tothe Maya work; kept as its own commit so it can be cherry-picked if preferred.
How Has This Been Tested?
dashpaybuild (xcodebuild -workspace DashWallet.xcworkspace -scheme dashpay -sdk iphonesimulator -destination 'generic/platform=iOS Simulator' ARCHS=arm64 build).MAYACHAIN route.
vault address and memo, memo byte lengths across the Maya-only asset set).
feat(key-wallet): OP_RETURN outputs, output-order control and change-to-VIN0 rust-dashcore#922 and an integration test in feat(sdk): expose OP_RETURN, output-order and change-to-VIN0 controls platform#4286.
Not covered: the refund path. Change-to-
VIN0only matters when MAYAChainrefunds a swap, and that has not been exercised end to end.
The unit-test target remains broken repo-wide, so no app-side tests were added.
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes