test: add marketplace wallet journey - #1220
Conversation
This comment has been minimized.
This comment has been minimized.
b296556 to
496b457
Compare
496b457 to
0a0f7c8
Compare
piotr-iohk
left a comment
There was a problem hiding this comment.
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.xmlpasses.- Compose identifier tests in
PaymentRequestsScreenTestmatch 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 addsPaymentRequestDismiss-<id>). - PR body QA notes cite request
5db852f6-…/ txa3a427ca…. README records different accepted runs (ad1a8463-…, then selector replayb7f67854-…). 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.
0a0f7c8 to
9552ab5
Compare
jvsena42
left a comment
There was a problem hiding this comment.
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?
9552ab5 to
419503d
Compare
|
@jvsena42 I pinned the marketplace driver and Locks harness to |
jvsena42
left a comment
There was a problem hiding this comment.
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 ContactPaymentsSwitch → ContactPaymentsToggle 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.
419503d to
8ed5247
Compare
8ed5247 to
cbc88a4
Compare
6df11ed to
04d0720
Compare
jvsena42
left a comment
There was a problem hiding this comment.
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.kt— only twotestTagadditions (PubkyAuthAuthorize,PubkyAuthOK). The extra ~10 lines versus what I last reviewed are the requester-label row from master'secb0499e9/75e1b4805, carried in by the rebase, not by this PR.git range-diffshows all six previously reviewed commits patch-equivalent.PaymentRequestsScreen.kt— hyphenated row tag plus a newPaymentRequestPay-<id>;onClickandenabledunchanged.SettingsScreen.kt—switchTestTagrename only, still inside the sameisPaykitEnabled && isPubkyAuthenticatedguard.
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.
onAuthorizestill guarded byuiState.authUrl == authUrl;ContentrendersLoadingunlessisCurrentRequest; Authorize ->requestAuthorize->RequestLocalAuth-> PIN/biometrics ->confirmAuthorize, withLocalAuthMode.Noneonly when neither PIN nor biometrics is enabled. The requesterclientIdand the full permission list still render, nothing truncated. NoBuildConfig, flag or injected fake anywhere in the diff — only tag strings. No auto-approve or auto-dismiss path:Dismissfires only fromviewModel.dismiss()on user Cancel/OK/back. - Scope pinning.
matchesWatchOnlyAccountCapabilitiesrejects any capability set other than exactly the two watch-only paths, withInvalidBitkitClaimCapabilitiesthrown 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.
canGoBackis false atConfirm, so back routes tohideSheet()->clearActiveContactPaymentContext(retry = true), whose defer condition is already false becauseonSheetVisiblecalledmarkPresentedon show. Nothing re-presents ahead of the bell tap.dismissEnabledis false only during HW signing or an unresolved broadcast, neither of which applies pre-swipe. Dismissal never rejects —rejectis called only from the Dismiss button,acceptonly after send — so the request stays in_pendingRequestsand 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.xmlresolves at head — the fourPubkyAuth*ids,ContactPaymentsToggle,Contact_<pk>,PaymentRequestsBell,PaymentRequestsSheet,PaymentRequestRow-/PaymentRequestPay-,ReviewAmount,ReviewContactRecipient(insideOnChainDetails, behind the show-details toggle, so step 34 is accurate),GRAB,SendSuccess,ActivityAmount/ActivityTxDetails. A sweep found no un-hyphenatedPaymentRequestRowreference 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 = 1and 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.
Closes #1212
This PR:
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 at0a0f7c8b. A later replay at pre-rebase headcbc88a40reached two fresh Android identities, a funded buyer, and seller authorization, where Paykit Android rc51 rejected the pinned fixture's legacypubkyauth://signinrequest because rc51 requires grant auth. Pre-merge head04d0720f8corrects the reviewed request-bell sequence. Current headb122caa6fpreserves 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
PaymentRequestRow-<id>and addsPaymentRequestPay-<id>.PubkyAuthAuthorize,PubkyAuthOK, andContactPaymentsToggleidentifiers.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.Preview
bitkit-android-1212-journey.mp4
QA Notes
Manual Tests
5db852f6-d47a-43c4-8b17-fb7a654ac4dd→ review the accepted fallback seller identitypubky…apfiy, 15,000 sats, and 141-sat fee → swipe once → verify transactiona3a427ca69a3f1d3f26fb47f38182c8543fdbd997227ddb8a2dec1ef0e927166at zero confirmations → mine one regtest block → verify confirmed Android activity, paid request history without a Pay action, confirmed Paykit delivery, and completed Locks purchase.cbc88a40created sellerpubky86…gfhdz4yand buyerpubkyr7…tu75yby, and the buyer received 1,000,000 sats in transaction21709c35a60fe081b1de0faeaae1ca403ec67cee76587262ccc6e4b37be6e840, confirmed at fixture height 16,424. At seller authorization, Android resolved and delivered the pinned server'spubkyauth://signinrequest, then Paykit Android0.1.0-rc51rejected it withcode=protocol_error, context=only Pubky grant auth URLs are supported. The pinned server at867fc883uses Paykit source6b241878/ Pubky0.8.0and has nosignin_grantsetup path, while rc51 embeds Pubky0.11.0grant 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, andjust lint: passed for pre-merge head04d0720f87d2a11b0cbbb5f12426016baa6c80b2; detekt reported only existing baseline findings outside this PR.git diff --checkandxmllint --noout journeys/pubky-marketplace/wallet-leg.xml: passed at pre-merge head04d0720f87d2a11b0cbbb5f12426016baa6c80b2.b122caa6fbrings in master2f67d5920and resolves the request-row conflict.04d0720f87d2a11b0cbbb5f12426016baa6c80b2with the isolated local-backend routing; APK SHA-256:8180be69e01c93c6e41f0f54a3e9f3a042e3565cd77c7ad358e0a39c47667fa0.just e2eat the pre-merge head reachedvalidateSigningDevReleaseand stopped because no release keystore was configured in that worktree. The debug-signed E2E variant above is the device-test artifact.b122caa6f, at user direction.e21ce4b0bea14e42c0bb437cd8d443deebe6e2ba40488c0b24dbaaad54ec05ec.