Repository navigation
fix: preserve pending backups across restart - #1689
ben-kaufman wants to merge 23 commits into
Conversation
|
Triage: 3.1.0. It fixes shipped backup handling, and 3.1.0 is due 28 Oct. |
 [Medium impact] The changes affect wallet backup and recovery work in the Android app. Fix the restart gap before merging so edits made during old cleanup are not silently left out of backups. Findings
SummaryThis PR makes reset wait for retained backup work and keeps explicit backups separate from observation.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Stop observation] --> B[Cancel old observer jobs]
B --> C[Restart requested]
C --> D[Wait for old cleanup]
D --> E[Start new data listeners]
E --> F[Drop initial values]
G[User edits settings during wait] --> D
G --> F
F --> H[Edit never marks backup required]
Reviews (1) · Last reviewed commit: "fix: preserve pending backups across res..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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. Moves every backup task under a reset-owned scope with a generation fence, so reset drains interrupted setup, uploads and status writes before clearing the VSS clients, and a stopped or restarted observer keeps pending categories. Reviewed a8f0810, full tier, reasoned from the code and CI (not built here).
What I checked, and 6 candidates I ruled out
Read in full: the BackupRepo.kt diff and its lifecycle paths (reset, startObservingBackups, scheduleBackup, withBackupOperation, performBackup, retryAfterTemporaryFailure), PaykitSdkOperationLock.kt, PaykitSdkService.withWalletWipe / exportBackupState, WipeWalletUseCase.kt, BackupsViewModel.kt, UsdtService.recoveryBackup
Call sites traced: reset (only WipeWalletUseCase.stopNode), triggerBackup, persistUsdtRecoveryBackup, scheduleFullBackup, start/stopObservingBackups
CI: build (includes testDevDebugUnitTest) and lint green; e2e jobs still running at review time
Ruled out
- Reset hangs the wipe:
resetjoins pending work underNonCancellablewhilewithWalletWipeholds the Paykit lock. A background WALLET export blocked there is inacquire, whoseawaitis cancellable and removes its waiter; the SDK call's ownNonCancellablesection cannot be entered while the wipe holds the lock. Covered bywipe admission drains a backup queued at the real SDK operation barrier. - Lost update while a scheduled job is mid-debounce: the old cancel-and-restart is gone, but the status observer skips because
runningis true, the job reads the latest status after the debounce, and a change during upload bumpsrequiredpastbackupRequired, so the job'sfinallyreschedules.full backup follows newer required data after observers coalescecovers the follow-up. - Reschedule loop in the job's
finally: a permanent failure setsfailedBackupRequired, soshouldBackupstops it;WalletBackupSnapshotChangedre-runs as it did on the base through therunningtrue to false transition. - Lock inversion:
categoryMutexesthenwalletBackupMutex/usdtBackupMutexon the scheduled path, the inner mutex alone onpersist*;lifecycleLockis only taken in non-suspending blocks, including inside the DataStore transform. - Restart dropped during reset:
startObservingBackupsreturns whileresetting, but the base already returned while wiping, and the only reset caller restarts observation itself on failure. - Greptile's restart gap: real, but the class predates this PR. On the base an edit made while observation is stopped is already discarded by the listeners'
drop(1)on restart; this change extends that window by the old observation's cancellation cleanup, which is short (cancellable uploads plus oneNonCancellablestatus write).
Merge confidence: 4/5, no findings, but the change is reasoned from code and CI rather than built here, and the e2e jobs had not finished.
Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed to the reviewing session)
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan: no findings.
Checked and clean:
- A backup is marked synced only after the put returns success. Stop, restart and wipe keep consumed pending markers. Only the fully-stopped window drops edits, which was already discussed here.
- After a wipe and restore, the old wallet's pending data can't upload under the new wallet: the generation fence and the drained join stop it.
BackupItemStatusis persisted in the same format as v3.0.0, so nothing to migrate.- Twin synonymdev/bitkit-ios#1123 serialises the observer lifecycle the same way.
Performance-relevant (observation restarts on every background/foreground cycle and now waits for the old cleanup). The gate measures the gap between Stopped observing and Start observing per cycle, and counts Backup starting for per single edit (expected 1), head vs master.
Device gate (partial): e585fcc — not run yet; pending that measurement. Device results will follow in a separate review.
|
@ben-kaufman please resolve conflicts. |
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for e585fcc: Android 15 (API 35), arm64. I built the reviewed head and drove an isolated fault fixture using the app's real DataStores and native encrypted VSS transport. Local hooks hold operations and expose their results; a proxy supplies a controlled VSS conflict. Recovery payloads are synthetic, so these results do not establish native transaction signing or a deployed-contract outage journey.
Test 1: pending changes across restart ✅ Settings and a due retry waited for old cleanup, then uploaded without another edit.
I held WIDGETS cancellation cleanup, restarted observation, changed synchronized settings to EUR, and let a BLOCKTANK retry become due. Replacement setup and uploads waited for release. Decrypted VSS readback contained EUR, with the same payload digest as the upload.
case1-2x.2x.mp4
Restart drain and native VSS readback, 2×.
Test 2: reset drains manual and recovery persistence ✅ Cancelled reset callers still waited; cancelled recovery was not acknowledged, and retry retained the same envelope.
Both VSS clients reset only after the held write returned. The recovery acknowledgement counter stayed at zero after cancellation, then advanced once on retry with observation stopped. Both writes used the same content-addressed key and payload digest. This checks the real app callback with a synthetic recovery string.
case2-2x.2x.mp4
Reset cancellation and recovery retry, 2×.
Test 3: wipe admission ✅ Manual, full and recovery requests produced no upload during wipe; explicit recovery worked after release.
SETTINGS and USDT recovery status snapshots remained unchanged while admission was closed after reset. The recovery request completed after release without starting observation.
case3-2x.2x.mp4
Wipe admission and stopped-observer recovery, 2×.
Test 4: full-backup follow-up ✅ New revisions, a held required-status write, and a full request during stopped-job cleanup all resumed without another edit.
I drove the shared scheduler through SETTINGS. A change during the first upload produced a second upload; a consumed change survived stop/restart while its cache write was held; a full request waited behind cancelled cleanup and completed with observation stopped. Final decrypted VSS readback contained CHF.
case4-2x.2x.mp4
Revision follow-up, cache-write drain and full-request admission, 2×.
Test 5: clean restart timing and upload count ✅ The head added only milliseconds on the idle fixture, and one edit produced one SETTINGS upload at both revisions.
Measured from the app's stop/start log timestamps: head gaps were 2–11 ms (median 5 ms); exact-base gaps were 0–1 ms (median 1 ms). These are direct observation cycles against ac2a7c668, rather than physical background transitions or a comparison with later master changes.
Head e585fcc |
Base ac2a7c668 |
|---|---|
case5-head-2x.2x.mp4 |
case5-base-2x.2x.mp4 |
Idle lifecycle cycles and a single settings edit, 2×.
Check 1: build and committed regressions ✅ assembleDevDebug and the focused BackupRepoTest suite passed on the unmodified head.
The restart finding is repaired at this head; I checked its listener and retry-readiness paths against the fresh regression and device results.
Coverage: Full ac2a7c668..e585fcc, including reached reset/wipe, SDK-lock and recovery-acknowledgement paths. No planned fixture case remains unrun. Device full-backup checks exercised SETTINGS; complete WALLET capture and signed-payment recovery internals were checked in code and the committed component suite, without a new complete payment-fault journey.
Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle matches the lifecycle and pending-change intent. Android invalidates observation immediately and drains on restart/reset; iOS awaits shutdown.
Local test controls — not pushed
The patch adds explicit holds and logging, plus a shell-only control surface for a disposable installation. It does not replace native VSS results or change the backup decisions under review. The proxy forwards encrypted traffic to staging and can return a protobuf conflict; the native client converts that into the app's normal retry path.
diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..c1aa86be7 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -11,5 +11,8 @@
android:authorities="${applicationId}.devtools"
android:exported="true"
tools:ignore="ExportedContentProvider" />
+ <!-- test hook -->
+ <provider android:name="to.bitkit.dev.BackupFaultProvider" android:authorities="${applicationId}.backupfault" android:exported="true" tools:ignore="ExportedContentProvider" />
+ <activity android:name="to.bitkit.dev.BackupFaultActivity" android:exported="true" />
</application>
</manifest>
diff --git a/app/src/main/java/to/bitkit/data/CacheStore.kt b/app/src/main/java/to/bitkit/data/CacheStore.kt
index 9966a479f..31a561531 100644
--- a/app/src/main/java/to/bitkit/data/CacheStore.kt
+++ b/app/src/main/java/to/bitkit/data/CacheStore.kt
@@ -38,6 +38,7 @@ class CacheStore internal constructor(
val backupStatuses: Flow<Map<BackupCategory, BackupItemStatus>> = data.map { it.backupStatuses }
suspend fun update(transform: (AppCacheData) -> AppCacheData) {
+ to.bitkit.testhook.BackupFault.hold("cache") // test hook
store.updateData(transform)
}
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..4d59ab2d0 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -1,5 +1,6 @@
package to.bitkit.data.backup
+import to.bitkit.testhook.BackupFault // test hook
import com.synonym.vssclient.KeyVersion
import com.synonym.vssclient.VssItem
import com.synonym.vssclient.vssDelete
@@ -36,6 +37,8 @@ class VssBackupClient @Inject constructor(
private val setupMutex = Mutex()
suspend fun setup(walletIndex: Int = 0): Result<Unit> = withContext(ioDispatcher) {
+ BackupFault.event("SETUP_ENTER") // test hook
+ BackupFault.hold("setup", drain = true) // test hook
setupMutex.withLock {
val gate = isSetup
runCatching {
@@ -109,6 +112,7 @@ class VssBackupClient @Inject constructor(
}
fun reset() {
+ BackupFault.event("RESET_VSS") // test hook
synchronized(this) {
isSetup.cancel()
isSetup = CompletableDeferred()
@@ -122,6 +126,7 @@ class VssBackupClient @Inject constructor(
data: ByteArray,
): Result<VssItem> = withContext(ioDispatcher) {
isSetup.await()
+ BackupFault.beforePut(key, data) // test hook
Logger.verbose("VSS 'putObject' call for '$key'", context = TAG)
runCatching {
vssStore(
@@ -129,8 +134,11 @@ class VssBackupClient @Inject constructor(
value = data,
)
}.onSuccess {
+ BackupFault.event("PUT_OK $key sha256=${BackupFault.digest(data)}") // test hook
+ BackupFault.hold("post:${BackupFault.gateKey(key)}", drain = true) // test hook
Logger.verbose("VSS 'putObject' success for '$key' at version: '${it.version}'", context = TAG)
}.onFailure {
+ BackupFault.event("PUT_ERROR $key ${it.javaClass.simpleName}") // test hook
Logger.verbose("VSS 'putObject' error for '$key'", it, context = TAG)
}
}
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
index 7842dbf19..7c6472c57 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
@@ -43,6 +43,7 @@ class VssBackupClientLdk @Inject constructor(
private val setupMutex = Mutex()
suspend fun setup(walletIndex: Int = 0): Result<Unit> = withContext(ioDispatcher) {
+ to.bitkit.testhook.BackupFault.event("LDK_SETUP_ENTER") // test hook
setupMutex.withLock {
val gate = isSetup
runCatching {
@@ -75,6 +76,7 @@ class VssBackupClientLdk @Inject constructor(
}
fun reset() {
+ to.bitkit.testhook.BackupFault.event("RESET_LDK") // test hook
synchronized(this) {
isSetup.cancel()
isSetup = CompletableDeferred()
diff --git a/app/src/main/java/to/bitkit/env/Env.kt b/app/src/main/java/to/bitkit/env/Env.kt
index 09797720d..6bd811c54 100644
--- a/app/src/main/java/to/bitkit/env/Env.kt
+++ b/app/src/main/java/to/bitkit/env/Env.kt
@@ -111,7 +111,7 @@ internal object Env {
val vssStoreIdPrefix get() = "bitkit_v1_${network.name.lowercase()}"
val vssServerUrl
- get() = when (network) {
+ get() = if (to.bitkit.BuildConfig.DEBUG) "http://127.0.0.1:24889" else when (network) { // test hook
Network.BITCOIN -> "https://bitkit.to/vss_rs_auth"
else -> "https://bitkit.stag0.blocktank.to/vss_rs_auth"
}
diff --git a/app/src/main/java/to/bitkit/testhook/BackupFault.kt b/app/src/main/java/to/bitkit/testhook/BackupFault.kt
new file mode 100644
index 0000000..f99eb5b
--- /dev/null
+++ b/app/src/main/java/to/bitkit/testhook/BackupFault.kt
@@ -0,0 +1,40 @@
+// test hook: local review fixture, never shipped.
+package to.bitkit.testhook
+
+import android.util.Log
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.NonCancellable
+import kotlinx.coroutines.withContext
+import java.security.MessageDigest
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.CopyOnWriteArrayList
+import to.bitkit.BuildConfig
+
+object BackupFault {
+ private val gates = ConcurrentHashMap<String, CompletableDeferred<Unit>>()
+ val events = CopyOnWriteArrayList<String>()
+ fun event(text: String) {
+ if (!BuildConfig.DEBUG) return
+ val line = "${android.os.SystemClock.elapsedRealtime()} $text"
+ events.add(line)
+ Log.i("BackupFault", line)
+ }
+ fun arm(name: String) { gates[name] = CompletableDeferred(); event("ARM $name") }
+ fun release(name: String) { gates.remove(name)?.complete(Unit); event("RELEASE $name") }
+ fun releaseAll() { gates.keys.toList().forEach { release(it) } }
+ suspend fun hold(name: String, drain: Boolean = false) {
+ if (!BuildConfig.DEBUG) return
+ val gate = gates[name] ?: return
+ event("HELD $name")
+ if (drain) withContext(NonCancellable) { gate.await() } else gate.await()
+ gates.remove(name, gate)
+ event("EXIT $name")
+ }
+ fun gateKey(key: String) = if (key.startsWith("USDT_RECOVERY_")) "USDT_RECOVERY" else key
+ suspend fun beforePut(key: String, data: ByteArray) {
+ event("PUT_ENTER $key sha256=${digest(data)}" + if (key == "SETTINGS") " currency=" + to.bitkit.di.json.decodeFromString<to.bitkit.models.SettingsBackupV1>(data.decodeToString()).settings.selectedCurrency else "")
+ try { hold("put:${gateKey(key)}") }
+ finally { hold("cleanup:${gateKey(key)}", drain = true) }
+ }
+ fun digest(data: ByteArray) = MessageDigest.getInstance("SHA-256").digest(data).joinToString("") { "%02x".format(it) }
+}
diff --git a/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt b/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt
new file mode 100644
index 0000000..82f5ad2
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt
@@ -0,0 +1,114 @@
+// test hook: shell-only control surface for disposable emulator.
+package to.bitkit.dev
+
+import android.app.Activity
+import android.content.ContentProvider
+import android.content.ContentValues
+import android.net.Uri
+import android.os.Binder
+import android.os.Bundle
+import android.graphics.Color
+import android.os.Handler
+import android.os.Looper
+import android.widget.TextView
+import android.widget.ScrollView
+import dagger.hilt.EntryPoint
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.hilt.android.EntryPointAccessors
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.CacheStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.data.backup.VssBackupClient
+import to.bitkit.models.BackupCategory
+import to.bitkit.models.BackupItemStatus
+import to.bitkit.repositories.BackupRepo
+import to.bitkit.services.UsdtService
+import to.bitkit.testhook.BackupFault
+import java.util.concurrent.ConcurrentHashMap
+
+@EntryPoint
+@InstallIn(SingletonComponent::class)
+interface BackupFaultDependencies {
+ fun backup(): BackupRepo
+ fun cache(): CacheStore
+ fun settings(): SettingsStore
+ fun keychain(): Keychain
+ fun vss(): VssBackupClient
+ fun usdt(): UsdtService
+}
+
+class BackupFaultProvider : ContentProvider() {
+ private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+ private val tasks = ConcurrentHashMap<String, Job>()
+ private val deps by lazy { EntryPointAccessors.fromApplication(requireNotNull(context), BackupFaultDependencies::class.java) }
+ override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
+ check(Binder.getCallingUid() == 2000)
+ fun asyncTask(name: String, block: suspend () -> Unit) {
+ tasks[name] = scope.launch {
+ BackupFault.event("TASK_BEGIN $name")
+ try { block(); BackupFault.event("TASK_OK $name") }
+ catch (e: Exception) { BackupFault.event("TASK_ERROR $name ${e.javaClass.simpleName}") }
+ }
+ }
+ val answer = when(method) {
+ "init" -> { asyncTask("init") {
+ check(deps.keychain().loadString(Keychain.Key.BIP39_MNEMONIC.name) == null)
+ deps.keychain().saveString(Keychain.Key.BIP39_MNEMONIC.name, org.lightningdevkit.ldknode.generateEntropyMnemonic(org.lightningdevkit.ldknode.WordCount.WORDS12))
+ deps.vss().setup().getOrThrow()
+ }; "started" }
+ "prepare" -> { asyncTask("prepare") {
+ BackupFault.releaseAll(); deps.backup().setWiping(false); deps.backup().reset()
+ deps.settings().update { it.copy(selectedCurrency = "USD") }
+ deps.cache().update { it.copy(backupStatuses = BackupCategory.entries.associateWith { BackupItemStatus(required = 1000, synced = 1000) }) }
+ deps.vss().setup().getOrThrow()
+ }; "started" }
+ "cycle" -> { asyncTask("cycle") { repeat(5) { deps.backup().stopObservingBackups(); deps.backup().startObservingBackups(); delay(500) } }; "started" }
+ "start" -> { deps.backup().startObservingBackups(); BackupFault.event("START_REQUEST"); "ok" }
+ "stop" -> { deps.backup().stopObservingBackups(); BackupFault.event("STOP_REQUEST"); "ok" }
+ "arm" -> { BackupFault.arm(requireNotNull(arg)); "ok" }
+ "release" -> { BackupFault.release(requireNotNull(arg)); "ok" }
+ "currency" -> { asyncTask("currency") { deps.settings().update { it.copy(selectedCurrency = requireNotNull(arg)) } }; "started" }
+ "mark" -> { asyncTask("mark") { deps.cache().updateBackupStatus(BackupCategory.valueOf(requireNotNull(arg))) { it.copy(required = it.required + 1) } }; "started" }
+ "manual" -> { asyncTask("manual") { deps.backup().triggerBackup(BackupCategory.valueOf(requireNotNull(arg))).getOrThrow() }; "started" }
+ "full" -> { deps.backup().scheduleFullBackup(); BackupFault.event("FULL_REQUEST"); "ok" }
+ "usdt" -> { asyncTask("usdt") { deps.usdt().recoveryBackup.persist("disposable-synthetic-same-operation") }; "started" }
+ "reset" -> { asyncTask("reset") { deps.backup().reset() }; "started" }
+ "cancel" -> { tasks[arg]?.cancel(); BackupFault.event("CANCEL $arg"); "ok" }
+ "wipe" -> { deps.backup().setWiping(arg == "true"); BackupFault.event("WIPE $arg"); "ok" }
+ "clear" -> { BackupFault.events.clear(); "ok" }
+ "fetch" -> { asyncTask("fetch") { val item = deps.vss().getObject(requireNotNull(arg)).getOrThrow(); BackupFault.event("FETCH $arg present=${item != null} sha256=${item?.value?.let(BackupFault::digest)}" + if (arg == "SETTINGS" && item != null) " currency=" + to.bitkit.di.json.decodeFromString<to.bitkit.models.SettingsBackupV1>(item.value.decodeToString()).settings.selectedCurrency else "") }; "started" }
+ "status" -> runBlocking(Dispatchers.IO) {
+ "ack=" + deps.usdt().recoveryGeneration + " currency=" + deps.settings().data.first().selectedCurrency + " statuses=" + deps.cache().backupStatuses.first().filterKeys { it.name == (arg ?: "SETTINGS") }.entries.joinToString { "${it.key}=${it.value}" }
+ }
+ else -> BackupFault.events.joinToString("\n")
+ }
+ if (method == "status") BackupFault.event(answer)
+ return Bundle().apply { putString("result", answer) }
+ }
+ override fun onCreate() = 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
+ override fun update(uri: Uri, values: ContentValues?, sel: String?, args: Array<String>?) = 0
+ override fun query(uri: Uri, proj: Array<String>?, sel: String?, args: Array<String>?, sort: String?) = null
+}
+
+class BackupFaultActivity : Activity() {
+ private val handler = Handler(Looper.getMainLooper())
+ private lateinit var text: TextView
+ private val refresh = object : Runnable {
+ override fun run() {
+ text.text = "Backup lifecycle fault fixture\nPR source + local hold/log hooks\nReal DataStore and native encrypted VSS\nSynthetic recovery payload; no funds\n\n" + BackupFault.events.takeLast(25).joinToString("\n")
+ handler.postDelayed(this, 250)
+ }
+ }
+ override fun onCreate(saved: Bundle?) {
+ super.onCreate(saved)
+ text = TextView(this).apply { setTextColor(Color.WHITE); setBackgroundColor(Color.BLACK); textSize = 14f; setPadding(24,60,24,20) }
+ setContentView(ScrollView(this).apply { addView(text) }); handler.post(refresh)
+ }
+ override fun onDestroy() { handler.removeCallbacks(refresh); super.onDestroy() }
+}The transport fault fixture:
from http.server import ThreadingHTTPServer,BaseHTTPRequestHandler
from urllib.request import Request,urlopen
from urllib.error import HTTPError
from pathlib import Path
import threading,time
lock=threading.Lock()
state={'fail':0}
class Handler(BaseHTTPRequestHandler):
def log_message(self,*args): pass
def do_GET(self): self.relay()
def do_POST(self): self.relay()
def relay(self):
body=self.rfile.read(int(self.headers.get('Content-Length','0')))
if self.path=='/control/fail':
with lock: state['fail']=1000
self.send_response(200); self.end_headers(); self.wfile.write(b'armed'); return
if self.path=='/control/healthy':
with lock: state['fail']=0
self.send_response(200); self.end_headers(); self.wfile.write(b'healthy'); return
with lock:
fail=state['fail']>0 and self.command=='POST' and 'put' in self.path.lower()
if fail: state['fail']-=1
if fail:
self.send_response(409); self.send_header('Content-Type','application/octet-stream'); self.end_headers(); self.wfile.write(b'\x08\x01\x12\x13controlled conflict');print(time.time(),'FAULT_409',self.command,self.path,flush=True); return
headers={k:v for k,v in self.headers.items() if k.lower() not in ('host','connection','accept-encoding')}
req=Request('https://bitkit.stag0.blocktank.to/vss_rs_auth'+self.path,data=body if self.command=='POST' else None,headers=headers,method=self.command)
try: result=urlopen(req,timeout=90)
except HTTPError as e: result=e
except Exception as e:
self.send_response(502);self.end_headers();self.wfile.write(type(e).__name__.encode());print('PROXY_ERROR',type(e).__name__,flush=True);return
data=result.read();self.send_response(result.status)
for k,v in result.headers.items():
if k.lower() not in ('connection','transfer-encoding','content-length','content-encoding'): self.send_header(k,v)
self.send_header('Content-Length',str(len(data))); self.end_headers(); self.wfile.write(data)
print(time.time(),self.command,self.path.split('?')[0],result.status,flush=True)
ThreadingHTTPServer(('127.0.0.1',24889),Handler).serve_forever()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 Resolved the parent conflict with signed additive merge c255458, preserving both connection-correction tests and lifecycle ownership. Final 4,681 unit/component tests, full compile/format/Detekt/Android Lint passed with zero findings; two independent final composition reviews were clean. All three commits in the live-parent range are locally verified and GitHub true/valid. New-head hosted checks are pending; no new authenticated composition journey is claimed. |
|
Published follow-up 49e9a99 retains explicit backup retry ownership after observation stops and reserves each queued USDT callback’s exact required revision before cancellation can interrupt its category wait. Older completions/timers preserve newer pending revisions and backoff. Controlled fail-before cases pass after repair; final 4,696 tests/255 suites (112 backup cases), full format/compile/Detekt/Android Lint and two consecutive independent final reviews pass. All five live-base commits verify locally and on GitHub. New-head hosted checks are pending; manual case 5/authenticated fault journeys remain unrun, and backup success does not acknowledge an earlier failed Core callback or submit a payment. Updated QA evidence preserves the prior head’s five failed E2E executions before eventual success. Requested Ovi’s affected re-review once; existing João/Piotr requests remain. |
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan of a102e0e and 49e9a99: one LOW inline (a stale failedBackupRequired after a cancelled USDT reservation). No HIGH or MEDIUM.
Checked and clean:
- No retry storm: every retry path goes through
retryAfterTemporaryFailure's backoff (10 s doubling to a 5-minute cap) or a single reschedule after a data change;backupJobsdedupes owner and explicit jobs. - Nothing is marked synced before it is durable:
syncedadvances only inonSuccessafterputObject, and cancelled callbacks never count as acknowledgements. - The transport classifier needs the request-error prefix, an http(s) URL with a host and the
/putObjectspath; HTTP status, decode, auth and cancellation stay permanent. reset()cancels and joinspendingScopebefore clearing explicit backups, andscheduleBackuprefuses while wiping.- The master merge's conflicts in
BackupRepo.ktandBackupRepoTest.ktkept the PR'striggerBackupand added master's test; nothing dropped. - Twin synonymdev/bitkit-ios#1123 has no
failedBackupRequired, so the LOW doesn't apply there.
Performance-relevant: explicit retries now continue after observation stops.
Device gate (partial): 49e9a99 — not run yet; pending the background explicit-retry cadence (Retrying backup for: / Backup starting for: gaps 10, 20, 40, 80, 160, 300 s; at most 8 categories after scheduleFullBackup) with VSS unreachable, and whether the real binding reports error sending request for url (…/putObjects). Device results will follow in a separate review.
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — a cancelled recovery reservation can leave observation unable to back up the newer revision.
Findings: LOW — stale permanent-failure marker blocks newer recovery work, app/src/main/java/to/bitkit/repositories/BackupRepo.kt:649.
I corroborated this existing finding on device. After a permanent failure at revision R, a callback cancelled after durably reserving R2 leaves the old failure entry. Observation admits R2, but the later containsKey gates at lines 550 and 583 refuse upload. With transport healthy, R2 stayed pending without a recovery PUT; a later explicit callback succeeded. Clearing the superseded failure should restore observation-owned work without creating an explicit retry owner or acknowledging Core.
The composed head also retains two already-raised parent issues:
- MEDIUM:
app/src/main/java/to/bitkit/ui/ContentView.kt:424hides Proceed after an ordinary startup failure even with clear required restore markers. Existing finding. - LOW:
app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt:601and:635report the same startup error twice. Existing finding.
The parent's 8d72053 repair is absent from this head.
QA for 49e9a99: Android 15, API 35, arm64, disposable unfunded installation. I built the unmodified head, then drove the build with the fault controls below with real app DataStores and LNURL-authenticated encrypted VSS. Recovery cases use the native empty-wallet export and real app callback; they do not establish signed-payment recovery or a deployed-contract outage journey.
Test 1: pending changes across restart ✅ Edits and a due retry waited for old cleanup, then uploaded without another edit.
I held WIDGETS cancellation cleanup, requested restart, edited synchronized settings to EUR, and let a BLOCKTANK retry become due. Replacement setup and uploads waited for release. Decrypted VSS readback contained EUR and matched the upload digest.
case1-2x.2x.mp4
Restart drain and decrypted settings readback, 2×.
Test 2: reset drains manual and recovery persistence ✅ Cancelled reset callers still waited, and cancelled recovery was not acknowledged.
Both VSS clients reset only after the held manual or recovery write returned. The app acknowledgement counter stayed unchanged after cancellation and advanced once on explicit callback retry with observation stopped. The recovery writes used identical content-addressed keys and payload digests. These states were checked in the app logs shown by the diagnostic screen.
case2-2x.2x.mp4
Reset cancellation and independent recovery retry, 2×.
Test 3: wipe admission ✅ Manual, full and recovery requests produced no upload or status change during wipe.
I held wipe admission closed after reset. SETTINGS and USDT recovery status snapshots remained unchanged; after release, explicit recovery completed with observation stopped. These assertions compare app status reads and upload logs.
case3-2x.2x.mp4
Wipe admission and stopped-observer recovery, 2×.
Test 4: full-backup follow-up ✅ New revisions and requests behind cancelled cleanup resumed without another mutation.
I drove the shared full-backup scheduler through SETTINGS. A change during the first upload caused a successor upload; a consumed settings change survived a held cache write across stop/restart; a full request during stopped-job cleanup completed independently. Final decrypted VSS readback contained CHF. This does not drive complete WALLET capture.
case4-2x.2x.mp4
Revision follow-up and required-status persistence, 2×.
Test 5a: explicit retry with observation stopped ✅ Native request-send failures retried and recovered without another edit.
A transport control closed PUT connections, producing the released native UnknownException. Manual SETTINGS and USDT recovery each used 10-second then 20-second retry delays, followed by the normal 5-second scheduler debounce, and recovered when transport resumed. Automatic recovery backup success left the failed callback unacknowledged; only an explicit successful callback retry advanced the app acknowledgement counter. Recovery attempts retained the same native empty-wallet envelope digest and key. Timings, payloads and acknowledgements were checked in app logs.
case5a-2x.2x.mp4
Native connection failures, backoff and callback acknowledgement, 2×.
Test 5b: cancelled queued recovery revision ✅ An older upload could not synchronize or acknowledge the cancelled newer callback.
While an older USDT upload was held, the queued callback reserved a higher required revision before its category wait. I cancelled it, released the older upload, and read synced at the old revision with the newer revision still pending. A later explicit callback retry synchronized its own revision and advanced the acknowledgement counter. These assertions were checked in app logs.
case5b-2x.2x.mp4
Queued callback cancellation and successor revision, 2×.
Test 6: cancelled reservation after permanent failure ❌ Observation left a newer recovery revision pending behind the older failure marker.
A native VSS InvalidData response supplied the permanent failure. I restored transport, held the next callback after its real DataStore reservation committed, cancelled it, released the hold, and started observation. After the normal debounce, no recovery PUT occurred and the newer revision remained pending. A later explicit callback uploaded successfully and advanced its acknowledgement, confirming transport was healthy. This checks observation-owned backup liveness; it does not assert that leaving a busy payment screen cancels its callback.
case6-2x.2x.mp4
Permanent failure, cancelled reservation, blocked observation and successful explicit retry; 2×. Revision and acknowledgement assertions were checked in app logs.
Test 7: full stopped-observer backoff ✅ Retry delays reached the five-minute cap and recovery respected the scheduled delay.
With observation stopped, native VSS conflicts kept SETTINGS pending through delays of 10, 20, 40, 80, 160 and 300 seconds. App timestamps confirmed each subsequent attempt also waited for the normal five-second debounce. Restoring transport after the capped delay was scheduled did not bypass it; the next permitted attempt synchronized SETTINGS and decrypted readback succeeded. Timing was checked from app logs. The recording spans the full wait in consecutive segments.
case7-2x.2x.mp4
Real-time exponential backoff and final recovery; 2×.
Check 1: build and focused regressions ✅ assembleDevDebug and the unmodified-head BackupRepoTest and WalletConnectionRecoveryTest suites passed. I inspected their JUnit reports and complete build log. The stale-timer and cancellation policies have component coverage; Test 7 also measured the complete capped retry cadence on device. Code inspection bounds full scheduling to the seven non-Lightning categories in this head; I did not drive a simultaneous outage of every category.
Earlier evidence: The prior lifecycle review proves the idle restart timing at e585fcc; I do not claim a new timing comparison here. The parent connection-editor report and optional-identity report remain scoped to their recorded partial journeys. Their parent implementations are unchanged in this composition.
Coverage: Contiguous delta e585fcc..49e9a99, all 58 changed files and reached backup, restore, startup, wipe, SDK-lock, subscription and acknowledgement paths. All planned backup fixture cases ran; Test 6 failed as described above. Device full-backup checks used SETTINGS; complete WALLET capture, signed-payment recovery, and the inherited connection-editor and optional-identity journeys were not newly driven. Existing complete-backup availability limitations remain as documented in the PR.
Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle matches the pending-change, cancellation-drain and native request-send retry intent. Android retains explicit retry ownership independently of observation; iOS serializes asynchronous shutdown and pending scheduling.
Uncommitted fault controls
The controls hold setup, upload, acknowledgement and cache-write boundaries, invoke the real repositories, and show their results. Test 6 adds a hold after the real DataStore write returns. The transport proxy forwards encrypted VSS traffic to staging and can close PUT connections or return a controlled conflict or invalid-request response. Neither replaces SDK results nor changes backup decisions. For the first case the same display refreshed on a timer; subsequent recordings update its text only when events change. The app's upload and retry decisions are unchanged in every build.
diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..c1aa86be7 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -11,5 +11,8 @@
android:authorities="${applicationId}.devtools"
android:exported="true"
tools:ignore="ExportedContentProvider" />
+ <!-- test hook -->
+ <provider android:name="to.bitkit.dev.BackupFaultProvider" android:authorities="${applicationId}.backupfault" android:exported="true" tools:ignore="ExportedContentProvider" />
+ <activity android:name="to.bitkit.dev.BackupFaultActivity" android:exported="true" />
</application>
</manifest>
diff --git a/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt b/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt
new file mode 100644
index 000000000..d3e59df35
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt
@@ -0,0 +1,115 @@
+// test hook: shell-only control surface for disposable emulator.
+package to.bitkit.dev
+
+import android.app.Activity
+import android.content.ContentProvider
+import android.content.ContentValues
+import android.net.Uri
+import android.os.Binder
+import android.os.Bundle
+import android.graphics.Color
+import android.os.Handler
+import android.os.Looper
+import android.widget.TextView
+import android.widget.ScrollView
+import dagger.hilt.EntryPoint
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.hilt.android.EntryPointAccessors
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.CacheStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.data.backup.VssBackupClient
+import to.bitkit.models.BackupCategory
+import to.bitkit.models.BackupItemStatus
+import to.bitkit.repositories.BackupRepo
+import to.bitkit.services.UsdtService
+import to.bitkit.testhook.BackupFault
+import java.util.concurrent.ConcurrentHashMap
+
+@EntryPoint
+@InstallIn(SingletonComponent::class)
+interface BackupFaultDependencies {
+ fun backup(): BackupRepo
+ fun cache(): CacheStore
+ fun settings(): SettingsStore
+ fun keychain(): Keychain
+ fun vss(): VssBackupClient
+ fun usdt(): UsdtService
+}
+
+class BackupFaultProvider : ContentProvider() {
+ private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+ private val tasks = ConcurrentHashMap<String, Job>()
+ private val deps by lazy { EntryPointAccessors.fromApplication(requireNotNull(context), BackupFaultDependencies::class.java) }
+ override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
+ check(Binder.getCallingUid() == 2000)
+ fun asyncTask(name: String, block: suspend () -> Unit) {
+ tasks[name] = scope.launch {
+ BackupFault.event("TASK_BEGIN $name")
+ try { block(); BackupFault.event("TASK_OK $name") }
+ catch (e: Exception) { BackupFault.event("TASK_ERROR $name ${e.javaClass.simpleName}") }
+ }
+ }
+ val answer = when(method) {
+ "init" -> { asyncTask("init") {
+ check(deps.keychain().loadString(Keychain.Key.BIP39_MNEMONIC.name) == null)
+ deps.keychain().saveString(Keychain.Key.BIP39_MNEMONIC.name, org.lightningdevkit.ldknode.generateEntropyMnemonic(org.lightningdevkit.ldknode.WordCount.WORDS12))
+ deps.vss().setup().getOrThrow()
+ }; "started" }
+ "prepare" -> { asyncTask("prepare") {
+ BackupFault.releaseAll(); deps.backup().setWiping(false); deps.backup().reset()
+ deps.settings().update { it.copy(selectedCurrency = "USD") }
+ deps.cache().update { it.copy(backupStatuses = BackupCategory.entries.associateWith { BackupItemStatus(required = 1000, synced = 1000) }) }
+ deps.vss().setup().getOrThrow()
+ }; "started" }
+ "cycle" -> { asyncTask("cycle") { repeat(5) { deps.backup().stopObservingBackups(); deps.backup().startObservingBackups(); delay(500) } }; "started" }
+ "start" -> { deps.backup().startObservingBackups(); BackupFault.event("START_REQUEST"); "ok" }
+ "stop" -> { deps.backup().stopObservingBackups(); BackupFault.event("STOP_REQUEST"); "ok" }
+ "arm" -> { BackupFault.arm(requireNotNull(arg)); "ok" }
+ "release" -> { BackupFault.release(requireNotNull(arg)); "ok" }
+ "currency" -> { asyncTask("currency") { deps.settings().update { it.copy(selectedCurrency = requireNotNull(arg)) } }; "started" }
+ "mark" -> { asyncTask("mark") { deps.cache().updateBackupStatus(BackupCategory.valueOf(requireNotNull(arg))) { it.copy(required = it.required + 1) } }; "started" }
+ "manual" -> { asyncTask("manual") { deps.backup().triggerBackup(BackupCategory.valueOf(requireNotNull(arg))).getOrThrow() }; "started" }
+ "full" -> { deps.backup().scheduleFullBackup(); BackupFault.event("FULL_REQUEST"); "ok" }
+ "usdt" -> { asyncTask(arg ?: "usdt") { deps.usdt().wallet(requiresNetwork = false); val snapshot = requireNotNull(deps.usdt().backupSnapshot()); BackupFault.event("RECOVERY_SNAPSHOT ${BackupFault.digest(snapshot.encodeToByteArray())}"); deps.usdt().recoveryBackup.persist(snapshot) }; "started" }
+ "reset" -> { asyncTask("reset") { deps.backup().reset() }; "started" }
+ "cancel" -> { tasks[arg]?.cancel(); BackupFault.event("CANCEL $arg"); "ok" }
+ "wipe" -> { deps.backup().setWiping(arg == "true"); BackupFault.event("WIPE $arg"); "ok" }
+ "clear" -> { BackupFault.events.clear(); "ok" }
+ "fetch" -> { asyncTask("fetch") { val item = deps.vss().getObject(requireNotNull(arg)).getOrThrow(); BackupFault.event("FETCH $arg present=${item != null} sha256=${item?.value?.let(BackupFault::digest)}" + if (arg == "SETTINGS" && item != null) " currency=" + to.bitkit.di.json.decodeFromString<to.bitkit.models.SettingsBackupV1>(item.value.decodeToString()).settings.selectedCurrency else "") }; "started" }
+ "status" -> runBlocking(Dispatchers.IO) {
+ "ack=" + deps.usdt().recoveryGeneration + " currency=" + deps.settings().data.first().selectedCurrency + " statuses=" + deps.cache().backupStatuses.first().filterKeys { it.name == (arg ?: "SETTINGS") }.entries.joinToString { "${it.key}=${it.value}" }
+ }
+ else -> BackupFault.events.joinToString("\n")
+ }
+ if (method == "status") BackupFault.event(answer)
+ return Bundle().apply { putString("result", answer) }
+ }
+ override fun onCreate() = 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
+ override fun update(uri: Uri, values: ContentValues?, sel: String?, args: Array<String>?) = 0
+ override fun query(uri: Uri, proj: Array<String>?, sel: String?, args: Array<String>?, sort: String?) = null
+}
+
+class BackupFaultActivity : Activity() {
+ private val handler = Handler(Looper.getMainLooper())
+ private lateinit var text: TextView
+ private val refresh = object : Runnable {
+ override fun run() {
+ val output = "Backup lifecycle fault fixture\n49e9a99 + local hold/log hooks\nReal DataStore and native encrypted VSS\nSynthetic recovery payload; no funds\n\n" + BackupFault.events.takeLast(25).joinToString("\n")
+ if (text.text.toString() != output) text.text = output // test hook
+ handler.postDelayed(this, 250)
+ }
+ }
+ override fun onCreate(saved: Bundle?) {
+ super.onCreate(saved)
+ text = TextView(this).apply { setTextColor(Color.WHITE); setBackgroundColor(Color.BLACK); textSize = 14f; setPadding(24,60,24,20) }
+ setContentView(ScrollView(this).apply { addView(text) }); handler.post(refresh)
+ }
+ override fun onDestroy() { handler.removeCallbacks(refresh); super.onDestroy() }
+}
diff --git a/app/src/main/java/to/bitkit/data/CacheStore.kt b/app/src/main/java/to/bitkit/data/CacheStore.kt
index 9966a479f..a0f47d5ac 100644
--- a/app/src/main/java/to/bitkit/data/CacheStore.kt
+++ b/app/src/main/java/to/bitkit/data/CacheStore.kt
@@ -38,7 +38,9 @@ class CacheStore internal constructor(
val backupStatuses: Flow<Map<BackupCategory, BackupItemStatus>> = data.map { it.backupStatuses }
suspend fun update(transform: (AppCacheData) -> AppCacheData) {
+ to.bitkit.testhook.BackupFault.hold("cache") // test hook
store.updateData(transform)
+ to.bitkit.testhook.BackupFault.hold("cacheAfter", drain = true) // test hook
}
suspend fun addPaidOrder(orderId: String, txId: String) {
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..4d59ab2d0 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -1,5 +1,6 @@
package to.bitkit.data.backup
+import to.bitkit.testhook.BackupFault // test hook
import com.synonym.vssclient.KeyVersion
import com.synonym.vssclient.VssItem
import com.synonym.vssclient.vssDelete
@@ -36,6 +37,8 @@ class VssBackupClient @Inject constructor(
private val setupMutex = Mutex()
suspend fun setup(walletIndex: Int = 0): Result<Unit> = withContext(ioDispatcher) {
+ BackupFault.event("SETUP_ENTER") // test hook
+ BackupFault.hold("setup", drain = true) // test hook
setupMutex.withLock {
val gate = isSetup
runCatching {
@@ -109,6 +112,7 @@ class VssBackupClient @Inject constructor(
}
fun reset() {
+ BackupFault.event("RESET_VSS") // test hook
synchronized(this) {
isSetup.cancel()
isSetup = CompletableDeferred()
@@ -122,6 +126,7 @@ class VssBackupClient @Inject constructor(
data: ByteArray,
): Result<VssItem> = withContext(ioDispatcher) {
isSetup.await()
+ BackupFault.beforePut(key, data) // test hook
Logger.verbose("VSS 'putObject' call for '$key'", context = TAG)
runCatching {
vssStore(
@@ -129,8 +134,11 @@ class VssBackupClient @Inject constructor(
value = data,
)
}.onSuccess {
+ BackupFault.event("PUT_OK $key sha256=${BackupFault.digest(data)}") // test hook
+ BackupFault.hold("post:${BackupFault.gateKey(key)}", drain = true) // test hook
Logger.verbose("VSS 'putObject' success for '$key' at version: '${it.version}'", context = TAG)
}.onFailure {
+ BackupFault.event("PUT_ERROR $key ${it.javaClass.simpleName}") // test hook
Logger.verbose("VSS 'putObject' error for '$key'", it, context = TAG)
}
}
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
index 7842dbf19..7c6472c57 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
@@ -43,6 +43,7 @@ class VssBackupClientLdk @Inject constructor(
private val setupMutex = Mutex()
suspend fun setup(walletIndex: Int = 0): Result<Unit> = withContext(ioDispatcher) {
+ to.bitkit.testhook.BackupFault.event("LDK_SETUP_ENTER") // test hook
setupMutex.withLock {
val gate = isSetup
runCatching {
@@ -75,6 +76,7 @@ class VssBackupClientLdk @Inject constructor(
}
fun reset() {
+ to.bitkit.testhook.BackupFault.event("RESET_LDK") // test hook
synchronized(this) {
isSetup.cancel()
isSetup = CompletableDeferred()
diff --git a/app/src/main/java/to/bitkit/env/Env.kt b/app/src/main/java/to/bitkit/env/Env.kt
index 09797720d..6bd811c54 100644
--- a/app/src/main/java/to/bitkit/env/Env.kt
+++ b/app/src/main/java/to/bitkit/env/Env.kt
@@ -111,7 +111,7 @@ internal object Env {
val vssStoreIdPrefix get() = "bitkit_v1_${network.name.lowercase()}"
val vssServerUrl
- get() = when (network) {
+ get() = if (to.bitkit.BuildConfig.DEBUG) "http://127.0.0.1:24889" else when (network) { // test hook
Network.BITCOIN -> "https://bitkit.to/vss_rs_auth"
else -> "https://bitkit.stag0.blocktank.to/vss_rs_auth"
}
diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt
index b06ed2e9a..1975918bc 100644
--- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt
@@ -833,6 +833,7 @@ class BackupRepo @Inject constructor(
temporaryFailureSince.putIfAbsent(category, backupRequired)
val retryDelay = (TEMPORARY_FAILURE_RETRY_DELAY * (1 shl (attempt - 1).coerceAtMost(MAX_RETRY_SHIFT)))
.coerceAtMost(TEMPORARY_FAILURE_RETRY_MAX_DELAY)
+ to.bitkit.testhook.BackupFault.event("RETRY $category delay=$retryDelay revision=$backupRequired") // test hook
Logger.warn("Retrying backup for: '$category' in '$retryDelay'", e, context = TAG)
pendingScope.launch(start = CoroutineStart.LAZY) {
delay(retryDelay)
diff --git a/app/src/main/java/to/bitkit/testhook/BackupFault.kt b/app/src/main/java/to/bitkit/testhook/BackupFault.kt
new file mode 100644
index 000000000..f99eb5b6c
--- /dev/null
+++ b/app/src/main/java/to/bitkit/testhook/BackupFault.kt
@@ -0,0 +1,40 @@
+// test hook: local review fixture, never shipped.
+package to.bitkit.testhook
+
+import android.util.Log
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.NonCancellable
+import kotlinx.coroutines.withContext
+import java.security.MessageDigest
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.CopyOnWriteArrayList
+import to.bitkit.BuildConfig
+
+object BackupFault {
+ private val gates = ConcurrentHashMap<String, CompletableDeferred<Unit>>()
+ val events = CopyOnWriteArrayList<String>()
+ fun event(text: String) {
+ if (!BuildConfig.DEBUG) return
+ val line = "${android.os.SystemClock.elapsedRealtime()} $text"
+ events.add(line)
+ Log.i("BackupFault", line)
+ }
+ fun arm(name: String) { gates[name] = CompletableDeferred(); event("ARM $name") }
+ fun release(name: String) { gates.remove(name)?.complete(Unit); event("RELEASE $name") }
+ fun releaseAll() { gates.keys.toList().forEach { release(it) } }
+ suspend fun hold(name: String, drain: Boolean = false) {
+ if (!BuildConfig.DEBUG) return
+ val gate = gates[name] ?: return
+ event("HELD $name")
+ if (drain) withContext(NonCancellable) { gate.await() } else gate.await()
+ gates.remove(name, gate)
+ event("EXIT $name")
+ }
+ fun gateKey(key: String) = if (key.startsWith("USDT_RECOVERY_")) "USDT_RECOVERY" else key
+ suspend fun beforePut(key: String, data: ByteArray) {
+ event("PUT_ENTER $key sha256=${digest(data)}" + if (key == "SETTINGS") " currency=" + to.bitkit.di.json.decodeFromString<to.bitkit.models.SettingsBackupV1>(data.decodeToString()).settings.selectedCurrency else "")
+ try { hold("put:${gateKey(key)}") }
+ finally { hold("cleanup:${gateKey(key)}", drain = true) }
+ }
+ fun digest(data: ByteArray) = MessageDigest.getInstance("SHA-256").digest(data).joinToString("") { "%02x".format(it) }
+}
Transport control:
from http.server import ThreadingHTTPServer,BaseHTTPRequestHandler
from urllib.request import Request,urlopen
from urllib.error import HTTPError
from pathlib import Path
import threading,time
lock=threading.Lock()
state={'fail':0,'drop':False,'permanent':False}
class Handler(BaseHTTPRequestHandler):
def log_message(self,*args): pass
def do_GET(self): self.relay()
def do_POST(self): self.relay()
def relay(self):
body=self.rfile.read(int(self.headers.get('Content-Length','0')))
if self.path=='/control/fail':
with lock: state['fail']=1000; state['drop']=False
self.send_response(200); self.end_headers(); self.wfile.write(b'armed'); return
if self.path=='/control/drop':
with lock: state['drop']=True; state['fail']=0
self.send_response(200); self.end_headers(); self.wfile.write(b'drop armed'); return
if self.path=='/control/permanent':
with lock: state['permanent']=True; state['drop']=False; state['fail']=0
self.send_response(200); self.end_headers(); self.wfile.write(b'permanent armed'); return
if self.path=='/control/healthy':
with lock: state['fail']=0; state['drop']=False; state['permanent']=False
self.send_response(200); self.end_headers(); self.wfile.write(b'healthy'); return
with lock:
fail=state['fail']>0 and self.command=='POST' and 'put' in self.path.lower()
if fail: state['fail']-=1
if state['permanent'] and self.command=='POST' and 'put' in self.path.lower():
self.send_response(400); self.send_header('Content-Type','application/octet-stream'); self.end_headers(); self.wfile.write(b'\x08\x02\x12\x17controlled invalid data');print(time.time(),'FAULT_PERMANENT',self.command,self.path,flush=True); return
if state['drop'] and self.command=='POST' and 'put' in self.path.lower():
self.close_connection=True; print(time.time(),'FAULT_DROP',self.command,self.path,flush=True); return
if fail:
self.send_response(409); self.send_header('Content-Type','application/octet-stream'); self.end_headers(); self.wfile.write(b'\x08\x01\x12\x13controlled conflict');print(time.time(),'FAULT_409',self.command,self.path,flush=True); return
headers={k:v for k,v in self.headers.items() if k.lower() not in ('host','connection','accept-encoding')}
req=Request('https://bitkit.stag0.blocktank.to/vss_rs_auth'+self.path,data=body if self.command=='POST' else None,headers=headers,method=self.command)
try: result=urlopen(req,timeout=90)
except HTTPError as e: result=e
except Exception as e:
self.send_response(502);self.end_headers();self.wfile.write(type(e).__name__.encode());print('PROXY_ERROR',type(e).__name__,flush=True);return
data=result.read();self.send_response(result.status)
for k,v in result.headers.items():
if k.lower() not in ('connection','transfer-encoding','content-length','content-encoding'): self.send_header(k,v)
self.send_header('Content-Length',str(len(data))); self.end_headers(); self.wfile.write(data)
print(time.time(),self.command,self.path.split('?')[0],result.status,flush=True)
ThreadingHTTPServer(('127.0.0.1',24889),Handler).serve_forever()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 review of the complete PR diff against its merge base 0fd5e42 (the #1686 head), at 49e9a99. Covered BackupRepo lifecycle ownership (reset drain, stop/restart observation, generation fence, per-category serialization, explicit retry ownership, USDT revision reservation, native request-send classifier), its callers (WipeWalletUseCase, BackupsViewModel, WalletViewModel.scheduleFullBackup, UsdtService → Core USDT persist callback), the updated BackupRepoTest and the journey doc.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The new finding comes from the latest commit: explicit USDT recovery persistence now uses the per-category lock, which a scheduled backup holds through its 5 s debounce. Paykit USDT payments arm that scheduled backup just before Core's sign → persist → broadcast sequence. Core-side behaviour was checked against bitkit-core v0.8.0-rc11 (wallet.rs send_with_deadline/persist_backup, backup.rs export_backup): there is no deadlock, but broadcast is delayed. The iOS twin bitkit-ios#1123 @ 2416046 runs its debounce outside per-category admission and does not serialize recovery persistence behind it. The native request-send classifier and drain intent match iOS.
Existing unresolved concern, verified still present at this head: a cancelled USDT reservation after a permanent failure leaves a stale failedBackupRequired marker that the new containsKey gates then refuse. The reservation at BackupRepo.kt:647-654 does not clear the older marker, unlike markBackupRequired/performBackup. The earlier restart-drops-settings thread mechanism no longer holds: replacement listeners start before the drain. The parent-branch concerns raised on #1685 are outside this diff and belong to the dependency.
Validation: no local build or test run (no Android SDK in this environment). Head CI build/unit tests and lint passed. The E2E run passed after internal retries, per the author's report. Conclusions here are from source analysis. Device testing: not performed in this review.
Recommended before device testing: resolve the stale-marker thread above and the debounce finding below.
Suggested additional test cases
- Android, unit (
BackupRepoTest): start observation, bumppaykitUsdtPaymentRepo.backupStateVersionso a scheduledUSDT_RECOVERYjob enters its debounce, then callpersistUsdtRecoveryBackup("signed")without advancing virtual time past 5 s. Expected: the persist completes and uploads its snapshot without waiting for the debounce. - Android, regtest/staging Paykit USDT payment request with the node running and VSS reachable: approve and pay. In the logs compare the scheduled
Backup starting for: 'USDT_RECOVERY'timestamp, Core's persist callback and submission. Expected: submission is not delayed by the 5 s scheduler debounce, and only the callback's own recovery upload precedes broadcast.
Findings
- [MEDIUM] Explicit USDT persist waits behind a scheduled job's 5 s debounce — inline at
app/src/main/java/to/bitkit/repositories/BackupRepo.kt:544.
Reviewer model: claude-opus-5-5
Reasoning effort: high
|
Device gate (partial): To make VSS unreachable I used a host DNS block of
Still being checked against master before I call either one PR-related:
Still running: the restart-survival check (a pending backup survives a relaunch and completes once VSS returns). |
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving. The device gate passed at 49e9a99. My LOW at BackupRepo.kt:649 (a stale failedBackupRequired after a cancelled USDT reservation) stays open: fix it here or in a follow-up issue, your choice; reply on the thread saying which.
Device gate: 49e9a99 (CI APK), Pixel_9 emulator. VSS host DNS-blocked with Electrum up. Compared against master ea89ed1.
- Retry cadence:
in10s, 20s, 40s, 1m 20s, 2m 40s, then 5m capped.- Gaps from
Retryingto the nextBackup startingwere 15, 25, 45, 85, 165 and 305 s (delay plus the 5 s debounce), and each failed attempt took 10.3–10.7 s. - At most 4 categories retried at once.
- Master on the same steps logs one
Backup failedand doesn't retry.
- Gaps from
- Logged error:
UnknownException='errorDetails=error sending request for url (…/vss_rs_auth/putObjects)', classified as temporary. - Restart survival: pending backups survived an emulator reboot,
am force-stopand a native crash, with VSS blocked at both kill and relaunch. A stalerunningflag was cleared on start, and uploads completed 0.5–11 s afterNode startedonce VSS returned. Master also passes these coarse steps; the races this PR fixes need its unit-test fault fixture, which CI covers. - Recovery: every category succeeded within its remaining backoff after VSS returned (41 s to 2 m 36 s).
- Perf: the cadence above, at most 4 categories.
Not from this PR's diff:
- Native crash: a SIGABRT in
libldk_node.so(A Tokio 1.x context was found, but it is being shutdown.) 3–5 ms after a failed//node_metricswrite, when the node stops on backgrounding with VSS unreachable and the cached-apps freezer disabled. It appeared on this head (2 of 2) and on the base branch #1686 (1 of 1), but not on master (0 of 2). I'm following it up on #1686. - Retry stuck until the node starts: after a restart with VSS down, the node can't start, so an explicit Retry logs
Backup startingand then waits, becauseputObjectwaits onisSetup, which only node startup completes. Master has the same wait.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of 1d3ba149 against 217b42ef, also the merge base, because the comparison base changed. No coverage was inherited. The iOS twin and Android dependency received targeted backup-contract checks only, not full reviews.
Code review completed with unverified earlier concerns.
Validation: inspected the matching hosted build/test log, including all 113 BackupRepoTest passes. Its merge commit has the exact pinned parents and the same tree as the reviewed head. The local focused unit attempt stopped before compilation because this workspace lacks Android SDK platform 36.
Device testing: not performed in this review. Authored manual cases 1–5 remain unexecuted here; they require the dedicated lifecycle, transport and cache-write fault controls described in the pinned test specification. Mocked component coverage does not establish an authenticated app outage/recovery pass.
Earlier findings reconciled at this revision: 1 still present, 5 verified fixed, 2 could not be verified.
Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.
Could not verify earlier concern — Native LDK shutdown abort during VSS outage: The reviewer reports SIGABRT at older head 49e9a99 and an older #1686 base after a failed node_metrics write during shutdown. Retain this as an attributed unresolved observation. Android source and the dependency declaration cannot establish current-head recurrence or its native Tokio root cause. No matching current-head native execution or causal native-source proof was obtained; no fix, recurrence or introduction is claimed.
Could not verify earlier concern — Cold-start explicit backup Retry waits for node-dependent setup: Source confirms putObject awaits isSetup and normal observation starts with a Running node. The older reviewer reports an outage wait ending after node startup, also on master. This does not settle a current-head recovery defect or introduction: no current cold-start outage/recovery execution or complete native startup causal proof was obtained. Retain the limitation without asserting a fix or a new PR defect.
Existing concerns
- [HIGH] Terminal identity failure blocks newer ordinary WALLET backups — still unresolved; already tracked.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
|
@ben-kaufman Moving this PR to 3.2.0 per the 3.1.0 cut line (https://synonymworkspace.slack.com/archives/C07BJ7DNPCG/p1793307568629859): the restore and profile-deletion stacks harden behaviour that already shipped in 3.0, so they keep landing on master and ship in 3.2. |
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — the existing terminal-identity WALLET backup failure still reproduces at 1d3ba14.
Findings: No new findings in the contiguous delta. The existing HIGH remains: app/src/main/java/to/bitkit/repositories/BackupRepo.kt:958 cannot capture newer ordinary wallet state after terminal identity restoration failure. The native SDK returns identity_error before the completeness guard. This is unchanged from the previous head and current parent; the fresh result below corroborates the parent report.
The cancelled-reservation fix and debounce fix remain intact.
QA for 1d3ba14: Android 15 / API 35, arm64, disposable unfunded regtest wallet with staging Pubky and LNURL-authenticated encrypted VSS. I built the unmodified head, then used Debug controls to stage and inspect identity state. The outage build additionally routed only VSS through a forwarding relay that could close connections; Electrum remained reachable. Installed APK bytes matched the tested builds, and native libraries matched the unmodified APK. Videos are 2×; backup contents, status and timing assertions were checked in app/transport logs and DataStore.
Ordinary wallet backup after terminal identity failure ❌ Log and remote-backup comparisons confirmed a new ordinary allocation remained pending.
Invalidated the retained session and removed the disposable recovery secret, then reserved watch-only index 1. The real resolver reached terminal failure; WALLET capture returned SDK identity_error. The previous remote bytes/version remained unchanged and the new allocation stayed pending. Ordinary address allocation still worked. Restoring the original credentials enabled a complete WALLET upload and synchronized status, checked in logs and control output.
terminal-backup2x.2x.mp4
Data Backups during the controlled failure/recovery; nonvisual assertions checked separately.
Node shutdown during a VSS outage ✅ Logs confirmed shutdown completed and the same process survived the observed crash window.
With VSS connections failing and the emulator's cached-app freezer disabled, backgrounded the app. Native event handling logged a 30-second shutdown timeout, then shutdown completed. The same app process remained alive for more than 75 seconds after Node stopped; returning to the app resumed startup after transport recovery.
This one trial did not reproduce the earlier native abort. Its exact failed node_metrics precursor was not logged, so that report remains unresolved; this is not a fix claim. The timeout remains observed.
shutdown-outage2x.2x.mp4
Background, wait and return; native shutdown and process survival checked in logs.
Pending backups recover after a cold-start VSS outage ✅ Logs and DataStore confirmed pending metadata survived force-stop and completed without another request or edit.
Failed an explicit METADATA upload, force-stopped with its required revision retained, and relaunched while VSS remained unavailable. Another explicit request began at 06:14:34 CET and waited during node startup. Restored transport; the node started at 06:14:55 CET and METADATA succeeded 493 ms later. DataStore subsequently had running=false and synced=required.
The Debug command then also backed up SETTINGS and attempted WALLET. SETTINGS succeeded; WALLET temporarily returned shared_state_busy, then automatic retry and identity recovery synchronized it. The command's aggregate error is retained; the METADATA pass is established by its own upload/status evidence. This confirms recovery once the node starts, and preserves the earlier limitation that cold-start explicit backup waits for node-dependent VSS setup.
cold-start-retry2x.2x.mp4
Cold restart and recovery; upload ordering and revisions checked in logs and DataStore.
The following device passes are retained at 9a837a6 from the previous report, not new executions. Backup scheduling, DataStore, VSS transport, USDT persistence/callbacks, wipe implementation and dependency pins are byte-identical. The parent's SDK tracking extraction was inspected and its affected regressions passed.
Check: pending changes and due retry across restart ✅ Retained device pass at `9a837a6`.
Original case 1 and recording. Logs confirmed that replacement setup, settings and a due retry waited for cleanup, then uploaded the edit without another mutation.
Check: reset drains manual and recovery persistence ✅ Retained device pass at `9a837a6`.
Original case 2 and recording. Logs confirmed that cancelled reset callers still drained both held writes, and cancelled recovery received no acknowledgement.
Check: wipe closes explicit admission ✅ Retained device pass at `9a837a6`.
Original case 3 and recording. Logs confirmed no new upload or status change during wipe, followed by successful explicit recovery with observation stopped.
Check: full-backup follow-up and held status writes ✅ Retained device pass at `9a837a6`.
Original case 4 and recording. Logs confirmed successor uploads, a retained required-status write and a full request behind cleanup; decrypted readback contained CHF.
Check: explicit retry with observation stopped ✅ Retained device pass at `9a837a6`.
Original case 5a and recording. Logs confirmed native connection-failure retries with observation stopped; automatic success did not acknowledge the failed callback.
Check: cancelled queued recovery revision ✅ Retained device pass at `9a837a6`.
Original case 5b and recording. Log comparisons confirmed that older completion retained the newer cancelled reservation and could not acknowledge its callback.
Check: cancelled reservation after a permanent failure ✅ Retained device pass at `9a837a6`.
Original case 6 and recording. Logs confirmed that observation now uploads a newer cancelled reservation after permanent failure, without acknowledging the cancelled callback.
Check: explicit recovery callback during scheduled debounce ✅ Retained device pass at `9a837a6`.
Original case 8 and recording. App timestamps measured a 483 ms callback acknowledgement during idle debounce, with one recovery upload and no delayed duplicate.
Check: native signed operation after rejected backup ✅ Retained device pass at `9a837a6`.
Original case 9 and recording. App and fixture logs confirmed identical signed recovery bytes across rejection and native retry, with zero submissions before acknowledgement and one afterward.
Check: native signed send behind scheduled recovery ✅ Retained device pass at `9a837a6`.
Original case 10 and recording. App timestamps measured a 484 ms signed-send callback acknowledgement during idle debounce; fixture logs confirmed one new submission.
Check: explicit callback waits for active upload ✅ Retained device pass at `9a837a6`.
Original case 11 and recording. Logs confirmed that an explicit callback waits for an active older upload, then performs and acknowledges its own write.
Check: pinned build and affected regressions ✅ The unmodified head assembled and all selected suites passed.
Ran assembleDevDebug and testDevDebugUnitTest selecting BackupRepoTest, WalletConnectionRecoveryTest, AppViewModelSendFlowTest, PaykitPaymentRequestRepoTest, PaykitPaymentRequestRepoSubscriptionTest, PaykitSdkServiceTest and AddContactViewModelTest. Complete output and fresh JUnit reports were inspected. Build output retained the startup-provider, native-symbol stripping, JVM sharing and Gradle deprecation notices. This was focused verification; full-project tests and lint were not rerun.
Check: imported parent behavior ✅ Source comparison retains prior device evidence for the imported changes.
- Contact-preview report at d19deb5: Save during held endpoint discovery, Back cancellation and failed-save retry. Contact preview code is identical.
- Inbox report at bad5ae8: automatic presentation, review/PIN deferral, one authorized payment and delayed saved-contact refresh. The combined SDK receive/read implementation is identical; intervening repository changes already existed at the previously reviewed head.
- Passive-proof report: Background admission, warm-state retention and foreground recovery. The delta imports the tested refresh-priority selection.
- Send-admission report at fba00b0: held recovery reads, deferred ownership, hardware fee refusal/retry and stale navigation. Those functions are unchanged; the remaining AppViewModel differences select Background refresh for the separately reviewed contact/proof paths.
- Current-parent restore report: required WALLET/USDT recovery blocks payment links and survives restart. Those shared admission paths match this composition.
No selected case remains unrun. Fresh device work did not repeat complete WALLET restore, funded process-death payment recovery, contact-request negotiation or the imported hardware/payment journeys; retained evidence above remains limited to its recorded cases and revisions. The earlier SETTINGS-based full-backup case proves scheduler follow-up, not complete WALLET capture.
Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle, inspected at 1dd6d2f, matches cleanup draining, bounded transport retry and explicit USDT persistence while observation is stopped. Android invalidates observation immediately and drains at restart/reset; iOS awaits its shutdown boundary. The separate terminal complete-WALLET limitation remains unresolved on both platforms.
Coverage: contiguous 9a837a6..1d3ba14, all 19 changed files and reached callers, with an independent source reviewer. The delta incorporates current parent 217b42e; backup production itself is unchanged. Live head remained 1d3ba14 at the final discussion recheck.
Debug controls and deciding output
Not pushed. Shell-only controls stage disposable credentials, invoke real repositories and compare real SDK/VSS/DataStore state. The outage-only override selects a VSS forwarding relay; it does not change retry, admission, capture or acknowledgement decisions. The relay forwarded encrypted requests to staging and closed connections during the outage.
terminal=true; ordinary watch-only index=1 reserved; failed=true; remote unchanged=true; pending=true; error=AppError
Backup failed for: 'WALLET' [AppError='code=identity_error, context=restore Pubky grant session from platform provider']
new address success=true; error=null
recovered original identity and complete WALLET upload
backup synchronized; sdk=true
Shutdown: event handling timed out after 30 seconds; Shutdown complete; Node stopped
Same app process remained alive more than 75 seconds after Node stopped
Cold start: METADATA required revision retained across force-stop
Backup starting for METADATA preceded node startup and VSS client setup
Node started -> METADATA succeeded 493 ms later
METADATA running=false, synced=required
WALLET shared_state_busy -> automatic temporary retries -> WALLET succeeded
diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 00c57a69a..c57ec93a7 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -42,6 +42,11 @@ 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("qa1686")) {
+ return kotlinx.coroutines.runBlocking(kotlinx.coroutines.Dispatchers.IO) {
+ Qa1686.run(requireNotNull(context), method)
+ }
+ }
return runCatching {
val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
ServiceQueue.LDK.blocking { command.execute(deps) }
diff --git a/app/src/debug/java/to/bitkit/dev/Qa1686.kt b/app/src/debug/java/to/bitkit/dev/Qa1686.kt
new file mode 100644
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/Qa1686.kt
@@ -0,0 +1,133 @@
+package to.bitkit.dev // test hook
+
+import android.content.Context // test hook
+import android.content.ClipData // test hook
+import android.content.ClipboardManager // test hook
+import android.os.Bundle // test hook
+import dagger.hilt.EntryPoint // test hook
+import dagger.hilt.InstallIn // test hook
+import dagger.hilt.android.EntryPointAccessors // test hook
+import dagger.hilt.components.SingletonComponent // test hook
+import kotlinx.coroutines.Dispatchers // test hook
+import kotlinx.coroutines.flow.first // test hook
+import kotlinx.coroutines.withContext // test hook
+import to.bitkit.data.CacheStore // test hook
+import to.bitkit.data.backup.VssBackupClient // test hook
+import to.bitkit.data.keychain.Keychain // test hook
+import to.bitkit.models.BackupCategory // test hook
+import to.bitkit.repositories.BackupRepo // test hook
+import to.bitkit.repositories.PubkyRepo // test hook
+import to.bitkit.repositories.WalletRepo // test hook
+import to.bitkit.repositories.LightningRepo // test hook
+import to.bitkit.services.PubkyService // test hook
+import to.bitkit.di.json // test hook
+import to.bitkit.models.WalletBackupV1 // test hook
+import to.bitkit.utils.Logger // test hook
+import java.io.File // test hook
+
+object Qa1686 { // test hook
+ @EntryPoint @InstallIn(SingletonComponent::class) // test hook
+ interface Dependencies { // test hook
+ fun keychain(): Keychain // test hook
+ fun backupRepo(): BackupRepo // test hook
+ fun pubkyRepo(): PubkyRepo // test hook
+ fun pubkyService(): PubkyService // test hook
+ fun vss(): VssBackupClient // test hook
+ fun cache(): CacheStore // test hook
+ fun wallet(): WalletRepo // test hook
+ fun accounts(): to.bitkit.data.WatchOnlyAccountStore
+ fun settings(): to.bitkit.data.SettingsStore
+ fun lightning(): LightningRepo // test hook
+ } // test hook
+ suspend fun run(context: Context, method: String): Bundle { // test hook
+ val d = EntryPointAccessors.fromApplication(context, Dependencies::class.java) // test hook
+ val result = kotlin.runCatching { // test hook
+ when (method) { // test hook
+ "qa1686InitCore" -> { // test hook
+ to.bitkit.async.ServiceQueue.CORE.background { // test hook
+ com.synonym.bitkitcore.initDb(to.bitkit.env.Env.bitkitCoreStoragePath(0)) // test hook
+ com.synonym.bitkitcore.updateBlocktankUrl(to.bitkit.env.Env.blocktankApiUrl) // test hook
+ } // test hook
+ "initialized real native database after app storage path became available" // test hook
+ } // test hook
+ "qa1686QuickState" -> {
+ "source=${d.keychain().exists(Keychain.Key.SHARED_PUBKY_SOURCE.name)}, savedSecret=${!d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrBlank()}, savedSession=${!d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name).isNullOrBlank()}, identity=${d.pubkyRepo().publicKey.value != null}, authenticated=${d.pubkyRepo().isAuthenticated.value}, profile=${d.pubkyRepo().profile.value?.name}, contacts=${d.pubkyRepo().contacts.value.size}, pendingRestore=${d.backupRepo().hasPendingWalletRestore()}, node=${d.lightning().lightningState.value.nodeLifecycleState}"
+ }
+ "qa1686CreateProfile" -> {
+ d.pubkyRepo().createIdentity("QA restore", "", emptyList(), emptyList(), null).getOrThrow()
+ "created disposable profile"
+ }
+ "qa1686Backup" -> {
+ for (c in listOf(BackupCategory.METADATA, BackupCategory.SETTINGS, BackupCategory.WALLET)) d.backupRepo().triggerBackup(c).getOrThrow()
+ val wallet = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+ val metadata = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+ val pending = d.cache().backupStatuses.first().filterKeys { it in listOf(BackupCategory.WALLET, BackupCategory.METADATA) }.any { it.value.isRequired }
+ check(!pending)
+ "backup synchronized; sdk=${json.decodeFromString<WalletBackupV1>(wallet.value.decodeToString()).paykitSdkBackupState != null}; metadataBytes=${metadata.value.size}"
+ }
+ "qa1686RequiredFault" -> {
+ d.backupRepo().stopObservingBackups()
+ val before = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+ File(context.filesDir, "qa1686-valid-wallet").writeBytes(before.value)
+ d.keychain().upsertString(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name, "invalid-json")
+ "staged malformed retained WALLET recovery payload"
+ }
+ "qa1686UsdtFault" -> {
+ d.backupRepo().stopObservingBackups()
+ val before = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+ File(context.filesDir, "qa1686-valid-wallet").writeBytes(before.value)
+ d.keychain().upsertString(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name, "invalid-json")
+ "staged malformed retained USDT recovery payload"
+ }
+ "qa1686Terminal" -> {
+ d.backupRepo().stopObservingBackups()
+ val secret = requireNotNull(d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name))
+ File(context.filesDir, "qa1686-original-secret").writeText(secret)
+ val session = requireNotNull(d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name))
+ File(context.filesDir, "qa1686-original-session").writeText(session)
+ d.pubkyService().forgetSessionAccess()
+ d.keychain().delete(Keychain.Key.PUBKY_SECRET_KEY.name)
+ d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, "qa1686-invalid-session")
+ d.pubkyRepo().initialize().getOrThrow()
+ check(d.pubkyRepo().publicKey.value == null)
+ val before = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+ val reserved = d.accounts().reserveAccountIndex(0, "qa1686-terminal-ordinary")
+ check(d.accounts().backupSnapshot().allocationState.highestAccountIndexByWallet["0"] == reserved)
+ val failed = d.backupRepo().triggerBackup(BackupCategory.WALLET)
+ val after = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+ val same = before.value.contentEquals(after.value) && before.version == after.version
+ val pending = d.cache().backupStatuses.first()[BackupCategory.WALLET]?.isRequired == true
+ check(failed.isFailure && same && pending)
+ "terminal=true; ordinary watch-only index=$reserved reserved; failed=true; remote unchanged=true; pending=true; error=${failed.exceptionOrNull()?.javaClass?.simpleName}"
+ }
+ "qa1686RepairIdentity" -> {
+ d.keychain().upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, File(context.filesDir, "qa1686-original-secret").readText())
+ d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, File(context.filesDir, "qa1686-original-session").readText())
+ d.pubkyRepo().initialize().getOrThrow()
+ d.backupRepo().triggerBackup(BackupCategory.WALLET).getOrThrow()
+ "recovered original identity and complete WALLET upload"
+ }
+ "qa1686RepairRequired" -> { // test hook
+ d.keychain().upsertString(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name, File(context.filesDir, "qa1686-valid-wallet").readText()) // test hook
+ if (d.keychain().exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)) { // test hook
+ d.keychain().upsertString(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name, File(context.filesDir, "qa1686-valid-wallet").readText()) // test hook
+ } // test hook
+ "repaired original required backup" // test hook
+ } // test hook
+ "qa1686SpendingGate" -> { // test hook
+ check(d.backupRepo().hasPendingWalletRestore()) // test hook
+ val result = d.lightning().payInvoice("qa1686-invalid-invoice") // test hook
+ check(result.exceptionOrNull()?.javaClass?.simpleName == "WalletBackupRestorePendingError") // test hook
+ "payment admission blocked by required restore before invoice parsing" // test hook
+ } // test hook
+ "qa1686AddressGate" -> { // test hook
+ val result = d.wallet().newAddress() // test hook
+ "new address success=${result.isSuccess}; error=${result.exceptionOrNull()?.javaClass?.simpleName}" // test hook
+ } // test hook
+ else -> "source=${d.keychain().exists(Keychain.Key.SHARED_PUBKY_SOURCE.name)}, savedSecret=${!d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrBlank()}, savedSession=${!d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name).isNullOrBlank()}, sdk=${d.pubkyService().currentPublicKey() != null}, identity=${d.pubkyRepo().publicKey.value != null}, profile=${d.pubkyRepo().profile.value?.name}, contacts=${d.pubkyRepo().contacts.value.size}, pendingRestore=${d.backupRepo().hasPendingWalletRestore()}, node=${d.lightning().lightningState.value.nodeLifecycleState}" // test hook
+ } // test hook
+ }.fold({ it }, { "ERROR: ${it.javaClass.simpleName}: ${it.message}" }) // test hook
+ Logger.info("QA1686 $method: $result", context="TestHook") // test hook
+ return Bundle().apply { putString("result", result) } // test hook
+ } // test hook
+} // test hook
diff --git a/app/src/main/java/to/bitkit/env/Env.kt b/app/src/main/java/to/bitkit/env/Env.kt
--- a/app/src/main/java/to/bitkit/env/Env.kt
+++ b/app/src/main/java/to/bitkit/env/Env.kt
@@ -102,6 +102,6 @@
val vssServerUrl
- get() = backupTestEndpoints?.vss ?: when (network) {
+ get() = if (BuildConfig.DEBUG) "http://127.0.0.1:24889" else backupTestEndpoints?.vss ?: when (network) { // test hook
Network.BITCOIN -> "https://bitkit.to/vss_rs_auth"
else -> "https://bitkit.stag0.blocktank.to/vss_rs_auth"
}Reviewed by gpt-6.1-sol-high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-approving at 9cfe052. No new findings.
9a837a6e7 puts recovery callbacks ahead of the backup debounce, so the USDT callback no longer waits up to 5 s behind it. The merge resolutions are clean. My earlier LOW thread is resolved: the exact-revision gates fixed it.
No device check needed for this delta. The behaviour is covered by the PR's unit tests, and nothing user-visible changes.
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — the existing terminal-identity WALLET backup failure still reproduces at 9cfe052.
Findings: No new delta findings. Existing HIGH: app/src/main/java/to/bitkit/repositories/BackupRepo.kt:994 still cannot capture newer ordinary wallet state when saved identity restoration fails terminally. The real SDK returns identity_error; the latest ordinary allocation remains pending while the prior remote WALLET stays unchanged. This adds evidence to the current-parent report, rather than a duplicate inline finding.
QA for 9cfe052: Android 15 / API 35, arm64, disposable unfunded regtest wallet with encrypted staging VSS. I built the unmodified head and Debug fault builds. Local controls invoke real repositories, hold write/cleanup boundaries and display assertions. Native libraries match the pristine APK; the final installed APK matches its build. Revisions, remote bytes and timing were checked in app logs, DataStore and Debug output. Videos play at 2×.
Ordinary wallet backup after terminal identity failure ❌ Debug and remote-backup comparisons confirmed a new ordinary allocation remained pending.
An invalid retained session with no recovery secret reached terminal identity failure. A new ordinary watch-only allocation remained pending because WALLET capture returned SDK identity_error. Remote WALLET bytes/version were unchanged. Restoring the original disposable credentials allowed a complete WALLET upload again.
terminal-backup2x.2x.mp4
Terminal failure and credential recovery; backup assertions checked through Debug output and app logs.
Wallet success automatically resumes blocked metadata ✅ Debug and upload logs confirmed one automatic METADATA write after WALLET persisted the exclusion.
With an actual ordinary address quarantined and WALLET pending, a METADATA request failed without replacing its remote bytes/version. I held the observation-owned WALLET upload, then released it. WALLET persisted the excluded address, advanced the pending metadata revision and automatically resumed METADATA after its debounce. Exactly one METADATA PUT occurred, with no manual metadata retry; decrypted WALLET readback contained the exclusion and both categories synchronized.
metadata-rearm2x.2x.mp4
Foreground observation, held WALLET and automatic METADATA completion; assertions checked in Debug output and logs.
Wallet and metadata writes serialize and drain on reset ✅ Logs confirmed the shared writer and both client resets waited for held cleanup.
A held WALLET upload prevented a parallel METADATA operation from reaching upload or changing its status. Reset cancelled both operations but still waited for the held cancellation cleanup after I cancelled the reset caller. Both VSS client resets occurred only after release. Cancelled WALLET remained required, neither cancelled upload synchronized, and fresh explicit WALLET/METADATA requests then completed with observation stopped.
serialized-reset2x.2x.mp4
Writer serialization, cancelled reset caller and subsequent explicit uploads; ordering checked in Debug output and logs.
Pending metadata recovers after a cold-start VSS outage ✅ DataStore and app logs confirmed the failed revision survived force-stop and a queued cold-start request uploaded after transport recovery.
The controlled forwarding relay closed only VSS connections; Electrum remained reachable. The failed METADATA required revision survived force-stop and relaunch. After the real native database was initialized with the available app storage path, a queued explicit request waited for normal node/VSS startup and completed after VSS recovered, without another metadata request or edit. A later ordinary metadata follow-up also synchronized, checked in logs/DataStore.
The bounded repeat used normal profile sign-out to isolate this scheduling case. In the earlier signed-in attempt the queued METADATA request also completed, but later WALLET attempts returned native transport_error / HTTP 405 while acquiring the Pubky write lock, leaving newer metadata gated. That later signed-in synchronization remains unverified. The first early Debug request also raced native database initialization; its failure is retained. These results do not establish VSS setup independent of node startup.
cold-start-retry2x.2x.mp4
Signed-out cold restart and recovery; persisted revisions and upload completion checked in app data/logs.
The following passes remain at their original tested head 9a837a6 from the scheduler device report. They are retained results, not fresh executions. Scheduler/admission/reset, USDT persistence and acknowledgement, VSS, DataStore, wipe and dependency pins are unchanged. The added WALLET/METADATA boundary was driven above.
Check: pending changes and due retry across restart ✅ Retained device pass at `9a837a6`.
Original case 1 and recording. Logs confirmed that replacement setup, settings and a due retry waited for cleanup, then uploaded the edit without another mutation.
Check: reset drains manual and recovery persistence ✅ Retained device pass at `9a837a6`.
Original case 2 and recording. Logs confirmed that cancelled reset callers still drained both held writes, and cancelled recovery received no acknowledgement.
Check: wipe closes explicit admission ✅ Retained device pass at `9a837a6`.
Original case 3 and recording. Logs confirmed no new upload or status change during wipe, followed by successful explicit recovery with observation stopped.
Check: full-backup follow-up and held status writes ✅ Retained device pass at `9a837a6`.
Original case 4 and recording. Logs confirmed successor uploads, a retained required-status write and a full request behind cleanup; decrypted readback contained CHF.
Check: explicit retry with observation stopped ✅ Retained device pass at `9a837a6`.
Original case 5a and recording. Logs confirmed native connection-failure retries with observation stopped; automatic success did not acknowledge the failed callback.
Check: cancelled queued recovery revision ✅ Retained device pass at `9a837a6`.
Original case 5b and recording. Log comparisons confirmed that older completion retained the newer cancelled reservation and could not acknowledge its callback.
Check: cancelled reservation after a permanent failure ✅ Retained device pass at `9a837a6`.
Original case 6 and recording. Logs confirmed that observation now uploads a newer cancelled reservation after permanent failure, without acknowledging the cancelled callback.
Check: explicit recovery callback during scheduled debounce ✅ Retained device pass at `9a837a6`.
Original case 8 and recording. App timestamps measured a 483 ms callback acknowledgement during idle debounce, with one recovery upload and no delayed duplicate.
Check: native signed operation after rejected backup ✅ Retained device pass at `9a837a6`.
Original case 9 and recording. App and fixture logs confirmed identical signed recovery bytes across rejection and native retry, with zero submissions before acknowledgement and one afterward.
Check: native signed send behind scheduled recovery ✅ Retained device pass at `9a837a6`.
Original case 10 and recording. App timestamps measured a 484 ms signed-send callback acknowledgement during idle debounce; fixture logs confirmed one new submission.
Check: explicit callback waits for active upload ✅ Retained device pass at `9a837a6`.
Original case 11 and recording. Logs confirmed that an explicit callback waits for an active older upload, then performs and acknowledges its own write.
Check: pinned build and affected regressions ✅ The unmodified head assembled and all selected suites passed.
Ran assembleDevDebug and testDevDebugUnitTest selecting backup, address reservation, public sharing/admission, receiving, allowance executor/store/wire format, shared send flow, contact settings, address usage and SDK suites. I inspected the build output and fresh JUnit reports. Full-project tests and lint were not rerun. The build retains startup-provider, native-symbol stripping, JVM and Gradle notices.
Check: retained recovery and outage evidence ✅ Source comparison establishes the limits of the retained reports.
- Current-parent restore report at 78a0ff7: required WALLET/USDT decoder gates, connection correction across restart, receiving before final activation, and ordinary backups after sign-out/unlink. The full/USDT/metadata restore implementations match this composition.
- Previous child report at 1d3ba14: one VSS-outage shutdown trial completed after a 30-second native event-handler timeout and survived the observed crash window. Native shutdown implementation and pins are unchanged. This remains one trial, not proof that the earlier native abort is fixed.
- Earlier capped-backoff report: the calculation and timer bodies still match the measured 10/20/40/80/160/300-second schedule.
No selected case remains unrun. The inherited allowance-payment/history, public-sharing publication UI and balance-loader journeys were not driven anew; they received source review and affected unit coverage. Complete funded process-death recovery and authenticated contact-request negotiation were not newly driven. The retained SETTINGS full-backup case proves scheduler follow-up, not complete WALLET capture.
Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle, inspected at 262a447, matches draining, independent explicit USDT persistence and protecting metadata behind WALLET recovery. Android shares a WALLET/METADATA mutex and rearms pending metadata after WALLET acknowledgement; iOS joins the existing writer and awaits observation shutdown. Terminal complete-WALLET availability remains unresolved on both platforms.
Coverage: contiguous 1d3ba14..9cfe052, all 82 changed paths and reached backup, restore, identity, address and allowance callers, with an independent source reviewer. The delta incorporates parent 78a0ff7. Live head remained 9cfe052 at the final discussion recheck.
Debug controls and deciding output
Not pushed. The shell-only controls stage disposable credentials and pending revisions, invoke real repositories, hold writes/cleanup and display assertions. The foreground overlay keeps the normal activity resumed. The outage build routes only encrypted VSS traffic through the forwarding relay below. Neither changes backup, SDK or acknowledgement decisions. The terminal case predates the foreground-overlay addition; the cold repeat adds only normal repository sign-out to that control surface.
terminal=true; ordinary allocation=1; WALLET failed=true; remote bytes/version unchanged=true; pending=true
original identity recovered; complete WALLET synchronized
553054 metadata blocked; remote bytes/version unchanged=true
559101 HELD put:WALLET
578019 PUT_OK WALLET
583132 PUT_ENTER METADATA
583191 PUT_OK METADATA
remote WALLET contains exclusion=true; both categories synchronized=true
607483 HELD put:WALLET
607712 BEGIN manual (METADATA)
620817 BEGIN reset
620820 HELD cleanup:WALLET
640920 RELEASE cleanup:WALLET
640932 RESET_VSS
640932 RESET_LDK
640933 CANCELLED reset
658969 PUT_OK WALLET
659115 PUT_OK METADATA
diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a..31a7c5b7 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -11,5 +11,7 @@
android:authorities="${applicationId}.devtools"
android:exported="true"
tools:ignore="ExportedContentProvider" />
+ <provider android:name="to.bitkit.dev.Qa1689Provider" android:authorities="${applicationId}.qa1689" android:exported="true" tools:ignore="ExportedContentProvider" />
+ <activity android:name="to.bitkit.dev.Qa1689Activity" android:exported="true" />
</application>
</manifest>
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 eef9af0b..bfb9400a 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -109,6 +109,7 @@ class VssBackupClient @Inject constructor(
}
fun reset() {
+ to.bitkit.dev.Qa1689.event("RESET_VSS") // test hook
synchronized(this) {
isSetup.cancel()
isSetup = CompletableDeferred()
@@ -122,6 +123,7 @@ class VssBackupClient @Inject constructor(
data: ByteArray,
): Result<VssItem> = withContext(ioDispatcher) {
isSetup.await()
+ to.bitkit.dev.Qa1689.beforePut(key) // test hook
Logger.verbose("VSS 'putObject' call for '$key'", context = TAG)
runCatching {
vssStore(
@@ -129,6 +131,7 @@ class VssBackupClient @Inject constructor(
value = data,
)
}.onSuccess {
+ to.bitkit.dev.Qa1689.event("PUT_OK $key") // test hook
Logger.verbose("VSS 'putObject' success for '$key' at version: '${it.version}'", context = TAG)
}.onFailure {
Logger.verbose("VSS 'putObject' error for '$key'", it, context = TAG)
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
index 7842dbf1..8d3e688c 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
@@ -75,6 +75,7 @@ class VssBackupClientLdk @Inject constructor(
}
fun reset() {
+ to.bitkit.dev.Qa1689.event("RESET_LDK") // test hook
synchronized(this) {
isSetup.cancel()
isSetup = CompletableDeferred()
diff --git a/app/src/main/java/to/bitkit/env/Env.kt b/app/src/main/java/to/bitkit/env/Env.kt
index 83a1ed56..f50bd609 100644
--- a/app/src/main/java/to/bitkit/env/Env.kt
+++ b/app/src/main/java/to/bitkit/env/Env.kt
@@ -101,7 +101,7 @@ internal object Env {
)
val vssServerUrl
- get() = backupTestEndpoints?.vss ?: when (network) {
+ get() = if (BuildConfig.DEBUG) "http://127.0.0.1:24889" else backupTestEndpoints?.vss ?: when (network) {
Network.BITCOIN -> "https://bitkit.to/vss_rs_auth"
else -> "https://bitkit.stag0.blocktank.to/vss_rs_auth"
}
diff --git a/app/src/main/java/to/bitkit/ui/MainActivity.kt b/app/src/main/java/to/bitkit/ui/MainActivity.kt
index cf0fe8b3..5487b425 100644
--- a/app/src/main/java/to/bitkit/ui/MainActivity.kt
+++ b/app/src/main/java/to/bitkit/ui/MainActivity.kt
@@ -346,6 +346,7 @@ class MainActivity : FragmentActivity() {
}
override fun onResume() {
+ to.bitkit.dev.Qa1689.activity = this // test hook
super.onResume()
appViewModel.onAppResumed()
// A permission dialog launched from a sheet that has since closed loses its result callback.
diff --git a/app/src/debug/java/to/bitkit/dev/Qa1689.kt b/app/src/debug/java/to/bitkit/dev/Qa1689.kt
new file mode 100644
index 0000000..3f7a5e7
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/Qa1689.kt
@@ -0,0 +1,216 @@
+package to.bitkit.dev // test hook: disposable review installation only
+
+import android.app.Activity
+import android.content.ContentProvider
+import android.content.ContentValues
+import android.graphics.Color
+import android.net.Uri
+import android.os.Binder
+import android.os.Bundle
+import android.os.Handler
+import android.os.Looper
+import android.widget.ScrollView
+import android.widget.TextView
+import dagger.hilt.EntryPoint
+import dagger.hilt.InstallIn
+import dagger.hilt.android.EntryPointAccessors
+import dagger.hilt.components.SingletonComponent
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.CacheStore
+import to.bitkit.data.WatchOnlyAccountStore
+import to.bitkit.data.backup.VssBackupClient
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.di.json
+import to.bitkit.models.BackupCategory
+import to.bitkit.models.BackupItemStatus
+import to.bitkit.models.WalletBackupV1
+import to.bitkit.repositories.*
+import to.bitkit.services.PubkyService
+import to.bitkit.utils.Logger
+import java.io.File
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.CopyOnWriteArrayList
+
+object Qa1689 {
+ var activity: Activity? = null // test hook: retained only in disposable Debug installation
+ private var overlay: TextView? = null
+ val events = CopyOnWriteArrayList<String>()
+ private val gates = ConcurrentHashMap<String, CompletableDeferred<Unit>>()
+ val tasks = ConcurrentHashMap<String, Job>()
+ private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+ fun event(value: String) {
+ val line = "${android.os.SystemClock.elapsedRealtime()} $value"
+ events += line
+ Logger.info(line, context = "QA1689")
+ overlay?.post { render() }
+ }
+ private fun render() {
+ overlay?.text = "9cfe052 + local Debug controls\nMain activity remains foreground\nReal DataStore, SDK and encrypted VSS\nDisposable unfunded wallet\n\n" + events.takeLast(27).joinToString("\n")
+ }
+ fun showOverlay() {
+ Handler(Looper.getMainLooper()).post {
+ val current = requireNotNull(activity)
+ val text = TextView(current).apply { setTextColor(Color.WHITE); setBackgroundColor(Color.BLACK); textSize = 12f; setPadding(16, 85, 16, 16) }
+ val scroll = ScrollView(current).apply { setBackgroundColor(Color.BLACK); addView(text) }
+ current.addContentView(scroll, android.view.ViewGroup.LayoutParams(-1, -1))
+ overlay = text
+ render()
+ }
+ }
+ fun arm(value: String) { gates[value] = CompletableDeferred(); event("ARM $value") }
+ fun release(value: String) { gates.remove(value)?.complete(Unit); event("RELEASE $value") }
+ suspend fun hold(value: String, drain: Boolean = false) {
+ val gate = gates[value] ?: return
+ event("HELD $value")
+ if (drain) withContext(NonCancellable) { gate.await() } else gate.await()
+ gates.remove(value, gate)
+ event("EXIT $value")
+ }
+ suspend fun beforePut(key: String) {
+ event("PUT_ENTER $key")
+ try { hold("put:$key") } finally { hold("cleanup:$key", true) }
+ }
+ fun launch(name: String, block: suspend () -> String) {
+ tasks[name] = scope.launch {
+ event("BEGIN $name")
+ try { event("DONE $name: ${withTimeout(180_000) { block() }}") }
+ catch (e: CancellationException) { event("CANCELLED $name"); throw e }
+ catch (e: Throwable) { event("ERROR $name: ${e.javaClass.simpleName}: ${e.message}") }
+ }
+ }
+}
+
+@EntryPoint @InstallIn(SingletonComponent::class)
+interface Qa1689Dependencies {
+ fun backup(): BackupRepo
+ fun cache(): CacheStore
+ fun keychain(): Keychain
+ fun vss(): VssBackupClient
+ fun pubky(): PubkyRepo
+ fun pubkyService(): PubkyService
+ fun wallet(): WalletRepo
+ fun accounts(): WatchOnlyAccountStore
+ fun reservations(): PrivatePaykitAddressReservationRepo
+}
+
+class Qa1689Provider : ContentProvider() {
+ private val d by lazy { EntryPointAccessors.fromApplication(requireNotNull(context), Qa1689Dependencies::class.java) }
+ override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
+ check(Binder.getCallingUid() == 2000)
+ val answer = when (method) {
+ "arm" -> { Qa1689.arm(requireNotNull(arg)); "ok" }
+ "release" -> { Qa1689.release(requireNotNull(arg)); "ok" }
+ "clear" -> { Qa1689.events.clear(); "ok" }
+ "start" -> { d.backup().startObservingBackups(); "started observation" }
+ "stop" -> { d.backup().stopObservingBackups(); "stopped observation" }
+ "cancel" -> { Qa1689.tasks[arg]?.cancel(); "cancelled" }
+ "state" -> runBlocking(Dispatchers.IO) {
+ val status = d.cache().backupStatuses.first().filterKeys { it in listOf(BackupCategory.WALLET, BackupCategory.METADATA) }
+ val value = "statuses=$status identity=${d.pubky().publicKey.value != null} pendingRestore=${d.backup().hasPendingWalletRestore()}"
+ Qa1689.event(value); value
+ }
+ "events" -> Qa1689.events.joinToString("\n")
+ "overlay" -> { Qa1689.showOverlay(); "debug display on current activity" }
+ else -> {
+ Qa1689.launch(method) {
+ when (method) {
+ "initCore" -> {
+ to.bitkit.async.ServiceQueue.CORE.background {
+ com.synonym.bitkitcore.initDb(to.bitkit.env.Env.bitkitCoreStoragePath(0))
+ com.synonym.bitkitcore.updateBlocktankUrl(to.bitkit.env.Env.blocktankApiUrl)
+ }; "real native database initialized"
+ }
+ "profile" -> { d.pubky().createIdentity("QA backup", "", emptyList(), emptyList(), null).getOrThrow(); "disposable profile created" }
+ "signOut" -> { d.pubky().signOut().getOrThrow(); "disposable profile signed out through normal repository" }
+ "backup" -> {
+ for (c in listOf(BackupCategory.WALLET, BackupCategory.METADATA, BackupCategory.SETTINGS)) d.backup().triggerBackup(c).getOrThrow()
+ "complete backup synchronized"
+ }
+ "prepare" -> {
+ d.backup().stopObservingBackups(); d.backup().reset(); d.vss().setup().getOrThrow()
+ d.cache().update { it.copy(backupStatuses = BackupCategory.entries.associateWith { BackupItemStatus(required = 1000, synced = 1000) }) }
+ "stopped observation and synchronized fixture statuses"
+ }
+ "quarantine" -> {
+ val address = d.wallet().newAddress().getOrThrow()
+ File(context!!.filesDir, "qa1689-excluded-address").writeText(address)
+ d.reservations().quarantinePublicAddress(address)
+ d.cache().update { it.copy(onchainAddress = "", bip21 = "", backupStatuses = it.backupStatuses + mapOf(BackupCategory.WALLET to BackupItemStatus(required = 2000, synced = 1000), BackupCategory.METADATA to BackupItemStatus(required = 2000, synced = 1000))) }
+ "real ordinary address quarantined; WALLET and METADATA pending"
+ }
+ "manual" -> { d.backup().triggerBackup(BackupCategory.valueOf(requireNotNull(arg))).getOrThrow(); "uploaded $arg" }
+ "expectMetadataBlocked" -> {
+ val before = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+ val result = d.backup().triggerBackup(BackupCategory.METADATA)
+ val after = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+ check(result.isFailure && before.version == after.version && before.value.contentEquals(after.value))
+ "metadata blocked; remote bytes/version unchanged=true"
+ }
+ "verifyExclusion" -> {
+ val excluded = File(context!!.filesDir, "qa1689-excluded-address").readText()
+ val bytes = requireNotNull(d.vss().getObject("WALLET").getOrThrow()).value
+ check(excluded in json.decodeFromString<WalletBackupV1>(bytes.decodeToString()).publicPaykitLegacyAddresses.orEmpty())
+ val statuses = d.cache().backupStatuses.first()
+ check(listOf(BackupCategory.WALLET, BackupCategory.METADATA).none { statuses[it]?.isRequired == true })
+ "remote WALLET contains exclusion=true; both categories synchronized=true"
+ }
+ "reset" -> { d.backup().reset(); "both clients reset after drain" }
+ "terminal" -> {
+ d.backup().stopObservingBackups()
+ val secret = requireNotNull(d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name))
+ val session = requireNotNull(d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name))
+ File(context!!.filesDir, "qa1689-secret").writeText(secret)
+ File(context!!.filesDir, "qa1689-session").writeText(session)
+ d.pubkyService().forgetSessionAccess()
+ d.keychain().delete(Keychain.Key.PUBKY_SECRET_KEY.name)
+ d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, "qa1689-invalid-session")
+ d.pubky().initialize().getOrThrow()
+ check(d.pubky().publicKey.value == null)
+ val before = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+ val reserved = d.accounts().reserveAccountIndex(0, "qa1689-terminal-ordinary")
+ val result = d.backup().triggerBackup(BackupCategory.WALLET)
+ val after = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+ check(result.isFailure && before.version == after.version && before.value.contentEquals(after.value))
+ check(d.cache().backupStatuses.first()[BackupCategory.WALLET]?.isRequired == true)
+ "terminal=true; ordinary allocation=$reserved; WALLET failed=true; remote bytes/version unchanged=true; pending=true; error=${result.exceptionOrNull()?.javaClass?.simpleName}"
+ }
+ "repair" -> {
+ d.keychain().upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, File(context!!.filesDir, "qa1689-secret").readText())
+ d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, File(context!!.filesDir, "qa1689-session").readText())
+ d.pubky().initialize().getOrThrow()
+ d.backup().triggerBackup(BackupCategory.WALLET).getOrThrow()
+ "original identity recovered; complete WALLET synchronized"
+ }
+ else -> error("Unknown QA method")
+ }
+ }; "started $method"
+ }
+ }
+ return Bundle().apply { putString("result", answer) }
+ }
+ override fun onCreate() = true
+ override fun getType(uri: Uri): String? = null
+ override fun insert(uri: Uri, values: ContentValues?): Uri? = null
+ override fun delete(uri: Uri, selection: String?, args: Array<String>?) = 0
+ override fun update(uri: Uri, values: ContentValues?, selection: String?, args: Array<String>?) = 0
+ override fun query(uri: Uri, projection: Array<String>?, selection: String?, args: Array<String>?, sort: String?) = null
+}
+
+class Qa1689Activity : Activity() {
+ private val handler = Handler(Looper.getMainLooper())
+ private lateinit var text: TextView
+ private val refresh = object : Runnable {
+ override fun run() {
+ val output = "9cfe052 + local Debug controls\nReal DataStore, SDK and encrypted VSS\nDisposable unfunded wallet\n\n" + Qa1689.events.takeLast(23).joinToString("\n")
+ if (text.text.toString() != output) text.text = output
+ handler.postDelayed(this, 250)
+ }
+ }
+ override fun onCreate(saved: Bundle?) {
+ super.onCreate(saved)
+ text = TextView(this).apply { setTextColor(Color.WHITE); setBackgroundColor(Color.BLACK); textSize = 12f; setPadding(16, 36, 16, 16) }
+ setContentView(ScrollView(this).apply { addView(text) }); handler.post(refresh)
+ }
+ override fun onDestroy() { handler.removeCallbacks(refresh); super.onDestroy() }
+}
VSS forwarding/fault control:
from http.server import ThreadingHTTPServer,BaseHTTPRequestHandler
from urllib.request import Request,urlopen
from urllib.error import HTTPError
from pathlib import Path
import threading,time
lock=threading.Lock()
state={'fail':0,'drop':False,'permanent':False,'outage':False}
class Handler(BaseHTTPRequestHandler):
def log_message(self,*args): pass
def do_GET(self): self.relay()
def do_POST(self): self.relay()
def relay(self):
body=self.rfile.read(int(self.headers.get('Content-Length','0')))
if self.path=='/control/outage':
with lock: state['outage']=True
self.send_response(200); self.end_headers(); self.wfile.write(b'outage armed'); return
if self.path=='/control/fail':
with lock: state['fail']=1000; state['drop']=False
self.send_response(200); self.end_headers(); self.wfile.write(b'armed'); return
if self.path=='/control/drop':
with lock: state['drop']=True; state['fail']=0
self.send_response(200); self.end_headers(); self.wfile.write(b'drop armed'); return
if self.path=='/control/permanent':
with lock: state['permanent']=True; state['drop']=False; state['fail']=0
self.send_response(200); self.end_headers(); self.wfile.write(b'permanent armed'); return
if self.path=='/control/healthy':
with lock: state['fail']=0; state['drop']=False; state['permanent']=False; state['outage']=False
self.send_response(200); self.end_headers(); self.wfile.write(b'healthy'); return
if state['outage']:
self.close_connection=True; print(time.time(),'FAULT_OUTAGE',self.command,self.path,flush=True); return
with lock:
fail=state['fail']>0 and self.command=='POST' and 'put' in self.path.lower()
if fail: state['fail']-=1
if state['permanent'] and self.command=='POST' and 'put' in self.path.lower():
self.send_response(400); self.send_header('Content-Type','application/octet-stream'); self.end_headers(); self.wfile.write(b'\x08\x02\x12\x17controlled invalid data');print(time.time(),'FAULT_PERMANENT',self.command,self.path,flush=True); return
if state['drop'] and self.command=='POST' and 'put' in self.path.lower():
self.close_connection=True; print(time.time(),'FAULT_DROP',self.command,self.path,flush=True); return
if fail:
self.send_response(409); self.send_header('Content-Type','application/octet-stream'); self.end_headers(); self.wfile.write(b'\x08\x01\x12\x13controlled conflict');print(time.time(),'FAULT_409',self.command,self.path,flush=True); return
headers={k:v for k,v in self.headers.items() if k.lower() not in ('host','connection','accept-encoding')}
req=Request('https://bitkit.stag0.blocktank.to/vss_rs_auth'+self.path,data=body if self.command=='POST' else None,headers=headers,method=self.command)
try: result=urlopen(req,timeout=90)
except HTTPError as e: result=e
except Exception as e:
self.send_response(502);self.end_headers();self.wfile.write(type(e).__name__.encode());print('PROXY_ERROR',type(e).__name__,flush=True);return
data=result.read();self.send_response(result.status)
for k,v in result.headers.items():
if k.lower() not in ('connection','transfer-encoding','content-length','content-encoding'): self.send_header(k,v)
self.send_header('Content-Length',str(len(data))); self.end_headers(); self.wfile.write(data)
print(time.time(),self.command,self.path.split('?')[0],result.status,flush=True)
server=ThreadingHTTPServer(('127.0.0.1',24889),Handler)
threading.Timer(3600,server.shutdown).start()
server.serve_forever()Reviewed by gpt-6.1-sol-high
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — the existing complete-WALLET failure still reproduces; the imported parent also contains an allowance-switch race.
Findings:
- Existing HIGH — terminal identity failure blocks ordinary WALLET backups,
app/src/main/java/to/bitkit/repositories/BackupRepo.kt:994. With an invalid retained session and no recovery secret, a new ordinary watch-only allocation remained pending. The real SDK returnedidentity_errorduring capture, and remote WALLET bytes/version stayed unchanged. Restoring the original credentials allowed complete upload. This corroborates the acknowledged terminal-identity finding. - MEDIUM — stale contact synchronization reactivates disabled allowances,
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:994. An enabled synchronization can suspend inpaykitPaymentRequestRepo.activate(). Turning the switch off lets the independent observer callpause(), but releasing the old synchronization still callspaykitAllowanceRepo.activate()from its captured flag. That immediately reopens executor admission before its awaited recovery/refresh finishes; the live flag is checked only afterward. An automatic request already resolving can pass admission during that interval despite the switch remaining off. The executed component regression observedpause → activate-disabled → pause → pause. Invalidate stale synchronization or enforce the current flag at activation/admission. This code arrived through the parent composition and is unchanged against the current PR base. No actual payment was driven for this finding.
QA for a3693b0: Android 15 / API 35, arm64, disposable unfunded regtest wallet, real SDK and encrypted staging VSS. I built the unmodified head and a Debug fault build. The installed APK matched its build, and all native-library bytes matched the pristine APK. Local controls stage disposable credentials, invoke real repositories and display assertions; backup and acknowledgement decisions are unchanged. Nonvisual assertions were checked in app logs, DataStore and remote-backup comparisons. Videos play at 2×.
Ordinary wallet backup after terminal identity failure ❌ A new ordinary allocation stayed pending; remote-state comparisons were checked through Debug output.
Invalid retained session without its recovery secret reached terminal failure. Ordinary account index 1 was reserved, but WALLET capture returned SDK identity_error. The previous remote bytes/version remained unchanged and the required revision stayed pending. Restoring the original disposable credentials recovered complete WALLET upload.
terminal2x.2x.mp4
Terminal failure and credential recovery; allocation, remote bytes/version and pending status were checked through Debug assertions and app logs.
Signed-in backups recover after a cold-start VSS outage ✅ DataStore and app logs confirmed retained work and later complete uploads.
A native request-send failure left METADATA pending. Its required/synced values survived force-stop and relaunch while only VSS was unavailable. A queued explicit METADATA request waited for normal node/VSS startup, then completed after transport recovered without another request or edit. The saved identity remained active. Later complete WALLET, METADATA and SETTINGS uploads succeeded; final WALLET/METADATA status had running=false and synced=required, checked in app logs and DataStore. No sign-out or credential replacement was used for the successful repeat.
cold-short2x.2x.mp4
Cold restart and recovery; persisted revisions, identity and upload acknowledgements were checked separately. Short capture gaps remain after a shorter repeat; the 2× copy preserves every changed frame.
An initial attempt stopped after the Debug overlay ran before activity resumption and crashed the test control. The corrected repeat waits for readiness. The passing repeat does not explain the older report's Pubky lock/HTTP 405 errors or prove setup independent of node startup.
Check: contact synchronization respects the disabled allowance switch ❌ A deterministic component regression observed activation while the switch was false.
The local AppViewModelSendFlowTest regression held payment-request identity activation, switched the allowance flow to false, observed the independent pause, then released the older enabled synchronization. Its assertion failed with pause, activate-disabled, pause, pause. The production view model is real; the repositories and setting flow are mocked. This establishes the stale activation ordering, with the executor admission consequence traced in source.
Check: pristine build and affected regressions ✅ The pinned head assembled and the selected suites passed.
Ran assembleDevDebug and testDevDebugUnitTest selecting the backup, allowance repository/executor, SDK, settings-store and shared send-flow suites. I inspected the full output and fresh JUnit XML. Full-project tests, lint and Compose instrumentation were not rerun. Build output retains startup-provider, native-symbol stripping, JVM-sharing and Gradle-deprecation notices. The added stale-switch regression above ran separately and failed.
Authored manual cases 1–5 retain the scheduler device report at 9a837a6: restart/drain, reset, wipe admission, full-backup follow-up, stopped explicit retry, queued cancellation and callback acknowledgement. The scheduler, VSS/DataStore, wipe, USDT persistence and native pins remain unchanged. Automatic METADATA rearming and shared WALLET/METADATA reset draining retain the 9cfe052 device report. These are retained executions at their original heads. The unchanged retry calculation retains the 49e9a99 capped-backoff measurement. The 1d3ba14 outage-shutdown trial completed with a native shutdown timeout and process survival; it does not explain or establish a fix for the earlier native abort. Shutdown implementation and pins remain unchanged.
The imported manual/automatic same-identity acceptance and disabled queued-offer boundaries retain the 78269d8 parent device report; their production paths match this composition. The stale enabled synchronization above is a separate ordering case. The ordinary switch journey retains the paired allowance report. Recipient navigation retains the complete two-identity report at 569137b93, and preparing colors retain the rail-color report; those UI implementations match this head. The artwork and copy retain their original feature report and preview evidence rather than a new device sweep.
Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle, inspected at 2c0c100, matches pending-work retention, drain ordering and independent explicit USDT persistence. Android shares a WALLET/METADATA mutex and rearms metadata after WALLET acknowledgement; iOS joins its wallet writer and awaits observation shutdown. The terminal complete-WALLET limitation remains unresolved on both platforms.
No selected case remains unrun.
Coverage: contiguous 9cfe052..a3693b0, all 44 changed paths and reached backup, allowance, SDK, identity and navigation callers, with an independent source reviewer. The backup implementation itself is unchanged. Imported UI changes received source review and retained evidence; no new funded payment, authenticated contact-request negotiation or process-death payment recovery was driven.
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: Corrected retained full reassessment of synonymdev/bitkit-android#1689 at a3693b0c against merge base 78269d8a. The original complete-diff coverage is retained. This correction rechecks finding provenance, affected callers and the current review discussions.
No new actionable code findings.
No additional introduced defect is established by the inherited allowance-toggle race. The existing complete-WALLET and cold-start Retry concerns remain linked below. The three resolved inline threads—settings edits during restart, stale USDT failure markers, and explicit-persist debounce—were rechecked against the current listener ordering, exact-revision failure gates and debounce-before-lock implementation; their reported fixes remain applicable.
The native shutdown failure remains separate. Validation here was pinned source and current discussion inspection. Earlier device evidence retains its original tested revision; no app build, app test suite, native shutdown experiment or device session was executed for this correction. Existing unresolved concerns are not cleared by the absence of a new PR finding.
Existing concerns
- [HIGH] Terminal identity failure blocks newer ordinary WALLET backups — The existing complete-WALLET concern remains: retained identity state still requires a usable Paykit snapshot, and capture fails when that state cannot be produced. The newer ordinary wallet allocation then remains pending. Prior runtime evidence remains attributed to its recorded revision; this correction inspected the current source only.
- [MEDIUM] Cold-start explicit backup Retry waits for node-dependent setup — The existing setup dependency remains: an explicit write awaits VSS setup, whose ordinary observation setup follows node startup. Queued Retry succeeding after transport and node recovery does not establish setup independent of the node. This is an existing limitation, not an additional regression established in this diff.
- [MEDIUM] Stale contact synchronization reactivates disabled allowances — The already reported stale contact-sync activation ordering is present in the comparison base and remains outside this PR’s new-defect count. The component regression establishes activation ordering; no actual payment was driven by that check. The unresolved concern is retained here, rather than presented as newly introduced by the backup lifecycle change.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — the existing terminal complete-WALLET availability failure still reproduces at 439b5d0. The allowance activation repair holds.
Findings: No new findings in this delta. The existing HIGH remains open: with an unusable saved session and no recovery secret, a new ordinary watch-only reservation stays pending because complete WALLET capture fails. The previous remote WALLET bytes/version remain unchanged. This is the acknowledged parent limitation, with fresh evidence below, rather than a new finding on an unchanged line. No partial or stale Paykit snapshot fallback was tested.
QA for 439b5d0: Android 15 / API 35, disposable unfunded regtest wallet, real Paykit SDK, DataStore and encrypted staging VSS. I built the unmodified head, then an unpushed Debug fixture that stages disposable credentials, calls the real repositories and displays assertions. The installed fixture matched its build; all native libraries matched the unmodified APK. The video plays at 2×.
Ordinary wallet backup after terminal identity failure ❌ A new ordinary reservation stayed pending; remote-state comparisons were checked through Debug assertions.
Rechecked the existing terminal identity failure. The real no-secret restoration path rejected an invalid retained session. Reserving ordinary account index 1 then left WALLET pending: capture returned the SDK identity_error before the completeness guard, and actual remote bytes/version stayed unchanged. Restoring the original disposable credentials allowed complete WALLET upload and cleared pending work. Credential, allocation, remote-state and required/synced assertions were checked through Debug output and app logs.
terminal2x.2x.mp4
Debug display during terminal failure and original-credential recovery; backup decisions and native calls are unchanged.
Check: backup and identity activation regressions ✅ The unmodified head built and the selected regression suites passed.
Ran assembleDevDebug and testDevDebugUnitTest for BackupRepoTest, PaykitAllowanceRepoTest, PaykitAllowanceExecutorTest, PubkyRepoTest, PaykitSdkServiceTest, AppViewModelSendFlowTest and PubkyAuthManifestTest. Fresh JUnit XML confirms no failures, errors or skips, including stale activation, off/on recovery, pending profile setup and rejected SDK admission. The full-project suite, lint and Compose instrumentation were not rerun. Build output retains the startup-provider, native-symbol stripping, JVM-sharing and Gradle-deprecation notices. A local routing pass does not establish the cause of the earlier hosted routing failure.
Retained device passes apply at their recorded heads. I compared their exercised production paths and dependency pins with this composition:
- Parent allowance report at b6751e2: delayed activation stays revoked while off; off/on and delayed contact synchronization restore fresh activation and the Active row without restart; disabled requests remain manual; same-identity acceptance survives refresh; old queued offers stay revoked and only a fresh attempt accepts. That report also confirms one real automatic Lightning payment after re-enable. The relevant allowance, view-model, SDK and executor files match this child.
- Parent reconnect report at 1b1426f: reconnect skips profile reads during pending setup, later recovers without blocking ordinary maintenance, rejects a late response after sign-out, and rejected allowance admission preserves the cached backup/version. These exercised paths match this child; unrelated profile-deletion wrappers differ.
- Scheduler report at 9a837a6: authored manual cases 1–5 retain their passing restart/drain, reset, wipe-admission, full-backup follow-up, stopped explicit retry, queued cancellation and callback-acknowledgement executions. The repaired failure-marker and debounce cases remain covered.
- Shared writer report at 9cfe052: automatic METADATA rearming after WALLET acknowledgement and shared WALLET/METADATA reset draining.
- Signed-in cold-start report at a3693b0: pending work survives a VSS outage/restart and later completes with the saved identity retained. This does not prove node-independent backup setup or explain the historical Pubky lock/HTTP 405 errors.
- Backoff report at 49e9a99: the unchanged capped retry schedule.
The backup scheduler, serialization, VSS/DataStore, wipe, USDT persistence and native pins are untouched by this delta. No selected case remains unrun. Funded payment replay across process death, nonempty USDT recovery and actual Ring application UI were not driven anew; the retained reports keep their original limits.
Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle, inspected at ff3ba67, matches pending-work retention, observation draining and independent explicit USDT persistence. Android shares WALLET/METADATA serialization and rearms metadata after WALLET acknowledgement; iOS joins its wallet writer during shutdown. Source comparison only; the terminal complete-WALLET limitation remains open on both platforms.
Review covered the contiguous a3693b0..439b5d0 delta, all 11 changed paths and reached callers, plus the previously open concerns, with an independent source reviewer. The head remained current at final recheck. Matching parent device results cover the changed activation/reconnect paths; the new manifest-test change adds failure diagnostics only.
Reviewed by gpt-6.1-sol-high
|
I reconciled the retained 9a837a6 scheduler report with 439b5d0: most lifecycle/retry paths and the native dependency pins match, while the later shared WALLET/METADATA writer retains its separate evidence. To credit the existing Manual 1–5 executions accurately, please attach the already-produced nonsecret build/install receipt (tested revision, complete control diff, installed APK SHA-256) and the original event/readback traces for Tests 1–5b, including required/synced revisions and callback acknowledgements. Test 2 describes an empty-wallet envelope: please clarify whether the same-signed-operation reset/cancellation variant was also executed, with its existing receipt if so. No new run or fixture access is requested. Please exclude credentials, tokens, seeds and private wallet data. I am preserving the reported historical passes and keeping terminal complete-WALLET availability separate. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: full reassessment of the complete PR at 439b5d0, against merge base b6751e2, because the comparison base changed and no reusable baseline was supplied. Coverage includes backup lifecycle/retry ownership and profile deletion. iOS ff3ba67 received targeted contract comparison only.
Code review completed with unverified earlier concerns.
Validation: inspected hosted CI logs, which record 372 passing cases across the three affected suites. The tested merge commit has exactly the pinned head’s tree and supplied base/head parents. Local focused tests could not execute because the Android SDK is unavailable.
Device testing: not performed in this review. The authored lifecycle/outage cases and uncertain-deletion journey require dedicated fault controls; unit coverage does not establish those app-level outcomes.
Earlier findings reconciled at this revision: 2 still present, 6 verified fixed, 1 could not be verified, 1 tracked separately outside this PR.
Tracked separately (outside this PR; unresolved): Native LDK shutdown abort during VSS outage. The attributed VSS-outage native shutdown abort remains tracked by open same-repository issue #1536. This PR does not change the native dependency, node shutdown implementation or its lifecycle callers. This disposition retains an unresolved external issue; it is neither a fix nor a current-head runtime pass. This is not a runtime verification or merge-readiness declaration.
Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.
Could not verify earlier concern — Signed-in outage recovery encountered Pubky lock/HTTP 405 errors: The review reports later signed-in WALLET lock/HTTP 405 failures at 9cfe052 and metadata remaining gated. A later signed-in repeat succeeded at a3693b0, but neither report establishes the earlier cause or a causal fix. Current source still propagates SDK snapshot failures and protects metadata behind pending exclusions. The original native/server trace and a current reproduction are unavailable in this code-only reconciliation, so current recurrence and resolution remain unverified.
Existing concerns
- [HIGH] Terminal identity failure blocks newer ordinary WALLET backups — still unresolved; already tracked.
- [MEDIUM] Cold-start explicit backup Retry waits for node-dependent setup — still unresolved; already tracked.
Reviewer model: gpt-6-astra
Reasoning effort: high
|
The cold-start Retry finding in #1689 (comment) is fixed in 8727da0. Ordinary backup requests initialize VSS even before the node starts; setup-owned timeout retry remains bounded and caller/reset cancellation still drains. Cached capture errors refuse upload instead of substituting incomplete data. Final required format/compile/test/lint passed (4,913 tests, zero static findings), with two independent affected reviews and all 23 live-parent commits locally/GitHub verified. Actual-app Manual 6 passed on the matching released-dependency APK: pending SETTINGS survived cold launch, explicit Retry returned the injected PUT-auth failure, restoring authentication while keeping node reads blocked allowed one acknowledged upload, and another cold restart retained the synchronized settings. Setup succeeded in that app case; setup-timeout/read-failure assertions are separate component coverage. New-head hosted CI is pending inspection. The body now records exact setup and evidence limits. This addresses the cold-start ordinary Retry finding only; complete-WALLET availability and unverified portions of historical Manual 1–5 remain open. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR at 8727da02549c3a885fbb48b500747b3401d3d649 after the comparison base changed from b6751e2 to 43e0f3fe04705d397a81f493cf8e57239a0be73e. No coverage baseline was supplied, so this pass covered the whole diff from that merge base, which is also synonymdev/bitkit-android#1686.
Code review completed with unverified earlier concerns.
Observation stop, reset, and wipe keep a pending revision from being acknowledged or dropped: reset joins retained work before replacing VSS clients, explicit retries survive stopped Lightning observation, and a canceled USDT callback does not increment the Core acknowledgement. Cold explicit backups for ordinary categories initialize VSS before upload, keep a setup timeout on the bounded retry path, and refuse failed Blocktank or activity reads instead of uploading empty envelopes. Profile deletion makes one attempt; it no longer refreshes the session and repeats contact cleanup and deletion.
Compared with synonymdev/bitkit-ios#1123 at ff3ba674464c796057415188d460e6009371496a: the narrow VSS request-send classifier matches. Android cancels observation immediately and the next start waits for that cleanup; iOS awaits its shutdown task. iOS still retries profile deletion after a session refresh.
Validation: source and the assertions in BackupRepoTest.kt, PubkyRepoTest.kt, and EditProfileViewModelTest.kt were inspected at this revision. Those tests were not executed here. Device testing was not performed. Historical validation claims in the PR description were not treated as execution by this review.
Suggested additional test cases
- Android, disposable wallet that already has a remote BLOCKTANK or ACTIVITY object, Lightning never started. Tap Retry for that category. Expected: a failed local read leaves the remote object unchanged; a successful read uploads the persisted cache rather than an empty list.
Earlier findings reconciled at this revision: 1 still present, 2 verified fixed, 1 could not be verified, 1 tracked separately outside this PR.
Tracked separately (outside this PR; unresolved): Native LDK shutdown abort during VSS outage. The reported VSS-outage native shutdown abort remains the open ldk-node shutdown failure in #1536. From base 43e0f3f to head 8727da0 this PR does not change LightningService.stop, the ldk-node-android 0.7.0-rc.78 pin, or native LDK sources. The issue stays open. This stage performed no native execution and does not treat the issue as a fix or a current-head pass. This is not a runtime verification or merge-readiness declaration.
Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.
Could not verify earlier concern — Signed-in outage recovery encountered Pubky lock/HTTP 405 errors: The HTTP 405 Pubky write-lock failure is an attributed device report at 9cfe052. A later attributed signed-in repeat at a3693b0 completed WALLET and METADATA and did not identify the earlier cause or a code fix. Current source still propagates Paykit snapshot failures and still refuses metadata replacement while a required WALLET backup has quarantined public addresses, but Paykit Transport is classified as temporary, so that source does not show a stuck 405. No current reproduction, native trace, or exact tracking issue was available in this code-only pass, so recurrence and resolution stay unverified.
Existing concerns
- [HIGH] Terminal identity failure blocks newer ordinary WALLET backups — still unresolved; already tracked.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Verdict: ⛔️ Request changes. The cold-start Retry hang is fixed, but an offline Retry now fails permanently, and the category stays failed after the server returns. The existing terminal complete-WALLET availability HIGH stays open as tracked.
Findings:
- MEDIUM: Offline cold-start Retry now fails permanently and blocks the later automatic upload (
app/src/main/java/to/bitkit/repositories/BackupRepo.kt:730).setup()succeeds without a network call; the native client authenticates on the firstputObject. With the backup host unreachable, that put fails withVssException.AuthException(error sending request for url (…/lnurl_auth/auth)).isTemporaryBackupFailuretreats that as permanent.markBackupFailedrecordsfailedBackupRequiredand schedules nothing, andshouldBackupkeeps skipping the category after the server returns and observation restarts.- On device, SETTINGS stayed "Failed Backup" after the host came back and the node started, while ACTIVITY and WIDGETS (never retried) uploaded on their own. It synced only after a second manual Retry.
- Before this commit, the same tap waited and then completed 0.5 s after node start (jvsena42's report).
- Manual 6 at this head saw the same authentication failure. Its second explicit Retry after restoring auth hides the missing automatic retry, which the Description promises: "Retry temporary failures for explicit backup requests even when Lightning observation is stopped, with capped backoff."
- The new
BackupClientSetupTimeoutmapping is never reached in this outage, because setup makes no network call. - Treating a request-send failure inside
AuthExceptionas temporary, like theputObjectsenvelope already is, would restore the bounded retry.
QA for 8727da0: Android 15 (API 35) emulator, devDebug built from the head, fresh unfunded staging wallet. The backup host's HTTPS port (VSS, LNURL auth, LDK backups, RGS, fx) was blocked on the device; Electrum on its own port stayed reachable. This is the same outage shape as the original report.
Backup Retry after a cold start with the backup server refusing connections ❌ Retry ended in about 10 s instead of hanging, but as a permanent failure with no retry scheduled (app logs).
The video shows the Default Unit edit, force-stop, cold start, and Settings at "Failed Backup" with Retry. Tapping Retry shows Running, then Failed Backup with Retry about 10 s later. App logs show the node build failing with KV store setup failed and no Node started during the run. They also show Backup failed for: 'SETTINGS' [AuthException=… lnurl_auth/auth] from the permanent branch, with no Retrying backup line.
case1b.2x.mp4
Backup Retry after a cold start with the backup server not answering ❌ The upload stayed Running for 5 min 12 s, then failed permanently with no retry scheduled (app logs).
With packets dropped instead of refused, app logs show VSS client setup with server immediately after the tap, then putObject hanging. It failed 5 min 12 s later with the same permanent AuthException. The native put bounds this outage, not the new setup-timeout mapping, so the long Running is an observation, not a finding. The video covers the first 2.5 minutes of Running; the screenshot shows the final Failed state.
https://github.com/user-attachments/assets/d0c4322f-c950-433d-9835-d8c75ee57a85
https://github.com/user-attachments/assets/37833734-74f4-44ce-a45e-d01686ef68ed
Pending backups sync once the server is back, and stay synced after a cold start ✅ One Retry synced SETTINGS and it stayed synced across a cold start; before that tap it stayed failed.
After unblocking, the node started in 11 s and observation resumed. ACTIVITY and WIDGETS uploaded on their own, but SETTINGS stayed "Failed Backup" for over 1.5 minutes until tapped (the finding above). One Retry gave Backup succeeded for: 'SETTINGS', and after a force-stop and cold start every row still showed a recent backup.
https://github.com/user-attachments/assets/ec6a0380-f89b-4d19-be43-d97412b9bf97
https://github.com/user-attachments/assets/c8289368-cb94-4126-a0cc-6006d7f49fd9
https://github.com/user-attachments/assets/e9300559-6816-4022-9993-d53782260796
https://github.com/user-attachments/assets/c0f65cf5-fa35-4dbe-ae35-95f925fc13d3
Not driven: Manual 1–5 need held setup, capture and upload controls. Their retained scheduler report at 9a837a6 and the later reports listed in our previous review keep their recorded heads and limits. This delta touches the ordinary upload path they rely on only by adding setup before the put. The BLOCKTANK read from Core and the ACTIVITY read refusal ran in the cold-start uploads above (ACTIVITY synced from a cold start), but no Core read failure was injected. Hosted CI at this head passes.
piotr-iohk's code-only QA review at this head reads the setup timeout as keeping cold Retry on the bounded retry path; the cases above show the outage arriving as an AuthException from the put instead. Its suggested remote BLOCKTANK/ACTIVITY overwrite case was not driven here.
Twin: bitkit-ios#1123 at ff3ba67 already sets up its VSS client lazily inside putObject, so it matches the new Android behaviour (source comparison only).
Review covered the 439b5d0..8727da0 delta. The base merge add2c04 leaves this PR's own patch byte-identical, so review focused on 8727da0. Outside this delta, uploadWalletBackup and UsdtRecoveryBackupStore still let a setup TimeoutCancellationException escape without failure bookkeeping. A category interrupted by process death also shows "Running" with no Retry until the node starts and observation clears it.
Reviewed by claude-opus-5-5-medium
| setup.onFailure { | ||
| if (it is TimeoutCancellationException) throw BackupClientSetupTimeout(it) | ||
| }.getOrThrow() | ||
| vssBackupClient.putObject(key = category.name, data = data).getOrThrow() |
There was a problem hiding this comment.
MEDIUM: With the backup host unreachable after a cold start, setup() succeeds without a network call, and this put fails with VssException.AuthException (error sending request for url (…/lnurl_auth/auth)). isTemporaryBackupFailure treats that as permanent: failedBackupRequired is set, no retry is scheduled, and shouldBackup keeps skipping the category after the server returns. On device, SETTINGS stayed "Failed Backup" after the node started, while untouched categories synced; it needed a second manual Retry. Before this commit, the same tap waited and completed after node start (jvsena42's report). Treating a request-send AuthException as temporary, like the putObjects envelope, would restore the bounded retry.
Cold-start ordinary Retry — November 3
Signed
8727da02549c3a885fbb48b500747b3401d3d649fixes the reported cold-start Retry hang. Ordinary categories now initialize their VSS client before upload even when the Lightning node has never started. A setup-owned timeout retains bounded retry; caller/reset cancellation still drains and propagates. Newly reachable cold captures read persisted Blocktank data without network refresh and refuse failed activity/receipt reads, rather than publishing incomplete snapshots.Final
just format,just compile,just testandjust lintpass on 1,797 matching inputs: 4,913 unit tests, 263 suites, zero failures/errors/skips and zero static findings. Complete structured diagnostics and both independent affected source reviews are sealed. Earlier controlled failures and baseline-attributed warnings remain retained. The full 23-commit live-parent range is verified locally and by GitHub (verified=true,reason=validfor every commit).Manual 6 passed in the actual app, using released Core RC11/Paykit RC2/VSS RC1/LDK rc78, a fresh unfunded Android user and verified ARM64 APK
f762d47c…beb0733. A real display-preference edit remained pending through process death. With auth unavailable and node startup failing, explicit Retry initialized VSS and returned an authentication failure after 10.276 seconds instead of hanging. Restoring authentication only, while node reads remained rejected, let the next explicit Retry receive a real VSS PUT acknowledgement and synchronize SETTINGS. Another cold restart retained the same settings bytes and synchronized revision. No node-start event occurred after the first cold launch. The original APK, routes and older users were restored and checked. This is a SETTINGS case with no prior uploaded SETTINGS object. The observed failure occurred at authentication for PUT after setup succeeded; setup-failure handling and read-refusal behavior have separate component coverage. It does not establish prior-object preservation, complete-WALLET availability, remote restoration, payments or the unverified portions of Manual 1–5.Manual 7A — prior ACTIVITY object and local read refusal passed in the actual app. On the same verified published inputs and unfunded user, one explicit Transaction Log Retry ran before successful node initialization. A temporary, reversible
closed_channelstable-name fault caused the actual Core SQLite read to fail before VSS setup. The required revision advanced from1793717566863to1793739130414; synced stayed1793717561910, and the operation returned idle. There was no VSS PUT. The independently authenticated existing ACTIVITY row retained version 1, all 192 ciphertext bytes (SHA25605f378372069fa546047cf3db96efedff6f88f83615cab4c6ab92801830001ef) and its store/key binding; all 15 scoped remote rows matched before, after and at restoration. Actual node reads were refused and no successful node setup/start/observer was observed in that cold process. The original table name, row fingerprints, column/index/foreign-key shape, APK, nine routes and older-user state were restored. SQLite file bytes and raw schema text changed during the rename round trip, so byte-identical database restoration is not claimed. Two earlier pre-fault stops remain recorded. The prior decrypted payload contains no activity records (124 bytes): Manual 7B, successful capture of a nonempty persisted cache, remains open. This result is not a terminal node-failure, lifetime-never-started, BLOCKTANK, wallet-restoration or payment claim. Complete private evidence was audited and all 22 sealed members verified:work/validation/android1689-prior-read-failure-runtime-c-20261103/report.json.New-head hosted native CI is complete on
8727da02and the identical synthetic tree: CI passed 4,913 unit tests and 256 Compose tests; lint has zero findings across all six structured reports; E2E passed 39 cases on their first internal attempts, with one explicitly disabledsettings_08skip. Six migration jobs were skipped, not passed. Complete logs and all ten report archives were inspected and verified; cursor, native callback-fixture initialization, wallet/network and backup-guard warnings remain documented, without claiming harmlessness. The external review check is separate from native CI and human approval. Runtime evidence:work/validation/android1689-cold-start-runtime-20261103; full native diagnostics:work/validation/android1689-cold-start-diagnostics-20261103.The current-head Piotr review verifies the cold-start source repair and retains the terminal-WALLET concern. Its suggested prior-remote BLOCKTANK/ACTIVITY object plus failed-local-read app case is not covered by Manual 6 or the component tests and remains an explicit acceptance gap. Historical HTTP 405 causation remains unverified. No repeated review request was sent for this just-completed review.
Prior validated parent composition — November 3
Signed additive merge
add2c04b684609b6ad74a30835864d816f0a79f6incorporates dependency #1686 at43e0f3fe04705d397a81f493cf8e57239a0be73e, including current master44bf50ec. The backup-observer patch retains the same added and removed lines after context shifts; the automatic merge preserves writer draining, observation ownership and the new restore-category result contract. Published history is preserved.Final
just format,just compile,just testandjust lintpassed on this actual composed tree: 4,905 unit tests across 263 suites, zero failures/errors/skips, and zero Detekt/Android Lint findings. All four phases share the same 1,797 frozen inputs. Complete logs, XML/HTML/SARIF, fresh structured static reports andignoreFailuresoutputs were inspected. Four cursor-close diagnostics remain recorded and trace to emitter sources identical to the actual parent; this is not a claim of runtime harmlessness. The independent affected-composition review is clean. All 22 live-parent range commits are locally and GitHub signature-verified.At
add2c04b, hosted native checks completed and were fully audited: 4,905 unit passes, 256 Compose passes, zero static findings and 39 unique eventual E2E passes, plus one disabled Settings case and six skipped migration jobs. The original Lightning-security attempt failed while downloading its system image, before tests. The single human-authorized retry (job 125932596605) ran both previously blocked cases successfully on its first in-job attempt, and the aggregate passed. The other 37 E2E passes and existing unit/Compose/static results retain their original execution credit; remapped job IDs are not new runs. All report digests/member CRCs and complete nested diagnostics are inspected, retaining the initial infrastructure failure and runtime warnings. Retry supplement:work/validation/android1688-1689-authorized-infra-retries-20261103/diagnostic-audit/STRUCTURED-REPORT.json(133 new sealed payloads); original full audit and failures remain preserved. Earlier hosted and authored/external runtime results below retain their original revision, artifact and fixture limits; this composition adds no new device or authenticated-journey claim. Evidence:work/validation/android1688-1689-master44bf-final-audit-20261103/STRUCTURED-REPORT.jsonandwork/validation/android1689-master44bf-composition-20261103.Retained scheduler acceptance evidence — November 2
Current review 5731374382 retains the earlier installed scheduler cases at
9a837a6. These are historical external executions, so the earlier blanket description that Manual 1–5 were unrun was too broad. The author has inspected the disclosed controls and inline log excerpts, and compared relevant source: 22/24 selected lifecycle/retry function slices and the VSS/cache/USDT files and dependency pins match439b5d0. Shared WALLET/METADATA serialization, allowance observation and restore/capture changes retain their separate later evidence.The original build/install binding and Manual 1–5 event/readback traces have not been independently verified; the checkboxes below remain open for those exact gaps. Manual 2's reported empty-wallet envelope does not alone establish the same-signed-operation reset variant. These results are not current-head author executions, complete WALLET capture or permission to use the reviewer's fixtures. Evidence:
work/validation/android1689-retained-acceptance-reconciliation-20261102/REPORT.md.Current-head hosted results — November 2
All hosted results for
439b5d0eea89a72ee6cdf1167515b95339a65c58are inspected: 4,857 unit passes, six static reports with zero findings, and 39 distinct eventual E2E passes. Oneonchain_2execution timed out waiting forActivity-3; its automatic second attempt passed. The failure image contains all three transactions across two date groups. Exact source counts date headers in the activity index, while the suite expects contiguous transaction indices; this supports a selector mismatch, but no captured semantics tree establishes the concrete runtime IDs. No wallet-data-loss or causal repair claim follows.The synthetic tree exactly matches the published tree and suite
611509ee. One disabled Settings case and eight cached-pass skip records are distinguished; six migration jobs did not execute. Complete logs, all nested streams, nine checksum/CRC-verified archives and failure media were inspected; the owner verified 885 sealed payloads. Four CloseGuard observations and runtime diagnostics remain recorded. No manual CI rerun, new authenticated backup/restore pass or installed-binary claim is implied. Evidence:work/validation/hosted-android1689-439b-20261102-current-a/REPORT.md.Final identity-recovery parent composition — November 1
Signed additive head
439b5d0eea89a72ee6cdf1167515b95339a65c58incorporates #1686/b6751e22956653c4f2a80112143880b47298e30dat tree820b8c27563810cebc079345e05ee6b680a5ea09. The parent's allowance off/on admission repair and failure-only routing diagnostics are included; this PR's backup-observer patch remains byte-for-byte intact. The affected composition review is clean and retains the two independent parent concurrency reviews.Final required compile, 4,857 tests / 260 suites, formatting and lint passed with zero failures/errors/skips/findings. All 1,781 frozen files match. Complete logs, XML/HTML/SARIF and fresh changed-unit lint analysis were inspected. Four aggregate lint reports were UP-TO-DATE with matching sealed hashes; they are not described as regenerated. Four CloseGuard events remain retained, including three present only in the full executor stderr. All 21 live-parent range commits verify locally and on GitHub. Current-head hosted results are recorded above; older failures and the unresolved parent routing-test cause remain preserved. No new authenticated backup/restore journey or installed-binary equivalence is inferred. Evidence:
work/validation/android1689-parent-b675-20261101/final-diagnostic-audit/STRUCTURED-INSPECTION.json.Hosted validation completed — November 1
At
2eb0c492439ebad8943457622ba67437d828d1f2, all seven E2E shards and the status job have completed: 39 first-attempt passes, with one disabled settings case. The earlier audited 4,856 unit tests and six zero-finding static reports remain tied to this exact head and parent1b1426fb.The sole audit combined prior immutable evidence with only the missing complete shard logs and checksum/CRC-verified report archives. The runtime metadata-backup refusal is retained: its exclusion guard matches the actual base, while worker/handler context differs, so no blanket pre-existing-runtime or eventual-backup-completion claim is made. Other diagnostics, npm advisories and skipped migration jobs remain explicit. This is not new authenticated recovery acceptance. Evidence:
work/validation/hosted-android1689-2eb0-20261101-tail-b/REPORT.md.Current parent composition — November 1, 21:48 UTC
Signed additive merge
2eb0c492439ebad8943457622ba67437d828d1f2incorporates live #1686 at1b1426fbebeccd616c29311fa5bf652089727a84; final treea5c0dd6e741a81d34a1736fce15c4f00f95e52f8. The owned child patch is unchanged. The incoming queued allowance-admission and pending-profile-setup corrections have one clean affected composition review, reusing the matching independent parent concurrency reviews.Required
just format,just compile,just testandjust lintpassed 4,856 tests /260 suites, zero failures/errors/skips, and zero findings across all six Detekt/Android Lint reports. All 1,782 tracked inputs match the composed tree. Complete logs and structured reports were inspected; four CloseGuard allocation observations and JVM notices remain recorded, and stale Gradle report copies are excluded. All 19 live-base commits verify locally and on GitHub (true/valid).Current-head hosted CI awaits complete inspection. No new authenticated app journey or causal repair of prior E2E failures is claimed. The terminal complete-WALLET limitation and separately scoped profile-removal gaps remain unchanged. Earlier acceptance and failures below retain their exact tested revisions. Evidence:
work/validation/android1689-parent1b14-review-20261101/andwork/validation/android1689-parent1b14-final-a/.Earlier parent composition — November 1
Signed additive merge
a3693b0c7061642a0b4305c07841a438a79046b9incorporates live #1686/78269d8a1cae1ac127be93a96fba16f7d68d1019; treec5943235d319bbf7171ebccca58fd10c0327b188. The backup-observer delta remains unchanged. The composition includes queued-offer admission and stable activation during same-identity contact refresh, with one clean affected review and independent parent concurrency reviews retained.Required final formatting, compile, full tests and lint passed 4,849 tests /260 suites, zero failures/errors/skips. All 1,781 frozen inputs match; complete logs and fresh Detekt/Android Lint reports contain zero findings. Three JVM notices and four verified unchanged-provider cursor warnings are retained; stale copied reports are excluded. All 18 live-base commits verify locally and on GitHub. New-head hosted CI is pending inspection. No new authenticated app runtime is claimed from these gates.
The earlier
9cfe0527external evidence and hosted recovery remain revision-specific. The first multi-address failure, internal retry and media-clock provenance limits are preserved; this composition is not their causal repair. The terminal-identity complete-WALLET availability finding stays open, with no partial/stale backup fallback. Evidence:work/validation/android1689-parent7826-final-a/inspection/andwork/validation/android1689-parent7826-20261101/publication/.Historical-head external evidence — October 30, 22:21 UTC
Automated review 5719299576 at
9cfe0527reports automatic METADATA recovery after durable WALLET exclusion and writer/reset draining with a disposable staging-VSS wallet. Its cold-start outage repeat completed after ordinary sign-out; later signed-in synchronization remains unverified after native Pubky lock errors. It also reproduces the existing terminal-identity complete-WALLET availability failure, which remains open. The owner verified the revision and read the report/control patch; raw reviewer logs, DataStore, binaries and media were not independently audited. These remain attributed external evidence, with no additional author acceptance or human approval inferred.Historical parent composition — 30 October 2026
Signed additive head
9cfe052761ebdf51a017cebc3b7a9ab173d953efincorporates the validated profile-session recovery branch atff5aa32f4da1ec33fb8be63e0b84597d463c82b1, including the current #1686 parent78a0ff7a. The GitHub base remainsfix/restore-identity-background; the existing stack and published history are preserved.WALLET and METADATA capture/upload share the wallet serialization boundary while retaining generation-owned observation, cancellation drains and restored public-address exclusions. A new observer regression proves that a WALLET acknowledgement automatically resumes previously blocked metadata exactly once, without a manual metadata retry. Omitting that rearm fails the direct expected-one/actual-zero assertion; restored final source passes.
Final
just compile,just test,just lintand unchangedjust formatpass on treee27b3e6112da04eda455031c70de9fe5709cf9f6: 4,839 fresh tests / 260 suites, zero failures/errors/skips, zero Detekt/Android Lint findings, all 1,778 frozen inputs matched. Complete logs and structured reports were inspected. Three JVM notices, four unchanged-provider CloseGuard chains and historical copied Gradle reports remain recorded; the compile reused matching outputs and is not a clean-rebuild claim. Failed helper, negative-control and intermediate formatting results are retained.Two independent affected source reviews are clean after closing the automatic-rearm coverage finding. All 15 commits against the live base verify locally and on GitHub (
verified=true,reason=valid). Current-head hosted CI passed 4,839 tests / 260 suites, zero failures/skips; lint has zero findings across all six uploaded reports. Complete logs, all 262 unit HTML pages and structured lint reports were inspected. E2E run 41104465012 reached 39 unique eventual passes and one unrun Settings case. Its original onboarding/backup/numberpad job failed Docker port binding before tests. One bounded job-only recovery used the original APK and exact suite611509ee, then passed all seven selected cases on its first test step. Separately, the multi-address case failed its firstSentactivity assertion and passed the workflow's internal retry; its failure, screenshot and all five encoded video frames remain preserved. This is not a first-attempt clean E2E result. The failed clip/runtime carry October 21 timestamps within the October 30 run; clock/history provenance and the assertion's cause remain unresolved.All executed job logs, nine checksum/CRC-verified small archives, their 307 members and complete runtime diagnostics were inspected; current CI merge tree equals this published head. A 60-second RN backup-list timeout and other retained runtime errors are not waived by test success. The hosted recovery downloader verified the original APK digest; independent local APK/native byte inspection remains unperformed after a transfer timeout and low-memory guard. No new full-app fault-journey acceptance is claimed. Earlier hosted passes below apply only to their recorded heads. These component controls do not complete manual cases 1–5 or the authenticated fault journeys. The existing terminal complete-WALLET availability concern remains open.
Historical parent composition — October 29
Signed additive merge
1d3ba149eef529f598ed9947d145bc9aa2bc9536incorporates current #1686 parent217b42ef889952a6c1f3a30b0c44a8b1001db4a5; final tree753c9b8b2edc93bd2f2ba2ee5b277cf4cac38c06. Observer drains, explicit retry while stopped, original ownership, cancellation and complete-backup gates remain intact. The source review caught a test-helper argument conflict; the named-argument correction preserves all assertions and both parent/child test sets.Final required compile, full tests, lint and format passed. 4,742 tests across 256 fresh suites passed with zero failures/errors/skips; all 1,760 regular inputs and six symlinks match. Complete logs, XML/SARIF and ignoreFailures configuration were inspected: zero compiler/lint/format findings. Four cursor-close warnings in test output and three JVM notices remain disclosed with actual-parent source comparisons and no runtime-causality claim. Full 113-case BackupRepo coverage passes. Existing authenticated restoration/outage and terminal complete-WALLET acceptance gaps remain distinct.
The complete nine-commit live-base range is locally and GitHub verified (
verified=true,reason=valid). Ready status is retained (draft=false). Current-head hosted CI passed 4,742 tests in 256 suites, and lint reports zero Android Lint/Detekt findings. E2E reached 39 unique eventual passes and one true settings skip. Settings attempt 1 timed out after 30 seconds waiting for ElectrumErrorToast; its automatic second attempt passed. The original exit 1/failure annotation remain retained; the cause is unproven, and failure-only media was not uploaded after eventual success. This is not a first-attempt clean pass or the authored fault journey. All 14 complete executed logs, 18 annotations, complete uploaded reports and four SHA-256/CRC-verified archives were inspected; the synthetic merge tree equals this head and its actual parent. Four unchanged-provider cursor stacks, three JVM notices and absent uploaded Gradle problem-detail reports remain disclosed. Migration was skipped, not passed; no rerun was requested. Historical execution evidence below remains revision-scoped.Historical scheduler repair — 29 October 2026
Signed head
9a837a6e7b9286d4dca24d57c451929131285b27includes the additive parent mergee80ca3f95from #1686 at9ce430274. A permanent failure suppresses only the revision that failed: a newer committed recovery revision remains eligible for observation after its caller cancels. Background coalescing now waits before taking the category mutex, so an explicit Core callback does not wait behind idle debounce. It still waits for an active upload and must obtain acknowledgement from its own immutable snapshot write.Both controlled regressions failed before the repair and pass afterward. Final
just format,just compile,just testandjust lintpassed on the combined tree: 4,714 fresh unit tests / 256 suites, including all 112 BackupRepoTest cases, with zero failures/errors/skips and zero Detekt/Android Lint findings. All 1,760 frozen inputs match. The complete 9,692-line test log, all XML/HTML/SARIF and embedded Gradle diagnostics were inspected; copied older reports were separated from fresh results. Three JVM notices, four verified pre-existing provider cursor warnings and the unchanged Detekt plugin deprecation remain disclosed. The canonical compile used matching up-to-date outputs; this is not a clean-rebuild claim.Two independent scoped correctness/concurrency reviews are clean. All eight live-base commits verify locally and on GitHub (
verified=true,reason=valid); published history is preserved. Current-head hosted CI passed 4,714 tests in 256 suites, and lint reports zero findings. E2E completed 39 distinct passes and one explicit settings skip: 38 cases passed first attempt; Lightning first failed with an invoice expired on October 22, then passed automatic attempt 2. Security's second-attempt marker skip follows its actual first-attempt pass. All 11 complete E2E logs (7,041 lines), every attempt and annotation were inspected; the earlier failure and unproved cause remain retained. The synthetic checkout tree equals this head and tests the current parent. CI/lint report archives were SHA-256/CRC verified; all seven emulator consumers checked the published APK archive digest, without a local installed-binary claim. Earlier failure media was not uploaded after eventual success. Migration was skipped, not passed. No workflow rerun was requested. Earlier hosted results below remain revision-scoped. These service-callback tests use mocked VSS and do not establish the authored full-app fault cases, process-death payment submission or an authenticated native journey. The current iOS twin already keeps idle debounce outside its USDT writer; no mechanical opposite-platform patch is indicated by the narrow source comparison.Prior parent composition — 28 October 2026
Signed additive merge
5fca5cd39df4fcf072eacb36bd63c758a55a0f5dincorporates parent #1686 at89e2702e, preserving the existing stack. The final tree2edb93a2passed 4,638 unit tests in 254 suites, zero failures/errors/skips, plus requiredjust format,just compileandjust lint. Complete logs, all XML/HTML/SARIF and embedded Gradle reports were inspected; Detekt and Android Lint have zero findings. All 1,753 frozen file inputs and the directory link match. Three JVM notices, four unchanged-provider CloseGuard stacks, the inherited manifest notice and Detekt deprecation remain disclosed against exact parent source/configuration; no fresh parent runtime baseline or warning-free claim is made.All 605 production feature patch operations are unchanged. The sole conflict removes two obsolete refund-pointer tests with the parent's Blocktank sunset; no retry, snapshot, reset or revision assertion was dropped. One independent affected-composition review is clean, retaining prior matching lifecycle/explicit-retry review coverage. These full unit gates do not establish the authored authenticated lifecycle/endpoint fault journeys or resolve terminal full-WALLET availability.
All 6 live-base commits verify locally and on GitHub (
verified=true,reason=valid), with exact sets matching. This PR remains ready. Current-head hosted CI passed 4,638 unit tests in 254 classes with zero failures/skips, and lint has zero Detekt/Android Lint findings. E2E reached 39 distinct passes and one explicit settings skip: boost_2, widgets_1 and widgets_3 first failed their expected-text, WidgetSave and PriceWidget assertions, then passed automatic attempt 2. Earlier failures and process annotations are retained; their causes remain unproven. This is not an all-first-attempt pass or the authored authenticated fault journey. The complete 14 executed job logs, all unit/lint reports and four SHA-256/CRC-verified archives were inspected; the executed merge tree equals this head and its tested parent matches the live stack. Failure-only media was not uploaded after eventual success. No workflow rerun was requested. Migration was skipped, not passed.Twin: synonymdev/bitkit-ios#1123
Dependency: #1686
This PR drains interrupted backup work before resetting its clients and preserves pending backups across observation restart.
Description
Out of Scope
Design
N/A — no UI changes.
Preview
N/A — background lifecycle repair.
QA Notes
Setup
Use the committed dev Debug dependencies: Core 0.8.0-rc11, Paykit 0.1.1-rc2, VSS 0.5.26-rc1 and LDK 0.7.0-rc.78. Automated tests use synthetic data, a real local DataStore, the SDK operation barrier and fake VSS transport; no funds or device wallets are changed. Manual cases require a dedicated disposable fault fixture with held setup, capture, upload and cache-write boundaries as described in
journeys/backup-restore/wallet-snapshot-consistency.md. Those app-level controls are not provisioned in the standard journey environment. Do not use live funds, reset personal wallets or reuse another worker's device.For Manual 6, use an isolated dev/E2E regtest build with a disposable local LNURL-auth/VSS backend. Set
E2E=true,E2E_BACKEND=local,E2E_VSS_SERVER_URL=http://localhost:18717/vssandE2E_LNURL_AUTH_SERVER_URL=http://localhost:18715/auth; reverse only those device ports to a dedicated fault proxy forwarding to the fixture. Keep authorization headers and native encryption intact. First reject authGET /authbefore forwarding and nodePOST /vss/getObject/listKeyVersions; then allow auth andputObjectswhile continuing to reject node reads. Use Settings → Default Unit for the edit and Settings → Security → Data Backups for Retry. Retain proxy outcomes, app lifecycle/backup logs, installed APK hash and nonsecret backup-status readbacks. No wallet reset, seed capture or funds are needed. Restore prior device routes and APK after testing.Journeys
N/A — not drivable; see Manual Tests.
Manual Tests
1. Hold backup setup or cancellation cleanup → stop/restart observation → edit settings while restart waits for old cleanup, including another category’s retry becoming due → no setup/upload before drain; latest changes resume without another edit — historical external Test 1 is reported; original event/readback and build/install binding remain to be verified.
2. Hold manual or USDT persistence → request fixture reset, including cancellation of its caller → both clients wait for old work; canceled persistence is not acknowledged and retry retains the same signed operation — historical external Test 2 covers an empty-wallet envelope; build/trace binding and the same-signed-operation reset variant remain unverified.
3. Keep wipe admission closed after reset → request manual/full/USDT backup → no new upload or cache write; after release, explicit USDT works with observation stopped — historical external Test 3 reports backup wipe-admission controls; its no-write/ack trace and build binding remain unverified.
4. Change required data during a full backup, hold its required-status write, or request it during stopped-job cleanup → release the gate and restart → the newer work uploads without another mutation — historical external Test 4 reports SETTINGS follow-up; held-write/revision/readback and build binding remain unverified.
5. With observation stopped, fail an explicit category or USDT backup temporarily, restore transport, and verify bounded backup retry without another edit; keep Core acknowledgement withheld until its own callback succeeds. Also cancel a newer callback while an older upload is held: the newer revision stays pending and the old upload cannot satisfy it — historical external Tests 5a/5b and later callback controls are reported; their original trace and build binding remain unverified.
6. On an unfunded disposable wallet, change a display preference and stop the app before backup debounce completes. Cold-start with VSS authentication unavailable and node initialization blocked; SETTINGS Retry must return failure with the revision pending. Restore authentication while continuing to reject node reads, tap Retry, then cold-restart again: SETTINGS remains synchronized without node startup. Executed on the exact candidate APK/source above; this does not cover other categories or remote restore.
Automated Checks
BackupRepoTest.kt— held setup and upload cleanup, reset cancellation, cache finalization, concurrent lifecycle calls with real DataStore, actual SDK wipe-lock ordering, full-backup follow-up, stopped-observer USDT persistence and actual service acknowledgement/retry record identity.Evidence
Controlled baseline reproduced late LDK setup after reset. Further fail-before tests reproduced lost full-backup follow-up, premature manual reset, fresh backup admission during wipe, a full request lost behind canceled cleanup, and consumed settings changes lost during stop/reset. The final focused suite has 93 cases, including 14 added lifecycle, admission and persistence regressions.
Final commit
a8f0810769b5c29c01c6f7c1f02e3988b048c7db, based on #1686 (ac2a7c668):just compile,just testandjust lintpassed: 4,639 tests across 254 suites, zero failures/errors/skips; fresh JUnit timestamps verified. Fulljust formatpassed. All Detekt and Android Lint SARIF/XML reports contain zero findings, with no compiler warnings; the JVM bootstrap-classpath notice and Gradle deprecation match recorded baseline diagnostics. Two consecutive final independent read-only reviews found no actionable correctness or quality issues.Follow-up verification on October 25 (signed head
e585fccf5): the settings-during-restart regression fails ona8f08107; a separate temporary WALLET retry with held WIDGETS cleanup fails without its admission guard. Both repaired cases pass in the 95-test backup suite. Finaljust compile,just test,just lintandjust formatpassed: 4,641 tests / 254 suites, zero failures/errors/skips, and all Detekt/Android Lint reports contain zero findings. Complete logs and reports inspected; structured Gradle diagnostics match the earlier baseline. Two consecutive independent final reviews were clean. Both commits in the actual-base PR range are locally verified and GitHub reportsverified=true,reason=valid. These controlled component cases extend manual cases 1 and 4; no full fault-injection journey pass is claimed.Prior published head
a8f08107also passed hosted CI (4,639 units) and all seven existing E2E shards (39 cases passed, one settings case skipped). Complete logs and checksum/CRC-verified uploaded lint artifact12583364581contain zero SARIF/XML findings. Those hosted results apply to that head, not the follow-up.Prior signed head
e585fccf5passed hosted CI40158027061(4,641 tests) and lint40158027049. Complete logs and checksum/CRC-verified artifact12585438925were inspected; all three SARIF/XML reports contain zero findings. Hosted E2E40158027317 subsequently completed all seven shards with39 passing cases. The backup case first failed waiting for the widget “Yes, Delete” confirmation, then passed on the workflow’s automatic retry. Complete logs were inspected; the initial widget-confirmation failure’s cause is still unproven, so this is not claimed as a clean first-attempt run. No additional workflow rerun was requested.Latest parent composition (October 26), signed head
c2554581e2b5e2c8e68802a577a76a56b291ae4b: additive merge of #1686 at0fd5e424, without rewriting published history. Both parent malformed connection-correction tests and the lifecycle ownership/USDT independence behavior are preserved. Finaljust compile,just test,just lint, andjust formatpassed: 4,681 tests / 255 suites, zero failures/errors/skips, 97 backup cases, and zero Detekt/Android Lint findings. Complete logs and structured reports were inspected; JVM, cursor and Detekt-plugin diagnostics match verified earlier parent evidence. All 1,765 frozen inputs match. Two consecutive independent final composition reviews were clean; all three current-base commits are locally signed and GitHub true/valid. Hosted composition checks passed 4,681 units, lint with zero findings, and 39 first-attempt E2E cases with one settings skip; complete logs and checksum/CRC-verified reports were inspected. These results apply to c255458, not the later transport commit, and are not an authenticated fault-journey pass.Latest transport follow-up, signed
a102e0ede565e60aaf7e033f3ad66a870739066b: the released VSS FFI represents request-send failures as UnknownException. Two controlled regressions fail before the fix; the narrow backup-only compatibility classifier and observed-settings retry then pass. Final4,683 tests/255suites, including99backup cases, pass with zero failures/errors/skips. Required format, compile, full Detekt and Android Lint pass with zero findings in all three fresh SARIF/XML reports. All1,760 frozen source inputs match. Complete9,629-line test log and structured reports were inspected; three JVM notices, the full cursor-allocation stack multiset and Detekt plugin diagnostics match verified parent evidence. Initial formatting corrected three introduced ordering/wrap findings before final gates. Two consecutive independent final reviews are clean; all four live-base commits are locally signed and GitHub true/valid. Hosted checks on a102e0e passed 4,683 units and lint with zero findings. Existing E2E cases reached 39 unique passes and one never-run settings skip after five failed executions: LNURL/Transfer failed two attempts before the third passed, and multiaddress required a second attempt. Complete logs and checksum/CRC-verified artifacts were inspected. These are eventual prior-head passes, not first-attempt or new-head evidence; the timeout causes remain unproven.This classifier is restricted to the typed native UnknownException with its anchored request-send envelope and valid HTTP(S) host/putObjects path. It does not change generic SDK mutation retry, cancellation or permanent unknown errors. Tests inject the actual native error representation into the repository; this is not a new authenticated endpoint-outage journey.
Latest explicit-retry follow-up, signed
49e9a99d35f72685f3187b9585fb2aaa3d64a904: manual/full/USDT backup retries retain their own ownership after observers stop. Supplied recovery snapshots reserve their exact required revision before a cancellable category wait, so canceled queued work remains pending and an older upload cannot acknowledge a newer snapshot. Superseded retry timers cannot bypass a newer failure’s backoff. Thirteen new regression cases include controlled before failures for stopped observation, successor revisions, stale timers and queued cancellation. The final suite passed 4,696 tests / 255 suites (112 backup cases), zero failures/errors/skips. Final format, compile, full Detekt and Android Lint pass with zero findings in all three structured reports. All 1,760 frozen hashes match; the complete 9,648-line test log, all XML/SARIF and retained Gradle diagnostics were inspected. Introduced formatting findings were corrected before the final unchanged gates; JVM/cursor/deprecation diagnostics match verified parent evidence. Two consecutive independent final reviews (rounds 2 and 3 after repairing the round-1 queued-callback finding) are clean. All five actual-live-base commits verify locally and on GitHub as true/valid. Current-head hosted CI40400088998 passed 4,696 tests/255 classes with zero failures/skips; lint40400089110 has zero findings in all structured reports. E2E40400089623 reached 39 unique passes and one never-run settings skip after three failed executions: RGS settings, LNURL and transfer each passed their second internal attempt. Complete 18,712 lines across 15 job logs and four SHA256/CRC-verified archives were inspected. The merge tree equals this head; JVM/cursor/deprecation diagnostics match verified parent evidence. The timeout causes remain unproven and failure screenshots were not uploaded after eventual success; no rerun was requested. Manual case 5 and complete authenticated fault journeys remain unrun; these controlled component tests use the real service callback with mocked transport and do not prove autonomous payment submission or process-death retry.Gaps
Explicit backup retry now has ownership independent of observation. Background backup success does not acknowledge an earlier failed Core callback or submit a payment; manual USDT Retry and foreground recovery must still resume and acknowledge the original signed operation. Cancellation retains required data without turning every canceled or permanent failure into a fresh automatic retry.
The new controls are component tests. They do not execute a complete app UI/Paykit/deployed-contract failure journey, and the VSS transport in these tests is mocked. Existing released-native signing/encrypted recovery evidence remains scoped to its recorded app revisions. No Android crash or historical contact/acquisition cause is claimed. The full-WALLET availability and migration engineering remain separate. Updated-app-only new-format restore is approved, but independent-only backups are not activated.
Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh);gpt-6.1-sol(reasoning:high)