diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/AppNavHost.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/AppNavHost.kt index d37ae44dd65..2c7c5005cf6 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/AppNavHost.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/AppNavHost.kt @@ -284,7 +284,12 @@ fun AppNavHost( } composable { entry -> - ShieldedFundScreen(entry.toRoute().walletIdHex, navController) + val route = entry.toRoute() + ShieldedFundScreen( + walletIdHex = route.walletIdHex, + navController = navController, + resumeOutPointHex = route.resumeOutPointHex.takeIf { it.isNotEmpty() }, + ) } composable { entry -> diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/Routes.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/Routes.kt index 1d0098c1ea9..0d025c16782 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/Routes.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/navigation/Routes.kt @@ -189,8 +189,21 @@ import kotlinx.serialization.Serializable /** Seed the shielded note pool (← `SeedShieldedPoolView.swift`). */ @Serializable data class SeedShieldedPool(val walletIdHex: String) -/** Shield funds from an asset lock (← `ShieldedFundFromAssetLockView.swift`). */ -@Serializable data class ShieldedFund(val walletIdHex: String) +/** + * Shield funds from an asset lock (← `ShieldedFundFromAssetLockView.swift`). + * + * [resumeOutPointHex] carries the `:` of an already- + * tracked orphan SHIELDED top-up lock (`fundingTypeRaw == 5`) when the + * screen is opened in RESUME mode — the shielded twin of + * [FundFromAssetLock.resumeOutPointHex], and the counterpart of the Swift + * view's `resumeFromLock` parameter. Empty means fresh-shield mode; nav args + * don't support a nullable `String`, so `""` stands in for "no lock to + * resume". + */ +@Serializable data class ShieldedFund( + val walletIdHex: String, + val resumeOutPointHex: String = "", +) /** * Live shielded-funding progress (← `ShieldedFundFromAssetLockProgressView.swift`). diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockController.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockController.kt index a71724c2a99..1139195c149 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockController.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockController.kt @@ -62,6 +62,18 @@ class ShieldedFundFromAssetLockController( var lastSubmittedAt: Long? = null private set + /** + * Identity of the operation currently occupying this slot — the + * fresh-shield marker or the resumed lock's outpoint. Set by every + * accepted [submit]. The coordinator compares it to tell a re-tap of + * the SAME operation (reuse the controller) from a DIFFERENT + * operation on the same `(walletId, recipient)` slot, whose body + * [submit] would otherwise silently drop (two resumable locks + * normally share the wallet's default shielded recipient). + */ + var operationId: String? = null + private set + private var task: Job? = null /** Composite id for stable list diffing — wallet hex + recipient hex. */ @@ -71,15 +83,17 @@ class ShieldedFundFromAssetLockController( /** * Submit the funding. Defensively rejects [Phase.InFlight] and * [Phase.Completed]; [Phase.Idle] / [Phase.Failed] are allowed - * restarts. [body] performs the FFI shield call (returns nothing) or - * throws; it runs on its own dispatcher, the terminal flip hops back to - * [scope]. ← Swift `submit`. + * restarts. [operationId] names the operation the body performs (see + * [operationId]); [body] performs the FFI shield call (returns + * nothing) or throws; it runs on its own dispatcher, the terminal + * flip hops back to [scope]. ← Swift `submit`. */ - fun submit(body: suspend () -> Unit) { + fun submit(operationId: String, body: suspend () -> Unit) { when (_phase.value) { is Phase.Idle, is Phase.Failed -> Unit is Phase.InFlight, is Phase.Completed -> return } + this.operationId = operationId _phase.value = Phase.InFlight lastSubmittedAt = now() task = scope.launch { diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockCoordinator.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockCoordinator.kt index 75755546e13..edf4d52de6b 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockCoordinator.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockCoordinator.kt @@ -22,6 +22,17 @@ import kotlinx.coroutines.launch * [StartFundingResult.BlockedByOtherWalletFunding] (mirroring the Rust-side * `shield_guard` mutex — two concurrent Orchard builds on one wallet would * race the note-commitment tree). + * + * Reuse of a controller additionally requires a matching **operation + * identity** (`operationId` — the resumed lock's outpoint, or a unique + * per-submission fresh-shield id): resumable locks normally default to + * the same wallet-owned shielded recipient, so two different locks share + * one slot key, and reusing the first lock's controller would silently + * drop the second lock's resume body. A different operation is blocked + * while the slot is in flight and replaces the retained controller once + * it has completed. Fresh shields mint a NEW id per submission for the + * same reason: a fixed marker would match the retained completed + * controller and suppress the next fresh shield's body. */ class ShieldedFundFromAssetLockCoordinator( private val scope: CoroutineScope, @@ -69,16 +80,31 @@ class ShieldedFundFromAssetLockCoordinator( _controllers.value.values.sortedByDescending { it.lastSubmittedAt ?: Long.MIN_VALUE } /** - * Start a funding for the slot. Reuses the existing controller if the - * SAME recipient is already in flight / just completed. Rejects a - * DIFFERENT recipient while another is in flight on the same wallet - * ([StartFundingResult.BlockedByOtherWalletFunding]). [Phase.Idle] / + * Start a funding for the slot. Reuses the existing controller only + * when the SAME operation ([operationId]) is already in flight / just + * completed on the slot — a re-tap. A DIFFERENT operation on an + * in-flight slot is reported as + * [StartFundingResult.BlockedByOtherWalletFunding] rather than + * silently reusing the controller (which would drop its body: two + * resumable locks normally share the wallet's default shielded + * recipient, so the slot key alone cannot tell them apart). A + * different operation on a *completed* slot is a fresh start — the + * retained controller is replaced. Rejects a DIFFERENT recipient + * while another is in flight on the same wallet. [Phase.Idle] / * [Phase.Failed] on the same slot are legitimate restarts. * ← Swift `startFunding`. + * + * [operationId] is the identity of the requested operation: the + * resumed lock's outpoint for a resume (stable, so a re-tap of the + * same lock rebinds), a unique per-submission id for a fresh shield + * (so a second fresh shield never rebinds to the previous one's + * retained result). Wallet-wide serialization is unchanged — at most + * one shield-class operation runs per wallet either way. */ fun startFunding( walletId: ByteArray, recipientRaw43: ByteArray, + operationId: String, body: suspend () -> Unit, ): StartFundingResult { val key = key(walletId, recipientRaw43) @@ -94,14 +120,30 @@ class ShieldedFundFromAssetLockCoordinator( } if (existing != null) { - when (existing.phase.value) { + when (val phase = existing.phase.value) { is ShieldedFundFromAssetLockController.Phase.InFlight, is ShieldedFundFromAssetLockController.Phase.Completed, - -> return StartFundingResult.Started(existing) + -> { + if (existing.operationId == operationId) { + // Re-tap of the same operation: bind to its state. + return StartFundingResult.Started(existing) + } + if (phase is ShieldedFundFromAssetLockController.Phase.InFlight) { + // A different operation while one is running on this + // wallet: same serialization verdict as a different + // recipient — the caller gets the blocker, not a + // controller that will never run its body. + return StartFundingResult.BlockedByOtherWalletFunding(existing) + } + // Completed slot + different operation: a fresh start, + // not a re-tap. Fall through and REPLACE the retained + // controller (its sweep is identity-guarded, so the old + // retention timer can't evict the replacement). + } is ShieldedFundFromAssetLockController.Phase.Idle, is ShieldedFundFromAssetLockController.Phase.Failed, -> { - existing.submit(body) + existing.submit(operationId, body) scheduleRetentionSweep(key, existing) return StartFundingResult.Started(existing) } @@ -109,7 +151,7 @@ class ShieldedFundFromAssetLockCoordinator( } val controller = ShieldedFundFromAssetLockController(walletId, recipientRaw43, scope, now) _controllers.update { it + (key to controller) } - controller.submit(body) + controller.submit(operationId, body) scheduleRetentionSweep(key, controller) return StartFundingResult.Started(controller) } @@ -131,6 +173,12 @@ class ShieldedFundFromAssetLockCoordinator( scope.launch { var completedAt: Long? = null while (true) { + // The slot may have been handed to a REPLACEMENT controller + // (a different operation started during this controller's + // completed-retention window). This sweep then owns nothing: + // exit without touching the slot, leaving the replacement's + // own sweep in charge. + if (_controllers.value[key] !== controller) return@launch when (controller.phase.value) { is ShieldedFundFromAssetLockController.Phase.Completed -> { val nowMs = now() diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/AddressFundProgressScreen.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/AddressFundProgressScreen.kt index eac4e92aee2..38517aaf1c8 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/AddressFundProgressScreen.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/AddressFundProgressScreen.kt @@ -35,7 +35,6 @@ import androidx.navigation.NavHostController import org.dashfoundation.dashsdk.persistence.entities.AssetLockEntity import org.dashfoundation.example.di.LocalAppContainer import org.dashfoundation.example.navigation.AddressFundProgress -import org.dashfoundation.example.navigation.FundFromAssetLock import org.dashfoundation.example.services.assetlock.AddressFundFromAssetLockController import org.dashfoundation.example.services.assetlock.AddressFundFromAssetLockController.Phase import org.dashfoundation.example.util.hexToBytes @@ -83,9 +82,16 @@ fun PendingAssetLockRow( * IdentitiesHome list embeds. Port of * `PendingPlatformFundFromAssetLocksList.swift`: merges two row sources — * (1) [controllers], the live in-flight fundings from the coordinator, and - * (2) [resumableLocks], DB-backed orphan `AssetLockEntity` rows - * (`fundingTypeRaw == 4`, `statusRaw ∈ [1, 3]`) recovered after a crash - * between asset-lock broadcast and ST submission. + * (2) [resumableLocks], DB-backed orphan `AssetLockEntity` rows recovered + * after a crash between asset-lock broadcast and ST submission. + * + * The orphan half covers BOTH top-up funding types — `4` + * (AssetLockAddressTopUp) and `5` (AssetLockShieldedAddressTopUp) — because + * neither has any other recovery home: the identity screens admit only + * funding types `0..2`. Each row's Resume is routed by funding type through + * [resumeRouteFor]; a shielded lock has to reach the shielded resume FFI, + * not the platform-address one, so surfacing the row without routing it + * would only move the dead end one tap later. * * Anti-double-consume gate: if ANY controller is [Phase.InFlight] * ([hasActiveFunding]), Resume is suppressed on every orphan row — the @@ -110,17 +116,15 @@ fun PendingAssetLocksList( ) controllers.forEach { PendingAssetLockRow(it, navController) } resumableLocks.forEach { (walletIdHex, lock) -> + // Fail closed: a funding type with no resume flow on this + // surface keeps the non-interactive indicator rather than + // dispatching to a screen that would submit the wrong + // transition for it. + val route = resumeRouteFor(walletIdHex, lock) ResumablePlatformFundFromAssetLockRow( lock = lock, - hasActiveFunding = hasActiveFundingFor(walletIdHex), - onResume = { - navController.navigate( - FundFromAssetLock( - walletIdHex = walletIdHex, - resumeOutPointHex = lock.outPointHex, - ), - ) - }, + hasActiveFunding = hasActiveFundingFor(walletIdHex) || route == null, + onResume = { route?.let { navController.navigate(it) } }, ) } } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/AssetLockDisplay.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/AssetLockDisplay.kt index 66ab6ed64c8..5c9cd464caf 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/AssetLockDisplay.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/AssetLockDisplay.kt @@ -4,34 +4,58 @@ import org.dashfoundation.dashsdk.persistence.entities.AssetLockEntity /** * Asset-lock display helpers — port of `PersistentAssetLockDisplay.swift`. - * Consolidates the 0/1/2/3/4 `AssetLockStatus` discriminants (a protocol- + * Consolidates the 0..5 `AssetLockStatus` discriminants (a protocol- * mirrored constant from the Rust side) into one place so the funding UIs * don't re-implement the same `when` blocks. Extensions on * [AssetLockEntity] so every list surface reads the status the same way. + * + * The status domain is NOT an ordered severity scale, and no predicate here + * may treat it as one. `4` (Consumed) is the terminal tombstone; `5` + * (RecoveredFromChain) sits above it numerically but is very much alive. + * Every predicate below therefore excludes `4` by name rather than by an + * upper bound. */ /** * `true` when the lock should appear on the resumable "Pending Platform Top - * Ups" orphan surface at all — `statusRaw ∈ [1, 3]` (Broadcast through - * ChainLocked). Lower bar than [canFundIdentity]: a Broadcast (1) lock isn't - * submittable yet but the user should still see it (as "Waiting for - * InstantSend / ChainLock…") so a crash-recovery situation has visible - * continuity through the IS-lock arrival. Upper bound at 3 is load-bearing: - * status 4 (Consumed) is terminal and `resume_asset_lock` rejects it, so a - * Consumed row must never resurface as a perpetual-spinner dead end. + * Ups" orphan surface at all — `statusRaw ∈ [1, 3] ∪ {5}`. Lower bar than + * [canFundIdentity]: a Broadcast (1) lock isn't submittable yet but the user + * should still see it (as "Waiting for InstantSend / ChainLock…") so a + * crash-recovery situation has visible continuity through the IS-lock + * arrival. + * + * Excluding `4` (Consumed) is load-bearing: it is terminal and + * `resume_asset_lock` rejects it, so a Consumed row must never resurface as + * a perpetual-spinner dead end. That exclusion is deliberately NOT written + * as an upper bound of `3` — `5` (RecoveredFromChain) is what the restore + * scan and the chainlock-promotion path write for a lock with proven Core + * finality and unknown Platform-side consumption, and a contiguous `1..3` + * range silently dropped every one of them: a chain-locked top-up the user + * really funded appeared on no surface at all and read as lost funds. * ← Swift `isVisibleAsResumable`. */ val AssetLockEntity.isVisibleAsResumable: Boolean - get() = statusRaw in 1..3 + get() = statusRaw in 1..3 || statusRaw == 5 /** * `true` when the lock has a usable IS-lock / chain-lock proof AND hasn't - * been consumed — `statusRaw == 2 || statusRaw == 3`. Only these can submit - * the funding ST immediately; Built (0) and Broadcast (1) still await - * finality. ← Swift `canFundIdentity`. + * been consumed — `statusRaw ∈ {2, 3, 5}`. Only these can submit the funding + * ST immediately; Built (0) and Broadcast (1) still await finality. + * + * `5` (RecoveredFromChain) qualifies: the restore scan and the + * chainlock-promotion path attach a real `ChainAssetLockProof` before + * writing that status, so Core-side finality is PROVEN and the lock is + * exactly as fundable as a ChainLocked (3) one. What is unknown for a `5` is + * whether Platform already consumed it — and Platform, not the client, is + * the arbiter of that: it rejects an already-spent outpoint with a typed + * error. A user-driven Resume is the surface allowed to ask. (Do not feed + * `5` into an automatic retry sweep — blind retries of historical locks are + * the failure mode the status exists to prevent.) Reading `5` as + * not-yet-final made the UI tell the user to wait for a finality that had + * already happened. ← Swift `canFundIdentity`. */ val AssetLockEntity.canFundIdentity: Boolean - get() = statusRaw == 2 || statusRaw == 3 + get() = statusRaw == 2 || statusRaw == 3 || statusRaw == 5 /** * Human-readable status label. Mirrors the Rust-side `AssetLockStatus` enum. @@ -44,6 +68,9 @@ val AssetLockEntity.statusLabel: String 2 -> "InstantSendLocked" 3 -> "ChainLocked" 4 -> "Consumed" + // Core finality proven, Platform-side consumption unknown. + // Rendered as "Unknown(5)" before this branch existed. + 5 -> "RecoveredFromChain" else -> "Unknown($statusRaw)" } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/ResumableTopUps.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/ResumableTopUps.kt new file mode 100644 index 00000000000..ef98641032d --- /dev/null +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/funding/ResumableTopUps.kt @@ -0,0 +1,104 @@ +package org.dashfoundation.example.ui.funding + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.flowOf +import org.dashfoundation.dashsdk.persistence.entities.AssetLockEntity +import org.dashfoundation.example.navigation.FundFromAssetLock +import org.dashfoundation.example.navigation.ShieldedFund +import org.dashfoundation.example.util.hexToBytes + +/** + * The two `AssetLockFundingType` discriminants that own the "Pending + * Platform Top Ups" recovery surface. + * + * The full domain is `0 IdentityRegistration, 1 IdentityTopUp, + * 2 IdentityTopUpNotBound, 3 IdentityInvitation, 4 AssetLockAddressTopUp, + * 5 AssetLockShieldedAddressTopUp`. Types `0..2` recover on the identity + * screens (`IdentitiesContentView.crossWalletResumableLocks` and its Kotlin + * counterpart, both of which admit only `0..2`); `3` is a bearer voucher + * consumed exclusively by the invitation reclaim flow. That leaves `4` and + * `5` with no recovery home other than this surface — and `5` had none at + * all until it was added here. + */ +internal const val FUNDING_TYPE_ADDRESS_TOP_UP = 4 +internal const val FUNDING_TYPE_SHIELDED_ADDRESS_TOP_UP = 5 + +/** + * Funding types the top-up recovery surface observes, in render order. + * + * This list is the whole point of `AssetLockDao.observeResumableTopUpsByFundingType` + * existing: the older `observeResumableAddressTopUps` hardcodes + * `fundingTypeRaw = 4`, so a stalled or `RecoveredFromChain` SHIELDED + * top-up (`5`) was returned by no query any screen ran. Widening the + * status predicate alone did not fix that — a row invisible on the + * funding-type axis stays invisible however generous the status axis is. + */ +internal val RESUMABLE_TOP_UP_FUNDING_TYPES = listOf( + FUNDING_TYPE_ADDRESS_TOP_UP, + FUNDING_TYPE_SHIELDED_ADDRESS_TOP_UP, +) + +/** + * Cross-wallet, cross-funding-type stream of resumable orphan top-up locks, + * each tagged with the hex wallet id that owns it (needed for the Resume + * navigation, which is wallet-scoped while the Identities tab is not). + * + * [observe] is the DAO seam — production passes + * `AssetLockDao::observeResumableTopUpsByFundingType`, which applies the + * recoverable-status predicate (`[1,3] ∪ {5}`) SQL-side. Taking it as a + * parameter keeps this function a pure combinator: the funding-type fan-out + * that the blocker was about is then assertable without a Room database or + * a Compose runtime. + * + * Emissions are ordered by (wallet, funding type) so the rendered list + * doesn't reshuffle between recompositions. + */ +internal fun resumableTopUpsAcrossWallets( + walletIdHexes: List, + observe: (walletId: ByteArray, fundingTypeRaw: Int) -> Flow>, +): Flow>> { + // `combine` over an empty source list never emits, which would leave the + // section stuck on its initial value instead of resolving to "nothing to + // recover". Short-circuit to an explicit empty emission. + if (walletIdHexes.isEmpty()) return flowOf(emptyList()) + + val slots: List> = walletIdHexes.flatMap { hex -> + RESUMABLE_TOP_UP_FUNDING_TYPES.map { fundingType -> hex to fundingType } + } + val flows = slots.map { (hex, fundingType) -> observe(hex.hexToBytes(), fundingType) } + return combine(flows) { emissions -> + slots.zip(emissions.toList()).flatMap { (slot, locks) -> + val (hex, _) = slot + locks.map { hex to it } + } + } +} + +/** + * The screen a Resume tap on an orphan top-up row must open, or `null` when + * the row's funding type has no resume flow on this surface. + * + * Routing on `fundingTypeRaw` is the second half of the fix: surfacing a + * type-5 row is useless if its Resume lands on + * [org.dashfoundation.example.ui.funding.FundFromAssetLockScreen], whose + * submit calls `resumeFundFromAssetLock` — the platform-ADDRESS resume. A + * shielded lock has to reach `shieldedResumeFundFromAssetLock` instead, + * which is what [ShieldedFund] in resume mode does. + * + * Fail-closed on anything else: identity-family types (`0..3`) recover on + * the identity screens, and an unknown discriminant must not be dispatched + * to a resume flow that would mis-handle it. + */ +internal fun resumeRouteFor(walletIdHex: String, lock: AssetLockEntity): Any? = + when (lock.fundingTypeRaw) { + FUNDING_TYPE_ADDRESS_TOP_UP -> FundFromAssetLock( + walletIdHex = walletIdHex, + resumeOutPointHex = lock.outPointHex, + ) + FUNDING_TYPE_SHIELDED_ADDRESS_TOP_UP -> ShieldedFund( + walletIdHex = walletIdHex, + resumeOutPointHex = lock.outPointHex, + ) + else -> null + } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/IdentitiesHomeScreen.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/IdentitiesHomeScreen.kt index 08d0e4517e2..06c2065cf06 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/IdentitiesHomeScreen.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/IdentitiesHomeScreen.kt @@ -36,9 +36,6 @@ import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavHostController import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.flowOf -import org.dashfoundation.dashsdk.persistence.entities.AssetLockEntity import org.dashfoundation.example.di.LocalAppContainer import org.dashfoundation.example.di.LocalAppState import org.dashfoundation.example.navigation.CreateIdentity @@ -49,8 +46,8 @@ import org.dashfoundation.example.navigation.StateTransitions import org.dashfoundation.example.ui.components.EntityRow import org.dashfoundation.example.ui.components.SectionHeader import org.dashfoundation.example.ui.funding.PendingAssetLocksList +import org.dashfoundation.example.ui.funding.resumableTopUpsAcrossWallets import org.dashfoundation.example.ui.wallet.toHexString -import org.dashfoundation.example.util.hexToBytes import org.dashfoundation.example.util.toHex /** @@ -81,26 +78,24 @@ fun IdentitiesHomeScreen(navController: NavHostController) { // DB-backed resumable orphan asset locks (ADDR-03), merged into the // "Pending Platform Top Ups" surface below alongside the in-flight // controllers. The Identities tab is network-scoped (not wallet-scoped), - // so we observe resumable address-topup locks across EVERY loaded wallet - // and tag each with its owning wallet id for the Resume navigation. + // so we observe resumable top-up locks across EVERY loaded wallet and tag + // each with its owning wallet id for the Resume navigation. // ← the DB-backed orphan half of `PendingPlatformFundFromAssetLocksList.swift`. + // + // BOTH top-up funding types are observed (see + // `RESUMABLE_TOP_UP_FUNDING_TYPES`). This used to call + // `observeResumableAddressTopUps`, which pins `fundingTypeRaw = 4`, so a + // stalled or RecoveredFromChain SHIELDED top-up (`5`) was returned by no + // query this screen — or any other production surface — ran, and the + // funds it represents were unreachable from the UI entirely. val loadedWallets by (manager?.wallets ?: MutableStateFlow(emptyMap())).collectAsStateWithLifecycle() val walletIdHexes = loadedWallets.keys.sorted() val resumableLocks by remember(walletIdHexes) { - if (walletIdHexes.isEmpty()) { - flowOf(emptyList>()) - } else { - val flows = walletIdHexes.map { hex -> - val walletId = hex.hexToBytes() - container.database.assetLockDao().observeResumableAddressTopUps(walletId) - } - combine(flows) { arrays -> - walletIdHexes.zip(arrays.toList()).flatMap { (hex, locks) -> - locks.map { hex to it } - } - } - } + resumableTopUpsAcrossWallets( + walletIdHexes = walletIdHexes, + observe = container.database.assetLockDao()::observeResumableTopUpsByFundingType, + ) }.collectAsStateWithLifecycle(initialValue = emptyList()) Scaffold( diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/shielded/ShieldedFundScreen.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/shielded/ShieldedFundScreen.kt index bb7aeb6b18a..b12de248558 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/shielded/ShieldedFundScreen.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/shielded/ShieldedFundScreen.kt @@ -33,6 +33,7 @@ import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavHostController import org.dashfoundation.dashsdk.funding.ShieldedProver +import org.dashfoundation.dashsdk.persistence.entities.AssetLockEntity import org.dashfoundation.example.di.LocalAppContainer import org.dashfoundation.example.navigation.ShieldedFundProgress import org.dashfoundation.example.services.shielded.ShieldedFundFromAssetLockCoordinator.StartFundingResult @@ -40,8 +41,13 @@ import org.dashfoundation.example.ui.components.ErrorAlertDialog import org.dashfoundation.example.ui.components.FormSection import org.dashfoundation.example.ui.components.LabeledContent import org.dashfoundation.example.ui.components.SubmitButton +import org.dashfoundation.example.ui.funding.canFundIdentity +import org.dashfoundation.example.ui.funding.parseOutPoint +import org.dashfoundation.example.ui.funding.shortOutPointDisplay +import org.dashfoundation.example.ui.funding.statusLabel import org.dashfoundation.example.util.hexToBytes import org.dashfoundation.example.util.toHex +import java.util.UUID /** * Shield funds from an asset lock — port of `ShieldedFundFromAssetLockView.swift`. @@ -58,20 +64,53 @@ import org.dashfoundation.example.util.toHex * `platform_wallet_manager_shielded_fund_from_asset_lock`; the screen then * navigates to the dismissal-safe progress view. The Orchard note arrives on * the next shielded sync pass, not synchronously. + * + * RESUME mode (← the Swift view's `resumeFromLock` parameter): when + * [resumeOutPointHex] is non-null the screen hides the Amount section (the + * lock and its amount were fixed at original build time) and routes Submit to + * [org.dashfoundation.dashsdk.wallet.PlatformWalletManager.shieldedResumeFundFromAssetLock] + * instead of `shieldedFundFromAssetLock`, seeded with the parsed outpoint. + * The recipient is still chosen here — a shielded orphan lock carries no + * recipient stamp, because the Orchard recipient is an external address + * picked at ST-submit time, not allocated from the wallet. + * + * This is the resume path for `fundingTypeRaw == 5` + * (AssetLockShieldedAddressTopUp) rows on the "Pending Platform Top Ups" + * surface. Before it existed those rows had nowhere to go: the surface only + * queried funding type 4, and its Resume opened the platform-ADDRESS screen, + * whose submit calls the wrong FFI for a shielded lock. */ @OptIn(ExperimentalMaterial3Api::class) @Composable -fun ShieldedFundScreen(walletIdHex: String, navController: NavHostController) { +fun ShieldedFundScreen( + walletIdHex: String, + navController: NavHostController, + resumeOutPointHex: String? = null, +) { ShieldedGate(navController) { val container = LocalAppContainer.current val walletId = remember(walletIdHex) { walletIdHex.hexToBytes() } val manager by container.walletManagerStore.activeManager.collectAsStateWithLifecycle() + val isResume = resumeOutPointHex != null var amountText by rememberSaveable { mutableStateOf("") } var recipientHex by rememberSaveable { mutableStateOf("") } var error by remember { mutableStateOf(null) } var isSubmitting by remember { mutableStateOf(false) } + // In resume mode, load the tracked lock so we can show its amount + + // status. Keyed by `outPointHex` (primary key); a null result means + // the lock was swept between opening the list and this screen. + // ← FundFromAssetLockScreen's identical resume-mode load. + val resumeLock by produceState( + initialValue = null, + resumeOutPointHex, + ) { + value = resumeOutPointHex?.let { + container.database.assetLockDao().getByOutPointHex(it) + } + } + // Prover status + fee estimate (bridged, single-note-with-change = 2 actions). val proverReady by produceState(initialValue = false) { runCatching { ShieldedProver.warmUp() } @@ -102,13 +141,18 @@ fun ShieldedFundScreen(walletIdHex: String, navController: NavHostController) { val recipient = overrideRecipient ?: defaultRecipient val amount = amountText.toLongOrNull() - val canSubmit = - manager != null && recipient != null && amount != null && amount > 0 && !isSubmitting + // Resume only needs a recipient (+ the loaded lock): the shield value + // is derived Rust-side from the existing lock. Fresh needs an amount + // too. ← FundFromAssetLockScreen's `canSubmit`. + val canSubmit = manager != null && recipient != null && !isSubmitting && + if (isResume) resumeLock != null else (amount != null && amount > 0) Scaffold( topBar = { TopAppBar( - title = { Text("Shield from Asset Lock") }, + title = { + Text(if (isResume) "Resume Shield" else "Shield from Asset Lock") + }, navigationIcon = { IconButton(onClick = { navController.popBackStack() }) { Icon(Icons.AutoMirrored.Filled.ArrowBack, contentDescription = "Back") @@ -137,6 +181,47 @@ fun ShieldedFundScreen(walletIdHex: String, navController: NavHostController) { ) } + if (isResume) { + // Read-only summary of the lock being resumed — replaces + // the Amount section (the locked amount is fixed by the + // original build). ← Swift `resumeFromAssetLockSection`. + FormSection(title = "Resuming") { + val lock = resumeLock + if (lock == null) { + Text( + "This asset lock is no longer tracked. Return to the " + + "Pending Platform Top Ups list.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + modifier = Modifier.testTag("shieldedFund.resume.missing"), + ) + } else { + Text( + "Asset Lock ${lock.shortOutPointDisplay}", + style = MaterialTheme.typography.bodyMedium, + modifier = Modifier.testTag("shieldedFund.resume.outpoint"), + ) + Text( + "${lock.amountDuffs} duffs · ${lock.statusLabel}", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + Text( + if (lock.canFundIdentity) { + "The asset lock already reached a usable proof state. " + + "Pick a recipient to complete the shield." + } else { + "The asset lock is broadcast and still awaiting " + + "InstantSend / ChainLock finality. Resuming will wait " + + "for finality, then shield into the pool." + }, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + } + FormSection(title = "Recipient") { OutlinedTextField( value = recipientHex, @@ -156,39 +241,86 @@ fun ShieldedFundScreen(walletIdHex: String, navController: NavHostController) { ) } - FormSection(title = "Amount") { - OutlinedTextField( - value = amountText, - onValueChange = { amountText = it.filter(Char::isDigit) }, - label = { Text("Amount (duffs)") }, - singleLine = true, - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), - modifier = Modifier.fillMaxWidth().testTag("shieldedFund.amount"), - ) + if (!isResume) { + FormSection(title = "Amount") { + OutlinedTextField( + value = amountText, + onValueChange = { amountText = it.filter(Char::isDigit) }, + label = { Text("Amount (duffs)") }, + singleLine = true, + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), + modifier = Modifier.fillMaxWidth().testTag("shieldedFund.amount"), + ) + } } SubmitButton( - text = "Shield", + text = if (isResume) "Resume Shield" else "Shield", isLoading = isSubmitting, enabled = canSubmit, modifier = Modifier.fillMaxWidth().testTag("shieldedFund.submit"), ) { val m = manager ?: return@SubmitButton val recipientBytes = recipient ?: return@SubmitButton - val amountDuffs = amount ?: return@SubmitButton + + // Resume mode dispatches to a different FFI than a fresh + // shield, so resolve the whole body up front — including + // the outpoint parse, which must not fail after the + // coordinator has already claimed the slot. The operation + // id carries the resumed lock's outpoint: resumable locks + // default to the same wallet-owned shielded recipient, so + // the coordinator's slot key alone cannot tell two locks + // apart and would silently reuse the first controller. + // A fresh shield mints a NEW id per submission: the + // coordinator retains a completed controller for ~30s, + // and a fixed marker would match it and rebind — showing + // the old result instead of running this submission's + // body. Minted here, at submission time, so returning to + // this form and shielding again is always a new + // operation ("resume:" stays stable so a + // re-tap of the SAME lock still rebinds). + val (operationId, submitBody) = if (isResume) { + val lock = resumeLock ?: return@SubmitButton + val parsed = parseOutPoint(lock.outPointHex) + if (parsed == null) { + error = "Could not parse asset lock outpoint: ${lock.outPointHex}" + return@SubmitButton + } + val (txid, vout) = parsed + val body: suspend () -> Unit = { + m.shieldedResumeFundFromAssetLock( + walletId = walletId, + outPointTxid = txid, + outPointVout = vout, + recipientRaw43 = recipientBytes, + ) + } + "resume:${lock.outPointHex}" to body + } else { + val amountDuffs = amount ?: return@SubmitButton + val body: suspend () -> Unit = { + m.shieldedFundFromAssetLock( + walletId = walletId, + recipientRaw43 = recipientBytes, + amountDuffs = amountDuffs, + ) + } + "shield:${UUID.randomUUID()}" to body + } + isSubmitting = true // Start the funding through the coordinator (dismissal-safe, // per-wallet serialized). The body performs the shield FFI. + // Resume shares the coordinator with fresh shields on + // purpose: both consume the same per-wallet shield_guard + // Rust-side, so a resume racing a fresh shield on one + // wallet has to be blocked by the same gate. val result = container.shieldedFundCoordinator.startFunding( walletId = walletId, recipientRaw43 = recipientBytes, - ) { - m.shieldedFundFromAssetLock( - walletId = walletId, - recipientRaw43 = recipientBytes, - amountDuffs = amountDuffs, - ) - } + operationId = operationId, + body = submitBody, + ) when (result) { is StartFundingResult.Started -> { navController.navigate( diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockCoordinatorTest.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockCoordinatorTest.kt index b4e846513ba..1b075da485c 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockCoordinatorTest.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/shielded/ShieldedFundFromAssetLockCoordinatorTest.kt @@ -11,6 +11,7 @@ import org.dashfoundation.example.services.shielded.ShieldedFundFromAssetLockCon import org.dashfoundation.example.services.shielded.ShieldedFundFromAssetLockCoordinator.StartFundingResult import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotSame import org.junit.Assert.assertNull import org.junit.Assert.assertSame import org.junit.Assert.assertTrue @@ -54,7 +55,7 @@ class ShieldedFundFromAssetLockCoordinatorTest { val coordinator = coordinator() val gate = CompletableDeferred() - val controller = coordinator.startFunding(walletA, recipientX) { gate.await() }.controller() + val controller = coordinator.startFunding(walletA, recipientX, "shield") { gate.await() }.controller() assertEquals(Phase.InFlight, controller.phase.value) assertTrue(controller.phase.value.isActive) @@ -70,7 +71,7 @@ class ShieldedFundFromAssetLockCoordinatorTest { val coordinator = coordinator() val gate = CompletableDeferred() - val controller = coordinator.startFunding(walletA, recipientX) { gate.await() }.controller() + val controller = coordinator.startFunding(walletA, recipientX, "shield") { gate.await() }.controller() gate.completeExceptionally(RuntimeException("proof build failed")) advanceUntilIdle() @@ -86,8 +87,8 @@ class ShieldedFundFromAssetLockCoordinatorTest { val gate = CompletableDeferred() var invocations = 0 - val first = coordinator.startFunding(walletA, recipientX) { invocations++; gate.await() }.controller() - val second = coordinator.startFunding(walletA, recipientX) { invocations++; gate.await() }.controller() + val first = coordinator.startFunding(walletA, recipientX, "shield") { invocations++; gate.await() }.controller() + val second = coordinator.startFunding(walletA, recipientX, "shield") { invocations++; gate.await() }.controller() assertSame(first, second) assertEquals(1, coordinator.controllers.value.size) @@ -103,10 +104,10 @@ class ShieldedFundFromAssetLockCoordinatorTest { val coordinator = coordinator() val gate = CompletableDeferred() - val started = coordinator.startFunding(walletA, recipientX) { gate.await() } + val started = coordinator.startFunding(walletA, recipientX, "shield") { gate.await() } assertTrue(started is StartFundingResult.Started) - val blocked = coordinator.startFunding(walletA, recipientY) { gate.await() } + val blocked = coordinator.startFunding(walletA, recipientY, "shield") { gate.await() } assertTrue(blocked is StartFundingResult.BlockedByOtherWalletFunding) assertSame( started.controller(), @@ -124,8 +125,8 @@ class ShieldedFundFromAssetLockCoordinatorTest { val coordinator = coordinator() val gate = CompletableDeferred() - val a = coordinator.startFunding(walletA, recipientX) { gate.await() } - val b = coordinator.startFunding(walletB, recipientY) { gate.await() } + val a = coordinator.startFunding(walletA, recipientX, "shield") { gate.await() } + val b = coordinator.startFunding(walletB, recipientY, "shield") { gate.await() } assertTrue(a is StartFundingResult.Started) assertTrue(b is StartFundingResult.Started) assertEquals(2, coordinator.controllers.value.size) @@ -140,11 +141,11 @@ class ShieldedFundFromAssetLockCoordinatorTest { val firstGate = CompletableDeferred() val secondGate = CompletableDeferred() - coordinator.startFunding(walletA, recipientX) { firstGate.await() } + coordinator.startFunding(walletA, recipientX, "shield") { firstGate.await() } firstGate.complete(Unit) runCurrent() // first → Completed (not active) - val second = coordinator.startFunding(walletA, recipientY) { secondGate.await() } + val second = coordinator.startFunding(walletA, recipientY, "shield") { secondGate.await() } assertTrue(second is StartFundingResult.Started) secondGate.complete(Unit) @@ -156,7 +157,7 @@ class ShieldedFundFromAssetLockCoordinatorTest { val coordinator = coordinator(retentionMillis = 30_000L, pollMillis = 1_000L) val gate = CompletableDeferred() - val controller = coordinator.startFunding(walletA, recipientX) { gate.await() }.controller() + val controller = coordinator.startFunding(walletA, recipientX, "shield") { gate.await() }.controller() gate.complete(Unit) runCurrent() assertTrue(controller.phase.value is Phase.Completed) @@ -175,7 +176,7 @@ class ShieldedFundFromAssetLockCoordinatorTest { val coordinator = coordinator() val gate = CompletableDeferred() - coordinator.startFunding(walletA, recipientX) { gate.await() } + coordinator.startFunding(walletA, recipientX, "shield") { gate.await() } gate.completeExceptionally(RuntimeException("nope")) advanceUntilIdle() assertEquals(1, coordinator.controllers.value.size) @@ -184,4 +185,174 @@ class ShieldedFundFromAssetLockCoordinatorTest { assertTrue(coordinator.controllers.value.isEmpty()) assertNull(coordinator.controller(walletA, recipientX)) } + + // ------------------------------------------------------------------ + // Operation identity: two resumable locks share the default recipient + // ------------------------------------------------------------------ + + @Test + fun `re-tapping the same resumable lock reuses the in-flight controller`() = runTest { + val coordinator = coordinator() + val gate = CompletableDeferred() + var invocations = 0 + + val first = coordinator + .startFunding(walletA, recipientX, "resume:aa:0") { invocations++; gate.await() } + .controller() + val second = coordinator.startFunding(walletA, recipientX, "resume:aa:0") { + invocations++ + gate.await() + } + + assertTrue(second is StartFundingResult.Started) + assertSame(first, second.controller()) + runCurrent() + assertEquals(1, invocations) + + gate.complete(Unit) + advanceUntilIdle() + } + + @Test + fun `a second resumable lock on the same recipient is blocked while the first is in flight`() = runTest { + val coordinator = coordinator() + val gate = CompletableDeferred() + var secondInvocations = 0 + + val first = coordinator.startFunding(walletA, recipientX, "resume:aa:0") { gate.await() } + assertTrue(first is StartFundingResult.Started) + + // Same wallet, same (default) recipient, DIFFERENT lock: reusing + // the first controller would silently drop this body. The caller + // must see the wallet-serialization verdict instead. + val second = coordinator.startFunding(walletA, recipientX, "resume:bb:0") { + secondInvocations++ + } + assertTrue(second is StartFundingResult.BlockedByOtherWalletFunding) + assertSame( + first.controller(), + (second as StartFundingResult.BlockedByOtherWalletFunding).blocker, + ) + assertEquals(1, coordinator.controllers.value.size) + runCurrent() + assertEquals(0, secondInvocations) + + gate.complete(Unit) + advanceUntilIdle() + } + + @Test + fun `a second resumable lock starts during the first's completed retention window`() = runTest { + val coordinator = coordinator(retentionMillis = 30_000L, pollMillis = 1_000L) + val firstGate = CompletableDeferred() + val secondGate = CompletableDeferred() + var secondInvocations = 0 + + val first = coordinator + .startFunding(walletA, recipientX, "resume:aa:0") { firstGate.await() } + .controller() + firstGate.complete(Unit) + runCurrent() + assertTrue(first.phase.value is Phase.Completed) + + // Within the 30s retention window, resume a DIFFERENT lock on the + // same recipient: a fresh controller must actually run its body — + // the retained completed controller may show first's terminal + // state to a re-tap of first only. + val second = coordinator.startFunding(walletA, recipientX, "resume:bb:0") { + secondInvocations++ + secondGate.await() + } + assertTrue(second is StartFundingResult.Started) + val secondController = second.controller() + assertNotSame(first, secondController) + runCurrent() + assertEquals(1, secondInvocations) + assertEquals(Phase.InFlight, secondController.phase.value) + + // The FIRST controller's retention sweep expires while the second + // operation is still in flight — it must not evict the + // replacement occupying its old slot. + advanceTimeBy(31_000L) + runCurrent() + assertSame(secondController, coordinator.controller(walletA, recipientX)) + assertEquals(Phase.InFlight, secondController.phase.value) + + secondGate.complete(Unit) + advanceUntilIdle() + assertEquals(Phase.Completed, secondController.phase.value) + } + + // ------------------------------------------------------------------ + // Operation identity: fresh shields mint a unique id per submission + // ------------------------------------------------------------------ + + @Test + fun `a second fresh shield with a distinct id replaces the retained completed controller`() = runTest { + val coordinator = coordinator(retentionMillis = 30_000L, pollMillis = 1_000L) + val firstGate = CompletableDeferred() + val secondGate = CompletableDeferred() + var secondInvocations = 0 + + // The call sites mint "shield:" at submission time; a + // FIXED marker here would rebind to the first controller's + // retained Completed state and never run the second body. + val first = coordinator + .startFunding(walletA, recipientX, "shield:attempt-1") { firstGate.await() } + .controller() + firstGate.complete(Unit) + runCurrent() + assertTrue(first.phase.value is Phase.Completed) + + // Within the retention window, a SECOND fresh shield to the + // same recipient must replace the retained controller and + // execute its own body. + val second = coordinator.startFunding(walletA, recipientX, "shield:attempt-2") { + secondInvocations++ + secondGate.await() + } + assertTrue(second is StartFundingResult.Started) + val secondController = second.controller() + assertNotSame(first, secondController) + runCurrent() + assertEquals(1, secondInvocations) + assertEquals(Phase.InFlight, secondController.phase.value) + + secondGate.complete(Unit) + runCurrent() + assertEquals(Phase.Completed, secondController.phase.value) + // The replacement owns the slot until its own retention sweep + // retires it as usual. + assertSame(secondController, coordinator.controller(walletA, recipientX)) + advanceUntilIdle() + assertNull(coordinator.controller(walletA, recipientX)) + } + + @Test + fun `a second fresh shield is blocked while the first is still in flight`() = runTest { + val coordinator = coordinator() + val gate = CompletableDeferred() + var secondInvocations = 0 + + val first = coordinator.startFunding(walletA, recipientX, "shield:attempt-1") { gate.await() } + assertTrue(first is StartFundingResult.Started) + + // Distinct per-submission ids mean a second fresh shield during + // the first's flight is a DIFFERENT operation: it must surface + // the wallet-serialization verdict, not silently rebind. + val second = coordinator.startFunding(walletA, recipientX, "shield:attempt-2") { + secondInvocations++ + } + assertTrue(second is StartFundingResult.BlockedByOtherWalletFunding) + assertSame( + first.controller(), + (second as StartFundingResult.BlockedByOtherWalletFunding).blocker, + ) + assertEquals(1, coordinator.controllers.value.size) + runCurrent() + assertEquals(0, secondInvocations) + + gate.complete(Unit) + advanceUntilIdle() + } } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/funding/AssetLockDisplayTest.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/funding/AssetLockDisplayTest.kt index 00f9c3ac01a..5bf52374030 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/funding/AssetLockDisplayTest.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/funding/AssetLockDisplayTest.kt @@ -33,21 +33,42 @@ class AssetLockDisplayTest { ) @Test - fun `isVisibleAsResumable is true only for statusRaw 1 through 3`() { + fun `isVisibleAsResumable covers 1 through 3 plus RecoveredFromChain`() { assertFalse(lock(0).isVisibleAsResumable) // Built assertTrue(lock(1).isVisibleAsResumable) // Broadcast assertTrue(lock(2).isVisibleAsResumable) // InstantSendLocked assertTrue(lock(3).isVisibleAsResumable) // ChainLocked assertFalse(lock(4).isVisibleAsResumable) // Consumed + assertTrue(lock(5).isVisibleAsResumable) // RecoveredFromChain } + /** + * The exclusion of `4` is by NAME, not by an upper bound — `5` sits + * above it numerically and is resumable. A `statusRaw <= 3` bound reads + * as equivalent and silently hides every restored lock. + */ @Test - fun `canFundIdentity is true only for statusRaw 2 or 3`() { + fun `isVisibleAsResumable excludes Consumed without bounding above it`() { + assertFalse(lock(4).isVisibleAsResumable) + assertTrue(lock(5).isVisibleAsResumable) + } + + /** + * RecoveredFromChain (5) is FUNDABLE. The restore scan and the + * chainlock-promotion path attach a real `ChainAssetLockProof` before + * writing it, so Core-side finality is proven — treating it as + * not-yet-final routed the row into the "waiting for InstantSend / + * ChainLock finality" copy and told the user to wait for something that + * had already happened. + */ + @Test + fun `canFundIdentity covers 2, 3 and RecoveredFromChain`() { assertFalse(lock(0).canFundIdentity) assertFalse(lock(1).canFundIdentity) assertTrue(lock(2).canFundIdentity) assertTrue(lock(3).canFundIdentity) assertFalse(lock(4).canFundIdentity) + assertTrue(lock(5).canFundIdentity) } @Test @@ -57,6 +78,8 @@ class AssetLockDisplayTest { assertEquals("InstantSendLocked", lock(2).statusLabel) assertEquals("ChainLocked", lock(3).statusLabel) assertEquals("Consumed", lock(4).statusLabel) + // Regression: rendered as "Unknown(5)" before the branch existed. + assertEquals("RecoveredFromChain", lock(5).statusLabel) assertEquals("Unknown(7)", lock(7).statusLabel) } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/funding/ResumableTopUpsTest.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/funding/ResumableTopUpsTest.kt new file mode 100644 index 00000000000..27373ad199f --- /dev/null +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/funding/ResumableTopUpsTest.kt @@ -0,0 +1,207 @@ +package org.dashfoundation.example.ui.funding + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.dashfoundation.dashsdk.persistence.entities.AssetLockEntity +import org.dashfoundation.example.navigation.FundFromAssetLock +import org.dashfoundation.example.navigation.ShieldedFund +import org.dashfoundation.example.util.toHex +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Caller-side tests for the "Pending Platform Top Ups" recovery surface. + * + * `AssetLockResumableDaoTest` (in `:sdk`) already pins the DAO query itself — + * that `observeResumableTopUpsByFundingType(walletId, 5)` returns the + * `[1,3] ∪ {5}` status set. What that cannot show is whether any production + * screen ASKS for funding type 5. It didn't: the only caller ran + * `observeResumableAddressTopUps`, which hardcodes `fundingTypeRaw = 4`, so a + * stalled or RecoveredFromChain shielded top-up was absent from every host + * surface no matter how correct the query was. + * + * These tests pin the two halves of that wiring: + * + * 1. [resumableTopUpsAcrossWallets] fans out over BOTH top-up funding + * types, for every loaded wallet. + * 2. [resumeRouteFor] sends each row to the resume flow that matches its + * funding type — a shielded lock must not land on the platform-address + * screen, whose submit calls the wrong FFI. + */ +class ResumableTopUpsTest { + + private val walletA = ByteArray(32) { 1 } + private val walletB = ByteArray(32) { 2 } + private val walletAHex = walletA.toHex() + private val walletBHex = walletB.toHex() + + private fun lock( + outPointHex: String, + fundingTypeRaw: Int, + statusRaw: Int = 5, + ): AssetLockEntity = AssetLockEntity( + outPointHex = outPointHex, + walletId = ByteArray(32), + transactionBytes = ByteArray(0), + fundingTypeRaw = fundingTypeRaw, + identityIndexRaw = 0, + amountDuffs = 100_000L, + statusRaw = statusRaw, + ) + + /** + * Records every `(walletIdHex, fundingTypeRaw)` the caller asks for, and + * serves whatever rows the test registered for that pair. + */ + private class RecordingObserver( + private val rows: Map, List> = emptyMap(), + ) { + val requested = mutableListOf>() + + fun observe(walletId: ByteArray, fundingTypeRaw: Int): Flow> { + val key = walletId.toHex() to fundingTypeRaw + requested += key + return flowOf(rows[key] ?: emptyList()) + } + } + + // ── funding-type fan-out ────────────────────────────────────────── + + /** + * The blocker itself. Funding type 5 must be among the types the screen + * observes — otherwise the parameterized DAO query has no production + * caller and the shielded rows it can return are never requested. + */ + @Test + fun `observes both address and shielded top-up funding types`() = runTest { + val observer = RecordingObserver() + + resumableTopUpsAcrossWallets(listOf(walletAHex), observer::observe).first() + + assertEquals( + listOf(walletAHex to 4, walletAHex to 5), + observer.requested, + ) + } + + /** + * The user-visible consequence: a shielded (`fundingTypeRaw == 5`) + * RecoveredFromChain lock reaches the rendered list, tagged with its + * owning wallet so Resume can navigate wallet-scoped. + */ + @Test + fun `surfaces a shielded RecoveredFromChain top-up`() = runTest { + val shielded = lock("shielded:0", fundingTypeRaw = 5, statusRaw = 5) + val observer = RecordingObserver(mapOf((walletAHex to 5) to listOf(shielded))) + + val rows = resumableTopUpsAcrossWallets(listOf(walletAHex), observer::observe).first() + + assertEquals(listOf(walletAHex to shielded), rows) + } + + /** Address rows are unaffected by the widening. */ + @Test + fun `still surfaces address top-ups alongside shielded ones`() = runTest { + val address = lock("address:0", fundingTypeRaw = 4, statusRaw = 1) + val shielded = lock("shielded:0", fundingTypeRaw = 5, statusRaw = 5) + val observer = RecordingObserver( + mapOf( + (walletAHex to 4) to listOf(address), + (walletAHex to 5) to listOf(shielded), + ), + ) + + val rows = resumableTopUpsAcrossWallets(listOf(walletAHex), observer::observe).first() + + assertEquals(listOf(walletAHex to address, walletAHex to shielded), rows) + } + + /** + * Each wallet is observed on both funding types, and every row keeps the + * hex of the wallet that owns it — the Identities tab is cross-wallet, + * so mis-tagging would send Resume to the wrong wallet's screen. + */ + @Test + fun `tags rows with their owning wallet across wallets`() = runTest { + val aLock = lock("a:0", fundingTypeRaw = 4, statusRaw = 2) + val bLock = lock("b:0", fundingTypeRaw = 5, statusRaw = 5) + val observer = RecordingObserver( + mapOf( + (walletAHex to 4) to listOf(aLock), + (walletBHex to 5) to listOf(bLock), + ), + ) + + val rows = resumableTopUpsAcrossWallets( + listOf(walletAHex, walletBHex), + observer::observe, + ).first() + + assertEquals(listOf(walletAHex to aLock, walletBHex to bLock), rows) + assertEquals( + listOf(walletAHex to 4, walletAHex to 5, walletBHex to 4, walletBHex to 5), + observer.requested, + ) + } + + /** + * With no wallets loaded the flow must still emit — `combine` over an + * empty source list never emits, which would leave the section pinned to + * its initial value instead of resolving to "nothing to recover". + */ + @Test + fun `emits an empty list when no wallets are loaded`() = runTest { + val observer = RecordingObserver() + + val rows = resumableTopUpsAcrossWallets(emptyList(), observer::observe).first() + + assertEquals(emptyList>(), rows) + assertEquals(emptyList>(), observer.requested) + } + + // ── resume routing ──────────────────────────────────────────────── + + /** + * The second half of the blocker: surfacing a shielded row is useless if + * its Resume opens the platform-address screen, whose submit calls + * `resumeFundFromAssetLock` rather than + * `shieldedResumeFundFromAssetLock`. + */ + @Test + fun `routes a shielded lock to the shielded resume screen`() { + val shielded = lock("shielded:0", fundingTypeRaw = 5) + + assertEquals( + ShieldedFund(walletIdHex = walletAHex, resumeOutPointHex = "shielded:0"), + resumeRouteFor(walletAHex, shielded), + ) + } + + @Test + fun `routes an address lock to the platform-address resume screen`() { + val address = lock("address:0", fundingTypeRaw = 4) + + assertEquals( + FundFromAssetLock(walletIdHex = walletAHex, resumeOutPointHex = "address:0"), + resumeRouteFor(walletAHex, address), + ) + } + + /** + * Fail closed. Identity-family types (0..3) recover on the identity + * screens; an unknown discriminant must not be dispatched into a resume + * flow that would submit the wrong transition for it. + */ + @Test + fun `refuses to route funding types that have no resume flow here`() { + for (fundingType in listOf(0, 1, 2, 3, 6, 99)) { + assertNull( + "fundingTypeRaw $fundingType must not route to a top-up resume screen", + resumeRouteFor(walletAHex, lock("x:0", fundingTypeRaw = fundingType)), + ) + } + } +} diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/AssetLockDao.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/AssetLockDao.kt index dddd64d56db..339f2b36bb9 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/AssetLockDao.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/AssetLockDao.kt @@ -59,18 +59,54 @@ interface AssetLockDao { /** * Resumable Platform-address top-up locks — `fundingTypeRaw == 4` - * (AssetLockAddressTopUp) and `statusRaw ∈ [1, 3]` (Broadcast through - * ChainLocked, excluding Built and Consumed). Backs the "Pending - * Platform Top Ups" orphan surface (← the SwiftData `@Query` behind - * `PendingPlatformFundFromAssetLocksList.swift`, whose Swift filter is - * `fundingTypeRaw == 4 && isVisibleAsResumable`). + * (AssetLockAddressTopUp) and a recoverable, non-terminal status. + * Backs the "Pending Platform Top Ups" orphan surface (← the SwiftData + * `@Query` behind `PendingPlatformFundFromAssetLocksList.swift`, whose + * Swift filter is `fundingTypeRaw == 4 && isVisibleAsResumable`). + * + * The recoverable set is `[1, 3] ∪ {5}` — Broadcast, InstantSendLocked, + * ChainLocked, and RecoveredFromChain. `0` (Built) is excluded because + * the funding transaction has not been broadcast; `4` (Consumed) is the + * terminal tombstone that Rust's `resume_asset_lock` rejects outright. + * + * `5` (RecoveredFromChain) is not a gap in the ordering — it is a + * distinct status written by the restore scan and by the chainlock + * promotion path for a lock whose Core finality is proven but whose + * Platform-side consumption is unknown (`sync/reconstruction.rs`). A + * range bounded at `3` dropped exactly those rows, so a chain-locked + * address top-up rebuilt from history appeared on no surface at all. */ @Query( "SELECT * FROM asset_locks WHERE walletId = :walletId " + - "AND fundingTypeRaw = 4 AND statusRaw >= 1 AND statusRaw <= 3" + "AND fundingTypeRaw = 4 " + + "AND ((statusRaw >= 1 AND statusRaw <= 3) OR statusRaw = 5)" ) fun observeResumableAddressTopUps(walletId: ByteArray): Flow> + /** + * Funding-type-scoped variant of [observeResumableAddressTopUps], using + * the identical recoverable-status predicate. + * + * Exists because shielded address top-ups (`fundingTypeRaw == 5`, + * `AssetLockShieldedAddressTopUp`) had no resumable query at all: the + * query above is pinned to `4`, and the identity-recovery surface fed by + * `TrackedAssetLock.eligibleFromNative` deliberately admits only funding + * types `0..2`. A stalled or chain-locked shielded top-up was therefore + * invisible on every host surface. + * + * Pass `4` (address) or `5` (shielded). Funding types `0..3` are + * identity-family locks, whose recovery surface is the identity screens. + */ + @Query( + "SELECT * FROM asset_locks WHERE walletId = :walletId " + + "AND fundingTypeRaw = :fundingTypeRaw " + + "AND ((statusRaw >= 1 AND statusRaw <= 3) OR statusRaw = 5)" + ) + fun observeResumableTopUpsByFundingType( + walletId: ByteArray, + fundingTypeRaw: Int, + ): Flow> + @Query("SELECT * FROM asset_locks WHERE outPointHex = :outPointHex") suspend fun getByOutPointHex(outPointHex: String): AssetLockEntity? diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/AssetLockEntity.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/AssetLockEntity.kt index 5181fe92545..c5505e3746b 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/AssetLockEntity.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/AssetLockEntity.kt @@ -40,7 +40,15 @@ data class AssetLockEntity( val amountDuffs: Long, /** * `AssetLockStatus` discriminant: 0 Built, 1 Broadcast, - * 2 InstantSendLocked, 3 ChainLocked, 4 Consumed. + * 2 InstantSendLocked, 3 ChainLocked, 4 Consumed, + * 5 RecoveredFromChain. + * + * `4` is terminal; `5` is NOT, its higher discriminant + * notwithstanding. `5` means Core finality is proven while + * Platform-side consumption is unknown — what the restore scan and the + * chainlock-promotion path write — so it belongs in every "still + * recoverable" predicate alongside `1..3`, and a contiguous `1..3` + * range silently drops it. */ val statusRaw: Int, /** diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/AssetLockResumableDaoTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/AssetLockResumableDaoTest.kt new file mode 100644 index 00000000000..0c5cdebcf0f --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/AssetLockResumableDaoTest.kt @@ -0,0 +1,182 @@ +package org.dashfoundation.dashsdk.persistence + +import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import org.dashfoundation.dashsdk.persistence.entities.AssetLockEntity +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * In-memory Room contract tests for the resumable asset-lock predicates on + * [org.dashfoundation.dashsdk.persistence.dao.AssetLockDao]. + * + * The behavior under test is the status domain. Rust's `AssetLockStatus` + * is `0 Built, 1 Broadcast, 2 InstantSendLocked, 3 ChainLocked, + * 4 Consumed, 5 RecoveredFromChain`, and the recoverable set is NOT a + * contiguous range: `4` is the terminal tombstone that must stay hidden, + * while `5` — written by the restore scan and the chainlock-promotion path + * for a lock with proven Core finality and unknown Platform-side + * consumption — must be visible. + * + * Expressing that as `statusRaw >= 1 AND statusRaw <= 3` dropped every + * recovered row, so a chain-locked top-up the user really funded appeared + * on no host surface at all. These tests pin both ends: `5` in, `4` out. + */ +@RunWith(RobolectricTestRunner::class) +class AssetLockResumableDaoTest { + + private lateinit var db: DashDatabase + + private val walletId = ByteArray(32) { 1 } + private val otherWalletId = ByteArray(32) { 2 } + + /** `AssetLockFundingType` discriminants. */ + private val fundingAddressTopUp = 4 + private val fundingShieldedTopUp = 5 + private val fundingIdentityRegistration = 0 + + @Before + fun setUp() { + db = DashDatabase.createInMemory(ApplicationProvider.getApplicationContext()) + } + + @After + fun tearDown() { + db.close() + } + + private suspend fun insert( + outPointHex: String, + statusRaw: Int, + fundingTypeRaw: Int = fundingAddressTopUp, + owner: ByteArray = walletId, + ) { + db.assetLockDao().upsert( + AssetLockEntity( + outPointHex = outPointHex, + walletId = owner, + transactionBytes = ByteArray(4), + fundingTypeRaw = fundingTypeRaw, + identityIndexRaw = 0, + amountDuffs = 10_000, + statusRaw = statusRaw, + ), + ) + } + + private suspend fun resumableAddressOutpoints(): List = + db.assetLockDao().observeResumableAddressTopUps(walletId).first() + .map { it.outPointHex } + .sorted() + + // ── observeResumableAddressTopUps ───────────────────────────────── + + /** + * The regression itself: status 5 must be returned. Before the fix the + * upper bound of `3` hid it, and the funds it represents were + * unreachable from the UI. + */ + @Test + fun resumableAddressTopUpsIncludeRecoveredFromChain() = runTest { + insert("aa:0", statusRaw = 5) + + assertEquals(listOf("aa:0"), resumableAddressOutpoints()) + } + + /** Every recoverable status, and only those. */ + @Test + fun resumableAddressTopUpsCoverTheWholeRecoverableDomain() = runTest { + insert("built:0", statusRaw = 0) + insert("broadcast:0", statusRaw = 1) + insert("islocked:0", statusRaw = 2) + insert("chainlocked:0", statusRaw = 3) + insert("consumed:0", statusRaw = 4) + insert("recovered:0", statusRaw = 5) + + assertEquals( + listOf("broadcast:0", "chainlocked:0", "islocked:0", "recovered:0"), + resumableAddressOutpoints(), + ) + } + + /** + * The terminal guard from #4347 must survive the widening. A Consumed + * row that re-surfaced would be a perpetual-spinner the underlying + * `resume_asset_lock` rejects with "already Consumed — nothing to + * resume". + */ + @Test + fun resumableAddressTopUpsStillExcludeConsumed() = runTest { + insert("consumed:0", statusRaw = 4) + + assertEquals(emptyList(), resumableAddressOutpoints()) + } + + /** Built (0) stays out: nothing has been broadcast yet. */ + @Test + fun resumableAddressTopUpsStillExcludeBuilt() = runTest { + insert("built:0", statusRaw = 0) + + assertEquals(emptyList(), resumableAddressOutpoints()) + } + + /** The funding-type and wallet scoping are unchanged by the widening. */ + @Test + fun resumableAddressTopUpsStayScopedToFundingTypeFourAndWallet() = runTest { + insert("identity:0", statusRaw = 5, fundingTypeRaw = fundingIdentityRegistration) + insert("shielded:0", statusRaw = 5, fundingTypeRaw = fundingShieldedTopUp) + insert("foreign:0", statusRaw = 5, owner = otherWalletId) + insert("mine:0", statusRaw = 5) + + assertEquals(listOf("mine:0"), resumableAddressOutpoints()) + } + + // ── observeResumableTopUpsByFundingType ─────────────────────────── + + /** + * Shielded address top-ups (funding type 5) previously had no resumable + * query at all — the address query is pinned to 4, and the + * identity-recovery surface (`TrackedAssetLock.eligibleFromNative`) + * deliberately admits only funding types 0..2 — so a stalled shielded + * top-up was invisible everywhere. + */ + @Test + fun resumableByFundingTypeCoversShieldedTopUps() = runTest { + insert("shielded-broadcast:0", statusRaw = 1, fundingTypeRaw = fundingShieldedTopUp) + insert("shielded-recovered:0", statusRaw = 5, fundingTypeRaw = fundingShieldedTopUp) + insert("shielded-consumed:0", statusRaw = 4, fundingTypeRaw = fundingShieldedTopUp) + insert("shielded-built:0", statusRaw = 0, fundingTypeRaw = fundingShieldedTopUp) + insert("address-recovered:0", statusRaw = 5, fundingTypeRaw = fundingAddressTopUp) + + val shielded = db.assetLockDao() + .observeResumableTopUpsByFundingType(walletId, fundingShieldedTopUp) + .first() + .map { it.outPointHex } + .sorted() + + assertEquals(listOf("shielded-broadcast:0", "shielded-recovered:0"), shielded) + } + + /** Parameterized with 4, it agrees with the dedicated address query. */ + @Test + fun resumableByFundingTypeAgreesWithTheAddressQuery() = runTest { + insert("a:0", statusRaw = 1) + insert("b:0", statusRaw = 5) + insert("c:0", statusRaw = 4) + insert("d:0", statusRaw = 0) + + val parameterized = db.assetLockDao() + .observeResumableTopUpsByFundingType(walletId, fundingAddressTopUp) + .first() + .map { it.outPointHex } + .sorted() + + assertEquals(resumableAddressOutpoints(), parameterized) + assertEquals(listOf("a:0", "b:0"), parameterized) + } +} diff --git a/packages/rs-platform-wallet-ffi/src/asset_lock/sync.rs b/packages/rs-platform-wallet-ffi/src/asset_lock/sync.rs index 5b840f9d039..a1274f5e22f 100644 --- a/packages/rs-platform-wallet-ffi/src/asset_lock/sync.rs +++ b/packages/rs-platform-wallet-ffi/src/asset_lock/sync.rs @@ -42,6 +42,46 @@ fn parse_outpoint(txid: *const [u8; 32], vout: u32) -> dashcore::OutPoint { /// the already-tracked lock state; signing the consume transition is /// the next stage's responsibility (e.g. /// [`crate::platform_wallet_register_identity_with_funding_signer`]). +/// +/// # Timeouts +/// +/// `timeout_secs` bounds only the stages that still have to WAIT for a +/// proof (`Built` / `Broadcast`, plus the defensive proof-less +/// `RecoveredFromChain` fallback). `InstantSendLocked` / `ChainLocked` +/// already carry a proof and return without ever consulting it. +/// +/// `timeout_secs == 0` does **not** request an unbounded wait — it +/// declines to specify one, and `resume_asset_lock` then applies the +/// recovery policy's own state-dependent default: +/// +/// - An ambiguous `Built` re-broadcast (the broadcaster reports +/// `MaybeSent` for an accepted and a rejected transaction alike), +/// a `Broadcast` row, and the defensive proof-less +/// `RecoveredFromChain` fallback all substitute the 180s +/// `UNCONFIRMED_BROADCAST_PROOF_TIMEOUT` bound (sized to +/// comfortably cover a ~2.5min ChainLock): none of them can +/// establish that the transaction is live on the network, and +/// waiting without a bound on that signal is a `Notify` loop with +/// no terminating event — under the `runtime().block_on(...)` +/// below it pins the calling host thread permanently rather than +/// merely delaying an answer. +/// - The one exception: a `Built` re-broadcast the broadcaster +/// positively ACCEPTED (`Ok`) keeps the unbounded wait — the same +/// positive-evidence wait the initial funding path performs after +/// its own successful broadcast. The proof arrives with the +/// transaction's ChainLock (~2.5min) in normal operation, but the +/// wait is not time-bounded: a caller that needs a hard upper +/// bound on this thread must pass a non-zero `timeout_secs`. +/// +/// Expiry is non-destructive: the tracked row keeps its status, so a +/// proof arriving afterwards is returned by the very next resume +/// straight from the record, without waiting at all. On the `Built` / +/// `Broadcast` arms the expiry surfaces as +/// `TransactionBroadcastUnconfirmed`. +/// +/// A non-zero `timeout_secs` keeps its exact semantics, +/// `FinalityTimeout` included — the substitution above is gated on the +/// caller having declined to choose. #[no_mangle] pub unsafe extern "C" fn asset_lock_manager_resume( handle: Handle, @@ -58,8 +98,13 @@ pub unsafe extern "C" fn asset_lock_manager_resume( check_ptr!(out_derivation_path); let out_point = parse_outpoint(txid, vout); - // `timeout_secs == 0` requests an unbounded wait (a ChainLock is - // guaranteed finality; a broadcast lock is pending, never failed). + // `timeout_secs == 0` declines to specify a bound. `resume_asset_lock` + // reads the resulting `None` as "apply the recovery policy's + // state-dependent default": the 180s + // `UNCONFIRMED_BROADCAST_PROOF_TIMEOUT` on every proof-waiting arm + // except a `Built` re-broadcast the broadcaster positively accepted, + // which keeps the unbounded initial-funding wait. See this + // function's `# Timeouts` section. let timeout = (timeout_secs != 0).then(|| Duration::from_secs(timeout_secs)); let option = ASSET_LOCK_MANAGER_STORAGE.with_item(handle, |manager| { @@ -94,8 +139,34 @@ pub unsafe extern "C" fn asset_lock_manager_resume( /// Returns `ok` on a successful proof resolution, an error on /// timeout / wait failure. The Swift caller is expected to schedule /// this on a background queue — `runtime().block_on(...)` parks the -/// calling thread for up to `timeout_secs` (or **indefinitely** when -/// `timeout_secs == 0`, since a ChainLock is guaranteed finality). +/// calling thread for the duration of the wait. +/// +/// # Timeouts +/// +/// Identical contract to [`asset_lock_manager_resume`], which this +/// delegates to: `timeout_secs == 0` selects the recovery policy's +/// state-dependent default rather than an unbounded wait. That +/// default is the 180s `UNCONFIRMED_BROADCAST_PROOF_TIMEOUT` on every +/// arm that waits for a proof without positive evidence the +/// transaction is on the network — an ambiguous `Built` re-broadcast, +/// a `Broadcast` row, the defensive proof-less `RecoveredFromChain` +/// fallback. The one exception is a `Built` re-broadcast the +/// broadcaster positively accepted (`Ok`): that arm keeps the +/// unbounded initial-funding wait, so the thread is parked until the +/// accepted transaction's proof arrives (its ChainLock, ~2.5min in +/// normal operation) rather than for a fixed bound. Pass a non-zero +/// `timeout_secs` for a hard upper bound. +/// +/// That policy is what makes this entry point safe to fan out at +/// launch. The catch-up sweep starts one call per stuck lock; when +/// zero meant "wait forever" on EVERY waiting arm, a device that was +/// offline (or an SPV session that never connected) turned each of +/// those into a permanently parked worker thread. An unconnected or +/// undeliverable broadcast can only take the bounded arms now (a +/// broadcaster that never dispatched reports `Rejected` / +/// `MaybeSent`, not `Ok`), expiry simply ends the pass, leaving the +/// row tracked and resumable, and the next sweep picks up a proof +/// that landed in between straight from the record. #[no_mangle] pub unsafe extern "C" fn asset_lock_manager_catch_up_blocking( handle: Handle, @@ -106,8 +177,13 @@ pub unsafe extern "C" fn asset_lock_manager_catch_up_blocking( check_ptr!(txid); let out_point = parse_outpoint(txid, vout); - // `timeout_secs == 0` requests an unbounded wait (a ChainLock is - // guaranteed finality; a broadcast lock is pending, never failed). + // `timeout_secs == 0` declines to specify a bound. `resume_asset_lock` + // reads the resulting `None` as "apply the recovery policy's + // state-dependent default": the 180s + // `UNCONFIRMED_BROADCAST_PROOF_TIMEOUT` on every proof-waiting arm + // except a `Built` re-broadcast the broadcaster positively accepted, + // which keeps the unbounded initial-funding wait. See this + // function's `# Timeouts` section. let timeout = (timeout_secs != 0).then(|| Duration::from_secs(timeout_secs)); tracing::info!( diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs index 4f3d415aa4b..f4056957dea 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs @@ -67,6 +67,51 @@ use crate::wallet::asset_lock::manager::AssetLockManager; #[cfg(feature = "shielded")] pub(crate) const CL_FALLBACK_TIMEOUT: Duration = Duration::from_secs(180); +/// Bounded ChainLock wait for the **already-consumed reconciliation** path +/// ([`AssetLockManager::reconcile_asset_lock_submit_result`]). +/// +/// Reconciliation is not a funding flow, so the "a ChainLock always +/// eventually arrives, therefore wait forever" reasoning behind +/// `upgrade_to_chain_lock_proof(None)` does not transfer to it. The +/// operation Platform was asked to perform has already terminated with an +/// unauthenticated `already consumed` report; the ChainLock is wanted only +/// as *evidence to durably record alongside that report*, and the caller is +/// blocked the whole time it is being fetched. +/// +/// Every production call site reaches this through an FFI entry point that +/// drives the future with `runtime().block_on(...)`, so an unbounded wait +/// here does not merely delay a result — it pins the host thread that made +/// the call. The realistic trigger is routine: an IS-locked lock consumed +/// seconds after broadcast reports `already consumed` while its ChainLock +/// is still ~2.5 minutes out, and never arrives at all when the device is +/// offline or SPV is not connected. +/// +/// On expiry the reconciliation still returns the typed +/// [`PlatformWalletError::AssetLockAlreadyConsumed`] — the code-24 signal +/// hosts branch on — having simply failed to attach the chain proof. That +/// matches the pre-#4357 behavior (typed error, no proof retained) while +/// keeping #4357's proof retention whenever the ChainLock is reachable +/// inside the bound. +pub(crate) const RECONCILIATION_CHAIN_LOCK_TIMEOUT: Duration = Duration::from_secs(180); + +/// Bounded proof wait applied after a resume re-broadcast came back +/// `MaybeSent` (see `sync::recovery::resume_asset_lock`). +/// +/// The unbounded `wait_for_proof(None)` used by the funding flows is +/// justified by the transaction being *known* broadcast — finality is then +/// only a matter of time. A `MaybeSent` verdict does not establish that: +/// `DapiBroadcaster` classifies every failure as `MaybeSent`, and the SPV +/// broadcaster reports `Rejected` only for `NotConnected`, so a genuinely +/// rejected transaction is indistinguishable from an accepted one. Waiting +/// without a bound on that signal converts a ~30s broadcast failure into a +/// permanent hang at the `resume_asset_lock(.., None)` call sites. +/// +/// Sized to comfortably cover a ChainLock (~2.5 min) so a transaction that +/// really was accepted still resolves inside the bound; on expiry the +/// caller gets `TransactionBroadcastUnconfirmed`, which is what the +/// pre-#4367 code returned immediately. +pub(crate) const UNCONFIRMED_BROADCAST_PROOF_TIMEOUT: Duration = Duration::from_secs(180); + /// Delay between retries when Platform rejected with CL-height-too-low. /// Each retry bumps `PutSettings::user_fee_increase` so the ST hash /// changes (Tenderdash caches rejected ST hashes for ~24h on @@ -397,6 +442,21 @@ impl AssetLockManager { /// lock to an SPV-backed ChainLock proof, durably retain it as /// consumption-unknown, and preserve the typed host signal. Successful and /// unrelated results pass through unchanged. + /// + /// `chain_lock_timeout` bounds the IS→CL promotion. `None` does **not** + /// mean "wait forever" here: it selects + /// [`RECONCILIATION_CHAIN_LOCK_TIMEOUT`]. Reconciliation always + /// terminates, because the promotion is a best-effort attempt to attach + /// evidence to an operation that has *already* finished, and every + /// production caller reaches it under an FFI `block_on` that would + /// otherwise pin the host thread for as long as the ChainLock is + /// missing (offline / unconnected SPV: forever). + /// + /// Failing to obtain the proof therefore degrades rather than + /// propagates: the lock keeps its current status and the typed + /// [`PlatformWalletError::AssetLockAlreadyConsumed`] is still returned, + /// so the host's code-24 branch is reached either way and the caller + /// may retry to pick the proof up later. pub(crate) async fn reconcile_asset_lock_submit_result( &self, result: Result, @@ -413,18 +473,54 @@ impl AssetLockManager { } let chain_proof = match effective_proof { - AssetLockProof::Chain(_) => effective_proof.clone(), + AssetLockProof::Chain(_) => Some(effective_proof.clone()), AssetLockProof::Instant(_) => { - self.upgrade_to_chain_lock_proof(out_point, chain_lock_timeout) - .await? + let bounded = chain_lock_timeout.or(Some(RECONCILIATION_CHAIN_LOCK_TIMEOUT)); + match self.upgrade_to_chain_lock_proof(out_point, bounded).await { + Ok(proof) => Some(proof), + // Bounded, so this arm is reachable in normal operation + // (an IS-locked lock consumed seconds after broadcast has + // no ChainLock yet). Record nothing, keep the code-24 + // signal, let the caller retry. + // + // Deliberately a catch-all, not `FinalityTimeout`-only. + // The code-24 classification above came from Platform's + // outpoint-matched consensus error, not from this local + // lookup, so a `WalletNotFound` / `AssetLockProofWait` + // (lock untracked after a restore, persister failure, + // record-map mismatch) does not invalidate it — the + // promotion is best-effort evidence-attachment either + // way. Propagating those errors instead would swap the + // host's actionable already-consumed branch for a + // generic local error in exactly the degraded-state + // scenarios where that branch is the only path that can + // still resolve the operation from Platform-side + // evidence. Failures on the RECORDING path below do + // propagate (`mark_asset_lock_consumption_unknown` keeps + // its `?`). + Err(e) => { + tracing::warn!( + outpoint = %out_point, + error = %e, + timeout = ?bounded, + "could not obtain a ChainLock proof for an unauthenticated \ + already-consumed report within the bound; reporting the lock \ + as already consumed without retaining consumption-unknown state" + ); + None + } + } } }; - self.mark_asset_lock_consumption_unknown(out_point, chain_proof) - .await?; - tracing::warn!( - outpoint = %out_point, - "recorded unauthenticated already-consumed report as consumption unknown" - ); + + if let Some(chain_proof) = chain_proof { + self.mark_asset_lock_consumption_unknown(out_point, chain_proof) + .await?; + tracing::warn!( + outpoint = %out_point, + "recorded unauthenticated already-consumed report as consumption unknown" + ); + } Err(PlatformWalletError::AssetLockAlreadyConsumed(*out_point)) } @@ -865,4 +961,310 @@ mod tests { ); } } + + // ----------------------------------------------------------------- + // Reconciliation promotion-failure regressions + // + // Two tests pin the two failure shapes of the IS→CL promotion inside + // `reconcile_asset_lock_submit_result`, one per arm of the deliberate + // catch-all on its `Err` branch: + // - `already_consumed_reconciliation_terminates_without_a_chainlock` + // pins the BOUNDED-WAIT shape (`FinalityTimeout`); + // - `already_consumed_reconciliation_downgrades_non_timeout_promotion_failure` + // pins the DEGRADED-LOCAL-STATE shape (`AssetLockProofWait`). + // Each first asserts the promotion error variant DIRECTLY, so the two + // scenarios cannot silently collapse onto the same path, then asserts + // the shared downgrade outcome. + // ----------------------------------------------------------------- + + use crate::test_support::{ + funded_wallet_manager, AlwaysRejectedBroadcaster, NoopTestPersister, + }; + use crate::wallet::asset_lock::manager::AssetLockManager; + use crate::wallet::asset_lock::tracked::{AssetLockStatus, TrackedAssetLock}; + use crate::wallet::persister::WalletPersister; + use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; + use dpp::consensus::basic::identity::IdentityAssetLockTransactionOutPointAlreadyConsumedError; + use dpp::identity::state_transition::asset_lock_proof::InstantAssetLockProof; + use std::sync::Arc; + + /// Shared fixture: a funded wallet with ONE tracked, IS-locked asset + /// lock whose effective proof is an Instant proof — the shape that + /// routes `reconcile_asset_lock_submit_result` through the + /// `upgrade_to_chain_lock_proof` promotion (a Chain proof would + /// short-circuit past it). + /// + /// The built funding transaction is deliberately NOT registered as a + /// `TransactionRecord` anywhere: `build_asset_lock_transaction` only + /// reserves inputs, nothing is broadcast, and `NoopTestPersister` + /// keeps the persistence trait's `Ok(None)` record lookup. Out of the + /// box the promotion therefore fast-fails with `AssetLockProofWait` + /// ("transaction not found"); a test that wants the bounded-wait + /// `FinalityTimeout` shape instead must register a (non-chain-locked) + /// record for `transaction` first. + struct InstantReconciliationContext { + manager: AssetLockManager, + wallet_manager: + Arc>>, + wallet_id: WalletId, + transaction: dashcore::Transaction, + out_point: OutPoint, + instant_proof: AssetLockProof, + } + + async fn instant_reconciliation_context() -> InstantReconciliationContext { + use dashcore::{InstantLock, Network}; + use key_wallet::account::account_type::StandardAccountType; + use tokio::sync::Notify; + + let (wallet_manager, wallet_id, _generation, signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + let sdk = Arc::new( + dash_sdk::SdkBuilder::new_mock() + .with_network(Network::Testnet) + .build() + .expect("mock sdk"), + ); + let manager = AssetLockManager::new( + sdk, + Arc::clone(&wallet_manager), + wallet_id, + Arc::new(Notify::new()), + Arc::new(AlwaysRejectedBroadcaster), + WalletPersister::new(wallet_id, Arc::new(NoopTestPersister)), + ); + let (transaction, _path) = manager + .build_asset_lock_transaction( + 1_000_000, + 0, + AssetLockFundingType::IdentityRegistration, + 0, + &signer, + ) + .await + .expect("build asset lock"); + let out_point = OutPoint::new(transaction.txid(), 0); + + // An INSTANT proof: this is what selects the `upgrade_to_chain_lock_proof` + // arm. A Chain proof would short-circuit and never exercise the wait. + let instant_proof = AssetLockProof::Instant(InstantAssetLockProof::new( + InstantLock::default(), + transaction.clone(), + 0, + )); + { + let mut wm = wallet_manager.write().await; + wm.get_wallet_info_mut(&wallet_id) + .expect("wallet must remain registered") + .tracked_asset_locks + .insert( + out_point, + TrackedAssetLock { + out_point, + transaction: transaction.clone(), + account_index: 0, + funding_type: AssetLockFundingType::IdentityRegistration, + identity_index: 0, + amount: 1_000_000, + status: AssetLockStatus::InstantSendLocked, + proof: Some(instant_proof.clone()), + }, + ); + } + + InstantReconciliationContext { + manager, + wallet_manager, + wallet_id, + transaction, + out_point, + instant_proof, + } + } + + /// The unauthenticated code-24 consensus response that puts + /// `reconcile_asset_lock_submit_result` on the reconciliation path. + fn already_consumed_error(out_point: OutPoint) -> dash_sdk::Error { + dash_sdk::Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new( + IdentityAssetLockTransactionOutPointAlreadyConsumedError::new( + out_point.txid, + out_point.vout as usize, + ) + .into(), + ))) + } + + /// The downgrade outcome both promotion-failure shapes must share: + /// `reconcile_asset_lock_submit_result` (called with `None`, exactly + /// what every production call site passes) still returns the typed + /// code-24 `AssetLockAlreadyConsumed`, and the tracked row keeps what + /// it had — no ChainLock proof was obtainable, so nothing may claim + /// consumption-unknown state, and a later retry can still pick the + /// proof up. + async fn assert_downgraded_to_already_consumed(ctx: &InstantReconciliationContext) { + let error = ctx + .manager + .reconcile_asset_lock_submit_result::<()>( + Err(already_consumed_error(ctx.out_point)), + &ctx.out_point, + &ctx.instant_proof, + None, + ) + .await + .expect_err("an already-consumed report always ends as a typed error"); + + assert!( + matches!( + error, + PlatformWalletError::AssetLockAlreadyConsumed(actual) if actual == ctx.out_point + ), + "a failed IS→CL promotion must DOWNGRADE to the code-24 signal the hosts \ + branch on — not propagate the promotion's own error, got {error:?}" + ); + + let wm = ctx.wallet_manager.read().await; + let lock = wm + .get_wallet_info(&ctx.wallet_id) + .expect("wallet") + .tracked_asset_locks + .get(&ctx.out_point) + .expect("lock stays tracked"); + assert_eq!( + lock.status, + AssetLockStatus::InstantSendLocked, + "without a chain proof the lock must NOT be promoted to RecoveredFromChain" + ); + assert_eq!( + lock.proof, + Some(ctx.instant_proof.clone()), + "the tracked proof must be untouched by a failed promotion" + ); + } + + /// Regression: the already-consumed reconciliation must TERMINATE when + /// the ChainLock it wants never arrives. + /// + /// Shape: the funding transaction is present and tracked and its record + /// is registered but not in a chain-locked block, the effective proof + /// is an InstantSend proof (so the IS→CL promotion runs and dispatches + /// to `wait_for_chain_lock`), and no SPV chainlock is ever delivered. + /// + /// Before the fix a `None` reconciliation timeout meant "wait forever" + /// and this future never resolved. Under FFI that is a permanently + /// pinned host thread, since every production call site is reached + /// through `runtime().block_on(...)`. The realistic trigger is + /// ordinary: a lock consumed seconds after broadcast is IS-locked but + /// not yet chain-locked (~2.5 min away), and never chain-locked at all + /// when the device is offline. + /// + /// `start_paused` lets the runtime auto-advance the bounded sleep, so + /// the assertion is that the call resolves at all — and resolves as the + /// typed code-24 `AssetLockAlreadyConsumed` the hosts branch on, not as + /// the `FinalityTimeout` of the failed promotion. + #[tokio::test(start_paused = true)] + async fn already_consumed_reconciliation_terminates_without_a_chainlock() { + use key_wallet::account::account_type::StandardAccountType; + use key_wallet::account::AccountType; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::managed_account::transaction_record::{ + TransactionDirection, TransactionRecord, + }; + use key_wallet::transaction_checking::{TransactionContext, TransactionType}; + + let ctx = instant_reconciliation_context().await; + + // Register the funding tx's record (mempool context, NOT chain- + // locked) under BIP44 account 0, so the promotion's record lookup + // succeeds and it genuinely dispatches to `wait_for_chain_lock`. + // Without this the lookup misses and the promotion fast-fails with + // `AssetLockProofWait` before any waiting — the OTHER regression's + // scenario, which must stay distinct from this one. + { + let record = TransactionRecord::new( + ctx.transaction.clone(), + AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP44Account, + }, + TransactionContext::Mempool, + TransactionType::Standard, + TransactionDirection::Outgoing, + Vec::new(), + Vec::new(), + 0, + ); + let mut wm = ctx.wallet_manager.write().await; + wm.get_wallet_info_mut(&ctx.wallet_id) + .expect("wallet must remain registered") + .core_wallet + .accounts + .standard_bip44_accounts + .get_mut(&0) + .expect("funded fixture has BIP44 account 0") + .transactions_mut() + .insert(ctx.out_point.txid, record); + } + + // Pin the SCENARIO, not just the outcome: the promotion itself must + // burn the bound and report `FinalityTimeout` (auto-advanced under + // `start_paused`), proving this test exercises the timeout arm of + // the reconciliation catch-all and not the fast-fail one. + let promotion_err = ctx + .manager + .upgrade_to_chain_lock_proof(&ctx.out_point, Some(RECONCILIATION_CHAIN_LOCK_TIMEOUT)) + .await + .expect_err("no ChainLock ever arrives: the promotion must fail"); + assert!( + matches!( + promotion_err, + PlatformWalletError::FinalityTimeout(actual) if actual == ctx.out_point + ), + "expected the promotion to time out waiting for a ChainLock, got {promotion_err:?}" + ); + + assert_downgraded_to_already_consumed(&ctx).await; + } + + /// Companion regression pinning the DELIBERATE breadth of the + /// promotion's `Err` catch-all in `reconcile_asset_lock_submit_result` + /// (see the comment on that arm): a NON-timeout promotion failure must + /// be downgraded to the code-24 signal exactly like a timeout, because + /// the already-consumed classification came from Platform's + /// outpoint-matched consensus error — a failed local lookup does not + /// invalidate it, and the non-timeout failures occur precisely in the + /// degraded-local-state scenarios where the host's code-24 branch is + /// the only path that can still resolve the operation. + /// + /// Shape: the lock is tracked, but its transaction record is + /// unavailable — never registered in any account's in-memory map (the + /// fixture never broadcasts) and unknown to the persister + /// (`NoopTestPersister` keeps the trait's `Ok(None)` default). That is + /// the post-restore / wallet-state-mismatch shape, and the promotion + /// fast-fails with `AssetLockProofWait` instead of waiting. + /// + /// "Fixing" the catch-all to propagate everything but + /// `FinalityTimeout` turns the reconcile result below into + /// `AssetLockProofWait` and fails this test — that narrowing was + /// proposed and declined in review (finding 9237664c50df); this test + /// keeps the decision from silently regressing. + #[tokio::test(start_paused = true)] + async fn already_consumed_reconciliation_downgrades_non_timeout_promotion_failure() { + let ctx = instant_reconciliation_context().await; + + // Pin the SCENARIO first: with the record unavailable, the + // promotion must fail with the NON-timeout `AssetLockProofWait` + // fast-fail. If a future fixture change made the record findable, + // this assertion — not a silently green downgrade check — fails. + let promotion_err = ctx + .manager + .upgrade_to_chain_lock_proof(&ctx.out_point, Some(RECONCILIATION_CHAIN_LOCK_TIMEOUT)) + .await + .expect_err("record unavailable: the promotion must fail"); + assert!( + matches!(promotion_err, PlatformWalletError::AssetLockProofWait(_)), + "expected the non-timeout AssetLockProofWait fast-fail, got {promotion_err:?}" + ); + + assert_downgraded_to_already_consumed(&ctx).await; + } } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs index fb3852dcef1..736e0516b4d 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs @@ -17,6 +17,7 @@ use crate::changeset::changeset::AssetLockChangeSet; use crate::error::PlatformWalletError; use super::super::manager::AssetLockManager; +use super::super::orchestration::UNCONFIRMED_BROADCAST_PROOF_TIMEOUT; use super::super::tracked::{AssetLockStatus, TrackedAssetLock}; // --------------------------------------------------------------------------- @@ -208,9 +209,22 @@ impl AssetLockManager { /// build phase when the credit output is later consumed on Platform. /// /// `timeout` is `Option` and is only consulted when the lock - /// still needs a proof (`Built` / `Broadcast`): `None` waits - /// **indefinitely** for finality. For `InstantSendLocked` / `ChainLocked` - /// the proof already exists and no wait happens, so the value is moot. + /// still needs a proof (`Built` / `Broadcast`, or the defensive + /// proof-less `RecoveredFromChain` fallback). For `InstantSendLocked` / + /// `ChainLocked` the proof already exists and no wait happens, so the + /// value is moot. + /// + /// `None` requests an unbounded wait, and gets one **only** where this + /// call obtained positive evidence the transaction is on the network: + /// the `Built` arm whose re-broadcast returned `Ok`. Every other + /// proof-waiting path substitutes + /// [`UNCONFIRMED_BROADCAST_PROOF_TIMEOUT`], because the alternative is a + /// `Notify` loop that never terminates under the FFI's + /// `runtime().block_on(...)` — a permanently pinned host thread rather + /// than a late answer. Expiry leaves the tracked row untouched, so the + /// next resume picks up a proof that arrives later straight from the + /// record; on the `Broadcast` arm it is reported as + /// [`PlatformWalletError::TransactionBroadcastUnconfirmed`]. pub async fn resume_asset_lock( &self, out_point: &OutPoint, @@ -264,24 +278,69 @@ impl AssetLockManager { // completed. Advancing to `Broadcast` and waiting matches // what the `Broadcast` arm below already does with the // identical signal. - match self.broadcaster.broadcast(&tx).await { - Ok(_) => {} + // + // `MaybeSent` is however ALSO what the broadcaster reports + // for a genuinely rejected transaction: `DapiBroadcaster` + // classifies every failure that way by construction, and the + // SPV broadcaster only reaches `Rejected` on `NotConnected`. + // So the advance above cannot be read as evidence the tx is + // live, and the proof wait that follows it must not be the + // unbounded one — at the `resume_asset_lock(.., None)` + // production call sites that would turn a prompt broadcast + // failure into a permanent hang. Bound it, and translate the + // expiry back into the `TransactionBroadcastUnconfirmed` the + // caller used to get immediately. + let maybe_sent_reason = match self.broadcaster.broadcast(&tx).await { + Ok(_) => None, Err(BroadcastError::MaybeSent { reason }) => { tracing::warn!( outpoint = %out_point, reason = %reason, "resume_asset_lock: re-broadcast of a Built lock returned an \ - unknown outcome (the network may already hold this tx); \ - advancing to Broadcast and waiting for proof" + unknown outcome (the network may already hold this tx, or may \ + have rejected it — the broadcaster cannot tell); advancing to \ + Broadcast and waiting for proof under a bounded timeout" ); + Some(reason) } Err(rejected) => return Err(rejected.into()), - } + }; let cs = self .advance_asset_lock_status(out_point, AssetLockStatus::Broadcast, None) .await?; self.queue_asset_lock_changeset(cs); - let proof = self.wait_for_proof(out_point, timeout).await?; + let proof = match (&maybe_sent_reason, timeout) { + // Ambiguous re-broadcast AND an unbounded wait: the only + // combination that can hang forever. Substitute the bound + // and translate its expiry back into the broadcast error + // the caller used to get immediately. + // + // Callers that passed their own timeout are left exactly + // as they were, `FinalityTimeout` and all — the shielded + // seed pool treats that error as a pacing signal and + // resumes the lock later, so re-typing it would break a + // working flow to fix an unrelated one. + (Some(reason), None) => { + match self + .wait_for_proof(out_point, Some(UNCONFIRMED_BROADCAST_PROOF_TIMEOUT)) + .await + { + Ok(proof) => proof, + Err(PlatformWalletError::FinalityTimeout(_)) => { + return Err(PlatformWalletError::TransactionBroadcastUnconfirmed( + format!( + "asset lock {} was re-broadcast with an unknown \ + outcome and no InstantSend/ChainLock proof arrived \ + within {:?}: {}", + out_point, UNCONFIRMED_BROADCAST_PROOF_TIMEOUT, reason + ), + )) + } + Err(e) => return Err(e), + } + } + _ => self.wait_for_proof(out_point, timeout).await?, + }; self.validate_or_upgrade_proof(proof, account_index, out_point) .await? } @@ -292,31 +351,149 @@ impl AssetLockManager { // default `-mempoolexpiry` is two weeks), or the original // broadcast may have reached no peers at all (SPV // connectivity gap). Once no node holds the tx, no IS/CL - // proof can ever arrive, and the wait — now unbounded for - // the user-facing funding flows — would hang forever. A - // re-broadcast revives an evicted/undelivered tx. + // proof can ever arrive and the wait below can only run out + // its bound. A re-broadcast revives an evicted/undelivered + // tx, so it is worth attempting before every wait. // - // Best-effort: unlike the `Built` arm, this tx was already - // broadcast once (that's what `Broadcast` means), so it may - // still be in a mempool or already mined — in which case the - // network reports "already known" / "already in block - // chain". The broadcaster can't distinguish that from a real - // rejection (DAPI classifies every failure as `MaybeSent`), - // so we log and proceed to `wait_for_proof` regardless + // Best-effort for the AMBIGUOUS verdict only: unlike the + // `Built` arm, this tx was already broadcast once (that's + // what `Broadcast` means), so it may still be in a mempool or + // already mined — in which case the network reports "already + // known" / "already in block chain", which the broadcaster + // cannot distinguish from a real rejection and reports as + // `MaybeSent`. We log that and proceed to `wait_for_proof` // rather than failing the resume on a tx that is actually - // fine. If the tx really was mined, `wait_for_proof` - // resolves immediately from the SPV/persisted record. + // fine. If the tx really was mined, `wait_for_proof` resolves + // immediately from the SPV/persisted record. + // + // A DEFINITE `Rejected` ends the resume early — but it says + // NOTHING about the row, and must not be read as one. In + // fact the row's RECORD may already hold the answer: a lock + // can sit at `Broadcast` while its transaction record + // carries an IS lock or a chain-locked context, because + // finality that arrives with no waiter active enriches the + // record without advancing the tracked status + // (`LockNotifyHandler` only wakes waiters, and + // `enrich_from_record` upgrades only chain-locked records + // on scan paths). So before surfacing the rejection, probe + // the record once, without waiting — `wait_for_proof` with + // a zero bound performs exactly one local record/persister + // check and expires before touching the network. On the + // canonical trigger (`catchUpStuckAssetLocks` resuming + // rows at launch before SPV connects) that probe is the + // difference between completing an already-final lock + // entirely offline and failing it every launch until + // connectivity returns. + // + // `Rejected` is scoped to the attempt that produced it. With + // the production `SpvBroadcaster` it is reachable from + // exactly two places, an unstarted client and dash-spv's + // zero-connected-peers check (`spv/runtime.rs`), so it means + // "*this* send never left the device" — not "the transaction + // is not on the network". The ORIGINAL broadcast that put + // this row at `Broadcast` happened in an earlier process, + // possibly days ago, and its outcome is untouched by a + // re-broadcast that never dispatched. + // + // So there is no untrack here. `catchUpStuckAssetLocks` runs + // on every wallet load, selects `statusRaw < 2` (which + // includes `Broadcast` = 1) and has no SPV-connected gate, so + // dropping the row on this verdict deleted tracking for a + // possibly-mined asset lock on an ordinary offline relaunch — + // and reconstruction only re-inserts on a FRESH detection + // event, which an already-recorded mined transaction never + // produces again. The row stays exactly as it was; a later + // resume, once SPV is connected, re-broadcasts and resolves + // it normally. + // + // Nor is there a state on this path where non-dispatch of the + // original send IS provable: a row can sit at `Built` after a + // successful broadcast too (app killed between the send and + // the status advance), which is precisely why the `Built` arm + // above also only surfaces the error and leaves its row alone. + let mut local_proof = None; if let Err(e) = self.broadcaster.broadcast(&tx).await { - tracing::debug!( - outpoint = %out_point, - error = %e, - "resume_asset_lock: defensive re-broadcast of a \ - Broadcast-status lock returned an error (likely \ - already in a mempool or mined); proceeding to wait \ - for proof" - ); + if matches!(e, BroadcastError::Rejected { .. }) { + match self.wait_for_proof(out_point, Some(Duration::ZERO)).await { + Ok(proof) => { + tracing::info!( + outpoint = %out_point, + error = %e, + "resume_asset_lock: defensive re-broadcast of a \ + Broadcast-status lock was rejected, but the \ + local record already holds finality — \ + completing the resume from the local proof" + ); + local_proof = Some(proof); + } + Err(probe_err) => { + tracing::warn!( + outpoint = %out_point, + error = %e, + probe = %probe_err, + "resume_asset_lock: defensive re-broadcast of a \ + Broadcast-status lock was definitively rejected \ + and no local proof exists — this attempt never \ + left the device, which proves nothing about the \ + original broadcast; leaving the row tracked at \ + Broadcast and failing the resume" + ); + return Err(e.into()); + } + } + } else { + tracing::debug!( + outpoint = %out_point, + error = %e, + "resume_asset_lock: defensive re-broadcast of a \ + Broadcast-status lock returned an unknown outcome (likely \ + already in a mempool or mined); proceeding to wait \ + for proof" + ); + } } - let proof = self.wait_for_proof(out_point, timeout).await?; + // Bounded like the `Built` arm, and for the same reason. This + // arm is only entered on a RESUME, i.e. for a transaction + // whose earlier broadcast window already failed to produce a + // proof — so "finality is only a matter of time", the premise + // that justifies the unbounded `wait_for_proof(None)` on the + // initial funding path, does not hold here. The `None` this + // receives from `resume_asset_lock(.., None)` (and from the + // FFI's `timeout_secs == 0`) drove an unbounded `Notify` loop + // under `runtime().block_on(...)`, which pins the calling host + // thread permanently rather than merely delaying a result. + // + // The bound costs nothing: the row is left at `Broadcast`, so + // a proof that lands after the expiry is picked up by the very + // next resume — `wait_for_proof` returns it on its first + // iteration, straight from the record, without waiting at all. + // + // Callers that supplied their own timeout keep their exact + // semantics, `FinalityTimeout` and all: `or` is the identity + // on `Some`, and the re-typing below is gated on the caller + // having asked for an unbounded wait. The shielded seed pool + // reads `FinalityTimeout` as a pacing signal, so re-typing it + // for everyone would break a working flow to fix another. + let proof = if let Some(proof) = local_proof { + proof + } else { + let bounded = timeout.or(Some(UNCONFIRMED_BROADCAST_PROOF_TIMEOUT)); + match self.wait_for_proof(out_point, bounded).await { + Ok(proof) => proof, + Err(PlatformWalletError::FinalityTimeout(_)) if timeout.is_none() => { + let reason = format!( + "asset lock {} is tracked as broadcast but no \ + InstantSend/ChainLock proof arrived within {:?}; the \ + lock remains tracked and resumable", + out_point, UNCONFIRMED_BROADCAST_PROOF_TIMEOUT + ); + return Err(PlatformWalletError::TransactionBroadcastUnconfirmed( + reason, + )); + } + Err(e) => return Err(e), + } + }; self.validate_or_upgrade_proof(proof, account_index, out_point) .await? } @@ -349,13 +526,31 @@ impl AssetLockManager { // was lost, and its wait resolves from the already // chain-locked record rather than blocking on new // network events. + // + // That last part is a "by construction" argument, and it + // holds only while the chain-locked record is still + // reachable — the same lost-state accident that produced a + // proof-less `RecoveredFromChain` row could equally have + // taken the record with it, and then the wait has nothing + // to resolve from and blocks forever on `Notify`. The bound + // is free where the argument holds (`wait_for_proof` returns + // from the record on its first iteration, before any deadline + // is consulted) and closes the case where it doesn't, so it + // is applied for uniformity with the two arms above. No + // re-typing here: nothing was broadcast on this path, so + // `FinalityTimeout` is the honest verdict. match existing_proof { Some(proof) => { self.validate_or_upgrade_proof(proof, account_index, out_point) .await? } None => { - let proof = self.wait_for_proof(out_point, timeout).await?; + let proof = self + .wait_for_proof( + out_point, + timeout.or(Some(UNCONFIRMED_BROADCAST_PROOF_TIMEOUT)), + ) + .await?; self.validate_or_upgrade_proof(proof, account_index, out_point) .await? } @@ -969,4 +1164,368 @@ mod tests { "re-derived credit-output path must match the build-time path" ); } + + /// Builds a tracked lock at `status` on a funded wallet and resumes it + /// through `broadcaster` with the given `timeout`, returning the resume + /// error and the lock's tracked state afterwards (`None` = untracked). + async fn resume_lock_at( + broadcaster: Arc, + status: AssetLockStatus, + timeout: Option, + ) -> (PlatformWalletError, Option) { + let (wallet_manager, wallet_id, _balance, signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + let sdk = Arc::new( + dash_sdk::SdkBuilder::new_mock() + .with_network(Network::Testnet) + .build() + .expect("mock sdk"), + ); + let manager = AssetLockManager::new( + sdk, + Arc::clone(&wallet_manager), + wallet_id, + Arc::new(Notify::new()), + broadcaster, + WalletPersister::new(wallet_id, Arc::new(RecordingPersistence::default())), + ); + let (transaction, _path) = manager + .build_asset_lock_transaction( + 1_000_000, + 0, + AssetLockFundingType::AssetLockAddressTopUp, + 4, + &signer, + ) + .await + .expect("build asset lock"); + let out_point = OutPoint::new(transaction.txid(), 0); + { + let mut wm = wallet_manager.write().await; + wm.get_wallet_info_mut(&wallet_id) + .expect("wallet must remain registered") + .tracked_asset_locks + .insert( + out_point, + TrackedAssetLock { + out_point, + transaction, + account_index: 0, + funding_type: AssetLockFundingType::AssetLockAddressTopUp, + identity_index: 4, + amount: 1_000_000, + status, + proof: None, + }, + ); + } + + let error = manager + .resume_asset_lock(&out_point, timeout) + .await + .expect_err("no proof event is ever delivered in these cases"); + let tracked = wallet_manager + .read() + .await + .get_wallet_info(&wallet_id) + .expect("wallet") + .tracked_asset_locks + .get(&out_point) + .map(|lock| lock.status.clone()); + (error, tracked) + } + + /// Regression: an ambiguous re-broadcast on the UNBOUNDED resume path + /// must not hang. + /// + /// `MaybeSent` is the broadcaster's verdict for a genuinely rejected + /// transaction as much as for an accepted one — `DapiBroadcaster` + /// classifies every failure that way, and the SPV broadcaster reaches + /// `Rejected` only on `NotConnected`. So advancing to `Broadcast` and + /// then waiting with `wait_for_proof(None)` — which is what the three + /// `resume_asset_lock(.., None)` production call sites do — turned a + /// broadcast failure that used to surface in ~30s into a wait that never + /// ends, because no proof can arrive for a tx that was never accepted. + /// + /// `start_paused` auto-advances the substituted bound, so this asserts + /// termination *and* that the caller gets the pre-#4367 typed error back. + #[tokio::test(start_paused = true)] + async fn unbounded_resume_of_an_ambiguous_rebroadcast_terminates() { + let (error, status) = resume_lock_at( + Arc::new(AlwaysMaybeSentBroadcaster), + AssetLockStatus::Built, + None, + ) + .await; + + assert!( + matches!( + error, + PlatformWalletError::TransactionBroadcastUnconfirmed(_) + ), + "an unbounded resume whose re-broadcast was ambiguous must end as a \ + broadcast-unconfirmed failure rather than hang, got {error:?}" + ); + assert_eq!( + status, + Some(AssetLockStatus::Broadcast), + "the advance itself is still correct — the row stays resumable so a \ + later pass can pick up a proof that does eventually arrive" + ); + } + + /// The bounded callers must be untouched by the fix above. The shielded + /// seed pool passes its own timeout and treats `FinalityTimeout` as a + /// pacing signal (pause, resume the lock later), so re-typing the error + /// for every caller would have broken a working flow to fix a different + /// one. + #[tokio::test] + async fn bounded_resume_of_an_ambiguous_rebroadcast_still_reports_finality_timeout() { + let (error, status) = resume_lock_at( + Arc::new(AlwaysMaybeSentBroadcaster), + AssetLockStatus::Built, + Some(Duration::from_millis(10)), + ) + .await; + + assert!( + matches!(error, PlatformWalletError::FinalityTimeout(_)), + "a caller-supplied timeout must keep its FinalityTimeout semantics: {error:?}" + ); + assert_eq!(status, Some(AssetLockStatus::Broadcast)); + } + + /// A DEFINITE rejection on the `Broadcast` arm is surfaced rather than + /// swallowed — and the row SURVIVES it. + /// + /// The first revision of this fix untracked the row here, reasoning that + /// `Rejected` proves the transaction never reached the network. It does + /// not: with the production `SpvBroadcaster` that verdict means an + /// unstarted client or zero connected peers, which is a fact about the + /// re-broadcast attempt, not about the ORIGINAL broadcast that put the + /// row at `Broadcast` in an earlier process. `catchUpStuckAssetLocks` + /// resumes every `statusRaw < 2` row on each wallet load with no + /// SPV-connected gate, so that untrack deleted tracking for + /// possibly-mined asset locks on ordinary offline relaunches, with no + /// path back (reconstruction re-inserts only on a fresh detection event). + #[tokio::test] + async fn definite_rejection_on_a_broadcast_lock_surfaces_without_untracking() { + let (error, tracked) = resume_lock_at( + Arc::new(AlwaysRejectedBroadcaster), + AssetLockStatus::Broadcast, + None, + ) + .await; + + assert!( + matches!(error, PlatformWalletError::TransactionBroadcast(_)), + "a definite rejection must fail the resume instead of falling through \ + to a proof wait, got {error:?}" + ); + assert_eq!( + tracked, + Some(AssetLockStatus::Broadcast), + "a re-broadcast that never left the device says nothing about the \ + original send — the row must survive, unchanged, for a later resume" + ); + } + + /// A definite rejection must consult the LOCAL record before failing + /// the resume. + /// + /// A row can sit at `Broadcast` while its transaction record already + /// carries finality: `LockNotifyHandler` only wakes waiters, so an + /// IS/CL event that arrives with no waiter active enriches the record + /// but never advances the tracked status, and `enrich_from_record` + /// upgrades only `InChainLockedBlock` records on scan paths — an + /// `InstantSend` context is invisible to it. On the next launch + /// `catchUpStuckAssetLocks` resumes the row before SPV connects, the + /// defensive re-broadcast draws `Rejected` (unstarted client / zero + /// peers), and the pre-fix arm failed the resume even though + /// `wait_for_proof` would have returned the proof on its first + /// iteration, straight from the record, without any network at all. + #[tokio::test] + async fn definite_rejection_on_a_broadcast_lock_yields_the_local_proof() { + use dashcore::ephemerealdata::instant_lock::InstantLock; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::managed_account::transaction_record::{ + TransactionDirection, TransactionRecord, + }; + use key_wallet::transaction_checking::{TransactionContext, TransactionType}; + + let (wallet_manager, wallet_id, _balance, signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + let sdk = Arc::new( + dash_sdk::SdkBuilder::new_mock() + .with_network(Network::Testnet) + .build() + .expect("mock sdk"), + ); + let manager = AssetLockManager::new( + sdk, + Arc::clone(&wallet_manager), + wallet_id, + Arc::new(Notify::new()), + Arc::new(AlwaysRejectedBroadcaster), + WalletPersister::new(wallet_id, Arc::new(RecordingPersistence::default())), + ); + let (transaction, _path) = manager + .build_asset_lock_transaction( + 1_000_000, + 0, + AssetLockFundingType::AssetLockAddressTopUp, + 4, + &signer, + ) + .await + .expect("build asset lock"); + let out_point = OutPoint::new(transaction.txid(), 0); + { + let mut wm = wallet_manager.write().await; + let info = wm + .get_wallet_info_mut(&wallet_id) + .expect("wallet must remain registered"); + // The finality that arrived while nobody was waiting: an + // IS-locked record for the funding tx, filed under the BIP44 + // account the lock was built from. + let record = TransactionRecord::new( + transaction.clone(), + AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP44Account, + }, + TransactionContext::InstantSend(InstantLock::default()), + TransactionType::Standard, + TransactionDirection::Outgoing, + Vec::new(), + Vec::new(), + 0, + ); + info.core_wallet + .accounts + .standard_bip44_accounts + .get_mut(&0) + .expect("funded wallet has BIP44 account 0") + .transactions_mut() + .insert(record.txid, record); + info.tracked_asset_locks.insert( + out_point, + TrackedAssetLock { + out_point, + transaction, + account_index: 0, + funding_type: AssetLockFundingType::AssetLockAddressTopUp, + identity_index: 4, + amount: 1_000_000, + status: AssetLockStatus::Broadcast, + proof: None, + }, + ); + } + + let (proof, _path) = manager + .resume_asset_lock(&out_point, None) + .await + .expect("a locally-proven lock must survive a rejected re-broadcast"); + assert!( + matches!(proof, dpp::prelude::AssetLockProof::Instant(_)), + "the proof must come from the record's InstantSend context: {proof:?}" + ); + assert_eq!( + wallet_manager + .read() + .await + .get_wallet_info(&wallet_id) + .expect("wallet") + .tracked_asset_locks + .get(&out_point) + .expect("lock stays tracked") + .status, + AssetLockStatus::InstantSendLocked, + "the resume must advance the row exactly as a waited-for proof would" + ); + } + + /// Regression: the `Broadcast` arm's proof wait must terminate on the + /// UNBOUNDED resume path too. + /// + /// The first revision of this fix bounded only the `Built` arm. Its own + /// retained behavior — advance an ambiguous `Built` lock to `Broadcast` + /// and leave the row there — routes exactly that lock into this arm on + /// the next resume pass, where a bare `wait_for_proof(None)` waits on + /// `Notify` forever. The hang was deferred by one pass, not removed, and + /// under the FFI's `runtime().block_on(...)` it pins a host thread. + /// + /// `start_paused` auto-advances the substituted bound, so this asserts + /// termination *and* the typed error the caller gets on expiry. + #[tokio::test(start_paused = true)] + async fn unbounded_resume_of_a_broadcast_row_terminates() { + let (error, status) = resume_lock_at( + Arc::new(AlwaysMaybeSentBroadcaster), + AssetLockStatus::Broadcast, + None, + ) + .await; + + assert!( + matches!( + error, + PlatformWalletError::TransactionBroadcastUnconfirmed(_) + ), + "an unbounded resume of a Broadcast row must end as a \ + broadcast-unconfirmed failure rather than hang, got {error:?}" + ); + assert_eq!( + status, + Some(AssetLockStatus::Broadcast), + "the row must stay exactly where it was so a proof arriving after the \ + bound is picked up by the next resume" + ); + } + + /// The bounded callers of the `Broadcast` arm keep their semantics, the + /// same way the `Built` arm's do: `or` is the identity on `Some`, and the + /// re-typing is gated on the caller having asked for an unbounded wait. + #[tokio::test] + async fn bounded_resume_of_a_broadcast_row_still_reports_finality_timeout() { + let (error, status) = resume_lock_at( + Arc::new(AlwaysMaybeSentBroadcaster), + AssetLockStatus::Broadcast, + Some(Duration::from_millis(10)), + ) + .await; + + assert!( + matches!(error, PlatformWalletError::FinalityTimeout(_)), + "a caller-supplied timeout must keep its FinalityTimeout semantics: {error:?}" + ); + assert_eq!(status, Some(AssetLockStatus::Broadcast)); + } + + /// The `RecoveredFromChain` proof-less fallback is bounded for the same + /// reason, even though its wait resolves immediately whenever the + /// chain-locked record it reads is still present. The accident that + /// leaves a `RecoveredFromChain` row without its persisted proof can take + /// the record too, and then the "resolves immediately by construction" + /// argument yields an unbounded `Notify` loop. No re-typing: nothing is + /// broadcast on this arm, so `FinalityTimeout` is the honest verdict. + #[tokio::test(start_paused = true)] + async fn unbounded_resume_of_a_proofless_recovered_row_terminates() { + let (error, status) = resume_lock_at( + Arc::new(AlwaysRejectedBroadcaster), + AssetLockStatus::RecoveredFromChain, + None, + ) + .await; + + assert!( + matches!(error, PlatformWalletError::FinalityTimeout(_)), + "a proof-less RecoveredFromChain resume must terminate, got {error:?}" + ); + assert_eq!( + status, + Some(AssetLockStatus::RecoveredFromChain), + "the row keeps its status — the resume proved nothing new about it" + ); + } } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs index 3fa36441b64..9a08a5930d9 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs @@ -85,6 +85,16 @@ impl AssetLockManager { cs } + // NOTE: there is deliberately no `untrack_unproven_broadcast_asset_lock` + // companion here. A `Rejected` verdict from a re-broadcast describes only + // that attempt (with the production `SpvBroadcaster`: an unstarted client + // or zero connected peers), never the ORIGINAL broadcast that moved the + // row to `Broadcast` in an earlier process — so it is not evidence that + // the transaction is absent from the network, and removing the row on it + // deleted tracking for possibly-mined asset locks during ordinary offline + // relaunches. `resume_asset_lock` now surfaces the typed error and leaves + // the row untouched. + /// Mark a tracked asset lock as /// [`Consumed`](AssetLockStatus::Consumed) after a successful /// identity registration or top-up. diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/IdentitiesContentView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/IdentitiesContentView.swift index 794331c1cf6..a5bca7088c5 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/IdentitiesContentView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/IdentitiesContentView.swift @@ -426,17 +426,25 @@ struct IdentitiesContentView: View { /// out: a tight crash window between TX build and broadcast /// with no useful UX action to take. /// - /// The upper bound (`<= 3`, ChainLocked) excludes `statusRaw == 4` - /// (Consumed) — the terminal state set when a lock has already - /// funded an identity. In the happy path the anti-join against - /// `PersistentIdentity` would hide a Consumed lock anyway - /// (every successful registration writes both rows at the same - /// slot), but the local-only delete-identity action removes - /// the identity row WITHOUT the asset-lock row, which frees the - /// slot in the anti-join. Without this upper bound a Consumed - /// lock would re-surface as a Resume row that can't advance — - /// `resume_asset_lock` rejects Consumed entries with - /// "already Consumed — nothing to resume". + /// `statusRaw == 4` (Consumed) is excluded — the terminal state + /// set when a lock has already funded an identity. In the happy + /// path the anti-join against `PersistentIdentity` would hide a + /// Consumed lock anyway (every successful registration writes + /// both rows at the same slot), but the local-only + /// delete-identity action removes the identity row WITHOUT the + /// asset-lock row, which frees the slot in the anti-join. + /// Without excluding it, a Consumed lock would re-surface as a + /// Resume row that can't advance — `resume_asset_lock` rejects + /// Consumed entries with "already Consumed — nothing to resume". + /// + /// `statusRaw == 5` (RecoveredFromChain) IS admitted. It is not a + /// state beyond Consumed despite the higher discriminant: it is + /// what the restore scan / chainlock-promotion path writes for a + /// lock with proven Core finality and unknown Platform-side + /// consumption, and a user-driven Resume is precisely the surface + /// allowed to try consuming one. Expressing the predicate as the + /// contiguous range `1...3` hid every recovered lock, so an + /// identity funded before a restore had no resume row at all. /// /// Generic over `AssetLockResumeRow` so the pure filter is /// unit-testable without a SwiftData container. @@ -446,7 +454,7 @@ struct IdentitiesContentView: View { ) -> [R] { locks.filter { lock in guard lock.fundingTypeRaw >= 0 && lock.fundingTypeRaw <= 2 else { return false } - guard lock.statusRaw >= 1 && lock.statusRaw <= 3 else { return false } + guard lock.isVisibleAsResumable else { return false } let slot = UInt32(bitPattern: lock.identityIndexRaw) return !usedSlots.contains( UsedSlot(walletId: lock.walletId, slot: slot) diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift index cd9affaee51..e5145830765 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift @@ -278,7 +278,20 @@ struct WalletDetailView: View { WithdrawPlatformAddressView(wallet: wallet) } .sheet(item: $resumingAssetLock) { lock in - FundFromAssetLockPlatformAddressView(wallet: wallet, resumeFromLock: lock) + // Route by funding type. Both top-up types reach this sheet from + // `PendingPlatformFundFromAssetLocksList`, and they consume their + // locks through DIFFERENT transitions: type 4 resumes via + // `resumeFundFromAssetLock` (credit a Platform address), type 5 + // via `shieldedResumeFundFromAssetLock` (Type 18 shield into the + // Orchard pool). Sending a shielded lock to the address view + // would submit the wrong transition against it, so surfacing the + // row without this branch would only move the dead end one tap + // later. + if lock.fundingTypeRaw == 5 { + ShieldedFundFromAssetLockView(wallet: wallet, resumeFromLock: lock) + } else { + FundFromAssetLockPlatformAddressView(wallet: wallet, resumeFromLock: lock) + } } .sheet(isPresented: $showShieldFromAssetLock) { ShieldedFundFromAssetLockView(wallet: wallet) diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/ShieldedFundFromAssetLockController.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/ShieldedFundFromAssetLockController.swift index d4e38dc3db5..e0558f0a28e 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/ShieldedFundFromAssetLockController.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/ShieldedFundFromAssetLockController.swift @@ -79,6 +79,15 @@ final class ShieldedFundFromAssetLockController: ObservableObject { /// purge ~30s after the success transition). private(set) var lastSubmittedAt: Date? + /// Identity of the operation currently occupying this slot — the + /// fresh-shield marker or the resumed lock's outpoint. Set by every + /// accepted `submit`. The coordinator compares it to tell a re-tap + /// of the SAME operation (reuse the controller) from a DIFFERENT + /// operation on the same `(walletId, recipient)` slot, whose body + /// `submit` would otherwise silently drop (two resumable locks + /// normally share the wallet's default shielded recipient). + private(set) var operationId: String? + /// Active funding task. Holds a reference so the coordinator's /// stash retains the work until completion; cancellation isn't /// wired today (the FFI call doesn't yet support clean abort). @@ -100,19 +109,21 @@ final class ShieldedFundFromAssetLockController: ObservableObject { /// the legitimate-restart flow through them (a user retries a /// failure via `failed → submit`). /// - /// `body` performs the actual FFI call. It runs detached on a - /// background priority. Unlike the address-funding sibling, the - /// FFI returns `Void` — the shielded note arrives via the next - /// sync, not from the broadcast call — so the controller flips - /// `phase` to `.completed` (no balance payload) / `.failed` + /// `operationId` names the operation the body performs (see + /// `operationId`). `body` performs the actual FFI call. It runs + /// detached on a background priority. Unlike the address-funding + /// sibling, the FFI returns `Void` — the shielded note arrives via + /// the next sync, not from the broadcast call — so the controller + /// flips `phase` to `.completed` (no balance payload) / `.failed` /// accordingly. - func submit(body: @escaping () async throws -> Void) { + func submit(operationId: String, body: @escaping () async throws -> Void) { switch phase { case .idle, .failed: break case .inFlight, .completed: return } + self.operationId = operationId phase = .inFlight lastSubmittedAt = Date() task = Task { [weak self] in diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/ShieldedFundFromAssetLockCoordinator.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/ShieldedFundFromAssetLockCoordinator.swift index a8ad016b24c..02fe09d2c7c 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/ShieldedFundFromAssetLockCoordinator.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/ShieldedFundFromAssetLockCoordinator.swift @@ -86,35 +86,62 @@ final class ShieldedFundFromAssetLockCoordinator: ObservableObject { } /// Start a funding for the slot, or reuse an existing controller - /// if one is already in flight on the same recipient. Returns a - /// `StartFundingResult` so the caller can distinguish a fresh - /// start from a wallet-level conflict (see the type doc for - /// rationale). + /// if the SAME operation is already in flight on the same + /// recipient. Returns a `StartFundingResult` so the caller can + /// distinguish a fresh start from a wallet-level conflict (see + /// the type doc for rationale). /// /// Single-flighting works on two levels: - /// 1. **Per-recipient** (slot key match): a second tap on the - /// same recipient during the FFI window re-presents the - /// existing controller, never races a duplicate FFI call. - /// 2. **Per-wallet** (new in this revision): if any controller - /// on the same wallet is `.inFlight` for a *different* - /// recipient, reject with + /// 1. **Per-operation** (slot key + `operationId` match): a + /// second tap on the same operation during the FFI window + /// re-presents the existing controller, never races a + /// duplicate FFI call. The operation id (the resumed lock's + /// outpoint, or a unique per-submission fresh-shield id) is + /// required because resumable locks normally default to the + /// same wallet-owned shielded recipient — two different locks + /// share one slot key, and reusing the first lock's controller + /// would silently drop the second lock's resume body. A + /// DIFFERENT operation is `.blockedByOtherWalletFunding` while + /// the slot is in flight, and replaces the retained controller + /// once it has completed. Fresh shields mint a NEW id per + /// submission for the same reason: a fixed marker would match + /// the retained completed controller and suppress the next + /// fresh shield's body. + /// 2. **Per-wallet**: if any controller on the same wallet is + /// `.inFlight` for a *different* recipient, reject with /// `.blockedByOtherWalletFunding`. Mirrors the Rust-side /// `shield_guard` mutex on `PlatformWallet` that /// serializes shield-class operations. func startFunding( walletId: Data, recipientRaw43: Data, + operationId: String, body: @escaping () async throws -> Void ) -> StartFundingResult { let key = SlotKey(walletId: walletId, recipientRaw43: recipientRaw43) if let existing = controllers[key] { switch existing.phase { - case .inFlight, .completed: - // Active or just-completed — don't re-enter. - // Returning the existing controller lets the caller - // bind to its progress / terminal state without - // disrupting it. - return .started(existing) + case .inFlight: + if existing.operationId == operationId { + // Re-tap of the same operation — bind to its progress. + return .started(existing) + } + // A different operation while one is running on this + // wallet: same serialization verdict as a different + // recipient — the caller gets the blocker, not a + // controller that will never run its body. + return .blockedByOtherWalletFunding(existing) + case .completed: + if existing.operationId == operationId { + // Re-tap after success — re-present the terminal + // state without re-entering. + return .started(existing) + } + // Completed slot + different operation: a fresh start, + // not a re-tap. Fall through past the per-wallet check + // below and REPLACE the retained controller (its sweep + // is identity-guarded, so the old retention timer can't + // evict the replacement). case .idle, .failed: // Legitimate restart paths. We've already checked // the same-recipient slot here; before letting the @@ -128,7 +155,7 @@ final class ShieldedFundFromAssetLockCoordinator: ObservableObject { ) { return .blockedByOtherWalletFunding(blocker) } - existing.submit(body: body) + existing.submit(operationId: operationId, body: body) // Spawn a fresh retention sweep. The original sweep // exited the moment the first attempt hit `.failed` // (see `scheduleRetentionSweep`'s `.failed: return` @@ -157,7 +184,7 @@ final class ShieldedFundFromAssetLockCoordinator: ObservableObject { recipientRaw43: recipientRaw43 ) controllers[key] = controller - controller.submit(body: body) + controller.submit(operationId: operationId, body: body) scheduleRetentionSweep(key: key, controller: controller) return .started(controller) } @@ -203,6 +230,15 @@ final class ShieldedFundFromAssetLockCoordinator: ObservableObject { guard let controller = controller else { return } var completedAt: Date? while !Task.isCancelled { + // The slot may have been handed to a REPLACEMENT + // controller (a different operation started during this + // controller's completed-retention window). This sweep + // then owns nothing: exit without touching the slot, + // leaving the replacement's own sweep in charge. + let ownsSlot = await MainActor.run { + self?.controllers[key] === controller + } + guard ownsSlot == true else { return } let phase = await MainActor.run { controller.phase } switch phase { case .completed: diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/PersistentAssetLockDisplay.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/PersistentAssetLockDisplay.swift index 9c51496a217..c6a8c78e591 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/PersistentAssetLockDisplay.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/PersistentAssetLockDisplay.swift @@ -26,7 +26,18 @@ extension AssetLockResumeRow { /// Platform layer. Consumed (4) was already used to fund an /// identity and cannot be reused; the persisted row survives /// only for historical lookup. - var canFundIdentity: Bool { statusRaw == 2 || statusRaw == 3 } + /// + /// RecoveredFromChain (5) also qualifies: the restore scan and the + /// chainlock-promotion path attach a real `ChainAssetLockProof` + /// before writing that status, so the lock is exactly as fundable + /// as a ChainLocked (3) one. What is unknown for a `5` is whether + /// Platform already consumed it — and Platform, not the client, is + /// the arbiter of that: it rejects an already-spent outpoint with a + /// typed error. A user-driven Resume is the surface allowed to ask. + /// (Do not feed `5` into an automatic retry sweep — blind retries + /// of historical locks are the failure mode the status exists to + /// prevent.) + var canFundIdentity: Bool { statusRaw == 2 || statusRaw == 3 || statusRaw == 5 } /// `true` when the lock should be surfaced on the Resumable /// Registrations section at all. Lower bar than `canFundIdentity` @@ -35,22 +46,31 @@ extension AssetLockResumeRow { /// in-flight crash-recovery situation has visible continuity /// through the IS-lock arrival. /// - /// Upper bound at `3` (ChainLocked) is load-bearing: status `4` - /// (Consumed) is the terminal state for a lock that already - /// funded an identity, and the underlying `resume_asset_lock` - /// rejects Consumed entries with "already Consumed — nothing - /// to resume". Without the upper bound, the "Delete identity - /// locally" action — which removes `PersistentIdentity` but - /// leaves the `PersistentAssetLock` alive — frees the - /// `(walletId, identityIndex)` slot in the anti-join and the - /// Consumed row would re-surface as a perpetual-spinner row - /// that can't be advanced. - var isVisibleAsResumable: Bool { statusRaw >= 1 && statusRaw <= 3 } + /// Excluding `4` (Consumed) is load-bearing: it is the terminal + /// state for a lock that already funded an identity, and the + /// underlying `resume_asset_lock` rejects Consumed entries with + /// "already Consumed — nothing to resume". Without that exclusion, + /// the "Delete identity locally" action — which removes + /// `PersistentIdentity` but leaves the `PersistentAssetLock` alive + /// — frees the `(walletId, identityIndex)` slot in the anti-join + /// and the Consumed row would re-surface as a perpetual-spinner + /// row that can't be advanced. + /// + /// `5` (RecoveredFromChain) IS included. It sits above the terminal + /// `4` numerically but is not terminal: it is what the restore scan + /// and the chainlock-promotion path write for a lock with proven + /// Core finality and unknown Platform-side consumption. Treating + /// this as a contiguous `1...3` range silently hid every such row, + /// which is the whole visible symptom — a chain-locked top-up the + /// user funded appears nowhere and reads as lost funds. + var isVisibleAsResumable: Bool { + (statusRaw >= 1 && statusRaw <= 3) || statusRaw == 5 + } } /// Human-readable label for `PersistentAssetLock.statusRaw`. Kept /// here (rather than as a `case` block re-implemented in every -/// view) so the 0/1/2/3 → label mapping has one home. Mirrors the +/// view) so the 0...5 → label mapping has one home. Mirrors the /// Rust-side `AssetLockStatus` enum. extension PersistentAssetLock { var statusLabel: String { @@ -60,6 +80,9 @@ extension PersistentAssetLock { case 2: return "InstantSendLocked" case 3: return "ChainLocked" case 4: return "Consumed" + // Core finality proven, Platform-side consumption unknown. + // Rendered as "Unknown(5)" before this case existed. + case 5: return "RecoveredFromChain" default: return "Unknown(\(statusRaw))" } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingPlatformFundFromAssetLocksList.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingPlatformFundFromAssetLocksList.swift index 1bc59ca9478..72d08fdba21 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingPlatformFundFromAssetLocksList.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingPlatformFundFromAssetLocksList.swift @@ -7,10 +7,17 @@ // // 1. In-flight controllers from `AddressFundFromAssetLockCoordinator` — the // live submit-still-running case. -// 2. Orphaned `PersistentAssetLock` rows with -// `fundingTypeRaw == AssetLockAddressTopUp` (4) and -// `statusRaw ∈ [1, 3]` — the crash-recovery case where the user -// killed the app between asset-lock broadcast and ST submission. +// 2. Orphaned `PersistentAssetLock` rows on either top-up funding type — +// `AssetLockAddressTopUp` (4) or `AssetLockShieldedAddressTopUp` (5) — +// at a recoverable status (`isVisibleAsResumable`, i.e. +// `[1, 3] ∪ {5}`): the crash-recovery case where the user killed the +// app between asset-lock broadcast and ST submission. +// +// Both top-up funding types belong here because neither has any other +// recovery home: the identity-side surfaces admit only funding types +// `0...2`, and `3` is an invitation voucher owned by the reclaim flow. +// Filtering to `4` alone left every shielded top-up — stalled or +// RecoveredFromChain — invisible on every surface in the app. // // Anti-join: an orphaned lock is hidden if its outpoint is already // claimed by an in-flight controller. (We index by outpoint here @@ -63,11 +70,11 @@ struct PendingPlatformFundFromAssetLocksList: View { let orphans = resumableLocks(excludingControllerOutpoints: Set(walletControllers.compactMap { _ in // Controllers don't currently store the outpoint of the // asset lock they're driving. The de-dupe set therefore - // never has entries today — but the SwiftData status - // filter (`>=1, <=3`) already excludes locks that have - // been Consumed, so an in-flight controller whose lock - // is mid-transition lands at status 2/3 and would only - // briefly co-render. The plumbing is here so a future + // never has entries today — but the status filter + // (`isVisibleAsResumable`) already excludes locks that + // have been Consumed, so an in-flight controller whose + // lock is mid-transition lands at status 2/3 and would + // only briefly co-render. The plumbing is here so a future // tweak (controller exposes its outpoint after broadcast) // can de-dupe by returning a non-nil here. nil @@ -121,18 +128,44 @@ struct PendingPlatformFundFromAssetLocksList: View { coordinator.activeControllers().filter { $0.walletId == walletId } } - /// Resumable asset-lock rows for this wallet — fundingType 4 - /// (AssetLockAddressTopUp) and status in 1..3 (Broadcast through - /// ChainLocked, excluding Consumed). Excludes outpoints already - /// owned by an in-flight controller. + /// Resumable asset-lock rows for this wallet — either top-up funding + /// type at a recoverable status. Excludes outpoints already owned by + /// an in-flight controller. private func resumableLocks( excludingControllerOutpoints excluded: Set ) -> [PersistentAssetLock] { assetLocks - .filter { $0.fundingTypeRaw == 4 } - .filter { $0.isVisibleAsResumable } + .filter { Self.isResumableTopUp($0) } .filter { !excluded.contains($0.outPointHex) } } + + /// Funding-type + status predicate for this surface, extracted as a + /// pure `static` (generic over `AssetLockResumeRow`) so it is + /// unit-testable without a SwiftData container — same shape as + /// `IdentitiesContentView.crossWalletResumableLocks`. + /// + /// Admits BOTH top-up funding types: + /// + /// * `4` AssetLockAddressTopUp — resumes into + /// `FundFromAssetLockPlatformAddressView`. + /// * `5` AssetLockShieldedAddressTopUp — resumes into + /// `ShieldedFundFromAssetLockView`. + /// + /// `5` was previously filtered out here. Because the identity + /// surfaces admit only `0...2` and the reclaim flow owns `3`, that + /// left a stalled or RecoveredFromChain shielded top-up on NO surface + /// in the app — real recoverable value with no way to reach it. The + /// status half is delegated to `isVisibleAsResumable`, which excludes + /// the terminal Consumed (`4`) by name while keeping + /// RecoveredFromChain (`5`). + /// + /// Identity-family types are rejected fail-closed: they have their own + /// resume surface, and dispatching one here would offer a resume flow + /// that submits the wrong transition for it. + nonisolated static func isResumableTopUp(_ lock: R) -> Bool { + guard lock.fundingTypeRaw == 4 || lock.fundingTypeRaw == 5 else { return false } + return lock.isVisibleAsResumable + } } private extension AddressFundFromAssetLockController { @@ -293,13 +326,13 @@ struct ResumablePlatformFundFromAssetLockRow: View { waitingIndicator } else { // Nothing is driving a fund on this wallet, so every visible row - // (statusRaw 1/2/3, per the list's `isVisibleAsResumable` + // (statusRaw 1/2/3/5, per the list's `isVisibleAsResumable` // filter) is a genuinely-resumable orphan with no driver: a - // proof-ready (`canFundIdentity`, statusRaw 2/3) lock submits as - // soon as a recipient is picked; a Broadcast (statusRaw 1) lock - // re-enters the (now unbounded) finality wait via - // `resume_asset_lock`. Without this the Broadcast case was a - // permanent "Waiting…" dead end. + // proof-ready (`canFundIdentity`, statusRaw 2/3/5) lock submits + // as soon as a recipient is picked; a Broadcast (statusRaw 1) + // lock re-enters the finality wait via `resume_asset_lock`. + // Without this the Broadcast case was a permanent "Waiting…" + // dead end. resumeButton } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ShieldedFundFromAssetLockView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ShieldedFundFromAssetLockView.swift index b29c094ef01..800e2bd2e6a 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ShieldedFundFromAssetLockView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ShieldedFundFromAssetLockView.swift @@ -658,6 +658,18 @@ struct ShieldedFundFromAssetLockView: View { let walletId = wallet.walletId let manager = walletManager + // The operation id carries the resumed lock's outpoint: resumable + // locks default to the same wallet-owned shielded recipient, so + // the coordinator's slot key alone cannot tell two locks apart + // and would silently reuse the first lock's controller. + // A fresh shield mints a NEW id per submission: the coordinator + // retains a completed controller for ~30s, and a fixed marker + // would match it and rebind — reopening the old result instead + // of running this submission's body. Minted here, at submission + // time, so dismissing a completed sheet and shielding again is + // always a new operation ("resume:" stays stable so a + // re-tap of the SAME lock still rebinds). + let operationId: String let body: () async throws -> Void if let lock = resumeFromLock { guard let parsed = parseOutPoint(lock.outPointHex) else { @@ -666,6 +678,7 @@ struct ShieldedFundFromAssetLockView: View { ) return } + operationId = "resume:\(lock.outPointHex)" body = { try await manager.shieldedResumeFundFromAssetLock( walletId: walletId, @@ -681,6 +694,7 @@ struct ShieldedFundFromAssetLockView: View { let fundingAccountIndex = fundingCoreAccountIndex, let duffs = parsedDuffs else { return } + operationId = "shield:\(UUID().uuidString)" body = { try await manager.shieldedFundFromAssetLock( walletId: walletId, @@ -694,17 +708,20 @@ struct ShieldedFundFromAssetLockView: View { } // Single-flight gate via the coordinator. Two levels: - // - Same recipient + in-flight: returns the existing - // controller (the user sees the same progress view). - // - Different recipient but another shielded funding in - // flight on this wallet: surfaces a typed "wait" - // error pointing at the in-flight recipient. Mirrors - // the Rust-side `shield_guard` mutex that serializes - // all shield-class ops per wallet. + // - Same operation (same recipient + operation id) + + // in-flight: returns the existing controller (the user + // sees the same progress view). + // - Any OTHER shielded funding in flight on this wallet — + // different recipient, or a different lock/fresh shield + // on the same recipient: surfaces a typed "wait" error + // pointing at the in-flight blocker. Mirrors the + // Rust-side `shield_guard` mutex that serializes all + // shield-class ops per wallet. let coordinator = walletManager.shieldedFundFromAssetLockCoordinator switch coordinator.startFunding( walletId: walletId, recipientRaw43: recipient, + operationId: operationId, body: body ) { case .started(let controller): diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift index fffc94c2c49..5514b577eb6 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift @@ -100,16 +100,38 @@ final class CreateIdentityResumableTests: XCTestCase { XCTAssertEqual(result, locks) } + /// Regression: `statusRaw == 5` (RecoveredFromChain) must be + /// ACCEPTED. It is what the restore scan and the chainlock-promotion + /// path write for a lock whose Core finality is proven (a real + /// `ChainAssetLockProof` is attached) but whose Platform-side + /// consumption is unknown. A user-driven Resume is exactly the + /// surface allowed to try consuming one — Platform is the arbiter + /// and rejects an already-spent outpoint with a typed error. + /// + /// The filter used to be the contiguous range `1...3`, which hid + /// every recovered lock: a chain-locked top-up the user had really + /// funded appeared on no surface at all and read as lost funds. + func testRecoveredFromChainLocksAreResumable() { + let lock = FakeAssetLockRow(walletId: walletA, statusRaw: 5, identityIndexRaw: 0) + let result = IdentitiesContentView.crossWalletResumableLocks( + in: [lock], + usedSlots: [] + ) + XCTAssertEqual(result, [lock]) + } + /// `statusRaw == 4` (Consumed) is the terminal state for a lock /// that already funded an identity. It must NOT surface on the /// Resumable Registrations list: in the happy path the anti-join /// against `PersistentIdentity` hides it, but the local-only /// delete-identity action removes the identity row WITHOUT the /// asset-lock row, which frees the slot in the anti-join. Without - /// the upper bound (`<= 3`) on the filter, a Consumed lock would - /// re-surface as a Resume row that can't advance - /// (`resume_asset_lock` rejects Consumed entries with - /// "already Consumed — nothing to resume"). + /// excluding `4`, a Consumed lock would re-surface as a Resume row + /// that can't advance (`resume_asset_lock` rejects Consumed entries + /// with "already Consumed — nothing to resume"). + /// + /// Note this is an exclusion of `4` specifically, NOT an upper + /// bound: `5` sits above it numerically and is resumable. func testConsumedLocksAreHiddenFromResumableList() { let lock = FakeAssetLockRow(walletId: walletA, statusRaw: 4, identityIndexRaw: 0) let result = IdentitiesContentView.crossWalletResumableLocks( diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/PendingPlatformTopUpResumeTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/PendingPlatformTopUpResumeTests.swift new file mode 100644 index 00000000000..159a4ad4fb2 --- /dev/null +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/PendingPlatformTopUpResumeTests.swift @@ -0,0 +1,126 @@ +import XCTest +import SwiftDashSDK +@testable import SwiftExampleApp + +/// Tests the pure funding-type + status predicate behind the wallet +/// detail screen's "Pending Platform Top Ups" surface. +/// +/// This is the only surface in the app that can recover an orphaned +/// TOP-UP asset lock. The identity-side +/// `IdentitiesContentView.crossWalletResumableLocks` admits only +/// funding types `0...2`, and `3` (IdentityInvitation) is a bearer +/// voucher owned by the reclaim flow — so a lock at funding type `4` +/// or `5` that this filter rejects is unreachable from anywhere in the +/// app, and reads to the user as lost funds. +/// +/// Two invariants can silently regress: +/// +/// 1. Funding type `5` (AssetLockShieldedAddressTopUp) must be +/// admitted. The filter was `fundingTypeRaw == 4` alone, which +/// hid every shielded top-up — stalled or RecoveredFromChain — +/// on every surface. Widening the STATUS predicate did not help: +/// a row excluded on the funding-type axis stays excluded +/// however generous the status axis is. +/// 2. The status half must keep excluding the terminal Consumed +/// (`4`) while admitting RecoveredFromChain (`5`) — delegated to +/// `isVisibleAsResumable`, and asserted here because this +/// surface is the one that composes the two axes. +final class PendingPlatformTopUpResumeTests: XCTestCase { + + private struct FakeAssetLockRow: AssetLockResumeRow, Equatable { + let walletId: Data + let statusRaw: Int + var identityIndexRaw: Int32 = 0 + let fundingTypeRaw: Int + } + + private let wallet = Data(repeating: 0xA1, count: 8) + + private func row(status: Int, fundingType: Int) -> FakeAssetLockRow { + FakeAssetLockRow(walletId: wallet, statusRaw: status, fundingTypeRaw: fundingType) + } + + // MARK: - funding-type axis + + /// The regression: a shielded top-up must be admitted, at every + /// recoverable status. + func testAdmitsShieldedAddressTopUps() { + for status in [1, 2, 3, 5] { + XCTAssertTrue( + PendingPlatformFundFromAssetLocksList.isResumableTopUp( + row(status: status, fundingType: 5) + ), + "shielded top-up at status \(status) must be resumable" + ) + } + } + + /// Address top-ups are unaffected by the widening. + func testAdmitsAddressTopUps() { + for status in [1, 2, 3, 5] { + XCTAssertTrue( + PendingPlatformFundFromAssetLocksList.isResumableTopUp( + row(status: status, fundingType: 4) + ), + "address top-up at status \(status) must be resumable" + ) + } + } + + /// Identity-family funding types recover on the identity screens. + /// Admitting one here would offer a resume flow that submits the + /// wrong transition against it. + func testRejectsIdentityFamilyAndUnknownFundingTypes() { + for fundingType in [0, 1, 2, 3, 6, 99] { + XCTAssertFalse( + PendingPlatformFundFromAssetLocksList.isResumableTopUp( + row(status: 2, fundingType: fundingType) + ), + "fundingTypeRaw \(fundingType) must not reach the top-up surface" + ) + } + } + + // MARK: - status axis + + /// Built (0) has not been broadcast; Consumed (4) is the terminal + /// tombstone `resume_asset_lock` rejects outright. Both stay out on + /// either funding type. + func testRejectsBuiltAndConsumedOnBothTopUpTypes() { + for fundingType in [4, 5] { + for status in [0, 4] { + XCTAssertFalse( + PendingPlatformFundFromAssetLocksList.isResumableTopUp( + row(status: status, fundingType: fundingType) + ), + "status \(status) on fundingType \(fundingType) must stay hidden" + ) + } + } + } + + /// `4` is excluded by NAME, not by an upper bound: `5` sits above + /// it numerically and is resumable. A `statusRaw <= 3` bound reads + /// as equivalent and silently drops every restored lock. + func testConsumedExclusionIsNotAnUpperBound() { + XCTAssertFalse( + PendingPlatformFundFromAssetLocksList.isResumableTopUp( + row(status: 4, fundingType: 5) + ) + ) + XCTAssertTrue( + PendingPlatformFundFromAssetLocksList.isResumableTopUp( + row(status: 5, fundingType: 5) + ) + ) + } + + /// RecoveredFromChain carries a real `ChainAssetLockProof`, so it is + /// as fundable as a ChainLocked (3) lock — the row must offer Resume + /// rather than the "waiting for finality" indicator. + func testRecoveredFromChainIsFundableNotWaiting() { + XCTAssertTrue(row(status: 5, fundingType: 5).canFundIdentity) + XCTAssertTrue(row(status: 5, fundingType: 4).canFundIdentity) + XCTAssertFalse(row(status: 1, fundingType: 5).canFundIdentity) + } +}