docs: codify paykit issuer contract - #719
Conversation
Greptile SummaryThe PR codifies Bitkit’s existing one-time Paykit issuer contract without changing production parsing behavior.
Confidence Score: 5/5The PR appears safe to merge because the production changes preserve existing validation behavior while adding focused contract coverage and documentation. Production callers continue to use the same environment network and parsing rules, the new files are automatically included through synchronized Xcode groups, and no concrete changed-code failure remains.
|
| Filename | Overview |
|---|---|
| Bitkit/Services/PaykitIssuerInterop.swift | Centralizes the existing asset, network-aware endpoint filtering, deduplication, and JSON payload parsing behavior. |
| Bitkit/Services/PaykitPaymentRequestService.swift | Uses the centralized issuer contract while retaining Env.network as the production default. |
| Bitkit/Services/PublicPaykitService.swift | Delegates endpoint payload parsing to the shared interop helper without changing the resulting endpoint shape. |
| Bitkit/Views/PaymentRequests/PaymentRequestsView.swift | Adds stable accessibility identifiers directly to actionable Pay and Reject buttons. |
| Bitkit/Views/Wallets/Send/SendConfirmationView.swift | Distinguishes ordinary and payment-request confirmation screens in the accessibility tree while preserving child elements. |
| BitkitTests/PaykitIssuerInteropTests.swift | Exercises the production request and endpoint parsers against the versioned cross-platform fixture. |
| BitkitTests/Fixtures/paykit-issuer-interoperability.json | Defines accepted and rejected issuer-contract cases for Bitcoin, testnet, signet, and regtest. |
| Docs/paykit-issuer-interoperability.md | Documents the exact request terms, endpoint identifiers, payload shape, and delivery prerequisites enforced by Bitkit. |
| journeys/payment-requests/issuer-interoperability.xml | Records the repeatable regtest journey from incoming request presentation to payment confirmation. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Issuer Payment Request] --> B[Validate btc asset and one-time terms]
B --> C[Filter endpoint identifiers for wallet network]
C --> D[Present incoming request]
D --> E[Resolve JSON endpoint payload]
E --> F[Open send confirmation]
F --> G[Pay or reject request]
Reviews (1): Last reviewed commit: "docs: codify Paykit issuer contract (#71..." | Re-trigger Greptile
# Conflicts: # Bitkit/Views/PaymentRequests/PaymentRequestsView.swift
jvsena42
left a comment
There was a problem hiding this comment.
Checked the documented contract against what the code actually enforces, and cross-checked the Android port (#1216). The code changes are behaviour-preserving and the doc matches the request gate. I empirically ruled out an expiry-parsing divergence — ISO8601DateFormatter handles 1-9 fractional digits and offsets here.
Two real divergences remain at the endpoint level, which the shared fixture cannot currently see. For the first one, note that iOS matches Docs/paykit-issuer-interoperability.md's two-stage design and Android is the one gating early — so the question is which stage should own the check, not which platform is wrong.
0f6919b to
2d05152
Compare
2d05152 to
ac1468b
Compare
jvsena42
left a comment
There was a problem hiding this comment.
Confirmed ac1468b resolves both threads I opened. The endpoint parser now rejects non-string min/max, and parseEndpoint takes an injectable network: with Env.network as the production default — numeric-min, boolean-max and foreign-network-endpoint are all in the shared fixture.
I diffed the fixture against the Android copy on codex/1208-paykit-interop: byte-identical (sha256 35f671bb…). I also hand-checked the three contract clauses the fixture doesn't cover, and the platforms agree on all of them:
| iOS | Android | |
|---|---|---|
| Amount precision | fraction.count <= 8 after stripping zeros → "0.0000000015" rejected |
toBigIntegerExact() throws → rejected |
| Amount cap | <= UInt64.max / 1000 + per-step overflow guards |
<= ULong.MAX_VALUE / 1000uL |
| Expiry | ISO8601DateFormatter .withFractionalSeconds then fallback |
kotlin.time.Instant.parse |
I ran the Swift parser against 1/2/3/6/9-digit fractional seconds, Z and +02:00 — all accepted, bare no-offset rejected on both sides. Those rules are covered by PaykitPaymentRequestServiceTests rather than the shared fixture, which is a coherent boundary; no reason to grow the fixture.
Two notes for the record, neither needing action here:
- Gating the network at parse means a foreign-network on-chain identifier now yields
.noEndpoint("hasn't shared payment data") instead of.notOpened. Reads better, just flagging the user-visible change. - The
Env.networkguard inisPayableEndpointstill earns its keep —payableEndpoints(from:)accepts endpoints built outsideparseEndpoint(PublicPaykitService.swift:442/:455,PrivatePaykitService+Endpoints.swift:67/:87), andPublicPaykitServiceTests.swift:171-178exercises exactly that. Not dead code.
One genuine cross-PR conflict inline.
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed at caa2484. No HIGH/MEDIUM, nothing new to file.
I treated this as a funds-and-trust-boundary change rather than a docs PR, since it adds an issuer-payload parser and touches SendConfirmationView. The question I cared about: can a hostile or non-conforming issuer payload make the executed payment differ from the confirmed one? It can't. terms.amount.value goes through sats(fromBitcoinAmount:) (unchanged here — digits-only, <=8 decimals, overflow-checked, >0, <= UInt64.max/1000) into amountSats, and SendConfirmationView renders wallet.sendAmountSats ?? invoice.amountSatoshis while performPayment executes the identical expression, after validateIncomingPaymentRequestAmounts re-checks acceptsPaymentAmount and the BOLT11 msat amount. canEditAmount is false for requests, and no normalisation happens after display — display and execution read the same field.
Destination is the same story: the endpoint value is decoded through the existing decode(invoice:) + validateBitcoinAddress + network-mismatch path, and the send uses the same invoice.address/invoice.bolt11 objects the details view renders. The counterparty on ReviewContactRecipient is the local saved contact, not issuer-supplied text.
On the trust boundary specifically — the only issuer strings reaching UI are note (pre-existing, trimmed) and a locally formatted numeric amount, so there's no spoofing surface on the confirmation line. min/max are retained but never read for a payment decision. No issuer field lands in a URL, a path, or a log; PaykitIssuerInterop has no Logger calls at all. No trapping path on hostile input either: try? JSON, as? casts, split(maxSplits: 1) on a non-empty array, and multipliedReportingOverflow/addingReportingOverflow rather than bare arithmetic.
Both pushed fixes are correct, not just present. The network gate at parse (PublicPaykitService.swift:229-230) uses Env.network, which is already LDKNode.Network, so production callers keep their meaning — and I checked the two PubkyService adapter call sites are payment selection, not own-endpoint cleanup, so dropping foreign-network candidates earlier only removes what isPayableEndpoint would have rejected anyway. The non-string min/max guard is right for JSONSerialization output: strings bridge to String, numbers and bools land as NSNumber and fail, NSNull is accepted and decodes to nil — which matches Android's strict isLenient = false decode. The numeric-min/boolean-max fixtures lock both.
The SendConfirmationView diff is reachable on the ordinary non-Paykit send path, so I checked it deliberately: it's a brace reformat plus accessibility modifiers, and .accessibilityElement(children: .contain) doesn't alter hit-testing or the swipe-to-pay gesture. Everything else is dev/QA-facing today (isUIEnabled defaults false).
One behaviour delta already on record: a foreign-network on-chain-only issuer now fails at parse with .noEndpoint rather than later at payability. No persisted format changed, so there's no migration concern from the shipped build.
Closes #713
Description
Codifies the issuer contract Bitkit applies to one-time Paykit Payment Requests:
Paykit protocol and SDK policy remain owned by Paykit.
Linked Issues/Tasks
Preview
N/A — no user-visible changes.
QA Notes
Product journey / regtest evidence
cc85df0e24b54be353a57700429d144b35264c1af97f3de41c503dc52f1e4792at height77318.journeys/payment-requests/issuer-interoperability.xmlrecords the repeatable path with a linked fixture issuer, exact request data, and stable accessibility identifiers.btc, strict JSON endpoint payloads, wrong-network endpoint rejection, and non-string bound rejection.Automated checks
git diff --checkpassed.