Skip to content

test: add marketplace wallet journey - #1220

Merged
jvsena42 merged 12 commits into
masterfrom
codex/1212-marketplace-journey
Sep 10, 2026
Merged

test: add marketplace wallet journey#1220
jvsena42 merged 12 commits into
masterfrom
codex/1212-marketplace-journey

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Closes #1212

This PR:

  1. Adds the Android wallet journey for a Pubky marketplace purchase, aligned with the iOS journey in test: add marketplace wallet journey bitkit-ios#720.
  2. Adds stable identifiers for watch-only authorization, contact payments, incoming requests, and payment approval.
  3. Records the live 2026-09-02 exact-head acceptance baseline and the first-release provenance for the existing claim and incoming-request surfaces.

Acceptance evidence covers the full controlled journey: seller watch-only claim, reciprocal linked-contact setup, fresh buyer funding, canonical Paykit delivery, payment approval, zero-confirmation detection, broadcast, one-block confirmation, paid request history, and Locks completion. This branch merges Android master e2fa21f86; the approved original recording and acceptance evidence were captured at 0a0f7c8b. A later replay at pre-rebase head cbc88a40 reached two fresh Android identities, a funded buyer, and seller authorization, where Paykit Android rc51 rejected the pinned fixture's legacy pubkyauth://signin request because rc51 requires grant auth. Pre-merge head 04d0720f8 corrects the reviewed request-bell sequence. Merge head b122caa6f preserves the request-row navigation from merged #1186 while retaining the reviewed hyphenated row and Pay-action identifiers. Current head 031314514 incorporates master e2fa21f86, including the #1242 screens-map update, without further conflicts. The current-head manual replay remains unchecked.

Description

  • Documents the controlled two-wallet integration fixture contract, emulator port mappings, evidence requirements, and exact Android buyer actions.
  • Aligns request-row identifiers to PaymentRequestRow-<id> and adds PaymentRequestPay-<id>.
  • Adds PubkyAuthAuthorize, PubkyAuthOK, and ContactPaymentsToggle identifiers.
  • Pins the marketplace driver and Locks harness at ed03a32e, cites merged upstream Paykit Server POC wake2pay from notification #2 for canonical request behavior, and cites merged Android feat: send paykit payment proofs #1178 for incoming-request swipe behavior.
  • Records successful live acceptance identifiers, both device identifiers, zero-confirmation and one-confirmation boundaries, and the intended first release milestone 2.6.0.

Preview

bitkit-android-1212-journey.mp4

QA Notes

Manual Tests

  • 1. Fresh funded Android buyer → receive request 5db852f6-d47a-43c4-8b17-fb7a654ac4dd → review the accepted fallback seller identity pubky…apfiy, 15,000 sats, and 141-sat fee → swipe once → verify transaction a3a427ca69a3f1d3f26fb47f38182c8543fdbd997227ddb8a2dec1ef0e927166 at zero confirmations → mine one regtest block → verify confirmed Android activity, paid request history without a Pay action, confirmed Paykit delivery, and completed Locks purchase.
  • 2. Re-run the full seller authorization and buyer purchase/payment journey for the current PR head. The latest replay at pre-rebase head cbc88a40 created seller pubky86…gfhdz4y and buyer pubkyr7…tu75yby, and the buyer received 1,000,000 sats in transaction 21709c35a60fe081b1de0faeaae1ca403ec67cee76587262ccc6e4b37be6e840, confirmed at fixture height 16,424. At seller authorization, Android resolved and delivered the pinned server's pubkyauth://signin request, then Paykit Android 0.1.0-rc51 rejected it with code=protocol_error, context=only Pubky grant auth URLs are supported. The pinned server at 867fc883 uses Paykit source 6b241878 / Pubky 0.8.0 and has no signin_grant setup path, while rc51 embeds Pubky 0.11.0 grant auth. The request, payment, and confirmation boundaries require a compatible marketplace fixture.

The checked replay is separate from the initial and selector-specific acceptance runs in README.md; its request and transaction identifiers are intentionally distinct. The Preview remains historical evidence until manual test 2 replaces it with a current-head recording.

Automated Checks

  • PaymentRequestsScreenTest.kt: all five focused Compose tests passed at the original reviewed head, including the incoming row and Pay identifiers.
  • just compile, just test, and just lint: passed for pre-merge head 04d0720f87d2a11b0cbbb5f12426016baa6c80b2; detekt reported only existing baseline findings outside this PR.
  • git diff --check and xmllint --noout journeys/pubky-marketplace/wallet-leg.xml: passed at pre-merge head 04d0720f87d2a11b0cbbb5f12426016baa6c80b2.
  • All nine pre-merge branch commits have valid signatures; signed merge commits b122caa6f and 031314514 bring in master through e2fa21f86 and resolve the request-row conflict.
  • The E2E debug build passed at pre-merge head 04d0720f87d2a11b0cbbb5f12426016baa6c80b2 with the isolated local-backend routing; APK SHA-256: 8180be69e01c93c6e41f0f54a3e9f3a042e3565cd77c7ad358e0a39c47667fa0.
  • just e2e at the pre-merge head reached validateSigningDevRelease and stopped because no release keystore was configured in that worktree. The debug-signed E2E variant above is the device-test artifact.
  • No tests, validation builds, manual QA, recordings, device launches, CI triggers, reruns, or watching were performed for merge heads b122caa6f and 031314514, at user direction.
  • Preview video SHA-256: e21ce4b0bea14e42c0bb437cd8d443deebe6e2ba40488c0b24dbaaad54ec05ec.

@greptile-apps

This comment has been minimized.

@ovitrif
ovitrif marked this pull request as draft September 2, 2026 10:05
@ovitrif
ovitrif force-pushed the codex/1212-marketplace-journey branch from b296556 to 496b457 Compare September 2, 2026 10:37
@ovitrif
ovitrif force-pushed the codex/1212-marketplace-journey branch from 496b457 to 0a0f7c8 Compare September 2, 2026 10:52
@ovitrif
ovitrif marked this pull request as ready for review September 2, 2026 11:42
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 identifiers + journey contract. Did not re-run the isolated marketplace fixture.

Checked latest (0a0f7c8b):

  • Cited IDs exist on this branch: PubkyAuthWatchOnlyConsent / Approve / Authorize / OK, ContactPaymentsToggle, PaymentRequestsSheet, PaymentRequestRow-<id>, PaymentRequestPay-<id>, ReviewAmount, ReviewContactRecipient, GRAB, SendSuccess, ActivityAmount, ActivityTxDetails.
  • xmllint --noout journeys/pubky-marketplace/wallet-leg.xml passes.
  • Compose identifier tests in PaymentRequestsScreenTest match the hyphenated tags.
  • CI green, including e2e-tests-staging - pubky_paykit.

Notes, not blockers:

  • Merge #1217 first. This PR’s PaymentRequestRow- / PaymentRequestPay- tags are a subset of #1217 (that one also adds PaymentRequestDismiss-<id>).
  • PR body QA notes cite request 5db852f6-… / tx a3a427ca…. README records different accepted runs (ad1a8463-…, then selector replay b7f67854-…). Fine if those are separate replays — worth one line so we do not treat it as a mismatch later.
  • README says #1209 “preserves rejected incoming requests as visible history.” On #1217, parse rejects stay log-only; the visible-history / retry path is the explicit Pay terminal toast. Wording is slightly off.

I am not blocking on not replaying the Locks/Paykit Server fixture here. The journey is documentation of an accepted 2026-09-02 run, and the app-side selectors it needs are present.

@ovitrif ovitrif changed the title test: prove marketplace wallet journey test: add marketplace wallet journey Sep 3, 2026
@ovitrif
ovitrif force-pushed the codex/1212-marketplace-journey branch from 0a0f7c8 to 9552ab5 Compare September 5, 2026 02:01
@ovitrif
ovitrif requested a review from piotr-iohk September 5, 2026 02:21
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.

Selector-parity pass looks right — I checked every tag the journey uses against bitkit-ios and all of them resolve there except one. Two notes below.

Also: the README pins Paykit Server 867fc883, the adb port maps and the homeserver key, but not the marketplace driver / Locks harness, so nobody else can replay this end to end yet. Is there a ref you can pin, like hardware-wallet/README.md does with bitkit-docker?

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

ovitrif commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

@jvsena42 I pinned the marketplace driver and Locks harness to BitcoinErrorLog/pubky-marketplace commit ed03a32e. That immutable payments-env ref includes scripts/verify.sh for the Locks, Paykit, Pubky, bitcoind, and Fulcrum path.

@ovitrif ovitrif self-assigned this Sep 7, 2026
@ovitrif
ovitrif requested a review from ben-kaufman September 7, 2026 16:15
Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated
@ovitrif
ovitrif requested a review from ben-kaufman September 8, 2026 14:01
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.

Two findings inline, both on the precondition block. Doc-only, so neither blocks.

Scope note: the title says test: but three production files change. I checked each against master rather than against my previously reviewed head, because the rebase makes the raw diff misleading:

  • PubkyAuthApprovalSheet.ktonly two testTag additions (PubkyAuthAuthorize, PubkyAuthOK). The extra ~10 lines versus what I last reviewed are the requester-label row from master's ecb0499e9/75e1b4805, carried in by the rebase, not by this PR. git range-diff shows all six previously reviewed commits patch-equivalent.
  • PaymentRequestsScreen.kt — hyphenated row tag plus a new PaymentRequestPay-<id>; onClick and enabled unchanged.
  • SettingsScreen.ktswitchTestTag rename only, still inside the same isPaykitEnabled && isPubkyAuthenticated guard.

So it is genuinely test-hook work and the consent gate is untouched. Worth stating explicitly since a consent-sheet change under a test: heading is exactly what gets waved through.

Delta since my last pass (ba75c85e8, 04d0720f8): XML only — "Return the buyer wallet to Home" and moving the bell check after the auto-review is dismissed. Both address ben-kaufman's threads exactly without over-correcting, and the code agrees: auto-presentation is blocked while currentSheet != null but not by the current route, and the bell exists only on Home, so being on Home before the purchase is what makes it reachable after back.

Checked and clean:

  • Consent gate unchanged. onAuthorize still guarded by uiState.authUrl == authUrl; Content renders Loading unless isCurrentRequest; Authorize -> requestAuthorize -> RequestLocalAuth -> PIN/biometrics -> confirmAuthorize, with LocalAuthMode.None only when neither PIN nor biometrics is enabled. The requester clientId and the full permission list still render, nothing truncated. No BuildConfig, flag or injected fake anywhere in the diff — only tag strings. No auto-approve or auto-dismiss path: Dismiss fires only from viewModel.dismiss() on user Cancel/OK/back.
  • Scope pinning. matchesWatchOnlyAccountCapabilities rejects any capability set other than exactly the two watch-only paths, with InvalidBitkitClaimCapabilities thrown before the Authorize screen renders — so step 15's vaguer wording can't wave through an over-scoped grant even though the text is loose.
  • Back on the auto-review behaves as the journey says. canGoBack is false at Confirm, so back routes to hideSheet() -> clearActiveContactPaymentContext(retry = true), whose defer condition is already false because onSheetVisible called markPresented on show. Nothing re-presents ahead of the bell tap. dismissEnabled is false only during HW signing or an unresolved broadcast, neither of which applies pre-swipe. Dismissal never rejects — reject is called only from the Dismiss button, accept only after send — so the request stays in _pendingRequests and the bell, row and Pay all render.
  • Pay tag placement matches the README's "paid request remained as history without Pay or Dismiss": the button row renders only when onPay != null || onReject != null, and history rows pass neither.
  • Every tag in wallet-leg.xml resolves at head — the four PubkyAuth* ids, ContactPaymentsToggle, Contact_<pk>, PaymentRequestsBell, PaymentRequestsSheet, PaymentRequestRow-/PaymentRequestPay-, ReviewAmount, ReviewContactRecipient (inside OnChainDetails, behind the show-details toggle, so step 34 is accurate), GRAB, SendSuccess, ActivityAmount/ActivityTxDetails. A sweep found no un-hyphenated PaymentRequestRow reference left anywhere.
  • Step 19 is accurate: a clean wallet completing in-app profile creation ends with setEnabled(true) on contact payments, so "verify enabled" holds for the standard flow.
  • Trust boundaries: nothing newly rendered from counterparty data; the requester row that arrived from master is bounded with maxLines = 1 and ellipsis.

Cross-repo with synonymdev/bitkit-ios#720: 30/30 actions in identical order once id->testTag is normalised. The remaining divergences are the platform-appropriate back wording, and action 4 — iOS now enumerates /pub/paykit/v0/bitkit/server and /pub/paykit/v0/private/bitkit/server with READ, WRITE, where this file still says "only the requested Paykit capabilities". The behaviour is identical on both platforms, so this is now the weaker file and the enumeration is worth porting here rather than weakening iOS.

Also stale here: the README still says PaymentRequestPay-<payment-request-id> is Android-only and that an iOS buyer must find its Pay control by accessible label. synonymdev/bitkit-ios#721 supersedes that.

On why this PR needs production changes when iOS needs none — iOS master already carried every identifier the journey uses and Android didn't, so this is pure instrumentation catch-up. That asymmetry is expected, not a smell.

Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated
Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated
@ovitrif
ovitrif requested a review from jvsena42 September 8, 2026 19:10

@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.

Clean — no findings.

Reviewed as an authorization change, not a test change: the test: label understates it, since three production files change and one of them is the auth consent sheet. Worth stating so it isn't waved through on the label — but also so it isn't over-escalated, because the verdict is that only testTag strings changed.

Consent sheet (PubkyAuthApprovalSheet.kt) — two tag additions, no behaviour change:

  • :355 .testTag("PubkyAuthAuthorize") on the Authorize button, :434 on the success button. No onClick, state, effect, flag, BuildConfig or injected-fake path changed.
  • Requester pinning intact end to end: onAuthorize guards uiState.authUrl == authUrl (:85); Content renders Loading unless isCurrentRequest (:208); requestAuthorize rejects unless state.authUrl == authUrl && state == Authorize (VM :104); transitionToAuthorizing re-checks the pin under CAS (VM :237-243); authorize() re-parses the same pinned string and bails if it moved (VM :170); validatedApprovalClientId throws RequesterChanged if the approved clientId differs from the displayed one (PaykitSdkService.kt:1021-1026).
  • Local auth still sits between the tap and the grant (RequestLocalAuth → PIN/biometrics → confirmAuthorize); None only when neither PIN nor biometrics is configured (:192-196). Disclosure unchanged: requester row :388, full permission list :465, TrustWarning :398. Nothing truncated or reordered.

SettingsScreen.kt:346switchTestTag = "ContactPaymentsToggle", still inside if (state.isPaykitEnabled && state.isPubkyAuthenticated) (:339). Nothing on the ungated settings path changed.

Journey verified against the merged code, not the pre-merge trace. The two merge commits brought a 214-line master delta into PaymentRequestsScreen.kt (#1217/#1186: card-level onClick, title now displayContact.name, onRejectonDismiss, subscriptions) and rewrote AppViewModel — which is exactly where steps 17-21 could have silently broken. They didn't:

  • All 18 tags resolve at head, PascalCase and exact.
  • Step 4's wording is what actually renders: displayPath strips the trailing slash (PubkyAuthRequest.kt:54), rw"READ, WRITE" (:57-63), and matchesWatchOnlyAccountCapabilities (:26) pins the set to the two bitkit/server paths, so over-scope fails before the Authorize screen.
  • Step 8 holds: profile creation routes to PayContacts, whose continueToProfile calls setEnabled(true) (PayContactsViewModel:43).
  • Step 18 holds post-merge: system back → SheetHost BackHandlerhideSheet()clearActiveContactPaymentContext(retry = true), which defers only if requestedPaymentRequestId == id (null for auto-presented) or the id is in automaticPendingRequests() (which excludes presented ids). No re-presentation, and the bell still reads raw _pendingRequests, which stays populated because dismissal never rejects.
  • Step 20 holds: subtitle is subscriptions.nameFor ?: note ?: date (:502), so "note when present" is exact.

Parity with synonymdev/bitkit-ios#720: 30/30 actions, identical order and selector names; remaining divergences are platform-appropriate (testTag vs id, "Press Android back" vs "Tap back"). iOS touches only journey files because its master already carried most identifiers, whereas Android lacked PubkyAuthAuthorize, PubkyAuthOK, PaymentRequestPay-<id>, the hyphenated row tag and the ContactPaymentsToggle name. So the production edits here are instrumentation catch-up, and the asymmetry is expected rather than suspicious.

Gating: SettingsStore.kt:42 defaults isPaykitEnabled false, written only by the Dev Settings toggle — dev/QA-facing today.

jvsena42
jvsena42 previously approved these changes Sep 10, 2026
@jvsena42
jvsena42 enabled auto-merge September 10, 2026 10:44
ben-kaufman
ben-kaufman previously approved these changes Sep 10, 2026
@jvsena42

Copy link
Copy Markdown
Member

@ovitrif conflicts

@ovitrif
ovitrif dismissed stale reviews from ben-kaufman and jvsena42 via 6e02b8b September 10, 2026 18:07
@jvsena42
jvsena42 merged commit 2a86a8d into master Sep 10, 2026
18 checks passed
@jvsena42
jvsena42 deleted the codex/1212-marketplace-journey branch September 10, 2026 18:36
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: prove the android pubky marketplace wallet leg

4 participants