Repository navigation
fix: pause paykit work during profile deletion - #1510
ben-kaufman wants to merge 10 commits into
Conversation
 [Medium impact] The PR appears safe to merge; no actionable issue was found. SummaryPauses automatic Paykit work while a profile is being deleted.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Begin profile deletion] --> B[Pause automatic Paykit work]
B --> C[Run explicit cleanup and deletion]
C --> D[End deletion pause in finally]
D --> E{Identity retained?}
E -->|Yes| F[Resume contact and endpoint refresh]
E -->|No| G[Remain signed out]
Reviews (1) · Last reviewed commit: "fix: pause automatic paykit work during ..." · Reviewed by Greptile |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Turns the profile-deletion flag into a StateFlow and pauses automatic Paykit work while it is set: endpoint publication, contact sync, Shop import, request intake and maintenance. Already-admitted work and payment bookkeeping still finish, and a paid public invoice is still cleared, without publishing a replacement. Reviewed bc00345, full tier, reasoned from the code and CI. On this track I do not build Android branches locally.
What I checked, and 5 candidates I ruled out
Read in full: the diff of AppViewModel.kt, PrivatePaykitRepo.kt, PublicPaykitRepo.kt and both test files; EditProfileViewModel.attemptDeleteProfile, PrivatePaykitRepo.closeAndClear / removePublishedEndpointsForCleanup, ContactPaymentSettingsRepo.reconcilePendingEndpoints
Call sites traced: beginProfileDeletion / endProfileDeletion (only EditProfileViewModel, with endProfileDeletion under NonCancellable in finally), syncCurrentPublishedEndpoints, republishIdentityIfNeeded, refreshPublishedBolt11ForPayment, retryPendingPaykitEndpointRemoval
CI: lint and detekt green; build and build-local still running at review time
Ruled out
- Contact sync never clears request state after a successful delete:
distinctUntilChangedruns beforefilterNotNull, so the flow emitsS(key), thennull, thenS(null). Once deletion ends, the collector receivesS(null)and takes the clear branch. - Stuck pause after a failed or cancelled delete: the only writer resets the flag in a
NonCancellablefinally. The address, proof and contact flows all combine with it, so they re-emit when it drops, and the first new test checks the replacement address is published. - Invoice-expiry refresh dropped for good during deletion:
refreshPublicPaykitEndpointsIfEnabledreturns early, but the address observer fires again when the flag clears and runs the same sync. - Explicit cleanup blocked by the new guards:
removePublishedEndpointsForCleanupandcloseAndClearare called directly by the deletion flow and do not go through any of the gatedAppViewModelpaths. - Paid invoice re-advertised: with
publish = false,refreshPublishedBolt11ForPaymentstill clears the matching metadata before returning.PublicPaykitRepoTestcovers both the matching and the unrelated hash.
Merge confidence: 4/5, the change is covered by the five new AppViewModelSendFlowTest cases and PublicPaykitRepoTest, but the build job had not finished when I reviewed.
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
|
New head Previous head Historical Earlier Controlled final-head cleanup-failure, admission and payment/device cases remain unchecked. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Reviewed the full PR diff against its merge base, at bc00345. No earlier assessment baseline was available.
2 actionable findings — resolve or provide an evidence-backed rebuttal.
Payment-triggered publication can still start after profile deletion has begun. The public replacement decision is taken once, before invoice creation. Private invoice and on-chain rotation still publish while the deletion flag is set, after cleanup-pending has been cleared. Contact sync, shop import, request intake, and maintenance do recheck the flag, and explicit cleanup stays on its direct path. On bitkit-ios#954 at 1ae24c1, private receive refresh returns from contact preparation while deletion is in progress, and public sync rechecks deletion immediately before publication.
The new AppViewModelSendFlowTest and PublicPaykitRepoTest cases cover a deletion flag that is already set before the payment event, and a publish = false cache clear. They do not cover the flag changing during public invoice creation, or private publication from handleReceivedPayment. Those tests were inspected and not run here. CI build and local e2e are green at this revision; those jobs do not exercise this deletion admission. Device testing: not performed in this review.
Suggested additional test cases
- Android, regtest wallet with public Paykit sharing and a published bolt11. Receive a payment for that invoice and confirm profile deletion while replacement invoice creation is still in progress. Expected: paid-invoice metadata is cleared, no replacement public endpoint is published before deletion finishes, and explicit cleanup still completes.
- Android, regtest wallet with private Paykit sharing and a saved contact invoice. After deletion cleanup has finished and while profile deletion is still running, settle that private invoice and, separately, a reserved on-chain receive. Expected: the payment is recorded and no private payment list is published before deletion ends.
Findings
- [MEDIUM] Recheck public invoice publication after deletion starts — inline at
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:2464. - [MEDIUM] Skip private endpoint rotation while profile deletion is in progress — inline at
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:2472.
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ⛔️ Request changes
The PR moved to eacdc26 after this run; these results are for bc00345.
Full review of 0801188...bc00345 (8 files), tested at bc00345. eacdc26 was pushed during the test run and is not covered here.
Findings:
- MEDIUM
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:855: an automatic public publication admitted just before deletion can land after sign-out's endpoint removal and stay published for the deleted profile (Test 6). - @piotr-iohk's two findings hold at bc00345 (public replacement, private rotation); Test 4 reproduces the first on a device.
- This PR widens the second: with contact sync paused,
removeSavedContacts(AppViewModel.kt:949) no longer runs afterdeleteAllContacts, so the deleted contacts stay inknownSavedContactKeysuntilcloseAndClear(PrivatePaykitRepo.kt:504). The rotation filters atPrivatePaykitRepo.kt:658and:684keep admitting them throughdeletePaykitProfileand sign-out.
- This PR widens the second: with contact sync paused,
QA for bc00345:
Tested on an Android 15 emulator (Pixel 7), dev build (regtest) with staging Pubky and Blocktank. Tests 2, 3, 4 and 6 ran on the head plus a test hook, not pushed and shown at the end: it logs each automatic Paykit admission and publication, and holds an operation until released. A hold only widens a window that already exists between an await and the SDK call; it fakes no result.
Test 1: delete-profile.xml ✅ Deletion shows progress, ends on onboarding, and the profile does not come back.
Head build, a profile with one saved contact. During deletion Delete Profile shows a spinner and Save stays dimmed with its label visible. Onboarding appears about 24 s after Yes, Delete, and Home, then Profile, shows onboarding again.
The 62-contact repeat was not driven: the author's two 62-contact runs in the description cover it, and bc00345 only renames the changelog fragment.
t1-delete-profile.2x.mp4 |
Test 2: admitted work finishes, nothing new starts during deletion ✅ The held publication and the deletion finish; no automatic work is admitted in between.
I held an automatic public publication before its SDK call, then confirmed deletion and held it right after it began. A background and foreground cycle, an on-chain receive and two minutes of polling ticks admitted no automatic work, as the hook log shows. After release, the held publication finished, explicit cleanup ran and deletion reached onboarding.
t2-admitted-publication.2x.mp4 |
09:27:36.726 auto public sync admitted
09:27:41.453 public apply start endpoints='[Bolt11=lnbcrt1p4dr5f2dqq…
09:27:41.453 hold 'public_apply' active
09:28:09.571 deletion begin
09:28:09.572 hold 'delete_after_begin' active
09:30:18.132 hold 'public_apply' released
09:30:26.877 public apply done failed='0'
09:30:31.403 hold 'delete_after_begin' released
09:30:31.406 explicit private cleanup start 'EditProfileViewModel' deleting=true
09:30:37.047 explicit private cleanup done 'EditProfileViewModel'
09:30:55.523 signout endpoint removal done success=false
09:30:57.088 deleteProfile result success=true
09:30:57.331 deletion end
Test 3: failed deletion after a public invoice settles ✅ The profile stays, automatic work resumes, and a new invoice is published.
I held deletion after explicit cleanup and paid the published bolt11; the hook log shows it cleared with publish=false. With the emulator offline, deletion failed ("Unable to Delete Profile", Retry or Disconnect), and Edit Profile and Profile still show the profile. In the log, automatic work resumes at once, and back online the app publishes a bolt11 with payment hash 08b6e779… in place of the paid 3a4f5200….
t3-failed-deletion-resume.2x.mp4 |
09:49:18.236 deletion begin
09:49:18.239 explicit private cleanup start 'EditProfileViewModel' deleting=true
09:49:23.430 explicit private cleanup done 'EditProfileViewModel'
09:49:23.464 hold 'delete_after_cleanup' active
09:49:30.811 payment received hash='3a4f5200…' deleting=true
09:49:30.818 paid public bolt11 cleared hash='3a4f5200…' publish=false
09:49:41.389 hold 'delete_after_cleanup' released
09:49:41.664 deleteProfile result success=false
09:49:41.669 deletion end
09:49:43.326 auto public sync admitted
09:50:14.604 auto public sync admitted
09:50:25.271 public apply start endpoints='[Bolt11=lnbcrt1p4dr4ehdqq…
09:50:34.938 public apply done failed='0'
republished cachedHash='08b6e779…
Test 4: replacement invoice started before deletion ❌ The replacement publishes while deletion is running.
This is @piotr-iohk's suggested test. I paid the published bolt11 and held the replacement after publish=true was sampled, then confirmed deletion and held it after explicit cleanup. On release, the hook log shows the replacement bolt11 and the on-chain address published about 15 s into deletion, after explicit cleanup and before sign-out.
t4-replacement-during-deletion.2x.mp4 |
09:45:43.364 payment received hash='d44a2090…' deleting=false
09:45:43.390 paid public bolt11 cleared hash='d44a2090…' publish=true
09:45:43.390 hold 'bolt11_replace' active
09:46:18.920 deletion begin
09:46:18.922 explicit private cleanup start 'EditProfileViewModel' deleting=true
09:46:25.138 explicit private cleanup done 'EditProfileViewModel'
09:46:25.203 hold 'delete_after_cleanup' active
09:46:32.972 hold 'bolt11_replace' released
09:46:33.524 public apply start endpoints='[Bolt11=lnbcrt1p4dr4jedqq…
09:46:42.791 public apply done failed='0'
09:46:45.359 hold 'delete_after_cleanup' released
09:47:04.993 signout endpoint removal done success=false
09:47:07.092 deleteProfile result success=true
09:47:07.208 deletion end
Test 5: private invoice settled during deletion ⏭️ Not run: this PR moved to eacdc26, which changes this path, while the two wallets were being linked.
@piotr-iohk's second suggested test. The run on the new head picks it up.
Test 6: publication admitted before deletion ❌ The deleted profile keeps advertising payment endpoints.
I held an automatic public publication before its SDK call, confirmed deletion, and held sign-out between its endpoint removal and the session revoke. I released the publication into that gap; the hook log shows it succeeded, and deletion then completed.
From a second wallet, Add Contact on the deleted pubky offers Pay next to Save. Pay resolves the on-chain address the first wallet published (the second wallet is empty, so the toast reads "Insufficient Savings: more ₿ needed to pay this Bitcoin invoice"). After a deletion without holds, the same lookup offers only Save.
t6a-held-publication-after-removal.2x.mp4 |
t6b-lookup-after-deletion.2x.mp4 |
t6c-lookup-control.2x.mp4 |
09:33:50.577 public apply start endpoints='...P2wpkh=bcrt1qqh83p7clzwrr7rqs0rp59ygl3dgs2akcxp5zx6]'
09:33:50.577 hold 'public_apply' active
09:34:15.850 deletion begin
09:34:15.855 explicit private cleanup start 'EditProfileViewModel' deleting=true
09:34:21.535 explicit private cleanup done 'EditProfileViewModel'
09:34:38.269 signout endpoint removal done success=false
09:34:38.270 hold 'signout_after_removal' active
09:34:47.802 hold 'public_apply' released
09:34:58.980 public apply done failed='0'
09:35:02.010 hold 'signout_after_removal' released
09:35:02.900 deleteProfile result success=true
09:35:03.033 deletion end
09:38:50.703 [wallet B] Pay on the deleted pubky decoded OnChain address=bcrt1qqh83p7clzwrr7rqs0rp59ygl3dgs2akcxp5zx6
test hook
Not pushed. It logs automatic Paykit admissions, public and private publication, and deletion begin and end. It holds a step while a file exists: an automatic public publication before syncPublicEndpoints, the paid-invoice replacement after the publish check, deletion after begin or after explicit cleanup, and sign-out between its endpoint removal and the session revoke.
diff --git a/app/src/main/java/to/bitkit/TestHook.kt b/app/src/main/java/to/bitkit/TestHook.kt
new file mode 100644
index 000000000..e6c16d8ec
--- /dev/null
+++ b/app/src/main/java/to/bitkit/TestHook.kt
@@ -0,0 +1,23 @@
+package to.bitkit
+
+import android.util.Log
+import kotlinx.coroutines.delay
+import java.io.File
+
+// test hook: not pushed. Logs automatic Paykit work and holds an operation until the tester removes its file.
+object TestHook {
+ private const val DIR = "/data/data/to.bitkit.dev/files"
+
+ fun log(message: String) {
+ if (BuildConfig.DEBUG) Log.i("TestHook", message)
+ }
+
+ suspend fun hold(name: String) {
+ if (!BuildConfig.DEBUG) return
+ val file = File("$DIR/testhook_hold_$name")
+ if (!file.exists()) return
+ log("hold '$name' active")
+ while (file.exists()) delay(250)
+ log("hold '$name' released")
+ }
+}
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index ca924ef82..589a0aee9 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -355,11 +355,13 @@ class PrivatePaykitRepo @Inject constructor(
suspend fun beginProfileDeletion() = withContext(serializedDispatcher) {
_isProfileDeletionInProgress.update { true }
+ to.bitkit.TestHook.log("deletion begin") // test hook
invalidateContactPreparation()
}
suspend fun endProfileDeletion() = withContext(serializedDispatcher) {
_isProfileDeletionInProgress.update { false }
+ to.bitkit.TestHook.log("deletion end") // test hook
}
suspend fun refreshSavedContactEndpoints(
@@ -486,10 +488,12 @@ class PrivatePaykitRepo @Inject constructor(
suspend fun removePublishedEndpointsForCleanup(context: String): Result<Unit> = withContext(serializedDispatcher) {
runSuspendCatching {
+ to.bitkit.TestHook.log("explicit private cleanup start '$context' deleting=$isDeletingProfile") // test hook
updateContactSharingCleanupPending(true)
removePublishedEndpoints().getOrThrow()
syncPaykitAppAfterCleanup().getOrThrow()
updateContactSharingCleanupPending(false)
+ to.bitkit.TestHook.log("explicit private cleanup done '$context'") // test hook
}.onFailure {
updateContactSharingCleanupPending(true)
Logger.warn("Failed to remove private Paykit endpoints during '$context'", it, context = TAG)
@@ -664,6 +668,7 @@ class PrivatePaykitRepo @Inject constructor(
if (matches.isEmpty()) return@runSuspendCatching
matches.forEach { (publicKey, paymentHash) -> rememberReceivedInvoicePaymentHash(paymentHash, publicKey) }
+ to.bitkit.TestHook.log("private invoice rotation matched '${matches.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
if (!canPublishPrivateEndpoints()) return@runSuspendCatching
publishLocalEndpoints(matches.map { it.first }.distinct(), reason = reason)
@@ -974,6 +979,7 @@ class PrivatePaykitRepo @Inject constructor(
runSuspendCatching {
val keys = publicKeys.mapNotNull { normalizedPublicKey(it) }.distinct()
if (keys.isEmpty()) return@runSuspendCatching
+ to.bitkit.TestHook.log("private publish start keys='${keys.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
val generation = preparationGeneration
val retry = currentCoroutineContext()[PrivateMessageDrainRetry]
val identity = retry?.identity ?: pubkyService.currentPublicKey()
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 93ed5efc3..83af9bc3b 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -1753,6 +1753,8 @@ class PubkyRepo @Inject constructor(
val hadPaykitState = settingsStore.data.first().hasPaykitState()
val endpointCleanupResult = removeBitkitPaymentEndpoints()
.onFailure { Logger.warn("Failed to remove Bitkit payment endpoints", it, context = TAG) }
+ to.bitkit.TestHook.log("signout endpoint removal done success=${endpointCleanupResult.isSuccess}") // test hook
+ to.bitkit.TestHook.hold("signout_after_removal") // test hook
val result = runSuspendCatching {
pubkyService.signOut()
diff --git a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
index fe37971fb..f34955a13 100644
--- a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
@@ -278,7 +278,9 @@ class PublicPaykitRepo @Inject constructor(
if (settings.publicPaykitBolt11PaymentHash != paymentHash) return@runSuspendCatching
clearPublicBolt11Metadata()
+ to.bitkit.TestHook.log("paid public bolt11 cleared hash='$paymentHash' publish=$publish") // test hook
if (!publish) return@runSuspendCatching
+ to.bitkit.TestHook.hold("bolt11_replace") // test hook
val desired = buildWalletEndpoints(refresh = true)
applyPublishedEndpoints(desired)
}
@@ -318,7 +320,10 @@ class PublicPaykitRepo @Inject constructor(
private suspend fun applyPublishedEndpoints(desiredEndpoints: List<Endpoint>) {
publishMutex.withLock {
requireCurrentPublicKey()
+ to.bitkit.TestHook.log("public apply start endpoints='${desiredEndpoints.map { "${it.methodId}=${it.value}" }}' cachedHash='${settingsStore.data.first().publicPaykitBolt11PaymentHash}'") // test hook
+ to.bitkit.TestHook.hold("public_apply") // test hook
val report = paykitSdkService.syncPublicEndpoints(desiredEndpoints)
+ to.bitkit.TestHook.log("public apply done failed='${report.failed.size}'") // test hook
report.failed.forEach { failure ->
Logger.warn(
"Failed to sync public Paykit endpoint '${failure.identifier}': ${failure.error}",
diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
index 4c5abbe74..9cc1999bc 100644
--- a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
+++ b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
@@ -269,8 +269,11 @@ class EditProfileViewModel @Inject constructor(
}
try {
privatePaykitRepo.beginProfileDeletion()
+ to.bitkit.TestHook.hold("delete_after_begin") // test hook
privatePaykitRepo.removePublishedEndpointsForCleanup(TAG)
+ to.bitkit.TestHook.hold("delete_after_cleanup") // test hook
val result = pubkyRepo.deleteProfileWithSessionRetry()
+ to.bitkit.TestHook.log("deleteProfile result success=${result.isSuccess}") // test hook
if (result.isSuccess) {
privatePaykitRepo.closeAndClear()
ToastEventBus.send(
diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
index d896e8a93..7f08bb8b8 100644
--- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
+++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
@@ -860,6 +860,7 @@ class AppViewModel @Inject constructor(
val onchainAddress = walletRepo.walletState.value.onchainAddress
if (onchainAddress.isBlank() && !lightningRepo.canReceive()) return
+ to.bitkit.TestHook.log("auto public sync admitted") // test hook
publicPaykitRepo.syncCurrentPublishedEndpoints(forceRefreshLightning = forceRefreshLightning)
.onFailure { Logger.warn("Failed to refresh public Paykit endpoints", it, context = TAG) }
}
@@ -887,6 +888,7 @@ class AppViewModel @Inject constructor(
}
private suspend fun synchronizePaykitContacts(state: PaykitContactSyncState) {
+ to.bitkit.TestHook.log("auto contact sync admitted signedIn=${state.publicKey != null} contacts='${state.contactKeys.size}'") // test hook
if (!state.isPaykitEnabled || state.publicKey == null) {
isPaymentRequestIdentityActivating = true
lastPrivatePaykitContactKeys = emptySet()
@@ -1108,6 +1110,7 @@ class AppViewModel @Inject constructor(
) {
return Result.failure(PaykitPaymentRequestError.RequestUnavailable)
}
+ to.bitkit.TestHook.log("auto request intake admitted mode='$mode'") // test hook
val result = if (forceFresh) {
paykitPaymentRequestRepo.refreshAfterStateChange(mode)
} else {
@@ -1139,6 +1142,7 @@ class AppViewModel @Inject constructor(
private suspend fun refreshPaymentRequestTargets(force: Boolean = false) {
if (privatePaykitRepo.isProfileDeletionInProgress.value) return
if (!isPaykitEnabled.value || pubkyRepo.publicKey.value == null || !walletRepo.walletExists()) return
+ to.bitkit.TestHook.log("auto request targets admitted") // test hook
paykitPaymentRequestRepo.refreshEligibleTargets(
savedPublicKeys = pubkyRepo.contacts.value.map { it.publicKey },
force = force,
@@ -1195,6 +1199,7 @@ class AppViewModel @Inject constructor(
private suspend fun refreshPaykitMaintenance() {
if (isPaykitEnabled.value && walletRepo.walletExists()) pubkyRepo.restoreSessionIfNeeded()
if (isAutomaticPaykitWorkPaused()) return
+ to.bitkit.TestHook.log("auto maintenance admitted") // test hook
pubkyRepo.republishIdentityIfNeeded()
retryPendingPaykitEndpointRemoval(
contactKeys = pubkyRepo.contacts.value.map { it.publicKey },
@@ -1236,6 +1241,7 @@ class AppViewModel @Inject constructor(
if (!isPaykitEnabled.value || isOnline.value != ConnectivityState.CONNECTED) return
if (pubkyRepo.publicKey.value == null || !walletRepo.walletExists()) return
shopContactImportJob = viewModelScope.launch {
+ to.bitkit.TestHook.log("auto shop import admitted") // test hook
pubkyRepo.importShopContacts()
}
}
@@ -1974,6 +1980,7 @@ class AppViewModel @Inject constructor(
private suspend fun retryPendingPaykitEndpointRemoval(contactKeys: Collection<String>, reason: String) {
if (privatePaykitRepo.isProfileDeletionInProgress.value) return
+ to.bitkit.TestHook.log("auto pending removal admitted reason='$reason'") // test hook
contactPaymentSettingsRepo.reconcilePendingEndpoints {
privatePaykitRepo.retryPendingEndpointRemoval(contactKeys)
.onFailure {
@@ -2459,6 +2466,7 @@ class AppViewModel @Inject constructor(
syncLdkPayments = false,
)
}
+ to.bitkit.TestHook.log("payment received hash='$paymentHash' deleting=${privatePaykitRepo.isProfileDeletionInProgress.value}") // test hook
publicPaykitRepo.refreshPublishedBolt11ForPayment(
paymentHash,
publish = !privatePaykitRepo.isProfileDeletionInProgress.value,Read, not driven: the private-only app registration gate, the pending-removal reconcile and the proof replay combine.
Pair PR synonymdev/bitkit-ios#954 at 1ae24c1 rechecks deletion right before public publication; this PR at bc00345 does not, and that gap is what Tests 4 and 6 hit.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up since bc00345c, reviewed at eacdc265, using the completed baseline review. Comparison base and merge base are still 0801188155dbbe996dfa14d3d9e8840f64b95b92. This pass covered the publication-admission fence, paid-invoice acknowledgement, private deletion rechecks, cleanup-pending ownership, the updated tests and journey notes, and parity with bitkit-ios#954 at f6cc033. Inherited baseline coverage is the unchanged Edit Profile deletion bracketing, the shop-import / request-intake / proof-refresh pause, and closeAndClear.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The late deletion checks address the earlier public and private publication comments on the previous head. Automatic public publication can still be dropped when its admission closure is created before session restore publishes the current sign-in. iOS reads the live session closure inside publication at the compared revision.
GitHub's build, lint, and detekt checks succeeded for this head. This review did not run the unit tests. Device testing: not performed. The iOS comparison is source-only and is not a review of that PR.
Suggested additional test cases
- Android. Public Paykit sharing is already enabled, a receive address is available, and session restore is still in progress with no public key. Let the debounced public refresh start, then finish restore with the saved identity. Expected: that identity's public endpoints are published without another address change, channel event, or payment. A sign-in that was current when refresh started, then replaced before publish, must not publish.
Findings
- [MEDIUM] Read the restored sign-in when admitting public publication — inline at
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:871.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up of the full PR at e8ade7db, since eacdc265 (previous review). Comparison base and merge base remain 0801188155dbbe996dfa14d3d9e8840f64b95b92. This pass inspected the SDK-lock admission recheck, startup publication after session restoration, the updated public, service, and view-model tests and profile journey note, and parity of that fence with bitkit-ios#954 at f6cc033. Inherited coverage is unchanged Edit Profile deletion bracketing, the shop-import / request-intake / proof-refresh pause, closeAndClear, private deletion publication fencing, and reconcile-pending cleanup retention.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The restored-sign-in admission added in this delta still publishes when contact payments are turned off during the restoration wait, and that publication can clear cleanup pending. The deletion flag is included in the callback now checked inside the SDK lock. The iOS comparison is source-only and is not a review of that PR.
build, lint, and detekt succeeded on this head. This review did not run the unit tests. Device testing: not performed in this review.
Suggested additional test cases
- Android. Public Paykit sharing is enabled, a receive address is available, and session restore is held with no public key. Let the debounced public refresh start, turn contact payments off, then release restore. Expected: endpoints stay unpublished, the switch remains off, and that refresh does not clear cleanup pending.
Findings
- [MEDIUM] Recheck public sharing after session restoration — inline at
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:862.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan at e8ade7d: one LOW inline. I found no HIGH or MEDIUM of my own.
I reviewed this as a funds and concurrency change (paid-invoice invalidation, publication admission, SDK lock).
Checked and clean:
- Paid public invoices:
refreshPublishedBolt11ForPaymentclears the paid invoice before any admission check (PublicPaykitRepo.kt:284). A deferred or failed replacement keepspublicPaykitCleanupPendingand is retried.acknowledgePublishedEndpointsclears pending only when the stored bolt11 equals the one just written, so an older admitted write cannot acknowledge a newer invalidation. On the private side,reusablePrivateInvoicerejects settled invoices throughisReceivedInvoiceSettled. - Cross-identity: the admission closes over
PubkySignIn(key + generation). It is rechecked after restore, after preparation, insidepublishMutex, inside the SDK lock and in the acknowledge transform.startSignInandclearAuthenticatedStatebump the generation. - Lifecycle:
endProfileDeletionruns infinallyunderNonCancellable, and the flag is aStateFlow, so the finalfalsealways reaches collectors. Back is blocked while busy (EditProfileScreen.kt:115). A failed deletion converges throughretryPendingPaykitEndpointRemoval. - Locks:
serializedDispatcherislimitedParallelism(1), so suspended work releases it. Lock order is the repo mutex, then the SDK operation lock. Nothing inside the SDK lock takes a repo mutex.completeSdkCallisNonCancellable.
The MEDIUM in @piotr-iohk's thread "recheck public sharing after session restoration" (AppViewModel :862) still holds at e8ade7d. sharesPublicPaykitEndpoints is read once before isRestoringSession.first { !it }. After that, neither the admission lambda nor syncCurrentPublishedEndpoints / applyPublishedEndpoints / acknowledgePublishedEndpoints re-read it. ContactPaymentSettingsRepo.disable() resets the per-method toggles to true, so endpoints are still built and published. Approval waits on that thread.
Performance-relevant: pause and resume of background polling and publication, and the SDK lock during deletion. To measure: Yes-Delete → onboarding with the 62-contact fixture against master, automatic admissions logged between deletion begin and end (expected zero), and the resume burst after a failed deletion (expected one publication and one contact pass).
Device gate (partial): e8ade7d — not run yet. Pending: journeys/profile/delete-profile.xml with the 62-contact fixture, timed against master, plus the failed-deletion resume count. The device result will follow.
The LOW can be fixed here or moved to a follow-up issue. Reply on the thread with which one.
|
Device gate (partial): Emulator
* Master sample 1 ran with With 0 contacts the PR is on par with master: 49.8–54.6 s against 51.4 s on the clean master run. About 38 s of each run sits between "deleted all contacts" and the session sign-out on both builds. The work during the deletion window is lighter on the PR:
No ANR or OOM. The only WARN/ERROR in the window on all four runs is Loading state, identical on both builds:
The deletion waits ~50 s with only that small inline spinner. That is pre-existing and outside this PR. |
There was a problem hiding this comment.
Verdict:
Delta review of bc00345...f22714b (4 production files, the tests and the profile journey notes), tested at f22714b.
Findings:
- LOW
app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt:360: a refresh already waiting insyncPaykitApp()can republish a paid invoice after its replacement is acknowledged, and nothing stays pending, so contacts paying from the public endpoint fail until the next foreground refresh. The race predates this PR; the new acknowledge, which keeps a replacement pending against older writes, covers only the other ordering (reproduction and a one-line change inline).
Earlier findings, checked at f22714b:
- Mine (publication after sign-out's removal): fixed. Test 1 replays the bc00345 sequence; the publication is now rejected inside the SDK lock and the deleted pubky offers nothing to pay.
- @piotr-iohk's four:
- public replacement: fixed (Test 3).
- private rotation: fixed (Test 4).
- restored sign-in: fixed (Test 8).
- sharing after restoration: fixed at SDK admission and for an admitted write (Test 5); the restore-held variant is covered by
PublicPaykitSharingAdmissionTest.ktonly.
- @jvsena42's forced Lightning refresh LOW still holds at f22714b.
QA for f22714b:
Android 15 emulators (Pixel 7), dev build (regtest) with staging Pubky and Blocktank. Wallet A deletes its profile; wallet B looks up A's pubky in Add Contact, where Pay appears only while A advertises payment endpoints. Tests 1 to 8 ran on the head plus a test hook, not pushed and shown at the end: it logs each automatic admission, each publication rejection point and each acknowledge, and holds a step until released. A hold only widens a window that already exists; it fakes no result.
Test 1: publication queued before the SDK lock ✅ It is rejected inside the lock, and the deleted pubky offers nothing to pay.
I held an automatic public publication after both repository checks, confirmed deletion, and held sign-out between its endpoint removal and the session revoke. I released the publication into that gap: the log shows it rejected inside the SDK lock, and deletion completed. Wallet B's preview of the deleted pubky then offers only Save. At bc00345 the same sequence published, and Pay resolved A's address.
t1a-publication-rejected-in-lock.2x.mp4 |
t1b-lookup-after-deletion.2x.mp4 |
12:28:49.928 public apply start endpoints='[Bolt11=lnbcrt1p4drlp0dqq…, P2wpkh=bcrt1qkr8ggk…]' cachedHash='87eafdc1…' pending=false
12:28:49.928 hold 'public_apply' active
12:33:15.237 deletion begin
12:33:15.240 explicit private cleanup start 'EditProfileViewModel' deleting=true
12:33:21.353 explicit private cleanup done 'EditProfileViewModel'
12:33:25.003 sdk public write admitted in lock endpoints='0'
12:33:40.279 signout endpoint removal done success=false
12:33:40.279 hold 'signout_after_removal' active
12:33:57.658 hold 'public_apply' released
12:33:58.332 sdk public write rejected in lock endpoints='2'
12:33:58.333 public apply done endpoints='2' result='rejected-in-sdk-lock'
12:34:03.095 hold 'signout_after_removal' released
12:34:04.064 deleteProfile result success=true
12:34:04.485 deletion end
Test 2: write admitted before deletion ✅ It finishes first, nothing new starts, and nothing stays published.
Control first: with QATHREE live, B's preview offers Pay and Save. I then held a public write inside the SDK lock and confirmed deletion. Explicit cleanup waited on the held write for 2 minutes. A background and foreground cycle and an on-chain receive admitted no automatic work, as the log shows. After release, the write finished, then cleanup, sign-out's removal and the end of deletion ran. B's preview then offers only Save.
t2a-lookup-control-live-profile.2x.mp4 |
t2b-admitted-write-during-deletion.2x.mp4 |
t2c-lookup-after-deletion.2x.mp4 |
12:39:02.192 public apply start endpoints='[Bolt11=lnbcrt1p4drlnsdqq…, P2wpkh=bcrt1qkr8ggk…]' cachedHash='cbebc5fe…' pending=false
12:39:02.684 hold 'sdk_write' active
12:39:30.657 deletion begin
12:39:30.659 explicit private cleanup start 'EditProfileViewModel' deleting=true
12:41:31.181 hold 'sdk_write' released
12:41:37.076 public apply done endpoints='2' result='failed=0'
12:41:37.082 public acknowledge published='lnbcrt1p4drlnsdqq…' stored='lnbcrt1p4drlnsdqq…' sharing=true pending=false
12:41:44.533 explicit private cleanup done 'EditProfileViewModel'
12:41:49.168 sdk public write admitted in lock endpoints='0'
12:42:06.488 signout endpoint removal done success=false
12:42:09.069 deleteProfile result success=true
12:42:09.078 deletion end
Test 3: paid public invoice, replacement released during deletion ✅ The replacement is rejected; only sign-out's removal is written.
@piotr-iohk's first suggested test. I paid the published bolt11 with the replacement held after its admission check, before invoice creation; the log shows the paid invoice cleared with pending set. A maintenance tick republished 10 s later and cleared pending; that is the retry this PR adds. I then confirmed deletion, held it after explicit cleanup, and released the replacement: it was rejected under the publish mutex. Between deletion begin and end, the only public write is sign-out's empty removal.
t3-paid-invoice-during-deletion.2x.mp4 |
12:51:13.997 payment received hash='e576b6d7…' deleting=false
12:51:14.014 paid public bolt11 cleared hash='e576b6d7…' pending=true
12:51:14.027 hold 'bolt11_replace' active
12:51:29.085 public apply start endpoints='[Bolt11=lnbcrt1p4dyqdvdqq…, P2wpkh=bcrt1qh7clpy…]' cachedHash='51ad7a60…' pending=true
12:51:29.124 sdk public write admitted in lock endpoints='2'
12:51:36.841 public apply done endpoints='2' result='failed=0'
12:51:36.850 public acknowledge published='lnbcrt1p4dyqdvdqq…' stored='lnbcrt1p4dyqdvdqq…' sharing=true pending=false
12:51:51.433 deletion begin
12:51:51.691 explicit private cleanup start 'EditProfileViewModel' deleting=true
12:51:58.346 explicit private cleanup done 'EditProfileViewModel'
12:51:58.524 hold 'delete_after_cleanup' active
12:52:04.992 hold 'bolt11_replace' released
12:52:05.018 public apply rejected 'under-mutex' endpoints='2'
12:52:11.321 hold 'delete_after_cleanup' released
12:52:14.287 sdk public write admitted in lock endpoints='0'
12:52:29.159 signout endpoint removal done success=false
12:52:30.990 deleteProfile result success=true
12:52:31.197 deletion end
Test 4: private invoice and on-chain receive during deletion ✅ Both payments are recorded, attribution is kept, and nothing is rotated.
@piotr-iohk's second suggested test, with A and B saved as each other's contacts. Control: before deletion, paying A's private invoice for B rotated it and published a new one. With deletion held right after it began, the next private invoice payment matched contact B and rotation was skipped. The activity detail shows CONTACT pubky…x771y. With deletion held after explicit cleanup, an on-chain receive to B's reserved address matched B and rotation was skipped each time. Both payments show in A's activity, and no private publication started during either deletion.
A private invoice paid after explicit cleanup no longer matches B: cleanup clears each contact's local invoice, which this PR does not change. The second recording covers the invoice half before cleanup.
t4a-private-and-onchain-after-cleanup.2x.mp4 |
t4b-private-invoice-before-cleanup.2x.mp4 |
13:02:21.437 payment received hash='3946a56c…' deleting=false
13:02:21.450 private invoice rotation matched '1' reason='invoice rotation' deleting=false
13:02:22.105 private publish start keys='1' reason='invoice rotation' deleting=false
13:02:23.915 private local invoice set key='pubky9p989ks' hash='ea562656…' bolt11='lnbcrt1p4dyppldqq…'
13:02:24.460 private sync start updates='1' reason='invoice rotation' deleting=false
13:02:31.603 private sync done reason='invoice rotation'
…
13:05:32.851 deletion begin
13:05:32.915 explicit private cleanup start 'EditProfileViewModel' deleting=true
13:05:52.993 explicit private cleanup done 'EditProfileViewModel'
13:05:53.337 hold 'delete_after_cleanup' active
13:06:04.925 payment received hash='ea562656…' deleting=true
13:07:20.620 private onchain rotation matched '1' deleting=true
13:07:20.620 private onchain rotation skipped deleting=true
13:07:28.101 private onchain rotation matched '1' deleting=true
13:07:28.101 private onchain rotation skipped deleting=true
13:07:48.139 hold 'delete_after_cleanup' released
13:07:55.677 sdk public write admitted in lock endpoints='0'
13:08:16.558 signout endpoint removal done success=false
13:08:20.193 deleteProfile result success=true
13:08:20.560 deletion end
13:14:31.141 deletion begin
13:14:31.148 hold 'delete_after_begin' active
13:14:33.007 payment received hash='61784076…' deleting=true
13:14:33.248 private invoice rotation matched '1' reason='invoice rotation' deleting=true
13:14:33.248 private invoice rotation skipped deleting=true
13:14:37.474 private invoice rotation matched '1' reason='invoice reconciliation' deleting=true
13:14:37.475 private invoice rotation skipped deleting=true
13:14:45.388 hold 'delete_after_begin' released
13:14:45.388 explicit private cleanup start 'EditProfileViewModel' deleting=true
13:14:47.686 private invoice rotation matched '1' reason='invoice reconciliation' deleting=true
13:15:18.317 sdk public write admitted in lock endpoints='0'
13:15:39.548 signout endpoint removal done success=false
13:15:45.002 deleteProfile result success=true
13:15:45.595 deletion end
Test 5: contact payments turned off while a publication waits ✅ No write lands after the switch, and the removal runs last.
@piotr-iohk's third suggested test, at SDK admission. With a publication held before the SDK lock, I turned Enable contact payments off: on release it was rejected inside the lock, and the empty removal ran. With a write held inside the lock, I turned it off again: the write finished, its acknowledge ran with sharing off and changed nothing, then the removal ran.
The restore-held variant cannot be driven from the UI: while the cold-start restore is held, Settings has no contact payments row (Test 8's recording shows it).
t5-sharing-off-while-publishing.2x.mp4 |
13:19:23.409 public apply start endpoints='[Bolt11=lnbcrt1p4dyplddqq…, P2wpkh=bcrt1qh7clpy…]' cachedHash='514c5408…' pending=false
13:19:23.409 hold 'public_apply' active
13:19:51.186 hold 'public_apply' released
13:19:51.219 sdk public write rejected in lock endpoints='2'
13:19:51.220 public apply done endpoints='2' result='rejected-in-sdk-lock'
13:19:51.221 public apply start endpoints='[]' cachedHash='514c5408…' pending=true
13:19:51.249 sdk public write admitted in lock endpoints='0'
13:19:57.779 public apply done endpoints='0' result='failed=0'
13:20:19.453 public apply start endpoints='[]' cachedHash='' pending=true
13:20:19.923 sdk public write admitted in lock endpoints='0'
13:20:23.177 public apply done endpoints='0' result='failed=0'
13:21:20.620 public apply start endpoints='[]' cachedHash='' pending=true
13:21:21.632 sdk public write admitted in lock endpoints='0'
13:21:24.809 public apply done endpoints='0' result='failed=0'
13:21:36.636 public apply start endpoints='[Bolt11=lnbcrt1p4dyz96dqq…, P2wpkh=bcrt1qh7clpy…]' cachedHash='5dd1fb6a…' pending=true
13:21:37.741 sdk public write admitted in lock endpoints='2'
13:21:46.060 public apply done endpoints='2' result='failed=0'
13:21:46.077 public acknowledge published='lnbcrt1p4dyz96dqq…' stored='lnbcrt1p4dyz96dqq…' sharing=true pending=false
13:22:15.087 auto pending removal admitted reason='foreground'
13:22:16.940 auto public sync admitted signIn=true restoring=false
13:22:16.940 auto public sync after restore signIn=true admission=true
13:22:23.208 public apply start endpoints='[Bolt11=lnbcrt1p4dyz96dqq…, P2wpkh=bcrt1qh7clpy…]' cachedHash='5dd1fb6a…' pending=false
13:22:23.902 sdk public write admitted in lock endpoints='2'
13:22:23.902 hold 'sdk_write' active
13:22:35.807 hold 'sdk_write' released
13:22:40.556 public apply done endpoints='2' result='failed=0'
13:22:40.560 public acknowledge published='lnbcrt1p4dyz96dqq…' stored='lnbcrt1p4dyz96dqq…' sharing=false pending=false
13:22:46.215 public apply start endpoints='[]' cachedHash='5dd1fb6a…' pending=true
13:22:47.452 sdk public write admitted in lock endpoints='0'
13:22:54.855 public apply done endpoints='0' result='failed=0'
Test 6: failed deletion after a paid invoice, first resumed publication fails ✅ The profile stays and a new invoice is published without another payment.
I held deletion after explicit cleanup and paid the published bolt11: it cleared with pending set, and the replacement was rejected. Offline, I released deletion: "Unable to Delete Profile" (Retry or Disconnect). The resumed publication for the kept identity failed at the Paykit app sync, and pending stayed set. Back online, with no foreground or payment, the pending reconcile published a bolt11 with payment hash ebffdb61… in place of the paid 7881b491…, and cleared pending. Profile shows QASEVEN.
t6-failed-deletion-retry.2x.mp4 |
13:33:52.832 deletion begin
13:33:53.586 explicit private cleanup start 'EditProfileViewModel' deleting=true
13:34:05.330 explicit private cleanup done 'EditProfileViewModel'
13:34:06.073 hold 'delete_after_cleanup' active
13:34:13.335 payment received hash='7881b491…' deleting=true
13:34:13.557 paid public bolt11 cleared hash='7881b491…' pending=true
13:34:13.557 paid bolt11 replacement rejected at admission 'after-clear'
13:34:30.401 hold 'delete_after_cleanup' released
13:34:35.484 deleteProfile result success=false
13:34:35.520 deletion end
13:34:37.015 auto contact sync admitted signedIn=true contacts='0'
13:34:39.158 auto public sync admitted signIn=true restoring=false
13:34:39.159 auto public sync after restore signIn=true admission=true
13:35:26.159 auto pending removal admitted reason='network restored'
13:35:44.244 public apply start endpoints='[Bolt11=lnbcrt1p4dyrqzdqq…, P2wpkh=bcrt1qh7clpy…]' cachedHash='ebffdb61…' pending=true
13:35:45.371 sdk public write admitted in lock endpoints='2'
13:35:55.508 public apply done endpoints='2' result='failed=0'
13:35:55.837 public acknowledge published='lnbcrt1p4dyrqzdqq…' stored='lnbcrt1p4dyrqzdqq…' sharing=true pending=false
Test 7: older write against a newer paid-invoice invalidation ✅ The older write keeps the replacement pending.
I held a write of invoice bb44eac3… inside the SDK lock, paid that invoice and held its replacement. The older write finished, and its acknowledge found the stored invoice already cleared and kept pending. The replacement then published 7881b491… and cleared pending.
t7-older-write-newer-invalidation.2x.mp4 |
13:31:21.829 hold 'sdk_write' active
13:31:25.319 payment received hash='bb44eac3…' deleting=false
13:31:25.326 paid public bolt11 cleared hash='bb44eac3…' pending=true
13:31:25.327 hold 'bolt11_replace' active
13:31:38.864 hold 'sdk_write' released
13:31:44.195 public apply done endpoints='2' result='failed=0'
13:31:44.199 public acknowledge published='lnbcrt1p4dyz4cdqq…' stored='' sharing=true pending=true
13:31:46.503 hold 'bolt11_replace' released
13:31:47.642 public apply start endpoints='[Bolt11=lnbcrt1p4dyzerdqq…, P2wpkh=bcrt1qh7clpy…]' cachedHash='7881b491…' pending=true
13:31:48.884 sdk public write admitted in lock endpoints='2'
13:32:16.515 public apply done endpoints='2' result='failed=0'
13:32:18.921 public acknowledge published='lnbcrt1p4dyzerdqq…' stored='lnbcrt1p4dyzerdqq…' sharing=true pending=false
Test 8: startup publication after a held session restore ✅ The refreshes started before restore publish for the restored identity.
@piotr-iohk's restored sign-in test. I cold-started A with sharing on and the session restore held. Three automatic refreshes started with no sign-in. After release, each was admitted for the restored identity and the publication finished, with no address, channel or payment event in between. A sign-in replaced during the wait is covered by AppViewModelSendFlowTest.kt only.
t8-startup-after-held-restore.2x.mp4 |
13:28:45.585 session restore start publicKey=false
13:28:45.594 hold 'session_restore' active
13:28:49.471 auto contact sync admitted signedIn=false contacts='0'
13:28:55.155 auto pending removal admitted reason='foreground'
13:28:58.797 auto public sync admitted signIn=false restoring=true
13:29:01.086 auto public sync admitted signIn=false restoring=true
13:29:30.409 auto public sync admitted signIn=false restoring=true
13:29:51.981 hold 'session_restore' released
13:30:00.317 session restore end publicKey=true
13:30:00.366 auto contact sync admitted signedIn=true contacts='0'
13:30:00.620 auto public sync after restore signIn=true admission=true
13:30:00.621 auto public sync after restore signIn=true admission=true
13:30:00.621 auto public sync after restore signIn=true admission=true
13:30:01.383 auto contact sync admitted signedIn=true contacts='0'
13:30:06.361 public apply start endpoints='[Bolt11=lnbcrt1p4dyzjgdqq…, P2wpkh=bcrt1qh7clpy…]' cachedHash='bb44eac3…' pending=false
13:30:17.751 sdk public write admitted in lock endpoints='2'
13:30:22.107 public apply done endpoints='2' result='failed=0'
Test 9: delete-profile.xml ✅ Deletion shows progress, ends on onboarding, and the profile does not come back.
Plain head build, profile QAONE with one saved contact. Delete Profile shows a spinner, and Cancel and Save are dimmed with their labels visible. "Profile deleted" and onboarding appear about 30 s after Yes, Delete. Home, then Profile, shows onboarding again.
t9-delete-profile.2x.mp4 |
Test 10: delete-profile.xml with 62 imported contacts ⏭️ Not run: Import All needs a Ring identity following 62 profiles.
@jvsena42's device gate times 0-contact deletion at f22714b against master, and his 62-contact runs are still going.
test hook
Not pushed. It logs automatic Paykit admissions, every public publication rejection point, each acknowledge with the pending flag, private rotation, session restore, and deletion begin and end. It holds a step while a file exists: a publication after both repository checks and before the SDK lock, a write inside the SDK lock, the paid-invoice replacement before invoice creation, the cold-start session restore, deletion after begin or after explicit cleanup, and sign-out between its endpoint removal and the session revoke.
diff --git a/app/src/main/java/to/bitkit/TestHook.kt b/app/src/main/java/to/bitkit/TestHook.kt
new file mode 100644
index 000000000..e6c16d8ec
--- /dev/null
+++ b/app/src/main/java/to/bitkit/TestHook.kt
@@ -0,0 +1,23 @@
+package to.bitkit
+
+import android.util.Log
+import kotlinx.coroutines.delay
+import java.io.File
+
+// test hook: not pushed. Logs automatic Paykit work and holds an operation until the tester removes its file.
+object TestHook {
+ private const val DIR = "/data/data/to.bitkit.dev/files"
+
+ fun log(message: String) {
+ if (BuildConfig.DEBUG) Log.i("TestHook", message)
+ }
+
+ suspend fun hold(name: String) {
+ if (!BuildConfig.DEBUG) return
+ val file = File("$DIR/testhook_hold_$name")
+ if (!file.exists()) return
+ log("hold '$name' active")
+ while (file.exists()) delay(250)
+ log("hold '$name' released")
+ }
+}
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index fe4ab94e2..33f39b4e2 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -361,11 +361,13 @@ class PrivatePaykitRepo @Inject constructor(
suspend fun beginProfileDeletion() = withContext(serializedDispatcher) {
_isProfileDeletionInProgress.update { true }
+ to.bitkit.TestHook.log("deletion begin") // test hook
invalidateContactPreparation()
}
suspend fun endProfileDeletion() = withContext(serializedDispatcher) {
_isProfileDeletionInProgress.update { false }
+ to.bitkit.TestHook.log("deletion end") // test hook
}
suspend fun refreshSavedContactEndpoints(
@@ -492,10 +494,12 @@ class PrivatePaykitRepo @Inject constructor(
suspend fun removePublishedEndpointsForCleanup(context: String): Result<Unit> = withContext(serializedDispatcher) {
runSuspendCatching {
+ to.bitkit.TestHook.log("explicit private cleanup start '$context' deleting=$isDeletingProfile") // test hook
updateContactSharingCleanupPending(true)
removePublishedEndpoints().getOrThrow()
syncPaykitAppAfterCleanup().getOrThrow()
updateContactSharingCleanupPending(false)
+ to.bitkit.TestHook.log("explicit private cleanup done '$context'") // test hook
}.onFailure {
updateContactSharingCleanupPending(true)
Logger.warn("Failed to remove private Paykit endpoints during '$context'", it, context = TAG)
@@ -670,7 +674,11 @@ class PrivatePaykitRepo @Inject constructor(
if (matches.isEmpty()) return@runSuspendCatching
matches.forEach { (publicKey, paymentHash) -> rememberReceivedInvoicePaymentHash(paymentHash, publicKey) }
- if (!canPublishPrivateEndpoints()) return@runSuspendCatching
+ to.bitkit.TestHook.log("private invoice rotation matched '${matches.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
+ if (!canPublishPrivateEndpoints()) {
+ to.bitkit.TestHook.log("private invoice rotation skipped deleting=$isDeletingProfile") // test hook
+ return@runSuspendCatching
+ }
publishLocalEndpoints(matches.map { it.first }.distinct(), reason = reason)
.onFailure { Logger.warn("Failed to rotate private Paykit invoice", it, context = TAG) }
@@ -689,7 +697,11 @@ class PrivatePaykitRepo @Inject constructor(
}
}.filter { it in knownSavedContactKeys }.distinct()
if (publicKeys.isEmpty()) return@runSuspendCatching
- if (!canPublishPrivateEndpoints()) return@runSuspendCatching
+ to.bitkit.TestHook.log("private onchain rotation matched '${publicKeys.size}' deleting=$isDeletingProfile") // test hook
+ if (!canPublishPrivateEndpoints()) {
+ to.bitkit.TestHook.log("private onchain rotation skipped deleting=$isDeletingProfile") // test hook
+ return@runSuspendCatching
+ }
publishLocalEndpoints(publicKeys, reason = "on-chain rotation").getOrThrow()
}
@@ -980,6 +992,7 @@ class PrivatePaykitRepo @Inject constructor(
runSuspendCatching {
val keys = publicKeys.mapNotNull { normalizedPublicKey(it) }.distinct()
if (keys.isEmpty()) return@runSuspendCatching
+ to.bitkit.TestHook.log("private publish start keys='${keys.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
val generation = preparationGeneration
val retry = currentCoroutineContext()[PrivateMessageDrainRetry]
val identity = retry?.identity ?: pubkyService.currentPublicKey()
@@ -1018,10 +1031,12 @@ class PrivatePaykitRepo @Inject constructor(
return@withLock
}
+ to.bitkit.TestHook.log("private sync start updates='${publication.updates.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
val report = paykitSdkService.syncPrivatePaymentListsWithReservations(
updates = publication.updates,
clearUnlistedLinkedPeers = false,
)
+ to.bitkit.TestHook.log("private sync done reason='$reason'") // test hook
val deliveryError = applyPrivatePaymentListDeliveryReport(report, reason, publicationWindow)
val firstError = publication.firstError ?: deliveryError
val retryKeys = (links.linkRetryKeys + privatePaymentListDeliveryRetryKeys(report)).distinct()
@@ -2228,6 +2243,7 @@ class PrivatePaykitRepo @Inject constructor(
private suspend fun setLocalInvoice(publicKey: String, invoice: StoredInvoice) {
val contactState = ensureState().contacts.getOrPut(publicKey) { ContactState() }
contactState.localInvoice = invoice
+ to.bitkit.TestHook.log("private local invoice set key='${publicKey.take(12)}' hash='${invoice.paymentHash}' bolt11='${invoice.bolt11}'") // test hook
}
private fun rememberSavedContacts(publicKeys: Collection<String>, replacing: Boolean): List<String> {
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 93ed5efc3..db3a87975 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -437,6 +437,8 @@ class PubkyRepo @Inject constructor(
private suspend fun initializeSession(notifyFailure: Boolean = true): Boolean {
_isRestoringSession.update { true }
try {
+ to.bitkit.TestHook.log("session restore start publicKey=${_publicKey.value != null}") // test hook
+ to.bitkit.TestHook.hold("session_restore") // test hook
val savedSession = runSuspendCatching { keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) }
val importedSession = runSuspendCatching {
ensureServiceInitialized(savedSession.getOrNull())
@@ -491,6 +493,7 @@ class PubkyRepo @Inject constructor(
return result is InitResult.Restored
} finally {
_isRestoringSession.update { false }
+ to.bitkit.TestHook.log("session restore end publicKey=${_publicKey.value != null}") // test hook
}
}
@@ -1753,6 +1756,8 @@ class PubkyRepo @Inject constructor(
val hadPaykitState = settingsStore.data.first().hasPaykitState()
val endpointCleanupResult = removeBitkitPaymentEndpoints()
.onFailure { Logger.warn("Failed to remove Bitkit payment endpoints", it, context = TAG) }
+ to.bitkit.TestHook.log("signout endpoint removal done success=${endpointCleanupResult.isSuccess}") // test hook
+ to.bitkit.TestHook.hold("signout_after_removal") // test hook
val result = runSuspendCatching {
pubkyService.signOut()
diff --git a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
index 864e78daf..8031d2696 100644
--- a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
@@ -258,13 +258,19 @@ class PublicPaykitRepo @Inject constructor(
): Result<Unit> = withContext(ioDispatcher) {
runSuspendCatching {
pubkyRepo.isRestoringSession.first { !it }
- if (!isAutomaticPublicationAllowed(canPublish)) return@runSuspendCatching
+ if (!isAutomaticPublicationAllowed(canPublish)) {
+ to.bitkit.TestHook.log("public sync rejected at admission 'after-restore'") // test hook
+ return@runSuspendCatching
+ }
val desired = buildWalletEndpoints(
refresh = false,
forceRefreshLightning = forceRefreshLightning,
requireEndpoint = requireEndpoint,
)
- if (!isAutomaticPublicationAllowed(canPublish)) return@runSuspendCatching
+ if (!isAutomaticPublicationAllowed(canPublish)) {
+ to.bitkit.TestHook.log("public sync rejected at admission 'after-preparation'") // test hook
+ return@runSuspendCatching
+ }
syncPaykitApp().getOrThrow()
if (applyPublishedEndpoints(desired) { isAutomaticPublicationAllowed(canPublish) }) {
acknowledgePublishedEndpoints(desired, canPublish)
@@ -282,7 +288,12 @@ class PublicPaykitRepo @Inject constructor(
if (settings.publicPaykitBolt11PaymentHash != paymentHash) return@runSuspendCatching
clearPublicBolt11Metadata(publicationPending = true)
- if (!isAutomaticPublicationAllowed(canPublish)) return@runSuspendCatching
+ to.bitkit.TestHook.log("paid public bolt11 cleared hash='$paymentHash' pending=${settingsStore.data.first().publicPaykitCleanupPending}") // test hook
+ if (!isAutomaticPublicationAllowed(canPublish)) {
+ to.bitkit.TestHook.log("paid bolt11 replacement rejected at admission 'after-clear'") // test hook
+ return@runSuspendCatching
+ }
+ to.bitkit.TestHook.hold("bolt11_replace") // test hook
val desired = buildWalletEndpoints(refresh = true)
if (applyPublishedEndpoints(desired) { isAutomaticPublicationAllowed(canPublish) }) {
acknowledgePublishedEndpoints(desired, canPublish)
@@ -329,10 +340,20 @@ class PublicPaykitRepo @Inject constructor(
canPublish: suspend () -> Boolean = { true },
): Boolean =
publishMutex.withLock {
- if (!canPublish()) return@withLock false
+ if (!canPublish()) {
+ to.bitkit.TestHook.log("public apply rejected 'under-mutex' endpoints='${desiredEndpoints.size}'") // test hook
+ return@withLock false
+ }
requireCurrentPublicKey()
- if (!canPublish()) return@withLock false
- val report = paykitSdkService.syncPublicEndpoints(desiredEndpoints, canPublish) ?: return@withLock false
+ if (!canPublish()) {
+ to.bitkit.TestHook.log("public apply rejected 'after-identity' endpoints='${desiredEndpoints.size}'") // test hook
+ return@withLock false
+ }
+ to.bitkit.TestHook.log("public apply start endpoints='${desiredEndpoints.map { "${it.methodId}=${it.value}" }}' cachedHash='${settingsStore.data.first().publicPaykitBolt11PaymentHash}' pending=${settingsStore.data.first().publicPaykitCleanupPending}") // test hook
+ if (desiredEndpoints.isNotEmpty()) to.bitkit.TestHook.hold("public_apply") // test hook
+ val report = paykitSdkService.syncPublicEndpoints(desiredEndpoints, canPublish)
+ to.bitkit.TestHook.log("public apply done endpoints='${desiredEndpoints.size}' result='${report?.let { "failed=${it.failed.size}" } ?: "rejected-in-sdk-lock"}'") // test hook
+ if (report == null) return@withLock false
report.failed.forEach { failure ->
Logger.warn(
"Failed to sync public Paykit endpoint '${failure.identifier}': ${failure.error}",
@@ -363,6 +384,9 @@ class PublicPaykitRepo @Inject constructor(
it
}
}
+ settingsStore.data.first().let { // test hook
+ to.bitkit.TestHook.log("public acknowledge published='${publishedInvoice.take(24)}' stored='${it.publicPaykitBolt11.take(24)}' sharing=${it.sharesPublicPaykitEndpoints} pending=${it.publicPaykitCleanupPending}") // test hook
+ } // test hook
}
private suspend fun buildWalletEndpoints(
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index b6beb1b02..2fbf13c5b 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -832,7 +832,12 @@ class PaykitSdkService @Inject constructor(
}
return operationLock.withLock(priority) {
withStateRevisionTracking { handle ->
- if (!canPublish()) return@withStateRevisionTracking null
+ if (!canPublish()) {
+ to.bitkit.TestHook.log("sdk public write rejected in lock endpoints='${endpoints.size}'") // test hook
+ return@withStateRevisionTracking null
+ }
+ to.bitkit.TestHook.log("sdk public write admitted in lock endpoints='${endpoints.size}'") // test hook
+ if (endpoints.isNotEmpty()) to.bitkit.TestHook.hold("sdk_write") // test hook
completeSdkCall {
handle.syncPublicEndpointsWithReceivingDetails(endpoints.map { it.toPublicReceivingDetail() })
}
diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
index 4c5abbe74..9cc1999bc 100644
--- a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
+++ b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
@@ -269,8 +269,11 @@ class EditProfileViewModel @Inject constructor(
}
try {
privatePaykitRepo.beginProfileDeletion()
+ to.bitkit.TestHook.hold("delete_after_begin") // test hook
privatePaykitRepo.removePublishedEndpointsForCleanup(TAG)
+ to.bitkit.TestHook.hold("delete_after_cleanup") // test hook
val result = pubkyRepo.deleteProfileWithSessionRetry()
+ to.bitkit.TestHook.log("deleteProfile result success=${result.isSuccess}") // test hook
if (result.isSuccess) {
privatePaykitRepo.closeAndClear()
ToastEventBus.send(
diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
index 80fa84a7c..6457be1c1 100644
--- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
+++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
@@ -858,8 +858,10 @@ class AppViewModel @Inject constructor(
return
}
+ to.bitkit.TestHook.log("auto public sync admitted signIn=${signIn != null} restoring=${pubkyRepo.isRestoringSession.value}") // test hook
pubkyRepo.isRestoringSession.first { !it }
val admission = publicPaykitPublicationAdmission(signIn ?: pubkyRepo.currentSignIn())
+ to.bitkit.TestHook.log("auto public sync after restore signIn=${(signIn ?: pubkyRepo.currentSignIn()) != null} admission=${admission()}") // test hook
val onchainAddress = walletRepo.walletState.value.onchainAddress
if (onchainAddress.isBlank() && !lightningRepo.canReceive()) return
@@ -899,6 +901,7 @@ class AppViewModel @Inject constructor(
}
private suspend fun synchronizePaykitContacts(state: PaykitContactSyncState) {
+ to.bitkit.TestHook.log("auto contact sync admitted signedIn=${state.publicKey != null} contacts='${state.contactKeys.size}'") // test hook
if (!state.isPaykitEnabled || state.publicKey == null) {
isPaymentRequestIdentityActivating = true
lastPrivatePaykitContactKeys = emptySet()
@@ -1120,6 +1123,7 @@ class AppViewModel @Inject constructor(
) {
return Result.failure(PaykitPaymentRequestError.RequestUnavailable)
}
+ to.bitkit.TestHook.log("auto request intake admitted mode='$mode'") // test hook
val result = if (forceFresh) {
paykitPaymentRequestRepo.refreshAfterStateChange(mode)
} else {
@@ -1151,6 +1155,7 @@ class AppViewModel @Inject constructor(
private suspend fun refreshPaymentRequestTargets(force: Boolean = false) {
if (privatePaykitRepo.isProfileDeletionInProgress.value) return
if (!isPaykitEnabled.value || pubkyRepo.publicKey.value == null || !walletRepo.walletExists()) return
+ to.bitkit.TestHook.log("auto request targets admitted") // test hook
paykitPaymentRequestRepo.refreshEligibleTargets(
savedPublicKeys = pubkyRepo.contacts.value.map { it.publicKey },
force = force,
@@ -1207,6 +1212,7 @@ class AppViewModel @Inject constructor(
private suspend fun refreshPaykitMaintenance() {
if (isPaykitEnabled.value && walletRepo.walletExists()) pubkyRepo.restoreSessionIfNeeded()
if (isAutomaticPaykitWorkPaused()) return
+ to.bitkit.TestHook.log("auto maintenance admitted") // test hook
pubkyRepo.republishIdentityIfNeeded()
retryPendingPaykitEndpointRemoval(
contactKeys = pubkyRepo.contacts.value.map { it.publicKey },
@@ -1248,6 +1254,7 @@ class AppViewModel @Inject constructor(
if (!isPaykitEnabled.value || isOnline.value != ConnectivityState.CONNECTED) return
if (pubkyRepo.publicKey.value == null || !walletRepo.walletExists()) return
shopContactImportJob = viewModelScope.launch {
+ to.bitkit.TestHook.log("auto shop import admitted") // test hook
pubkyRepo.importShopContacts()
}
}
@@ -1986,6 +1993,7 @@ class AppViewModel @Inject constructor(
private suspend fun retryPendingPaykitEndpointRemoval(contactKeys: Collection<String>, reason: String) {
if (privatePaykitRepo.isProfileDeletionInProgress.value) return
+ to.bitkit.TestHook.log("auto pending removal admitted reason='$reason'") // test hook
contactPaymentSettingsRepo.reconcilePendingEndpoints {
privatePaykitRepo.retryPendingEndpointRemoval(contactKeys)
.onFailure {
@@ -2473,6 +2481,7 @@ class AppViewModel @Inject constructor(
syncLdkPayments = false,
)
}
+ to.bitkit.TestHook.log("payment received hash='$paymentHash' deleting=${privatePaykitRepo.isProfileDeletionInProgress.value}") // test hook
publicPaykitRepo.refreshPublishedBolt11ForPayment(
paymentHash,
canPublish = publicPaykitPublicationAdmission(),Reviewed: bc00345...f22714b. Not driven on a device: sharing turned off during a held restore (no toggle while restoring), the 62-contact repeats (Test 10), and the race in the LOW (unit test only). iOS twin synonymdev/bitkit-ios#954 at d7e0e1f matches: the same deletion, session and sharing rechecks before public publication, and its post-publish pending clear has the same gap as the LOW.
|
Device gate (partial):
The settled run spends 300 s in one Paykit safety wait. A shared-state write during endpoint cleanup could not be confirmed, so the SDK held its shared-state lock for the full uncertain-write cooldown before sign-out could run: Log lines (device time, UTC−3)The deletion still completed: the endpoint-removal failure is tolerated, the session is revoked and onboarding shows. During the whole wait the only feedback is the small spinner on the Delete Profile button. The wait is the SDK's uncertain-write safety path, which the PR body lists as unchanged. Master's "right after import" run hit no cooldown and no |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up of the full PR at 71d9ef7, since e8ade7d (previous review). Comparison base and merge base remain 0801188155dbbe996dfa14d3d9e8840f64b95b92. This pass inspected the two later commits: sharing is rechecked across restoration, preparation, and SDK admission, and deferred Lightning refreshes are retained for an existing session. Inherited coverage is unchanged Edit Profile deletion bracketing, the shop-import / request-intake / proof-refresh pause, closeAndClear, private deletion publication fencing, and reconcile-pending cleanup retention.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The sharing-during-restore publication from that previous review does not reproduce here. Automatic publication reads the sharing flag again after restoration and inside the SDK lock, and acknowledgement leaves cleanup pending when sharing is already off. Private retention matches the new tests: a failed queue keeps the peer, a queued bolt11 consumes only that peer, and a replaced sign-in does not inherit the refresh. Public retention does not. A channel event that starts before a public key exists never records a pending refresh, so a skipped or failed attempt after restore is not retried by maintenance.
bitkit-ios#954 at d7e0e1f also rechecks sharing and the captured session before automatic publication. It does not retain a deferred Lightning refresh; its batch force flag is still cleared when preparation starts. That iOS revision also retains a contact link across session recovery. Android still cancels that scan on sign-out, and this delta does not change it.
build and lint succeeded on this head. This review inspected the new unit tests and did not run them. The settings local e2e job failed; its log was not available here, so it is not evidence for or against this Paykit change. Device testing: not performed in this review.
Suggested additional test cases
- Android. Public contact payments are on, a cached public bolt11 exists, and session restore is held with no public key. Make a Lightning channel usable, or run the delayed channel-ready refresh, before the public key appears. Finish restore and skip or fail that first publication. Expected: the restored sign-in keeps a pending Lightning refresh, and the next normal maintenance publication creates a new bolt11 instead of reusing the cached one. A replacement sign-in must drop that pending refresh without publishing it.
Findings
- [MEDIUM] Record the public Lightning refresh after the sign-in appears — inline at
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:856.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Delta review e8ade7d..9fddab9: one new LOW inline. I found no HIGH or MEDIUM.
My thread on the forced Lightning refresh during deletion is fixed in 71d9ef7 and 9fddab9.
- Public side: the request is recorded per
PubkySignInbefore the deletion return (AppViewModel.kt:856,:860-863). It is consumed on the next admitted sync (PublicPaykitRepo.kt:273-283) and cleared only after a successful apply for the same sign-in (:286-289). Maintenance retries it (AppViewModel.kt:2021-2023). - Private side: it is stored per state (
PrivatePaykitRepo.kt:399-405) and acknowledged per queued key (:1064-1076), so a partial failure keeps the rest. - Neither side leaks across identities. A generation change drops the public request, and the private refresh is fenced by
owner === stateandisStillCurrentand cleared incloseAndClearandrestoreBackup.
@piotr-iohk's "recheck public sharing after session restoration" is fixed in f22714b. isAutomaticPublicationAllowed re-reads sharesPublicPaykitEndpoints after the restore wait, after the build, inside publishMutex and inside the SDK lock. acknowledgePublishedEndpoints checks it inside the atomic settings transform. Confirming here; resolving that thread is his call.
Also clean: an older write landing after a replacement re-arms pending (:377-379). canPublish is suspend and still checked inside the operation lock (PaykitSdkService.kt:822-825). No new suspension points.
Performance: retryPendingPaykitEndpointRemoval now runs one extra public sync per existing trigger while a Lightning refresh is pending. It is not a loop.
Device gate (partial): 9fddab9 — delete-profile.xml passed at f22714b, so it needs a re-run at this head. The deletion timing posted earlier is single samples on a loaded host. Further timing is tracked in #1522 and pubky/paykit-rs#197, not on this PR.
The LOW can be fixed here or moved to a follow-up issue. Reply on the thread with which one.
There was a problem hiding this comment.
Verdict:
The PR moved to d46bd6c after this run; these results are for 9fddab9.
Delta review of f22714b...9fddab9 (3 production files and their tests), tested at 9fddab9. d46bd6c was pushed during the test run and is not covered here.
Findings:
- LOW
app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt:280: while a forced Lightning refresh is pending and publication keeps failing, every retry creates and caches a new public invoice: one invoice and one settings write per attempt, on each maintenance pass and each foreground or address refresh, for as long as the write fails. Test 3 shows five invoices for one request in 87 s. - LOW
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:861: a force that starts before sign-in is still dropped when the first session restore is deferred (SharedStateBusy), and nothing re-requests it when a retry restores the session (Test 3, second run). After a channel change, the public invoice keeps its old route hints until it expires or another event refreshes it.
Earlier findings, checked at 9fddab9:
- Mine (older snapshot after its replacement): fixed. In Test 1 the older write re-arms pending and maintenance republishes the replacement.
- @piotr-iohk's refresh after the sign-in appears: fixed when the first restore completes (Test 3). The deferred-restore case is the second LOW above.
- @jvsena42's two threads:
- forced refresh during deletion: fixed in code. Its device case, Test 2, was not driven.
- private refresh before contacts load: reproduced on device at 9fddab9 (Test 3, second run). The fix in d46bd6c is not checked here.
QA for 9fddab9:
Android 15 emulators (Pixel 7), dev build (regtest) with staging Pubky and Blocktank. Wallet A has a Blocktank channel and a profile sharing public and private payment endpoints; wallet B is A's contact. Tests 1 and 3 ran on the head plus a test hook, not pushed and shown at the end: it logs each Lightning refresh request, record and acknowledge, and holds a step until released. A hold only widens a window that already exists; it fakes no result. In Test 3 the device refused connections to the homeserver to make publication fail; the Lightning peer and Electrum stayed connected.
Test 1: older public snapshot after its replacement ✅ The older write re-arms pending, and maintenance republishes the replacement.
I held a foreground refresh carrying the cached invoice f80a75fb… before its app sync, then paid that invoice from Blocktank. The replacement f6e06f03… published and its acknowledge cleared pending. On release the older write landed and its acknowledge set pending again. The next maintenance reconcile, with no other event, republished f6e06f03… and cleared pending.
The other ordering still holds: I held a write of f6e06f03… inside the SDK lock while that invoice was paid. Its acknowledge found the invoice cleared and kept pending, and the replacement then published and cleared it.
t1-older-snapshot-after-replacement.2x.mp4 |
17:01:20.935 public sync prepared force=false requested=false bolt11='lnbcrt1p4dyw7pdqq…' cachedHash='f80a75fb…'
17:01:20.936 hold 'sync_before_app' active
17:01:28.923 payment received hash='f80a75fb…' deleting=false
17:01:28.965 paid public bolt11 cleared hash='f80a75fb…' pending=true
17:01:29.516 public apply start endpoints='[Bolt11=lnbcrt1p4dy0zfdqq…, P2wpkh=bcrt1qzcewrj…]' cachedHash='f6e06f03…' pending=true
17:01:37.737 public acknowledge published='lnbcrt1p4dy0zfdqqnp4qwps' stored='lnbcrt1p4dy0zfdqqnp4qwps' sharing=true pending=false
17:01:43.631 hold 'sync_before_app' released
17:01:45.899 public apply start endpoints='[Bolt11=lnbcrt1p4dyw7pdqq…, P2wpkh=bcrt1qzcewrj…]' cachedHash='f6e06f03…' pending=false
17:01:54.959 public acknowledge published='lnbcrt1p4dyw7pdqqnp4qwps' stored='lnbcrt1p4dy0zfdqqnp4qwps' sharing=true pending=true
17:02:06.899 reconcile public pending=true sharing=true pendingRefresh=false
17:02:09.575 public apply start endpoints='[Bolt11=lnbcrt1p4dy0zfdqq…, P2wpkh=bcrt1qzcewrj…]' cachedHash='f6e06f03…' pending=true
17:02:18.541 public acknowledge published='lnbcrt1p4dy0zfdqqnp4qwps' stored='lnbcrt1p4dy0zfdqqnp4qwps' sharing=true pending=false
17:02:42.800 public sync prepared force=false requested=false bolt11='lnbcrt1p4dy0zfdqq…' cachedHash='f6e06f03…'
17:02:47.333 hold 'sdk_write' active
17:02:51.878 payment received hash='f6e06f03…' deleting=false
17:02:51.908 paid public bolt11 cleared hash='f6e06f03…' pending=true
17:02:51.910 hold 'bolt11_replace' active
17:03:12.786 hold 'sdk_write' released
17:03:18.278 public acknowledge published='lnbcrt1p4dy0zfdqqnp4qwps' stored='' sharing=true pending=true
17:03:21.019 hold 'bolt11_replace' released
17:03:21.364 public apply start endpoints='[Bolt11=lnbcrt1p4dy09edqq…, P2wpkh=bcrt1qzcewrj…]' cachedHash='e3977664…' pending=true
17:03:29.703 public acknowledge published='lnbcrt1p4dy09edqqnp4qwps' stored='lnbcrt1p4dy09edqqnp4qwps' sharing=true pending=false
Test 2: forced refresh during a held deletion, then a failed deletion ⏭️ Not run: d46bd6c was pushed before it started.
It also covers f22714b's admitted-write and failed-deletion cases, which go through code this delta changed.
Test 3: forced refresh before the restored sign-in ✅ It survives failed publications, and maintenance publishes a new invoice.
Cold start with the session restore held. The channel turned usable while no sign-in existed, and the forced public refresh waited for the restore. After release the request was recorded for the restored sign-in and the forced build cached 52548271…. With the homeserver refusing connections, five attempts failed within 9 s and the request stayed pending. Each attempt that carried it built another invoice (38e57957…, 980a21a5…, db5d6cb9…): the first LOW. The next maintenance pass ran with pending=false pendingRefresh=true and no channel, address or payment event. It published 9df2b882… and consumed the request. requested=true in the log means the build was forced by the retained request.
t3-forced-refresh-after-restore.2x.mp4 |
17:25:19.001 hold 'session_restore' active
17:25:30.700 usable channels=true had=false deleting=false
17:25:33.238 auto public refresh entry force=true signIn=false deleting=false
17:25:42.307 hold 'session_restore' released
17:25:48.119 session restore end publicKey=true
17:25:48.131 public lightning refresh requested current=true pendingRefresh=true
17:25:48.131 auto public sync after restore signIn=true admission=true force=true
17:25:48.211 public sync prepared force=false requested=false bolt11='lnbcrt1p4dystxdqq…' cachedHash='cb54805b…'
17:25:48.384 public sync prepared force=true requested=true bolt11='lnbcrt1p4dys0udqq…' cachedHash='52548271…'
17:25:57.044 public sync result success=false error='code=transport_error, context=acquire Pubky write lock' pendingRefresh=true
17:25:57.208 public sync result success=false error='code=transport_error, context=acquire Pubky write lock' pendingRefresh=true
17:25:57.213 reconcile public pending=false sharing=true pendingRefresh=true
17:25:57.463 public sync prepared force=false requested=true bolt11='lnbcrt1p4dyss9dqq…' cachedHash='38e57957…'
17:25:57.574 public sync result success=false error='code=transport_error, context=acquire Pubky write lock' pendingRefresh=true
17:25:58.294 public sync prepared force=false requested=true bolt11='lnbcrt1p4dyssxdqq…' cachedHash='980a21a5…'
17:25:58.537 public sync result success=false error='code=transport_error, context=acquire Pubky write lock' pendingRefresh=true
17:26:04.764 reconcile public pending=false sharing=true pendingRefresh=true
17:26:04.987 public sync prepared force=false requested=true bolt11='lnbcrt1p4dyssvdqq…' cachedHash='db5d6cb9…'
17:26:05.078 public sync result success=false error='code=transport_error, context=acquire Pubky write lock' pendingRefresh=true
17:27:15.188 reconcile public pending=false sharing=true pendingRefresh=true
17:27:15.386 public sync prepared force=false requested=true bolt11='lnbcrt1p4dysjndqq…' cachedHash='9df2b882…'
17:27:25.090 public acknowledge published='lnbcrt1p4dysjndqqnp4qwps' stored='lnbcrt1p4dysjndqqnp4qwps' sharing=true pending=false
17:27:25.090 public lightning refresh consumed pendingRefresh=false
17:27:25.090 public sync result success=true error='null' pendingRefresh=false
A second run, where the previous process had stopped mid-publication, deferred the first restore with SharedStateBusy. The forced public refresh ended with no sign-in and was dropped. Nothing re-requested it when a retry restored the session 18 s later: the second LOW. The private force from the same channel event was recorded before contacts loaded (keys='0'), and B's invoice was then reused with force=false, as in @jvsena42's thread.
t3a-deferred-restore.2x.mp4 |
17:18:55.757 reconcile public pending=false sharing=true pendingRefresh=false
17:19:07.132 auto public refresh entry force=true signIn=false deleting=false
17:19:20.620 [app] Deferred session restoration, keeping saved session [SharedStateBusy='code=shared_state_busy, context=Pubky shared state remains locked; retry later'] - PubkyRepo
17:19:20.655 auto public sync after restore signIn=false admission=false force=true
17:19:20.666 reconcile public pending=false sharing=true pendingRefresh=false
17:19:20.711 private lightning refresh recorded keys='0' reason='channel usable' deleting=false
17:19:27.205 [app] Deferred session restoration, keeping saved session [AppError='code=shared_state_busy, context=Pubky shared state remains locked; retry later'] - PubkyRepo
17:19:38.648 session restore end publicKey=true
17:19:39.234 reconcile public pending=false sharing=true pendingRefresh=false
17:19:56.615 private list update key='pubky6e9ko7q' force=false
17:19:56.615 private publish lightning refresh remaining='0' reason='contact preparation'
17:20:00.944 private lightning refresh acknowledged refreshed='1' queued='1' remaining='0'
17:20:37.095 reconcile public pending=false sharing=true pendingRefresh=false
Test 4: delete-profile.xml ⏭️ Not run: d46bd6c was pushed before it started.
@jvsena42's 62-contact samples at f22714b reached onboarding; reopening Profile afterwards is not in that report.
f22714b's Tests 1, 3, 4 and 5 carry: the delta leaves their admission checks and the sharing-off branch of the acknowledge unchanged. The partial private queue failure and a replacement session inheriting a retained refresh are covered by the PR's unit tests only.
synonymdev/bitkit-ios#954 at aac9d0b keeps the forced refresh differently: it invalidates the cached invoice once and marks cleanup pending, so failed retries reuse one replacement, and before sign-in it keys the request to the stored identity rather than the restored session.
test hook
Not pushed. It logs each Lightning refresh request, record, consumption and acknowledge, the usable-channel transition and the reconcile decision. It holds a step while a file exists: the cold-start session restore, an automatic public refresh before its app sync, a write inside the SDK lock, and the paid-invoice replacement.
diff --git a/app/src/main/java/to/bitkit/TestHook.kt b/app/src/main/java/to/bitkit/TestHook.kt
new file mode 100644
index 000000000..e6c16d8ec
--- /dev/null
+++ b/app/src/main/java/to/bitkit/TestHook.kt
@@ -0,0 +1,23 @@
+package to.bitkit
+
+import android.util.Log
+import kotlinx.coroutines.delay
+import java.io.File
+
+// test hook: not pushed. Logs automatic Paykit work and holds an operation until the tester removes its file.
+object TestHook {
+ private const val DIR = "/data/data/to.bitkit.dev/files"
+
+ fun log(message: String) {
+ if (BuildConfig.DEBUG) Log.i("TestHook", message)
+ }
+
+ suspend fun hold(name: String) {
+ if (!BuildConfig.DEBUG) return
+ val file = File("$DIR/testhook_hold_$name")
+ if (!file.exists()) return
+ log("hold '$name' active")
+ while (file.exists()) delay(250)
+ log("hold '$name' released")
+ }
+}
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index d74d032ac..db86af6b2 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -308,6 +308,7 @@ class PrivatePaykitRepo @Inject constructor(
forceRefreshLightning: Boolean = false,
requeueActive: Boolean = false,
) {
+ to.bitkit.TestHook.log("private schedule preparation keys='${publicKeys.size}' force=$forceRefreshLightning deleting=$isDeletingProfile") // test hook
if (isDeletingProfile) return
pendingPreparationKeys.addAll(
publicKeys.filter { requeueActive || forceRefreshLightning || it !in activePreparationKeys }
@@ -319,6 +320,7 @@ class PrivatePaykitRepo @Inject constructor(
awaitContactPreparationActive(Priority.Background)
if (pendingPreparationKeys.isEmpty()) break
val keys = pendingPreparationKeys.intersect(knownSavedContactKeys)
+ to.bitkit.TestHook.log("private preparation run keys='${keys.size}' deleting=$isDeletingProfile") // test hook
pendingPreparationKeys.clear()
activePreparationKeys = keys
val generation = preparationGeneration
@@ -362,11 +364,13 @@ class PrivatePaykitRepo @Inject constructor(
suspend fun beginProfileDeletion() = withContext(serializedDispatcher) {
_isProfileDeletionInProgress.update { true }
+ to.bitkit.TestHook.log("deletion begin") // test hook
invalidateContactPreparation()
}
suspend fun endProfileDeletion() = withContext(serializedDispatcher) {
_isProfileDeletionInProgress.update { false }
+ to.bitkit.TestHook.log("deletion end") // test hook
}
suspend fun refreshSavedContactEndpoints(
@@ -402,6 +406,7 @@ class PrivatePaykitRepo @Inject constructor(
val owner = ensureState()
if (isStillCurrent?.invoke() == false) return@runSuspendCatching
pendingLightningRefresh = LightningRefresh(owner, knownSavedContactKeys.toMutableSet(), isStillCurrent)
+ to.bitkit.TestHook.log("private lightning refresh recorded keys='${knownSavedContactKeys.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
}
scheduleContactPreparation(knownSavedContactKeys.toList(), forceRefreshLightning, requeueActive)
}.onFailure {
@@ -500,10 +505,12 @@ class PrivatePaykitRepo @Inject constructor(
suspend fun removePublishedEndpointsForCleanup(context: String): Result<Unit> = withContext(serializedDispatcher) {
runSuspendCatching {
+ to.bitkit.TestHook.log("explicit private cleanup start '$context' deleting=$isDeletingProfile") // test hook
updateContactSharingCleanupPending(true)
removePublishedEndpoints().getOrThrow()
syncPaykitAppAfterCleanup().getOrThrow()
updateContactSharingCleanupPending(false)
+ to.bitkit.TestHook.log("explicit private cleanup done '$context'") // test hook
}.onFailure {
updateContactSharingCleanupPending(true)
Logger.warn("Failed to remove private Paykit endpoints during '$context'", it, context = TAG)
@@ -679,7 +686,11 @@ class PrivatePaykitRepo @Inject constructor(
if (matches.isEmpty()) return@runSuspendCatching
matches.forEach { (publicKey, paymentHash) -> rememberReceivedInvoicePaymentHash(paymentHash, publicKey) }
- if (!canPublishPrivateEndpoints()) return@runSuspendCatching
+ to.bitkit.TestHook.log("private invoice rotation matched '${matches.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
+ if (!canPublishPrivateEndpoints()) {
+ to.bitkit.TestHook.log("private invoice rotation skipped deleting=$isDeletingProfile") // test hook
+ return@runSuspendCatching
+ }
publishLocalEndpoints(matches.map { it.first }.distinct(), reason = reason)
.onFailure { Logger.warn("Failed to rotate private Paykit invoice", it, context = TAG) }
@@ -698,7 +709,11 @@ class PrivatePaykitRepo @Inject constructor(
}
}.filter { it in knownSavedContactKeys }.distinct()
if (publicKeys.isEmpty()) return@runSuspendCatching
- if (!canPublishPrivateEndpoints()) return@runSuspendCatching
+ to.bitkit.TestHook.log("private onchain rotation matched '${publicKeys.size}' deleting=$isDeletingProfile") // test hook
+ if (!canPublishPrivateEndpoints()) {
+ to.bitkit.TestHook.log("private onchain rotation skipped deleting=$isDeletingProfile") // test hook
+ return@runSuspendCatching
+ }
publishLocalEndpoints(publicKeys, reason = "on-chain rotation").getOrThrow()
}
@@ -989,6 +1004,7 @@ class PrivatePaykitRepo @Inject constructor(
runSuspendCatching {
val keys = publicKeys.mapNotNull { normalizedPublicKey(it) }.distinct()
if (keys.isEmpty()) return@runSuspendCatching
+ to.bitkit.TestHook.log("private publish start keys='${keys.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
val generation = preparationGeneration
val retry = currentCoroutineContext()[PrivateMessageDrainRetry]
val identity = retry?.identity ?: pubkyService.currentPublicKey()
@@ -1015,6 +1031,7 @@ class PrivatePaykitRepo @Inject constructor(
}
val lightningRefresh = currentLightningRefresh()
+ to.bitkit.TestHook.log("private publish lightning refresh remaining='${lightningRefresh?.remainingKeys?.size}' reason='$reason'") // test hook
val publication = preparePrivatePaymentListReservations(
links.publicKeys,
reason,
@@ -1029,10 +1046,12 @@ class PrivatePaykitRepo @Inject constructor(
return@withLock
}
+ to.bitkit.TestHook.log("private sync start updates='${publication.updates.size}' reason='$reason' deleting=$isDeletingProfile") // test hook
val report = paykitSdkService.syncPrivatePaymentListsWithReservations(
updates = publication.updates,
clearUnlistedLinkedPeers = false,
)
+ to.bitkit.TestHook.log("private sync done reason='$reason'") // test hook
val deliveryError = applyPrivatePaymentListDeliveryReport(report, reason, publicationWindow)
val firstError = publication.firstError ?: deliveryError
val retryKeys = (links.linkRetryKeys + privatePaymentListDeliveryRetryKeys(report)).distinct()
@@ -1070,6 +1089,7 @@ class PrivatePaykitRepo @Inject constructor(
}.map { it.counterparty }.toSet()
val queuedKeys = report.queued.mapNotNull { normalizedPublicKey(it.counterparty) }.toSet()
refresh.remainingKeys.removeAll(refreshedKeys.intersect(queuedKeys))
+ to.bitkit.TestHook.log("private lightning refresh acknowledged refreshed='${refreshedKeys.size}' queued='${queuedKeys.size}' remaining='${refresh.remainingKeys.size}'") // test hook
if (refresh.remainingKeys.isEmpty()) {
pendingLightningRefresh = null
}
@@ -1117,6 +1137,7 @@ class PrivatePaykitRepo @Inject constructor(
for (publicKey in publicKeys) {
if (generation != preparationGeneration) break
if (publicKey in knownSavedContactKeys) {
+ to.bitkit.TestHook.log("private list update key='${publicKey.take(12)}' force=${publicKey in forceRefreshLightningKeys}") // test hook
runSuspendCatching { privatePaymentListUpdate(publicKey, publicKey in forceRefreshLightningKeys) }
.onSuccess { updates += it }
.onFailure {
@@ -2265,6 +2286,7 @@ class PrivatePaykitRepo @Inject constructor(
private suspend fun setLocalInvoice(publicKey: String, invoice: StoredInvoice) {
val contactState = ensureState().contacts.getOrPut(publicKey) { ContactState() }
contactState.localInvoice = invoice
+ to.bitkit.TestHook.log("private local invoice set key='${publicKey.take(12)}' hash='${invoice.paymentHash}' bolt11='${invoice.bolt11}'") // test hook
}
private fun rememberSavedContacts(publicKeys: Collection<String>, replacing: Boolean): List<String> {
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 93ed5efc3..db3a87975 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -437,6 +437,8 @@ class PubkyRepo @Inject constructor(
private suspend fun initializeSession(notifyFailure: Boolean = true): Boolean {
_isRestoringSession.update { true }
try {
+ to.bitkit.TestHook.log("session restore start publicKey=${_publicKey.value != null}") // test hook
+ to.bitkit.TestHook.hold("session_restore") // test hook
val savedSession = runSuspendCatching { keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) }
val importedSession = runSuspendCatching {
ensureServiceInitialized(savedSession.getOrNull())
@@ -491,6 +493,7 @@ class PubkyRepo @Inject constructor(
return result is InitResult.Restored
} finally {
_isRestoringSession.update { false }
+ to.bitkit.TestHook.log("session restore end publicKey=${_publicKey.value != null}") // test hook
}
}
@@ -1753,6 +1756,8 @@ class PubkyRepo @Inject constructor(
val hadPaykitState = settingsStore.data.first().hasPaykitState()
val endpointCleanupResult = removeBitkitPaymentEndpoints()
.onFailure { Logger.warn("Failed to remove Bitkit payment endpoints", it, context = TAG) }
+ to.bitkit.TestHook.log("signout endpoint removal done success=${endpointCleanupResult.isSuccess}") // test hook
+ to.bitkit.TestHook.hold("signout_after_removal") // test hook
val result = runSuspendCatching {
pubkyService.signOut()
diff --git a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
index a012d6ba5..27a608495 100644
--- a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt
@@ -208,6 +208,7 @@ class PublicPaykitRepo @Inject constructor(
internal fun requestLightningRefresh(signIn: PubkySignIn) {
if (pubkyRepo.isCurrent(signIn)) pendingLightningRefresh.set(LightningRefresh(signIn))
+ to.bitkit.TestHook.log("public lightning refresh requested current=${pubkyRepo.isCurrent(signIn)} pendingRefresh=${pendingLightningRefresh.get() != null}") // test hook
}
internal val hasPendingLightningRefresh: Boolean
@@ -269,7 +270,10 @@ class PublicPaykitRepo @Inject constructor(
): Result<Unit> = withContext(ioDispatcher) {
runSuspendCatching {
pubkyRepo.isRestoringSession.first { !it }
- if (!isAutomaticPublicationAllowed(canPublish)) return@runSuspendCatching
+ if (!isAutomaticPublicationAllowed(canPublish)) {
+ to.bitkit.TestHook.log("public sync rejected at admission 'after-restore'") // test hook
+ return@runSuspendCatching
+ }
val requestedRefresh = pendingLightningRefresh.get()?.takeIf { request ->
pubkyRepo.isCurrent(request.signIn).also { current ->
if (!current) pendingLightningRefresh.compareAndSet(request, null)
@@ -280,15 +284,23 @@ class PublicPaykitRepo @Inject constructor(
forceRefreshLightning = forceRefreshLightning || requestedRefresh != null,
requireEndpoint = requireEndpoint,
)
- if (!isAutomaticPublicationAllowed(canPublish)) return@runSuspendCatching
+ if (!isAutomaticPublicationAllowed(canPublish)) {
+ to.bitkit.TestHook.log("public sync rejected at admission 'after-preparation'") // test hook
+ return@runSuspendCatching
+ }
+ to.bitkit.TestHook.log("public sync prepared force=$forceRefreshLightning requested=${requestedRefresh != null} bolt11='${desired.firstOrNull { it.methodId == MethodId.Bolt11 }?.value?.take(40)}' cachedHash='${settingsStore.data.first().publicPaykitBolt11PaymentHash}'") // test hook
+ if (desired.isNotEmpty()) to.bitkit.TestHook.hold("sync_before_app") // test hook
syncPaykitApp().getOrThrow()
if (applyPublishedEndpoints(desired) { isAutomaticPublicationAllowed(canPublish) }) {
acknowledgePublishedEndpoints(desired, canPublish)
if (requestedRefresh != null && pubkyRepo.isCurrent(requestedRefresh.signIn)) {
pendingLightningRefresh.compareAndSet(requestedRefresh, null)
+ to.bitkit.TestHook.log("public lightning refresh consumed pendingRefresh=${pendingLightningRefresh.get() != null}") // test hook
}
}
- }
+ }.also { // test hook
+ to.bitkit.TestHook.log("public sync result success=${it.isSuccess} error='${it.exceptionOrNull()?.message?.take(120)}' pendingRefresh=${pendingLightningRefresh.get() != null}") // test hook
+ } // test hook
}
suspend fun refreshPublishedBolt11ForPayment(
@@ -301,7 +313,12 @@ class PublicPaykitRepo @Inject constructor(
if (settings.publicPaykitBolt11PaymentHash != paymentHash) return@runSuspendCatching
clearPublicBolt11Metadata(publicationPending = true)
- if (!isAutomaticPublicationAllowed(canPublish)) return@runSuspendCatching
+ to.bitkit.TestHook.log("paid public bolt11 cleared hash='$paymentHash' pending=${settingsStore.data.first().publicPaykitCleanupPending}") // test hook
+ if (!isAutomaticPublicationAllowed(canPublish)) {
+ to.bitkit.TestHook.log("paid bolt11 replacement rejected at admission 'after-clear'") // test hook
+ return@runSuspendCatching
+ }
+ to.bitkit.TestHook.hold("bolt11_replace") // test hook
val desired = buildWalletEndpoints(refresh = true)
if (applyPublishedEndpoints(desired) { isAutomaticPublicationAllowed(canPublish) }) {
acknowledgePublishedEndpoints(desired, canPublish)
@@ -348,10 +365,20 @@ class PublicPaykitRepo @Inject constructor(
canPublish: suspend () -> Boolean = { true },
): Boolean =
publishMutex.withLock {
- if (!canPublish()) return@withLock false
+ if (!canPublish()) {
+ to.bitkit.TestHook.log("public apply rejected 'under-mutex' endpoints='${desiredEndpoints.size}'") // test hook
+ return@withLock false
+ }
requireCurrentPublicKey()
- if (!canPublish()) return@withLock false
- val report = paykitSdkService.syncPublicEndpoints(desiredEndpoints, canPublish) ?: return@withLock false
+ if (!canPublish()) {
+ to.bitkit.TestHook.log("public apply rejected 'after-identity' endpoints='${desiredEndpoints.size}'") // test hook
+ return@withLock false
+ }
+ to.bitkit.TestHook.log("public apply start endpoints='${desiredEndpoints.map { "${it.methodId}=${it.value}" }}' cachedHash='${settingsStore.data.first().publicPaykitBolt11PaymentHash}' pending=${settingsStore.data.first().publicPaykitCleanupPending}") // test hook
+ if (desiredEndpoints.isNotEmpty()) to.bitkit.TestHook.hold("public_apply") // test hook
+ val report = paykitSdkService.syncPublicEndpoints(desiredEndpoints, canPublish)
+ to.bitkit.TestHook.log("public apply done endpoints='${desiredEndpoints.size}' result='${report?.let { "failed=${it.failed.size}" } ?: "rejected-in-sdk-lock"}'") // test hook
+ if (report == null) return@withLock false
report.failed.forEach { failure ->
Logger.warn(
"Failed to sync public Paykit endpoint '${failure.identifier}': ${failure.error}",
@@ -382,6 +409,9 @@ class PublicPaykitRepo @Inject constructor(
it
}
}
+ settingsStore.data.first().let { // test hook
+ to.bitkit.TestHook.log("public acknowledge published='${publishedInvoice.take(24)}' stored='${it.publicPaykitBolt11.take(24)}' sharing=${it.sharesPublicPaykitEndpoints} pending=${it.publicPaykitCleanupPending}") // test hook
+ } // test hook
}
private suspend fun buildWalletEndpoints(
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index b6beb1b02..2fbf13c5b 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -832,7 +832,12 @@ class PaykitSdkService @Inject constructor(
}
return operationLock.withLock(priority) {
withStateRevisionTracking { handle ->
- if (!canPublish()) return@withStateRevisionTracking null
+ if (!canPublish()) {
+ to.bitkit.TestHook.log("sdk public write rejected in lock endpoints='${endpoints.size}'") // test hook
+ return@withStateRevisionTracking null
+ }
+ to.bitkit.TestHook.log("sdk public write admitted in lock endpoints='${endpoints.size}'") // test hook
+ if (endpoints.isNotEmpty()) to.bitkit.TestHook.hold("sdk_write") // test hook
completeSdkCall {
handle.syncPublicEndpointsWithReceivingDetails(endpoints.map { it.toPublicReceivingDetail() })
}
diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
index 4c5abbe74..9cc1999bc 100644
--- a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
+++ b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt
@@ -269,8 +269,11 @@ class EditProfileViewModel @Inject constructor(
}
try {
privatePaykitRepo.beginProfileDeletion()
+ to.bitkit.TestHook.hold("delete_after_begin") // test hook
privatePaykitRepo.removePublishedEndpointsForCleanup(TAG)
+ to.bitkit.TestHook.hold("delete_after_cleanup") // test hook
val result = pubkyRepo.deleteProfileWithSessionRetry()
+ to.bitkit.TestHook.log("deleteProfile result success=${result.isSuccess}") // test hook
if (result.isSuccess) {
privatePaykitRepo.closeAndClear()
ToastEventBus.send(
diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
index 578153fd7..dd8d9d8ee 100644
--- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
+++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
@@ -795,6 +795,7 @@ class AppViewModel @Inject constructor(
.map { state -> state.channels.any { it.isUsable } }
.distinctUntilChanged()
.collect { hasUsableChannels ->
+ to.bitkit.TestHook.log("usable channels=$hasUsableChannels had=$hadUsableChannels deleting=${privatePaykitRepo.isProfileDeletionInProgress.value}") // test hook
if (hasUsableChannels && !hadUsableChannels) {
refreshPaykitEndpointsAfterChannelAvailabilityChanged(
reason = "channel usable",
@@ -853,15 +854,18 @@ class AppViewModel @Inject constructor(
val signIn = pubkyRepo.currentSignIn()
val settings = settingsStore.data.first()
if (!isPaykitEnabled.value || !settings.sharesPublicPaykitEndpoints) return
+ to.bitkit.TestHook.log("auto public refresh entry force=$forceRefreshLightning signIn=${signIn != null} deleting=${privatePaykitRepo.isProfileDeletionInProgress.value}") // test hook
if (forceRefreshLightning && signIn != null) publicPaykitRepo.requestLightningRefresh(signIn)
if (privatePaykitRepo.isProfileDeletionInProgress.value) return
+ to.bitkit.TestHook.log("auto public sync admitted signIn=${signIn != null} restoring=${pubkyRepo.isRestoringSession.value}") // test hook
pubkyRepo.isRestoringSession.first { !it }
val restoredSignIn = signIn ?: pubkyRepo.currentSignIn()
if (forceRefreshLightning && signIn == null && restoredSignIn != null) {
publicPaykitRepo.requestLightningRefresh(restoredSignIn)
}
val admission = publicPaykitPublicationAdmission(restoredSignIn)
+ to.bitkit.TestHook.log("auto public sync after restore signIn=${restoredSignIn != null} admission=${admission()} force=$forceRefreshLightning") // test hook
val onchainAddress = walletRepo.walletState.value.onchainAddress
if (onchainAddress.isBlank() && !lightningRepo.canReceive()) return
@@ -901,6 +905,7 @@ class AppViewModel @Inject constructor(
}
private suspend fun synchronizePaykitContacts(state: PaykitContactSyncState) {
+ to.bitkit.TestHook.log("auto contact sync admitted signedIn=${state.publicKey != null} contacts='${state.contactKeys.size}'") // test hook
if (!state.isPaykitEnabled || state.publicKey == null) {
isPaymentRequestIdentityActivating = true
lastPrivatePaykitContactKeys = emptySet()
@@ -981,6 +986,7 @@ class AppViewModel @Inject constructor(
) {
val signIn = if (forceRefreshLightning) pubkyRepo.currentSignIn() else null
val isStillCurrent = signIn?.let { { pubkyRepo.isCurrent(it) } }
+ to.bitkit.TestHook.log("auto private refresh entry reason='$reason' force=$forceRefreshLightning signIn=${signIn != null} deleting=${privatePaykitRepo.isProfileDeletionInProgress.value}") // test hook
if (privatePaykitRepo.isProfileDeletionInProgress.value) {
if (forceRefreshLightning && isPaykitEnabled.value && signIn != null) {
privatePaykitRepo.refreshKnownSavedContactEndpoints(
@@ -1144,6 +1150,7 @@ class AppViewModel @Inject constructor(
) {
return Result.failure(PaykitPaymentRequestError.RequestUnavailable)
}
+ to.bitkit.TestHook.log("auto request intake admitted mode='$mode'") // test hook
val result = if (forceFresh) {
paykitPaymentRequestRepo.refreshAfterStateChange(mode)
} else {
@@ -1175,6 +1182,7 @@ class AppViewModel @Inject constructor(
private suspend fun refreshPaymentRequestTargets(force: Boolean = false) {
if (privatePaykitRepo.isProfileDeletionInProgress.value) return
if (!isPaykitEnabled.value || pubkyRepo.publicKey.value == null || !walletRepo.walletExists()) return
+ to.bitkit.TestHook.log("auto request targets admitted") // test hook
paykitPaymentRequestRepo.refreshEligibleTargets(
savedPublicKeys = pubkyRepo.contacts.value.map { it.publicKey },
force = force,
@@ -1231,6 +1239,7 @@ class AppViewModel @Inject constructor(
private suspend fun refreshPaykitMaintenance() {
if (isPaykitEnabled.value && walletRepo.walletExists()) pubkyRepo.restoreSessionIfNeeded()
if (isAutomaticPaykitWorkPaused()) return
+ to.bitkit.TestHook.log("auto maintenance admitted") // test hook
pubkyRepo.republishIdentityIfNeeded()
retryPendingPaykitEndpointRemoval(
contactKeys = pubkyRepo.contacts.value.map { it.publicKey },
@@ -1272,6 +1281,7 @@ class AppViewModel @Inject constructor(
if (!isPaykitEnabled.value || isOnline.value != ConnectivityState.CONNECTED) return
if (pubkyRepo.publicKey.value == null || !walletRepo.walletExists()) return
shopContactImportJob = viewModelScope.launch {
+ to.bitkit.TestHook.log("auto shop import admitted") // test hook
pubkyRepo.importShopContacts()
}
}
@@ -2010,6 +2020,7 @@ class AppViewModel @Inject constructor(
private suspend fun retryPendingPaykitEndpointRemoval(contactKeys: Collection<String>, reason: String) {
if (privatePaykitRepo.isProfileDeletionInProgress.value) return
+ to.bitkit.TestHook.log("auto pending removal admitted reason='$reason'") // test hook
contactPaymentSettingsRepo.reconcilePendingEndpoints {
privatePaykitRepo.retryPendingEndpointRemoval(contactKeys)
.onFailure {
@@ -2018,6 +2029,7 @@ class AppViewModel @Inject constructor(
val settings = settingsStore.data.first()
if (privatePaykitRepo.isProfileDeletionInProgress.value) return@reconcilePendingEndpoints
+ to.bitkit.TestHook.log("reconcile public pending=${settings.publicPaykitCleanupPending} sharing=${settings.sharesPublicPaykitEndpoints} pendingRefresh=${publicPaykitRepo.hasPendingLightningRefresh}") // test hook
if (settings.publicPaykitCleanupPending ||
settings.sharesPublicPaykitEndpoints && publicPaykitRepo.hasPendingLightningRefresh
) {
@@ -2499,6 +2511,7 @@ class AppViewModel @Inject constructor(
syncLdkPayments = false,
)
}
+ to.bitkit.TestHook.log("payment received hash='$paymentHash' deleting=${privatePaykitRepo.isProfileDeletionInProgress.value}") // test hook
publicPaykitRepo.refreshPublishedBolt11ForPayment(
paymentHash,
canPublish = publicPaykitPublicationAdmission(),
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up at d46bd6c, re-reviewing changes since 9fddab9 under baseline bc-fcb05c71-32d6-40e7-8e63-7454abe45ac5. Comparison base and merge base remain 0801188. This pass covered late contact enrollment into a pending private Lightning refresh, the updated private repository tests and profile journey note, and restored-session admission against bitkit-ios#954 at aac9d0b. Inherited coverage is the unchanged deletion, sharing, cleanup, and public refresh behavior from that baseline.
No new actionable code findings.
Contacts first remembered while a private Lightning refresh is current are added to that refresh and published with a fresh bolt11. Repeating a list of contacts already known leaves completed peers on their current invoice. A refresh whose sign-in fence is false leaves later contacts off that request, and a partial queue still retries only the peers that were not queued. The iOS comparison is limited to this refresh behavior: public retention across deferred and failed restoration is unchanged, and that iOS revision still drops a private force refresh when no contacts are known.
build, lint, detekt, and e2e-status passed at this head. Those jobs do not exercise the contact-load gate. This review inspected the new PrivatePaykitRepoTest cases and did not run them; the Android SDK is not configured in this environment. Device testing: not performed in this review.
Two external threads on the unchanged public refresh path remain open: a failing publication creates another public invoice on every retry, and a force that starts before sign-in is dropped when the first restore is deferred. The previous assessment recorded that deferred-restore skip. This delta does not change either path.
Non-blocking consistency suggestions
At aac9d0b, iOS returns before recording a private force refresh when no saved contacts are known, so contacts that load later keep their cached invoice. Android now enrolls those contacts. journeys/profile/README.md marks the check Android-specific. The iOS counterpart can adopt the same cold-start route-hint refresh if the two apps should match.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving at d46bd6c. No HIGH or MEDIUM left.
- My startup-refresh LOW is fixed in
d46bd6c:rememberSavedContactsadds newly learned keys tocurrentLightningRefresh(), which is fenced byowner === stateandisStillCurrent. The test is included. - My deletion-window thread and @piotr-iohk's sharing re-check were confirmed fixed earlier.
- The two LOW threads from ovi-reviewer are open and do not block.
Device gate: d46bd6c — delete-profile.xml: 7/7 actions pass on Android, using the CI APK and a fresh profile with 0 contacts and Paykit on. The Delete pill spinner shows from +3.3 s to +30.9 s, with inputs dimmed and Cancel and Save disabled. Onboarding appears with "Profile deleted". The profile does not return on reopen or after a force-stop and relaunch, and pubky.json ownerPublicKey is null. No crash or ANR. iOS twin synonymdev/bitkit-ios#954 passes the same journey.
perf: delete profile ~31–35 s at 0 contacts on this emulator:
16:38:14.7 tap "Yes, Delete"
16:38:20.894 INFO [PubkyRepo.kt:1095] Deleted all contacts - PubkyRepo
16:38:46.433 DEBUG [Keychain.kt:118] Deleted value for key 'PUBKY_SECRET_KEY' - Keychain
16:38:46.518 INFO [PubkyAuthHandlerRegistrar.kt:89] Updated Pubky handler 'to.bitkit.ui.MainActivityPubkySignup' to 'enabled'
The master comparison and the 62-contact numbers are in my earlier partials. Further deletion timing is tracked in #1522 and pubky/paykit-rs#197.
l10n: no UI text changed.
CI green at d46bd6c, including all e2e shards.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-approving at 98cab12 ("retain public refresh across retries"). Delta d46bd6c..98cab12: no findings.
- ovi-reviewer's "new public invoice at every retry" (
PublicPaykitRepo.kt:280) is fixed.LightningRefresh.invalidateInvoiceis consumed once under the newinvoiceMutex(:449-458), andsyncCurrentPublishedEndpointsbuilds withforceRefreshLightning && requestedRefresh == null(:288), so retries reuse the cached replacement. A payment (refreshPublishedBolt11ForPaymentclears the hash) and expiry still force a new invoice, and only the sync holding the same request object clears it (:296-298). - ovi-reviewer's "force dropped when the first restore is deferred" (
AppViewModel.kt:861) is fixed.savedIdentityGuard()(PubkyRepo.kt:771-779) fences onstoredIdentityRevision. A replacingstartSignIn,clearAuthenticatedStateandadoptRingIdentitybump it, while restore (continueSignIn) and the deferred-restore clear do not. A replacement identity drops the request (PublicPaykitRepo.kt:224). - Concurrency:
invalidateInvoiceis touched only underinvoiceMutex, and the request is published throughAtomicReference.invoiceMutexis never taken insidepublishMutex, so lock order holds. No new loop: invoice creations per failing publication drop to one per request.
Device gate: 98cab12 — no re-run needed for this delta. It changes retry logic in the repositories with no UI or journey change, and delete-profile.xml passed 7/7 at d46bd6c. perf: invoice creations during failed publications are bounded by request now (read from code).
CI green at 98cab12.
This branch has not been deployed
4 participants
Related to #1453 and #1406; these issues remain open for recovery and broader latency investigation.
iOS counterpart: synonymdev/bitkit-ios#954.
This PR pauses automatic Paykit refresh and publication during profile deletion so they do not compete with explicit cleanup.
Description
Observes the existing deletion lifecycle to defer automatic endpoint publication, contact refresh, Shop imports, request intake and maintenance, then resume for a retained identity after deletion ends.
Rechecks admission after awaited work while allowing admitted operations, explicit cleanup, payment bookkeeping and proof completion to finish.
Clears matching paid public-invoice metadata and retains a publication retry while deletion or a transient error defers replacement. Admission is rechecked after preparation, identity lookup and SDK-lock acquisition immediately before the write; a skipped write stays pending and an older write cannot acknowledge a newer paid-invoice invalidation.
Preserves private received-payment attribution while deferring private invoice/onchain rotation, and prevents invalidated preparation from falling back to new link work.
Waits for initial session restoration before admitting startup public refresh when no sign-in existed. An existing sign-in retains its generation fence and cannot be replaced by a later session during the wait. A forced channel refresh captures the saved identity before restoration and settings waits, survives deferred/failed restoration, and binds to the first restored sign-in. Explicit identity removal/replacement invalidates it; normal maintenance retries without another channel event.
Rechecks persisted public sharing after restoration/preparation and inside the SDK lock. Disabling sharing blocks queued publication; an already-admitted write may finish but cannot clear the newly requested cleanup.
Retains forced public/private Lightning refresh across deletion and skipped or failed publication for the originating session. Newly loaded saved contacts join a pending private refresh for that same current state; repeated lists do not re-force completed peers. Partial private success consumes only peers whose refreshed invoice was queued. Normal maintenance retries remaining work without a new channel event.
Prepares one replacement public invoice per forced refresh and reuses that valid invoice across failed/skipped publication retries. New channel refreshes, payments and ordinary invoice expiry still invalidate or replace it; invoice preparation is serialized locally.
Re-arms pending reconciliation if an older public snapshot lands after an acknowledged replacement. This repairs eventual endpoint state; it does not prevent every transient stale write.
Out of Scope
Design
N/A - no UI changes.
Preview
N/A - no UI changes.
QA Notes
Journeys
delete-profile.xml- deletion reaches onboarding without the old profile returning, including the 62-contact fixture; shared notes distinguish safety waits from controlled admission/failure checks.Manual Tests
regression: hold an admitted SDK operation, begin deletion, trigger contact/payment/maintenance events, then release it -> admitted work and explicit cleanup finish without new automatic intake/publication; operation-admission gates are not in Capabilities.
regression: hold invoice creation or final identity lookup, begin deletion and release -> no new public/private endpoint rotation, while paid-invoice invalidation and receipt attribution still complete; controlled payment events and admission gates are not in Capabilities.
regression: fail deletion after a public invoice settles, then fail the first resumed publication -> the retained identity remains guarded and normal foreground maintenance retries without another payment; controlled cleanup, settlement and publication-failure gates are not in Capabilities.
regression: let an admitted publication finish after another paid invoice is invalidated -> the newer replacement remains pending; controlled publication ordering is not in Capabilities.
regression: queue public publication behind SDK work, end its admission and let endpoint removal complete before session revocation -> the stale publication must not restore the removed endpoints. A write already inside the SDK must finish before cleanup, including when its caller is cancelled; controlled operation gates are not in Capabilities.
regression: start with public sharing enabled while the initial session restore is held and no public key is available, then release restoration -> publish for the restored identity without another address/channel/payment event. Replacing an already captured sign-in must instead reject the stale attempt; controlled restore ordering is not in Capabilities.
regression: hold restoration with public sharing enabled, disable sharing, then release -> no endpoint preparation/publication resumes. Repeat at SDK admission; already-admitted writes finish without consuming sharing-off cleanup. Controlled restore and operation gates are outside journey Capabilities.
Hold deletion, make a channel usable/ready, then fail deletion retaining the identity: public and private invoices refresh rather than reuse old route hints. Repeat with deletion beginning during cleanup reconciliation; skipped/failed publication must retain the obligation and a replacement session must not inherit it. Verify partial private queue failure retries only unfinished peers. Hold old public snapshot X until replacement Y is acknowledged, then release X: normal reconciliation must restore Y. Controlled channel/SDK/failure gates are outside journey Capabilities.
Hold initial restoration before sign-in exists, make a channel usable, then defer/fail restoration before a later successful retry. Restore readiness/connectivity without another channel event: maintenance must retain the forced invoice refresh for the saved identity. Neither an initial request nor an existing token may transfer to an explicitly replaced identity/session. Controlled startup and publication gates are outside journey Capabilities.
Keep public publication unavailable across several maintenance retries after a channel change: prepare one replacement invoice and reuse it while valid, keeping publication pending. A later channel change, payment or expiry must still replace it. Controlled failures and invoice-observation gates are outside journey Capabilities.
Manual cold-start acceptance: hold saved-contact loading while a channel becomes usable, then load contacts. The pending private invoice refresh must apply without another channel event; repeated lists must not re-force completed peers, and a replacement session must not inherit the request. Contact-load gates are outside journey Capabilities. Native regression coverage is not a device pass.
Automated Checks
AppViewModelSendFlowTest.kt- publication/contact/polling deferral, failure resume, reconciliation and maintenance suspension boundaries, payment bookkeeping without fresh intake, startup publication after restoration without another wallet event, and same-identity generation replacement rejection.PublicPaykitSharingAdmissionTest.kt- sharing admission, one-time invoice replacement across retries and overlapping refreshes, deferred restoration ownership and same-key generation rejection.PubkyRepoTest.kt- saved-identity ownership survives restoration failures but rejects explicit removal/replacement and unreadable or missing credentials.PublicPaykitRepoTest.kt- held invoice/identity preparation, admitted-write completion, deferred retry after failure and preservation of newer paid-invoice invalidation.PaykitSdkServiceTest.kt- real Kotlin service/operation-lock coverage for queued publication rejection and admitted-write completion before cleanup despite cancellation; only the native payload constructor is mocked.PrivatePaykitRepoTest.kt- received-invoice attribution and onchain deferral, final identity/generation checks, and rejection of invalidated fallback preparation.Final author validation on
98cab12b6: 4,277 native tests across 240 suites passed, zero failures/errors/skips, plus compile and APK build. The focused suites passed 692 tests before the entrypoint-only simplification; the complete final suite covers that final form. Full Detekt, including formatting and ignoreFailures diagnostics, reports 1,401 verified baseline findings and no introduced violations. One unchanged hardware-wallet method differs in length from newer master; its exact source and 95-line diagnostic were verified against this branch's original base, rather than called an exact current-master multiset match. All 210 emitted final compiler diagnostics match verified baseline messages and source. Five tests added, none removed. Fresh combined correctness/native/polish review is clean. All ten PR commits are cryptographically signed and verified locally and on GitHub. Current-head hosted build, lint/Detekt, E2E build, all seven E2E shards and required e2e-status117105185830 pass. No rerun was dispatched. Controlled current-head device/payment cases remain unchecked.The invoice-retry and deferred-initial-restoration regressions failed on unchanged
d46bd6cacproduction. The first retry table row stops on its red assertion, so later failure rows are not claimed as separately reproduced. The final tests cover retry reuse, overlapping/new requests, deferred restoration, explicit identity replacement/removal and strict same-key generation checks. The saved-owner revision is transient and does not grant authentication or replace existing sign-in generations. No new device or payment flow was run; SDK source and bindings are unchanged.The late-contact regression failed on unchanged
9fddab92bproduction by publishing the cached invoice instead of the newly generated one. It now verifies the published payload, repeated-list idempotence, partial-queue progress and stale-session rejection. This is native fixture evidence; no new device or payment flow was run.The initial-restoration force regression failed on unchanged
71d9ef7afproduction with no retained request after a readiness skip. Its final table also covers failed publication; the first red assertion stopped before that later row, so a separate pre-fix failure is not claimed for it. Existing repository tests cover retained-request retry and session fencing. This is native fixture evidence, not a new mounted-device acceptance pass.Controlled regressions cover lost force during deletion and deletion beginning while reconciliation is suspended, skipped/failed publication, partial private queue success, session replacement, newer refresh ownership, and older public snapshot X landing after replacement Y. The late-snapshot race also existed on master; the narrow pending-reconciliation repair completes this PR's replacement contract without redesigning publication serialization. These are native tests, not device/payment acceptance. No new SDK, bindings, dependency, stored format or safety wait was changed.
The new restoration and preparation sharing-disable regressions failed on unchanged production source before the fix. Final repository tests exercise preparation, queued admission and admitted-write cleanup ownership; existing Kotlin-service tests cover the real operation lock. Settings are mocked in the focused tests, so real DataStore persistence ordering and controlled device toggles are not claimed as tested. Explicit removal and matching paid-invoice bookkeeping remain unchanged.
The receipt/onchain and held-public-invoice regressions reproduced the unwanted publication before correction. The public red run used a callback-signature adapter retaining the original single early admission sample; the private red run used the original production source. Additional regressions reproduced a stale fallback link and an older publication clearing a newer pending invalidation. The SDK-queue regression also failed before its admission check was added; that red run used a signature-compatible overload preserving the previous admission behavior, not an untouched source tree. The startup regression failed on unchanged production source because the restored identity was still rejected, then passed after correction. No device or funded test was run for this review update.
An earlier full run failed one unchanged Trezor enumeration test with an empty list under its 50ms management timeout. The entire class and two subsequent complete suites passed unchanged; its original cause is not established.
Operation-level baseline traces show new automatic publication/intake alongside explicit deletion cleanup. The preceding
bc00345cruntime APK completed two disposable 62-contact import/deletion cycles, with the second deletion completed through the Android CLI after the Appium driver failed. The first reached onboarding in 38.5 seconds measured from the Profile flow start, including navigation/confirmation. The second reached onboarding within 55 seconds of confirmation, after a settled recovery; returning Home and reopening Profile did not restore the deleted profile. These are different, individual observations, not comparable speedup samples or complete controlled-failure acceptance.The second cycle's cold restart exposed public profile controls in 9.7 seconds and authenticated editing in 68.7 seconds. The driver then incorrectly searched for Profile controls after its helper returned Home; a separate Appium reattachment failed before app interaction. Neither tooling failure is an app deletion defect or a passing end-to-end spec. The subsequent CLI deletion completed on the same unfunded wallet without reset or payment. Recovery latency remains open.
Android cancels an in-flight contact scan awaiting signed-in contacts refresh when deletion signs out. This PR does not change that into the iOS retained-link retry behavior.
Reviewer QA on the earlier
bc00345chead reported ordinary deletion, one held-operation path and failed-deletion/public-retry coverage, but found both the invoice-preparation gap and a stale SDK-queued public write after endpoint removal. Those paths now have separate admission checks and deterministic regressions; the corresponding current-head controlled device retest remains unchecked. The private-payment rotation case was not completed in that review because the branch changed during the run. These older reviewer observations do not establish current-head acceptance.Reviewer 5547401806 exercised eight controlled cases on historical
f22714b3with disclosed uncommitted hold/log hooks: SDK-queue rejection, admitted-write cleanup ordering, paid-invoice invalidation/attribution, sharing-off admission, failed-deletion replacement retry and startup restoration. It found the late-old-snapshot race now addressed above. Plain-head one-contact deletion passed; restore-held sharing-off and identity replacement were unit-only, and the 62-contact repeat remained in progress. This is reviewer-reported old-head evidence, not final-head or author-funded acceptance.Reviewer comment 6216228877 reports historical
f22714b362-contact deletion samples of 127 and 373 seconds; the latter included a logged 300-second SDK uncertain-write cooldown before sign-out. The updated master immediate-deletion sample was 79.7s with no cooldown; the master idle case was not run. These are single samples on a loaded host and neither establish nor rule out a regression. They are not current-head acceptance or a general benchmark; no safety wait was shortened.