Skip to content

fix: republish pubky identity records - #753

Merged
jvsena42 merged 5 commits into
masterfrom
codex/republish-pubky-identity
Sep 17, 2026
Merged

jvsena42 merged 5 commits into
masterfrom
codex/republish-pubky-identity

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Description

This PR keeps the signed Pubky identity record discoverable by periodically republishing it through Paykit 0.1.0-rc55.

  • Reuses the existing foreground maintenance loop and checks on network restoration, identity activation, and auth approval. A locally held identity can be republished before restoring its session.
  • Reuses the SDK client cache, allows one publication at a time, and throttles successful publication to once every 30 minutes. Missing records or failures may retry after one minute on the next eligible trigger.
  • Schedules publication independently during SDK initialization so it does not delay startup. Other callers wait at most five seconds, with a log when that wait expires. Failures do not fail authentication, cancellation before authorization is respected, and no identity record is reconstructed or changed.
  • Keeps the single-flight guard held until a native call finishes, even if it ignores Swift cancellation. Currency refresh runs before this maintenance on network restoration.
  • Adds no background service or UI. Payment polling intervals and public/private payment behavior are unchanged.

Delegated identities without a local key are republished once the app knows their public key after authentication/session restoration. No new persisted identity or throttle state is introduced.

Linked Issues/Tasks

Design

N/A — no UI changes.

Screenshot / Video

N/A — no UI changes.

QA Notes

Manual Tests

  • 1. Signed-in Profile → background and reopen Bitkit: profile/session remains available; repeated foreground transitions do not repeatedly republish a successful record within 30 minutes.
  • 2. Scanner → approve a Pubky auth request: approval still completes when identity republishing fails.
  • 3. Background Bitkit: periodic identity maintenance stops; foreground/network restoration resumes eligible maintenance.

Automated Checks

  • Seven tests in PubkyIdentityRepublishTests.swift cover success throttling/client reuse, missing-record/error retries, identity changes, concurrent triggers, slow-call deadlines/retries, signing-identity publication before ordinary/companion auth approval even on publication failure, and cancellation during publication across ordinary/companion/Ring approval paths.
  • Built against the published rc55 Swift package and binary archive. After the startup follow-up, the normal CI unit-test selection passed 1,300 tests and 130 focused identity/auth/profile tests also passed.
  • SwiftFormat, whitespace checks, and independent correctness/quality review passed.
  • Foreground/network, initialization, and activation wiring was source-reviewed. Dedicated iOS lifecycle/activation caller tests and successful Ring approval ordering are not covered by the new tests. No new lifecycle or persistence test architecture was introduced.
  • The unrestricted test target also includes live integration tests. Its earlier Blocktank regtest deposit call returned HTTP 404; the passing unit run uses the same integration exclusions as CI. No new iOS live-network test or battery benchmark is claimed.

@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 3/5

The PR is not yet safe to merge because identity publication can block authorization and payment maintenance, and concurrent activation triggers can be lost.

Findings

  1. P1 Publication Blocks Critical Work
  2. P1 Concurrent Triggers Are Lost

Summary

This PR upgrades Paykit to rc55 and adds periodic, throttled republication of existing Pubky identity records.

  • Adds foreground, network-restoration, initialization, session-activation, and authorization publication triggers.
  • Caches the regular Pubky bootstrap and serializes publication attempts.
  • Adds tests for throttling, retries, identity changes, client reuse, and concurrent triggers.
  • Inline publication can delay authorization and payment polling, while overlapping identity-specific triggers can be lost.

Diagram

sequenceDiagram
    participant Trigger as Foreground/Auth/Session Trigger
    participant Service as PaykitSdkService
    participant Bootstrap as PubkySessionBootstrap
    participant Work as Approval or Payment Refresh

    Trigger->>Service: republishIdentityIfNeeded(identity)
    alt Publication already running
        Service-->>Trigger: Return without queuing identity
    else Eligible
        Service->>Bootstrap: await republishIdentity(identity)
        Bootstrap-->>Service: success, missing record, or error
        Service-->>Trigger: Return
    end
    Trigger->>Work: Continue dependent operation
Loading

Reviews (1) · Last reviewed commit: "fix: republish pubky identity records"

Comment thread Bitkit/Services/PubkyService.swift
Comment thread Bitkit/Services/PubkyService.swift

@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 findings. Reviewed at d76e9eb2 as a key-material + dependency-bump change, alongside the android twin synonymdev/bitkit-android#1271. d76e9eb2 "fix: bound identity republishing" landed mid-review, so this covers both commits.

The bounding commit is correct. The extra machinery relative to android's one-line withTimeoutOrNull(5.seconds) is justified rather than gratuitous: the uniffi binding has no cancellation hook (a withUnsafeContinuation poll loop, freeFunc only after completion), so the FFI genuinely ignores cancellation as the comment at :382 says. A withTaskGroup race wouldn't work — the group awaits all children on scope exit, so the caller would block for the full publication anyway. Detaching the publication and racing a signal is the only way to release the caller early while keeping the in-flight guard tied to the real publication.

Things I specifically tried to break and couldn't:

  • isRepublishingIdentity leak. On the timeout path publication.cancel() (:393) is ignored, so the publication runs to completion and only then clears the flag via defer (:403) — held for exactly the real FFI duration. That duration is bounded: republish_identity is resolve(CacheOnly)resolve(NetworkOnly)publish, and pkarr 8.0.0 publish is a tokio::join! of relay (reqwest, config.request_timeout) and DHT (mainline, 2s per-request). paykit-ffi never tunes the pkarr builder, so it runs on pkarr's 2s default. Worst case is seconds, not the process lifetime, so republishing can't be permanently disabled.
  • Deadline starvation. deadline is actor-isolated but Task.sleep suspends without holding the actor, and every Rust call goes through uniffiRustCallAsync's withUnsafeContinuation, which releases it. The only actor-blocking work is millisecond-scale synchronous FFI/keychain calls. The 5s bound is honoured to within those.
  • Stream race. Continuation.finish() is idempotent; a stream finished before iteration returns immediately (the desired fast path for the :401 early return); nothing is yielded so buffering is moot. deadline is only cancelled in the defer, i.e. after the loop already exited, so it always fires within the timeout while the loop runs. The third finish() at :395 is redundant but harmless.
  • Caller cancellation. AsyncStream iteration terminates on cancellation and the wrapper returns promptly. Unstructured Task {} children don't inherit cancellation — the explicit defer cancels are what make this work, and they do.
  • Lock hold. PaykitSdkOperationLock is held across the wrapper at activateBootstrapResult :1183 (from importSession :458, signUp :475, activateRegisteredIdentity :501, signIn :522, completeAuth :570). That hold was previously the full FFI duration and is now ≤5s; the background publication doesn't take the lock. Strictly shorter than before.
  • AppScene reorder. Purely ordering. currency.refresh() is non-throwing, catches internally, and is bounded by URLSession timeouts, so it can't drop the republish. One nuance: scenePhase == .active is now evaluated after the fetch, so backgrounding mid-fetch skips the republish until the next foreground trigger. Not a defect.

Carried over from the first commit — the dependency bump rc54 → rc55 is additive (only republish_identity, its FFI wrapper, version bumps and two dev-deps; no pubky/pkarr bump, no storage format change). republish_identity takes only a public key — no secret, no session, no re-signing — and the key is derived locally and normalized before any network call. The republished packet is pre-signed and pkarr rejects lower-seq packets, so publishing key B's record from a device holding key A is inert. cachedBootstrap is stateless w.r.t. sessions. Sign-out clears .paykitSession and .pubkySecretKey, so a signed-out device has no identity to republish. No payment paths touched.

Twin parity vs synonymdev/bitkit-android#1271 — both sides landed the same bounding fix at the same 5s, shaped to their platform. Everything material is present on both: the throttle (30 min / 60 s / identity-change bypass / single-flight), the republish in initialize() and activateBootstrapResult, the three approval paths, connectivity-gated poll-start and maintenance triggers, and the network-restore trigger (iOS gates on scenePhase == .active, android on the polling job being active — same meaning). Android's PubkyRepo layer is absent on iOS by structure; AppScene passes pubkyProfile.publicKey, the same identity source, so there's no safety gap.

One genuine divergence worth knowing about, and it favours iOS — see my reply on greptile's publication-blocking thread for the test-coverage note.

Comment thread Bitkit/Services/PubkyService.swift

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

236c8d068 closes the coverage gap I noted. Test-only — git diff --quiet d76e9eb2 HEAD -- Bitkit is clean, so everything I verified at d76e9eb2 stands unchanged.

The added assertion is not a tautology. With defer { isRepublishingIdentity = false } (PubkyService.swift:403) removed, the first publication has already returned true so nextIdentityRepublishAt is now + 1800 and the identity matches; every retry call then enters republishIdentity, hits the guard !isRepublishingIdentity at :401 with the flag still held, and returns before reaching bootstrap().republishIdentity at :415. publicKeys.count stays 1, the loop spins to its deadline, and :109 fails 1 != 2. So it discriminates exactly the property that was missing.

The poll loop rather than my two-line suggestion is the right call, and worth recording why: finished.fulfill() fires inside the stub before the publication Task has resumed on the actor to run the defer, so a single retry call could legitimately observe the flag still held and early-return. Asserting directly would have been racy against ordering the code doesn't guarantee. Swapping bootstrap.operation at :102 before the retry is also necessary — otherwise the second publication re-fulfils started/finished and XCTest's over-fulfil check fails.

Determinism is at least as good as the existing test: no wall-clock reads, no sleeps, ContinuousClock is monotonic, and the only timing assumption is that an already-runnable Task gets its actor turn within 1s, which is the same class of bound as the 20ms and 1s deadlines already at :93 and :99. Each iteration's 5s deadline Task is cancelled by the wrapper's defer at :394, so nothing accumulates across iterations.

The caller-cancellation case is still uncovered — the android twin has both halves, iOS has retry only. That was the optional half of my note and I'm not asking for it.

Nothing further from me on this PR.

ovi-reviewer[bot]

This comment was marked as resolved.

ovi-reviewer[bot]

This comment was marked as resolved.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Applied the relevant part of João's Android emulator findings in d6bfdf0. SDK initialization now schedules identity publication without waiting for it, and the five-second caller deadline logs when it expires.

iOS already allows the native publication to finish after that deadline and keeps the single-flight guard until completion, so it did not need Android's separate network-operation timeout change. Auth cancellation checks are unchanged. All 1,300 unit tests and the 130 focused identity/auth/profile tests pass, along with SwiftFormat and the final independent review.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve


Reaudit: diff 1 file.
Retest suggested: item 1 (SDK initialization now starts identity republication without waiting; Manual Test 1 exercises this reopen path).
Counterpart synonymdev/bitkit-android#1271: equivalent.

Findings:
1 inline (non-blocking)

Audit:
Already done in comment.

QA: sim-1 iPhone 17 Pro simulator on iOS 26.5, exact-head E2E regtest build d6bfdf0

  1. passed: the profile and session survived three background/foreground cycles; successful publication remained throttled to one attempt.

    Test 1:
    Signed-in Profile → background and reopen Bitkit: profile/session remains available; repeated foreground…
    1.mp4
  2. passed: Scanner authorization reached the success screen after the forced missing-record publication failure.

    Test 2:
    Scanner → approve a Pubky auth request: approval still completes when identity republishing fails.
    2.mp4
  3. passed: no retry occurred during 75 seconds in the background; foregrounding immediately resumed eligible maintenance.

    Test 3:
    Background Bitkit: periodic identity maintenance stops; foreground/network restoration resumes eligible…
    3.mp4

Tip

Test 1 worth a journey:

  • Create and sign into a Bitkit profile
  • Send Bitkit to the background
  • Reopen Bitkit and verify the profile remains available
  • Repeat the background and foreground transition three times
  • Verify successful identity publication is not repeated

Test 2 worth a journey:

  • Create and sign into a local Bitkit profile
  • Restart the local Pubky testnet with an empty identity-record cache
  • Open Scanner and paste a Pubky authorization request
  • Tap Authorize
  • Verify authorization succeeds

Test 3 worth a journey:

  • Create and sign into a Bitkit profile
  • Make the profile identity service unavailable
  • Foreground Bitkit and verify one maintenance attempt occurs
  • Send Bitkit to the background for more than 60 seconds
  • Verify no maintenance attempt occurs while backgrounded
  • Foreground Bitkit and verify eligible maintenance resumes

Coverage:
QA: 3 of 3 Manual Tests passed


Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer test · retest · audit (author or owner)

Comment thread Bitkit/Services/PubkyService.swift
@jvsena42

Copy link
Copy Markdown
Member

Simulator test: d6bfdf09

Setup: Debug build without E2E_BUILD (regtest + staging homegate), iPhone 17 simulator on iOS 26.5. The wallet already had a Pubky identity (pubkyxgpi…f71o) with a restored Paykit session. Logs are from the app's file logger in the app group.

Result: nothing blocking. The three manual QA checks in the description behave as described. Two non-blocking notes are below.

What was run

# Scenario Result
1 Cold launch, twice (after a fresh install and after a simulator reboot) ✅ One Republished Pubky identity per launch
2 Three background/foreground cycles, 8s in background each, within the 30-minute window ✅ No extra republish. The throttle held and the profile stayed available
3 App left in the foreground for 30 minutes ✅ The maintenance loop republished 30m28s after the previous success
4 Main scanner → paste a pubkyauth://signin_grant request (private random relay secret) → Authorize ✅ Reached "Authorization Successful" within 2s of the tap. The republish on this path was inside the throttle window, so it returned immediately
5 Startup cost compared with earlier builds on the same simulator that have no republishing ✅ No regression. "Scene active" to "Paykit session restored": 3.1s and 4.0s on this branch vs 4.5s and 4.7s before. Contacts loaded at 4.8s and 6.5s vs 6.3s and 7.5s
Cold launch 1 (fresh install)
[2026-09-16 16:34:56.371 UTC] INFOℹ️: Scene phase changed: active - AppScene
[2026-09-16 16:35:00.360 UTC] INFOℹ️: Paykit session restored for pubkyxgpi8a4epnmb9z5m9bokaf5w6hm1myffk4odmo1gbb3ip6cpf71o - PubkyProfileManager
[2026-09-16 16:35:01.402 UTC] WARN⚠️: Stopped waiting for Pubky identity republishing - PaykitSdkService [PubkyService.swift: republishIdentityIfNeeded(publicKey:now:timeout:) line: 401]
[2026-09-16 16:35:02.446 UTC] DEBUG: Republished Pubky identity - PaykitSdkService [PubkyService.swift: republishIdentity(publicKey:now:) line: 429]
[2026-09-16 16:35:02.824 UTC] DEBUG: Loaded 1 SDK contact records - ContactsManager
Background/foreground cycles (no republish logged)
[2026-09-16 16:35:45.559 UTC] INFOℹ️: Scene phase changed: background - AppScene
[2026-09-16 16:35:52.449 UTC] INFOℹ️: Scene phase changed: active - AppScene
[2026-09-16 16:36:07.998 UTC] INFOℹ️: Scene phase changed: background - AppScene
[2026-09-16 16:36:15.836 UTC] INFOℹ️: Scene phase changed: active - AppScene
[2026-09-16 16:36:31.370 UTC] INFOℹ️: Scene phase changed: background - AppScene
[2026-09-16 16:36:39.203 UTC] INFOℹ️: Scene phase changed: active - AppScene

grep -c "Pubky identity" stayed at 2 for the whole session: the cold-start warning and its republish.

Cold launch 2 (after sim reboot) and the 30-minute periodic republish
[2026-09-16 16:38:16.050 UTC] INFOℹ️: Scene phase changed: inactive - AppScene
[2026-09-16 16:38:16.746 UTC] INFOℹ️: Scene phase changed: active - AppScene
[2026-09-16 16:38:19.882 UTC] INFOℹ️: Paykit session restored for pubkyxgpi8a4epnmb9z5m9bokaf5w6hm1myffk4odmo1gbb3ip6cpf71o - PubkyProfileManager
[2026-09-16 16:38:21.387 UTC] WARN⚠️: Stopped waiting for Pubky identity republishing - PaykitSdkService [PubkyService.swift: republishIdentityIfNeeded(publicKey:now:timeout:) line: 401]
[2026-09-16 16:38:21.593 UTC] DEBUG: Loaded 1 SDK contact records - ContactsManager
[2026-09-16 16:38:22.640 UTC] DEBUG: Republished Pubky identity - PaykitSdkService [PubkyService.swift: republishIdentity(publicKey:now:) line: 429]
[2026-09-16 16:39:16.405 UTC] INFOℹ️: Scene phase changed: active - AppScene      <- paste permission alert
[2026-09-16 16:39:16.733 UTC] DEBUG: Showing sheet pubkyAuthApproval after delay - SheetViewModel
                                                                                  <- Authorize tapped ~16:39:34.7, success screen by 16:39:36.7
[2026-09-16 17:08:50.573 UTC] DEBUG: Republished Pubky identity - PaykitSdkService [PubkyService.swift: republishIdentity(publicKey:now:) line: 429]

Non-blocking notes

1. A misleading WARN is logged on every cold start.
PaykitSdkService.initialize() runs Task { await republishIdentityIfNeeded() } (PubkyService.swift:363) with the default 5s deadline. Nothing awaits that task, but the deadline still fires and logs Stopped waiting for Pubky identity republishing. Both cold launches logged it. In launch 2 the wait began at ~16:38:16.39 (warning time minus 5s), before the scene became active, so it was the initialize() task and not a real caller. Someone reading logs will take it for a stalled caller. Consider calling republishIdentity(publicKey:now:) directly from that fire-and-forget task so it skips the deadline wrapper.

2. The first publication of a launch takes longer than the 5s cap.
Both cold launches took about 6.0s and 6.25s, measured from when the wait began to Republished Pubky identity. In the tested flows this didn't matter: startup doesn't wait on it, and the approval arrived inside the throttle window. But when an auth approval is the first eligible trigger (for example, the poll loop skipped republishing because network.isConnected was false, or a failed attempt's 1-minute retry has come due), Authorize will stall for the full 5s before approving. Consider whether the approval path needs to wait at all, or whether a shorter deadline is enough there.

Not covered on the simulator

  • Network-restoration trigger (handleNetworkChange): the simulator shares the host's network, so connectivity can't be toggled from here.
  • Approval while republishing fails: there was no way to make pkarr publication fail without changing host networking. The unit tests cover it.
  • Companion-claim (bitkit://pubky-auth/setup) and Ring approvals, and delegated identities without a local key.

Local build note for reviewers: the rc54 → rc55 bump fails the first incremental build with the stale paykitFFI.h .pcm error. just clean modules fixes it (for XcodeBuildMCP's DerivedData, set BITKIT_DERIVED_DATA_PATH to that path).

@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, merging now to avoid conflicts

@jvsena42
jvsena42 enabled auto-merge September 17, 2026 11:33
@piotr-iohk

Copy link
Copy Markdown
Collaborator

QA reviewed on d6bfdf0.

Ran on iPhone 17 C0525820-E7A8-44DD-8912-9ABF7A825153 (local staging sim build, pubky fixture): watch-only setup link → Approve → Authorize, Authorization Successful in ~4s. Cold-start of the same link: same. Skipped 30m soak.
Twin bitkit-android#1271 is merged.

No findings.

Checked and clean

  • initialize() schedules republish without awaiting it.
  • Auth paths republish, then checkCancellation, then approve; failures do not fail approval.
  • AppScene only republishes when connected / scenePhase is active.
  • Authorize on this SHA did not hang past the 5s caller deadline.

QA LGTM

@jvsena42
jvsena42 merged commit 5cdcf18 into master Sep 17, 2026
17 of 19 checks passed
@jvsena42
jvsena42 deleted the codex/republish-pubky-identity branch September 17, 2026 16:03
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.

4 participants