Skip to content

docs: codify paykit issuer contract - #719

Merged
jvsena42 merged 15 commits into
masterfrom
codex/713-paykit-interop
Sep 10, 2026
Merged

docs: codify paykit issuer contract#719
jvsena42 merged 15 commits into
masterfrom
codex/713-paykit-interop

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Closes #713

Description

Codifies the issuer contract Bitkit applies to one-time Paykit Payment Requests:

  • documents exact lowercase asset, network-specific on-chain identifier, chain-independent Lightning identifier, and JSON endpoint payload requirements
  • centralizes those production gates and aligns endpoint rejection for wrong-network identifiers and non-string bounds across platforms
  • adds a versioned cross-platform fixture covering P2TR, P2WPKH, P2SH, and P2PKH on Bitcoin, testnet, signet, and regtest, plus both Lightning identifiers on every network
  • locks the fixture to production parsing with focused tests and explicit network injection
  • adds the repeatable incoming-request journey and the accessibility identifiers it needs

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

  • The source wallet-leg run completed the canonical regtest path on 2026-08-22: Bitkit presented the incoming request, opened the on-chain payment, broadcast it, and confirmed transaction cc85df0e24b54be353a57700429d144b35264c1af97f3de41c503dc52f1e4792 at height 77318.
  • journeys/payment-requests/issuer-interoperability.xml records the repeatable path with a linked fixture issuer, exact request data, and stable accessibility identifiers.
  • The canonical fixture locks 16 on-chain network/script pairs, both Lightning identifiers on every network, lowercase btc, strict JSON endpoint payloads, wrong-network endpoint rejection, and non-string bound rejection.

Current-head update

  • Merged current master into d0b8ca0 and resolved the Paykit parser and journey-index conflicts.
  • Addressed live-journey feedback in b92860d: clarified server and wallet receiver paths, made the composite row identifier exact, and documented container lookup behavior.
  • Tests, builds, lint/check suites, simulator/device QA, CI reruns, and CI watching were not run for this update by direction.

Earlier validation

Before the current master merge:

  • Paykit issuer, public endpoint, and payment-request suites: 68 passed, 0 failed, 0 skipped.
  • Standard iOS Simulator build passed with XcodeBuildMCP.
  • SwiftFormat lint passed for all changed Swift files.
  • Canonical JSON fixture and journey XML parsed successfully.
  • git diff --check passed.

@ovitrif
ovitrif marked this pull request as ready for review September 1, 2026 23:57
@ovitrif
ovitrif requested a review from ben-kaufman September 1, 2026 23:57
@greptile-apps

This comment has been minimized.

# Conflicts:
#	Bitkit/Views/PaymentRequests/PaymentRequestsView.swift
@ovitrif ovitrif changed the title docs: codify Paykit issuer contract docs: codify paykit issuer contract Sep 3, 2026

@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.

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.

Comment thread Bitkit/Services/PublicPaykitService.swift
Comment thread Bitkit/Services/PaykitIssuerInterop.swift
@ovitrif
ovitrif force-pushed the codex/713-paykit-interop branch from 0f6919b to 2d05152 Compare September 4, 2026 23:40
Comment thread journeys/payment-requests/issuer-interoperability.xml
@jvsena42 jvsena42 mentioned this pull request Sep 7, 2026
6 tasks

@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.

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.network guard in isPayableEndpoint still earns its keep — payableEndpoints(from:) accepts endpoints built outside parseEndpoint (PublicPaykitService.swift:442/:455, PrivatePaykitService+Endpoints.swift:67/:87), and PublicPaykitServiceTests.swift:171-178 exercises exactly that. Not dead code.

One genuine cross-PR conflict inline.

Comment thread Bitkit/Views/PaymentRequests/PaymentRequestsView.swift 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.

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.

@jvsena42 jvsena42 mentioned this pull request Sep 7, 2026
3 tasks
@ovitrif
ovitrif requested a review from jvsena42 September 7, 2026 22:58
ben-kaufman
ben-kaufman previously approved these changes Sep 8, 2026
Comment thread Bitkit/Services/PaykitIssuerInterop.swift
@jvsena42

jvsena42 commented Sep 9, 2026

Copy link
Copy Markdown
Member

Cross-platform journey run — iOS side

Ran journeys/payment-requests/issuer-interoperability.xml on a live iPhone 17 simulator against the Android twin (bitkit-android#1216) on a Pixel_9 emulator, plus the shared fixture suite. All 12 actions pass.

The journey nominally needs an external fixture issuer, and no such harness exists in either repo. It turns out one isn't needed: Bitkit can issue to itself (Contacts → contact → ContactPay → Request → amount → PaymentRequestSend), so the Android build acted as the issuer and iOS as the receiver. Identities: iOS pubky19nuszfku3aqde9s66uihyxx3m8zqudxosk1nyemciwe53pkcwiy ("Paykit QA"), issuer pubkya83fojuspu61rp5no8u9mro99tgmii6gh9j9opby6yu8x79jxjhy ("aaa"), each saved as the other's contact.

Request 018cb054-f24a-4856-b056-681a35873116, 100,000 sats / $66.59. Confirmation presented with the right amount, dismissed, bell showed "1 pending", sheet listed the row, Pay reopened the confirmation, Show Details resolved ReviewContactRecipient to the saved contact "aaa". No payment was completed — the journey stops at the recipient check.

BitkitTests/PaykitIssuerInteropTests: Executed 3 tests, with 0 failures. The fixture JSON is byte-identical to the Android copy (sha256 35f671bb…), and both suites are data-driven over all 48 cases (33 request + 15 endpoint) with matching test names.

Two things worth a look

1. The receiver path is bitkit/wallet, not bitkit/server. journeys/payment-requests/README.md says to link the issuer on receiver path bitkit/server. Bitkit-to-Bitkit actually links on bitkit/wallet — the issuer's paykit logs show PaykitReceiverPath("bitkit/wallet") on both ends, and the row identifier only resolves as:

PaymentRequestRow-018cb054-…-pubkya83fojuspu61rp5no8u9mro99tgmii6gh9j9opby6yu8x79jxjhy-bitkit/wallet-one-time

I deliberately did not change the journey: bitkit/server may well be correct for the third-party fixture issuer the journey actually describes, which I never ran. Worth a word from whoever owns that fixture — if it is also bitkit/wallet, both READMEs need the fix.

2. PaymentRequestConfirm and PaymentRequestRow-* are containers and never appear in snapshot-ui targets. They resolve fine via wait-for-ui --identifier X --predicate exists, which needs the exact string (there is no prefix predicate, so the row assertion can't be checked without knowing the counterparty and receiver path). journeys/README.md already documents this class of gap for .onTapGesture controls; it applies to .accessibilityElement(children: .contain) containers too, and it reads as journey drift when it isn't. Might be worth widening that README note.

Notes, not problems

  • Incoming requests poll on a 30s→60s→120s backoff (AppScene.swift:7) — identical constant to Android. In my run the natural tick hadn't fired after 70s; foreground-cycling the app forced an immediate refresh. Fine, but journeys shouldn't assume prompt arrival.
  • This branch pins paykit-rs 0.1.0-rc46 while Android is on paykit-android 0.1.0-rc51, so the contract is currently verified across a five-RC gap. feat: upgrade paykit to rc51 #697 closes it.
  • Expiry tabs are Tab-1 hour/Tab-1 day/… here vs PaymentRequestExpiryHour/Day/… on Android, and Tab-* is English-only. Not journey-relevant today, but it would break a localized run.

jvsena42
jvsena42 previously approved these changes Sep 9, 2026

@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.

No blocking issues found, worth a follow-up

@ovitrif

ovitrif commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

I clarified both journey notes in b92860d: the canonical server fixture stays on bitkit/server, while a Bitkit-to-Bitkit issuer uses the negotiated bitkit/wallet path. The row step now gives the complete composite identifier formula, and the general journey guide covers .contain containers plus wait-for-ui's exact-match requirement.

@ovitrif
ovitrif requested a review from jvsena42 September 9, 2026 18:58

@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.

Verified the new claims against the code and they hold: the .accessibilityElement(children: .contain) container note (journeys/README.md:64-69) is real — SendConfirmationView.swift:251-252 applies .contain immediately before the conditional SendConfirm/PaymentRequestConfirm identifier; the row-identifier formula at payment-requests/README.md:32 matches PaymentRequestsView.swift:174 including period defaulting to one-time; and the receiver-path claim matches PubkyService.swift:30-33 and :1053.

One inline note where the new exactness and the new caveat pull against each other.

Comment thread journeys/payment-requests/issuer-interoperability.xml Outdated
@ovitrif
ovitrif requested a review from jvsena42 September 9, 2026 22:55

@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.

Reviewed as a funds + trust-boundary change, not a docs change. The docs: label understates the diff: it adds Bitkit/Services/PaykitIssuerInterop.swift (a parser for counterparty-supplied endpoint payloads on the pay path), adds a network gate to PublicPaykitService.parseEndpoint, routes the request gate through the new parser, and edits SendConfirmationView.swift, which runs on the ordinary non-Paykit send path. Flagging that because reviewers skim by type. The production behaviour turned out to be preserving.

No HIGH, no MEDIUM. One LOW inline, which happens to be the root cause of a defect showing up on two sibling PRs — see below.

Delta since my last full review (4d077e4c): b92860d3 (journey docs), d0b8ca00 + a6b46f48 (master syncs), 6fbb7d81 (journey xml). I checked both merges with git show --cc: the only lines differing from both parents are the expected PaykitIssuerInterop.bitcoinAsset / supportedEndpointIdentifiers(…, network: Env.network) substitutions plus the README/journey text. No evil-merge content. PaykitSubscription.swift and PrivatePaykitService+Payments.swift are in the diff only because of those constant substitutions.

Trust boundary — the centre of this PR. PaykitIssuerInterop.swift:28-44 fails closed on empty/whitespace, non-UTF-8, malformed JSON, non-object top level, missing/null/non-string/nested/empty value, and non-string min/max. No field is used in a path, a URL, or a Logger call. min/max are stored on Endpoint and never read for a payment decision anywhere. There is no issuer allowlist and none is needed — trust is the linked-peer + saved-contact relationship, and unknown identifiers fail closed at :16-17.

Amount and destination pinning. terms.amount.valuesats(fromBitcoinAmount:) (PaykitPaymentRequestService.swift:261-282: ASCII digits only, ≤8 significant fractional digits, multipliedReportingOverflow/addingReportingOverflow, > 0) → amountSats, then the UInt64.max/1000 cap at :90. SendConfirmationView displays and executes the identical expression (:182/:189 vs :729/:786), canEditAmount is hard false for a request (:142), and validateIncomingPaymentRequestAmounts re-checks exact equality before pay (:706, :896-903). The diff adds no new read of the amount. Issuer value reaches the pay path only through isPayableEndpoint (PublicPaykitService.swift:472-509: decode(invoice:), isExpired, network mismatch, validateBitcoinAddress).

Ungated path is accessibility-only. SendConfirmationView.swift:251-252 adds .accessibilityElement(children: .contain) plus a conditional identifier reading app.contactPaymentContext?.incomingPaymentRequest, which exists on master regardless of the flag. No hit-testing, gesture or state change; SwipeButton at :235-245 untouched.

Docs vs code: every clause in Docs/paykit-issuer-interoperability.md maps to an enforced guard. The one wording simplification — "lifecycle state is proposed" while the code also admits .accepted (:93) — is Bitkit-local post-acceptance state that an issuer cannot produce, and Android does the identical thing, so it isn't an issuer-facing divergence. Not filing it.

Parity with synonymdev/bitkit-android#1216: fixture byte-identical on both heads (sha256 35f671bb…7a2a5e); docs differ only in the fixture path and the cross-reference sentence. Parser is same-shaped — iOS dedupes via Set-insert then filters, Android filters then distinct(), which commute to identical output; iOS trims Unicode whitespace before JSONSerialization where Android relies on kotlinx ASCII skipping. Structural: iOS PaykitSubscription.swift:480-483 passes Env.network inline rather than threading it through as Android does.

Comment thread journeys/README.md Outdated
@ovitrif
ovitrif requested a review from jvsena42 September 10, 2026 09:56
ben-kaufman
ben-kaufman previously approved these changes Sep 10, 2026
jvsena42
jvsena42 previously approved these changes Sep 10, 2026

@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.

Manually reviewed and cross-tested with synonymdev/bitkit-android#1216

@jvsena42
jvsena42 enabled auto-merge September 10, 2026 10:40

@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.

Fix confirmed at 6ec26998. journeys/README.md:127 now reads:

| Payment Request row | `PaymentRequestRow-<id>` | `PaymentRequestRow-<id>-<counterparty>-<receiverPath>-<period>` |

The Android cell matches what Android actually emits — PaymentRequestsScreen.kt:525 is .testTag("PaymentRequestRow-${request.paymentRequestId}") at synonymdev/bitkit-android#1216 head. The iOS cell was already correct.

The knock-on fixes landed on the two PRs that inherited the wrong form from this table: #720 and #721 both now spell the composite form. Confirmed on each.

That was my only finding here. Clean from my side.

@jvsena42

Copy link
Copy Markdown
Member

@ovitrif conflicts again — this re-conflicted after the d0b8ca0 resolution, so the "conflict-free" note above is stale.

mergeable=CONFLICTING at head 6ec26998. Master moved since (the android twin #1216 merged, and #720 landed).

The journeys/README.md:127 selector fix I confirmed earlier today is still good; it just needs the rebase on top.

# Conflicts:
#	journeys/README.md
@ovitrif
ovitrif dismissed stale reviews from jvsena42 and ben-kaufman via 0e1a7d6 September 10, 2026 14:56

@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.

Conflict resolved — mergeable again at 0e1a7d63, and the resolution is correct.

The selector fix I confirmed survived: journeys/README.md:127 still reads

| Payment Request row | `PaymentRequestRow-<id>` | `PaymentRequestRow-<id>-<counterparty>-<receiverPath>-<period>` |

The conflict was in the journeys registry table, and git show --cc 0e1a7d63 shows it resolved as a union — this PR's payment-requests row and master's pubky-marketplace row (from #720) both retained, each carrying a single-parent marker. No line differs from both parents, so nothing was invented in the resolution. That's the right outcome: both suites stay registered and the counts stay accurate.

Nothing else to re-review.

@jvsena42
jvsena42 merged commit 05c4fe0 into master Sep 10, 2026
11 checks passed
@jvsena42
jvsena42 deleted the codex/713-paykit-interop branch September 10, 2026 18:57
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.

docs: codify Paykit issuer interoperability

3 participants