feat: add private payment requests - #1172
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Greptile SummaryThis PR adds private Paykit payment-request creation, incoming request presentation and history, rejection, identity-scoped suppression, and stricter private endpoint handling. It also upgrades Paykit and adds local E2E homeserver and stale-session initialization support.
Confidence Score: 4/5The identity-change creation race should be fixed before merging because it can create a request remotely while leaving the sender with no success or failure feedback. A successful SDK proposal can be excluded from sentRequests after an identity generation change, and the ViewModel then suppresses the only callback that advances the creation flow. Files Needing Attention: app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt, app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
|
| Filename | Overview |
|---|---|
| app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt | Adds identity-scoped incoming/outgoing request state, proposal and rejection operations, expiry, target eligibility, and presentation persistence; successful proposals can be omitted from local sent state after an identity race. |
| app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt | Orchestrates request polling, automatic/manual presentation, retries, identity transitions, creation, and rejection; its conditional success callback can silently strand a completed proposal. |
| app/src/main/java/to/bitkit/services/PaykitSdkService.kt | Adds payment-request SDK operations, capability discovery, exact identity checks, and stale-session initialization recovery. |
| app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt | Adds amount, note, expiry, recipient selection, and sent-state screens for outgoing requests. |
| app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt | Adds incoming preview and complete incoming/sent request history with pay and reject actions. |
| app/src/main/java/to/bitkit/ui/components/SheetHost.kt | Adds sheet visibility callbacks and dismissal locking for durable request creation. |
| app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestPresentationStore.kt | Persists encrypted, identity-scoped sets of already presented request identifiers. |
Sequence Diagram
sequenceDiagram
participant UI as Receive UI
participant VM as AppViewModel
participant Repo as PaymentRequestRepo
participant SDK as Paykit SDK
participant Peer as Contact
UI->>VM: createPaymentRequest(draft, target)
VM->>Repo: propose(...)
Repo->>SDK: proposePaymentRequest(...)
SDK-->>Peer: queue private request
Repo->>SDK: processPendingPrivateMessages()
Repo-->>VM: "Result<PaymentRequest>"
VM-->>UI: onCreated(request)
Reviews (1): Last reviewed commit: "feat: add private payment requests" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
ba7f745 to
e8183a5
Compare
ovitrif
left a comment
ovitrif
left a comment
There was a problem hiding this comment.
Looks good. Durably queued requests stay confirmed after an identity change, and a consumed private payment list is never reused or replaced by public details.
5120677 to
7877465
Compare
Note this cc. @jvsena42 @piotr-iohk |
As far as paykit-server it would be good to have a staging deployment since paykit e2e tests are run against our staging regtest and also using staging homeserver (as far as I understand that was the plan, see: #1084 (comment))
|
af0aaa4 to
e2516e9
Compare
| PaymentRequestDetailsScreen( | ||
| amountInputViewModel = paymentRequestAmountViewModel, | ||
| initialDraft = paymentRequestDraft, | ||
| onBack = { navController.popBackStack() }, |
There was a problem hiding this comment.
Confirmed on-device issue: Payment Requests → Request Payment opens Receive on the Payment Request form, but that sheet still hard-codes startDestination = ReceiveRoute.QR and then navigateTo(startRoute). The receive QR is therefore behind the sheet page we reach, and the in-sheet back arrow pops onto it instead of closing the sheet.
Could we start the Receive NavHost on the Payment Request subscreen, the same way Send already starts on sheet.route?
NavHost(
navController = navController,
startDestination = startRoute,
)Send already does this:
SendSheet(
...
startDestination = sheet.route,
)NavHost(
navController = navController,
startDestination = startDestination,
)PS. Fixing this will also reach behavior parity with how iOS does this (not device-confirmed, code-based reasoning by looking at how the nav back event is routed there).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Review of the payment request flows. just compile and just test pass; the ComposeUi instrumented lane was run on a Pixel 9 AVD and all 15 new tests pass (after 864f008 fixed CreatePaymentRequestScreenTest querying a merged-semantics node). The 8 other failures in that lane reproduce on master and are unrelated.
Findings inline, roughly most-severe first. The SheetHost dismissEnabled work looks sound to me: programmatic hide() bypasses confirmValueChange, the disabled scrim sits under the sheet, and the inner NavHost BackHandlers still take priority over the swallowed sheet back handler.
| showSheet(Sheet.PaymentRequests) | ||
| } | ||
|
|
||
| fun openIncomingPaymentRequest(id: PaykitPaymentRequestId) { |
There was a problem hiding this comment.
requestedPaymentRequestId can stay set forever, wedging the Pay button.
This bails out when requestedPaymentRequestId != null, and the only things that clear it are onSheetVisible (via markPresented), the request leaving pendingRequests, or the retry give-up.
If openContactPayment -> handleScan aborts before the Send sheet ever becomes visible (e.g. the peer's payload hits the duplicated-BIP21/decoding-error branch that toasts and calls hideSheet), nothing clears it: Pay becomes a silent no-op on every request, and rejectIncomingPaymentRequest returns OperationInProgress for that request.
Worse, hideSheet makes currentSheet null, which retriggers presentNextIncomingPaykitPaymentRequest, which re-selects the same requestedId and re-runs the failing scan - an error-toast loop.
The old code handled this via handlePaymentPreparationFailure(context, error) -> deferPaymentRequestPresentation; that call was dropped.
| onPay: (() -> Unit)? = null, | ||
| onReject: (suspend () -> Result<Unit>)? = null, | ||
| ) { | ||
| val scope = rememberCoroutineScope() |
There was a problem hiding this comment.
Reject runs on the composition scope and gets cancelled mid-flight.
onReject() is launched here on rememberCoroutineScope(), and it ends up in PaykitPaymentRequestRepo.updateRequest, which does:
_pendingRequests.update { requests -> requests.filterNot { it.id == current.id } }
discardExpiredRequestsLocked()
processPendingMessages() // <- actually delivers the rejection to the counterpartyAs soon as that _pendingRequests update lands, this card (and, in the sheet, the whole sheet via LaunchedEffect(requests.isEmpty())) leaves composition. That cancels the scope, and therefore the still-running withContext(ioDispatcher) body - so the outbound rejection delivery is dropped.
Launch this in viewModelScope instead.
| 120.seconds, | ||
| 300.seconds, | ||
| ) | ||
| private val PAYKIT_PAYMENT_REQUEST_PRESENTATION_RETRY_DELAYS = List(14) { 2.seconds } |
There was a problem hiding this comment.
Automatic presentation retry window shrank from ~8.5 min to 28 s.
This went from [30s, 60s, 120s, 300s] to List(14) { 2.seconds }. The 2 s cadence makes sense for the new user-initiated "Pay" path, but the same constant also drives the automatic path that waits for the counterparty to publish an updated private payment list.
Once retryAttempts > size, paymentRequestsForPresentation filters that request out permanently (attempts are only cleared when the request leaves pendingRequests or is marked presented). So an incoming request whose details take >28 s to publish will never auto-surface again for the rest of the process lifetime - and the automatic branch has no user-visible fallback on give-up.
Worth splitting into two schedules.
| FillHeight() | ||
|
|
||
| if (showPaymentRequestButton) { | ||
| SecondaryButton( |
There was a problem hiding this comment.
"Send Payment Request" is not gated on an amount.
This SecondaryButton has no enabled condition, and onClickPaymentRequest passes amountInputUiState.sats.toULong(), which is 0 when the user hasn't typed an amount (perfectly normal in the receive/QR flow).
The user then picks a recipient and taps Send, and PaykitPaymentRequestRepo.propose throws RequestUnavailable (draft.amountSats == 0uL), surfacing the generic "Payment request is unavailable" toast with no hint that the amount was the problem.
Gate the button on amountInputUiState.sats > 0.
| 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.
Paste/search can never resolve a non-contact pubky.
recipients is built by intersecting targets with contacts, and targets themselves derive from pubkyRepo.contacts. The Paste affordance only writes into query, which then filters that same contact-derived list.
So pasting a valid pubky that isn't a saved contact just empties the list with no message, while the placeholder ("Enter pubky") and the prominent Paste button imply arbitrary keys are accepted.
Either resolve a pasted key into a target directly, or show an explicit empty-state explaining that the recipient must be a saved contact.
|
|
||
| return savedKeys.mapNotNull { publicKey -> | ||
| val linked = linkedPaths[publicKey] ?: return@mapNotNull null | ||
| val capable = runSuspendCatching { paykitSdkService.paymentRequestReceiverPaths(publicKey) } |
There was a problem hiding this comment.
O(contacts) SDK round trips on every poll.
paymentRequestReceiverPaths(publicKey) is called serially for each saved contact inside synchronizeLocked, i.e. on every 30/60/120 s refresh, and each call takes PaykitSdkService.operationMutex (and may fetch the peer's receiver marker).
With a sizeable contact list this stalls other Paykit operations behind the refresh loop. The results are effectively static - worth caching, or scoping to linked peers only.
| } | ||
|
|
||
| @Composable | ||
| fun MoneyStack( |
There was a problem hiding this comment.
MoneyStack looks like dead code.
Nothing in app/src references it. Per the repo rules unused code should be removed rather than landed.
| <string name="wallet__payment_requests_empty_headline"><![CDATA[No Payment\n<accent>History</accent>]]></string> | ||
| <string name="wallet__payment_requests_earlier">Earlier</string> | ||
| <string name="wallet__payment_requests_not_now">Not Now</string> | ||
| <string name="wallet__payment_requests_pending_count">%1$d pending payment requests</string> |
There was a problem hiding this comment.
This is a plain %1$d string, so it renders "1 pending payment requests". It is only used as a stateDescription on the bell so the impact is cosmetic, but it should be a <plurals>.
| isSetup.await() | ||
| return operationMutex.withLock { | ||
| handle().listPaymentRequests( | ||
| com.synonym.paykit.PaymentRequestFilter( |
There was a problem hiding this comment.
Inline fully-qualified name - com.synonym.paykit.PaymentRequestFilter should be imported, per the repo rule "ALWAYS add imports instead of inline fully-qualified names".
| canRequestPayment: Boolean, | ||
| onBack: () -> Unit, | ||
| onRequestPayment: () -> Unit, | ||
| onPay: (to.bitkit.repositories.PaykitPaymentRequestId) -> Unit, |
There was a problem hiding this comment.
Inline fully-qualified name: to.bitkit.repositories.PaykitPaymentRequestId is already imported at line 51, so this (and the same parameter on PaymentRequestsList) can just use the simple name.
5 participants
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.