Skip to content

test: add marketplace wallet journey - #720

Open
ovitrif wants to merge 14 commits into
masterfrom
codex/718-marketplace-journey
Open

test: add marketplace wallet journey#720
ovitrif wants to merge 14 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 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 77934b54 is based on 60e75e18; validation was not run for this documentation-only review fix 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
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