Repository navigation
fix: guard wallet operations until required restore completes - #1685
Conversation
|
Triage: 3.1.0. It protects wallet operations during a shipped restore flow, and 3.1.0 is due 28 Oct. |
 [Medium impact] The PR appears safe to merge; no actionable issues were found. SummaryBlocks payments, new receive addresses, channel changes and fee changes while required wallet recovery remains pending.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Wallet operation requested] --> B{Required restore pending?}
B -->|Yes| C[Return failure]
B -->|No| D[Prepare or authorize operation]
D --> E[Native queue rechecks restore markers]
E -->|Pending| C
E -->|Clear| F[Run native operation]
G[Restore retry] --> H{Restore already running?}
H -->|Yes| I[Skip overlapping retry]
H -->|No| J[Restore wallet data]
J --> K{Required recovery complete?}
K -->|No| L[Keep Retry available]
K -->|Yes| M[Clear durable markers]
Reviews (1) · Last reviewed commit: "fix: guard wallet operations while requi..." · Reviewed by Greptile |
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Blocks new receive data, payments, channel and fee mutations and prepared broadcasts while either durable restore marker (PAYKIT_PENDING_BACKUP_RESTORE, USDT_PENDING_BACKUP_RESTORE) exists, checked at repository admission and again inside the LDK queue, hides Proceed Without Backup while a marker remains, and coalesces overlapping backup retries. Reviewed 837aa1c, full tier, stacked on #1681 (reviewed as the delta against fix/usdt-recovery-parity), reasoned from the code: per the repo's review rules this branch is not built here.
What I checked, and 6 candidates I ruled out
Read in full: the diff; WalletRestoreState.kt; performFullRestoreFromLatestBackup, restoreUsdtRecovery, restoreWalletBackup, performRestore and captureWalletBackupDataBytes in BackupRepo.kt; executeWhenNodeRunning, sendOnChain, payInvoice, prepareRelease in LightningRepo.kt; newAddress*, prepareOnchainSend, prepareRelease, send, bumpFeeByRbf, accelerateByCpfp, reconcileWatchOnlyAccounts in LightningService.kt; broadcastPreparedCandidate, current, releaseInterruptedUnsubmittedPreparation in OnchainSendAttemptStore.kt; restoreFromBackup, onBackupRestoreRetry, onProceedWithoutRestore, collectStates, RestoreState in WalletViewModel.kt; the restore branch of ContentView; WalletRestoreErrorView.
Call sites traced: every caller of newAddress/newAddressInfoForType (PrivatePaykitAddressReservationRepo, BlocktankRepo refund allocation, WalletRepo.newAddress, TransferViewModel reverse swap claim), payInvoice (AppViewModel, QuickPayRepo, PaykitAllowanceExecutor, reverse swap), sendOnChain, openChannel, closeChannel, bumpFeeByRbf, accelerateByCpfp, retryOriginalOnchainSend.
CI: build, build-local, lint, detekt and three of the four local e2e shards green; e2e-tests-local - send still pending at review time.
Ruled out
- Restore deadlocking on its own gate:
restoreWalletBackupwrites the marker first, then runsreconcileWatchOnlyAccountsandreconcileReservedIndexesWithLdk, which reach LDK only throughrevealReceiveAddressesToAccountandrevealReceiveAddressesTo. Neither is gated, andPrivatePaykitRepo.restoreBackupandPaykitPaymentRequestRepo.activateallocate no address, so the marker cannot block the restore that clears it. The newLightningRepoTestcase assertsrevealReceiveAddressesandsyncstill succeed. - Receive address left empty after restore: the restored cache is cleared by
resetBip21()while the marker is set, and gatednewAddresscalls fail until it clears. Opening the Receive sheet callsrefreshReceiveState, andrefreshReusableReceiveAddressOrThrowregenerates an empty address, after the marker is gone, so nothing stays empty past the success screen. - Throwing after
beforeDispatchpersistedinitialDispatchAttempted = true: a marker that appears between the two checks leaves the attempt marked dispatched with no broadcast, andsendOnChainreturnsOnchainSendPendingError. That fails closed (the attempt keeps blocking the next send until recovery resolves it), and the window needs a restore to begin during an in-flight send while the restore UI blocks the wallet. - Proceed racing a new marker:
onProceedWithoutRestorerechecks after thestop()delay and an unreadable keychain reads as pending (getOrDefault(true)), so dismissal cannot outrun a marker written meanwhile. The defaultBackupFailed(count, hasPendingWalletRestore = true)also keeps the button hidden on thecollectStatescold-start path. - Retry coalescing dropping a request:
tryLockreturns early only while a restore is running, and that restore reads the same durable markers when it fails, so the dropped second tap has nothing the first did not do.unlocksits in the existingfinally, aftersetRestorePending(false). - Parity with iOS: iOS
BackupService.hasPendingWalletRestore()also treats an unreadable state as pending, matching the fail-closed reads here.
Merge confidence: 4/5, no findings, but the send e2e shard had not finished and J1 is unrun (its fixture is not provisioned, as the PR says).
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
One MEDIUM inline, reproduced with a failing unit test: a restore marker now also removes "Proceed Without Backup" from the node-error screen, which can leave a user Retry-only behind a server they can't reach to change.
Checked and clean:
- There's no startup race: the markers are read synchronously from durable storage, and the only unguarded window, before the marker exists in a fresh restore, is already on master.
- Markers persist, and only a successful restore or a wipe clears them. An upgrade from v3.0.0 with a lingering marker heals itself on a successful Retry once the node can start.
- The new operation guard keeps payments and receive closed in the main UI while a marker exists.
- iOS has no twin; iOS master already gates at wallet start.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Reviewed the full PR diff against merge base 2493dbb, at 55b31ee. No earlier assessment baseline was available.
2 actionable findings — resolve or provide an evidence-backed rebuttal.
Repository and native admission block new addresses, invoices, payments, channel open/close, fee bumps, and prepared broadcasts while either required marker is set, and reservation reveal stays available. The restricted Electrum/RGS editor covers the unreachable-server case from the earlier Proceed thread while a required marker remains. Startup failures that are not an endpoint problem still suppress Proceed after the markers are clear.
Hosted build, detekt, and lint passed at this revision. E2E run 40341310833 failed. This review did not run the unit suite or J1/J2. Device testing: not performed. iOS connection recovery was not compared from a pinned checkout; the PR and journeys mark that editor as unfinished. Compatibility with draft bitkit-android#1681 was limited to the three commits from this base to 7c21884 (Paykit RC2 pin, backup test endpoints, and USDT recovery feedback). Those files do not change the pending-marker admission API.
Suggested additional test cases
- Android. Disposable wallet with both restore markers absent. Force node startup to fail with a non-endpoint error. Tap Try Again twice. Proceed Without Backup is shown, and confirming it leaves the error screen. Each failed attempt shows one error toast.
Findings
- [MEDIUM] Non-endpoint startup failures hide Proceed — inline at
app/src/main/java/to/bitkit/ui/ContentView.kt:424. - [LOW] One failed node start enqueues two error toasts — inline at
app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt:601.
Reviewer model: grok-4.7
Reasoning effort: high
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict:
Findings: None. I checked the restricted settings repair and both repository/native restore gates.
QA for 55b31ee: API 36, disposable unfunded regtest wallet, real staging Electrum and LNURL-authenticated encrypted VSS, with the uncommitted debug hook below.
Test T1: restricted connection editor ✅ Invalid Electrum is rejected; a validated correction saves without dismissing required recovery.
T1-editor-final-2x.2x.mp4
Restricted editor, failed probe, explicit Save and return to recovery; 2× speed.
Both durable markers stayed present after Save. Cold-restart persistence was checked separately through stored settings and device logs. Separate UI snapshots and settings reads confirmed that RGS Save and a failed RGS probe retained the last acknowledged endpoint.
Test T2: restore gates and selective replay ✅ Authenticated restore preserves connection corrections, clears both markers and opens the wallet.
T4-restore-replay-2x.2x.mp4
Retry, Wallet Restored and Home; 2× speed.
Before restoration, device logs and the hook assertion confirmed a Running node refused native address allocation with WalletBackupRestorePendingError in its cause chain; invoice creation was also refused. After repeated failed downloads, restoring backup access produced successful SETTINGS and WALLET imports. Direct settings reads showed USD restored over local EUR while the corrected Electrum/RGS values survived. Both markers then read clear.
Test J1: pending wallet recovery admission ⏭️ Partial; unresolved-payment and optional-profile portions remain unrun.
The authenticated unfunded fixture exercised download failure, retry, restart and admission restrictions. It did not contain an unresolved signed payment or reservation ceilings, so prepared-broadcast recovery was not exercised. An isolated post-restore profile failure and the Proceed race were not staged.
Test J2: complete connection recovery journey ⏭️ Partial; its optional-profile-failure action remains unrun.
T1/T2 cover the connection repair and real empty-wallet restore. They do not establish the final profile-failure action or restoration of unresolved USDT/payment state. I have not marked the complete journey passed.
Check 1: focused recovery checks ✅ Head build and targeted unit checks passed.
I built the committed head with assembleDevDebug assembleDevDebugAndroidTest and ran testDevDebugUnitTest for WalletConnectionRecoveryTest, WalletViewModelTest, LightningRepoTest, LightningServiceTest and BackupRepoTest. Broader component results are attributed to the author's final-head report.
Check 2: hosted E2E ⚠️ The existing workflow remains failed.
The current-head E2E run still has failed send, security and multi-address shards. I inspected the failed logs, which include SendSuccess timeouts; these passes do not waive that failure.
Uncommitted test hook and deciding evidence
The hook creates only a disposable wallet, uploads its own backups through the real repositories, stages durable markers, redirects app backup traffic to a refused endpoint on demand, and checks native rejection. It does not replace SDK responses or change recovery decisions. The earlier hook revision differed only in its native-error assertion; the final assertion follows the wrapped cause chain. Built and installed APK hashes were matched.
native newAddress blocked by AppError
Error executing 'createInvoice' [WalletBackupRestorePendingError='Wallet backup restoration is incomplete. Retry the restore before continuing.']
Restore success for: 'SETTINGS'
Restore success for: 'WALLET'
Full restore success
paykit=false usdt=false ... node=Running
diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 00c57a69a..61a345262 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -11,6 +11,7 @@ import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.android.EntryPointAccessors
import dagger.hilt.components.SingletonComponent
+import kotlinx.coroutines.flow.first // test hook
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import to.bitkit.async.ServiceQueue
@@ -32,6 +33,13 @@ class DevToolsProvider : ContentProvider() {
@EntryPoint
@InstallIn(SingletonComponent::class)
interface Dependencies {
+ fun keychain(): to.bitkit.data.keychain.Keychain // test hook
+ fun settings(): to.bitkit.data.SettingsStore // test hook
+ fun wallet(): to.bitkit.repositories.WalletRepo // test hook
+ fun backup(): to.bitkit.repositories.BackupRepo // test hook
+ fun vss(): to.bitkit.data.backup.VssBackupClient // test hook
+ fun migration(): to.bitkit.services.MigrationService // test hook
+ fun lightning(): to.bitkit.services.LightningService // test hook
fun lightningRepo(): LightningRepo
}
@@ -42,6 +50,54 @@ class DevToolsProvider : ContentProvider() {
override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+ if (method.startsWith("review")) { // test hook
+ return runCatching { // test hook
+ kotlinx.coroutines.runBlocking(kotlinx.coroutines.Dispatchers.IO) { // test hook
+ when (method) { // test hook
+ "reviewSeed" -> { // test hook
+ check(!deps.wallet().walletExists()) // test hook
+ deps.wallet().createWallet(null).getOrThrow() // test hook
+ deps.migration().markMigrationChecked() // test hook
+ deps.settings().update { it.copy(electrumServer = "tcp://127.0.0.1:1") } // test hook
+ "created disposable unfunded wallet" // test hook
+ } // test hook
+ "reviewBackup" -> { // test hook
+ deps.vss().setup().getOrThrow() // test hook
+ for (category in listOf(to.bitkit.models.BackupCategory.WALLET, to.bitkit.models.BackupCategory.SETTINGS)) { // test hook
+ deps.backup().triggerBackup(category).getOrThrow() // test hook
+ } // test hook
+ "acknowledged WALLET and SETTINGS backups" // test hook
+ } // test hook
+ "reviewFault" -> { // test hook
+ val file = java.io.File(requireNotNull(context).filesDir, "review-vss-unavailable") // test hook
+ if (arg == "on") file.writeText("enabled") else file.delete() // test hook
+ deps.vss().reset() // test hook
+ "VSS endpoint fault ${arg}" // test hook
+ } // test hook
+ "reviewHold" -> { // test hook
+ val file = java.io.File(requireNotNull(context).filesDir, "review-vss-hold") // test hook
+ if (arg == "on") file.writeText("enabled") else file.delete() // test hook
+ "VSS get hold ${arg}" // test hook
+ } // test hook
+ "reviewPending" -> { // test hook
+ deps.keychain().upsertString("PAYKIT_PENDING_BACKUP_RESTORE", "") // test hook
+ deps.keychain().upsertString("USDT_PENDING_BACKUP_RESTORE", "") // test hook
+ "persisted both pending markers" // test hook
+ } // test hook
+ "reviewState" -> { // test hook
+ val settings = deps.settings().data.first() // test hook
+ "paykit=${deps.keychain().exists("PAYKIT_PENDING_BACKUP_RESTORE")} usdt=${deps.keychain().exists("USDT_PENDING_BACKUP_RESTORE")} electrum=${settings.electrumServer} rgs=${settings.rgsServerUrl} node=${deps.lightningRepo().lightningState.value.nodeLifecycleState}" // test hook
+ } // test hook
+ "reviewNativeGate" -> { // test hook
+ val error = runCatching { deps.lightning().newAddress() }.exceptionOrNull() // test hook
+ check(generateSequence(error) { it.cause }.any { it is to.bitkit.data.keychain.WalletBackupRestorePendingError }) { "Unexpected native result: $error" } // test hook
+ "native newAddress blocked by ${checkNotNull(error).javaClass.simpleName}" // test hook
+ } // test hook
+ else -> error("Unknown test hook") // test hook
+ }.also { android.util.Log.i("ReviewHook", it) } // test hook
+ }.let { bundleOf("result" to it) } // test hook
+ }.getOrElse { bundleOf("error" to "${it.javaClass.simpleName}: ${it.message}") } // test hook
+ } // test hook
return runCatching {
val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
ServiceQueue.LDK.blocking { command.execute(deps) }
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
index eef9af0b1..2d603482d 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -46,7 +46,7 @@ class VssBackupClient @Inject constructor(
withTimeout(30.seconds) {
Logger.debug("VSS client setting up…", context = TAG)
- val vssUrl = Env.vssServerUrl
+ val vssUrl = if (Env.isDebug && java.io.File(Env.logDir.parentFile, "review-vss-unavailable").exists()) "http://127.0.0.1:1/vss" else Env.vssServerUrl // test hook
val lnurlAuthServerUrl = Env.lnurlAuthServerUrl
val vssStoreId = vssStoreIdProvider.getVssStoreId(walletIndex)
Logger.verbose("Building VSS client with vssUrl: '$vssUrl'", context = TAG)
@@ -136,6 +136,11 @@ class VssBackupClient @Inject constructor(
}
suspend fun getObject(key: String): Result<VssItem?> = withContext(ioDispatcher) {
+ if (Env.isDebug) { // test hook
+ val hold = java.io.File(Env.logDir.parentFile, "review-vss-hold") // test hook
+ if (hold.exists()) Logger.info("Held VSS read until explicit release", context = "ReviewHook") // test hook
+ while (hold.exists()) delay(100) // test hook
+ } // test hook
isSetup.await()
Logger.verbose("VSS 'getObject' call for '$key'", context = TAG)
runCatching {Coverage: full 2493dbb4..55b31ee, including lifecycle/cancellation, identity/wipe, sibling callers and translations. RN migration, wallet replacement, prepared broadcasts and optional-profile failure were reviewed in code but not driven in this device run.
Twin: no matching iOS PR is declared; the iOS restricted connection editor remains unfinished, as this PR discloses.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Requesting changes: one MEDIUM inline (a saved connection correction restarts the running node on every foreground and Try Again while the restore is pending), plus one LOW. Both come from 55b31ee and are new since my last review.
Since my last review:
- My Proceed thread is covered by the Connection settings route in
55b31ee:WalletRestoreErrorViewopensWalletConnectionRecoveryView, and the user can save a reachable Electrum/RGS without clearing the gate.8d72053brings Proceed back for marker-free startup failures after two retries. Marker plus non-endpoint failure staying Retry-only is the documented limitation. - The two items @piotr-iohk raised on
55b31ee(Proceed hidden after non-endpoint failures, the double toast) are fixed in8d72053.
The LOW can be fixed here or moved to a follow-up issue, your choice; please reply on the thread with which.
pending-wallet-recovery.xml and connection-recovery.xml through the dev hook, counting Restarting node across three foregrounds after a saved correction, and Forgot PIN → restore after a failed start.
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — two existing findings reproduced on the device at 8d72053.
The PR moved to 7b823d2 after this run; these results are for 8d72053.
The PR moved to 7b823d2 during the run. The restart follow-up and wipe follow-up report fixes there; this review has not verified that head.
Findings: No additional distinct defect in the delta. I added evidence to these existing threads:
- MEDIUM — unchanged connection corrections keep restarting the node.
app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt:699. With required recovery still pending, the initial repair caused one restart and three foregrounds caused three more. A backup-only retry with clear markers also advancedBackupFailed.countfrom 2 to 4. Existing finding. - LOW — the previous wallet's startup error survives wipe.
app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt:423. The old spending-balance error appeared over the next restore while its metadata reads were held. New startup eventually cleared it and restoration completed. Existing finding.
QA for 8d72053: Android API 36; disposable BTC regtest wallet; real staging Electrum and LNURL-authenticated encrypted VSS; the uncommitted debug hook below. The USDT recovery journal was empty.
Test T3: ordinary startup failure ✅ Proceed appears at the retry threshold, rejects a newly pending marker and dismisses after clear-marker confirmation.
| Marker recheck | Clear-marker confirmation |
|---|---|
T3-redo-2x.2x.mp4 |
T3-clear-redo-2x.2x.mp4 |
Controlled startup failure and confirmation, 2× speed. A hook observer counted one startup toast per failed attempt; the repeated check advanced from one toast to two. The production ViewModel reached Settled only after the admitted confirmation.
Test T4: foreground after a saved correction ❌ Every foreground rebuilt the running node with the unchanged correction.
T4-2x.2x.mp4
Initial repair followed by three foregrounds, 2× speed.
Device logs counted Restarting node once for repair and once on each foreground; both markers remained present. Separately, ViewModel state reads during the optional-profile run showed one backup-only retry advancing the failure count from 2 to 4. All three supplied unit reproductions also fail against the unmodified head.
Test T5: wipe followed by restore ❌ The old startup error covered the following restore until new startup cleared it.
T5-2x.2x.mp4
Same-process wipe, held metadata reads and eventual Wallet Restored, 2× speed.
The hook invokes the production wipe and restore ViewModel entry points used by Forgot PIN and onboarding. State reads retained the old startup failure after wipe and during the new restore; the video shows the wrong error screen. PIN setup and the Forgot PIN screen itself were not driven. After releasing the hold, the original prepared transaction and reservation ceiling were restored.
Test T6: editor cancellation and optional profile failure ✅ A held probe can be cancelled, and optional recovery permits Proceed only with clear required markers.
| Cancelled probe | Profile-only failure and confirmation |
|---|---|
T6-editor-2x.2x.mp4 |
T6-profile-redo-2x.2x.mp4 |
Back/reopen and confirmation checks, 2× speed. Editor state reads confirmed a fresh, non-busy editor after cancellation. Required imports completed before an injected profile-initialization error. Adding a marker before confirmation prevented dismissal; another acknowledged required restore allowed confirmed dismissal to Home.
Test J1: pending wallet recovery admission ✅ The BTC fixture retained admission restrictions through failed downloads, retries and restart, then recovered its payment and address state.
| Required download failures and restart | Repeated failed retries |
|---|---|
J1-pending-2x.2x.mp4 |
J1-retries-2x.2x.mp4 |
Required-recovery failure controls, 2× speed. The failure count reached four without offering Proceed while markers remained present.
Hook assertions verified repository BTC/LN sends and address allocation returned WalletBackupRestorePendingError in the cause chain. The final admission check used a valid BTC destination and a signed regtest Lightning invoice. A real native prepared transaction was separately refused before dispatch while markers were set. No payment was broadcast by these checks.
Two overlapping retry requests produced one Full restore starting event. After authenticated import, the attempt ID and prepared txid matched the acknowledged backup, its original input remained present, and the address reservation ceiling was still 12. A later wipe/restore recovered the same state from remote backup. T6 covers the optional-profile failure and confirmation race.
Test J2: connection recovery ✅ Validated corrections survived restart and older SETTINGS restoration while required recovery continued to gate operations.
| Restricted editor and failed probe | Cold restart | Final authenticated replay |
|---|---|---|
J2-editor-redo-2x.2x.mp4 |
J2-restart-2x.2x.mp4 |
J2-final-replay-2x.2x.mp4 |
Restricted controls, restart and Wallet Restored, 2× speed.
Settings reads showed remote USD replacing local EUR while the corrected Electrum endpoint survived. An independent RGS save used the reachable explicit-port URL https://bitkit.stag0.blocktank.to:443/rgs/snapshot; a failed probe retained that acknowledged choice. The final SETTINGS read retained the explicit-port RGS correction after authenticated replay. Both durable markers cleared, and a new receive address became available afterward. T6 covers the post-restore profile-only failure.
Check 1: head build and focused regressions ✅ The unmodified head builds and its focused recovery suites pass; the existing finding reproductions fail.
I ran assembleDevDebug assembleDevDebugAndroidTest testDevDebugUnitTest with the five focused recovery suites before adding hooks. Separately, the three tests supplied in the existing findings reproduce repeated restarts, double retry counting and retained startup failure. The final hook APK's built and installed SHA-256 values match.
Check 2: hosted validation ✅ CI, lint and E2E at 8d72053 were successful.
The CI run passed. The lint run passed. The E2E run passed. These exercise the merge composition described in the PR; the earlier failed E2E report retains its original revision scope.
The J1/J2 results apply to the stated BTC fixture. Signed USDT recovery contents, RN migration, unreadable device keychain state, and the PIN UI were not exercised on the device; controlled unit coverage remains separate evidence.
Coverage: contiguous delta 55b31ee..8d72053, plus open findings and reached lifecycle, identity, cancellation and sibling paths. The previous review retains its earlier evidence.
Twin: bitkit-ios#1139 fix: recover connections during wallet restore matches the restricted-editor and durable-replay scope at 9481b503; iOS restores before node startup. It remains draft, and device parity was not driven here.
Uncommitted test hook and deciding evidence
The hook stages the disposable wallet, its signed transaction and reservation state; controls VSS availability and read/probe holds; injects startup/profile errors before their service calls; and invokes production ViewModel/repository APIs. It also initializes real native services after application storage is ready. Native signing, LNURL authentication, encrypted downloads and imports use the committed implementations. Earlier clips used the same controls before the valid-invoice helper was added.
repository BTC/LN/address and native address gates rejected; validInvoice=true
foreground 1: restart count 1 -> 2
foreground 2: restart count 2 -> 3
foreground 3: restart count 3 -> 4
restore=BackupFailed(count=2, hasPendingWalletRestore=false)
restore=BackupFailed(count=4, hasPendingWalletRestore=false)
attempt=3801d569-69da-4d2f-b359-006143f1b5a3
txid=17fd11ef8a8374ad7af589a2a988f61174792213780e1f09584691b33d76029a
inputs=1 dispatched=true restored=true reservations={p2wpkh=12}
dispatched=true is the conservative restored-backup flag; the fixture transaction was prepared without broadcast. The same txid survived restoration.
diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 00c57a69a..df2cedaa4 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -11,6 +11,8 @@ import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.android.EntryPointAccessors
import dagger.hilt.components.SingletonComponent
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.flow.first // test hook
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import to.bitkit.async.ServiceQueue
@@ -32,6 +34,16 @@ class DevToolsProvider : ContentProvider() {
@EntryPoint
@InstallIn(SingletonComponent::class)
interface Dependencies {
+ fun keychain(): to.bitkit.data.keychain.Keychain // test hook
+ fun settings(): to.bitkit.data.SettingsStore // test hook
+ fun wallet(): to.bitkit.repositories.WalletRepo // test hook
+ fun backup(): to.bitkit.repositories.BackupRepo // test hook
+ fun vss(): to.bitkit.data.backup.VssBackupClient // test hook
+ fun migration(): to.bitkit.services.MigrationService // test hook
+ fun lightning(): to.bitkit.services.LightningService // test hook
+ fun pubky(): to.bitkit.repositories.PubkyRepo
+ fun attempts(): to.bitkit.repositories.OnchainSendAttemptStore
+ fun reservations(): to.bitkit.repositories.PrivatePaykitAddressReservationRepo
fun lightningRepo(): LightningRepo
}
@@ -42,6 +54,138 @@ class DevToolsProvider : ContentProvider() {
override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+ if (method.startsWith("review")) { // test hook
+ return runCatching { // test hook
+ kotlinx.coroutines.runBlocking(kotlinx.coroutines.Dispatchers.IO) { // test hook
+ when (method) { // test hook
+ "reviewSeed" -> { // test hook
+ check(!deps.wallet().walletExists()) // test hook
+ deps.wallet().createWallet(null).getOrThrow() // test hook
+ deps.migration().markMigrationChecked() // test hook
+ deps.settings().update { it.copy(electrumServer = "tcp://127.0.0.1:1") } // test hook
+ "created disposable unfunded wallet" // test hook
+ } // test hook
+ "reviewInit" -> {
+ to.bitkit.async.ServiceQueue.CORE.background {
+ com.synonym.bitkitcore.initDb(to.bitkit.env.Env.bitkitCoreStoragePath(0))
+ }
+ deps.pubky().initialize()
+ "initialized native database and profile service through their real APIs"
+ }
+ "reviewFlag" -> {
+ val parts = requireNotNull(arg).split(":")
+ require(parts[0] in listOf("startup", "profile", "probe"))
+ val file = java.io.File(requireNotNull(context).filesDir, "review-${parts[0]}")
+ if (parts[1] == "on") file.writeText("enabled") else file.delete()
+ "fault $arg"
+ }
+ "reviewClearMarkers" -> {
+ deps.keychain().delete("PAYKIT_PENDING_BACKUP_RESTORE")
+ deps.keychain().delete("USDT_PENDING_BACKUP_RESTORE")
+ "cleared disposable markers"
+ }
+ "reviewCounters" -> "startupToasts=${startupToasts.get()}"
+ "reviewVmRetryState" -> kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.Main) {
+ walletVm().onRestoreRetry()
+ "ordinary retry requested"
+ }
+ "reviewVmState" -> kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.Main) {
+ val vm = walletVm()
+ "restore=${vm.restoreState.value} error=${vm.connectionStartupError.value?.message} clearance=${vm.connectionStartupError.value?.canProceedWithoutRestore} editor=${vm.connectionEditor.value?.let { "id=${it.id},busy=${it.busy},owner=${it.owner != null},saved=${it.saved}" }}"
+ }
+ "reviewRetry" -> {
+ kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.Main) { walletVm().onBackupRestoreRetry() }.join()
+ "backup retry finished"
+ }
+ "reviewWipe" -> {
+ savedSeed = deps.keychain().loadString("BIP39_MNEMONIC")
+ kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.Main) { walletVm().wipeWallet() }.join()
+ check(!deps.wallet().walletExists())
+ "disposable wallet wiped through production ViewModel"
+ }
+ "reviewRestore" -> {
+ kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.Main) { walletVm().restoreWallet(checkNotNull(savedSeed), null) }
+ "disposable seed restored through production ViewModel"
+ }
+ "reviewInvoice" -> {
+ reviewInvoice = deps.lightning().receive(1000uL, "disposable restore gate fixture", 3600u)
+ "signed valid regtest Lightning invoice for admission check"
+ }
+ "reviewAddress" -> deps.lightningRepo().newAddress().getOrThrow()
+ "reviewElectrum" -> {
+ deps.settings().update { it.copy(electrumServer = requireNotNull(arg)) }
+ "Electrum setting changed through SettingsStore"
+ }
+ "reviewCurrency" -> {
+ deps.settings().update { it.copy(selectedCurrency = requireNotNull(arg)) }
+ "currency changed"
+ }
+ "reviewPrepare" -> {
+ deps.lightningRepo().sync().getOrThrow()
+ val address = deps.lightning().newAddress()
+ val attempt = deps.attempts().admit(
+ walletId = to.bitkit.models.WalletScope.default, requestId = null, orderId = null,
+ address = address, amountSats = 5000uL, isMaxAmount = false, feeRateSatsPerVByte = 2uL,
+ isTransfer = false, channelId = null, tags = emptyList(), beforeSendAttempt = {})
+ val signed = deps.lightning().prepareOnchainSend(address, 5000uL, 2uL)
+ deps.attempts().retainPreparedReceipt(attempt.attemptId, attempt.walletIndex, signed.receipt, false)
+ prepared = signed
+ deps.reservations().restoreBackup(mapOf("p2wpkh" to 12)).getOrThrow()
+ deps.reservations().reconcileReservedIndexesWithLdk().getOrThrow()
+ "prepared unsigned-dispatch attempt=${attempt.attemptId} txid=${signed.receipt.txid}; reservation p2wpkh=12"
+ }
+ "reviewAttempt" -> {
+ val attempt = deps.attempts().currentDurable()
+ "attempt=${attempt?.attemptId} txid=${attempt?.txid} inputs=${attempt?.originalInputs?.size} dispatched=${attempt?.initialDispatchAttempted} restored=${attempt?.restoredFromBackup} reservations=${deps.reservations().backupSnapshot().getOrThrow()}"
+ }
+ "reviewGates" -> {
+ fun blocked(error: Throwable?) {
+ check(generateSequence(error) { it.cause }.any { it is to.bitkit.data.keychain.WalletBackupRestorePendingError }) { "unexpected gate result: $error" }
+ }
+ blocked(deps.lightningRepo().newAddress().exceptionOrNull())
+ blocked(deps.lightningRepo().payInvoice(reviewInvoice ?: "invalid-unused-invoice").exceptionOrNull())
+ blocked(deps.lightningRepo().sendOnChain(deps.attempts().currentDurable()?.address ?: "invalid-unused-address", 1000uL).exceptionOrNull())
+ blocked(runCatching { deps.lightning().newAddress() }.exceptionOrNull())
+ if (prepared != null) blocked(runCatching { prepared!!.broadcast {} }.exceptionOrNull())
+ "repository BTC/LN/address and native address gates rejected; validInvoice=${reviewInvoice != null} prepared=${prepared != null}"
+ }
+ "reviewBackup" -> { // test hook
+ deps.vss().setup().getOrThrow() // test hook
+ for (category in (arg?.let { listOf(to.bitkit.models.BackupCategory.valueOf(it)) } ?: listOf(to.bitkit.models.BackupCategory.WALLET, to.bitkit.models.BackupCategory.SETTINGS))) { // test hook
+ deps.backup().triggerBackup(category).getOrThrow() // test hook
+ } // test hook
+ "acknowledged WALLET and SETTINGS backups" // test hook
+ } // test hook
+ "reviewFault" -> { // test hook
+ val file = java.io.File(requireNotNull(context).filesDir, "review-vss-unavailable") // test hook
+ if (arg == "on") file.writeText("enabled") else file.delete() // test hook
+ deps.vss().reset() // test hook
+ "VSS endpoint fault ${arg}" // test hook
+ } // test hook
+ "reviewHold" -> { // test hook
+ val file = java.io.File(requireNotNull(context).filesDir, "review-vss-hold") // test hook
+ if (arg == "on") file.writeText("enabled") else file.delete() // test hook
+ "VSS get hold ${arg}" // test hook
+ } // test hook
+ "reviewPending" -> { // test hook
+ deps.keychain().upsertString("PAYKIT_PENDING_BACKUP_RESTORE", "") // test hook
+ deps.keychain().upsertString("USDT_PENDING_BACKUP_RESTORE", "") // test hook
+ "persisted both pending markers" // test hook
+ } // test hook
+ "reviewState" -> { // test hook
+ val settings = deps.settings().data.first() // test hook
+ "paykit=${deps.keychain().exists("PAYKIT_PENDING_BACKUP_RESTORE")} usdt=${deps.keychain().exists("USDT_PENDING_BACKUP_RESTORE")} electrum=${settings.electrumServer} rgs=${settings.rgsServerUrl} node=${deps.lightningRepo().lightningState.value.nodeLifecycleState}" // test hook
+ } // test hook
+ "reviewNativeGate" -> { // test hook
+ val error = runCatching { deps.lightning().newAddress() }.exceptionOrNull() // test hook
+ check(generateSequence(error) { it.cause }.any { it is to.bitkit.data.keychain.WalletBackupRestorePendingError }) { "Unexpected native result: $error" } // test hook
+ "native newAddress blocked by ${checkNotNull(error).javaClass.simpleName}" // test hook
+ } // test hook
+ else -> error("Unknown test hook") // test hook
+ }.also { android.util.Log.i("ReviewHook", it) } // test hook
+ }.let { bundleOf("result" to it) } // test hook
+ }.getOrElse { bundleOf("error" to "${it.javaClass.simpleName}: ${it.message}") } // test hook
+ } // test hook
return runCatching {
val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
ServiceQueue.LDK.blocking { command.execute(deps) }
@@ -51,7 +195,33 @@ class DevToolsProvider : ContentProvider() {
}.toBundle()
}
- override fun onCreate() = true
+ private var activity: java.lang.ref.WeakReference<android.app.Activity>? = null
+ private var savedSeed: String? = null
+ private val startupToasts = java.util.concurrent.atomic.AtomicInteger()
+ private var reviewInvoice: String? = null
+ private var prepared: to.bitkit.repositories.PreparedOnchainSend? = null
+ private fun walletVm(): to.bitkit.viewmodels.WalletViewModel =
+ androidx.lifecycle.ViewModelProvider(checkNotNull(activity?.get()) as androidx.activity.ComponentActivity)[to.bitkit.viewmodels.WalletViewModel::class.java]
+ override fun onCreate(): Boolean {
+ kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.SupervisorJob() + kotlinx.coroutines.Dispatchers.IO).launch {
+ to.bitkit.ui.shared.toast.ToastEventBus.events.collect {
+ if (it.description == "Injected storage unavailable") {
+ android.util.Log.i("ReviewHook", "startup toast count=${startupToasts.incrementAndGet()}")
+ }
+ }
+ }
+ (requireNotNull(context).applicationContext as android.app.Application).registerActivityLifecycleCallbacks(
+ object : android.app.Application.ActivityLifecycleCallbacks {
+ override fun onActivityCreated(a: android.app.Activity, b: Bundle?) { if (a is to.bitkit.ui.MainActivity) activity = java.lang.ref.WeakReference(a) }
+ override fun onActivityStarted(a: android.app.Activity) {}
+ override fun onActivityResumed(a: android.app.Activity) { if (a is to.bitkit.ui.MainActivity) activity = java.lang.ref.WeakReference(a) }
+ override fun onActivityPaused(a: android.app.Activity) {}
+ override fun onActivityStopped(a: android.app.Activity) {}
+ override fun onActivitySaveInstanceState(a: android.app.Activity, b: Bundle) {}
+ override fun onActivityDestroyed(a: android.app.Activity) {}
+ })
+ return true
+ }
override fun getType(uri: Uri): String? = null
override fun insert(uri: Uri, values: ContentValues?): Uri? = null
override fun delete(uri: Uri, sel: String?, args: Array<String>?) = 0
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
index eef9af0b1..2d603482d 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -46,7 +46,7 @@ class VssBackupClient @Inject constructor(
withTimeout(30.seconds) {
Logger.debug("VSS client setting up…", context = TAG)
- val vssUrl = Env.vssServerUrl
+ val vssUrl = if (Env.isDebug && java.io.File(Env.logDir.parentFile, "review-vss-unavailable").exists()) "http://127.0.0.1:1/vss" else Env.vssServerUrl // test hook
val lnurlAuthServerUrl = Env.lnurlAuthServerUrl
val vssStoreId = vssStoreIdProvider.getVssStoreId(walletIndex)
Logger.verbose("Building VSS client with vssUrl: '$vssUrl'", context = TAG)
@@ -136,6 +136,11 @@ class VssBackupClient @Inject constructor(
}
suspend fun getObject(key: String): Result<VssItem?> = withContext(ioDispatcher) {
+ if (Env.isDebug) { // test hook
+ val hold = java.io.File(Env.logDir.parentFile, "review-vss-hold") // test hook
+ if (hold.exists()) Logger.info("Held VSS read until explicit release", context = "ReviewHook") // test hook
+ while (hold.exists()) delay(100) // test hook
+ } // test hook
isSetup.await()
Logger.verbose("VSS 'getObject' call for '$key'", context = TAG)
runCatching {
diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
index bce9e7dbf..9246f527e 100644
--- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
@@ -1035,6 +1035,10 @@ class LightningRepo @Inject constructor(
}
suspend fun validateElectrumServer(url: String): Result<Unit> = withContext(bgDispatcher) {
+ if (Env.isDebug) {
+ val hold = java.io.File(Env.logDir.parentFile, "review-probe")
+ while (hold.exists()) kotlinx.coroutines.delay(100)
+ }
runSuspendCatching { ElectrumServer.parse(url) }
.fold(
onSuccess = { electrumProbeService.probe(it) },
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 39a03aefb..16152eaf2 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -351,6 +351,7 @@ class PubkyRepo @Inject constructor(
}
suspend fun initialize() = withContext(ioDispatcher) {
+ if (Env.isDebug && runCatching { java.io.File(Env.logDir.parentFile, "review-profile").exists() }.getOrDefault(false)) throw to.bitkit.utils.AppError("Injected profile cache unavailable")
val restored = initializeMutex.withLock { initializeSession() }
if (restored) loadIdentityData()
checkAdoptedSource()
diff --git a/app/src/main/java/to/bitkit/services/LightningService.kt b/app/src/main/java/to/bitkit/services/LightningService.kt
index a30dadb99..d57f2c2dc 100644
--- a/app/src/main/java/to/bitkit/services/LightningService.kt
+++ b/app/src/main/java/to/bitkit/services/LightningService.kt
@@ -397,6 +397,7 @@ class LightningService internal constructor(
}
suspend fun start(timeout: Duration? = null, onEvent: NodeEventHandler? = null) {
+ if (Env.isDebug && java.io.File(Env.logDir.parentFile, "review-startup").exists()) throw to.bitkit.utils.AppError("Injected storage unavailable")
val node = this.node ?: throw ServiceError.NodeNotSetup()
Logger.debug("Starting node…", context = TAG)Reviewer model: gpt-6.1-sol
Reasoning effort: high
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: full reassessment of the whole PR at 7b823d2a, against merge base 416c1e35 (the head of #1681). A full reassessment was needed because the comparison base moved from 2493dbb4 to 416c1e35, and that change touched LightningService.kt and its tests. No earlier coverage was inherited. Every changed file was inspected: the admission gates in LightningRepo and LightningService, WalletConnectionRecovery (owner/generation admission, wipe drain, save/replay/complete), the session threading in BackupRepo restore, the identity changes in WalletRepo and WipeWalletUseCase, the retry/proceed/startup flow in WalletViewModel, the gating in ContentView, both recovery views, strings, unit/Compose tests and the J1/J2 journeys.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The operation gates cover every new address, invoice, Lightning/on-chain send, prepared or release broadcast, channel open/close, RBF/CPFP and LNURL withdraw/channel entry point at both the repository and native layers. No ungated spend or receive path was found. Nested restore steps reuse the admitted session rather than taking the operation mutex again, and no lock-order inversion was found between the operation and startup mutexes. The RN-migration start still passes channel migration, because startNode now builds it internally.
One MEDIUM finding: any node-start failure for an already-settled wallet now opens the full-screen restore-error view. The iOS twin synonymdev/bitkit-ios#1139 at 9481b503 was used for parity only (not reviewed in full). It limits that screen to a pending restore or a saved connection correction.
Validation: this is source analysis only. No Gradle build or unit tests were run locally. Hosted checks at this head pass: build, lint and the local e2e shards. The authenticated J1/J2 journeys remain unrun, as the PR states.
Device testing: not performed in this review.
Suggested additional test cases:
- Android, existing settled wallet with no restore markers. Set Settings › Electrum to an unreachable server, background the app, then foreground it. Expected: the wallet home stays available with a node-error toast and status, not the "Spending balance error" restore screen. This matches iOS.
- Android, unit test. With
walletExists = trueandrestoreState = Settled, makelightningRepo.startfail and callsut.start(). Expected:connectionStartupErrorstays null, or the screen-level gate is not triggered without a pending restore or a saved correction.
Earlier findings reconciled at this revision: 5 verified fixed.
Findings
- [MEDIUM] Ordinary foreground node-start failure now replaces the wallet with the restore-error screen — inline at
app/src/main/java/to/bitkit/ui/ContentView.kt:445.
Reviewer model: claude-opus-5-5
Reasoning effort: high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed to f9ff9d2. Both of my threads are fixed, and I have no new findings.
- MEDIUM, restart on every foreground and the doubled retry count: fixed in the merge commit
7b823d2(it doesn't show if you diff8d72053..f9ff9d2without merge commits).AppliedConnection(walletIndex, generation, revision)is recorded when the native start succeeds.isRestoreRetrynow requiresrevision != null && appliedConnection != connection(WalletViewModel.kt:743,:574), andcountRetry()runs only when!backupOnly(:575). - LOW, startup error surviving a wipe: fixed by the wallet-removal reset at
:435-439. The regression tests I proposed are inWalletViewModelTest. f9ff9d2itself fixes piotr-iohk's ContentView thread throughshouldPresentWalletRecoveryandConnectionStartupFailure.requiresRecovery. An ordinary cold-start or foreground failure on a settled wallet now behaves as on master (toast plus wallet UI), and the fail-closed paths are kept.
Out of scope, LOW: Proceed with a saved but unapplied correction stores onboarding__connection_saved as a message that WalletRestoreErrorView never renders, so Proceed disappears without explanation until Try Again.
Device gate (partial): f9ff9d2, not run yet. Pending:
pending-wallet-recovery.xmlandconnection-recovery.xml.- With a saved correction, three foregrounds log one
Restarting nodein total. - A settled wallet with unreachable Electrum keeps Home plus the toast, on foreground and on cold start.
- Forgot PIN → restore shows initializing, not "Restore failed".
The REQUEST_CHANGES stays until this runs.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up review of f9ff9d27 since 7b823d2a (baseline). The merge base is unchanged at 416c1e35. This pass covered the new startup-failure classification and screen gate; unchanged restore admission, native dispatch, recovery sessions, backup restore, wallet create/wipe, and the prior journey contracts stay inherited.
No new actionable code findings.
An ordinary foreground node failure with readable, clear restore markers and no saved connection correction leaves the wallet and Settings in place. Pending or unreadable markers, and a saved or unreadable correction, still open the restricted recovery screen. Proceed permission stays the marker check. That split matches requiresRestrictedStartup on bitkit-ios#1139 at 93181d59. The four-file delta does not change the bitkit-android#1681 contract.
Validation: the new unit tests were inspected and not executed here. Device testing: not performed in this review. Hosted E2E on this revision failed in the onboarding/backup and onchain shards because deleteWidgets never saw “Yes, Delete” in the backup spec. The same suite ref passed on the previous head. No path from this gate to that dialog showed up in the delta.
Suggested additional test cases
- Android, disposable wallet, both restore markers clear, no saved Electrum/RGS correction. Force the next foreground node start to fail. Home, the menu, and Settings stay available, and the restore-failed screen stays off. Repeat with a pending marker or a saved correction: the restricted recovery screen appears and wallet actions stay unavailable.
Device testing: not performed in this review.
Earlier findings reconciled at this revision: 4 verified fixed.
Reviewer model: grok-4.7
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the whole PR at 825bb3a9 against merge base 8a176f03. The comparison base moved from 416c1e3, and no earlier coverage was inherited.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
A required-restore refusal during the on-chain dispatch write-ahead leaves that attempt pending even though native broadcast did not run. Receive, payment, channel, and fee admission otherwise fail closed while either durable marker is present or unreadable. Proceed still depends on affirmative marker clearance, and an ordinary foreground failure with clear markers and no saved correction keeps the wallet available. The earlier Proceed, duplicate-toast, repeated-restart, wipe, and foreground-routing threads do not reproduce at this revision.
Compared the startup contract with bitkit-ios#1139 at 93181d59: iOS holds node start until required restore finishes, while Android keeps the node up and gates dispatch. That iOS pin was not fully reviewed. Parent bitkit-android#1681 at the merge base supplies the attempt store; this PR adds the checks that run after its write-ahead.
Validation: CI and lint passed on this revision. E2E was still in progress, so it is not a result. Unit and Compose tests were inspected and not executed. J1 and J2 have not been run.
Suggested additional test cases
- Android, disposable wallet, both restore markers clear, with an on-chain send prepared and inside broadcast. Set
PAYKIT_PENDING_BACKUP_RESTOREwhileinitialDispatchAttemptedis saved and before native broadcast. The transaction is not broadcast, the attempt does not stay dispatch-attempted, and the next send is not blocked by it.
Device testing: not performed in this review.
Findings
- [MEDIUM] Restore refusal after the dispatch write-ahead leaves the send pending — inline at
app/src/main/java/to/bitkit/services/LightningService.kt:1025.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Verdict:
Findings: No new actionable findings. The previously reported restart, wipe, ordinary-startup routing and dispatch-refusal repairs hold in the checks below.
QA for 2986fde: Android API 36, disposable BTC regtest wallet, staging Electrum and LNURL-authenticated encrypted VSS. The uncommitted debug controls staged failures and invoked production recovery and dispatch paths. The USDT recovery journal was empty.
Test T4: saved correction across foregrounds and retries ✅ Three foregrounds and a backup retry did not rebuild the node for the unchanged correction.
T4redo-2x.2x.mp4
Device logs showed no Restarting node after the correction had been applied. Both markers stayed present; the backup failure count advanced once. A separate optional-profile failure with clear markers also advanced once on retry.
Test T5: wipe followed by restore ✅ The old startup error cleared before the next restore and did not cover initialization.
T5redo-2x.2x.mp4
A hook invoked the production wipe and restore ViewModel entry points and held the metadata read. State reads showed the old error cleared after wipe and during the new restore. The PIN and Forgot PIN screens were not driven.
Test T7: ordinary and pending startup failures ✅ Home and Settings remained available with clear markers, while pending markers kept recovery restricted.
T7-2x.2x.mp4
An injected startup failure was checked on cold start and foreground. State reads confirmed clear markers for the normal wallet and both markers present for restricted recovery.
Test T8: refusal after dispatch write-ahead ✅ A restore marker inserted after the durable write produced a pre-native refusal and released the local attempt.
| Refusal | Next preparation |
|---|---|
T8-2x.2x.mp4 |
T8b-2x.2x.mp4 |
The hook used a real signed regtest preparation, inserted the marker immediately after the journal callback and checked the typed refusal, the empty attempt store and that the wallet did not know the transaction. After clearing the marker, a fresh preparation succeeded. The recordings show the running app; the assertions were checked through the hook and device logs.
Test J1: pending wallet recovery admission ✅ The BTC fixture retained admission restrictions through failure and restart, then recovered its attempt and reservation state.
| Pending admission | Restart and retries | Authenticated recovery | Marker recheck |
|---|---|---|---|
J1-2x.2x.mp4 |
J2restart-2x.2x.mp4 |
Jrestore-2x.2x.mp4 |
Jprofile-2x.2x.mp4 |
Hook assertions checked BTC and Lightning sends and address allocation with a valid destination and signed invoice. T8 exercises a prepared broadcast at the dispatch boundary. Two overlapping retries produced one restore start while a VSS read was held. After authenticated import, the original attempt and its input and reservation ceiling remained; both markers cleared. An injected optional-profile failure exposed Proceed, and a marker added before confirmation prevented dismissal. This fixture has an empty USDT journal.
Test J2: connection recovery ⏭️ Partial: a non-default RGS correction followed by a failed replacement probe and replay remains unverified at this head.
| Editor | Restart | Selective replay | Optional profile |
|---|---|---|---|
J2-2x.2x.mp4 |
J2restart-2x.2x.mp4 |
Jrestore-2x.2x.mp4 |
Jprofile-2x.2x.mp4 |
The restricted editor saved the Electrum correction, rejected an RGS probe, and saved the default RGS setting. Both corrections survived restart and authenticated restoration of older SETTINGS: direct settings reads showed the corrected endpoints and the unrelated USD preference restored over local EUR. The node stayed running through the repeated foregrounds in T4. Required markers cleared before the optional-profile controls appeared. The earlier 8d72053 device report drove the non-default RGS slice; this run did not repeat that precise sequence.
Check 1: focused recovery and dispatch tests ✅ The head build and focused service, ViewModel and screen-state unit suites passed.
The dispatch tests cover both required markers, unreadable storage, rollback failure, wallet or journal replacement, prior and imported candidates, and post-native uncertainty. The device fault controls were present in the focused test build.
Check 2: hosted E2E ⚠️ Two shards failed on this head.
The onboarding/backup and on-chain/widgets shards timed out waiting for WidgetSave on all three attempts; the latter also reported widget lookup timeouts. Their cause is not established. CI and lint passed; migration was skipped.
Coverage: delta 8d72053..2986fde, including the open lifecycle, startup and dispatch findings. The base-composition changes were also considered against the full 825bb3a review. The uncertain/imported dispatch safeguards were exercised by controlled tests, not a device broadcast.
Twin: synonymdev/bitkit-ios#1139 implements the same restricted connection recovery, with required restore before node startup; Android keeps the node available for restoration and gates operations.
Reviewer model: Unknown
Reasoning effort: Not exposed
The available session record did not identify the model or reasoning setting.
Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
Dismissed per @ovitrif: this review was on an older head, its threads are resolved, and the PR has moved on. @jvsena42 is off until 1 Nov; the current head needs a fresh review from @ben-kaufman or @piotr-iohk.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving at 2986fde. Both of my threads were fixed in 7b823d2, and 2986fde adds no finding: PreparedOnchainSendRefused is thrown only before prepared.broadcast(). Rollback also needs the local preparation captured before the callback and an unchanged journal, so a send that reached the network stays guarded.
Device gate: 2986fde, from ovi-reviewer's run at this head (#1685 (review)). It covers every item my f9ff9d2 review left pending:
- three foregrounds with a saved correction log one
Restarting node(T4); - the startup error is gone after Forgot PIN, and restore shows initializing (T5);
- a settled wallet with unreachable Electrum keeps Home plus the toast (T7);
- a send refused before native dispatch is released and the next preparation succeeds (T8);
pending-wallet-recovery.xmlpassed (J1).
Skipped, because ovi-reviewer already drove these at this head: J1 and T4/T5/T7/T8.
connection-recovery.xml (J2), meaning a non-default RGS, a failed replacement probe and the replay. It rests on the code scan and unit tests.
Perf: not relevant.
The hosted E2E at 2986fde has two shard failures (WidgetSave timeouts) unrelated to this PR; it needs a re-run before merge.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up of 2986fde since 825bb3a, with merge base 8a176f0. The inherited baseline is the previous review. This pass rechecked the pre-native on-chain refusal release. Unchanged address, invoice, Lightning, and channel admission, connection recovery, startup routing, wallet identity, and the prior journeys stay inherited.
No new actionable code findings.
A prepared on-chain handle now fails as PreparedOnchainSendRefused after the dispatch write-ahead and before native broadcast(). The journal is released only for that first local in-flight preparation when the reloaded record is still the same preparation. Recovery and RBF retries, imported or already-dispatched candidates, a replaced wallet or journal, a failed rollback, and an exception from native broadcast remain guarded. A Shop payment keeps the guard until the existing proof cleanup removes it. iOS #1139 at 911f24c uses the same split: a refusal immediately before native broadcast is not submitted, and a native broadcast failure stays guarded.
Device testing: not performed in this review. The new LightningServiceTest cases were inspected and not executed.
Earlier findings reconciled at this revision: 4 verified fixed.
Reviewer model: grok-4.7
Reasoning effort: high
|
@ben-kaufman pls fix conflicts that appeared after target PR got merged and base branch switched to |
|
Retargeted this PR to master before merging its base, #1681, so deleting that branch would not close it. The base change dismissed the earlier approval, so it needs a re-approval. |
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — required recovery can expose Home while its download is still pending.
Findings:
- MEDIUM — keep recovery visible during automatic retry.
app/src/main/java/to/bitkit/ui/ContentView.kt:251. After repairing a cold-start connection failure, Try Again can expose Home before required restoration completes. The new retry continuation changesBackupFailedtoInProgress.Metadata; if Compose misses the briefInitializingstate, every recovery-screen predicate becomes false. This is a newly discovered issue in the original PR, unchanged by the merge. Repository/native operation guards remained active.
QA for 5b4674c: Android API 36, disposable unfunded BTC regtest wallet, staging Electrum and real LNURL-authenticated encrypted VSS. The uncommitted debug controls below stage the backup and failures and read production state. The USDT journal is empty.
Test J2: connection recovery ❌ The non-default RGS correction survives failed probing and authenticated replay, but node Retry exposes Home during required restoration.
| Home before required completion | Authenticated restoration |
|---|---|
 |
v4-1685-J2-complete-2x.2x.mp4 |
Captured Home during the refused required download; subsequent successful recovery at 2× speed.
From a cold-start node error with both markers set, I saved reachable Electrum and explicit-port RGS corrections. A failed RGS replacement probe displayed RecoveryError and retained the acknowledged RGS. Direct settings reads confirmed both corrections survived restart. After Back → Try Again, the node restarted and the required download began, but Home appeared before any required import succeeded. Both markers were still pending; the screenshot and production restore log establish that timing.
After restoring backup access, real SETTINGS and WALLET imports succeeded. Direct reads showed remote USD replacing local EUR while the corrected Electrum and https://bitkit.stag0.blocktank.to:443/rgs/snapshot remained. Both markers then cleared and Wallet Restored appeared. This completes the previously missing RGS checks, while the recovery-navigation expectation fails. Unchanged optional-profile and repeated-foreground checks retain the prior report's scope below.
Test T9: invoice admission after conflict resolution ✅ Both invoice paths refuse pending recovery and create regtest invoices after authenticated recovery.
| Required markers pending | Required markers clear |
|---|---|
v4-1685-T9-pending-2x.2x.mp4 |
v4-1685-T9-clear-2x.2x.mp4 |
Stationary app captures at 2× speed; assertions were checked through the debug controls and device logs.
With the node Running and both markers set, sats and msats calls at both the repository and native service boundaries returned WalletBackupRestorePendingError. After authenticated imports cleared both markers, both repository calls returned signed regtest invoices. No payment was sent.
Check 1: build, focused regressions and hosted checks ✅ The exact-head app build and focused recovery suites passed; all seven current-head E2E shards report success.
I ran assembleDevDebug testDevDebugUnitTest for WalletConnectionRecoveryTest, WalletViewModelTest, LightningServiceTest, LightningRepoTest and BackupRepoTest, then built the debug controls separately. The installed test APK matched the built bytes.
Current-head CI, lint and E2E shard statuses are successful. Earlier widget failures remain scoped to their earlier revision; these statuses do not validate the recovery-navigation case.
Coverage: contiguous delta 2986fde..5b4674c, both invoice conflict resolutions, affected callers and earlier findings. The previous device report supplies unchanged T4/T5/T7/T8/J1 and the other J2 steps; they were not re-driven here. Their signed BTC preparation, reservation, lifecycle and optional-profile evidence retains its original revision and fixture limits. Incoming master transfer/peer behavior was reviewed for integration but received no separate device campaign. No additional planned case remains unrun.
Twin: bitkit-ios#1139 fix: recover connections during wallet restore at 753d6d1f matches restricted endpoint recovery and durable selective replay. iOS completes required restoration before node startup; Android keeps the node available and gates operations. The twin was compared in source, not driven here.
Uncommitted test controls and deciding assertions
The controls create only a disposable wallet, invoke real backup APIs, stage older settings and required markers, redirect VSS to a refused local endpoint, read settings, and assert production invoice results. They do not replace native results or recovery decisions. Built and installed test APK SHA-256: 03c70b332186aa66029adc935a1f7a732c970f81a7966527c23ce9bcdc74297c.
paykit=true usdt=true ... rgs=https://bitkit.stag0.blocktank.to:443/rgs/snapshot currency=EUR
Full restore starting
[Home captured before any successful required import]
sats and msats invoice repository and native gates rejected required recovery
Restore success for: 'SETTINGS'
Restore success for: 'WALLET'
Full restore success
paykit=false usdt=false ... rgs=https://bitkit.stag0.blocktank.to:443/rgs/snapshot currency=USD
sats and msats regtest invoices created after acknowledged recovery
diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 00c57a69a..0e330e50d 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -11,6 +11,7 @@ import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.android.EntryPointAccessors
import dagger.hilt.components.SingletonComponent
+import kotlinx.coroutines.flow.first // test hook
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import to.bitkit.async.ServiceQueue
@@ -32,6 +33,14 @@ class DevToolsProvider : ContentProvider() {
@EntryPoint
@InstallIn(SingletonComponent::class)
interface Dependencies {
+ fun keychain(): to.bitkit.data.keychain.Keychain // test hook
+ fun settings(): to.bitkit.data.SettingsStore // test hook
+ fun wallet(): to.bitkit.repositories.WalletRepo // test hook
+ fun backup(): to.bitkit.repositories.BackupRepo // test hook
+ fun vss(): to.bitkit.data.backup.VssBackupClient // test hook
+ fun migration(): to.bitkit.services.MigrationService // test hook
+ fun lightning(): to.bitkit.services.LightningService // test hook
+ fun pubky(): to.bitkit.repositories.PubkyRepo // test hook
fun lightningRepo(): LightningRepo
}
@@ -42,6 +51,75 @@ class DevToolsProvider : ContentProvider() {
override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+ if (method.startsWith("review")) { // test hook
+ return runCatching { // test hook
+ kotlinx.coroutines.runBlocking(kotlinx.coroutines.Dispatchers.IO) { // test hook
+ when (method) { // test hook
+ "reviewSeed" -> { // test hook
+ check(!deps.wallet().walletExists()) // test hook
+ deps.wallet().createWallet(null).getOrThrow() // test hook
+ deps.migration().markMigrationChecked() // test hook
+ "created disposable wallet" // test hook
+ } // test hook
+ "reviewInit" -> { // test hook
+ to.bitkit.async.ServiceQueue.CORE.background { // test hook
+ com.synonym.bitkitcore.initDb(to.bitkit.env.Env.bitkitCoreStoragePath(0)) // test hook
+ } // test hook
+ deps.pubky().initialize() // test hook
+ "initialized real native database and profile service" // test hook
+ } // test hook
+ "reviewBackup" -> { // test hook
+ deps.vss().setup().getOrThrow() // test hook
+ for (category in listOf(to.bitkit.models.BackupCategory.WALLET, to.bitkit.models.BackupCategory.SETTINGS)) { // test hook
+ deps.backup().triggerBackup(category).getOrThrow() // test hook
+ } // test hook
+ "acknowledged WALLET and SETTINGS backup" // test hook
+ } // test hook
+ "reviewSettings" -> { // test hook
+ deps.settings().update { it.copy(electrumServer = "tcp://127.0.0.1:1", rgsServerUrl = "https://127.0.0.1:1/snapshot", selectedCurrency = "USD") } // test hook
+ "old unreachable endpoints and USD staged for backup" // test hook
+ } // test hook
+ "reviewCurrency" -> { // test hook
+ deps.settings().update { it.copy(selectedCurrency = requireNotNull(arg)) } // test hook
+ "currency changed" // test hook
+ } // test hook
+ "reviewPending" -> { // test hook
+ deps.keychain().upsertString("PAYKIT_PENDING_BACKUP_RESTORE", "") // test hook
+ deps.keychain().upsertString("USDT_PENDING_BACKUP_RESTORE", "") // test hook
+ "persisted both required markers" // test hook
+ } // test hook
+ "reviewFault" -> { // test hook
+ val flag = java.io.File(requireNotNull(context).filesDir, "review-vss-unavailable") // test hook
+ if (arg == "on") flag.writeText("enabled") else flag.delete() // test hook
+ deps.vss().reset() // test hook
+ "VSS unavailable=$arg" // test hook
+ } // test hook
+ "reviewState" -> { // test hook
+ val settings = deps.settings().data.first() // test hook
+ "paykit=${deps.keychain().exists("PAYKIT_PENDING_BACKUP_RESTORE")} usdt=${deps.keychain().exists("USDT_PENDING_BACKUP_RESTORE")} electrum=${settings.electrumServer} rgs=${settings.rgsServerUrl} currency=${settings.selectedCurrency} node=${deps.lightningRepo().lightningState.value.nodeLifecycleState}" // test hook
+ } // test hook
+ "reviewInvoicesBlocked" -> { // test hook
+ val errors = listOf( // test hook
+ deps.lightningRepo().createInvoice(1uL, "restore admission").exceptionOrNull(), // test hook
+ deps.lightningRepo().createInvoiceMsats(1001uL, "restore admission").exceptionOrNull(), // test hook
+ runCatching { deps.lightning().receive(1uL, "native restore admission") }.exceptionOrNull(), // test hook
+ runCatching { deps.lightning().receiveMsats(1001uL, "native restore admission") }.exceptionOrNull(), // test hook
+ ) // test hook
+ check(errors.all { generateSequence(it) { error -> error.cause }.any { error -> error is to.bitkit.data.keychain.WalletBackupRestorePendingError } }) // test hook
+ "sats and msats invoice repository and native gates rejected required recovery" // test hook
+ } // test hook
+ "reviewInvoicesClear" -> { // test hook
+ check(!deps.keychain().exists("PAYKIT_PENDING_BACKUP_RESTORE")) // test hook
+ check(!deps.keychain().exists("USDT_PENDING_BACKUP_RESTORE")) // test hook
+ val invoices = listOf(deps.lightningRepo().createInvoice(1uL, "restore admission").getOrThrow(), deps.lightningRepo().createInvoiceMsats(1001uL, "restore admission").getOrThrow()) // test hook
+ check(invoices.all { it.startsWith("lnbcrt") }) // test hook
+ "sats and msats regtest invoices created after acknowledged recovery" // test hook
+ } // test hook
+ else -> error("Unknown review control") // test hook
+ }.also { android.util.Log.i("ReviewHook", it) } // test hook
+ }.let { bundleOf("result" to it) } // test hook
+ }.getOrElse { bundleOf("error" to "${it.javaClass.simpleName}: ${it.message}") } // test hook
+ } // test hook
return runCatching {
val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
ServiceQueue.LDK.blocking { command.execute(deps) }
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
index eef9af0b1..23ef26785 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -46,7 +46,7 @@ class VssBackupClient @Inject constructor(
withTimeout(30.seconds) {
Logger.debug("VSS client setting up…", context = TAG)
- val vssUrl = Env.vssServerUrl
+ val vssUrl = if (Env.isDebug && java.io.File(Env.logDir.parentFile, "review-vss-unavailable").exists()) "http://127.0.0.1:1/vss" else Env.vssServerUrl // test hook
val lnurlAuthServerUrl = Env.lnurlAuthServerUrl
val vssStoreId = vssStoreIdProvider.getVssStoreId(walletIndex)
Logger.verbose("Building VSS client with vssUrl: '$vssUrl'", context = TAG)Reviewed by gpt-6.1-sol-high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of c747cc0b against merge base f9d4b4fb, following the comparison-base change; no coverage inherited. Recovery contracts were compared with iOS at be804c7c, without a full iOS review.
No new actionable code findings.
The recovery-visibility repair is supported by the pinned source and regression assertions. The previously noted LOW concern about missing feedback for an unapplied correction remains present; it is not duplicated here.
Validation: diff checks and changed-XML validation passed. Local JVM tests stopped before execution because the Android SDK was unavailable. Hosted CI and lint passed on synthetic merge 146c0d6a, incorporating later base 204df78e; exact pinned-composition execution remains unverified. E2E finished successfully after multi_address_3 failed its transaction-ID-change assertion and passed on retry; one settings case was skipped. The first-attempt failure's cause is unproven.
Device testing: not performed in this review. Current-head authenticated J1/J2 and visual parity remain untested here; earlier device reports retain their original revisions and fixture limits.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
There was a problem hiding this comment.
Verdict: ✅ Approve — the required-recovery visibility repair holds.
Findings: No new actionable findings. The previous routing finding is fixed: in-progress restoration selects recovery directly, and Retry retains the required startup error through replay and restart.
QA for c747cc0: Android API 36, disposable unfunded BTC regtest wallet, staging Electrum and real LNURL-authenticated encrypted VSS, with the uncommitted controls below. The independent USDT journal is empty.
Test J2: connection recovery ✅ Recovery stays restricted until required imports succeed; optional-profile failure retains marker-checked Proceed.
| Held download after repaired startup | Cold restart and repeated foregrounds |
|---|---|
v4-1685-J2-held-retry-2x.2x.mp4 |
v4-1685-J2-restart-2x.2x.mp4 |
| Authenticated completion | Optional profile and confirmation recheck |
|---|---|
v4-1685-J2-complete-2x.2x.mp4 |
v4-1685-J2-profile-2x.2x.mp4 |
Recordings at 2× speed. Live UI inspection and direct settings reads confirmed independent Electrum and non-default RGS saves, RecoveryError on a failed RGS replacement probe, and retention of the acknowledged RGS correction.
With both markers pending, Retry rebuilt the node and reached InProgress.Metadata while the authenticated download was held. The original required startup error remained and ordinary wallet actions stayed unavailable. After cold restart, the corrections and markers remained. Device logs confirmed three foregrounds and a backup-only retry added no restart for the unchanged correction.
A later held metadata retry with no startup error remained in initialization. Releasing the hold produced real SETTINGS and WALLET imports: direct reads showed remote USD replacing local EUR while corrected Electrum and explicit-port RGS survived. Both markers cleared before Wallet Restored and Home. Provider assertions also confirmed both sats/msats invoice paths refused pending recovery at repository/native boundaries and created signed regtest invoices afterward.
An injected optional-profile error followed successful required imports, with clear markers and no startup error. Proceed appeared after repeated failures. Adding markers before confirmation prevented dismissal; another authenticated restore cleared them, and confirmed Proceed reached Home. No payment was broadcast.
Check 1: build and recovery regressions ✅ The committed head builds and its focused routing, ViewModel and connection-recovery suites pass.
Ran assembleDevDebug testDevDebugUnitTest for ContentViewTest, WalletViewModelTest and WalletConnectionRecoveryTest, including held metadata, delayed initial-marker observation and successful terminal clearance. The separately built controls APK matched the installed bytes. These are focused checks; no fresh full-suite or lint execution is claimed here.
Current-head build, lint and E2E checks report success. Migration checks are skipped.
Reviewed the contiguous delta from 5b4674c to c747cc0, the prior finding, terminal transitions and affected callers. Inherited master changes were checked for recovery interaction and received no separate device campaign. The 2986fde report supplies unchanged J1 signed-preparation/reservation, wipe and admission evidence; the 5b4674c report supplies unchanged invoice evidence. J2's previously failing route was re-driven here. No planned case remains unrun. Nonempty USDT recovery, RN migration and the PIN UI retain their earlier limits.
Twin: bitkit-ios#1139 fix: recover connections during wallet restore at be804c7 matches restricted connection recovery and selective replay; iOS completes required restore before node startup. Compared in source, not driven here.
Uncommitted test controls and deciding assertions
The controls create a disposable wallet, upload its real backups, stage required markers and older settings, hold backup reads, redirect VSS to a refused endpoint, inject an optional-profile failure before initialization, and read production state. They do not replace authenticated backup results or routing decisions. Installed controls APK SHA-256: fb4afc951639120cda32e5a80cd96d0434af1f5ffc636029660a58e5f76c02d2.
InProgress.Metadata; node=Running; required=true; both markers pending
Held authenticated backup download
[same restricted recovery screen throughout hold]
InProgress.Metadata; startup error=null; initialization remains visible
Restore success for: 'SETTINGS'
Restore success for: 'WALLET'
paykit=false usdt=false; corrected endpoints retained; currency=USD
Completed; startup error=null
BackupFailed(count=2, hasPendingWalletRestore=true) after confirmation recheck
Settled; startup error=null; both markers clear after acknowledged retry
diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 00c57a69a..9aea9e501 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -11,6 +11,7 @@ import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.android.EntryPointAccessors
import dagger.hilt.components.SingletonComponent
+import kotlinx.coroutines.flow.first // test hook
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import to.bitkit.async.ServiceQueue
@@ -32,6 +33,14 @@ class DevToolsProvider : ContentProvider() {
@EntryPoint
@InstallIn(SingletonComponent::class)
interface Dependencies {
+ fun keychain(): to.bitkit.data.keychain.Keychain // test hook
+ fun settings(): to.bitkit.data.SettingsStore // test hook
+ fun wallet(): to.bitkit.repositories.WalletRepo // test hook
+ fun backup(): to.bitkit.repositories.BackupRepo // test hook
+ fun vss(): to.bitkit.data.backup.VssBackupClient // test hook
+ fun migration(): to.bitkit.services.MigrationService // test hook
+ fun lightning(): to.bitkit.services.LightningService // test hook
+ fun pubky(): to.bitkit.repositories.PubkyRepo // test hook
fun lightningRepo(): LightningRepo
}
@@ -42,6 +51,102 @@ class DevToolsProvider : ContentProvider() {
override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+ if (method.startsWith("review")) { // test hook
+ return runCatching { // test hook
+ kotlinx.coroutines.runBlocking(kotlinx.coroutines.Dispatchers.IO) { // test hook
+ when (method) { // test hook
+ "reviewHold" -> {
+ val hold = java.io.File(requireNotNull(context).filesDir, "review-vss-hold")
+ if (arg == "on") hold.writeText("enabled") else hold.delete()
+ "VSS download hold=$arg"
+ }
+ "reviewProfile" -> {
+ val flag = java.io.File(requireNotNull(context).filesDir, "review-profile")
+ if (arg == "on") flag.writeText("enabled") else flag.delete()
+ "optional profile failure=$arg"
+ }
+ "reviewElectrum" -> {
+ deps.settings().update { it.copy(electrumServer = requireNotNull(arg)) }
+ "Electrum staged"
+ }
+ "reviewVmState" -> kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.Main) {
+ val vm = walletVm()
+ "restore=${vm.restoreState.value} error=${vm.connectionStartupError.value?.message} required=${vm.connectionStartupError.value?.requiresRecovery} busy=${vm.connectionRetryBusy.value} editor=${vm.connectionEditor.value?.let { "busy=${it.busy},saved=${it.saved}" }}"
+ }
+ "reviewClearMarkers" -> {
+ deps.keychain().delete("PAYKIT_PENDING_BACKUP_RESTORE")
+ deps.keychain().delete("USDT_PENDING_BACKUP_RESTORE")
+ "cleared disposable markers"
+ }
+ "reviewBackupRetry" -> kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.Main) {
+ walletVm().onBackupRestoreRetry()
+ "requested backup retry"
+ }
+ "reviewSeed" -> { // test hook
+ check(!deps.wallet().walletExists()) // test hook
+ deps.wallet().createWallet(null).getOrThrow() // test hook
+ deps.migration().markMigrationChecked() // test hook
+ "created disposable wallet" // test hook
+ } // test hook
+ "reviewInit" -> { // test hook
+ to.bitkit.async.ServiceQueue.CORE.background { // test hook
+ com.synonym.bitkitcore.initDb(to.bitkit.env.Env.bitkitCoreStoragePath(0)) // test hook
+ } // test hook
+ deps.pubky().initialize() // test hook
+ "initialized real native database and profile service" // test hook
+ } // test hook
+ "reviewBackup" -> { // test hook
+ deps.vss().setup().getOrThrow() // test hook
+ for (category in listOf(to.bitkit.models.BackupCategory.WALLET, to.bitkit.models.BackupCategory.SETTINGS)) { // test hook
+ deps.backup().triggerBackup(category).getOrThrow() // test hook
+ } // test hook
+ "acknowledged WALLET and SETTINGS backup" // test hook
+ } // test hook
+ "reviewSettings" -> { // test hook
+ deps.settings().update { it.copy(electrumServer = "tcp://127.0.0.1:1", rgsServerUrl = "https://127.0.0.1:1/snapshot", selectedCurrency = "USD") } // test hook
+ "old unreachable endpoints and USD staged for backup" // test hook
+ } // test hook
+ "reviewCurrency" -> { // test hook
+ deps.settings().update { it.copy(selectedCurrency = requireNotNull(arg)) } // test hook
+ "currency changed" // test hook
+ } // test hook
+ "reviewPending" -> { // test hook
+ deps.keychain().upsertString("PAYKIT_PENDING_BACKUP_RESTORE", "") // test hook
+ deps.keychain().upsertString("USDT_PENDING_BACKUP_RESTORE", "") // test hook
+ "persisted both required markers" // test hook
+ } // test hook
+ "reviewFault" -> { // test hook
+ val flag = java.io.File(requireNotNull(context).filesDir, "review-vss-unavailable") // test hook
+ if (arg == "on") flag.writeText("enabled") else flag.delete() // test hook
+ deps.vss().reset() // test hook
+ "VSS unavailable=$arg" // test hook
+ } // test hook
+ "reviewState" -> { // test hook
+ val settings = deps.settings().data.first() // test hook
+ "paykit=${deps.keychain().exists("PAYKIT_PENDING_BACKUP_RESTORE")} usdt=${deps.keychain().exists("USDT_PENDING_BACKUP_RESTORE")} electrum=${settings.electrumServer} rgs=${settings.rgsServerUrl} currency=${settings.selectedCurrency} node=${deps.lightningRepo().lightningState.value.nodeLifecycleState}" // test hook
+ } // test hook
+ "reviewInvoicesBlocked" -> { // test hook
+ val errors = listOf( // test hook
+ deps.lightningRepo().createInvoice(1uL, "restore admission").exceptionOrNull(), // test hook
+ deps.lightningRepo().createInvoiceMsats(1001uL, "restore admission").exceptionOrNull(), // test hook
+ runCatching { deps.lightning().receive(1uL, "native restore admission") }.exceptionOrNull(), // test hook
+ runCatching { deps.lightning().receiveMsats(1001uL, "native restore admission") }.exceptionOrNull(), // test hook
+ ) // test hook
+ check(errors.all { generateSequence(it) { error -> error.cause }.any { error -> error is to.bitkit.data.keychain.WalletBackupRestorePendingError } }) // test hook
+ "sats and msats invoice repository and native gates rejected required recovery" // test hook
+ } // test hook
+ "reviewInvoicesClear" -> { // test hook
+ check(!deps.keychain().exists("PAYKIT_PENDING_BACKUP_RESTORE")) // test hook
+ check(!deps.keychain().exists("USDT_PENDING_BACKUP_RESTORE")) // test hook
+ val invoices = listOf(deps.lightningRepo().createInvoice(1uL, "restore admission").getOrThrow(), deps.lightningRepo().createInvoiceMsats(1001uL, "restore admission").getOrThrow()) // test hook
+ check(invoices.all { it.startsWith("lnbcrt") }) // test hook
+ "sats and msats regtest invoices created after acknowledged recovery" // test hook
+ } // test hook
+ else -> error("Unknown review control") // test hook
+ }.also { android.util.Log.i("ReviewHook", it) } // test hook
+ }.let { bundleOf("result" to it) } // test hook
+ }.getOrElse { bundleOf("error" to "${it.javaClass.simpleName}: ${it.message}") } // test hook
+ } // test hook
return runCatching {
val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
ServiceQueue.LDK.blocking { command.execute(deps) }
@@ -51,7 +156,22 @@ class DevToolsProvider : ContentProvider() {
}.toBundle()
}
- override fun onCreate() = true
+ private var activity: java.lang.ref.WeakReference<android.app.Activity>? = null
+ private fun walletVm(): to.bitkit.viewmodels.WalletViewModel =
+ androidx.lifecycle.ViewModelProvider(checkNotNull(activity?.get()) as androidx.activity.ComponentActivity)[to.bitkit.viewmodels.WalletViewModel::class.java]
+ override fun onCreate(): Boolean {
+ (requireNotNull(context).applicationContext as android.app.Application).registerActivityLifecycleCallbacks(
+ object : android.app.Application.ActivityLifecycleCallbacks {
+ override fun onActivityCreated(a: android.app.Activity, b: Bundle?) { if (a is to.bitkit.ui.MainActivity) activity = java.lang.ref.WeakReference(a) }
+ override fun onActivityStarted(a: android.app.Activity) {}
+ override fun onActivityResumed(a: android.app.Activity) { if (a is to.bitkit.ui.MainActivity) activity = java.lang.ref.WeakReference(a) }
+ override fun onActivityPaused(a: android.app.Activity) {}
+ override fun onActivityStopped(a: android.app.Activity) {}
+ override fun onActivitySaveInstanceState(a: android.app.Activity, b: Bundle) {}
+ override fun onActivityDestroyed(a: android.app.Activity) {}
+ })
+ return true
+ }
override fun getType(uri: Uri): String? = null
override fun insert(uri: Uri, values: ContentValues?): Uri? = null
override fun delete(uri: Uri, sel: String?, args: Array<String>?) = 0
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
index eef9af0b1..972093da0 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -46,7 +46,7 @@ class VssBackupClient @Inject constructor(
withTimeout(30.seconds) {
Logger.debug("VSS client setting up…", context = TAG)
- val vssUrl = Env.vssServerUrl
+ val vssUrl = if (Env.isDebug && java.io.File(Env.logDir.parentFile, "review-vss-unavailable").exists()) "http://127.0.0.1:1/vss" else Env.vssServerUrl // test hook
val lnurlAuthServerUrl = Env.lnurlAuthServerUrl
val vssStoreId = vssStoreIdProvider.getVssStoreId(walletIndex)
Logger.verbose("Building VSS client with vssUrl: '$vssUrl'", context = TAG)
@@ -136,6 +136,11 @@ class VssBackupClient @Inject constructor(
}
suspend fun getObject(key: String): Result<VssItem?> = withContext(ioDispatcher) {
+ if (Env.isDebug) {
+ val hold = java.io.File(Env.logDir.parentFile, "review-vss-hold")
+ if (hold.exists()) Logger.info("Held authenticated backup download", context = "ReviewHook")
+ while (hold.exists()) delay(100)
+ }
isSetup.await()
Logger.verbose("VSS 'getObject' call for '$key'", context = TAG)
runCatching {
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 57a3d390d..cf53f4f39 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -351,6 +351,7 @@ class PubkyRepo @Inject constructor(
}
suspend fun initialize() = withContext(ioDispatcher) {
+ if (Env.isDebug && java.io.File(Env.logDir.parentFile, "review-profile").exists()) throw to.bitkit.utils.AppError("Injected optional profile cache unavailable")
val restored = initializeMutex.withLock { initializeSession() }
if (restored) loadIdentityData()
checkAdoptedSource()Reviewed by gpt-6.1-sol-high
|
@ben-kaufman Merging this PR on the approvals of head c747cc0, with ovi-reviewer's device run on that head, checks green and no open threads. The PRs stacked on this branch are retargeted to master first. |
|
@ben-kaufman Merged, as it was approved and green. |
Recovery visibility fix — 2026-10-28
Published repair
f3911053and signed additive mergec747cc0b79bd6ae6f7a6ea56f64cf9f0bed31062keep the restricted recovery screen visible throughout required downloads. The routing predicate now handlesRestoreState.InProgressdirectly. Retry also retains the original required-recovery error until the admitted startup attempt reaches its existing terminal transition, covering the earlier initial-marker/restart window. Payment, receive and restore admission gates remain intact.Two controlled before-fix cases reproduced the missing recovery screen during a held metadata download and the premature error clear during delayed initial-marker observation. Both pass after the repair, including successful terminal clearance. Two independent scoped source reviews are clean.
The final combined tree
0317940796a7b33220c1c36bd8d80f139650bf51incorporates masterf9d4b4fband passed required format, compile, full tests and lint: 4,598 tests / 254 suites, zero failures/errors/skips; zero Detekt/Android Lint/format findings. Complete logs, XML/SARIF/HTML and Gradle diagnostics were inspected; all 1,750 file hashes and the directory symlink match. Three JVM notices and four provider cursor-close warnings remain recorded; their source/configuration matches the actual base, without claiming a base runtime reproduction. Earlier failed controls, format failure and the corrected test-import compilation failure remain preserved.All 11 commits against live master
204df78everify locally and on GitHub (verified=true,reason=valid). The later master change concerns request asset/source confirmation and does not modify this repair or its required-restore APIs; its behavior will be retained in the separately composed fee child. The PR remains ready. Current CI passed 4,599 tests / 254 suites and Lint reports zero findings. They execute synthetic146c0d6a, tree8226290f, against actual master204df78e: fourteen inherited paths differ from this published tree, with all forty-four recovery-feature paths preserved. This is not identical-head-tree validation.Current E2E eventually passed 39 distinct cases, with one disabled settings case. The first multi-address/RBF attempt failed its transaction-ID-change assertion after the fee-increase assertion passed; attempt 2 passed, skipping the two already completed cases. This is not first-attempt-clean. All fourteen job logs, eighteen annotations and four SHA/CRC-verified report/APK archives were inspected. Failure media/application logs were not uploaded after eventual success, so native causation remains unproved; no unchanged retry was requested. Earlier failures remain revision-scoped and unwaived. Complete authenticated J1/J2 remains unproved.
Prior master integration at
5b4674c3— 2026-10-28Signed additive merge
5b4674c377313aeadeb9953f8e16568df4309f1bincorporates master7b5f30caafter #1681 merged. Both invoice conflicts retain the required-restore check immediately before native receive while preserving master’s removal of the obsolete geo-state call. The restriction on recovery navigation and the original-operation refusal/rollback safeguards remain intact.The exact combined tree passed required
just compile,just test,just lintandjust format: 4,563 tests / 253 suites, zero failures/errors/skips, and zero Detekt/Android Lint/format findings. Complete logs, XML/SARIF/HTML and Gradle diagnostics were inspected. All 1,748 tracked inputs match the reviewed tree. Existing Gradle/manifest/JVM notices and four cursor-close warnings remain documented; their relevant sources/configuration match the actual incorporated base, without claiming a base runtime reproduction. An independent affected-delta review found no introduced issue; prior reviews remain applicable to unchanged recovery logic.All nine commits against live master verify locally and on GitHub (
verified=true,reason=valid). The PR remains ready and is mergeable. Master subsequently advanced to527208e6with unrelated USDT receive-page transitions; those six changed paths do not overlap this PR’s delta. Local results cover the exact committed tree above. Current-head CI passed all 4,563 tests with zero failures/skips; Lint reports zero Detekt/Android Lint findings. Complete logs and all checksum/CRC-verified reports/APKs were inspected. These jobs execute synthetic025fea53, treeb53e7bff, against base527208e6.E2E eventually passed 39 distinct cases, with one explicitly unexecuted settings case. The first send attempt still timed out after 30 seconds waiting for
InsufficientSavingsToast; attempt 2 passed it and skipped the two send cases already passed on attempt 1. All 11 job logs, all attempts and annotations were inspected, including the failure annotation despite green final job conclusions. The seven shards used harness4c0cbef0and the same digest-verified E2E APK. Failure media/app logs were generated but not uploaded after eventual success, so the cause is unproven. This is not a first-attempt-clean result, a waiver of older failures, or a complete authenticated J1/J2 pass. No new full authenticated J1/J2 or device pass is claimed; the narrower earlier evidence and all prior failures remain below.Restore-refusal fix at
2986fde9— 2026-10-27Final head
2986fde93b896e75685b438c83c58a281a6ad2ff, live base8a176f032a59c8888aab8dae87090103f033e8b0. A prepared native handle now explicitly reports refusal before native broadcast. The journal releases only the exact first local preparation after acknowledged rollback; prior/imported candidates, changed journals, failed persistence and post-native uncertainty remain guarded. Shop proof cleanup still precedes guard deletion.Eight new service/store regression tests exercise durable WALLET/USDT restore markers, unreadable marker storage, rollback failure, wallet/journal replacement, prior/imported candidates, positive evidence, post-native failure and Shop cleanup. The controlled before-fix run failed on the intended refusal regressions. Final
just format,just compile,just testandjust lintpassed: 4,720 tests / 257 suites, zero failures/errors/skips, zero Detekt/Android Lint/format findings. Complete logs and structured reports were inspected; the unchanged provider cursor traces and JVM notices match retained baseline diagnostics, and the Detekt Gradle deprecation is unchanged. All 1,777 tracked inputs matched the reviewed final tree. Two consecutive independent final reviews are clean, including affected-path rechecks after repository-rule corrections. All eight live-base commits verify locally and on GitHub (verified=true,reason=valid).Hosted CI, Lint and E2E are running on this head; complete logs and artifacts will be audited by the coordinating chat. Prior hosted failures below remain disclosed. This follow-up adds controlled regression coverage; it does not replace the journey gaps documented below.
Prior parent composition — 2026-10-27
Signed
825bb3a9d2c6eb163ab14ab1a4e38bb02306d139additively incorporates #1681 /8a176f032a59c8888aab8dae87090103f033e8b0, including current master. The combined tree passed 4,712 tests in 257 suites with zero failures/errors/skips, full compilation, Detekt/Android Lint zero, and unchanged final formatting. Complete logs and structured reports were inspected; three JVM notices and four unchanged-provider cursor traces remain disclosed without a timing-cause claim. Two consecutive independent final reviews are clean. All seven live-base commits are locally and GitHub verified.Two journey descriptions now accurately reference the implemented iOS #1139 counterpart while preserving each platform’s revision and fixture limits. Existing installed-app evidence is retained at its recorded build. João’s review 5669903603 confirms the fixes but retains device acceptance requirements; this publication does not turn that comment into approval or claim those remaining app cases passed. Current-head hosted checks are pending. The earlier widget/Blocktank failures remain revision-scoped evidence.
Foreground startup follow-up at f9ff9d2
An ordinary foreground node failure with affirmatively clear required markers and no saved correction now keeps the wallet and normal Settings available. Pending or unreadable markers and retained/unreadable corrections still require the restricted recovery screen. This classification is separate from permission to Proceed.
Final unchanged inputs passed 4,705 unit tests across 257 suites, required compile/format and full Detekt/Android Lint with zero findings; all 1,773 hashes matched. Two consecutive independent source reviews are clean. The installed combined #1701 app (APK SHA-256 40b5484fffc7a4de41da9d46dae95884648c45d12c83a2527b6fd89b74272cc0) contains the exact four parent files. After real restoration cleared both markers, an isolated Electrum failure caused both native startup attempts to fail and one outer error report; the wallet, menu and Settings remained reachable. Restoring the endpoint and foregrounding again started the node successfully. The brief toast was not visually captured, and this narrower case is not complete J1/J2. Complete private runtime/UI receipts and the earlier failure evidence are retained. Every commit in the six-commit live-base range verifies locally and on GitHub. New-head hosted checks are pending.
Twin: synonymdev/bitkit-ios#1139 (ready; acceptance evidence remains revision-scoped)
Description
A failed wallet restore could leave recovery pending while repeated-failure navigation allowed payments or new receive addresses. Block these operations at repository admission and native dispatch until required restoration completes, including typed retries, channel/fee mutations and prepared broadcasts. Keep node sync and reservation reveal available for restoration, and coalesce overlapping retries.
When a saved Electrum or RGS endpoint prevents node startup, the required-recovery screen now offers a restricted connection editor. Users can validate, save or reset either endpoint without starting the node or opening the wallet. Corrections are durably bound to the current wallet and revision before Settings changes, survive restart, and are selectively reapplied after remote SETTINGS restoration. Retry rebuilds the failed node through the existing startup coordinator and continues required restoration. Identity replacement, including replacement with the same seed, revokes and drains old editor/startup work.
Proceed remains unavailable while either required marker exists or cannot be read. After repeated ordinary node failures with affirmatively clear markers, Proceed is also available; confirmation rechecks both markers under the original wallet owner before dismissing the failure. The optional-profile-only path remains available after required recovery. Creating a new wallet after cancelled restoration resets the old screen state only after creation succeeds. Startup failures produce one toast per attempt. Opening, saving and retrying the editor release busy state on cancellation, and old editor work cannot publish into a replacement wallet or editor.
Out of Scope
No new backup format, payment replay, SDK import policy or full-wallet split. The paired iOS connection-recovery implementation is tracked in #1139.
Design
Existing recovery UI with a restricted Electrum/RGS editor and explicit Save, Reset and Retry actions. It exposes no payment, receive or general wallet navigation while required restoration is pending.
Preview
A user can correct unreachable connection settings and retry restoration while all required recovery gates remain active.
QA Notes
Setup
Committed Debug dependencies: Core 0.8.0-rc11, Paykit 0.1.1-rc2 and VSS 0.5.26-rc1. Unit tests use controlled repository/native adapters and durable local state; installed Compose tests render the actual recovery components on API 36. Complete journeys require a disposable test-network backup containing unresolved payment/address state, VSS failure controls, controllable Electrum/RGS endpoints and a post-restore profile failure. No real wallet reset or live funds are required or authorized.
Journeys
pending-wallet-recovery.xml— repeated failure, restart, operation admission, overlapping retry and safe profile-only failure.connection-recovery.xml— unreachable restored connection settings, restricted editing, durable selective replay, cancellation/restart and retained spending gates.Neither complete authenticated journey has been run. Component results below are separate evidence.
Automated Checks
WalletConnectionRecoveryViewTest/WalletRestoreErrorViewTestCompose cases passed on the owned API 36 emulator. Both installed app/test APKs matched the validated build bytes. Complete app recovery and authenticated fault journeys remain unrun.Evidence
Review follow-up
8d72053c4fd480c5b21ec33a8285b966672bf6c5: controlled before cases reproduced duplicate toast delivery and cancelled editor busy state. The final 73-case WalletViewModel suite covers ordinary marker-free failures, both real keychain marker selectors, unavailable and newly appearing markers during confirmation, original owner/attempt changes, exact editor cancellation and replacement-editor cleanup. The five installed Compose cases include the production clearance helper, retry threshold, confirmation, revocation and dismissal; repository/native resources remain controlled, so this is not an authenticated J1/J2 pass. Final inputs match the frozen validation manifest. Two consecutive independent final reviews are clean. Current-head hosted validation is complete; merge-composition evidence is recorded below.Tests cover both durable markers, unreadable state, queue and post-callback races, prepared broadcasts, retry coalescing, optional profile failure, settings persistence before mutation, restart/revision replay, explicit default RGS, owner changes, same-seed replacement, cancellation/drain and node rebuild. The final source was reviewed against its merge base 2493dbb. The live target branch is 7c21884; these local native results use this branch’s committed RC1 dependency and do not claim execution of the later RC2 merge composition. Parent adoption is additive; no published history was rewritten.
Gaps
Repository recreation and installed Compose controls do not establish a complete process-kill VSS restore journey. No funded payment was submitted. Matching restricted connection recovery is ready in synonymdev/bitkit-ios#1139; the original 9481b503 evidence remains revision-scoped, with required startup gates retained. Its local native checks passed; complete authenticated J1 and manual acceptance remain unrun. Current hosted success does not replace the required authenticated J1/J2 journeys.
Hosted validation at 8d72053
CI 40422268351 passed 4,681 tests across 256 suites with zero failures/skips. Lint 40422268557 has zero findings in complete Detekt and Android Lint SARIF/XML. Uploaded reports matched their SHA-256 digests and passed ZIP CRC checks. E2E 40422268909 passed all 39 tests across seven shards on first attempts; second/third attempts were skipped. All fourteen completed-job logs and available annotations were inspected. Migration was skipped and is not passing evidence.
These jobs tested synthetic merge
a1d4daa9with live parent7c218843, including the parent's RC2 adoption, rather than the local head tree used for the 4,676-test run. Nonfatal JVM, native-strip, provider/cursor, Gradle and external E2E harness notices remain documented. The four complete cursor-warning stacks match retained earlier evidence; affected source files are unchanged across the actual merge base, live target and head, without a timing/root-cause claim. No failure artifacts exist for these successful first-attempt shards. Full authenticated J1/J2 remain unrun, and the prior 502 failures below are not waived.Prior hosted validation at 55b31ee
Hosted CI passed all 4,656 unit tests, and the complete uploaded Detekt/Android Lint reports contain zero findings. E2E run 40341310833 remains failed and unwaived: send, security and multi-address shards timed out at SendSuccess on all three workflow attempts. Retained application logs directly show Blocktank info HTTP 502 errors in those sends. The on-chain/RBF shard eventually passed on its third attempt after earlier failures. Complete logs and failure artifacts were inspected; no unchanged rerun was requested.
The on-chain fee lookup and affected send implementation are byte-identical to the actual parent 2493dbb; this PR did not introduce that dependency. This comparison does not prove that earlier failures had the same cause, or establish a causal repair for those historical failures. Controlled fee-service fault testing and any safely separable repair remain separate follow-up work. The restricted restore controls passed their four installed Compose cases at that earlier revision, which are narrower than J1/J2.
Current combined tree at 7b823d2
The signed additive merge
7b823d2ab278e00ef70823fb97f360f07327c1d5incorporates live parent #1681 at416c1e35, including Paykit 0.1.1-rc2. A running node now remembers the exact wallet lifecycle and connection revision it successfully applied. Foreground and backup-only retries no longer rebuild it for the same retained correction, and backup-only failures count once. Wallet removal clears stale recovery UI.The same repair retires an acknowledged correction after required restoration even if optional profile loading fails, so a later normal Settings change cannot be overwritten. Proceed refuses an unapplied correction and retains the restricted editor until explicit retry succeeds. Required and unreadable restore markers remain fail closed.
Controlled before runs reproduced three reviewer failures and two journal/Proceed failures; all 82 focused cases pass afterward. Final combined-tree validation passed 4,701 fresh unit tests in 257 suites with zero failures/errors/skips, required compile/format and full Detekt/Android Lint with zero findings. All 1,773 frozen inputs match. Complete logs and structured reports were inspected; retained JVM/cursor warnings match actual-base source/evidence. Archived Gradle problem HTML belongs to an earlier focused invocation and is not counted as a fresh full-run report. The preceding helper compilation failure remains recorded separately.
Two consecutive independent final reviews (4 and 5) are clean. Every commit in the full five-commit live-parent range verifies locally and on GitHub (
verified=true,reason=valid). New-head hosted checks are pending. This composition does not claim a new installed Compose run or complete authenticated J1/J2; the earlier installed UI and hosted results below retain their exact revisions.Hosted validation at
2986fde9— October 27CI passed 4,720 tests with zero failures/skips. Lint has zero Detekt/Android Lint findings. Complete logs and digest/CRC-verified reports were inspected; the synthetic merge tree equals the published head.
E2E failed: 37 unique cases eventually passed and two remained failing, with eight failed executions across three cases retained. Both persistent cases time out on a fallback second WidgetSave after the price widget is absent; the backup case fails before its seed/restore steps. A reset-widget case failed twice (including a visible New USDT sheet) before passing. All shard logs, attempts, annotations and failure artifacts were inspected. The harness is
4c0cbef0, already containing the earlier obscured-delete guard; these failures are not attributed to that prior cause. No unchanged workflow retry was requested, and no complete hosted backup/restore pass is claimed. The shared harness owner has the exact evidence for triage.Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)