Repository navigation
chore: adopt Paykit 0.1.0 - #991
Conversation
|
Note this depends on Homeserver 0.16.2 deployment |
[Medium impact] No actionable defect was found in the dependency-pin changes. SummaryUpdates Paykit from
Acknowledged deferrals: ben-kaufman explicitly keeps consumer acceptance, runtime tests, upgrade journeys, device checks, live payment scenarios, and the required Homeserver rollout outside this PR. Reviews (1) · Last reviewed commit: "chore: adopt paykit 0.1.0" · Reviewed by Greptile |
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Pins Paykit to exact version 0.1.0 (revision dd8e75e) in the project and in Package.resolved. Reviewed 8be4a53, light tier. I reasoned from the code, the upstream tag diff and CI. iOS does not build on this reviewer's Linux host.
What I checked, and 4 candidates I ruled out
Read in full: the project.pbxproj hunk (exactVersion requirement) and the paykit-rs pin in Package.resolved
Upstream: git diff v0.1.0-rc77 v0.1.0 in pubky/paykit-rs. There are 26 files. paykit-ffi/src has no changes, and the changelog says the Swift and Kotlin binding APIs are unchanged.
CI: validate is green. Run Tests and Run Integration Tests were still running when I reviewed.
Ruled out
- Pin and tag disagree:
v0.1.0resolves todd8e75e5588cf7a0b3b45cacbab9125576ee90dfupstream, which matchesPackage.resolved. That tag'sPackage.swiftcarries checksum7e967980...c1b933, the value the PR body says the XCFramework ZIP was verified against. - Stale
originHash: the previous rc bumps (67bae1f,e3796d9) also changed only the paykit pin and leftoriginHashalone. CI resolves with-onlyUsePackageVersionsFromResolvedFile, so a mismatch would fail resolution in Run Tests and not pass silently. - Binding API drift: no line under
paykit-ffi/srcchanged between the two tags, so the generated Swift surface is the same. - Behaviour change hidden in a version bump: the real change is in
paykit-sdk/src/storage/pubky_shared.rs. A pending write is now reconciled by republishing the exact ciphertext under the lock, replacing the five-minute cooldown. It also requires Homeserver 0.16.2+. That is a deployment precondition, not a code defect in this diff, and the author already called it out on the PR.
Merge confidence: 4/5, the test jobs had not finished, and the shared-state reconciliation path depends on the homeserver version with no test here covering it.
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full review of the complete PR diff against merge base bd099662, at 8be4a537. No saved assessment baseline was available.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The exact pin matches signed tag v0.1.0 (dd8e75e5588cf7a0b3b45cacbab9125576ee90df). That tag's Package.swift records XCFramework checksum 7e967980432855bf5ed3b031c7725aa0c32f8535f91835e3814c3823c7c1b933; this review did not re-hash the zip. Swift FFI sources are unchanged from 0.1.0-rc77 (1d2ca711). Shared-state recovery changes: 0.1.0 removes the five-minute uncertain-write cooldown and republishes the observed ciphertext under the lock. That publication fences older writes on Homeserver 0.16.2 with GCS.
The author's note and the merge hold already name the homeserver deployment. The pinned app still assigns homeservers through Homegate, and Docs/pubky-auth-companion-claims.md still specifies Homeserver 0.15 and the five-minute cooldown. Paykit Server 14ae4a72 still builds paykit 03d4f057 on Pubky 0.15.0, which keeps that cooldown. bitkit-android master 722c824 still pins 0.1.0-rc77. Those repositories were compared only for this recovery contract. bitkit-android#1555 was not reviewed.
Validation: translation validation passed on this revision. build-local, Run Tests, and Run Integration Tests were still pending. The author reports an unsigned generic iOS build with locked package resolution; this review did not compile the app or run those tests.
Device testing: not performed in this review.
Suggested additional test cases
- Platform: iOS, Paykit UI enabled, disposable identity.
- Setup: leave a pending
/pub/paykit/v0/shared-state-writes/marker from an uncertain shared-state write, on a homeserver that is still short of verified 0.16.2 GCS, with Paykit Server on03d4f057. - Action: open Contacts or Profile so Bitkit loads shared state.
- Expected: the marker remains, and contacts, payment requests, and backup records stay free of the in-flight write. After every serving instance is 0.16.2 GCS, older writes are drained, and co-writers use this same recovery, repeat the load: it finishes without a five-minute wait and keeps the latest committed state.
Findings
- [MEDIUM] Uncertain shared-state recovery clears markers before the current homeserver can fence them — inline at
Bitkit.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved:36.
There was a problem hiding this comment.
Verdict: ✅ Approve
Review: diff 2 files. No findings. Under Paykit 0.1.0 (Pubky SDK 0.16.2, pubky-noise 0.1.0), sessions stored by the rc77 build still restore, and the new conflict and auth-link errors land in Bitkit's existing handling.
Pair PR synonymdev/bitkit-android#1555 pins com.synonym:paykit-android to 0.1.0 and matches.
QA for 8be4a53:
Tested on iOS 26.5 simulators (iPhone 17 Pro) against the staging homeserver (0.16.2), with a Debug build of this head and, for the update, a build of the base bd09966 (Paykit 0.1.0-rc77).
🟢 Tests 1, 2, 3
Test 1
Updating from the rc77 build keeps the same pubky signed in with its profile and contact, Contacts opens straight away, and a contact tag saved after the update is still there after a relaunch. In the second recording the old app was stopped while it held the shared-state lock (during a wallet backup), so the first launch shows Try Again for about a minute until the lock lease runs out; the rc77 build does the same after the same stop (third recording), so this PR does not change it. One other update, with the old app idle, showed the same one-minute wait; I did not isolate what held the lock there.
test1-update-from-rc77.2x.mp4 | test1-tag-after-update.2x.mp4 | control-rc77-lock-wait.2x.mp4 |
Test 2
A new profile and a contact created on this build are saved and still there after a relaunch; the contact after relaunch is the last screen of Test 3's recording, on the same wallet.
test2-new-profile-head.2x.mp4 |
Test 3
After granting a test client Paykit access in the Authorize sheet, I planted an empty marker under /pub/paykit/v0/shared-state-writes/. The app cleared it within 13 s with no five-minute wait, and a contact tag saved right after took 2 s and survives a relaunch. The test client read the marker listing from the homeserver.
test3-pending-marker.2x.mp4 |
Not driven:
- @piotr-iohk's suggested case on a homeserver short of 0.16.2 with GCS: Paykit
0.1.0requires 0.16.2 with GCS, accepted as a known low risk (thread). Test 3 drives the case's post-rollout half.
Relied on:
- @piotr-iohk's finding on the homeserver precondition and
Docs/pubky-auth-companion-claims.md. - @talosmachina's review for the pin, checksum and binding sources.
Covered: a full review of this PR's diff and the Paykit 0.1.0-rc77 to 0.1.0 changes it pulls in. No device case drove the HPKE auth-link rejection (it ends in the existing "Invalid auth request" toast), the HTTP 409 conflict mapping or the encrypted-link retry classification.
|
@ben-kaufman on the note about the Homeserver 0.16.2 deployment: the same Paykit 0.1.0 adoption on Android passes its basic flows against a homeserver on 0.16.1. The flows were identity and restore, contacts, requests both ways and an app restart. Results: synonymdev/bitkit-android#1555 (comment) This PR's iOS build was not run against 0.16.1. |
This PR adopts the released Paykit 0.1.0 Swift package.
Description
0.1.0and release revisiondd8e75e5588cf7a0b3b45cacbab9125576ee90dfso the app uses the canonical published package.Design
N/A - no UI changes.
Preview
N/A
QA Notes
Journeys
N/A - no user-visible behaviour change in this dependency-only diff.
Manual Tests
N/A
Automated Checks
Integration Requirements
The public XCFramework ZIP was downloaded and verified against
Package.swift: SHA2567e967980432855bf5ed3b031c7725aa0c32f8535f91835e3814c3823c7c1b933.Keep this PR draft pending consumer acceptance. XCTest runtime suites, in-place upgrade journeys, physical-device checks, and live payment/fault scenarios were not run. No tests were added, removed, or consolidated.
Shared-state recovery requires Homeserver 0.16.2+ with the required GCS behavior on every serving instance, drained older writes, and coordinated writers. This dependency change does not perform that rollout. Caller authorization, durable storage, deadlines, retry policy, and Unknown-payment protections remain required. The release's dependency warnings and independent-crypto/runtime limitations remain applicable.