feat: add private payment requests - #1172
Conversation
This comment has been minimized.
This comment has been minimized.
ba7f745 to
e8183a5
Compare
5120677 to
7877465
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
af0aaa4 to
e2516e9
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Flagging while QA’ing with the staging public-pay pubkys — not a prod issue (Paykit UI is still hidden). Enabling payments with contacts fail-closes with Unknown error if a saved contact’s receiver marker doesn’t parse (here: missing
In principle the same thing could happen with any broken / outdated profile marker, not only these fixtures. Recording attached. Same surface on iOS after #676 (toast there is Private Paykit is not available). Steps:
Screen.Recording.2026-08-26.at.10.12.34.mov |
|
Flagging from QA: paying a payment request that’s larger than the wallet balance. Same setup, two directions, both recordings attached:
Screen.Recording.2026-08-26.at.14.06.24.mov
Screen.Recording.2026-08-26.at.14.18.16.movSteps:
Expected: don’t enter confirm if the amount is above spendable. On Android, Dismiss should still work after a failed Pay (no already in progress loop). |
|
Addressed both QA findings in signed commit
The matching iOS fixes are isolated in synonymdev/bitkit-ios#684: malformed remote markers no longer block other contacts, and incoming request amounts are used during initial LN/on-chain balance validation so the standard insufficient Spending/Savings error appears before coin selection. Local verification: 193 focused Android tests passed; detekt completed successfully with only unrelated pre-existing findings. |
|
Retested after 5d39cd9. Enable — still fails. ❌ Toggle Enable payments with contacts → Unknown error, toggle does not stay on. Same staging contact as before ( This is not the Same user-facing miss on iOS #684 (toast there is Private Paykit is not available). Steps:
Screen.Recording.2026-08-27.at.10.24.05.-.android.movOver-balance — looks good on Android. ✅ Incoming request larger than the wallet: one Insufficient Savings / Spending toast, request stays pending and dismissible. No already in progress loop. iOS #684 still repeats that toast (see that PR). |
jvsena42
left a comment
There was a problem hiding this comment.
Code review
Reviewed git diff master...HEAD at 56ce990 (merge-base 6ae3cb7) — 34 files, ~3.8k added lines. 10 findings inline, roughly in severity order: the ReceiveSheet start-destination change and the dropped firstError in PrivatePaykitRepo look like the two most likely to bite in practice.
Checked and looked correct: the generation/identity guards in PaykitPaymentRequestRepo (activate/refresh/synchronizeLocked/isCurrentState), creationMutex.tryLock with finally unlock in propose, expiration rescheduling, the hideSheet re-entrancy guard, SheetHost's confirmValueChange + rememberUpdatedState pairing, the scrim's pointer-consuming modifier, the stopped/recursion loop in presentNextIncomingPaykitPaymentRequest, and the sats/BTC BigDecimal conversions.
One judgement call I did not flag as a defect: the string-literal match in canDeferStaleSession is brittle against SDK message changes.
| NavHost( | ||
| navController = navController, | ||
| startDestination = ReceiveRoute.QR, | ||
| startDestination = startRoute, |
There was a problem hiding this comment.
Back stack loses ReceiveRoute.QR when the sheet opens at a non-QR route.
Replacing LaunchedEffect(startRoute) { navController.navigateTo(startRoute) } with startDestination = startRoute means QR is no longer the root of the inner NavHost stack.
ContentView.kt:936 (FundingScreen -> "Fund") opens Sheet.Receive(route = ReceiveRoute.Amount). In that flow:
ReceiveAmountScreen'sonBack = { navController.popBackStack() }now returnsfalse— the back arrow is a dead control.ReceiveRoute.Confirm'sonContinuedoesnavigateTo(QR) { popUpTo(QR) { inclusive = true } }. WithQRabsent, thepopUpTois a no-op and the stack becomes[Amount, Confirm, QR], so back from the QR screen lands on Confirm instead of closing the sheet.
| }, | ||
| onSent = { | ||
| createdPaymentRequest = it | ||
| navController.navigateTo(ReceiveRoute.PaymentRequestSent) |
There was a problem hiding this comment.
Duplicate payment request via system back.
onSent navigates to PaymentRequestSent without popping PaymentRequestRecipient, and navigateTo only sets launchSingleTop. The Sent screen has no BackHandler or back arrow, so system back returns to the recipient picker with selectedTarget still set and isCreating == false — "Send Request" is enabled and proposes a second request for the same draft.
Suggest popUpTo(ReceiveRoute.PaymentRequestRecipient) { inclusive = true }.
| val publicationReceiverPaths = receiverPathSelection.publishableReceiverPaths | ||
| receiverPathSelection.error?.let { | ||
| firstError = firstError ?: it | ||
| logPrivateReceiverPathSelectionFailure(publicKey, reason, it) |
There was a problem hiding this comment.
Dropping firstError = firstError ?: it silently swallows total publication failure.
This hunk removes the firstError assignment, so a receiverPathSelection.error is now only logged, never propagated.
If selection fails for every contact, publicationReceiverPaths is empty, so preparation.updates.isEmpty() and the if (requireImmediatePublication) preparation.firstError?.let { throw it } guard sees null and returns success. The requireImmediatePublication = true callers — ContactPaymentSettingsRepo.kt:49, :79, :148, the "share private payment details" toggles — then report success to the user while nothing was published.
| private fun isCurrentState(generation: Long, expectedIdentity: String?): Boolean = | ||
| stateGeneration.get() == generation && PubkyPublicKeyFormat.matches(activeIdentity, expectedIdentity) | ||
|
|
||
| private suspend fun eligibleTargets( |
There was a problem hiding this comment.
eligibleTargets now issues remote lookups on every refresh().
It runs from synchronizeLocked, and per saved contact does paykitReceiverPaths(publicKey) plus a paykitReceiverMarker(publicKey, path) remote read. refresh() is driven by startInitialPaykitPaymentRequestPolling (15 runs at 2s intervals, re-armed on every initial link burst) and then by the 30/60/120s poll loop.
With N contacts that is roughly 15 x N x 2 remote reads inside 30s, on a path that previously only listed local records. Worth caching per identity/generation or computing it off the refresh path.
| } | ||
| return | ||
| } else { | ||
| PAYKIT_PAYMENT_REQUEST_REFRESH_INTERVALS.last() |
There was a problem hiding this comment.
Automatic presentations now retry forever.
The give-up branch above only applies when requestedPaymentRequestId == request.id. For a non-requested (automatic) presentation, once PAYKIT_PAYMENT_REQUEST_PRESENTATION_RETRY_DELAYS (now List(14) { 2.seconds }) is exhausted, attempt saturates at 14 via coerceAtMost and retryDelay falls back to PAYKIT_PAYMENT_REQUEST_REFRESH_INTERVALS.last() (120s) indefinitely.
An incoming request whose private payment details never resolve calls beginPaymentRequest every 2s for ~28s, then every 2 minutes for the request's whole lifetime (up to 30 days). The previous code stopped after 4 attempts.
Related: the requested-branch return at line 869 leaves paymentRequestPresentationRetryAttempts[request.id] at its saturated value. A later "Pay" tap on the same request therefore gives up on its first failure — no retry window, no "waiting for details" toast, and the log misreports the attempt count.
| val idsByIdentity: Map<String, List<PaykitPaymentRequestId>> = emptyMap(), | ||
| ) | ||
|
|
||
| fun load(identity: String): Set<PaykitPaymentRequestId> { |
There was a problem hiding this comment.
Store can never repair a corrupt entry.
Both load (line 28) and save (line 35) call Json.decodeFromString<State>(...) on the raw keychain value with no fallback. If that entry is ever corrupt, or written by a future/older schema, load throws (callers swallow it and see an empty set) and every subsequent save throws too — so the bad value is never overwritten.
Effect: presented-request ids stop persisting permanently, and already-surfaced payment requests are auto-presented again on every app start. save should treat a parse failure as State() and overwrite.
| onClick = { | ||
| if (isRejecting || onReject == null) return@SecondaryButton | ||
| isRejecting = true | ||
| scope.launch { |
There was a problem hiding this comment.
Reject runs in the card's rememberCoroutineScope().
scope is rememberCoroutineScope() from PaymentRequestCard (line 456), so it is tied to the card's composition. onReject is a suspend () -> Result<Unit>; closing the sheet mid-flight cancels it, which can leave the request rejected in the SDK but still shown as pending (and the outbound message unflushed) until the next refresh.
createPaymentRequest already runs this kind of work in viewModelScope; same is warranted here.
| var selectedTarget by remember { mutableStateOf<PaykitPaymentRequestTarget?>(null) } | ||
| var query by remember { mutableStateOf("") } | ||
|
|
||
| val recipients = remember(targets, contacts, query) { |
There was a problem hiding this comment.
Pasting an unknown pubky gives a blank screen with no explanation.
The field is labelled "RECIPIENT" with placeholder "Enter pubky" and a Paste button, but query only filters targets, which are derived from saved, linked contacts. Pasting a valid pubky that is not already a saved and linked contact yields an empty recipients list with no message, so the user cannot tell why the recipient is unusable.
Worth an empty-state explaining that the recipient must be a saved contact with private payment details linked.
| fun MoneyDisplay( | ||
| sats: Long, | ||
| onClick: (() -> Unit)? = null, | ||
| showSymbol: Boolean? = null, |
There was a problem hiding this comment.
Dead parameter, and it is not part of the remember key.
showSymbol has no callers — both existing call sites omit it.
If it is kept, line 34 is a problem: if (showSymbol == null) rememberMoneyText(sats) else rememberMoneyText(sats, showSymbol = showSymbol) creates two distinct composable call sites, so toggling it at runtime discards remembered state. More importantly rememberMoneyText keys on remember(currencies, sats, unit) — showSymbol is not a key, so a future caller that flips it for the same sats/unit gets the stale string.
Either drop the parameter or add showSymbol to the remember key.
| <string name="wallet__payment_request_sent_description">You have sent a payment request</string> | ||
| <string name="wallet__payment_request_sent_headline"><![CDATA[Payment <accent>Requested</accent>]]></string> | ||
| <string name="wallet__payment_request_sent_title">Sent</string> | ||
| <string name="wallet__payment_request_sending">Queued for delivery</string> |
There was a problem hiding this comment.
Three new entries are out of alphabetical order (CLAUDE.md: "ALWAYS add new localizable string resources in alphabetical order in strings.xml"):
wallet__payment_request_sending(this line) sorts beforewallet__payment_request_sent_*, not afterwallet__payment_request_waiting_for_details(line 1209) sorts beforewallet__payment_request_waiting_for_recipientwallet__payment_requests_earlier(line 1213) sorts beforewallet__payment_requests_empty_*
This PR adds private Paykit Payment Requests to Bitkit.
Description
0.1.0-rc44and adds local E2E homeserver configuration plus safe cold-start restoration for externally managed Pubky sessions.The request payload itself remains SDK-backed and durable; Bitkit persists only encrypted, identity-scoped presentation suppression, not a duplicate request queue. Payment proofs and receipts remain out of scope.
Dependencies:
41cda2567226a690a012770017d5e7c1d49e2a2b.Preview
N/A — proof recordings were completed locally and are intentionally not attached to the PR.
QA Notes
Manual Tests
Automated Checks
PaykitPaymentRequestRepoTest.kt: covers mapping, eligibility, proposal delivery, rejection, expiry, identity-scoped presentation state, and action serialization.PaykitSdkServiceTest.kt: covers exact identity enforcement and safe deferred session restoration.AppViewModelSendFlowTest.kt: covers automatic/manual presentation, sheet transitions, identity changes, newer-list retry, strict private resolution, and payment lifecycle races.PaymentRequestExpirationTest.kt: covers expiry selection and retained draft state.SheetHostTest.kt: covers locked sheet dismissal and scrim input isolation during durable proposal creation.git diff --check.