fix: preserve contact payment setup errors

[ben-kaufman]

This PR reports private-payment access errors during contact payment setup instead of silently completing with private sharing disabled.

Description

• Propagates failed access checks to the existing setup error handling before changing preferences or publishing endpoints.
• Preserves confirmed public-only access, cancellation, and successful retry without changing background publication checks.

Out of Scope

• Existing saved sharing preferences are not changed automatically; public-only sharing remains a supported choice.

Design

N/A - no UI changes. Existing error and retry handling is reused.

Preview

N/A - existing setup screen and error toast are unchanged.

QA Notes

Journeys

N/A - not drivable; see Manual Tests.

Manual Tests

• 1 Pay Contacts -> make the private-access check fail once -> Continue stays on setup with the existing error, without changing sharing preferences or publishing endpoints; restore access and retry -> private sharing is enabled. Controlled SDK access-check failure injection is not in Capabilities.

Automated Checks

• updated ContactPaymentSettingsRepoTest.kt - access failure leaves settings and publication untouched, retry succeeds, and cancellation releases the sharing lock. Existing public-only coverage is retained.
• updated PrivatePaykitRepoTest.kt - access-check errors and cancellation propagate; contact-preparation identity-error coverage is preserved separately.

Local validation: 3,590 unit tests passed, including existing setup error/navigation tests. Compilation passed; all 19 Detekt findings, including formatting, match the unchanged base. Controlled device fault injection was not run.

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Changes error handling in contact payment setup flow.

This PR appears safe to merge; no actionable issues were found.

What we checked:

• Cancellation does not block retry: runSuspendCatching rethrows cancellation, and setEnabled holds the lock through withLock. The added test checks that another enable attempt succeeds after cancellation.
• Public-only sharing still works: The SDK returns false when the identity lacks private access. enable still uses that result to disable only private sharing.

Summary

Contact payment setup now reports access-check errors instead of treating them as a choice to disable private sharing.

• Failed checks leave saved settings and published endpoints untouched.
• Confirmed public-only access remains supported.
• Added tests cover retry and cancellation. Background publication checks remain unchanged.

Review was based on code and test inspection; no tests were run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Enable contact payments] --> B{Check private access}
  B -->|Error| C[Return error without changes]
  B -->|Cancelled| D[Release lock and stop]
  B -->|Confirmed false| E[Enable public sharing only]
  B -->|Confirmed true| F[Enable public and private sharing]
  C --> G[Stay on setup and allow retry]

Loading

_{Reviews (1) · Last reviewed commit: "fix: preserve contact payment setup erro..." · Reviewed by Greptile}

[github-actions]

Regtest APK

Built from b059b9d (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

[ovi-reviewer]

Suggestion: 👍 Approve

Review: diff 5 files.
iOS already reports thrown setup failures and stays on Pay Contacts. Its setup guard, retained in synonymdev/bitkit-ios#856, checks local or adopted credentials synchronously. This Android fix handles a failing SDK access lookup that iOS does not call, so there is no matching lookup to port.

QA:
Tests queued.

---

Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

[jvsena42]

No findings at 5670fae.

Checked:

• hasPrivatePaymentAccess() (PrivatePaykitRepo.kt:156) now throws, and ContactPaymentSettingsRepo.enable() (:79-81) returns the failure before any settings write. A failed access check persists nothing, publishes nothing and releases the sharing lock; later failure points keep their existing rollback.
• An identity without private capability still returns false, so the public-only path is unchanged. The two background checks (:986, :1681) keep the non-throwing wrapper.
• Error text: both callers map to fixed string resources (PayContactsViewModel.kt:61-69, SettingsViewModel.kt:258-264); no SDK or homeserver message, key or path reaches the toast. One log, at the ViewModel.
• Cancellation: runSuspendCatching rethrows, the lock releases, isLoading resets in finally, nothing is logged as an error.
• iOS has no SDK capability lookup to swallow (ContactPaymentsService.swift:85-88 decides synchronously), so there is nothing to port.

Device gate: no device check needed (failure path only, not drivable; covered by ContactPaymentSettingsRepoTest.kt and PrivatePaykitRepoTest.kt). CI is green at this head, including e2e.

[piotr-iohk]

utACK

[talosmachina]

No findings. Lets an error from the private-payment access check fail ContactPaymentSettingsRepo.enable() before any setting is written, instead of being read as "no access" and silently enabling public-only sharing. Reviewed 5670fae, full tier.

What I checked, and 3 candidates I ruled out

Read in full: ContactPaymentSettingsRepo.kt, the hasPrivatePaymentAccess paths in PrivatePaykitRepo.kt and PaykitSdkService.kt
Call sites traced: setEnabled from SettingsViewModel and PayContactsViewModel; the internal hasPrivatePaymentAccessForCurrentProfile() callers in PrivatePaykitRepo
CI: build, detekt, unit and e2e jobs green; only the human-review check pending

Ruled out

• Internal callers now see exceptions: hasPrivatePaymentAccessForCurrentProfile() keeps its own runSuspendCatching { ... }.getOrDefault(false), so the background sharing paths behave as before.
• Cancellation swallowed as a failure: runSuspendCatching rethrows CancellationError, and the access check runs before settingsStore.update, so a cancelled toggle writes nothing and the withLock releases sharingMutex; covered by the new cancellation test.
• Partial state on failure: the early return Result.failure(it) precedes every write, so no rollback is needed for this path.

Merge confidence: 5/5, CI green and the change is covered by the two new ContactPaymentSettingsRepoTest cases.

[ovi-reviewer]

Verdict: ⛔️ Request Changes

Tests for the review: Test 1 fails.

QA:
Tested on Android 15 emulator.

🔴 Test 1

Test 1

Continue showed no progress feedback for 6+ seconds; Profile appeared by 8.05 seconds.

1.mp4

Tip

Could we show loading feedback while Continue is pending?

Note

The controlled access-check failure and restore/retry were not exercised; they require a selective SDK failure hook.

---

Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

addressed