feat(dashpay): authorize a token purchase from a DashConnect link - #1109
Open
romchornyi wants to merge 3 commits into
Open
feat(dashpay): authorize a token purchase from a DashConnect link#1109romchornyi wants to merge 3 commits into
romchornyi wants to merge 3 commits into
Conversation
`WalletStartupStatus` gained `seedBindingUnverified` and `identityScanIncomplete`, so the DashPay readiness log stopped compiling. Both are logged as warnings rather than errors: unlike `discoveryFailed`, which is a local fault nothing will clear on its own, each of these is recoverable on a later start — a rerun with the matching signer completes the queued contact-account work, and an incomplete identity scan stays on record so the next launch re-scans instead of taking the warm shortcut.
A `dash-st:` link was parsed strictly as an `IdentityUpdateTransition` — the DashConnect key-registration step — so a dApp asking the wallet to authorize a token purchase failed with "Expected IdentityUpdateTransition, got Batch(...)". Yappr's own fallback was to ask the user to paste a CRITICAL private key into a web page, which is exactly what DashConnect exists to avoid. The `dash-st` path now parses once through the SDK's new `parseStateTransition` and switches on the kind rather than on a thrown error. Identity updates keep today's key-registration behaviour unchanged, including its wrong-identity, unexpected-mutation and mismatched-key validation. A token purchase gets an approval sheet showing what is being authorized — token count, the total price rendered in DASH, and which identity of the user's will be charged — and only then signs. The wallet never signs the bytes it was handed. It reads the intent from them, shows it, and rebuilds the purchase through `tokenPurchase`, passing the parsed price as `expectedTotalCost` so the amount shown and the amount charged cannot diverge. Ownership is checked before the sheet is built and again immediately before signing, since the sheet can sit open while the active identity changes. Signing uses a CRITICAL key, which the signer selects itself; an identity without one fails with a visible message rather than a silent no-op. Two helpers shared with the connection sheet were promoted out of it rather than copied.
`approveLogin` logged that it started and then went silent until it finished, so a stall anywhere across key derivation, the Platform document write and the connection preview looked identical from a log — the whole approval was one opaque span. Adds four markers around the steps that can block, which is what let a report of "the QR never disappears" be narrowed to the wallet having completed its side and the dApp not picking the document up.
|
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: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
🕓 Ready for review — 12 ahead in queue (commit 43bfabd) |
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
A
dash-st:link was parsed strictly as anIdentityUpdateTransition— the DashConnect key-registration step — so a dApp asking the wallet to authorize a token purchase failed outright withExpected IdentityUpdateTransition, got Batch(...TokenDirectPurchase...).Yappr's fallback without a wallet path was to ask the user to paste their CRITICAL private key into a web page, which is precisely what DashConnect exists to prevent — the approval sheet's own promise is that the app never sees your keys.
Depends on dashpay/platform#4584, which adds the parser this uses. Merge that first; this branch does not build against a
v4.2-devwithout it.What was done?
dash-stpath parses once through the SDK's newparseStateTransitionand switches on the kind, rather than branching on a thrown "expected X, got Y" error. Identity updates keep today's key-registration behaviour unchanged, including its wrong-identity, unexpected-mutation and mismatched-derived-key validation; the protocol method becamehandleStateTransitionand the old body is now a private continuation, so no rule moved.ApproveTokenPurchaseSheet) showing what is being authorized: token count, the total price rendered in DASH, the token id, and which identity of the user's will be charged.tokenPurchase, passing the parsed price asexpectedTotalCostso the amount shown and the amount charged cannot diverge.approveLogingained step markers. It logged that it started and then went silent until it finished, so a stall in key derivation, the Platform write or the connection preview looked identical from a log.Also included, because the branch builds against a bumped platform: the
WalletStartupStatusswitch fix. It is submitted separately as #1108 againstdevelop, sincedevelopdoes not currently compile againstv4.2-devhead without it; the copy here is what lets this branch build and will fold away once #1108 lands.How Has This Been Tested?
xcodebuild -workspace DashWallet.xcworkspace -scheme dashpay -sdk iphonesimulator ARCHS=arm64— succeeds.End to end on a device against devnet-moutai: a real Yappr
dash-st:purchase link opens the wallet, the sheet renders 100 tokens for 0.001 DASH against the correct identity, and the purchase completes on chain. The price shown was cross-checked against Yappr's own screen independently.Getting there also surfaced, and required, a platform bump to
v4.2-devhead: an older pin could not read therankedCountableat-form introduced by dashpay/platform#4531, which Yappr's token contract uses, and failed withProof verification error: rankedCountable value must be a boolean.Not covered: the unit-test target is the known-broken one described in
CLAUDE.md, so tests compile but were not run. The identity-update (dash-stkey registration) path was verified by reading the final code rather than re-exercised, since its body is unchanged.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only