Repository navigation
fix: guard wallet operations until required restore completes - #1685
ben-kaufman wants to merge 6 commits into
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
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 (draft; authenticated acceptance pending)
Dependency: #1681
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 now published in draft synonymdev/bitkit-ios#1139 at 9481b503, 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.Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)