Skip to content

test: add marketplace wallet journey - #1220

Open
ovitrif wants to merge 10 commits into
masterfrom
codex/1212-marketplace-journey
Open

test: add marketplace wallet journey#1220
ovitrif wants to merge 10 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 2f67d5920; 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. Current head b122caa6f preserves the request-row navigation from merged #1186 while retaining the reviewed hyphenated row and Pay-action identifiers. 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 commit b122caa6f brings in master 2f67d5920 and resolves 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 head b122caa6f, 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
@ovitrif
ovitrif requested a review from jvsena42 September 7, 2026 16:15

@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 419503d. No HIGH/MEDIUM, nothing to file.

The thing I wanted to rule out on a "test:" PR that touches three production files is a consent gate loosened to make a journey pass. It isn't: the PubkyAuthApprovalSheet diff is two testTag additions (PubkyAuthAuthorize on the Authorize button, PubkyAuthOK on the success button) with no onClick, state, or effect change. onAuthorize is still guarded by if (uiState.authUrl == authUrl), Content still renders ApprovalState.Loading unless isCurrentRequest, and local auth still sits between requestAuthorize and confirmAuthorize — so no auto-approve, no approving a stale or different request than the one displayed. I also grepped app/src/main for the diff looking for anything keyed off BuildConfig, a flag, or an injected fake that only exists for tests: nothing, only tag strings changed.

PaymentRequestsScreen is the PaymentRequestRow-${id} / PaymentRequestPay-${id} hyphenation only — lazyListKey is untouched, so no reorder or item loss, and no other reference to the old un-hyphenated tag exists at head. SettingsScreen is a ContactPaymentsSwitchContactPaymentsToggle rename, still inside if (state.isPaykitEnabled && state.isPubkyAuthenticated); the remaining ContactPaymentsSwitch hits are in SettingsSwitchRowTest, which passes its own switchTestTag, so that's self-consistent rather than a dangling rename.

On the earlier threads — I checked the pushed fixes are correct, not just present. ben-kaufman's point on wallet-leg.xml:27: the journey now waits for auto-review, backs out, taps the bell, then asserts the sheet/row/Pay. That holds because pendingPaymentRequests reads the raw _pendingRequests, not the presented-filtered view, and dismissing the send sheet neither accepts nor rejects (accept only fires on send completion) — so the request really is still pending when the bell is tapped. My own two: all 15 id " are now testTag " with none left, and the README correctly documents PaymentRequestPay-<id> as Android-only with the accessible-label fallback for an iOS buyer.

Every selector in wallet-leg.xml resolves against real code at head, so the journey isn't encoding a flow the app doesn't have. All dev/QA-facing today — isPaykitEnabled defaults false and is Dev-Settings-only.

@ovitrif
ovitrif force-pushed the codex/1212-marketplace-journey branch from 419503d to 8ed5247 Compare September 7, 2026 23:06
@ovitrif
ovitrif requested a review from jvsena42 September 7, 2026 23:07
@ovitrif
ovitrif force-pushed the codex/1212-marketplace-journey branch from 8ed5247 to cbc88a4 Compare September 8, 2026 13:41
@ovitrif
ovitrif requested review from ben-kaufman, jvsena42 and piotr-iohk and removed request for ben-kaufman, jvsena42 and piotr-iohk September 8, 2026 13:44
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
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