Skip to content

test: add marketplace wallet journey - #720

Merged
jvsena42 merged 18 commits into
masterfrom
codex/718-marketplace-journey
Sep 10, 2026
Merged

test: add marketplace wallet journey#720
jvsena42 merged 18 commits into
masterfrom
codex/718-marketplace-journey

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

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

  • Defines the integration-fixture protocol and runtime contract, with producer provenance from pubky/paykit-server#2 at 867fc883.
  • Documents the pre-launch local-E2E Fulcrum route, Paykit activation confirmation, and Bitkit-generated identity requirement for both clean wallets.
  • Records the buyer Home return, fresh-request auto-review, dismissal, visible header bell, and request-sheet sequence on both platforms.
  • Documents the iOS in-sheet back adaptation and scopes Locks content delivery outside this wallet-leg journey.

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

  • The accepted replay established a watch-only seller and linked buyer, received the canonical 15,000-sat request, paid it once, and confirmed it after one authorized block.
  • The replay recorded PaymentRequestsSheet, request-specific Pay, review, broadcast, and confirmed activity evidence.
  • Record the buyer Home return, fresh-request automatic review, dismissal, visible header bell, and return to PaymentRequestsSheet on the current head.

Automated Checks

  • Current head 3f14a2d7 is based on 1cf0a425; this review fix documents the complete composite request-row identifier while preserving the historical replay identifiers. Validation was not run at user direction.
  • Prior head e3020496: every journey XML file parses with xmllint; the marketplace flow has 30 actions and matches Android head 04d0720f after the required testTag/id and back-control adaptations, while retaining the accepted explicit capability assertion.
  • Existing sanitized H.264 replay was retained; SHA-256 ec6f0819c9e4e4066e8969f092328615bb286925599b6f92c733eba4b999c122.
  • The fixture-setup review is covered by the documented local-E2E Fulcrum configuration route.
  • GitHub Actions are not being monitored or rerun for this review fix.

@ovitrif
ovitrif marked this pull request as ready for review September 2, 2026 11:21
@greptile-apps

This comment was marked as resolved.

Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated
@ovitrif

ovitrif commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed in 48a85d1: the journey now requires the fixture Fulcrum endpoint at tcp://127.0.0.1:60001 before either first launch and provides the exact xcodebuildmcp local-E2E build command for both simulators. This uses Bitkit’s local-E2E default, so no stored in-app Electrum override is required.

piotr-iohk
piotr-iohk previously approved these changes Sep 2, 2026

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.xml passes.
  • Greptile Electrum note is addressed: E2E_BUILD + E2E_BACKEND=local resolves Electrum to tcp://127.0.0.1:60001 in Env.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.

@ovitrif ovitrif changed the title test: add marketplace wallet-leg journey test: add marketplace wallet journey Sep 3, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread journeys/pubky-marketplace/wallet-leg.xml
Comment thread journeys/README.md Outdated
Comment thread journeys/pubky-marketplace/README.md Outdated

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread journeys/pubky-marketplace/README.md Outdated
Comment thread journeys/pubky-marketplace/README.md
Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:5 binds @AppStorage(PaykitFeatureFlags.uiEnabledKey), the toggle at :88-104 carries testIdentifier: "PaykitUiToggle", the warning alert with the Enable button is at :208-215, and Settings → Advanced → Dev Settings is AdvancedSettingsView.swift:35 gated on Env.isDebug.
  • Capability enumeration — falsifiable and right. PubkyAuthClaim.watchOnlyAccountCapabilities is /pub/paykit/v0/bitkit/server/:rw,/pub/paykit/v0/private/bitkit/server/:rw; displayPath drops the trailing slash and displayAccess renders rw as READ, WRITE, and permissionRow puts both in the tree as plain Text. 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.

Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated
@ovitrif
ovitrif requested a review from jvsena42 September 7, 2026 23:07

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

jvsena42
jvsena42 previously approved these changes Sep 8, 2026
@ovitrif
ovitrif force-pushed the codex/718-marketplace-journey branch from 3a571fd to 77547ed Compare September 8, 2026 12:54
@ovitrif
ovitrif requested a review from jvsena42 September 8, 2026 12:56

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 a ContactPaymentContext, calls handleScannedData, and contactPaymentRoute maps .quickpay/.amount -> .confirm when incomingPaymentRequest != nil, so the send sheet opens on SendConfirmationView. Action 18's "bell visible after dismissal" holds and isn't flaky: SendSheet.onAppear calls markPresentedIfPending, so the 700ms re-presentation triggered by activeSheetConfiguration -> nil finds requestsForPresentation() empty and doesn't reopen the review, while the request stays in pendingRequests so the bell keeps rendering. onDisappear clearing contactPaymentContext is what unblocks the later bell -> Pay path.
  • Bell -> sheet -> Pay actually chains. onPay uses hideSheetBeforePerforming { requestPresentation(request) }, so the sheet is dismissed before requestedPresentationId is set — otherwise the activeSheetConfiguration == nil guard 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 rebasewatchOnlyAccountCapabilities, displayPath dropping the trailing slash, displayAccess rendering rw as READ, WRITE, both rows in the tree as plain Text.
  • receivePrivateMessagesFromLinkedPeers() named at README.md:62 is a real iOS symbol, not an Android-ism carried over.
  • The #715 dependency is real and correctly stated: pubkyauth appears in Info.plist only under LSApplicationQueriesSchemes, never CFBundleURLTypes, so iOS genuinely can't receive the setup URL as a link today — which is what README.md:54-55 lists as required app work. All four listed issues are still open.
  • Structure: xmllint --noout passes, the file matches the <journey>/<description>/<actions> shape of the existing suites, and journeys/README.md:138 registers 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_PUBKY build 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.

Comment thread journeys/pubky-marketplace/README.md
@ovitrif
ovitrif requested a review from jvsena42 September 8, 2026 19:08
@jvsena42

jvsena42 commented Sep 9, 2026

Copy link
Copy Markdown
Member

@ovitrif conflicts

@ovitrif

ovitrif commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

I merged current master into the branch and resolved the journey-index conflict by retaining the upstream 16-journey hardware-wallet count alongside the marketplace entry. The PR is now 0 commits behind and GitHub reports it mergeable at 91717349.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both points from my last pass are closed — verified at head, not just read.

Paykit UI flag precondition. README.md:59-61 and wallet-leg.xml:9-11 now name Settings → Advanced → Dev Settings → Enable Paykit UI (PaykitUiToggle), the confirm alert, and — correctly — place it after both wallets are created but before the identity step, since the profile entry is itself gated on isPaykitUIActive. Re-checked the reachability chain at head: AdvancedSettingsView.swift:7 (@AppStorage("showDevSettings") = Env.isDebug) and :25-35 gate the row, DevSettingsView.swift:88-104 carries the PaykitUiToggle identifier, and the Enable branch is at :210-215.

Action 16 passing vacuously. wallet-leg.xml:20 now enumerates /pub/paykit/v0/bitkit/server and /pub/paykit/v0/private/bitkit/server with READ, WRITE — which matches PubkyAuthRequest.swift:8 watchOnlyAccountCapabilities through the displayPath/displayAccess transforms, pinned by PubkyAuthRequestTests.swift:228,238. It can fail now.

The new PaykitUiEnabledToast assertion is real rather than aspirational: DevSettingsView.swift:213 emits it inside the Enable alert branch, and the identifier reaches the tree via Toast.swift:17ToastView.swift:27 .accessibilityIdentifierIfPresent. The bb4569d7 master sync resolves only journeys/README.md, keeping both the upstream hardware-wallet count and the marketplace row.

Nothing further from me.

@ovitrif
ovitrif requested a review from jvsena42 September 9, 2026 23:30

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One LOW inline. Otherwise clean — I verified the journey step by step against the real code at head rather than assuming the earlier trace still held, and the master sync in this head (91717349..486252ac, 52 files) changed only one accessibility identifier line (PassphraseInput, unrelated).

Verified accurate against real code:

  • Preconditions: PaykitUiToggle (DevSettingsView.swift:103), PaykitUiEnabledToast (:213), and Fulcrum at tcp://127.0.0.1:60001 under E2E_BUILD+E2E_BACKEND=local (Env.swift:195-199, with e2eLocalHost defaulting to 127.0.0.1 at :78-80 — so "no stored override required" holds).
  • Actions 2-6: PubkyAuthWatchOnlyConsent / …Approve / PubkyAuthAuthorize / PubkyAuthOK all at PubkyAuthApprovalSheet.swift:139, 141, 184, 221.
  • Action 8 I probed specifically, because the contact-payments toggle binding defaults false on a clean wallet (GeneralSettingsView.swift:26-31). It's correct as written given the journey's own precondition: CreateProfileView.swift:214 navigates to .payContacts right after createIdentity, and PayContactsView.swift:65-70 calls setEnabled(true) on Continue. Not a finding.
  • Action 17 auto-review (ReviewAmount, SendConfirmationView.swift:184,191); action 18 in-sheet back then PaymentRequestsBell (Header.swift:70); action 19 PaymentRequestsSheet (PaymentRequestsView.swift:231); action 20 row shows seller and note-when-present (title senderName at :44-45, subtitle falling through to request.note at :33-40 — the wording is exact).
  • Action 22 is right to require a details reveal: showDetails defaults false (SendConfirmationView.swift:23), the block is gated at :200, and contactRecipient renders at :360/:477.
  • Action 24 GRAB (SwipeButton.swift:82), action 25 SendSuccess (SendSuccess.swift:90), action 28 ActivityAmount/ActivityTxDetails/StatusConfirmed (ActivityItemView.swift:400, :604, :200).
  • journeys/README.md:138 registers the suite with count 1, matching one journey file.

Parity with synonymdev/bitkit-android#1220: action-for-action identical, 30/30, same order and wording bar the deliberate back-gesture divergence. The asymmetry in the diffs is explained: Android had to add the production testTags its journey needs (PubkyAuthAuthorize, PubkyAuthOK, PaymentRequestPay-<id>, the hyphenated row tag, ContactPaymentsToggle), while iOS master already carried nearly all of them. The two that don't line up on iOS are PaymentRequestPay-<id> (absent, supplied by #721 — already covered by the existing merge-order caveat, not re-filing) and the row identifier below.

Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated
@ovitrif
ovitrif requested a review from jvsena42 September 10, 2026 09:56
@jvsena42
jvsena42 enabled auto-merge September 10, 2026 10:43

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix confirmed at 3f14a2d7, and you resolved it better than I suggested.

I'd proposed describing the identifier as a prefix. You spelled out the full construction instead — PaymentRequestRow-<payment-request-id>-<seller-public-key>-<negotiated-receiver-path>-one-time — in both wallet-leg.xml:36 and README.md:77, and added "Using the seller public key and negotiated receiver path" so the runner knows where the segments come from. That gives whoever automates it an exact selector rather than a matching rule to implement, which is the more useful artifact.

Verified against the real code at this head — PaymentRequestsView.swift:168-172:

let period = request.billingPeriod.map { PaykitSubscriptionTimestamp.string(from: $0.startsAt) } ?? "one-time"
return "PaymentRequestRow-\(request.paymentRequestId)-\(request.counterparty)-\(request.counterpartyReceiverPath)-\(period)"

Segment-for-segment match, and one-time is right for a non-subscription request.

You also left README.md:111 and :127 untouched, which is what I asked — those record the 2026-09-02 replay at selector head 70cd2634, where the identifier genuinely was the short form, so they're accurate history rather than stale docs.

That was my only finding. Clean from my side.

@jvsena42
jvsena42 merged commit 3bb9a7e into master Sep 10, 2026
10 of 11 checks passed
@jvsena42
jvsena42 deleted the codex/718-marketplace-journey branch September 10, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: add the Pubky marketplace wallet-leg journey

3 participants