Repository navigation
fix: prioritize explicit contact linking - #1451
Conversation
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
Review: diff 5 files.
synonymdev/bitkit-ios#905 implements the same per-contact retry schedule, coalesced 20-second foreground window, and readiness refresh after linked intake; the default iOS branch still uses a shared retry schedule.
Findings:
2 inline (1 HIGH, 1 LOW)
QA:
No reviewer test ran because the PR description lists only automated checks and device testing already completed by the author.
Reviewed by gpt-6-astra-medium via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Lets an explicit add or refresh of a contact jump ahead of background link maintenance, with its own short interactive retry window per peer. Reviewed ea8d728, full tier, reasoned from the code and CI (this branch was not built here).
What I checked, and 4 candidates I ruled out
Read: the full diff of PrivatePaykitRepo.kt and AddContactViewModel.kt, plus the functions around it: advanceLinkIfIdle, advancePendingPrivateLinks, preparePrivateLinks, schedulePendingPrivateMessageDrainRetries, unresolvedPrivateLinkResult
Call sites traced: RefreshContactPaykitLinkUseCase from AddContactViewModel, ContactsViewModel and AppViewModel; PaykitSdkService.ensureLinkWithPeer down to PaykitSdkOperationLock
CI: build red on 1 of 3628 unit tests, PubkyAuthManifestTest > signup and authorization links resolve only through their enabled aliases (AssertionError: pubkyring://signup?hs=homeserver). The PR touches no manifest, alias or auth-link code, and CI on master at the base was green, so I could not tie this failure to the change. lint and detekt green, e2e still running.
Ruled out
- Dependency cycle from injecting
PaykitPaymentRequestRepo: its constructor takes nothing that leads back toPrivatePaykitRepo. - Retry loops swallowing cancellation:
runSuspendCatchingrethrowsCancellationException(ext/Coroutines.kt), socloseAndClearand profile deletion still stop them. - An explicit link starved by maintenance already holding the lock:
ensureLinkWithPeerpasses the priority toPaykitSdkOperationLock.withLock, andwaking an explicit peer does not cancel another admitted advancecovers the case where an admitted advance is in flight. - Duplicate taps extending the interactive window:
explicit peer wakes promptly and duplicate intent does not extend interactive retriespins this.
Merge confidence: 3/5, CI is red on PubkyAuthManifestTest for a reason I could not tie to the change.
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
There was a problem hiding this comment.
Suggestion: 👍 Approve
Reaudit: diff 4 files.
No new findings; the rest is in the review.
synonymdev/bitkit-ios#905 at eef17472 carries the same per-contact retry feature and an identical link-contact-after-resume.xml journey; the iOS default branch still has the older shared retry schedule until that PR lands.
QA:
Tests running.
Reviewed by gpt-6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
There was a problem hiding this comment.
Suggestion: 👍 Approve
Note
This result is for 0b3de66, the commit the tests ran on. The pull request has moved to 8cc6c9e since; its new commits get their own review, and the tests that follow drive what they can affect.
Interim test report for the review; the rest resumes on its own.
QA:
Tested on two Android 15 emulators; two Android 16 emulators (Pixel 10 Pro), regtest.
1 not tested because on 0b3de66 + test hook: the held link call returned a Pubky write-lock transport error; ContactPay stayed silent for 35 seconds and 30 seconds while the staging handshake recovered.
🟢 Test J1
Test J1
Saved contact reached Request or Pay after resume and remained unique.
Tip
Test J1 worth a journey
Test J1
- Create two staging wallets and profiles, then save the first profile on the second wallet.
- Open Contacts on the first wallet and add the second profile by its Pubky key.
- Save the contact and check its displayed name.
- Press Home, wait five seconds, and reopen Bitkit.
- Open the saved contact from Contacts without adding it again.
- Tap Pay until Request or Pay is available with an enabled Request button.
- Tap Request and check the amount screen.
- Close the request without sending and check that Contacts has one row for the profile.
Reviewed by gpt-6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
There was a problem hiding this comment.
Suggestion: 👍 Approve
Reaudit: diff 2 files.
No new findings; the rest is in the review.
synonymdev/bitkit-ios#905 at 85421c1 also shares reads across due contact retries while keeping each contact's priority and checking again after changes. Both platforms retain the link-contact-after-resume.xml journey.
QA:
Tests queued.
Reviewed by gpt-6.1-sol-medium via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
There was a problem hiding this comment.
Suggestion: 👍 Approve
Reaudit: diff 4 files.
No new findings; the rest is in the review.
The paired synonymdev/bitkit-ios#905 also pins Paykit rc72 at 03d7380 and carries the same contact-link journey. Both consumers retain private-link retry behavior without app-owned identity reset for observation failures.
QA:
Tests wait for CI.
1 not tested because on 0b3de66 + test hook: the held link call returned a Pubky write-lock transport error; ContactPay stayed silent for 35 seconds and 30 seconds while the staging handshake recovered.
Test J1 passed at 0b3de66.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
There was a problem hiding this comment.
Verdict: ⛔️ Request Changes
Tests for the review: journey J1 fails; Test 1 passes.
QA:
Tested on two Android 16 emulators (Pixel 10 Pro); two Android 15 emulators, regtest.
🟢 Test 1
Test 1
On 04b0b0e + test hook: passed in round 2—admitted work settled in background, later SDK admissions paused, and the retained peer retry resumed without overlap; round 1 stopped observing before the five-minute retry cooldown ended.
1.mp4 | 1-resume.mp4 | 1-r2.mp4 | 1-r2-resume.mp4 | 1-r2-retry.mp4 |
test hook
Debug-only, uncommitted hook: hold one admitted link call for 20 seconds and record admission, return and foreground gates. The native result and retry logic remain unchanged.
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index 01ece7ef7..d7b4da619 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -1,5 +1,7 @@
package to.bitkit.repositories
+import android.util.Log // test hook
+
import com.synonym.bitkitcore.Scanner
import com.synonym.paykit.IdentityStatus
import com.synonym.paykit.LinkedPeerState
@@ -38,6 +40,7 @@ import kotlinx.serialization.encodeToString
import org.lightningdevkit.ldknode.PaymentDirection
import org.lightningdevkit.ldknode.PaymentKind
import org.lightningdevkit.ldknode.PaymentStatus
+import to.bitkit.BuildConfig // test hook
import to.bitkit.App
import to.bitkit.async.appScope
import to.bitkit.data.PrivatePaykitCacheStore
@@ -253,6 +256,7 @@ class PrivatePaykitRepo @Inject constructor(
}
fun setContactPreparationActive(active: Boolean) {
+ if (BuildConfig.DEBUG) Log.i("QaLinkHold", "preparation active=$active") // test hook
isContactPreparationActive.update { active }
}
@@ -1074,6 +1078,7 @@ class PrivatePaykitRepo @Inject constructor(
): LinkedPeerState? {
currentCoroutineContext().ensureActive()
val admittedPriority = contactPreparationPriority(priority)
+ if (BuildConfig.DEBUG) Log.i("QaLinkHold", "retry advancement admitted priority=$admittedPriority peer=${publicKey.take(8)}") // test hook
activeLinkPreparations[publicKey]?.let {
val isExplicitRetry = currentCoroutineContext()[PrivateMessageDrainRetry]?.interactiveUntil != null
return if (isExplicitRetry) it.await() else null
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index af2f61b68..adbf15404 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -1,6 +1,7 @@
package to.bitkit.services
import android.content.Context
+import android.util.Log // test hook
import androidx.annotation.VisibleForTesting
import com.synonym.paykit.ContactRecord
import com.synonym.paykit.ContactUpdate
@@ -71,6 +72,7 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.NonCancellable
import kotlinx.coroutines.currentCoroutineContext
+import kotlinx.coroutines.delay // test hook
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
@@ -86,6 +88,7 @@ import kotlinx.coroutines.sync.withPermit
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
import org.lightningdevkit.ldknode.Network
+import to.bitkit.BuildConfig // test hook
import to.bitkit.async.BaseCoroutineScope
import to.bitkit.data.PubkyStore
import to.bitkit.data.SettingsStore
@@ -828,7 +831,17 @@ class PaykitSdkService @Inject constructor(
isSetup.await()
operationLock.withLock(priority) {
withStateRevisionTracking { handle ->
- completeSdkCall { handle.ensureLinkWithPeer(counterparty, maxAdvanceSteps) }
+ completeSdkCall {
+ if (BuildConfig.DEBUG) Log.i("QaLinkHold", "advance admitted priority=$priority peer=${counterparty.take(8)}") // test hook
+ if (BuildConfig.DEBUG && context.filesDir.resolve("qa_hold_link").delete()) { // test hook
+ Log.i("QaLinkHold", "sdk call held") // test hook
+ delay(20_000) // test hook
+ Log.i("QaLinkHold", "sdk call released") // test hook
+ } // test hook
+ handle.ensureLinkWithPeer(counterparty, maxAdvanceSteps).also { // test hook
+ if (BuildConfig.DEBUG) Log.i("QaLinkHold", "sdk call completed state=${it.state}") // test hook
+ } // test hook
+ }
}
}
}🔴 Test J1
Test J1
Private linking took 73.8 seconds without persistent progress in round 3, after 47.1 seconds in round 1 and 72.9 seconds in round 2; Request worked after resume.
J1.mp4 | J1-r2.mp4 | J1-r2-request.mp4 | J1-r3.mp4 | J1-r3-request.mp4 |
Tip
A saved contact must finish linking promptly and show progress while it is not ready.
Note
Checked manual test 1 in the PR description.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
|
Thanks for the retest. The controlled pending-operation check now passes, and Request worked after resume. I updated the QA notes to reflect that. The 47.1/72.9/73.8-second waits and lack of persistent linking progress are still valid follow-ups in #1419. This PR improves retry scheduling, not end-to-end latency or UI. J1 has explicitly recorded timing without a fixed latency assertion since it was added. Its timing also includes intake, eligibility discovery and the repeated Pay checks, so it does not isolate the Noise handshake. I checked both platforms. The 20-second window bounds foreground priority, not link completion. Pending work continues at background priority after it expires. Persistent link progress was not removed by either PR. I found no introduced lost-retry or lifecycle regression, but these runs alone do not prove a speedup or explain the wait. Could you separate any failed functional J1 step from the latency/progress follow-up and reassess the scheduling change? J1 stays unchecked and #1419 stays open. Redacted queue/SDK/intake/discovery timestamps from these runs would help identify the remaining delay. @ovi-reviewer review |
There was a problem hiding this comment.
Suggestion: 👍 Approve
Review: diff 10 files.
I withdraw the blocking J1 latency and progress finding from the earlier review. Those concerns remain tracked in #1419; J1 has no fixed completion-time assertion, and its recorded functional steps passed. This full re-review found no new findings.
synonymdev/bitkit-ios#905 at 08f02ff carries independent contact deadlines, the 20-second priority window, shared scheduling reads, Paykit rc72 and the identical link-contact-after-resume.xml journey; its default branch still uses the shared retry schedule. End-to-end latency and progress remain tracked in #1419 and synonymdev/bitkit-ios#868.
QA:
Manual test 1 previously passed at 04b0b0e. J1 has no recorded overall pass at this head; use @ovi-reviewer test for a new QA verdict. No device test ran during this re-review.
Replies:
ben-kaufman: Thanks for the retest. The controlled pending-operation check now passes, and Request worked after resume. I updated the QA notes to reflect that. The… (comment)
I withdraw the latency and progress objection as a blocker for this scheduling change. The journey has no fixed latency assertion, all eight functional steps passed in the recorded runs, and the existing follow-up is tracked in #1419. The 20-second window limits foreground priority; pending work continues afterward. The recorded 47.1, 72.9 and 73.8 seconds measure Save to the SDK private-link restoration marker, rather than an isolated Noise handshake or a proven speedup. In the last run, those markers were Save at 02:07:58.245 UTC, public fallback while linking at 02:08:27.656 UTC, and private-link restoration at 02:09:12.017 UTC. The captured logs do not separate queue, SDK, intake and eligibility-discovery durations.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
|
@ben-kaufman ea8d728 is unsigned, and the branch rule that requires signed commits blocks the merge. Re-signing it also brings the branch up to date with master: |
04b0b0e to
a34eaa0
Compare
|
Signing-only update to a34eaa0: all four PR commits now show Verified on GitHub. Every corresponding patch and file tree is unchanged, and a backup branch is retained. Fresh Detekt/format diagnostics exactly match the 19 existing findings. Prior code and device evidence is unchanged; CI will rerun for the new commit hashes. The open linking latency/progress concern remains open. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full review of the complete PR diff from merge base ecc44d4 at a34eaa0. No earlier assessment baseline was inherited.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
Saving a contact while an older Paykit contact sync is still applying its snapshot can drop the new explicit link and withdraw a payment list that retry already published. The following sync prepares the contact again at background priority.
Independent deadlines, the foreground window, publication until queue acknowledgement, blocked peers, and shared scheduling reads match the added unit tests. The CI unit-test step succeeded on this revision; this review did not re-run it. The journey text matches merged bitkit-ios#905 at 08f02ff. That comparison covered the shared journey and the explicit-link unsaved-key check.
Suggested additional test cases
- Android. Leave Paykit contact sync in progress, such as a cold start still activating Paykit or refreshing the app record. Save a new contact before that sync finishes. The explicit link retry should keep running, and a payment list already published for that contact should stay published. After the sync finishes, Request or Pay should become available for that contact without saving it again.
Device testing: not performed in this review.
Findings
- [MEDIUM] Preserve an explicit link when contact sync is already running — inline at
app/src/main/java/to/bitkit/ui/screens/contacts/AddContactViewModel.kt:161.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up review of the changes since a34eaa0 at 98a2f05, using the completed baseline review. Inherited coverage is the per-contact retry window, publication until queue acknowledgement, blocked-peer retirement, and shared scheduling reads; Paykit 0.1.0-rc72 observation-versus-transport classification; the previously reviewed scheduler tests and link-contact-after-resume journey; and the iOS comparison at 08f02ff. This pass re-checked stale contact-sync cancellation through enable, schedule, prune, removal, assignment cleanup, and private-list withdrawal.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The captured-list replacement in the previous finding is guarded: preparation and prune re-check the sign-in and saved-key set immediately before replacing knownSavedContactKeys. An obsolete enable is still reported as success. When private cleanup is already pending, that leaves the flag set after contact payments are turned on, and the next removal retry withdraws private payment lists.
This review did not run unit tests or a device. Build and detekt on this revision passed in CI. The new iOS overlap notes in journeys/contacts/README.md were not compared with bitkit-ios 6db1097f.
Device testing: not performed in this review.
Suggested additional test cases
- Android unit test. Setup:
cleanupPendingis true and contact payments are being turned on for saved contact A. Action: change the saved-contact set after the enable snapshot is read and beforeenableSharingAndPrepareSavedContactspasses its currency check, then runretryPendingEndpointRemoval. Expected: sharing stays on,cleanupPendingis clear, A's private payment list is not withdrawn, and the added contact keeps its explicit retry deadline. - Android unit test. Setup: the same pending cleanup, with contact preparation already active. Action: let that preparation observe the pending flag and drop its keys, then let enable clear the flag and skip scheduling because the snapshot changed. Expected: the current contacts are scheduled again and the explicit deadline is unchanged.
Findings
- [MEDIUM] Obsolete enable leaves private cleanup armed — inline at
app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt:443.
|
I checked the failed send shard on
The fixture had no saved Pubky session and never entered the changed contact-sync path. The affected app paths are unchanged by this PR. Backend connection errors in the teardown dump were startup events or followed the failures, not evidence of their cause. This is not a passing E2E result. The new head will run CI again; the test helpers need focused readiness/dismissal follow-up, and persistent Lightning QR omission would need separate investigation. No assertions or checks were weakened. |
There was a problem hiding this comment.
Suggestion: 👍 Approve
Reaudit: diff 11 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#887 at 6db1097f carries saved-contact snapshot and session guards with the shared overlap instructions. Its follow-up for preparation skipped during deferred cleanup is still pending there; no linking-time improvement is claimed on either platform.
QA:
Tests queued.
Test 1 passed at 04b0b0e (review).
Test J1 passed at 0b3de66.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
There was a problem hiding this comment.
Reviewed at 57a370e. Two MEDIUMs, both reproduced by unit tests: one inline on Add Contact, and one as a reply on the publication retry thread (an explicit retry for a peer with no Paykit records never ends). One question inline about a platform difference.
The tests ran at 98a2f05. The commit since then changes enableSharingAndPrepareSavedContacts and the settings repo; the code both findings sit in is unchanged.
Checked and clean:
- Lock fairness: an interactive waiter never overtakes an ordered one and passes at most three background waiters, and an admitted SDK call is never cancelled.
- Background pause from #1436 is respected by both the explicit and the scheduled lane.
- Retry state is touched only on the serialized dispatcher; retries stop on contact removal, identity mismatch, sign-out, wipe and a blocked peer.
- Stale contact syncs are dropped at each mutation, and an obsolete pass is followed by a fresh one with the current contact set.
- Address reservations: the change adds only the stale-sync guards. An address is still per contact, rotates once used, and survives a failed or cancelled link.
- Cancellation is rethrown and never logged as an error; no SDK or homeserver text reaches the UI.
- The journey file is byte-identical to the iOS one.
- CI: the red
e2e-tests-local - sendshard at the previous head failed in a send spec on amount and channel timing, with no contacts code on that path. At this head the unit build is green and e2e is still running.
Not posted: an obsolete contact sync that aborts inside the backup-state tracking wrapper triggers one wallet backup with nothing changed. Rare, and any thrown SDK call there does the same.
Device gate: 57a370e — link-contact-after-resume.xml passed on two emulators (Pixel Tablet as the first device with a freshly created identity, Pixel 9 as the second, contact payments on for both).
- Save returned in about 1 s and the contact view opened.
⚠️ The "Contact Saved" confirmation was not captured; the saved contact showed the right name. - Home for 5 s, reopened, tapped Pay 22 s after Save: the Pay button spun for 52 s and ended with
recovery_required("Encrypted Link Handshake is still in progress"), with no sheet. - Second Pay tap 229 s after Save: Request or Pay opened within 11 s with Request enabled. I did not retry between 74 s and 229 s, so the link became available somewhere in that window.
- Request opened the Payment Request amount screen. After closing it, Contacts lists GateP3 once.
- The Save stall from the inline finding did not show in this run: nothing else held the SDK lock at that moment.
There was a problem hiding this comment.
Suggestion: 👍 Approve
Reaudit: diff 5 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#887 at 502430d also retires missing-peer retries without dropping queued delivery and carries controlled identity and wait-isolation checks. Its current tests likewise keep link_observation_failed distinct from recovery.
QA:
Tests running.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving at 1c8f938: the two findings from my last review are fixed and I have no new finding in the commit since.
- Never-ending retry: a confirmed
NotFoundnow clears the retry's publication and readiness flags for that peer only (PrivatePaykitRepo.kt:159-164,:1084), and the attempt loop exits when neither flag is set and nothing is queued for the peer (:1435,:1457-1459). Queued outbound work, or a failed state read, keeps the retry. A later explicit refresh starts a new one and clears the cooldown. - Save and the SDK lock: scheduling now receives the cached sign-in and an ownership guard, so it no longer reads the identity through the operation lock before "Contact Saved". The SDK identity is still checked on the first attempt and a mismatch retires the retry before any link or publish.
link_observation_failed: answered, iOS follows Android.- Removal, sign-out and identity switch still cancel the retry, including inside the new exit read.
- The new tests pin both fixes.
Read only for this round. Unit build and lint are green at this head; three e2e shards were still running.
Device gate: link-contact-after-resume.xml passed on two emulators at 57a370e (details in my previous review). 1c8f938.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up at 1c8f938 of the changes since 98a2f05, using the completed baseline review. This pass covered the live saved-contact enable after a pending private withdrawal, explicit-refresh sign-in scheduling, and retirement of a confirmed missing-peer retry while queued outbound delivery still drains. Inherited coverage remains the unchanged prune, removal, assignment, and withdrawal guards; previously reviewed overlap assertions; the 20-second window, blocked-peer retirement, and shared scheduling reads; Paykit 0.1.0-rc72 observation-versus-transport classification; the unchanged link-contact-after-resume journey; and the earlier iOS comparison at 08f02ff.
No new actionable code findings.
Enabling contact payments clears deferred private cleanup while the same sign-in is current, then prepares the saved contacts read after that update. A contact-set change during the cleanup-flag update keeps sharing enabled, leaves the old withdrawal unrun, and keeps an existing explicit retry deadline. A sign-in change inside that update leaves cleanup pending and reports success without preparing contacts.
Explicit refresh passes the current sign-in into scheduling, so Contact Saved does not wait on an SDK identity read. Execution still checks SDK identity before linking. A signed-out refresh does not schedule. A NotFound result for a peer absent from the linked-peer list clears that peer's publication and readiness flags and stops the explicit retry; the five-minute cooldown remains, and a later explicit refresh can try again. Queued outbound work, or a failed link-state read, keeps that retry. The concerns in the 57a370e review match this code and are addressed at this head.
Scoped comparison of bitkit-ios 502430d shows the same missing-peer retirement and drops recovery classification for link_observation_failed. That commit was not a full iOS review.
Validation: the new cases in PrivatePaykitRepoTest.kt, RefreshContactPaykitLinkUseCaseTest.kt, and ContactPaymentSettingsRepoTest.kt were inspected and not executed here; this environment has no Android SDK. On this commit, GitHub reports successful build, lint, detekt, and the local e2e shards. Linking latency remains in #1419.
Ready for device testing.
Device testing: not performed in this review. The link-contact-after-resume device run cited on the pull request is for 57a370e.
|
Thanks for the current-head checks. Save settling within one second while unrelated SDK work is held supports the foreground-wait isolation fix. Tests 2/3 remain partial, and the eligibility delay without visible progress remains open in #1419; I updated the QA notes without marking those checks passed. For Test 3's 62-second reciprocal-link interval, could you include the SDK-hold admission/release, Save, first real retry and readiness timestamps? The posted hook can deliberately hold the SDK queue for up to 60 seconds, so we need to separate that injected time from queue/SDK execution before treating this as normal linking latency. This does not explain or dismiss J1's separate 13-second observation. |
There was a problem hiding this comment.
Verdict: ♻️ Comment
Reaudit: diff 5 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#887 at 502430d also retires missing-peer retries without dropping queued delivery and carries controlled identity and wait-isolation checks. Its current tests likewise keep link_observation_failed distinct from recovery.
QA:
Tested on two Android 16 emulators (Pixel 10 Pro); Android 15 emulator (Pixel 10 Pro); four Android 15 emulators, regtest.
Not tested:
- 2: the controlled setup did not complete two required overlap assertions within the item budget; missing: identity endpoint preservation before old synchronization releases, and re-enable while withdrawal is still pending
- 3: it did not establish missing-peer retirement or the remaining controlled isolation cases; missing: a profile without Paykit records and complete queued-message controls
🟢 Test 1
Test 1
On the alternate lane, held the admitted SDK call for 20.002 seconds, pressed Home, and observed its normal transport-error return 3.022 seconds after release; no later SDK admission ran while the preparation gate was inactive. Resume preserved the same profile and contact without re-adding or restarting, and the retained peer advanced after the five-minute unavailable-link cooldown plus its next scheduling tick: first SDK admission at 320.064 seconds after resume, completion 5.903 seconds later, then two sequential calls reached LINKED state with no overlapping admission. Passed in round 2 on m5a-android-1 after round 1 on m1a could not establish completion because its 96-second resume observation was shorter than the five-minute cooldown; this controlled check makes no normal linking-latency, OS-suspension or process-death claim.
Ran on 04b0b0e. Original report. Device evidence.
1.mp4 | 1-resume.mp4 | 1-r2.mp4 | 1-r2-resume.mp4 | 1-r2-retry.mp4 |
test hook
Debug-only, uncommitted hook: hold one admitted link call for 20 seconds and record admission, return and foreground gates. The native result and retry logic remain unchanged.
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index 01ece7ef7..d7b4da619 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -1,5 +1,7 @@
package to.bitkit.repositories
+import android.util.Log // test hook
+
import com.synonym.bitkitcore.Scanner
import com.synonym.paykit.IdentityStatus
import com.synonym.paykit.LinkedPeerState
@@ -38,6 +40,7 @@ import kotlinx.serialization.encodeToString
import org.lightningdevkit.ldknode.PaymentDirection
import org.lightningdevkit.ldknode.PaymentKind
import org.lightningdevkit.ldknode.PaymentStatus
+import to.bitkit.BuildConfig // test hook
import to.bitkit.App
import to.bitkit.async.appScope
import to.bitkit.data.PrivatePaykitCacheStore
@@ -253,6 +256,7 @@ class PrivatePaykitRepo @Inject constructor(
}
fun setContactPreparationActive(active: Boolean) {
+ if (BuildConfig.DEBUG) Log.i("QaLinkHold", "preparation active=$active") // test hook
isContactPreparationActive.update { active }
}
@@ -1074,6 +1078,7 @@ class PrivatePaykitRepo @Inject constructor(
): LinkedPeerState? {
currentCoroutineContext().ensureActive()
val admittedPriority = contactPreparationPriority(priority)
+ if (BuildConfig.DEBUG) Log.i("QaLinkHold", "retry advancement admitted priority=$admittedPriority peer=${publicKey.take(8)}") // test hook
activeLinkPreparations[publicKey]?.let {
val isExplicitRetry = currentCoroutineContext()[PrivateMessageDrainRetry]?.interactiveUntil != null
return if (isExplicitRetry) it.await() else null
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index af2f61b68..adbf15404 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -1,6 +1,7 @@
package to.bitkit.services
import android.content.Context
+import android.util.Log // test hook
import androidx.annotation.VisibleForTesting
import com.synonym.paykit.ContactRecord
import com.synonym.paykit.ContactUpdate
@@ -71,6 +72,7 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.NonCancellable
import kotlinx.coroutines.currentCoroutineContext
+import kotlinx.coroutines.delay // test hook
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
@@ -86,6 +88,7 @@ import kotlinx.coroutines.sync.withPermit
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
import org.lightningdevkit.ldknode.Network
+import to.bitkit.BuildConfig // test hook
import to.bitkit.async.BaseCoroutineScope
import to.bitkit.data.PubkyStore
import to.bitkit.data.SettingsStore
@@ -828,7 +831,17 @@ class PaykitSdkService @Inject constructor(
isSetup.await()
operationLock.withLock(priority) {
withStateRevisionTracking { handle ->
- completeSdkCall { handle.ensureLinkWithPeer(counterparty, maxAdvanceSteps) }
+ completeSdkCall {
+ if (BuildConfig.DEBUG) Log.i("QaLinkHold", "advance admitted priority=$priority peer=${counterparty.take(8)}") // test hook
+ if (BuildConfig.DEBUG && context.filesDir.resolve("qa_hold_link").delete()) { // test hook
+ Log.i("QaLinkHold", "sdk call held") // test hook
+ delay(20_000) // test hook
+ Log.i("QaLinkHold", "sdk call released") // test hook
+ } // test hook
+ handle.ensureLinkWithPeer(counterparty, maxAdvanceSteps).also { // test hook
+ if (BuildConfig.DEBUG) Log.i("QaLinkHold", "sdk call completed state=${it.state}") // test hook
+ } // test hook
+ }
}
}
}🔴 Test J1
Test J1
Request eligibility had at least13 seconds with no visible progress; the resumed contact still reached the amount screen without being re-added.
J1-part1.mp4 | J1.mp4 | J1-r2-first.mp4 | J1-r2-final.mp4 |
Warning
Slow waits:
- Test J1: Waiting for Request eligibility after the public fallback, with the Contact screen showing no linking progress took 13.0 s with no visible progress.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
This comment has been minimized.
This comment has been minimized.
|
For the new J1 run, could you include the exact unavailable-contact message and redacted logs from the Pay tap through SDK completion/public fallback, plus whether any debug hold was active? Those paths can produce different failures, so the 11-second spinner alone does not identify the cause. Please include the later successful Request timestamp too. I'm keeping J1 and the incomplete overlap/delivery checks open. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
Tested 1c8f938c on an Android emulator with an iOS #887 peer: adding a contact, background/resume, and opening Request worked without re-adding the contact. Controlled foreground-save isolation also passed: Contact Saved appeared while the SDK queue was held, and the retry/deadline survived background/resume.
Build, lint, unit tests and E2E are green. Remaining controlled race checks were not completed. This does not establish a linking-performance improvement or close #1419.
|
Confirmed the Test 3 gap and fixed it in f9fa0b5. A retained NOT_LINKED peer row kept the retry alive after NotFound even when that peer had no outbound work. The retry now remembers that result and retires once fresh scheduling reads confirm no same-peer delivery is queued. Failed reads and transient transport errors still retry, and explicit refresh can try again. The retained-row regression failed before the fix. Final compilation and all 3,658 tests pass, including queued delivery failing once and later draining. Full Detekt/format reports match all 19 baseline findings. All eight commits are signed and verified. Please rerun the missing-peer case on this head. The broader overlap/delivery device checks remain incomplete. I also updated J1 to reflect the clarification that its unfunded fallback toast is expected; the 14-second wait remains open, not explained by this fix. The matching iOS case needs a separate follow-up because #887 has merged. |
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-read at f9fa0b5, with device runs for the missing-contact case. No finding from me in the new commit. My approval at 1c8f938 was given without a device run of this case, and the retained-row gap found by the other review is exactly what that run was for.
The commit: a NotFound now marks the retry as missing (PrivatePaykitRepo.kt:160-166), the link-advance failure path reports it too (:1335), and scheduling drops that peer unless it has queued outbound work (:1480-1482). An explicit refresh clears the mark (:1383). The new test covers queued delivery surviving until it drains.
Device runs, Pixel 9 emulator, dev flavor, contact payments on. I saved a contact and collected the app log for seven minutes each time:
| Head | Contact | Link attempts logged | Error |
|---|---|---|---|
1c8f938 |
valid key with no homeserver | 3 in the first 2.5 min, then none for 5 min | transport_error, "fetch Paykit Noise key authorization" |
1c8f938 |
fresh Bitkit profile, contact payments never enabled | 1 in 7 min | same |
f9fa0b5 |
valid key with no homeserver | 3 in the first 1.5 min, 1 more at 6.5 min | same |
So on staging both kinds of contact come back as a transport error and follow the 5-minute cooldown, which is the behaviour the PR describes for transient errors. Save returned at once in all three runs.
NotFound answer this commit handles. Neither contact above produced it, and I did not find a way to get one from the staging homeserver without a test hook. If there is a plain way to create such a peer, tell me and I will run it. Until then the retirement rests on the unit tests.
link-contact-after-resume.xml (it passed at 57a370e).
There was a problem hiding this comment.
Suggestion: 👍 Approve
Reaudit: diff 3 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#887 has merged. Its retry still lacks the retained-unlinked-row exclusion added here; @ben-kaufman acknowledges a separate iOS follow-up in the PR discussion.
QA:
Tests running.
Test 1 passed at 04b0b0e (review).
2 not tested because the controlled cleanup did not let sharing enable reach the changed-contact overlap before the time limit; missing: enable entry with cleanup pending, different final contacts, a live new assignment and later removal proof.
Tests 3, J1 failed at 1c8f938.
Replies:
@ben-kaufman: Could you separate any failed functional J1 step from the latency/progress follow-up? (comment)
The later 1c8f938 J1 record completed the Request route and clarified the unfunded fallback; its Pay delay has no established attribution to this delta. I found no new defect in the retained-row fix, and the current-head device checks remain queued.
@ben-kaufman: For Test 3’s 62-second reciprocal-link interval, could you include the timestamps? (comment)
I cannot isolate that interval from the injected hold using the recorded evidence, so I am not treating it as ordinary linking latency. The incomplete isolation and delivery checks remain open.
@ben-kaufman: Could you include the exact unavailable-contact message and whether any debug hold was active? (comment)
The final 1c8f938 J1 record identifies the toast as Insufficient Savings, with zero balances and no debug hold; Request later completed without sending a payment. The 14-second Pay wait remains unattributed and does not establish a new error-handling regression.
@ben-kaufman: Confirmed the Test 3 gap and fixed it in f9fa0b5. (comment)
I verified the retained-row fix and the same-peer queued-delivery regression in the delta. Its controlled device verification remains queued; the broader overlap checks and the separate iOS follow-up remain open.
Reviewed by gpt-6.1-sol-medium via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
|
Interim test results for f9fa0b5 The run is still going; the final review follows when it ends. This is a progress note, nothing is approved or requested here. This comment is updated as tests finish and removed when the final result posts. 1 passed so far, 1 failed, 2 not tested. 🔴 Test J1: failed at f9fa0b5 (app issue); round 2 is driving itPay showed a spinner for at least 10 seconds before Insufficient Savings; Request worked after resume without re-adding. J1-1.mp4🟠 Test 2: not tested because its identity-change and pending-cleanup controls were not ready before the case limit; missing: completed four-case overlap setup; waits for round 2On f9fa0b5 + test hook: overlap setup did not complete all four controlled cases before the 20-minute case limit. test hookNot pushed. Bounded original captured refresh, guarded cleanup, enable and deferred-removal holds; post-real-publication hold outside SDK lock, actual identity/deadline/active/PRESENT snapshots and durable trace. No fabricated outcomes or store writes. diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index b2f6aa868..bda24b2c1 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -1,5 +1,7 @@
package to.bitkit.repositories
+import to.bitkit.utils.QaContactSyncHook
+
import com.synonym.bitkitcore.Scanner
import com.synonym.paykit.IdentityStatus
import com.synonym.paykit.LinkedPeerState
@@ -243,9 +245,12 @@ class PrivatePaykitRepo @Inject constructor(
): Result<Unit> = withContext(serializedDispatcher) {
runContactSync {
checkPaykitContactSync(isStillCurrent)
+ QaContactSyncHook.log("enable cleanupBefore=${isContactSharingCleanupPending()} retries=${pendingMessageDrainRetries.values.map { it.publicKey to it.interactiveUntil }}") // test hook
+ QaContactSyncHook.hold("enable") // test hook
updateContactSharingCleanupPending(false, isStillCurrent)
checkPaykitContactSync(isStillCurrent)
val keys = rememberSavedContacts(savedPublicKeys(), replacing = true)
+ QaContactSyncHook.log("enable cleanupAfter=${isContactSharingCleanupPending()} keys=$keys retries=${pendingMessageDrainRetries.values.map { it.publicKey to it.interactiveUntil }}") // test hook
scheduleContactPreparation(keys)
}
}
@@ -389,9 +394,11 @@ class PrivatePaykitRepo @Inject constructor(
): Result<Unit> = withContext(serializedDispatcher) {
runSuspendCatching {
if (isDeletingProfile) return@runSuspendCatching
+ QaContactSyncHook.hold("removal_retry") // test hook: hold before reading deferred cleanup, so re-enable can retire it
val settings = settingsStore.data.first()
val hasDisabledPublications = !settings.sharesPrivatePaykitEndpoints && hasPublishedPrivateEndpoints()
val cleanupPending = isContactSharingCleanupPending()
+ QaContactSyncHook.log("removalRetry cleanupPending=$cleanupPending sharesPrivate=${settings.sharesPrivatePaykitEndpoints} hasDisabled=$hasDisabledPublications retries=${pendingMessageDrainRetries.values.map { it.publicKey to it.interactiveUntil }}") // test hook
if (cleanupPending || hasDisabledPublications) {
if (!cleanupPending) updateContactSharingCleanupPending(true)
removePublishedEndpoints().getOrThrow()
@@ -411,6 +418,8 @@ class PrivatePaykitRepo @Inject constructor(
): Result<Unit> = withContext(serializedDispatcher) {
runContactSync {
checkPaykitContactSync(isStillCurrent)
+ QaContactSyncHook.hold("cleanup")
+ QaContactSyncHook.log("prune before guard current=${isStillCurrent?.invoke()} known=$knownSavedContactKeys retries=${pendingMessageDrainRetries.values.map { it.publicKey to it.interactiveUntil }}")
val contacts = ensureState().contacts
checkPaykitContactSync(isStillCurrent)
val savedKeys = rememberSavedContacts(savedPublicKeys, replacing = true).toSet()
@@ -428,6 +437,8 @@ class PrivatePaykitRepo @Inject constructor(
isStillCurrent: (() -> Boolean)? = null,
): Result<Unit> = withContext(serializedDispatcher) {
runContactSync {
+ QaContactSyncHook.hold("remove")
+ QaContactSyncHook.log("remove before guard current=${isStillCurrent?.invoke()} requested=$publicKeys retries=${pendingMessageDrainRetries.values.map { it.publicKey to it.interactiveUntil }}")
checkPaykitContactSync(isStillCurrent)
val keys = publicKeys.mapNotNull(::normalizedPublicKey).toSet()
if (keys.isEmpty()) return@runContactSync
@@ -1361,11 +1372,13 @@ class PrivatePaykitRepo @Inject constructor(
pendingMessageDrainRetries.put(publicKey, it)?.job?.cancel()
}
if (explicit) promoteExplicitLinkRetry(retry)
+ QaContactSyncHook.log("retry scheduled peer=$publicKey deadline=${retry.interactiveUntil} known=${publicKey in knownSavedContactKeys} generation=${retry.generation}")
if (retry.job == null) {
retry.job = retryScope.launch(retry) {
try {
runPrivateMessageDrainRetries(retry, reason)
} finally {
+ QaContactSyncHook.log("retry finally peer=$publicKey identity=${retry.identity} deadline=${retry.interactiveUntil} active=${retry.job?.isActive} owns=${pendingMessageDrainRetries[publicKey] === retry}")
if (pendingMessageDrainRetries[publicKey] === retry) {
pendingMessageDrainRetries.remove(publicKey)
}
@@ -1385,6 +1398,10 @@ class PrivatePaykitRepo @Inject constructor(
retry.wake.trySend(Unit)
}
+ suspend fun qaContactSnapshot() = withContext(serializedDispatcher) {
+ QaContactSyncHook.log("snapshot cleanup=${isContactSharingCleanupPending()} keys=$knownSavedContactKeys retries=${pendingMessageDrainRetries.values.map { "peer=${it.publicKey},identity=${it.identity},deadline=${it.interactiveUntil},active=${it.job?.isActive},prepare=${it.prepareEndpoints},readiness=${it.refreshReadiness}" }}")
+ }
+
private suspend fun runPrivateMessageDrainRetries(retry: PrivateMessageDrainRetry, reason: String) {
var retryIndex = 0
while (true) {
@@ -1404,7 +1421,10 @@ class PrivatePaykitRepo @Inject constructor(
val keepRetrying = runSuspendCatching { drainPrivateMessageRetry(retry, reason, priority) }
.onFailure { Logger.warn("Failed to retry private Paykit messages", it, context = TAG) }
.getOrDefault(true)
- if (!keepRetrying && !retry.prepareEndpoints) return
+ if (!keepRetrying && !retry.prepareEndpoints) {
+ QaContactSyncHook.log("retry naturally retired peer=${retry.publicKey} identity=${retry.identity} deadline=${retry.interactiveUntil}")
+ return
+ }
if (!wasWoken) retryIndex = (retryIndex + 1).coerceAtMost(privateMessageDrainRetryDelays.lastIndex)
}
}
@@ -1436,6 +1456,8 @@ class PrivatePaykitRepo @Inject constructor(
if (retry.prepareEndpoints) {
if (canPublishPrivateEndpoints(status)) {
publishLocalEndpoints(keys, reason, priority = priority).getOrThrow()
+ QaContactSyncHook.log("retry published peer=${retry.publicKey} identity=${retry.identity} deadline=${retry.interactiveUntil} present=${pendingMessageDrainRetries[retry.publicKey] === retry} prepare=${retry.prepareEndpoints} readiness=${retry.refreshReadiness}")
+ QaContactSyncHook.hold("retry_publish_${retry.publicKey}")
return hasPendingPrivateMessageRetry(retry, priority)
}
retry.prepareEndpoints = false
@@ -1742,6 +1764,7 @@ class PrivatePaykitRepo @Inject constructor(
isStillCurrent: (() -> Boolean)? = null,
): Result<Unit> =
runSuspendCatching {
+ QaContactSyncHook.log("withdraw entered requested=$publicKeys identity=${pubkyService.currentPublicKey()}") // test hook
checkPaykitContactSync(isStillCurrent)
val peers = paykitSdkService.linkedPeers()
checkPaykitContactSync(isStillCurrent)
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 91d27abdb..4d068f5b1 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -615,6 +615,37 @@ class PubkyRepo @Inject constructor(
* found, never the result of a failed or empty lookup. Like iOS, it ends the current [PubkySignIn] before it
* installs the adopted session, so an adoption that fails at sign-in ends it too.
*/
+ // test hook: 2; production authentication and ownership gates, private fixture credential only.
+ suspend fun qaContactIdentityControl(operation: String): Result<Unit> = withContext(ioDispatcher) {
+ runSuspendCatching {
+ check(to.bitkit.BuildConfig.DEBUG)
+ val secretFile = java.io.File("/data/data/${to.bitkit.BuildConfig.APPLICATION_ID}/files/qa_identity_secret")
+ if (operation == "export") {
+ secretFile.writeText(requireNotNull(activeSecretKeyHex()))
+ to.bitkit.utils.QaContactSyncHook.log("auth exported publicKey=${currentSignIn()?.publicKey}")
+ return@runSuspendCatching
+ }
+ check(operation == "switch")
+ val secret = secretFile.readText().trim()
+ secretFile.delete()
+ val before = currentSignIn()
+ initializeMutex.withLock {
+ ensureServiceInitialized()
+ val rawPublicKey = pubkyService.publicKeyFromSecret(secret)
+ check(!PubkyPublicKeyFormat.matches(rawPublicKey, before?.publicKey ?: ""))
+ signInGeneration.incrementAndGet()
+ signInOrSignUpAdoptedIdentity(secret, rawPublicKey)
+ val publicKey = rawPublicKey.ensurePubkyPrefix()
+ clearProfileIfIdentityChanged(publicKey)
+ startSignIn(publicKey)
+ notifyBackupStateChanged()
+ }
+ loadProfile()
+ loadContacts()
+ to.bitkit.utils.QaContactSyncHook.log("auth switched previous=${before?.publicKey} current=${currentSignIn()?.publicKey} sdk=${pubkyService.currentPublicKey()} oldCurrent=${before?.let(::isCurrent)}")
+ }
+ }
+
suspend fun adoptRingIdentity(
pubky: String,
knownProfile: () -> PubkyProfile? = { null },
diff --git a/app/src/main/java/to/bitkit/ui/MainActivity.kt b/app/src/main/java/to/bitkit/ui/MainActivity.kt
index c17983214..61c2bbbfc 100644
--- a/app/src/main/java/to/bitkit/ui/MainActivity.kt
+++ b/app/src/main/java/to/bitkit/ui/MainActivity.kt
@@ -1,5 +1,10 @@
package to.bitkit.ui
+import androidx.lifecycle.lifecycleScope
+import to.bitkit.BuildConfig
+import to.bitkit.repositories.PubkyRepo
+import to.bitkit.repositories.PrivatePaykitRepo
+import to.bitkit.utils.QaContactSyncHook
import android.app.NotificationManager
import android.content.Intent
import android.hardware.usb.UsbDevice
@@ -94,6 +99,12 @@ class MainActivity : FragmentActivity() {
@Inject
lateinit var hwWalletRepo: HwWalletRepo
+ @Inject
+ lateinit var qaPubkyRepo: PubkyRepo // test hook
+
+ @Inject
+ lateinit var qaPrivateRepo: PrivatePaykitRepo
+
private val appViewModel by viewModels<AppViewModel>()
private val walletViewModel by viewModels<WalletViewModel>()
private val blocktankViewModel by viewModels<BlocktankViewModel>()
@@ -287,6 +298,17 @@ class MainActivity : FragmentActivity() {
}
private fun handleLaunchIntent(intent: Intent) {
+ if (BuildConfig.DEBUG && intent.action == "to.bitkit.QA_CONTACT_SNAPSHOT") {
+ lifecycleScope.launch { qaPrivateRepo.qaContactSnapshot() }
+ return
+ }
+ if (BuildConfig.DEBUG && intent.action == "to.bitkit.QA_CONTACT_IDENTITY") { // test hook
+ lifecycleScope.launch {
+ qaPubkyRepo.qaContactIdentityControl(intent.getStringExtra("operation") ?: "")
+ .onFailure { QaContactSyncHook.log("auth fixture failed kind=${it.javaClass.simpleName}") }
+ }
+ return
+ }
if (intent.getBooleanExtra(EXTRA_PAYKIT_SUBSCRIPTION_PAYMENT_DUE, false)) {
intent.removeExtra(EXTRA_PAYKIT_SUBSCRIPTION_PAYMENT_DUE)
appViewModel.onPaykitSubscriptionNotificationTapped(
diff --git a/app/src/main/java/to/bitkit/utils/QaContactSyncHook.kt b/app/src/main/java/to/bitkit/utils/QaContactSyncHook.kt
new file mode 100644
index 000000000..4a4f20815
--- /dev/null
+++ b/app/src/main/java/to/bitkit/utils/QaContactSyncHook.kt
@@ -0,0 +1,38 @@
+package to.bitkit.utils
+
+import android.util.Log
+import java.io.File
+import java.util.UUID
+import kotlinx.coroutines.withTimeoutOrNull
+import kotlinx.coroutines.delay
+import to.bitkit.BuildConfig
+
+// test hook: 2; hold/log only, never alter contact sync decisions.
+object QaContactSyncHook {
+ fun log(value: String) {
+ if (BuildConfig.DEBUG) {
+ Log.i("QaContactSync", value) // test hook
+ File("/data/data/${BuildConfig.APPLICATION_ID}/files/qa_contact_trace.log").appendText("${java.time.Instant.now()} $value\n") // test hook: durable observations survive driver logcat clears
+ }
+ }
+ fun consume(stage: String): Boolean = BuildConfig.DEBUG && File("/data/data/${BuildConfig.APPLICATION_ID}/files/qa_$stage").delete()
+ suspend fun hold(stage: String) {
+ if (!BuildConfig.DEBUG) return
+ val gate = File("/data/data/${BuildConfig.APPLICATION_ID}/files/qa_hold_$stage")
+ val claimed = File(gate.parentFile, "qa_held_${stage}_${UUID.randomUUID()}")
+ if (!gate.renameTo(claimed)) return
+ log("hold entered stage=$stage token=${claimed.name}")
+ try {
+ val released = withTimeoutOrNull(300_000L) {
+ while (claimed.exists()) delay(100)
+ true
+ }
+ log("hold released stage=$stage token=${claimed.name} timeout=${released == null}")
+ } catch (e: kotlinx.coroutines.CancellationException) {
+ log("hold canceled stage=$stage token=${claimed.name}")
+ throw e
+ } finally {
+ claimed.delete()
+ }
+ }
+}
diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
index 91ff986ab..8a637fcb4 100644
--- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
+++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
@@ -79,6 +79,7 @@ import org.lightningdevkit.ldknode.PaymentId
import org.lightningdevkit.ldknode.SpendableUtxo
import org.lightningdevkit.ldknode.SyncType
import org.lightningdevkit.ldknode.Txid
+import to.bitkit.utils.QaContactSyncHook
import to.bitkit.BuildConfig
import to.bitkit.R
import to.bitkit.data.CacheStore
@@ -795,6 +796,8 @@ class AppViewModel @Inject constructor(
paykitPaymentRequestRepo.activate(state.publicKey)
if (!pubkyRepo.isCurrent(signIn)) return
paymentRequestIdentity = state.publicKey
+ QaContactSyncHook.log("sync refresh capturedCount=${state.contactKeys.size} capturedKeys=${state.contactKeys} identity=${state.publicKey} identityChanged=$identityChanged")
+ QaContactSyncHook.hold("refresh")
refreshPrivateOnlyPaykitApp("contact sync", onlyIfNeeded = !identityChanged)
if (!state.contactsLoaded || !synchronizeSavedPaykitContacts(state.contactKeys, isStillCurrent)) return
if (isPaymentRequestPollingStopped.value) return
@@ -818,6 +821,7 @@ class AppViewModel @Inject constructor(
contactKeys: Set<String>,
isStillCurrent: () -> Boolean,
): Boolean {
+ QaContactSyncHook.log("sync guard current=${isStillCurrent()} capturedCount=${contactKeys.size}")
if (!isStillCurrent()) return false
val removedKeys = lastPrivatePaykitContactKeys - contactKeys
if (removedKeys.isNotEmpty()) {
@@ -831,6 +835,7 @@ class AppViewModel @Inject constructor(
privatePaykitRepo.pruneUnsavedContactState(contactKeys, isStillCurrent)
.onFailure { Logger.warn("Failed to prune private Paykit contact state", it, context = TAG) }
if (!isStillCurrent()) return false
+ QaContactSyncHook.log("sync settled capturedCount=${contactKeys.size}")
lastPrivatePaykitContactKeys = contactKeys
return true
}🟠 Test 3: not tested because it needs a retained unlinked contact with a queued message; missing: ready contact and message setup; waits for round 2On f9fa0b5 + test hook: retained-contact and queued-recovery setup stopped before its assertions; foreground save, identity isolation and missing-contact retirement were observed. test hookNot pushed. Holds unrelated SDK work after contact persistence, logs sign-in/retry scheduling and actual SDK rows, queues real requests, and exposes debug-only native peer block/unblock setup; no retry decisions replaced. diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index b2f6aa868..84fcfe9e7 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -1,5 +1,6 @@
package to.bitkit.repositories
+import to.bitkit.utils.QaWaitIsolation
import com.synonym.bitkitcore.Scanner
import com.synonym.paykit.IdentityStatus
import com.synonym.paykit.LinkedPeerState
@@ -1069,7 +1070,7 @@ class PrivatePaykitRepo @Inject constructor(
if (peerStates[publicKey] == LinkedPeerState.LINKED) {
LinkedPeerState.LINKED
} else {
- advanceLinkIfIdle(publicKey, priority)
+ run { QaWaitIsolation.hold("prepare_$publicKey"); advanceLinkIfIdle(publicKey, priority) }
}
}.onSuccess {
unavailableLinkRetryAt.remove(publicKey)
@@ -1081,6 +1082,7 @@ class PrivatePaykitRepo @Inject constructor(
if (generation != preparationGeneration) return@onFailure
val isUnavailable = isPrivateLinkUnavailable(publicKey, it, priority)
if (generation != preparationGeneration) return@onFailure
+ to.bitkit.utils.QaWaitIsolation.log("prepare result peer=$publicKey unavailable=$isUnavailable error=${it.javaClass.simpleName}") // test hook
if (isUnavailable) {
unavailableLinkRetryAt[publicKey] = clock.now() + unavailableLinkRetryDelay
currentCoroutineContext()[PrivateMessageDrainRetry]?.stopIfUnavailable(publicKey, it)
@@ -1272,10 +1274,11 @@ class PrivatePaykitRepo @Inject constructor(
awaitContactPreparationActive(priority)
currentCoroutineContext().ensureActive()
if (generation != preparationGeneration) return@runSuspendCatching emptySet<String>()
- val pendingKeys = paykitSdkService.pendingOutboundPrivateCounterparties(
+ val pendingFull = paykitSdkService.pendingOutboundPrivateCounterparties(
contactPreparationPriority(priority)
- )
- .mapNotNull(::normalizedPublicKey).toSet().intersect(retryKeys)
+ ).mapNotNull(::normalizedPublicKey).toSet()
+ val pendingKeys = pendingFull.intersect(retryKeys)
+ QaWaitIsolation.log("drain reason=$reason retryKeys=$retryKeys pendingFull=$pendingFull selected=$pendingKeys")
pendingKeys.forEach { publicKey ->
awaitContactPreparationActive(priority)
currentCoroutineContext().ensureActive()
@@ -1292,6 +1295,7 @@ class PrivatePaykitRepo @Inject constructor(
val linkedKeys = paykitSdkService.linkedPeers(contactPreparationPriority(priority))
.filter { it.state == LinkedPeerState.LINKED }
.mapNotNull { normalizedPublicKey(it.counterparty) }.toSet().intersect(retryKeys)
+ QaWaitIsolation.log("drain receive reason=$reason retryKeys=$retryKeys selected=$linkedKeys")
val receivedKeys = mutableSetOf<String>()
linkedKeys.forEach { publicKey ->
awaitContactPreparationActive(priority)
@@ -1361,6 +1365,7 @@ class PrivatePaykitRepo @Inject constructor(
pendingMessageDrainRetries.put(publicKey, it)?.job?.cancel()
}
if (explicit) promoteExplicitLinkRetry(retry)
+ to.bitkit.utils.QaWaitIsolation.log("retry scheduled peer=$publicKey identity=$identity deadline=${retry.interactiveUntil}") // test hook
if (retry.job == null) {
retry.job = retryScope.launch(retry) {
try {
@@ -1368,6 +1373,7 @@ class PrivatePaykitRepo @Inject constructor(
} finally {
if (pendingMessageDrainRetries[publicKey] === retry) {
pendingMessageDrainRetries.remove(publicKey)
+ to.bitkit.utils.QaWaitIsolation.log("retry retired peer=$publicKey identity=${retry.identity} prepare=${retry.prepareEndpoints} readiness=${retry.refreshReadiness}") // test hook
}
}
}
@@ -1425,7 +1431,9 @@ class PrivatePaykitRepo @Inject constructor(
receiveLinkedPeers = retry.refreshReadiness,
)
}
+ to.bitkit.utils.QaWaitIsolation.log("retry identity lookup peer=${retry.publicKey} identity=${retry.identity}") // test hook
val status = paykitSdkService.identityStatus(contactPreparationPriority(priority))
+ to.bitkit.utils.QaWaitIsolation.log("retry lookup returned peer=${retry.publicKey} active=${status?.publicKey}") // test hook
if (!PubkyPublicKeyFormat.matches(status?.publicKey, retry.identity) ||
status?.capability != PubkyIdentityCapability.PRIVATE_LINK_CAPABLE
) {
@@ -1558,6 +1566,7 @@ class PrivatePaykitRepo @Inject constructor(
.mapNotNull(::normalizedPublicKey)
.toSet()
+ QaWaitIsolation.log("fresh scheduling state peers=$linkedPeers pending=$pendingOutbound") // test hook
return PrivateMessageDrainState(linkedPeers, pendingOutbound)
}
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 91d27abdb..6afc6ff7c 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -615,6 +615,37 @@ class PubkyRepo @Inject constructor(
* found, never the result of a failed or empty lookup. Like iOS, it ends the current [PubkySignIn] before it
* installs the adopted session, so an adoption that fails at sign-in ends it too.
*/
+ // test hook: 3; production authentication and ownership gates, private fixture credential only.
+ suspend fun qaContactIdentityControl(operation: String): Result<Unit> = withContext(ioDispatcher) {
+ runSuspendCatching {
+ check(to.bitkit.BuildConfig.DEBUG)
+ val secretFile = java.io.File("/data/data/${to.bitkit.BuildConfig.APPLICATION_ID}/files/qa_identity_secret")
+ if (operation == "export") {
+ secretFile.writeText(requireNotNull(activeSecretKeyHex()))
+ to.bitkit.utils.QaWaitIsolation.log("auth exported publicKey=${currentSignIn()?.publicKey}")
+ return@runSuspendCatching
+ }
+ check(operation == "switch")
+ val secret = secretFile.readText().trim()
+ secretFile.delete()
+ val before = currentSignIn()
+ initializeMutex.withLock {
+ ensureServiceInitialized()
+ val rawPublicKey = pubkyService.publicKeyFromSecret(secret)
+ check(!PubkyPublicKeyFormat.matches(rawPublicKey, before?.publicKey ?: ""))
+ signInGeneration.incrementAndGet()
+ signInOrSignUpAdoptedIdentity(secret, rawPublicKey)
+ val publicKey = rawPublicKey.ensurePubkyPrefix()
+ clearProfileIfIdentityChanged(publicKey)
+ startSignIn(publicKey)
+ notifyBackupStateChanged()
+ }
+ loadProfile()
+ loadContacts()
+ to.bitkit.utils.QaWaitIsolation.log("auth switched previous=${before?.publicKey} current=${currentSignIn()?.publicKey} sdk=${pubkyService.currentPublicKey()} oldCurrent=${before?.let(::isCurrent)}")
+ }
+ }
+
suspend fun adoptRingIdentity(
pubky: String,
knownProfile: () -> PubkyProfile? = { null },
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index f04d68ee7..1717a2551 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -1,5 +1,7 @@
package to.bitkit.services
+import to.bitkit.BuildConfig
+import to.bitkit.utils.QaWaitIsolation
import android.content.Context
import androidx.annotation.VisibleForTesting
import com.synonym.paykit.ContactRecord
@@ -414,6 +416,25 @@ class PaykitSdkService @Inject constructor(
return bootstrap().republishIdentity(publicKey)
}
+ suspend fun qaHoldAfterPersistence() { // test hook
+ if (!to.bitkit.BuildConfig.DEBUG) return // test hook
+ val gate = java.io.File(context.filesDir, "qa_hold_sdk") // test hook
+ if (!gate.exists()) return // test hook
+ val admitted = CompletableDeferred<Unit>() // test hook
+ launch { // test hook
+ operationLock.withLock { // test hook
+ android.util.Log.i("QaWaitIsolation", "SDK hold admitted") // test hook
+ admitted.complete(Unit) // test hook
+ try { // test hook
+ kotlinx.coroutines.withTimeoutOrNull(60000) { // test hook
+ while (gate.exists()) kotlinx.coroutines.delay(100) // test hook
+ } // test hook
+ } finally { android.util.Log.i("QaWaitIsolation", "SDK hold released") } // test hook
+ } // test hook
+ } // test hook
+ admitted.await() // test hook
+ } // test hook
+
suspend fun currentPublicKey(): String? {
isSetup.await()
return operationLock.withLock {
@@ -900,6 +921,7 @@ class PaykitSdkService @Inject constructor(
isSetup.await()
operationLock.withLock(priority) {
withStateRevisionTracking { handle ->
+ QaWaitIsolation.log("sdk receive admitted peer=$counterparty priority=$priority")
completeSdkCall { handle.receivePrivateMessages(counterparty) }
}
}
@@ -917,6 +939,11 @@ class PaykitSdkService @Inject constructor(
expectedIdentity: String?,
) = run {
isSetup.await()
+ QaWaitIsolation.hold("outbound_$counterparty")
+ if (BuildConfig.DEBUG && java.io.File(context.filesDir, "qa_fail_outbound_$counterparty").delete()) { // test hook
+ QaWaitIsolation.log("injected outbound failure peer=$counterparty") // test hook
+ throw PaykitException.Transport("qa_one_off", "QA delivery failure") // test hook
+ } // test hook
val generation = runtimeGeneration
val deferDuringPayment = priority == Priority.Background
var report: OutboundPrivateSendReport?
@@ -935,6 +962,7 @@ class PaykitSdkService @Inject constructor(
}
}
if (deferDuringPayment && isPaymentSubmissionActive.value) return@withStateRevisionTracking null
+ QaWaitIsolation.log("sdk send admitted peer=$counterparty priority=$priority")
completeSdkCall { handle.processOutboundPrivateMessages(counterparty) }
}
}
@@ -989,6 +1017,37 @@ class PaykitSdkService @Inject constructor(
}
}
+ suspend fun qaMessageControl(operation: String, peer: String): Result<Unit> = runSuspendCatching {
+ check(BuildConfig.DEBUG)
+ when (operation) {
+ "queue" -> {
+ val identity = requireNotNull(identityStatus()?.publicKey)
+ val proposal = PaykitPaymentRequestProposalTerms(
+ amountValue = "0.00000001",
+ paymentReference = "bitkit-${java.util.UUID.randomUUID()}",
+ proposalExpiresAt = "2026-10-10T23:59:00Z",
+ acceptedPaymentEndpointIdentifiers = listOf("lightning"),
+ metadataJson = "{}",
+ )
+ val record = proposePaymentRequest(peer, proposal, identity)
+ QaWaitIsolation.log("queue created peer=$peer record=$record")
+ }
+ "deliver" -> QaWaitIsolation.log("delivery result peer=$peer report=${processOutboundPrivateMessages(peer)}")
+ "retained" -> operationLock.withLock { // test hook
+ withStateRevisionTracking { handle -> // test hook
+ completeSdkCall { handle.blockPeer(peer) } // test hook
+ val row = completeSdkCall { handle.unblockPeer(peer) } // test hook
+ QaWaitIsolation.log("retained actual=$row") // test hook
+ } // test hook
+ } // test hook
+ "pending" -> QaWaitIsolation.log("pending actual=${pendingOutboundPrivateCounterparties()} peers=${linkedPeers()}")
+ "identity" -> QaWaitIsolation.log("identity actual=${identityStatus()?.publicKey}")
+ else -> error("Unknown QA message operation")
+ }
+ QaWaitIsolation.log("message control settled operation=$operation peer=$peer")
+ Unit
+ }
+
suspend fun identityStatus(): IdentityStatus? = identityStatus(Priority.Ordered)
internal suspend fun identityStatus(priority: Priority): IdentityStatus? {
@@ -1197,6 +1256,7 @@ class PaykitSdkService @Inject constructor(
counterparty: String,
): PaykitPublicContactPaymentResolution {
val resolution = publicRead { handle ->
+ to.bitkit.utils.QaWaitIsolation.hold("public_$counterparty")
val result = handle.resolvePublicContactPayment(counterparty, amount = null)
check(sdk === handle) { "Paykit runtime changed while resolving public payment endpoints" }
result
@@ -1364,7 +1424,12 @@ class PaykitSdkService @Inject constructor(
if (result.sessionAccess.exportLocalSecretKey() != null) {
completeSdkCall { handle.publishPaykitNoiseKeyAuthorization() }
}
- publishAppIfLiveSessionAvailable(handle, identityStatus)
+ if (to.bitkit.BuildConfig.DEBUG && java.io.File(context.filesDir, "qa_hold_publish").exists()) {
+ launch {
+ to.bitkit.utils.QaWaitIsolation.hold("publish")
+ publishAppIfLiveSessionAvailable(handle, identityStatus)
+ }
+ } else publishAppIfLiveSessionAvailable(handle, identityStatus)
launchIdentityRepublish(publicKey = result.publicKey)
}
diff --git a/app/src/main/java/to/bitkit/ui/MainActivity.kt b/app/src/main/java/to/bitkit/ui/MainActivity.kt
index c17983214..543fb3e31 100644
--- a/app/src/main/java/to/bitkit/ui/MainActivity.kt
+++ b/app/src/main/java/to/bitkit/ui/MainActivity.kt
@@ -1,5 +1,11 @@
package to.bitkit.ui
+import androidx.lifecycle.lifecycleScope
+import to.bitkit.BuildConfig
+import to.bitkit.repositories.PubkyRepo
+import to.bitkit.services.PaykitSdkService
+import to.bitkit.utils.QaWaitIsolation
+
import android.app.NotificationManager
import android.content.Intent
import android.hardware.usb.UsbDevice
@@ -94,6 +100,12 @@ class MainActivity : FragmentActivity() {
@Inject
lateinit var hwWalletRepo: HwWalletRepo
+ @Inject
+ lateinit var qaPubkyRepo: PubkyRepo // test hook
+
+ @Inject
+ lateinit var qaSdk: PaykitSdkService
+
private val appViewModel by viewModels<AppViewModel>()
private val walletViewModel by viewModels<WalletViewModel>()
private val blocktankViewModel by viewModels<BlocktankViewModel>()
@@ -287,6 +299,21 @@ class MainActivity : FragmentActivity() {
}
private fun handleLaunchIntent(intent: Intent) {
+ if (BuildConfig.DEBUG && intent.action == "to.bitkit.QA_CONTACT_MESSAGES") {
+ lifecycleScope.launch {
+ qaSdk.qaMessageControl(intent.getStringExtra("operation") ?: "", intent.getStringExtra("peer") ?: "")
+ .onFailure { QaWaitIsolation.log("message control failed kind=${it::class.simpleName}") }
+ }
+ return
+ }
+ if (BuildConfig.DEBUG && intent.action == "to.bitkit.QA_CONTACT_IDENTITY") { // test hook
+ lifecycleScope.launch {
+ qaPubkyRepo.qaContactIdentityControl(intent.getStringExtra("operation") ?: "")
+ .onFailure { QaWaitIsolation.log("auth fixture failed kind=${it.javaClass.simpleName}") }
+ }
+ return
+ }
+
if (intent.getBooleanExtra(EXTRA_PAYKIT_SUBSCRIPTION_PAYMENT_DUE, false)) {
intent.removeExtra(EXTRA_PAYKIT_SUBSCRIPTION_PAYMENT_DUE)
appViewModel.onPaykitSubscriptionNotificationTapped(
diff --git a/app/src/main/java/to/bitkit/usecases/RefreshContactPaykitLinkUseCase.kt b/app/src/main/java/to/bitkit/usecases/RefreshContactPaykitLinkUseCase.kt
index 45c230ebe..6ea3175a8 100644
--- a/app/src/main/java/to/bitkit/usecases/RefreshContactPaykitLinkUseCase.kt
+++ b/app/src/main/java/to/bitkit/usecases/RefreshContactPaykitLinkUseCase.kt
@@ -14,6 +14,7 @@ class RefreshContactPaykitLinkUseCase @Inject constructor(
@IoDispatcher private val ioDispatcher: CoroutineDispatcher,
private val pubkyRepo: PubkyRepo,
private val privatePaykitRepo: PrivatePaykitRepo,
+ private val qaSdk: to.bitkit.services.PaykitSdkService, // test hook
) {
companion object {
private const val TAG = "RefreshContactPaykitLinkUseCase"
@@ -21,11 +22,15 @@ class RefreshContactPaykitLinkUseCase @Inject constructor(
suspend operator fun invoke(publicKey: String): Result<Unit> = withContext(ioDispatcher) {
runSuspendCatching {
+ qaSdk.qaHoldAfterPersistence() // test hook
val signIn = pubkyRepo.currentSignIn() ?: return@runSuspendCatching
+ android.util.Log.i("QaWaitIsolation", "captured identity=${signIn.publicKey} peer=$publicKey") // test hook
+ to.bitkit.utils.QaWaitIsolation.hold("schedule") // test hook
val savedPublicKeys = (pubkyRepo.contacts.value.map { it.publicKey } + publicKey).distinct()
privatePaykitRepo.refreshSavedContactEndpoints(publicKey, savedPublicKeys, signIn.publicKey) {
pubkyRepo.isCurrent(signIn)
}.getOrThrow()
+ android.util.Log.i("QaWaitIsolation", "schedule returned current=${pubkyRepo.isCurrent(signIn)} peer=$publicKey") // test hook
}.onFailure {
Logger.warn(
"Failed to refresh the Paykit link for '${PubkyPublicKeyFormat.redacted(publicKey)}'",
diff --git a/app/src/main/java/to/bitkit/utils/QaWaitIsolation.kt b/app/src/main/java/to/bitkit/utils/QaWaitIsolation.kt
new file mode 100644
index 000000000..772140091
--- /dev/null
+++ b/app/src/main/java/to/bitkit/utils/QaWaitIsolation.kt
@@ -0,0 +1,14 @@
+package to.bitkit.utils
+
+// test hook: only delays/logs, never replaces a scheduling or retirement decision.
+object QaWaitIsolation {
+ fun log(text: String) { if (to.bitkit.BuildConfig.DEBUG) android.util.Log.i("QaWaitIsolation", text) } // test hook
+ suspend fun hold(stage: String) { // test hook
+ if (!to.bitkit.BuildConfig.DEBUG) return // test hook
+ val gate = java.io.File("/data/data/${to.bitkit.BuildConfig.APPLICATION_ID}/files/qa_hold_$stage") // test hook
+ if (!gate.exists()) return // test hook
+ log("hold entered stage=$stage") // test hook
+ kotlinx.coroutines.withTimeoutOrNull(240000) { while (gate.exists()) kotlinx.coroutines.delay(100) } // test hook
+ log("hold released stage=$stage") // test hook
+ } // test hook
+}Test 1 passed at 04b0b0e. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
Re-reviewed f9fa0b5; no new findings. The contact save/background/resume/Request flow passed on an emulator at 1c8f938c. The latest missing-peer retirement fix is covered by unit tests; I haven’t rerun device testing on this head.
5 participants
Description
Give each contact its own private-link retry schedule, so a newly added or refreshed contact does not inherit another contact's backoff.
0.1.0-rc72, including prepared handshake-read reuse and distinct link-observation failures. Invalid metadata and transport errors remain distinct from SDK-owned recovery; neither triggers app-owned relinking.Existing retry intervals, queue fairness, identity/deletion guards, background lifecycle gates and admitted-write completion remain intact. Linked alone is not treated as payable. No protocol or UI changes.
Out of Scope
Design
N/A - no UI changes.
Preview
N/A
QA Notes
Journeys
link-contact-after-resume.xml: save a contact, background/resume, and reach Request or Pay without re-adding it; mirrored in merged iOS fix: polish buttons to match figma design #887. Piotr passed this functional journey on1c8f938with an iOS peer in review 5471481153, without claiming a speedup. The rolling controlled run still marks J1 failed: its latest Pay tap spun at least 14 seconds, while the unfunded fallback toast was clarified as expected and Request later completed. The prior 11-second unavailable-contact report is therefore not treated as proof of a separate error-handling bug. Exact queue/SDK/network attribution remains unestablished. Earlier 53.070 and 47.1/72.9/73.8-second values measured Save-to-SDK-private-link-restoration, not UI readiness or Noise alone. No current-head device or performance pass is claimed.Manual Tests
journeys/contacts/README.md: hold synchronization, save or re-add a contact, then release it; the saved contact, explicit retry deadline and endpoint assignment survive. Repeat with an identity change and with private withdrawal pending while sharing is re-enabled. Controlled blocking and retry-state inspection are not journey-runner capabilities. The 57a370e interim run with debug hooks observed stale-refresh and pending-withdrawal guards preserving the current contact. Switching to a different identity was not reached in that older run. Current1c8f938with debug hooks observed stale-save and queued-cleanup guards preserving deadlines and assignments, but new-identity endpoint preservation and sharing re-enable after pending withdrawal remain untested. The complete test remains unchecked.journeys/contacts/README.md: hold unrelated SDK work after contact persistence, verify Contact Saved does not wait for identity lookup, then leave the screen and verify the retry survives. Repeat with an identity change and a contact with no Paykit records, including a retained unlinked SDK peer row. Empty missing-peer retries must retire while same-peer queued delivery remains recoverable. Controlled blocking and retry inspection are not journey-runner capabilities. Piotr observed foreground-save isolation and retry/deadline survival on1c8f938. The rolling run then found a retained unlinked row kept an empty retry alive after NotFound.f9fa0b568fixes that reproduced case; its device retest and the full overlapping-preparation/queued-delivery checks remain outstanding. The historical 62-second reciprocal-link interval is still unattributed relative to its artificial hold, not ordinary-app latency.Earlier controlled Android API 36 testing retained the same pending retry across background/resume. An admitted operation finished in background; later advancement waited for resume without duplicate advancement. SDK-boundary stubs do not establish real Homeserver durability, OS suspension, process-death recovery or linking latency.
Automated Checks
PrivatePaykitRepoTest.kt: shared reads preserve independent deadlines, priority, cancellation and post-mutation checks; unavailable peers retire and can be refreshed; acknowledged outbound work still drains after the priority window; cached identity schedules without SDK I/O but is validated before linking; stale sign-ins are discarded after local state loading.RefreshContactPaykitLinkUseCaseTest.kt: forwards the current sign-in and live ownership guard, and skips signed-out scheduling.PaykitExceptionExtTest.kt: observation and transport failures remain distinct from recovery-required state.PaykitSdkServiceTest.kt: preserve observation and transport exceptions through the SDK boundary and allow subsequent retry.AppViewModelSendFlowTest.kt,ContactPaymentSettingsRepoTest.kt,PrivatePaykitRepoTest.kt,PrivatePaykitAddressReservationRepoTest.ktandPaykitSdkServiceTest.kt: cover save/re-add during publication and cleanup, identity changes, cold cache and lock admission, preservation of the original explicit retry deadline, completion of admitted writes, and re-enabling sharing after cleanup skipped preparation.At
f9fa0b568, required compilation and all 3,658 unit tests in 217 suites passed. This follow-up extends the missing-peer regression with a retained unlinked row, unrelated outbound, and sharing enabled/disabled, and adds one same-peer queued-delivery failure/retry test. The retained-row regression failed before the fix. Complete Detekt/format reports match all 19 verified baseline findings and source contexts, with no introduced diagnostics. Independent final correctness and repository-fit review found no actionable issues. Current-head controlled device checks remain open.The preceding
1c8f938cbpassed build, lint, Detekt and all seven E2E shards, with approvals from jvsena42 and Piotr. Freshf9fa0b568CI and re-review are pending; prior-head checks and approvals are not current-head validation. Neither code approval nor CI closes the manual checks above or #1419.The SDK is the published GitHub Maven rc72 AAR, without a local override; its previously verified SHA-256 is
3d218b088b4318fbd13a1370493460a228a9fe32e88fee9a69c7930b0fe5f261.Supplemental Android lint is not claimed passed: the Navigation
EmptyNavDeepLinkDetectorcrashes on an unchanged source file, independently reproduced on the original PR head. No detector was suppressed.