Skip to content

feat: enable paykit by default - #818

Open
ovitrif wants to merge 7 commits into
masterfrom
feat/enable-paykit-by-default
Open

ovitrif wants to merge 7 commits into
masterfrom
feat/enable-paykit-by-default

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #802
Twin: synonymdev/bitkit-android#1359
Refs: synonymdev/bitkit-e2e-tests#259

Description

  • Turns Paykit on by default, so new wallets and wallets updated from 2.5.0 show contacts, payment requests and subscriptions without a change in Dev Settings.
  • Keeps Paykit off for anyone who turned the Dev Settings switch off, since that choice is stored and still wins over the new default.
  • Starts a wiped wallet with Paykit on, like a new install, instead of writing Paykit off during the reset.
  • Restores the Paykit-on default on update for wallets that were wiped on 2.3.2 to 2.5.0, whose wipe stored Paykit off, when the wallet shows no sign of Paykit use (no shared endpoints, no pending cleanup, no stored Pubky identity). A tester who turned Paykit on and then off without ever creating a profile looks the same and gets Paykit back on.
  • Turns Paykit off in Dev Settings without an error toast on a wallet that never shared endpoints, because the switch now unpublishes only the public or private endpoints that were shared, as Android does.
  • Drops the Dev Settings step from the Pubky marketplace and Pubky auth journey preconditions, since no journey needs to turn Paykit on anymore.

Out of Scope

  • Dev Settings: the Enable Paykit UI switch stays in 2.6.0 as an internal option; no journey or E2E spec uses it anymore.
  • FEATURE_PAYKIT_UI_DISABLED build flag: unchanged; a build with it set still hides Paykit.
  • bitkit-e2e-tests: the specs stop opening Dev Settings to turn Paykit on in the companion PR.

Design

N/A — no UI changes; the existing Paykit screens are now shown by default.

Preview

paykit-preview-ios.mp4

QA Notes

Journeys

  • updated wallet-leg.xml — the precondition no longer opens Dev Settings

Manual Tests

  • Install 2.5.0 → create a wallet without opening Dev Settings → update to this build → Home shows the profile button and the drawer shows Subscriptions — an older release build to update from not in Capabilities
  • Install 2.5.0 → create a wallet, turn Enable Paykit UI on and then off in Dev Settings → update to this build → Paykit stays off — an older release build to update from not in Capabilities

Automated Checks

N/A

@ovitrif

ovitrif commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

@ben-kaufman FYI, fixed inline in this PR (2bb41fc). With Paykit on by default, a new wallet without a Pubky profile that turned Enable Paykit UI off in Dev Settings got an error toast instead of the success one:

Paykit UI disabled
Paykit.PaykitError.Identity(code: "identity_error", context: "no Pubky session available")

disablePaykitUI() always tried to unpublish endpoints, and with no Pubky session that call threw although nothing was ever published. It now unpublishes only the public or private endpoints that were shared, or whose cleanup is still pending, as Android does.

@ovitrif
ovitrif marked this pull request as ready for review September 28, 2026 15:45
@ovitrif
ovitrif requested review from a team, coreyphillips and pwltr and removed request for a team September 28, 2026 15:46
@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Changes the default state of a feature flag across the app.

The PR appears safe to merge, though the new journey should distinguish a successful disablement from a cleanup error.

Findings

  1. P2 Journey accepts cleanup errors ▶

Summary

The PR enables Paykit UI when no preference is stored, preserves an explicitly stored off preference, restores the on default after wallet wipe, and updates tests and journeys.

  • The new journey checks the switch’s visible state but cannot distinguish successful disablement from a cleanup error.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A["Stored Paykit preference?"] -->|Off| B["Paykit UI off"]
  A -->|On or absent| C["Paykit UI on, if build permits"]
  C -->|Switch off| D["Attempt endpoint cleanup"]
  D --> E["Switch off; cleanup succeeds or remains pending"]
Loading

Reviews (1) · Last reviewed commit: "docs: note paykit disable toast differen..."

Comment thread journeys/paykit/default-on.xml Outdated
coreyphillips
coreyphillips previously approved these changes Sep 28, 2026
@ovitrif

ovitrif commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed up to 8d6b2f3 since the review request:

  • Turning Enable Paykit UI off in Dev Settings no longer shows an error toast on a wallet without a Pubky profile: it now unpublishes only the public or private endpoints that were shared, as Android does (2bb41fc).
  • Removed the new default-on.xml journey and the added PublicPaykitServiceTests cases. The preview video shows a fresh install landing on Home with Paykit on.
  • The Pubky marketplace and Pubky auth journey preconditions no longer open Dev Settings. The switch stays in 2.6.0 as an internal option; the E2E specs stop using it in test: stop enabling paykit in dev settings bitkit-e2e-tests#259.
  • Merged master (workflow files only).

This merges together with synonymdev/bitkit-android#1359 and synonymdev/bitkit-e2e-tests#259.

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

One medium finding inline: on upgrade, released iOS builds keep Paykit off for every user who ever wiped a wallet, which Android does not.

Checked and clean:

  • The toggle stays in Dev Settings, so users can still opt out. An explicit off stays off, and an absent key reads as on.
  • The ~19 view edits only change the @AppStorage default. Every gate still reads isUIAvailable && isPaykitUIEnabled, and no conditional lost a second check (isAuthenticated, non-empty eligibleTargets). FEATURE_PAYKIT_UI_DISABLED builds stay fully hidden.
  • First launch with no identity: resolveSessionInitialization returns .noSession, so there is no "Profile Disconnected" toast. The node and sync hooks return on empty contact and reservation sets. No homeserver sign-up, keychain identity write or notification prompt happens without user action.
  • Incoming payment requests still go through SendConfirmationView. The only automatic payment remains the first subscription payment after the user accepts the review sheet (pre-existing).
  • Backups never carry paykitUiEnabled, so a restore cannot flip it.

Merge order: with Paykit on for everyone, #796 (restore failure shows "Profile Disconnected" and resets contacts on an offline launch) and #761 (stale Noise key after wipe and recreate in the same process) fix bugs present at this head and should land first. #774 reviewer notes rely on the toggle being off by default and need updating after this.

Comment thread Bitkit/ViewModels/SettingsViewModel.swift
@ovitrif

ovitrif commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

Pushed 88f13f3: a one-shot AppDataMigrations step restores the Paykit-on default for wallets wiped on 2.3.2 to 2.5.0, whose wipe stored Paykit off. It removes that stored off only when the wallet has no Paykit footprint, so an opt-out from anyone who set Paykit up stays. This answers the inline finding from @jvsena42. The full unit test suite passes locally except testchannelPurchaseFlow, which calls the regtest Blocktank deposit endpoint; that endpoint returned 404 during the local run.

Merge order: with Paykit on for everyone, #796 (restore failure shows "Profile Disconnected" and resets contacts on an offline launch) and #761 (stale Noise key after wipe and recreate in the same process) fix bugs present at this head and should land first.

@jvsena42 Added to the 2.6.0 merge order; #774's reviewer notes will be updated after this merges.

@ovitrif
ovitrif requested a review from jvsena42 September 28, 2026 16:43
@ovitrif

ovitrif commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

Merge order: with Paykit on for everyone, #796 (restore failure shows "Profile Disconnected" and resets contacts on an offline launch) and #761 (stale Noise key after wipe and recreate in the same process) fix bugs present at this head and should land first.

@jvsena42 We are not gating this PR on merge order after all, which replaces my earlier reply saying it was added. 2.6.0 ships only once every change in its milestone is merged, and the issues those PRs close, #811 and #814, are both in 2.6.0. The fixes reach users in the same release whatever order they land on master; waiting would only keep Paykit off in the RC builds before then. This PR merges together with synonymdev/bitkit-android#1359 and synonymdev/bitkit-e2e-tests#259.

@ovitrif

ovitrif commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

@jvsena42 @pwltr @coreyphillips please hold off merging this. I'll merge it myself once we decide whether to release it; that decision follows the approval of locks in the homeserver.

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.

feat: enable paykit by default

3 participants