test: add marketplace wallet journey - #720
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
Addressed in 48a85d1: the journey now requires the fixture Fulcrum endpoint at |
piotr-iohk
left a comment
There was a problem hiding this comment.
QA LGTM on the journey docs. Did not re-run the isolated marketplace fixture.
Checked latest (48a85d1f):
- Suite is registered;
xmllint --noout journeys/pubky-marketplace/wallet-leg.xmlpasses. - Greptile Electrum note is addressed:
E2E_BUILD+E2E_BACKEND=localresolves Electrum totcp://127.0.0.1:60001inEnv.electrumServerUrl. No in-app override required. - IDs that already exist on this tree:
PubkyAuthWatchOnlyConsent/Approve/Authorize/OK,ContactPaymentsToggle,PaymentRequestsScreen,PaymentRequestRow-<id>,ReviewAmount,ReviewContactRecipient,GRAB,SendSuccess,ActivityAmount,ActivityTxDetails,StatusConfirmed.
Note, not a blocker if merge order is explicit:
PaymentRequestPay-<id>is not on this branch. It is added in #721 (PaymentRequestsView.swift). The README already lists #714 / sibling app work as a dependency. Do not treat this PR’s tree as an executable journey until that Pay selector is on master (or merge #721 first).
I am not blocking on not replaying the two-simulator Locks fixture. The contract and the 2026-09-02 acceptance record are consistent with the Android counterpart.
jvsena42
left a comment
There was a problem hiding this comment.
Checked this as a port of Android #1220. It is a faithful one: same file name, same <journey name>, and 27/27 actions identical bar the single step where the platforms genuinely differ. No Android step dropped, and 13 of the 14 asserted accessibility identifiers resolve in Bitkit/.
The 14th, PaymentRequestPay-<id>, does not exist on this branch or on master — but @piotr-iohk already flagged that with the merge-order caveat and approved, so I have not re-filed it. Worth keeping explicit: this journey stops at action 18 of 27 until #721 lands.
Three documentation items below.
48a85d1 to
2149c6a
Compare
jvsena42
left a comment
There was a problem hiding this comment.
Checked every selector this journey names against the iOS codebase — all resolve except PaymentRequestPay, which is still absent from master (the CustomButton at PaymentRequestsView.swift:82-90 has no .accessibilityIdentifier; git grep PaymentRequestPay origin/master is empty). That's the known #721 dependency, already covered by piotr-iohk, so I'm not re-filing it — but there's a wrinkle on it inline.
Three doc-only notes below, none blocking.
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed at 4ed957f. One parity note inline — not blocking. Docs-only, nothing runs these in CI, and Paykit UI is behind the Dev Settings flag.
Your pushed fixes are correct, and I checked them against real code rather than just confirming they landed:
- Paykit UI precondition — the path, id and confirmation step all match:
DevSettingsView.swift:5binds@AppStorage(PaykitFeatureFlags.uiEnabledKey), the toggle at:88-104carriestestIdentifier: "PaykitUiToggle", the warning alert with theEnablebutton is at:208-215, and Settings → Advanced → Dev Settings isAdvancedSettingsView.swift:35gated onEnv.isDebug. - Capability enumeration — falsifiable and right.
PubkyAuthClaim.watchOnlyAccountCapabilitiesis/pub/paykit/v0/bitkit/server/:rw,/pub/paykit/v0/private/bitkit/server/:rw;displayPathdrops the trailing slash anddisplayAccessrendersrwasREAD, WRITE, andpermissionRowputs both in the tree as plainText. Exactly what the action now claims.
Everything else resolves: the auth ids (PubkyAuthWatchOnlyConsent, PubkyAuthWatchOnlyApprove, PubkyAuthAuthorize, PubkyAuthOK), PaymentRequestRow-<id>, PaymentRequestsScreen, and the screen's active card really does expose Pay/Dismiss for an actionable request. Actions 1-15 and 17-27 match Android head one-to-one modulo the id/testTag vocabulary. PaymentRequestPay-<id> still being absent is the known #721 dependency piotr-iohk already has covered, so I'm not re-filing it.
I also chased whether the naming-table row and the "iOS opens the persistent screen instead of Android's transient sheet" sentences were wrong, since iOS does have PaymentRequestsSheet and PaymentRequestsBell verbatim. They're fine as written — the prose says the journey drives the screen, which is an accurate surface choice, and the table row is there because I asked for it. Leaving that alone.
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed the delta at 3a571fde. No HIGH/MEDIUM. The parity drift is closed — details on the resolved thread.
Worth noting you fixed it in the direction that preserves coverage rather than the one that just silences the note: the journey now exercises the automatic review and the bell-to-sheet path that Android exercises, instead of keeping the screen-only route and documenting a divergence. Dropping the journeys/README.md naming-table row is right too — that table is the corpus-wide authority on identifiers that genuinely cannot match, and PaymentRequestsSheet/PaymentRequestsBell both exist verbatim on iOS, so the row would have misled the next port.
The remaining stated difference — iOS uses the review's in-sheet back control where Android uses system back — is a real platform difference and reads correctly.
3a571fd to
77547ed
Compare
jvsena42
left a comment
There was a problem hiding this comment.
One finding inline, on the precondition. Doc-only, so it doesn't block — but it's the same shape as the PaykitUiToggle omission from the last round, and it stops an operator at action 2 of 30.
Scope is clean: test: matches the diff exactly, no production Swift, in contrast to the Android twin.
Delta since my last review: one commit, e3020496 "test: align marketplace journey parity" (+2/-1, wallet-leg.xml only) — adds action 14 "Return the buyer wallet to Home" and rewrites action 18 to dismiss the automatic review, verify the bell, then tap it. Both are accurate against real code and nothing earlier was lost in the rebase.
Checked and clean:
- The two new delta actions are true of the app. Action 17's auto-review:
presentNextIncomingPaykitPaymentRequest()claims aContactPaymentContext, callshandleScannedData, andcontactPaymentRoutemaps.quickpay/.amount->.confirmwhenincomingPaymentRequest != nil, so the send sheet opens onSendConfirmationView. Action 18's "bell visible after dismissal" holds and isn't flaky:SendSheet.onAppearcallsmarkPresentedIfPending, so the 700ms re-presentation triggered byactiveSheetConfiguration -> nilfindsrequestsForPresentation()empty and doesn't reopen the review, while the request stays inpendingRequestsso the bell keeps rendering.onDisappearclearingcontactPaymentContextis what unblocks the later bell -> Pay path. - Bell -> sheet -> Pay actually chains.
onPayuseshideSheetBeforePerforming { requestPresentation(request) }, so the sheet is dismissed beforerequestedPresentationIdis set — otherwise theactiveSheetConfiguration == nilguard would have blocked the review. Actions 18-21 are coherent. - Every identifier resolves at base, except the known #721 dependency: the four
PubkyAuth*ids,ContactPaymentsToggle,Contact_\(publicKey),ReviewAmount,ReviewContactRecipient,GRAB,SendSuccess,ActivityAmount/ActivityTxDetails/StatusConfirmed,PaymentRequestsBell,PaymentRequestsSheet,PaymentRequestRow-<id>,PaykitUiToggle. - The capability assertion is exact after the rebase —
watchOnlyAccountCapabilities,displayPathdropping the trailing slash,displayAccessrenderingrwasREAD, WRITE, both rows in the tree as plainText. receivePrivateMessagesFromLinkedPeers()named atREADME.md:62is a real iOS symbol, not an Android-ism carried over.- The #715 dependency is real and correctly stated:
pubkyauthappears inInfo.plistonly underLSApplicationQueriesSchemes, neverCFBundleURLTypes, so iOS genuinely can't receive the setup URL as a link today — which is whatREADME.md:54-55lists as required app work. All four listed issues are still open. - Structure:
xmllint --nooutpasses, the file matches the<journey>/<description>/<actions>shape of the existing suites, andjourneys/README.md:138registers the suite with a count that matches reality. - No secrets or live-network material. The acceptance record holds regtest-only artifacts, two z32 identities and a simulator UUID from a throwaway fixture, plus a video hash. No seed, token or homeserver credential; the
E2E_HOMESERVER_PUBKYbuild arg is a placeholder.
Cross-repo with synonymdev/bitkit-android#1220: 30/30 actions in identical order, same journey name and file layout — e3020496 closed the drift rather than adding to it. Two deliberate divergences remain and both are fine: the back-gesture wording, and action 4 where you enumerate the two capability paths with READ, WRITE while Android still says "only the requested Paykit capabilities". Android is now the weaker file there — that enumeration is worth porting to #1220 rather than weakening this one. Android's README is also stale in claiming PaymentRequestPay-<id> is Android-only; #721 supersedes that.
On why Android needs production changes here and you need none: iOS master already carries every identifier the journey uses, and Android lacked them. I resolved all of them by grep at base. The one genuine iOS gap is PaymentRequestPay-<id>, which #721 supplies — the known merge-order caveat piotr-iohk already approved with, not something to re-file.
Closes #718
Adds the two-wallet Pubky marketplace wallet-leg journey for watch-only seller setup, linked-buyer Payment Request receipt, on-chain approval, broadcast, and regtest confirmation.
Description
pubky/paykit-server#2at867fc883.Linked Issues/Tasks
Preview
52-ios-marketplace-wallet-leg.mp4
The sanitized replay shows the request-specific Pay action, 15,000-sat seller review, one swipe to
SendSuccess, paid request history, confirmed activity, and exact transaction details. It predates the automatic-review, dismissal, and header-bell parity step; a replacement replay remains required.QA Notes
Manual Tests
PaymentRequestsSheet, request-specific Pay, review, broadcast, and confirmed activity evidence.PaymentRequestsSheeton the current head.Automated Checks
77934b54is based on60e75e18; validation was not run for this documentation-only review fix at user direction.e3020496: every journey XML file parses withxmllint; the marketplace flow has 30 actions and matches Android head04d0720fafter the requiredtestTag/idand back-control adaptations, while retaining the accepted explicit capability assertion.ec6f0819c9e4e4066e8969f092328615bb286925599b6f92c733eba4b999c122.