Skip to content

fix: export wallet backup without paykit registry #1784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jvsena42 wants to merge 2 commits into from
Closed

fix: export wallet backup without paykit registry #1784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99b23c7
fix: export wallet backup without fetching paykit registry
jvsena42 Nov 5, 2026
17ef309
chore: rename changelog fragment
jvsena42 Nov 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/src/main/java/to/bitkit/services/PaykitSdkService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1613,7 +1613,8 @@ class PaykitSdkService @Inject constructor(
backup = operationLock.withLock(priority) {
check(runtimeGeneration == generation) { "Paykit runtime changed before backup export" }
if (priority == Priority.Background && isPaymentSubmissionActive.value) return@withLock null
val state = withPaykitKey { completeSdkCall { it.exportBackupString() } }
// Export reads local SDK storage only; deriving the Paykit key would fetch the App Registry.
val state = completeSdkCall { handle().exportBackupString() }

@talosmachina talosmachina Nov 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Export still takes a homeserver write lock, so WALLET backup still fails offline

Bitkit builds the SDK with withPaymentAdapterAndPubkySharedStateAndClientConfig, which wires PubkySharedStateStorage (paykit-ffi sdk.rs:250 at v0.1.1-rc2, the pinned version). With that adapter export_backup_string is not a local read: backup::export_backup_state calls storage.transaction, which with no active operation goes through run_operation_erased (pubky_shared.rs:381). That needs the Paykit identity secret in the session access, takes the remote lock with with_write_lock (storage.lock(...), an HTTP call to the homeserver, locks.rs:170), then reads the pending markers and the encrypted state blob from the homeserver. With the homeserver blackholed (manual test 1), the get_paykit_app_registry error goes away but the export fails at acquire Pubky write lock, and the WALLET backup retries as before. The changelog line is not delivered. The new test passes only because PaykitSdk is a mock, so it cannot see this path. Making the backup survive an outage needs a change below this call, either in the SDK (an export that can serve the last observed state) or in how BackupRepo treats a Paykit transport failure.

Verified by: traced exportBackupString through paykit-rs v0.1.1-rc2 (sdk.rs:349 > runtime/backup.rs:11 > backup/mod.rs:240 > storage/mod.rs:170 > pubky_shared.rs:517-523 > run_operation_erased > with_shared_state_lock > locks.rs:170). A cold reader from another model family checked the same trace and found it holds.

@jvsena42 jvsena42 Nov 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Confirmed. This change removes the App Registry fetch but does not fix #1746, and the test passes only because PaykitSdk is a mock.

Device run (Android, 17ef309, rooted google_apis emulator, homeserver 34.65.231.81 dropped with iptables at 12:36:43): before the block the WALLET backup succeeded (12:34:07). After it, WALLET retried at 10 s, 20 s and 40 s, each failing with acquire Pubky write lock (request timeout), while ACTIVITY, METADATA and WIDGETS kept succeeding. The backoff from #1552 works; the export itself still needs the homeserver.

I'm leaving this thread open and the PR as a draft until we decide where the fix belongs: below this call (an SDK export from a local snapshot, or a separate backup category for Paykit state), or closing #1746 as working by design.

PaykitSdkBackupExport(state, backupStateVersion.value)
}
} while (backup == null)
Expand Down
15 changes: 15 additions & 0 deletions app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,21 @@ class PaykitSdkServiceTest {
verify(sdk, times(2)).processPendingPrivateMessages()
}

@Test
fun `backup export does not derive the paykit key from the app registry`() = runTest {
val keychain = mock<Keychain>()
whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(ByteArray(32) { 1 }.toHex())
val sdk = mock<PaykitSdk>()
whenever(sdk.paykitAppRegistry(any())).thenThrow(IllegalStateException("homeserver unreachable"))
whenever(sdk.exportBackupString()).thenReturn("backup")
val service = PaykitSdkService(mock(), keychain, mock(), settingsStore = mock()) { sdk }

assertEquals("backup", service.exportBackupState(Priority.Interactive).state)

verify(keychain, never()).loadString(Keychain.Key.PUBKY_SECRET_KEY.name)
verify(sdk, never()).paykitAppRegistry(any())
}

@Test
fun `state observation validates identity and propagates unavailable session failures`() = runTest {
val observation = PaykitStateObservation(
Expand Down
1 change: 1 addition & 0 deletions changelog.d/next/1784.fixed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Wallet backups now complete while the Pubky homeserver is unreachable.
Loading