feat(credits)!: SDK-only Buy Credits — background recovery + spend-everything MAX

[HashEngineering]

What

Buy Credits becomes SDK-only end to end (MO-998 / #1520 Phase 1B): the dashj purchase path is deleted, the purchase runs as unique background work with crash/interruption recovery, and MAX ("spend everything") works — using the same design the shielded Internal Transfer screen already ships.

Based on feat/kotlin-sdk-phase1 — independent of #1535 (feat/maya-sdk-route); the two can merge in either order.

Purchase pipeline

• The purchase runs as UNIQUE WorkManager work (PerformTopUpWorker): a lock screen, rotation, or process death cannot cancel it mid-flight, and a double tap attaches to the running work instead of buying twice. The screen only observes.
• Interrupted attempts are completed in the background by ResumeTopUpsWorker from the SDK's tracked asset lock — the resume gate re-consumes the same lock, so no double pay.
• A Platform "output already completely used" rejection is treated as terminal, not retryable: those credits provably landed on an earlier attempt. Without this, one stale tracked lock blocked every future purchase (observed live). The SDK-side root fix is a separate platform branch.
• Confirmed top-ups get a credited state in transaction details.

MAX — the Internal Transfer pattern

MAX fills the FULL spendable balance; the worker submits it as-is and, when the asset-lock coin selection comes up short pre-broadcast (nothing spent, selection released), retries ONCE with an estimated fee reserve withheld. Over-reserve is lossless — the builder returns the excess as change. Differences from the shielded original, each forced by a live testnet failure:

• The retry matches classifyBroadcastFailure's verdict (two named, provably-pre-broadcast reasons), not an engine message — the top-up build's shortfall text differs from the shielded path's, and matching the wrong one made the first live MAX fail without retrying.
• The fee reserve is sized from the SDK's own eligible-UTXO count (a COUNT twin of the preflight's eligibility SQL, shared predicate), not dashj's spendableUtxoCount().
• The adjusted retry is refused below Platform's 50,500-duff top-up floor rather than stranding a lock Platform deterministically rejects.
• The funding preflight is MAX-aware: the withheld reserve IS the fee allowance, so the preflighted figure absorbs the fixed headroom instead of stacking on it.

Verified end to end on testnet: full-balance attempt refused pre-broadcast → auto-adjusted (reserve from a 2-UTXO count) → broadcast → chain-locked next block → credited → lock consumed. Also confirmed en route: an InstantSend-locked deposit is spendable by MAX immediately while unconfirmed.

UI fixes found during testing

• The credits explainer re-appeared on every visit (its "seen" flag was written from the sheet's own lifecycle scope and cancelled by dismissal) — persisted from the ViewModel scope with NonCancellable.
• The Send button now holds its progress state until the work reaches a terminal state, so every failure shows its dialog on this screen. Closing at the SDK hand-off was tried and reverted — it silenced all post-hand-off failure dialogs; the code comment records why.
• The silent minimum explains itself; the confirmation screen's unused dashj dry-run figures (and their null-fee crash site) are deleted; BuyCreditsFragment no longer imports dashj at all.

Not in this PR

• The SDK pin: local testing used 0.1.0-v41int18-maya8-SNAPSHOT (uncommitted pin); the branch builds against the phase1 base's committed pin as well.
• Display-balance corrections for asset-lock spends (two overlaid-balance queries) are held back pending the platform-side fix (fix/asset-lock-spend-visibility), which makes them redundant.
• The SDK root causes found during this work are addressed platform-side:
  • fix(platform-wallet): settle already-consumed asset locks and survive an ambiguous resume broadcast platform#4337 — consumed-lock settlement + resume idempotency. Once pinned, this PR's error-string workaround for the "already completely used" rejection can be deleted (it is the typed root fix).
  • fix(platform-wallet): make asset-lock spends visible to every balance reader platform#4336 — asset-lock spend visibility for balance readers (isSpent flip, per-account aggregates, UTXO-restore heal). Once pinned, the held-back display-balance corrections become optional hardening.
  • Two further branches remain local: chain-scan rediscovery of top-up locks after phrase restore, and drain (true send-all) for top-ups.

Review response (round 1)

All eight review points are addressed on the branch — fixes in 5c000e201 (scoped stale-work gate replacing the global prune; inclusive 50,500-duff floor end to end; single observer registration), documentation in 5f0ad15c6 (restore caveat on the credited-state readers; lifecycle idiom; imports), and hardening in 3c51d0eb8 (typed shortfall arms). Point 6's dependency (rediscovery before pre-cutover users lose the legacy retry path) is recorded in MO-998. Full per-item detail is in the review-reply comment below.

v41int21 verification (review point 2) — done, from source

The int21 AAR binary isn't in any cache here, but its source line is: bfoss765/pool/qa5-4350. The wallet's exact top-up call path was traced hop by hop on that branch:

SdkTransparentTopUp.topUp → IdentityCredits.topUpFromCore → JNI topUpIdentityFromCore → platform_wallet_top_up_identity_with_funding_signer → IdentityWallet::top_up_identity_with_funding → resolve_funding_with_is_timeout_fallback(FromWalletBalance) → create_funded_asset_lock_proof → broadcast_funded_asset_lock → build_asset_lock_transaction_with_funding, whose coin-selection error is wrapped by map_builder_error → PlatformWalletError::AssetLockInsufficientFunds (every shortfall shape, including zero-candidate NoUtxosAvailable) → FFI code 29 (ErrorAssetLockInsufficientFunds; the duff amounts travel in the message "asset lock coin selection is short: …") → Kotlin 29 -> DashSdkError.PlatformWallet.AssetLockInsufficientFunds(message, cause) — the same class, nesting, and constructor this branch's new typed arm matches.

Confirmed against the built artifact too: with the local pin on 0.1.0-v41int21-maya10-SNAPSHOT, the wallet compiles and the platform-service test sweep is green (912 tests, 0 failures), including the typed-shortfall classifier test constructing int21's actual error classes.

Conclusion: on the int21 line the MAX retry survives twice over — the typed arm added in 3c51d0eb8 catches FFI 29 regardless of message text, and the pre-existing message arm still matches the wording. CoreInsufficientFunds (22) does not occur on the top-up path (it's the atomic Core-send selection); its typed arm stays as cross-path armor.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7215f56b-ffd4-4a64-9847-b8ef8682fb74

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[bfoss765]

This is arguably the best-protected surface of the three PRs — double-pay is defended in depth (unique work + KEEP, the resume gate re-consumes the same tracked lock after process death, Ambiguous is never re-run as a purchase, the already-consumed skip only proceeds when Platform has proven the old lock spent), and getting the wallet password out of WorkManager's database is a real security improvement. Comments below; the two blocking-flavored ones are coordination/verification rather than defects in the diff.

1. Coordination — SdkAssetLockFundingPreflight.kt: there is an in-flight refactor on the branch tip that rewrites ELIGIBLE_ASSET_LOCK_DUFFS_SQL into a shared join spine with a parameterized finality term; a merge simulation confirms this file conflicts. When rebasing, please rebuild the COUNT twin over the same spine — if the count query keeps the old finality term while the sum gains the extension, the two populations diverge and the MAX fee reserve gets sized from a different UTXO set than the one the build selects.

2. Verify on the current AAR line — PerformTopUpWorker MAX retry trigger: the retry keys off classifyBroadcastFailure matching two message shapes. On the v41int19+ AAR line the engine raises typed errors for funding shortfalls (CoreInsufficientFunds, FFI 22) — that exact migration silently disabled the send-all adjust-down retry once before. Please verify the top-up asset-lock build's shortfall still carries a matched message on int21, and consider adding the typed arm(s) to classifyBroadcastFailure defensively — otherwise the first live MAX on the new AAR fails without retrying.

3. PerformTopUpOperation.prune(): WorkManager.pruneWork() prunes every finished work item in the app, not just the top-up chain — tag-observed finished states used by other features (e.g. the username-vote broadcast status readers) can vanish out from under their observers. Consider scoping this rather than a global prune.

4. BuyCreditsFragment.MIN_TOP_UP = 50_000 vs PLATFORM_TOP_UP_FLOOR_DUFFS = 50_500: the screen accepts 50,001–50,500 duffs, which the floor then rejects — a failure dialog for an amount the UI said was valid. Suggest raising the screen minimum to 50,500 so the "Enter more than %s" message names the real floor. Also, adjusted > PLATFORM_TOP_UP_FLOOR_DUFFS refuses an adjusted retry of exactly 50,500; if the FFI floor is inclusive this should be >=.

5. BuyCreditsFragment.observeTopUpWork(): called from handleSdkTopUp on every purchase, and each call registers a fresh observer on the same viewLifecycleOwner — a second purchase in one screen session leaves two observers and a later FAILED state shows two failure dialogs. Register once (e.g. onViewCreated) or unregister before re-observing. Related: re-entering the screen mid-purchase shows no spinner until Continue is tapped again — observing from onViewCreated would surface in-flight work immediately.

6. TopUpRepositoryImpl.checkTopUps: dropping the legacy dashj retry loops is right post-cutover, but it also removes the retry for pre-cutover users holding an unused dashj-era top-up, and SDK chain-rediscovery of those locks is still unlanded — until it lands, those top-ups sit stranded-but-safe with no in-app path. Please make sure the tracking ticket names that dependency explicitly.

7. SdkTopUpRecoveryService.isTopUpPending / sdkTopUpCredited: "no pending lock" is read as "credited", but after a phrase restore the tracked-lock table is empty, so an actually-unclaimed SDK top-up displays as credited. Display-only, and restore-rediscovery will fix the root — a code comment noting the restore caveat would prevent this being trusted for anything stronger than a label.

8. Nits: EnterAmountFragment.setContinueLoading uses deprecated launchWhenStarted (repeatOnLifecycle is the current idiom); TransactionResultViewBinder fully-qualifies de.schildbach.wallet.service.platform.sdk.AssetLockKind inline three times — import it.

[HashEngineering]

Thanks for the review — all eight points are addressed. Per item:

1. Preflight SQL / shared join spine — already converged on the current tip, by construction. When this branch rebased onto feat/kotlin-sdk-phase1 it picked up the spine refactor (17c450467, IS/CL-context finality + core_addresses join), and this PR's own diff extracts that predicate into a single shared constant (ELIGIBLE_ASSET_LOCK_PREDICATE_SQL) from which BOTH the SUM and the COUNT twin are built, with the same single bind arg. The two populations cannot diverge — there is only one predicate to edit. If your merge simulation ran against the pre-rebase (maya-based) snapshot, this should now come up clean; if it still conflicts against something newer than the phase1 tip, point me at the branch.

2. MAX retry on the typed-error AAR line — verified from source and hardened (3c51d0eb8). The int21 AAR itself isn't available on this machine to inspect (nothing newer than the int18 line in any local cache, and the artifacts are locally-published rather than on a public repo), so I verified against the newest line you've published: pool/qa5-4350. There, a top-up asset-lock build shortfall surfaces as the typed AssetLockInsufficientFunds (FFI 29) — not CoreInsufficientFunds (22) — via map_builder_error, which promotes every builder shortfall shape including the zero-candidate NoUtxosAvailable; its message is still "asset lock coin selection is short: …", which the classifier already matched, so the retry was surviving on the message arm. Defensively, classifyBroadcastFailure now has typed arms for BOTH 22 and 29, mapped to the same named retryable reasons the retry keys off, with a test that includes deliberately unrecognizable message text so classification holds on type alone. If int21 differs from pool/qa5-4350 on this path, a pointer to the actual branch/AAR would let me close the gap — but the typed arms should hold regardless of message wording.

3. Global pruneWork() — replaced entirely (5c000e201). WorkManager offers no scoped prune, so instead of pruning, the fragment gates on having seen the purchase active in the current view: a terminal state delivered before any active state is a leftover from a previous visit and is ignored. Other features' finished work records are untouched.

4. Screen minimum vs floor — fixed (5c000e201). Confirmed in the FFI source that the floor is inclusive (amount_duffs < MIN_TOP_UP_DUFFS rejects), so: the screen minimum is now PLATFORM_TOP_UP_FLOOR_DUFFS itself (inclusive, via setMinAmount(isIncludedMin = true)), the adjusted-retry check is >=, and the error string names the real floor ("Enter at least %s").

5. Stacked observers — fixed (5c000e201). observeTopUpWork() registers once in onViewCreated; no re-observe per purchase. Your related note is covered too: re-entering mid-purchase now shows the spinner immediately.

6. Stranded pre-cutover top-ups — the dependency is now named explicitly in MO-998: rediscovery must ship in a pinned SDK before (or soon after) this PR reaches pre-cutover users; until then dashj-era top-ups are stranded-but-safe with no in-app path.

7. Restore reads unclaimed as credited — documented at both ends (5f0ad15c6): isTopUpPending carries a RESTORE CAVEAT block ("display-only — never gate a spend or a retry on it") and sdkTopUpCredited states that after a phrase restore "no pending lock" also matches an unclaimed top-up, until rediscovery lands.

8. Nits — done (5f0ad15c6): setContinueLoading uses withStarted on the view lifecycle (only the PR-added call; the file's pre-existing launchWhenStarted uses are out of scope), and AssetLockKind is imported.

🤖 Generated with Claude Code

[bfoss765]

The pre-cutover dependency does not apply

Flagging this because it was raised as a shipping dependency on this PR (point 6 of the earlier review): that #1536 removes the only in-app retry path for pre-cutover users holding an unused dashj-era top-up, so asset-lock rediscovery must ship in a pinned SDK before or soon after this reaches them.

There is no pre-cutover user. The rollout decision is that the SDK owns the wallet for every user-facing feature immediately — new wallets, upgraded wallets and restored wallets alike. Every install is effectively CUT_OVER from first launch. The graded CutoverState values are development scaffolding, not a description of a field population.

We have documented this directly in CutoverStateMachine.kt on feat/kotlin-sdk-phase1, with a TODO to delete the state machine once no code path has dashj owning L1, because this distinction has repeatedly led both people and AI agents to invent dependencies on a cohort that does not exist.

One nuance that survives, and is worth stating precisely: a user who topped up in 11.9 and then upgraded does still hold a legacy asset lock, and they are SDK-owned immediately. But their dashj retry loop cannot run in that state either — dashj is held. So #1536 is not what strands that lock; the cutover already did. Deleting the loops removes dead code, not a working recovery path.

Rediscovery remains worth having for exactly that population, but it is decoupled follow-up, not a gate on this PR.

Review notes

Read the purchase and recovery paths in full. The fund-safety design holds up:

• PerformTopUpWorker runs as unique work, so a lock screen, rotation or process death cannot cancel mid-flight, and a double tap attaches rather than buying twice.
• The MAX convergence retry fires only on NotBroadcast with REASON_PRE_BROADCAST_BUILD_SHORTFALL / REASON_PRE_BROADCAST_ASSET_LOCK_SELECTION — provably pre-broadcast, nothing on the wire — and is one-shot, with the PLATFORM_TOP_UP_FLOOR_DUFFS guard preventing a retry that Core accepts and Platform deterministically rejects. Sizing the reserve from the SDK's own eligible-UTXO query rather than dashj's spendableUtxoCount() is the right call post-cutover.
• Ambiguous never re-runs the purchase; it hands off to ResumeTopUpsWorker, which resumes only the tracked lock from its persisted outpoint and never builds new funding. That is the property that makes reruns safe.
• drainPendingTopUps isolates per-lock failures, treats already-consumed as terminal rather than retrying forever, and returns "failed" (retryable) rather than dropping when locks exist but no identity does.

Two minor observations, neither blocking:

1. retryableShortfall already requires first is NotBroadcast, so the if re-tests it — harmless, just redundant.
2. isAlreadyConsumed message-matches "already completely used" down the cause chain. Documented as temporary pending the SDK reconciling the local row, with the bounded 8-hop walk — fine as written, but it will silently stop working if that wording changes upstream. Worth a follow-up once the typed error covers the Platform-side case.

The branch is currently CONFLICTING against its base and needs a rebase before it can merge.

[bfoss765]

Reviewed at head 7d0b39243 (post-rebase; review round 2 counting the earlier cutover-dependency comment).

Verified against the real integration line, since the rebase landed today:

• Merges clean onto the current feat/kotlin-sdk-phase1 head (e13129029, pushed today — includes the metadata/backfill and Buy Credits identity-balance work): zero conflicts.
• Compiles on the merged tree (:wallet:compile_testNet3DebugKotlin).
• The PR's own tests pass on the merged tree: SdkTopUpRecoveryServiceTest 13/13.

The earlier note stands and is now moot in the right direction: the rollout has no pre-cutover population, so the SDK-only Buy Credits path is the shipping path from first launch, and the background recovery + spend-evidence work here is exactly what that requires.

Approving. Merge whenever you're ready — it's your PR.