feat: support Pubky signup - #1224
Conversation
d02c926 to
09dff52
Compare
Greptile SummaryThis PR adds Pubky Ring signup URL parsing and scanner routing, registers the wallet-derived identity with the requested Homeserver, activates Paykit, and resumes profile setup through durable local state.
Confidence Score: 3/5This PR should not merge until signup activation can recover from the second network operation failing and pending profile setup can be exited without an immediate navigation loop. The new signup sequence can complete remote registration and authorization while leaving Bitkit without a local session, and the successful path's durable pending marker makes the CreateProfile back action ineffective. Files Needing Attention: app/src/main/java/to/bitkit/services/PaykitSdkService.kt, app/src/main/java/to/bitkit/repositories/PubkyRepo.kt, app/src/main/java/to/bitkit/ui/ContentView.kt
|
| Filename | Overview |
|---|---|
| app/src/main/java/to/bitkit/models/PubkyAuthRequest.kt | Adds strict Ring signup parsing, query validation, and conversion into the existing Pubky authorization request model. |
| app/src/main/java/to/bitkit/repositories/PubkyRepo.kt | Coordinates signup registration, authorization, activation, and pending profile state, but the multi-step flow can strand remotely completed signup without a local session. |
| app/src/main/java/to/bitkit/services/PaykitSdkService.kt | Adds registration without activation, discarding an activatable signup result and requiring a failure-prone second sign-in. |
| app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt | Routes Ring signup through the normal scanner while explicitly rejecting Pubky requests in payment-only contexts. |
| app/src/main/java/to/bitkit/ui/ContentView.kt | Resumes pending profile setup automatically, but conflicts with the unchanged dismissible CreateProfile back action. |
| app/src/main/java/to/bitkit/ui/screens/profile/PubkyAuthApprovalViewModel.kt | Dispatches Ring signup approval, handles existing identities, and dismisses the approval sheet before profile setup. |
Sequence Diagram
sequenceDiagram
participant Scanner
participant Approval as Approval UI
participant Repo as PubkyRepo
participant Server as Homeserver
participant App as Requesting app
participant Paykit
Scanner->>Approval: Pubky Ring signup request
Approval->>Repo: Approve signup
Repo->>Server: Register derived identity
Repo->>App: Approve authorization
Repo->>Paykit: Sign in and activate session
Paykit-->>Repo: Active local session
Repo-->>Approval: Dismiss approval
Repo->>Repo: Mark profile setup pending
Repo-->>Scanner: Navigate to profile setup
Reviews (1): Last reviewed commit: d02c926 | Re-trigger Greptile
|
Regtest device QA, home Scan, QR from staging.pubky.app. Staging e2e doesn’t finish — is that expected? No spinner after scan. Already signed in toast works and the scanner closes (iOS leaves it up — noted on #724). bitkit_logs_2026-09-03_09-44-43-android.zip Screen_Recording_20260903_113712_Bitkit.Regtest-android.mp4 |
|
@piotr-iohk Thanks for the device QA and logs. You found a real signup interop bug: Bitkit treated every The ordinary sign-in QR rejection is separate. These PRs use Paykit rc50’s app-scoped grant auth model, while staging Pubky App is still generating the older auth request format. Pubky App needs to update its sign-in flow to the new grant model for ordinary sign-in to work with Bitkit. Could you please recheck the staging signup path on this head? |
447e781 to
36bfd02
Compare
174e4c8 to
504ae04
Compare
|
Retested the rebased head on a physical Samsung S22 using the regtest build and a signup QR from staging.pubky.app. Signup now completes end to end: authorization progress is visible, Bitkit creates the Pubky identity and opens profile setup, and the staging website continues successfully. The logs confirm the authorization completed and the local identity/session were created. Ordinary sign-in still returns Authorization failed because staging currently generates the older non-grant authorization request. As clarified, that is outside the scope of this signup PR. Non-blocking UI parity note: the approval screen differs between platforms. iOS (left) always shows the placeholder profile card, while Android (right) omits it because no profile exists yet. It would be good to align the intended design, but I don’t think this should block the signup fix.
bitkit_logs_2026-09-03_14-48-43.zip Screen_Recording_20260903_164721_Bitkit.Regtest.mp4 |
|
needs conflict resolution @ben-kaufman |
504ae04 to
94bc950
Compare
ea18746 to
f331ad8
Compare
21d5a06 to
a681dfc
Compare
f331ad8 to
ef49aa1
Compare
|
I checked the failing |
ef49aa1 to
ff24be9
Compare
|
Restacked onto the updated #1200 head ( |
ff24be9 to
f54f1f3
Compare
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed the delta at dc72c36b5. The homeserver re-homing is fixed — trace on the resolved thread. No HIGH/MEDIUM.
Scoping note so it isn't misread: the canDeferStaleSession narrowing in this range is not part of this fix — it comes from the restacked parent (d8882611f refactor: remove paykit migration handling). I'm reviewing that separately on #1200 and will report there.
The test coverage here is stronger than the iOS twin's: these drive the real repo with the keychain mock, so they pin the actual wiring rather than an injected coordinator. never loadString(BIP39_MNEMONIC) in the restore test is a nice touch — it asserts the seed path isn't even touched when a stored key exists. One coverage note only: pubkyService is mocked, so the SDK-level persistSessionAccess deletion isn't exercised; I checked by reading that a signIn failure can't delete the key, so it's a gap in the net rather than a hole in the behaviour.
jvsena42
left a comment
There was a problem hiding this comment.
Reviewed the delta at 540c6a455. No HIGH/MEDIUM — clean. I treated this as a remote-input-surface change since it registers a URL scheme.
The new route is runtime-toggled, not statically enabled — that was the distinction I most wanted to settle, because a manifest entry is normally live at install regardless of any runtime flag. It isn't here: the new pubkyring://signup filter sits inside the existing .ui.MainActivityPubkyAuth alias, which is android:enabled="false" in the manifest, and the only thing that flips it is PubkyAuthHandlerRegistrar.setAliasEnabled behind isPaykitUiEnabled && (!hasIdentity || hasSecretKey). So the new scheme is delivered under exactly the conditions pubkyauth:// already was. No new component, no change to exported, no change to MainActivity's own filters or to <queries>.
No lock or approval bypass. isProtocolUrl returns true for pubkyring + host signup, so the link takes the same branch as pubkyauth://: the Paykit flag and walletRepo.walletExists() gate, then launchScan → enqueueDeferredScan when locked, and after unlock flushDeferredScan → handleScan → handlePubkyAuth → showSheet(Sheet.PubkyAuth). There's no path from this route to approveSignupAuth that skips the sheet.
Redaction holds for the new scheme. A pubkyring://signup?hs=..&relay=..&secret=.. link is logged twice, and both are safe: sanitizedDeeplinkLogValue drops query and fragment so the line is literally pubkyring://signup, and scanLogId falls through to sanitizedQrLogValue giving redacted#<8-byte sha256>. Nothing in this diff adds a new sink.
On collisions — claiming only the signup host is the right call. pubkyring://auth stays unclaimed, so Ring's own callback is untouched, and if both apps are installed Android shows the chooser, which is the intended "either app can perform signup" behaviour rather than a hijack.
The new PubkyAuthManifestTest is not trivially true. It resolves against the merged manifest via Robolectric's PackageManager, so it would catch a flavour override, and it pins both directions: the route must resolve while the alias is enabled and must resolve to nothing while disabled. Deleting the new filter or moving it onto a statically-enabled component both break it, which is exactly the pair of regressions worth guarding here.
73328c7 to
6df1707
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
addressed - reaudit confirmed
jvsena42
left a comment
There was a problem hiding this comment.
One LOW inline, diagnostics-only. Not blocking.
Gating: every entry point needs isPaykitEnabled (DataStore default false, written only from Dev Settings) and PaykitFeatureFlags.isUiAvailable. Nothing in .github or the justfile sets PAYKIT_UI_DISABLED, so the UI is available in a mainnet release build but off until the Dev Settings toggle. Both manifest aliases ship android:enabled="false" and are flipped only by PubkyAuthHandlerRegistrar. So: reachable only by someone who has opened Dev Settings and enabled Paykit.
Delta since my last pass: 0ff1a0129 merges master with no conflict-resolution edits, and 6df1707e1 "separate pubky signup and authorization routes" answers the thread on PubkyAuthHandlerRegistrar.kt:95. Over-correction check passed — the auth-alias predicate is now byte-identical to the pre-PR merge-base (isPaykitUiEnabled && hasIdentity && hasSecretKey), so session-only Ring users are advertised exactly as before, and the new signup alias claims only pubkyauth://signup, pubkyauth://direct_signup and pubkyring://signup. Nothing else was introduced. None of my threads were pending.
Checked and clean:
- Manifest split.
MainActivityPubkySignupisenabled="false",exported="true", with filter 1 = schemepubkyauthx hosts {signup,direct_signup} and filter 2 =pubkyring+signup. Since Android merges<data>attributes per filter, that cross-product is exactly what's intended.PubkyAuthManifestTestresolves against the merged manifest via Robolectric in all three states and pins that with the auth alias enabled,pubkyauth://signup*goes to the auth alias (wherehandlePubkyAuthanswers "Already signed in") andpubkyring://signupgoes nowhere. - Key material. The identity is seed-derived, never generated. The hex secret is passed in-process to
bootstrap().signUpand to bitkitcoreapprovePubkyAuth, and persisted only bypersistSessionAccessinto the AndroidKeyStore-backedKeychain. On logging:scanLogIdhashes any non-SamRock QR,sanitizedDeeplinkLogValuedrops query and fragment, andLogger.errorrenders only[Class='message']. Every new parse error message inPubkyAuthRequest.ktis value-free, and aURISyntaxException— whose message embeds the whole input — can't reach the signup path becauseisSignupUrlalready requiresURI(rawUrl)to succeed. One limitation worth stating: my local bitkit-core checkout predates the pubky module, so I could not readapprovePubkyAuth's Rust error strings. That sink is shared with the pre-existing non-signupapproveAuthpath, so it isn't new exposure, but it is unverified rather than cleared. - Authorization / TOCTOU. Display state is built from
parseAuthUrl(authUrl); approval re-parses the same immutable string with pure functions, so thehomeserverPublicKeyrendered on the sheet is the same field handed toregisterIdentity.requestAuthorizepinsstate.authUrl == authUrl,transitionToAuthorizingandinFlightAuthorizationare both CAS, andPubkyAuthorizationLocalAuthconfirms only thependingAuthUrlit was asked to authenticate. - Caps shown equal caps signed. Kotlin
parseCapabilitiesdrops only segments pubky-common'sCapability::try_fromalso rejects, and any non-r/waction makes Rust reject the whole capability — so displayed is always a superset of granted.validateSignupRequestruns bitkitcore's parser on the constructed URL first. - Trust boundary.
hsis format-checked only, with no allowlist — you declined that after my earlier thread and mitigated by rendering the homeserver plus the trust warning, so I'm not reopening it.relay/secretare never rendered and only reach the network via the constructedpubkyauth:///?relay=…handed to bitkitcore, which is pre-existing for ordinary auth URLs.x-bitkit-claimon a signup is rejected, andpubkyauth://signup?hs=Xwith arelaybut nosecretfailsrequiredSingle("secret")rather than silently downgrading to direct signup. - Manifest exposure. Both aliases are exported, so any installed app can fire the intent while an alias is enabled — but the most it gets is the sheet.
processDeeplinkrequires Paykit on and a wallet to exist, routes throughlaunchScan(allowPubkyAuth = true)which defers while locked, andapproveSignupAuthis reachable only via Authorize ->RequestLocalAuth-> PIN/biometric (or the tap alone when neither is configured, consistent with the rest of the app). Both are pinned by tests. - Lifecycle and partial failure.
approveRingAuthfailing writes nothing locally and the remote account is retried via rc51's 409 path; activation failure runsclearRegisteredIdentityActivationLockedunderNonCancellable; a failed pending-flag write runsforgetSessionAccess+clearLocalState; process death between activation and the pending write leaves the session restorable on next launch with the profile creatable from the Profile tab.createIdentitywith a stored key goes straight tosignIn, never Homegate. - Cancellation. The ViewModel is
hiltViewModel()in the sheet host, a sibling ofRootNavHostundersetContent, so it's Activity-scoped — dismissing the sheet cannot cancelapproveSignupAuth. All rollbacks areNonCancellable._effectsis aSharedFlowwithextraBufferCapacity = 1, so aDismisswith no collector is dropped, which is harmless because CreateProfile navigation is driven by the persisted pending flag rather than the effect. - Cross-identity.
hasIdentity()checks in-memory key plus both keychain entries, so a session-only Ring identity blocks a second seed-derived signup.activateBootstrapResultdeletesPAYKIT_SDK_STATEwhen the key changes, andsharesPrivatePaykitEndpointsis reset before registration. - Scanner containment. Contact-payment, shop and webview scans pass
allowPubkyAuth = falseand are rejected before touching payment state. - Migration limit clause. The
createIdentityrestructure movedshouldRevokeSessionOnFailure = truefrom the shared path into the Homegate branch only. I checked what else that assignment carried — nothing but revoke-on-failure for a freshly created Homegate session, which is exactly what it was meant to confine. No migration finding.
One observation, not a finding: the registrar keys on the in-memory _publicKey while the runtime check uses the keychain, so after a RestorationFailed the signup alias is advertised and the app answers "Already signed in". The outcome is correct and it predates this delta.
Cross-repo: the alias split is Android-only by construction — iOS registers URL schemes statically in Info.plist and can't runtime-gate them, so the "advertised for links it can't handle" concern is structural there rather than a bug. Worth a note on synonymdev/bitkit-ios#722 rather than a fix. The LOW below should also be checked against synonymdev/bitkit-ios#724 approval-failure handler, which I haven't re-reviewed this round.
|
This reuses the existing authorization sheet and Create Profile screen, with signup details and a homeserver card added to the sheet. |
Confirmed: no Figma source exists for these reused screens, and this delta changes no UI layout, copy, or resource. The design-reference note is settled. |

Description
pubkyring://signupand auth-bearingpubkyauth://signuprequests, plus directpubkyauth://direct_signupand parameter-only legacypubkyauth://signup, through the normal scanner and deep-link flow.This PR is stacked on #1200 and uses its Paykit rc51 authorization model. Ordinary Pubky App sign-in must use that grant-auth model; compatibility with the older sign-in request is intentionally outside this signup PR.
Preview
Not included; this reuses the existing scanner, authorization approval sheet, loading treatment, and profile setup UI.
QA Notes
Validation:
testDevDebugUnitTest: 2,346 tests passed on the restacked branch.compileDevDebugKotlinanddetektpassed.PubkyRepoTest.kt: 97 tests passed, including existing-key recovery, secure-storage errors, sign-in/profile failure retries, cancellation, and no-key signup. Detekt reports no changed-file findings.