Skip to content

refactor: batch paykit contact cleanup - #1099

Merged
ovitrif merged 4 commits into
codex/paykit-incoming-payment-requests-rc39from
codex/batch-paykit-contact-cleanup
Aug 4, 2026
Merged

refactor: batch paykit contact cleanup#1099
ovitrif merged 4 commits into
codex/paykit-incoming-payment-requests-rc39from
codex/batch-paykit-contact-cleanup

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR builds on #1098 to batch and harden private Paykit contact cleanup:

  • Reads one shared SDK linked-peer snapshot per multi-contact publication or cleanup pass, with one batch-level retry instead of per-contact fallback reads.
  • Clears receiver-scoped payment lists, drains outbound private messages, and checks pending status once for the whole batch.
  • Processes all pending deleted contacts through the collection cleanup path instead of repeating snapshots and drains per contact.
  • Serializes publication and cleanup with the same mutex and preserves endpoint cache state that changes while remote cleanup is suspended.
  • Keeps per-contact failure isolation for attributable clear failures: successful contacts are cleared while failed contacts remain cached for retry.
  • Drops malformed persisted cleanup keys with a warning because they cannot address SDK state, allowing cleanup to converge.
  • Batches deleted-contact marker updates into one DataStore write.

Error-path behavior is intentionally conservative:

  • If linked-peer inspection fails twice, cleanup still clears locally tracked published paths, but retains all affected contacts for retry.
  • If the shared post-drain inspection fails, the whole batch remains cached and pending because delivery status cannot be attributed safely per contact.

Preview

N/A — repository-only refactor.

QA Notes

Manual Tests

N/A

Automated Checks

  • PrivatePaykitRepoTest.kt: all 43 focused tests passed, including malformed-state recovery, shared snapshot retry, batched deleted-contact retry, drain-inspection failure, partial cleanup failure, and a concurrent cache update during remote cleanup.
  • ./gradlew compileDevDebugKotlin passed as part of the focused and full test runs.
  • ./gradlew testDevDebugUnitTest passed.
  • ./gradlew detekt passed with only pre-existing findings outside this change.
  • git diff --check passed.

@ben-kaufman
ben-kaufman marked this pull request as ready for review July 22, 2026 09:32
@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR batches private Paykit contact publication and cleanup work. The main changes are:

  • Reads linked peers once per multi-contact pass.
  • Clears payment lists before one shared message drain.
  • Preserves cached state for contacts with reported cleanup failures.
  • Adds tests for batching and partial failure handling.

Confidence Score: 4/5

The batched cleanup path can skip remote cleanup or block unrelated contacts, so these cases should be fixed before merging.

  • Invalid keys can be treated as successfully cleaned and lose their retry marker.
  • One linked-peer lookup failure now prevents cleanup for the whole batch.
  • A peer change during the pass can leave remote state without matching cached cleanup state.

app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt

Important Files Changed

Filename Overview
app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Adds shared linked-peer snapshots, batched cleanup and draining, and per-contact cache reconciliation.
app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt Adds valid tests for one linked-peer read, one shared drain, and partial cleanup failure.

Reviews (1): Last reviewed commit: "refactor: batch paykit contact cleanup" | Re-trigger Greptile

Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
@jvsena42 jvsena42 added this to the 2.5.0 milestone Jul 22, 2026
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-incoming-payment-requests-rc39 branch from 6959e91 to 9bae4d5 Compare July 22, 2026 13:14
@ben-kaufman
ben-kaufman force-pushed the codex/batch-paykit-contact-cleanup branch from e56f9dc to 9cf060e Compare July 22, 2026 13:14
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-incoming-payment-requests-rc39 branch 3 times, most recently from eda7df2 to 4457134 Compare July 24, 2026 09:57
@ben-kaufman
ben-kaufman force-pushed the codex/batch-paykit-contact-cleanup branch from 9cf060e to 85d80c8 Compare July 24, 2026 09:57
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-incoming-payment-requests-rc39 branch from 4457134 to ece10d4 Compare July 27, 2026 18:44
@ben-kaufman
ben-kaufman force-pushed the codex/batch-paykit-contact-cleanup branch from 85d80c8 to 0b91ae2 Compare July 27, 2026 18:44

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the batching refactor against the parallel iOS findings on synonymdev/bitkit-ios#638.

The structure is a clear improvement over the base, and the per-contact failure isolation contract does hold — I traced clear-failure, drain-still-pending, and link-inspection-failure, and each ends with the contact in failedPublicKeys, cache retained, retry marker intact. Test coverage is also meaningfully ahead of the iOS PR, which ships none. ./gradlew testDevDebugUnitTest --tests to.bitkit.repositories.PrivatePaykitRepoTest passes locally.

iOS parity summary

iOS #638 finding Android status
greptile P1 — shared lookup failure drops cleanup + retries ✅ already fixed in 0b91ae2 (ahead of the iOS fix)
N+1 linkedPeers() on snapshot failure ⚠️ present
duplicated snapshot-or-fallback block ⚠️ partly — helper exists but isn't reused in cleanup
clears now attempted after link-inspection failure ⚠️ present, undocumented
drain-check failure blast radius = whole batch ⚠️ present
widened reentrancy window on the deferred state wipe ⚠️ present in analogous form
eager vs deferred failure markers n/a — Android markers are caller-driven

Requesting changes on one item: the invalid-key handling added in 0b91ae2 turns a previously self-healing no-op into a permanently non-converging failure that blocks profile-delete, disable-sharing, and wipe cleanup for all contacts. Details inline. Everything else is either an iOS-parity item worth resolving consistently across platforms, or a cleanup.

Description nits: "no user-facing behavior changes are intended" isn't quite accurate — the error path now issues clearPrivatePaymentList calls it previously skipped, and transient-failure granularity is coarser. Both worth a bullet.

Changelog: correctly omitted for a refactor: with no user-facing change.

Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
@ben-kaufman
ben-kaufman force-pushed the codex/batch-paykit-contact-cleanup branch from 0b91ae2 to b668941 Compare August 3, 2026 07:23
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-incoming-payment-requests-rc39 branch from cf14578 to 44d58d2 Compare August 3, 2026 07:36
@ben-kaufman
ben-kaufman force-pushed the codex/batch-paykit-contact-cleanup branch from b668941 to cd4cd1e Compare August 3, 2026 07:36

Copy link
Copy Markdown
Contributor Author

Restacked onto the current #1098 head after the base rewrite. The cleanup review-fix commit previously referenced as b6689414e is now signed commit cd4cd1ec7; the cleanup diff is unchanged.

The current build failure is the same inherited #1084 WatchOnlyAccountStore.serializedExtendedPubkey compile failure already present on #1084 and #1098.

@ben-kaufman
ben-kaufman force-pushed the codex/paykit-incoming-payment-requests-rc39 branch from 44d58d2 to 796bc2b Compare August 3, 2026 11:13
@ben-kaufman
ben-kaufman force-pushed the codex/batch-paykit-contact-cleanup branch from cd4cd1e to 39a1ae2 Compare August 3, 2026 11:13
@ben-kaufman
ben-kaufman force-pushed the codex/batch-paykit-contact-cleanup branch from 39a1ae2 to 670e9e7 Compare August 4, 2026 06:48
@ben-kaufman
ben-kaufman requested review from jvsena42 and ovitrif August 4, 2026 06:50

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The batched cleanup keeps per-contact clear isolation, drops malformed pending keys without failing the pass, and preserves cache state that changes while remote cleanup is suspended.

@ovitrif
ovitrif merged commit 830d135 into codex/paykit-incoming-payment-requests-rc39 Aug 4, 2026
18 checks passed
@ovitrif
ovitrif deleted the codex/batch-paykit-contact-cleanup branch August 4, 2026 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants